From 828ddd4e4fcf1c8b2808b11adaf49ff39d92303c Mon Sep 17 00:00:00 2001 From: Alexander Marshalov <_@marshalov.org> Date: Wed, 1 Nov 2023 20:59:46 +0100 Subject: [PATCH] =?UTF-8?q?vmauth:=20add=20browser=20authorization=20reque?= =?UTF-8?q?st=20for=20http=20requests=20without=E2=80=A6=20(#5234)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * vmauth: add browser authorization request for http requests without credentials to a route that is not in the `unauthorized_user` section (when `unauthorized_user` is specified). * add link to issue in CHANGELOG * Extend vmauth docs * wip --------- Co-authored-by: Aliaksandr Valialkin --- app/vmauth/README.md | 4 ++++ app/vmauth/auth_config.go | 12 ++++++++++++ app/vmauth/example_config.yml | 2 ++ app/vmauth/example_config_ent.yml | 2 ++ app/vmauth/main.go | 10 +++++++++- docs/CHANGELOG.md | 1 + docs/vmauth.md | 4 ++++ 7 files changed, 34 insertions(+), 1 deletion(-) diff --git a/app/vmauth/README.md b/app/vmauth/README.md index 3855568ad..bc366a797 100644 --- a/app/vmauth/README.md +++ b/app/vmauth/README.md @@ -222,6 +222,8 @@ users: # For example, request to http://vmauth:8427/non/existing/path are proxied: # - to http://default1:8888/unsupported_url_handler?request_path=/non/existing/path # - or http://default2:8888/unsupported_url_handler?request_path=/non/existing/path + # + # Regular expressions are allowed in `src_paths` entries. - username: "foobar" url_map: - src_paths: @@ -248,6 +250,8 @@ users: # Requests are routed in round-robin fashion between `url_prefix` backends. # The deny_partial_response query arg is added to all the routed requests. # The requests are re-tried if url_prefix backends send 500 or 503 response status codes. +# Note that the unauthorized_user section takes precedence when processing a route without credentials, +# even if such a route also exists in the users section (see https://github.com/VictoriaMetrics/VictoriaMetrics/issues/5236). unauthorized_user: url_prefix: - http://vmselect-az1/?deny_partial_response=1 diff --git a/app/vmauth/auth_config.go b/app/vmauth/auth_config.go index fbb9bb414..ab67bc498 100644 --- a/app/vmauth/auth_config.go +++ b/app/vmauth/auth_config.go @@ -420,6 +420,18 @@ func parseAuthConfig(data []byte) (*AuthConfig, error) { } ui := ac.UnauthorizedUser if ui != nil { + if ui.Username != "" { + return nil, fmt.Errorf("field username can't be specified for unauthorized_user section") + } + if ui.Password != "" { + return nil, fmt.Errorf("field password can't be specified for unauthorized_user section") + } + if ui.BearerToken != "" { + return nil, fmt.Errorf("field bearer_token can't be specified for unauthorized_user section") + } + if ui.Name != "" { + return nil, fmt.Errorf("field name can't be specified for unauthorized_user section") + } ui.requests = metrics.GetOrCreateCounter(`vmauth_unauthorized_user_requests_total`) ui.requestsDuration = metrics.GetOrCreateSummary(`vmauth_unauthorized_user_request_duration_seconds`) ui.concurrencyLimitCh = make(chan struct{}, ui.getMaxConcurrentRequests()) diff --git a/app/vmauth/example_config.yml b/app/vmauth/example_config.yml index b21771c03..023adb991 100644 --- a/app/vmauth/example_config.yml +++ b/app/vmauth/example_config.yml @@ -82,6 +82,8 @@ users: # For example, request to http://vmauth:8427/non/existing/path are proxied: # - to http://default1:8888/unsupported_url_handler?request_path=/non/existing/path # - or http://default2:8888/unsupported_url_handler?request_path=/non/existing/path + # + # Regular expressions are allowed in `src_paths` entries. - username: "foobar" url_map: - src_paths: diff --git a/app/vmauth/example_config_ent.yml b/app/vmauth/example_config_ent.yml index ed9ae26e8..36e646e1e 100644 --- a/app/vmauth/example_config_ent.yml +++ b/app/vmauth/example_config_ent.yml @@ -20,6 +20,8 @@ users: # For example, request to http://vmauth:8427/non/existing/path are proxied: # - to http://default1:8888/unsupported_url_handler?request_path=/non/existing/path # - or http://default2:8888/unsupported_url_handler?request_path=/non/existing/path + # + # Regular expressions are allowed in `src_paths` entries. - username: "foobar" url_map: - src_paths: diff --git a/app/vmauth/main.go b/app/vmauth/main.go index 9e50366de..709f83f02 100644 --- a/app/vmauth/main.go +++ b/app/vmauth/main.go @@ -158,8 +158,16 @@ func processRequest(w http.ResponseWriter, r *http.Request, ui *UserInfo) { up, hc, retryStatusCodes := ui.getURLPrefixAndHeaders(u) isDefault := false if up == nil { - missingRouteRequests.Inc() if ui.DefaultURL == nil { + // Authorization should be requested for http requests without credentials + // to a route that is not in the configuration for unauthorized user. + // See https://github.com/VictoriaMetrics/VictoriaMetrics/issues/5236 + if ui.BearerToken == "" && ui.Username == "" && len(*authUsers.Load()) > 0 { + w.Header().Set("WWW-Authenticate", `Basic realm="Restricted"`) + http.Error(w, "missing `Authorization` request header", http.StatusUnauthorized) + return + } + missingRouteRequests.Inc() httpserver.Errorf(w, r, "missing route for %q", u.String()) return } diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 74bae86aa..7868e9409 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -90,6 +90,7 @@ The sandbox cluster installation is running under the constant load generated by * BUGFIX: [vmagent](https://docs.victoriametrics.com/vmagent.html): do not print redundant error logs when failed to scrape consul or nomad target. See [this pull request](https://github.com/VictoriaMetrics/VictoriaMetrics/pull/5239). * BUGFIX: [vmstorage](https://docs.victoriametrics.com/Cluster-VictoriaMetrics.html): prevent deleted series to be searchable via `/api/v1/series` API if they were re-ingested with staleness markers. This situation could happen if user deletes the series from the target and from VM, and then vmagent sends stale markers for absent series. Thanks to @ilyatrefilov for the [issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/5069) and [pull request](https://github.com/VictoriaMetrics/VictoriaMetrics/pull/5174). * BUGFIX: [vmstorage](https://docs.victoriametrics.com/Cluster-VictoriaMetrics.html): log warning about switching to ReadOnly mode only on state change. Before, vmstorage would log this warning every 1s. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/5159) for details. +* BUGFIX: [vmauth](https://docs.victoriametrics.com/vmauth.html): show browser authorization window for unauthorized requests to unsupported paths if the `unauthorized_user` section is specified. This allows properly authorizing the user. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/5236) for details. * BUGFIX: [vmui](https://docs.victoriametrics.com/#vmui): fix the `Disable cache` toggle at `JSON` and `Table` views. Previously response caching was always enabled and couldn't be disabled at these views. ## [v1.94.0](https://github.com/VictoriaMetrics/VictoriaMetrics/releases/tag/v1.94.0) diff --git a/docs/vmauth.md b/docs/vmauth.md index 7ee675c58..9e68019f8 100644 --- a/docs/vmauth.md +++ b/docs/vmauth.md @@ -233,6 +233,8 @@ users: # For example, request to http://vmauth:8427/non/existing/path are proxied: # - to http://default1:8888/unsupported_url_handler?request_path=/non/existing/path # - or http://default2:8888/unsupported_url_handler?request_path=/non/existing/path + # + # Regular expressions are allowed in `src_paths` entries. - username: "foobar" url_map: - src_paths: @@ -259,6 +261,8 @@ users: # Requests are routed in round-robin fashion between `url_prefix` backends. # The deny_partial_response query arg is added to all the routed requests. # The requests are re-tried if url_prefix backends send 500 or 503 response status codes. +# Note that the unauthorized_user section takes precedence when processing a route without credentials, +# even if such a route also exists in the users section (see https://github.com/VictoriaMetrics/VictoriaMetrics/issues/5236). unauthorized_user: url_prefix: - http://vmselect-az1/?deny_partial_response=1