From 5d8596865999f3eca4547f0fdadf80c79473efb2 Mon Sep 17 00:00:00 2001 From: andriibeee <154226341+andriibeee@users.noreply.github.com> Date: Fri, 15 Nov 2024 13:28:25 +0200 Subject: [PATCH] app/vmauth: fix unauthorized_user routing inconsistency This commit makes vmauth respect the routing config for unauthorized requests for requests that despite having Authorization header failed to authorize successfully. It covers the following use-cases: - vmauth is used at load-balanacer and must forward requests as is. There is no any authorization configs. - vmauth has authorization config, but it must forward requests with invalid credential tokens to some other backend. related issue: https://github.com/VictoriaMetrics/VictoriaMetrics/issues/7543 --------- Signed-off-by: Andrii --- app/vmauth/main.go | 6 ++++++ app/vmauth/main_test.go | 14 ++++++++++++++ docs/changelog/CHANGELOG.md | 1 + 3 files changed, 21 insertions(+) diff --git a/app/vmauth/main.go b/app/vmauth/main.go index 466147bf2..bcf296b4f 100644 --- a/app/vmauth/main.go +++ b/app/vmauth/main.go @@ -123,6 +123,12 @@ func requestHandler(w http.ResponseWriter, r *http.Request) bool { ui := getUserInfoByAuthTokens(ats) if ui == nil { + uu := authConfig.Load().UnauthorizedUser + if uu != nil { + processUserRequest(w, r, uu) + return true + } + invalidAuthTokenRequests.Inc() if *logInvalidAuthTokens { err := fmt.Errorf("cannot authorize request with auth tokens %q", ats) diff --git a/app/vmauth/main_test.go b/app/vmauth/main_test.go index 6414ef803..9078de8a4 100644 --- a/app/vmauth/main_test.go +++ b/app/vmauth/main_test.go @@ -90,6 +90,20 @@ User-Agent: vmauth X-Forwarded-For: 12.34.56.78, 42.2.3.84` f(cfgStr, requestURL, backendHandler, responseExpected) + // routing of all failed to authorize requests to unauthorized_user (issue #7543) + cfgStr = ` +unauthorized_user: + url_prefix: "{BACKEND}/foo" + keep_original_host: true` + requestURL = "http://foo:invalid-secret@some-host.com/abc/def" + backendHandler = func(w http.ResponseWriter, r *http.Request) { + fmt.Fprintf(w, "requested_url=http://%s%s", r.Host, r.URL) + } + responseExpected = ` +statusCode=200 +requested_url=http://some-host.com/foo/abc/def` + f(cfgStr, requestURL, backendHandler, responseExpected) + // keep_original_host cfgStr = ` unauthorized_user: diff --git a/docs/changelog/CHANGELOG.md b/docs/changelog/CHANGELOG.md index 315e3da9e..03039e448 100644 --- a/docs/changelog/CHANGELOG.md +++ b/docs/changelog/CHANGELOG.md @@ -20,6 +20,7 @@ See also [LTS releases](https://docs.victoriametrics.com/lts-releases/). * SECURITY: upgrade Go builder from Go1.23.1 to Go1.23.3. See the list of issues addressed in [Go1.23.2](https://github.com/golang/go/issues?q=milestone%3AGo1.23.2+label%3ACherryPickApproved) and [Go1.23.3](https://github.com/golang/go/issues?q=milestone%3AGo1.23.3+label%3ACherryPickApproved). +* BUGFIX: [vmauth](https://docs.victoriametrics.com/vmauth/): fixed unauthorized routing behavior inconsistency. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/7543) for details. * BUGFIX: [vmctl](https://docs.victoriametrics.com/vmctl/): drop rows that do not belong to the current series during import. The dropped rows should belong to another series whose tags are a superset of the current series. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/7301) and [this pull request](https://github.com/VictoriaMetrics/VictoriaMetrics/pull/7330). Thanks to @dpedu for reporting and cooperating with the test. * BUGFIX: [vmsingle](https://docs.victoriametrics.com/single-server-victoriametrics/), `vmselect` in [VictoriaMetrics cluster](https://docs.victoriametrics.com/cluster-victoriametrics/): keep the order of resulting time series when `limit_offset` is applied. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/7068). * BUGFIX: [graphite](https://docs.victoriametrics.com/#graphite-render-api-usage): properly handle xFilesFactor=0 for `transformRemoveEmptySeries` function. See [this PR](https://github.com/VictoriaMetrics/VictoriaMetrics/pull/7337) for details.