From 04a6324162bb61609b781b60613f4c808db91435 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 <valyala@victoriametrics.com>
---
 app/vmauth/README.md              |  7 +++++++
 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                    |  7 +++++++
 7 files changed, 40 insertions(+), 1 deletion(-)

diff --git a/app/vmauth/README.md b/app/vmauth/README.md
index babaa7872f..840acf6226 100644
--- a/app/vmauth/README.md
+++ b/app/vmauth/README.md
@@ -181,6 +181,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:
@@ -201,6 +203,11 @@ users:
   - "http://default2:8888/unsupported_url_handler"
 
 # Requests without Authorization header are routed according to `unauthorized_user` section.
+# 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_map:
   - src_paths:
diff --git a/app/vmauth/auth_config.go b/app/vmauth/auth_config.go
index 6fab91dc27..9568d2498f 100644
--- a/app/vmauth/auth_config.go
+++ b/app/vmauth/auth_config.go
@@ -378,6 +378,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 e7721be0ea..e46808cd00 100644
--- a/app/vmauth/example_config.yml
+++ b/app/vmauth/example_config.yml
@@ -76,6 +76,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 ac0880a4ee..8afe045411 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 925bfb7c60..36a5301dd3 100644
--- a/app/vmauth/main.go
+++ b/app/vmauth/main.go
@@ -154,8 +154,16 @@ func processRequest(w http.ResponseWriter, r *http.Request, ui *UserInfo) {
 	up, headers := 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 cc635cf88b..2e832746c1 100644
--- a/docs/CHANGELOG.md
+++ b/docs/CHANGELOG.md
@@ -17,6 +17,7 @@ The following `tip` changes can be tested by building VictoriaMetrics components
 * 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.
 
 ## [v1.93.6](https://github.com/VictoriaMetrics/VictoriaMetrics/releases/tag/v1.93.6)
 
diff --git a/docs/vmauth.md b/docs/vmauth.md
index b39ef1b5be..fa54c63741 100644
--- a/docs/vmauth.md
+++ b/docs/vmauth.md
@@ -192,6 +192,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:
@@ -212,6 +214,11 @@ users:
   - "http://default2:8888/unsupported_url_handler"
 
 # Requests without Authorization header are routed according to `unauthorized_user` section.
+# 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_map:
   - src_paths: