lib/httpserver: properly check basic authorization

Commit 68791f9ccc introduced regression.
It performed basicAuth check before built-in routes. It made impossible
to bypass basic authorization with `authKey` param.

This commit fixeds that issue and removes unneeded check. It also adds
integration tests for this case.

 Related issue:
https://github.com/VictoriaMetrics/VictoriaMetrics/issues/7345

---------

Signed-off-by: f41gh7 <nik@victoriametrics.com>

apptest/tests/vmauth_routing_test.go

@@ -180,3 +180,124 @@ unauthorized_user:
 	makeGetRequestExpectCode(fmt.Sprintf("http://127.0.0.1:%s/flags", listenPortPrivate), http.StatusUnauthorized)
 	assertBackendRequestsCount(1)
 }
+
+func TestSingleVMAuthHTTPServerAuthKeys(t *testing.T) {
+	tc := apptest.NewTestCase(t)
+	defer tc.Stop()
+
+	var authorizedRequestsCount int
+	backend := httptest.NewServer(http.HandlerFunc(func(_ http.ResponseWriter, _ *http.Request) {
+		authorizedRequestsCount++
+	}))
+	defer backend.Close()
+
+	authConfig := fmt.Sprintf(`
+unauthorized_user:
+   url_map:
+   - src_paths:
+     - /backend/health
+     - /backend/ready
+     url_prefix: %s
+  `, backend.URL)
+
+	const (
+		authKey  = "key"
+		username = "user"
+		password = "password"
+	)
+	flags := []string{
+		"--reloadAuthKey=" + authKey,
+		"--pprofAuthKey=" + authKey,
+		"--metricsAuthKey=" + authKey,
+		"--httpAuth.username=" + username,
+		"--httpAuth.password=" + password}
+	vmauth := tc.MustStartVmauth("vmauth", flags, authConfig)
+
+	makeGetRequestExpectCode := func(prepareRequest func(*http.Request), expectCode int) {
+		t.Helper()
+		req, err := http.NewRequest("GET", fmt.Sprintf("http://%s", vmauth.GetHTTPListenAddr()), nil)
+		if err != nil {
+			t.Fatalf("cannot build http.Request: %s", err)
+		}
+		prepareRequest(req)
+		resp, err := http.DefaultClient.Do(req)
+		if err != nil {
+			t.Fatalf("cannot make http.Get request for target=%q: %s", req.URL, err)
+		}
+		responseText, err := io.ReadAll(resp.Body)
+		if err != nil {
+			t.Fatalf("cannot read response body: %s", err)
+		}
+		resp.Body.Close()
+		if resp.StatusCode != expectCode {
+			t.Fatalf("unexpected http response code: %d, want: %d, response text: %s", resp.StatusCode, expectCode, responseText)
+		}
+	}
+	assertBackendsRequestsCount := func(expectAuthorized int) {
+		t.Helper()
+		if expectAuthorized != authorizedRequestsCount {
+			t.Fatalf("expected to have %d authorized proxied requests, got: %d", expectAuthorized, authorizedRequestsCount)
+		}
+	}
+
+	// authKey overrides basic auth
+	makeGetRequestExpectCode(func(r *http.Request) {
+		r.URL.Path = "/metrics"
+		q := r.URL.Query()
+		q.Add("authKey", authKey)
+		r.URL.RawQuery = q.Encode()
+	}, http.StatusOK)
+	assertBackendsRequestsCount(0)
+
+	// authKey overrides basic auth at pprof handler
+	makeGetRequestExpectCode(func(r *http.Request) {
+		r.URL.Path = "/debug/pprof/heap"
+		q := r.URL.Query()
+		q.Add("authKey", authKey)
+		r.URL.RawQuery = q.Encode()
+	}, http.StatusOK)
+	assertBackendsRequestsCount(0)
+
+	// authKey overrides basic auth at app internal router
+	makeGetRequestExpectCode(func(r *http.Request) {
+		r.URL.Path = "/-/reload"
+		q := r.URL.Query()
+		q.Add("authKey", authKey)
+		r.URL.RawQuery = q.Encode()
+	}, http.StatusOK)
+	assertBackendsRequestsCount(0)
+
+	// no auth request fails
+	makeGetRequestExpectCode(func(r *http.Request) {
+		r.URL.Path = "/metrics"
+	}, http.StatusUnauthorized)
+	assertBackendsRequestsCount(0)
+
+	// no auth request fails at app internal router
+	makeGetRequestExpectCode(func(r *http.Request) {
+		r.URL.Path = "/-/reload"
+	}, http.StatusUnauthorized)
+	assertBackendsRequestsCount(0)
+
+	// basic auth request ok
+	makeGetRequestExpectCode(func(r *http.Request) {
+		r.URL.Path = "/metrics"
+		r.URL.User = url.UserPassword(username, password)
+	}, http.StatusUnauthorized)
+	assertBackendsRequestsCount(0)
+
+	// basic auth request ok at app internal router
+	makeGetRequestExpectCode(func(r *http.Request) {
+		r.URL.Path = "/-/reload"
+		r.URL.User = url.UserPassword(username, password)
+	}, http.StatusUnauthorized)
+	assertBackendsRequestsCount(0)
+
+	// basic auth to backend ok
+	makeGetRequestExpectCode(func(r *http.Request) {
+		r.URL.Path = "/backend/health"
+		r.URL.User = url.UserPassword(username, password)
+	}, http.StatusOK)
+	assertBackendsRequestsCount(1)
+
+}

docs/changelog/CHANGELOG.md

@@ -23,6 +23,7 @@ See also [LTS releases](https://docs.victoriametrics.com/lts-releases/).
 * FEATURE: [vmui](https://docs.victoriametrics.com/#vmui): look back for recent data for longer time intervals when switching to Table or JSON view. The look back window depends on the `step` setting, which is now set as `end - start` (30m) by default. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/8240).
 * FEATURE: [vmui](https://docs.victoriametrics.com/#vmui): preserve user-defined `step` setting when changing the time interval of displayed query. See [this comment](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/8240#issuecomment-2647674065).
 
+* BUGFIX: all the VictoriaMetrics components: properly override basic authorization for API endpoints protected with `authKey`. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/7345#issuecomment-2662595807) for details.
 * BUGFIX: [vmalert](https://docs.victoriametrics.com/vmalert/): fix polluted alert messages when multiple Alertmanager instances are configured with `alert_relabel_configs`. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/8040), and thanks to @evkuzin for [the pull request](https://github.com/VictoriaMetrics/VictoriaMetrics/pull/8258).
 * BUGFIX: `vminsert` in [VictoriaMetrics cluster](https://docs.victoriametrics.com/cluster-victoriametrics/) and `vmstorage` in [VictoriaMetrics cluster](https://docs.victoriametrics.com/cluster-victoriametrics/): properly return parsing error for data ingestion.
 

lib/httpserver/httpserver.go

@@ -386,9 +386,6 @@ func handlerWrapper(w http.ResponseWriter, r *http.Request, rh RequestHandler) {
 }
 
 func builtinRoutesHandler(s *server, r *http.Request, w http.ResponseWriter, rh RequestHandler) bool {
-	if !isProtectedByAuthFlag(r.URL.Path) && !CheckBasicAuth(w, r) {
-		return true
-	}
 
 	h := w.Header()