app/vmauth: remove enforcement of "vm_access" claim

Allow skipping "vm_access" claim validation in order to use claims match based routing. Previously, that required to
modify the token and add an artificial "vm_access: {}" value which is inconvinient.

app/vmauth/example_config.yml

@@ -140,6 +140,16 @@ users:
   - "ProjectID: {{.MetricsProjectID}}"
   url_prefix: "http://vminsert:8480/insert/prometheus"
 
+# JWT-based routing that relies solely on custom claims.
+# The `vm_access` claim is not required - tokens are routed by their custom claims,
+# e.g. {"role": "admin"}.
+- name: jwt-custom-claims
+  jwt:
+    skip_verify: true
+    match_claims:
+      role: admin
+  url_prefix: "http://vmselect-admin:8481/select/0/prometheus"
+
 # Requests without Authorization header are proxied according to `unauthorized_user` section.
 # Requests are proxied in round-robin fashion between `url_prefix` backends.
 # The deny_partial_response query arg is added to all the proxied requests.

app/vmauth/jwt.go

@@ -433,7 +433,6 @@ func validateJWTPlaceholdersForURL(up *URLPrefix, isAllowed bool) error {
 			}
 			if strings.Contains(p, placeholderPrefix) {
 				return fmt.Errorf("invalid placeholder found in URL request path: %q, supported values are: %s", bu.Path, strings.Join(allPlaceholders, ", "))
-
 			}
 		}
 		for param, values := range bu.Query() {
@@ -488,7 +487,6 @@ func hasAnyPlaceholders(u *url.URL) bool {
 				return true
 			}
 		}
-
 	}
 	return false
 }

app/vmauth/main_test.go

@@ -739,6 +739,12 @@ users:
 		"vm_access": map[string]any{},
 	}, false)
 
+	// token without vm_access claim, but with a custom claim usable for routing
+	roleToken := genToken(t, map[string]any{
+		"exp":  time.Now().Add(10 * time.Minute).Unix(),
+		"role": "admin",
+	}, true)
+
 	fullToken := genToken(t, map[string]any{
 		"exp": time.Now().Add(10 * time.Minute).Unix(),
 		"vm_access": map[string]any{
@@ -779,6 +785,23 @@ statusCode=401
 Unauthorized`
 	f(simpleCfgStr, request, responseExpected)
 
+	// token without vm_access claim is accepted when it matches custom claims
+	request = httptest.NewRequest(`GET`, "http://some-host.com/abc", nil)
+	request.Header.Set(`Authorization`, `Bearer `+roleToken)
+	responseExpected = `
+statusCode=200
+path: /foo/abc
+query:
+headers:`
+	f(fmt.Sprintf(`
+users:
+- jwt:
+    public_keys:
+    - %q
+    match_claims:
+      role: admin
+  url_prefix: {BACKEND}/foo`, string(publicKeyPEM)), request, responseExpected)
+
 	// expired token
 	request = httptest.NewRequest(`GET`, "http://some-host.com/abc", nil)
 	request.Header.Set(`Authorization`, `Bearer `+expiredToken)

docs/victoriametrics/CONTRIBUTING.md

@@ -28,7 +28,7 @@ If you like VictoriaMetrics and want to contribute, then it would be great:
 ## Issues
 
 When making a new issue, make sure to create no duplicates. Use GitHub search to find whether similar issues exist already.
-The new issue should be written in English and contain a concise description of the problem and the environment where it exists.
+The new issue should be written in English and contain concise description of the problem and environment where it exists.
 We'd very much prefer to have a specific use-case included in the description, since it could have workaround or alternative solutions.
 
 When looking for an issue to contribute, always prefer working on [bugs](https://github.com/VictoriaMetrics/VictoriaMetrics/issues?q=is%3Aopen+is%3Aissue+label%3Abug)
@@ -48,7 +48,7 @@ We use [labels](https://docs.github.com/en/issues/using-labels-and-milestones-to
 1. `need more info`, assigned to issues that require elaboration from the issue creator.
   For example, if we weren't able to reproduce the reported bug based on the ticket description then we ask additional
   questions which could help to reproduce the issue and add `need more info` label. This label helps other maintainers
-  to understand that this issue wasn't forgotten but waits for the feedback from the user.
+  to understand that this issue wasn't forgotten but waits for the feedback from user.
 1. `completed`, assigned to issues that required code changes and those changes were merged to upstream, but not released yet.
   Once a release is made, maintainers go through all labeled issues, leave a comment about the new release, and close the issue.
 1. `vmui`, assigned to issues related to [vmui](https://docs.victoriametrics.com/victoriametrics/single-server-victoriametrics/#vmui) or [VictoriaLogs webui](https://docs.victoriametrics.com/victorialogs/querying/#web-ui)
@@ -63,31 +63,32 @@ Pull requests requirements:
 1. Don't use `master` branch for making PRs, as it makes it impossible for reviewers to modify the changes.
 1. All commits need to be [signed](https://docs.github.com/en/authentication/managing-commit-signature-verification/signing-commits).
 1. Pull request title should be prefixed with `<dir>/<component>:` to show what component has been changed, i.e. `app/vmalert: fix...`.
-   Pull request description should contain a clear and concise description of what was done, why it is needed and for what purpose.
+   Pull request description should contain clear and concise description of what was done, why it is needed and for what purpose.
    Use clear language, so reviewers can quickly understand the change and its impact.
 1. A link to the issue(s) related to the change, if any. Use `Fixes [issue link]` if the PR resolves the issue, or `Related to [issue link]` for reference.
 1. Tests proving that the change is effective. Tests are expected for non-trivial new functionality or non-trivial modifications.
    Bug fixes must include tests unless a maintainer explicitly agrees otherwise.
-   See [this style guide](https://itnext.io/f-tests-as-a-replacement-for-table-driven-tests-in-go-8814a8b19e9e) for tests. See [this section](#testing) for how to run tests.
+   See [this style guide](https://itnext.io/f-tests-as-a-replacement-for-table-driven-tests-in-go-8814a8b19e9e) for tests.
+   To run tests and code checks locally, execute commands `make test-full` and `make check-all`.
 1. Try to not extend the scope of the pull requests outside the issue, do not make unrelated changes.
-1. Update [docs](https://github.com/VictoriaMetrics/VictoriaMetrics/tree/master/docs) if needed. For example, adding a new flag or changing the behavior of existing flags or features
+1. Update [docs](https://github.com/VictoriaMetrics/VictoriaMetrics/tree/master/docs) if needed. For example, adding a new flag or changing behavior of existing flags or features
    requires reflecting these changes in the documentation. For new features add `{{%/* available_from "#" */%}}` shortcode to the documentation.
    It will be later automatically replaced with an actual release version.
 1. A line in the [changelog](https://docs.victoriametrics.com/victoriametrics/changelog/#tip) mentioning the change and related issue in a way
    that would be clear to other readers even if they don't have the full context.
-1. Avoid modifying code in the `/vendor` folder manually, even when the vendored package originates from the VictoriaMetrics GitHub organization.
+1. Avoid modifying code in the `/vendor` folder manually, even when the vendored package originates are from the VictoriaMetrics GitHub organization.
    For instance, VictoriaLogs vendors packages under the `/lib` folder from VictoriaMetrics, and VictoriaTraces vendors the `/lib/logstorage` package from VictoriaLogs.
-   Submit a pull request to the upstream repository first. Afterward, a separate pull request can be opened to update the version of the vendored folder in the downstream repository.
+   Submit a pull request to the upstream repository first. Afterward, a separate pull request can be opened to update the version of the vendored folder in downstream repository.
    * For common packages, the vendored package can be updated with this command: `go get <dependency>@vX.Y.Z`.  
    * For VictoriaMetrics packages, use `go get <dependency>@canonical_commit_hash`. 
    Finally, run `go mod tidy` and `go mod vendor` to update `go.mod`, `go.sum`, and `/vendor`.
 1. Ping reviewers who you think have the best expertise on the matter.
 
-See a good example of a [pull request](https://github.com/VictoriaMetrics/VictoriaMetrics/pull/6487).
+See good example of a [pull request](https://github.com/VictoriaMetrics/VictoriaMetrics/pull/6487).
 
 ## Merging Pull Request
 
-The person who merges the Pull Request is responsible for satisfying the requirements below:
+The person who merges the Pull Request is responsible for satisfying requirements below:
 
 1. Make sure that PR satisfies [Pull Request checklist](https://docs.victoriametrics.com/victoriametrics/contributing/#pull-request-checklist),
    it is approved by at least one reviewer, all CI checks are green.
@@ -96,9 +97,9 @@ The person who merges the Pull Request is responsible for satisfying the require
 1. If applicable, cherry-pick the change to [LTS release lines](https://docs.victoriametrics.com/victoriametrics/lts-releases/)
    and mention in the PR comment what was or wasn't cherry-picked.
 1. Update related issues with a meaningful message of what has changed and when it will be
-   released. _This helps users to understand the change without reading the PR._
+   released. _This helps users to understand the change without reading PR._
 1. Add label `completed` to related issues.
-1. Do not close related tickets until the release is made. If the ticket was auto-closed by GitHub or a user - re-open it.
+1. Do not close related tickets until release is made. If ticket was auto-closed by GitHub or user - re-open it.
 
 ## KISS principle
 
@@ -114,9 +115,9 @@ We are open to third-party pull requests provided they follow [KISS design princ
 - Minimize the number of moving parts in the distributed system.
 - Avoid automated decisions, which may hurt cluster availability, consistency, performance or debuggability.
 
-Adhering to the `KISS` principle, simplifies the resulting code and architecture so it can be reviewed, understood and debugged by a wider audience.
+Adhering to `KISS` principle, simplifies the resulting code and architecture so it can be reviewed, understood and debugged by a wider audience.
 
-Due to `KISS`, [cluster version of VictoriaMetrics](https://docs.victoriametrics.com/victoriametrics/cluster-victoriametrics/) has none of the following "features" popular in distributed computing:
+Due to `KISS`, [cluster version of VictoriaMetrics](https://docs.victoriametrics.com/victoriametrics/cluster-victoriametrics/) has none of the following "features" popular in distributed computing world:
 
 - Fragile gossip protocols. See [failed attempt in Thanos](https://github.com/improbable-eng/thanos/blob/030bc345c12c446962225221795f4973848caab5/docs/proposals/completed/201809_gossip-removal.md).
 - Hard-to-understand-and-implement-properly [Paxos protocols](https://www.quora.com/In-distributed-systems-what-is-a-simple-explanation-of-the-Paxos-algorithm).
@@ -125,17 +126,3 @@ Due to `KISS`, [cluster version of VictoriaMetrics](https://docs.victoriametrics
 - Automatic cluster resizing, which may cost you a lot of money if improperly configured.
 - Automatic discovering and addition of new nodes in the cluster, which may mix data between dev and prod clusters :)
 - Automatic leader election, which may result in split brain disaster on network errors.
-
-## Testing
-
-We recommend running the following sequence of checks and tests before submitting a pull request:
-```sh
-# run static checks
-make check-all
-
-# run unit test
-make test-full
-
-# run integration tests
-make apptest
-```

docs/victoriametrics/vmauth.md

@@ -270,7 +270,7 @@ users:
   url_prefix: "http://victoria-metrics:8428/"
 ```
 
-JWT tokens must contain a `"vm_access": {}` claim, more on that in [JWT claim-based request templating](https://docs.victoriametrics.com/victoriametrics/vmauth/#jwt-claim-based-request-templating)
+The `vm_access` claim is optional starting from {{% available_from "#" %}}: when present it is used for [request templating](https://docs.victoriametrics.com/victoriametrics/vmauth/#jwt-claim-based-request-templating), and when absent the default tenant `0:0` is assumed for any `vm_access`-based placeholders. Routing can rely solely on other token claims via [JWT claim matching](https://docs.victoriametrics.com/victoriametrics/vmauth/#jwt-claim-matching).
 
 For testing, skip signature verification with `skip_verify: true` (not recommended for production).
 

lib/jwt/jwt.go

@@ -105,6 +105,10 @@ type body struct {
 	Scope         string `json:"scope,omitempty"`
 	vmAccessClaim VMAccessClaim
 
+	// hasVMAccess is set to true when the token body contains a `vm_access` claim.
+	// Presence enforcement is left to the caller via Token.HasVMAccess.
+	hasVMAccess bool
+
 	buf []byte
 	p   *fastjson.Parser
 
@@ -121,7 +125,6 @@ type body struct {
 }
 
 func (b *body) parse(src string) error {
-
 	var err error
 	b.buf, err = decodeB64(b.buf[:0], src)
 	if err != nil {
@@ -132,6 +135,9 @@ func (b *body) parse(src string) error {
 	if err != nil {
 		return err
 	}
+	if jv.Type() != fastjson.TypeObject {
+		return fmt.Errorf("unexpected non json object; type: %q", jv.Type())
+	}
 	if expObject := jv.Get("exp"); expObject != nil {
 		b.Exp, err = expObject.Int64()
 		if err != nil {
@@ -153,30 +159,31 @@ func (b *body) parse(src string) error {
 	}
 
 	vaObject := jv.Get("vm_access")
-	if vaObject == nil {
-		return ErrVMAccessFieldMissing
-	}
-	// some IDPs encode custom claims as a string
-	// try parsing as an object and fallback to a string
-	switch vaObject.Type() {
-	case fastjson.TypeObject:
-		if err := b.vmAccessClaim.parseFrom(vaObject); err != nil {
-			return err
-		}
-	case fastjson.TypeString:
-		b.claimsParser = parserPool.Get()
-		va, err := b.claimsParser.ParseBytes(vaObject.GetStringBytes())
-		if err != nil {
-			return fmt.Errorf("cannot parse `vm_access` string json: %w", err)
-		}
-		if err := b.vmAccessClaim.parseFrom(va); err != nil {
-			return fmt.Errorf("cannot parse `vm_access` values from string json: %w", err)
-		}
-		b.vmAccessClaimObject = va
-	case fastjson.TypeNull:
-		return ErrVMAccessFieldMissing
+	switch {
+	case vaObject == nil || vaObject.Type() == fastjson.TypeNull:
+		b.hasVMAccess = false
 	default:
-		return fmt.Errorf("unexpected type for `vm_access` field; got: %q, want object {}", vaObject.Type())
+		// some IDPs encode custom claims as a string
+		// try parsing as an object and fallback to a string
+		switch vaObject.Type() {
+		case fastjson.TypeObject:
+			if err := b.vmAccessClaim.parseFrom(vaObject); err != nil {
+				return err
+			}
+		case fastjson.TypeString:
+			b.claimsParser = parserPool.Get()
+			va, err := b.claimsParser.ParseBytes(vaObject.GetStringBytes())
+			if err != nil {
+				return fmt.Errorf("cannot parse `vm_access` string json: %w", err)
+			}
+			if err := b.vmAccessClaim.parseFrom(va); err != nil {
+				return fmt.Errorf("cannot parse `vm_access` values from string json: %w", err)
+			}
+			b.vmAccessClaimObject = va
+		default:
+			return fmt.Errorf("unexpected type for `vm_access` field; got: %q, want object {}", vaObject.Type())
+		}
+		b.hasVMAccess = true
 	}
 	b.Jti = bytesutil.ToUnsafeString(jv.GetStringBytes("jti"))
 
@@ -218,6 +225,7 @@ func (b *body) reset() {
 	b.buf = b.buf[:0]
 	b.allClaims = nil
 	b.vmAccessClaim.reset()
+	b.hasVMAccess = false
 	if b.p != nil {
 		parserPool.Put(b.p)
 		b.p = nil
@@ -229,11 +237,9 @@ func (b *body) reset() {
 	if b.vmAccessClaimObject != nil {
 		b.vmAccessClaimObject = nil
 	}
-
 }
 
 // Parse parses JWT token from given source string
-//
 // Token field is valid until src is reachable
 func (t *Token) Parse(src string, enforceAuthPrefix bool) error {
 	if enforceAuthPrefix && (len(src) < len(prefix) || !strings.EqualFold(src[:len(prefix)], prefix)) {
@@ -268,6 +274,11 @@ func (t *Token) Parse(src string, enforceAuthPrefix bool) error {
 	return nil
 }
 
+// HasVMAccess reports whether the parsed token contains a `vm_access` claim.
+func (t *Token) HasVMAccess() bool {
+	return t.body.hasVMAccess
+}
+
 // Issuer returns `iss` claim value from token body
 func (t *Token) Issuer() string {
 	return t.body.Iss
@@ -425,7 +436,6 @@ func (vac *VMAccessClaim) reset() {
 }
 
 func (vac *VMAccessClaim) parseFrom(jv *fastjson.Value) error {
-
 	if err := vac.Tenant.parseFrom(jv); err != nil {
 		return err
 	}
@@ -569,6 +579,9 @@ func NewToken(auth string, enforceAuthPrefix bool) (*Token, error) {
 	if err := t.parse(jwt[0], jwt[1], jwt[2]); err != nil {
 		return nil, err
 	}
+	if !t.body.hasVMAccess {
+		return nil, ErrVMAccessFieldMissing
+	}
 	return &t, nil
 }
 

lib/jwt/jwt_test.go

@@ -168,17 +168,10 @@ func TestParseJWTBody_Failure(t *testing.T) {
 		true,
 	)
 
-	// invalid body type json
+	// non-object body type
 	f(
 		`[]`,
-		"missing `vm_access` claim",
-		true,
-	)
-
-	// missing vm_access claim
-	f(
-		`{}`,
-		"missing `vm_access` claim",
+		`unexpected non json object; type: "array"`,
 		true,
 	)
 
@@ -189,13 +182,6 @@ func TestParseJWTBody_Failure(t *testing.T) {
 		true,
 	)
 
-	// vm_access claim null
-	f(
-		`{"vm_access": null}`,
-		"missing `vm_access` claim",
-		true,
-	)
-
 	// invalid vm_access: account_id type mismatch
 	f(
 		`{"vm_access": {"tenant_id": {"account_id": "1", "project_id": 5}}}`,
@@ -555,6 +541,33 @@ func TestParseJWTBody_Success(t *testing.T) {
 	)
 }
 
+func TestParseJWTBody_VMAccessPresence(t *testing.T) {
+	f := func(data string, wantHasVMAccess bool) {
+		t.Helper()
+
+		encodedLen := base64.RawURLEncoding.EncodedLen(len(data))
+		encoded := make([]byte, encodedLen)
+		base64.RawURLEncoding.Encode(encoded, []byte(data))
+
+		var b body
+		if err := b.parse(string(encoded)); err != nil {
+			t.Fatalf("unexpected error: %s", err)
+		}
+		if b.hasVMAccess != wantHasVMAccess {
+			t.Fatalf("unexpected hasVMAccess; got %v; want %v", b.hasVMAccess, wantHasVMAccess)
+		}
+	}
+
+	// vm_access claim is present
+	f(`{"vm_access": {}}`, true)
+	f(`{"vm_access": {"metrics_account_id": 1}}`, true)
+
+	// vm_access claim is absent or null - parsing must succeed with hasVMAccess=false
+	f(`{}`, false)
+	f(`{"vm_access": null}`, false)
+	f(`{"role": "admin"}`, false)
+}
+
 func TestNewTokenFromRequest_Failure(t *testing.T) {
 	f := func(r *http.Request) {
 		t.Helper()
@@ -866,7 +879,6 @@ func TestNewTokenFromRequest_Success(t *testing.T) {
 }
 
 func TestTokenMatchClaims(t *testing.T) {
-
 	/*
 		{
 		  "iss": "https://login.microsoftonline.com/-6691-4868-a77b-1b0f9bbe5f43/v2.0",