From c8e56b05a82f21d5b6fcc2763b0192763c7f491d Mon Sep 17 00:00:00 2001 From: Michelangelo Mori <328978+blkt@users.noreply.github.com> Date: Tue, 14 Apr 2026 14:20:48 +0200 Subject: [PATCH 1/3] Grant default `aud` when `resource` is absent The `TokenHandler` only granted an audience claim when the client included the RFC 8707 `resource` parameter. Clients that omit this optional parameter received a token with an empty `aud`, which caused `VirtualMCPServer` to reject every token its own auth server issued: the incoming OIDC validator requires `audience` to be set and rejects tokens that do not carry a matching claim. When no `resource` parameter is present and `AllowedAudiences` contains exactly one entry, the handler now defaults to granting that audience. The intended audience is unambiguous in that case, and `AllowedAudiences` is always non-empty (enforced at config validation time). The defaulting is restricted to `authorization_code` grants; for `refresh_token` grants fosite carries the originally-granted audience forward through the stored session automatically, so re-granting would conflict with its matching strategy. Fixes #4794 --- pkg/authserver/server/handlers/token.go | 12 +++++ pkg/authserver/server/handlers/token_test.go | 52 ++++++++++++++++++++ 2 files changed, 64 insertions(+) diff --git a/pkg/authserver/server/handlers/token.go b/pkg/authserver/server/handlers/token.go index 6fc5865cff..72a18dde0c 100644 --- a/pkg/authserver/server/handlers/token.go +++ b/pkg/authserver/server/handlers/token.go @@ -75,6 +75,18 @@ func (h *Handler) TokenHandler(w http.ResponseWriter, req *http.Request) { "resource", resource, ) accessRequest.GrantAudience(resource) + } else if accessRequest.GetGrantTypes().ExactOne("authorization_code") && len(h.config.AllowedAudiences) == 1 { + // No resource parameter provided during an authorization_code exchange; default to the + // sole allowed audience. AllowedAudiences is always non-empty (enforced at config + // validation time), and when there is exactly one entry the intended audience is + // unambiguous. We restrict this defaulting to authorization_code grants: for + // refresh_token grants, fosite already carries the originally-granted audience forward + // through the session, so re-granting here would conflict with fosite's audience + // matching strategy. + slog.Debug("no resource parameter, defaulting to sole allowed audience", + "audience", h.config.AllowedAudiences[0], + ) + accessRequest.GrantAudience(h.config.AllowedAudiences[0]) } // Generate the access response (tokens) diff --git a/pkg/authserver/server/handlers/token_test.go b/pkg/authserver/server/handlers/token_test.go index 84cee335ca..053cb18243 100644 --- a/pkg/authserver/server/handlers/token_test.go +++ b/pkg/authserver/server/handlers/token_test.go @@ -4,6 +4,7 @@ package handlers import ( + "encoding/json" "net/http" "net/http/httptest" "net/url" @@ -11,6 +12,8 @@ import ( "testing" "time" + "github.com/go-jose/go-jose/v4" + josejwt "github.com/go-jose/go-jose/v4/jwt" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -204,6 +207,55 @@ func TestTokenHandler_ResourceParameter(t *testing.T) { assert.Contains(t, body, "access_token") } +func TestTokenHandler_DefaultsAudienceWhenNoResourceParam(t *testing.T) { + t.Parallel() + handler, storState, _ := handlerTestSetup(t) + + // Simulate the authorize/callback flow to obtain a valid authorization code. + // baseTestSetup sets AllowedAudiences to []string{"https://api.example.com"}. + authorizeCode := simulateAuthorizeFlow(t, handler, storState) + + // Exchange the code WITHOUT a resource parameter. + form := url.Values{ + "grant_type": {"authorization_code"}, + "client_id": {testAuthClientID}, + "redirect_uri": {testAuthRedirectURI}, + "code": {authorizeCode}, + "code_verifier": {testPKCEVerifier}, + // Intentionally no "resource" parameter. + } + req := httptest.NewRequest(http.MethodPost, "/oauth/token", strings.NewReader(form.Encode())) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + rec := httptest.NewRecorder() + + handler.TokenHandler(rec, req) + + require.Equal(t, http.StatusOK, rec.Code, "expected 200 OK, got %d: %s", rec.Code, rec.Body.String()) + + // Decode the token response JSON to extract the access_token. + var tokenResp map[string]any + err := json.NewDecoder(rec.Body).Decode(&tokenResp) + require.NoError(t, err, "response body should be valid JSON") + + accessToken, ok := tokenResp["access_token"].(string) + require.True(t, ok, "access_token should be a non-empty string") + require.NotEmpty(t, accessToken, "access_token should not be empty") + + // Parse the JWT payload without signature verification to inspect claims. + parsedToken, err := josejwt.ParseSigned(accessToken, []jose.SignatureAlgorithm{jose.RS256}) + require.NoError(t, err, "access_token should be a parseable JWT") + + var claims map[string]any + err = parsedToken.UnsafeClaimsWithoutVerification(&claims) + require.NoError(t, err, "should be able to extract JWT claims without verification") + + // The sole AllowedAudience should have been granted automatically. + aud, ok := claims["aud"].([]any) + require.True(t, ok, "aud claim should be an array, got: %T %v", claims["aud"], claims["aud"]) + require.Len(t, aud, 1, "aud claim should contain exactly one entry") + assert.Equal(t, "https://api.example.com", aud[0], "aud should default to the sole AllowedAudience") +} + func TestTokenHandler_RouteRegistered(t *testing.T) { t.Parallel() handler, _, _ := handlerTestSetup(t) From f5d9c30f2c1ed9ba33f48c52419090df45d99510 Mon Sep 17 00:00:00 2001 From: Michelangelo Mori <328978+blkt@users.noreply.github.com> Date: Tue, 14 Apr 2026 15:27:11 +0200 Subject: [PATCH 2/3] Address review comments on #4805 An explicit empty `resource=` parameter was hitting the `len(resources) == 1` branch and calling `GrantAudience("")`, producing `aud:[""]` instead of the intended default. The fix is to treat an empty string the same as an absent parameter by adding `&& resources[0] != ""` to the condition, so an explicit empty value falls through to the defaulting `else if` branch. A test covering this case is also added. The comment on the defaulting branch overstated the invariant by attributing safety to config-validation-time enforcement. The actual safety comes from the `len == 1` guard on the same line; the comment is updated to reflect that. --- pkg/authserver/server/handlers/token.go | 15 +++---- pkg/authserver/server/handlers/token_test.go | 46 ++++++++++++++++++++ 2 files changed, 53 insertions(+), 8 deletions(-) diff --git a/pkg/authserver/server/handlers/token.go b/pkg/authserver/server/handlers/token.go index 72a18dde0c..05478cb6dd 100644 --- a/pkg/authserver/server/handlers/token.go +++ b/pkg/authserver/server/handlers/token.go @@ -49,7 +49,7 @@ func (h *Handler) TokenHandler(w http.ResponseWriter, req *http.Request) { server.ErrInvalidTarget.WithHint("Multiple resource parameters are not supported")) return } - if len(resources) == 1 { + if len(resources) == 1 && resources[0] != "" { resource := resources[0] // Validate URI format per RFC 8707 if err := server.ValidateAudienceURI(resource); err != nil { @@ -76,13 +76,12 @@ func (h *Handler) TokenHandler(w http.ResponseWriter, req *http.Request) { ) accessRequest.GrantAudience(resource) } else if accessRequest.GetGrantTypes().ExactOne("authorization_code") && len(h.config.AllowedAudiences) == 1 { - // No resource parameter provided during an authorization_code exchange; default to the - // sole allowed audience. AllowedAudiences is always non-empty (enforced at config - // validation time), and when there is exactly one entry the intended audience is - // unambiguous. We restrict this defaulting to authorization_code grants: for - // refresh_token grants, fosite already carries the originally-granted audience forward - // through the session, so re-granting here would conflict with fosite's audience - // matching strategy. + // No resource parameter provided (or provided as empty) during an authorization_code + // exchange; default to the sole allowed audience. The len == 1 guard makes the + // intended audience unambiguous and the index access safe. We restrict this defaulting + // to authorization_code grants: for refresh_token grants, fosite already carries the + // originally-granted audience forward through the session, so re-granting here would + // conflict with fosite's audience matching strategy. slog.Debug("no resource parameter, defaulting to sole allowed audience", "audience", h.config.AllowedAudiences[0], ) diff --git a/pkg/authserver/server/handlers/token_test.go b/pkg/authserver/server/handlers/token_test.go index 053cb18243..49b85a08a6 100644 --- a/pkg/authserver/server/handlers/token_test.go +++ b/pkg/authserver/server/handlers/token_test.go @@ -256,6 +256,52 @@ func TestTokenHandler_DefaultsAudienceWhenNoResourceParam(t *testing.T) { assert.Equal(t, "https://api.example.com", aud[0], "aud should default to the sole AllowedAudience") } +func TestTokenHandler_EmptyResourceParamDefaultsAudience(t *testing.T) { + t.Parallel() + handler, storState, _ := handlerTestSetup(t) + + authorizeCode := simulateAuthorizeFlow(t, handler, storState) + + // Exchange the code with an explicit but empty resource parameter. + // This should be treated the same as omitting resource entirely and + // default to the sole AllowedAudience rather than granting aud:[\""]. + form := url.Values{ + "grant_type": {"authorization_code"}, + "client_id": {testAuthClientID}, + "redirect_uri": {testAuthRedirectURI}, + "code": {authorizeCode}, + "code_verifier": {testPKCEVerifier}, + "resource": {""}, // explicit empty value + } + req := httptest.NewRequest(http.MethodPost, "/oauth/token", strings.NewReader(form.Encode())) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + rec := httptest.NewRecorder() + + handler.TokenHandler(rec, req) + + require.Equal(t, http.StatusOK, rec.Code, "expected 200 OK, got %d: %s", rec.Code, rec.Body.String()) + + var tokenResp map[string]any + err := json.NewDecoder(rec.Body).Decode(&tokenResp) + require.NoError(t, err, "response body should be valid JSON") + + accessToken, ok := tokenResp["access_token"].(string) + require.True(t, ok, "access_token should be a string") + require.NotEmpty(t, accessToken) + + parsedToken, err := josejwt.ParseSigned(accessToken, []jose.SignatureAlgorithm{jose.RS256}) + require.NoError(t, err) + + var claims map[string]any + err = parsedToken.UnsafeClaimsWithoutVerification(&claims) + require.NoError(t, err) + + aud, ok := claims["aud"].([]any) + require.True(t, ok, "aud claim should be an array, got: %T %v", claims["aud"], claims["aud"]) + require.Len(t, aud, 1) + assert.Equal(t, "https://api.example.com", aud[0], "explicit empty resource should default to sole AllowedAudience") +} + func TestTokenHandler_RouteRegistered(t *testing.T) { t.Parallel() handler, _, _ := handlerTestSetup(t) From 89a40d6bd7d326e92aa02859ad19217b7ab4f110 Mon Sep 17 00:00:00 2001 From: Michelangelo Mori <328978+blkt@users.noreply.github.com> Date: Tue, 14 Apr 2026 15:41:32 +0200 Subject: [PATCH 3/3] Consolidate audience claim tests into table-driven test The three separate tests covering the `aud` claim behaviour of `TokenHandler` shared identical structure. Merge them into a single `TestTokenHandler_AudienceClaim` table-driven test with subtests for the explicit-resource, absent-resource, and explicit-empty-resource cases. Also upgrades the former `TestTokenHandler_ResourceParameter`, which only asserted a 200 response, to verify the `aud` claim in the issued JWT. --- pkg/authserver/server/handlers/token_test.go | 186 +++++++------------ 1 file changed, 67 insertions(+), 119 deletions(-) diff --git a/pkg/authserver/server/handlers/token_test.go b/pkg/authserver/server/handlers/token_test.go index 49b85a08a6..df4e9b7c9c 100644 --- a/pkg/authserver/server/handlers/token_test.go +++ b/pkg/authserver/server/handlers/token_test.go @@ -176,130 +176,78 @@ func TestTokenHandler_Success(t *testing.T) { assert.Contains(t, body, "expires_in") } -func TestTokenHandler_ResourceParameter(t *testing.T) { +func TestTokenHandler_AudienceClaim(t *testing.T) { t.Parallel() - handler, storState, _ := handlerTestSetup(t) - - // Simulate authorize flow - authorizeCode := simulateAuthorizeFlow(t, handler, storState) - // Exchange code with RFC 8707 resource parameter - form := url.Values{ - "grant_type": {"authorization_code"}, - "client_id": {testAuthClientID}, - "redirect_uri": {testAuthRedirectURI}, - "code": {authorizeCode}, - "code_verifier": {testPKCEVerifier}, - "resource": {"https://api.example.com"}, + // ptr is a helper to take the address of a string literal. + ptr := func(s string) *string { return &s } + + tests := []struct { + name string + resource *string // nil = omit parameter; non-nil = include (possibly empty) + wantAud string + }{ + { + name: "explicit resource grants matching audience", + resource: ptr("https://api.example.com"), + wantAud: "https://api.example.com", + }, + { + name: "absent resource defaults to sole AllowedAudience", + resource: nil, + wantAud: "https://api.example.com", + }, + { + name: "explicit empty resource defaults to sole AllowedAudience", + resource: ptr(""), + wantAud: "https://api.example.com", + }, } - req := httptest.NewRequest(http.MethodPost, "/oauth/token", strings.NewReader(form.Encode())) - req.Header.Set("Content-Type", "application/x-www-form-urlencoded") - rec := httptest.NewRecorder() - - handler.TokenHandler(rec, req) - require.Equal(t, http.StatusOK, rec.Code, "expected 200 OK, got %d: %s", rec.Code, rec.Body.String()) - - // The resource parameter should be granted as audience in the JWT - // We can't easily verify the JWT contents here without decoding, - // but we verify the request succeeded - body := rec.Body.String() - assert.Contains(t, body, "access_token") -} - -func TestTokenHandler_DefaultsAudienceWhenNoResourceParam(t *testing.T) { - t.Parallel() - handler, storState, _ := handlerTestSetup(t) - - // Simulate the authorize/callback flow to obtain a valid authorization code. - // baseTestSetup sets AllowedAudiences to []string{"https://api.example.com"}. - authorizeCode := simulateAuthorizeFlow(t, handler, storState) - - // Exchange the code WITHOUT a resource parameter. - form := url.Values{ - "grant_type": {"authorization_code"}, - "client_id": {testAuthClientID}, - "redirect_uri": {testAuthRedirectURI}, - "code": {authorizeCode}, - "code_verifier": {testPKCEVerifier}, - // Intentionally no "resource" parameter. + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + handler, storState, _ := handlerTestSetup(t) + authorizeCode := simulateAuthorizeFlow(t, handler, storState) + + form := url.Values{ + "grant_type": {"authorization_code"}, + "client_id": {testAuthClientID}, + "redirect_uri": {testAuthRedirectURI}, + "code": {authorizeCode}, + "code_verifier": {testPKCEVerifier}, + } + if tc.resource != nil { + form.Set("resource", *tc.resource) + } + + req := httptest.NewRequest(http.MethodPost, "/oauth/token", strings.NewReader(form.Encode())) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + rec := httptest.NewRecorder() + + handler.TokenHandler(rec, req) + + require.Equal(t, http.StatusOK, rec.Code, "got %d: %s", rec.Code, rec.Body.String()) + + var tokenResp map[string]any + require.NoError(t, json.NewDecoder(rec.Body).Decode(&tokenResp)) + + accessToken, ok := tokenResp["access_token"].(string) + require.True(t, ok, "access_token should be a string") + require.NotEmpty(t, accessToken) + + parsedToken, err := josejwt.ParseSigned(accessToken, []jose.SignatureAlgorithm{jose.RS256}) + require.NoError(t, err) + + var claims map[string]any + require.NoError(t, parsedToken.UnsafeClaimsWithoutVerification(&claims)) + + aud, ok := claims["aud"].([]any) + require.True(t, ok, "aud claim should be an array, got: %T %v", claims["aud"], claims["aud"]) + require.Len(t, aud, 1) + assert.Equal(t, tc.wantAud, aud[0]) + }) } - req := httptest.NewRequest(http.MethodPost, "/oauth/token", strings.NewReader(form.Encode())) - req.Header.Set("Content-Type", "application/x-www-form-urlencoded") - rec := httptest.NewRecorder() - - handler.TokenHandler(rec, req) - - require.Equal(t, http.StatusOK, rec.Code, "expected 200 OK, got %d: %s", rec.Code, rec.Body.String()) - - // Decode the token response JSON to extract the access_token. - var tokenResp map[string]any - err := json.NewDecoder(rec.Body).Decode(&tokenResp) - require.NoError(t, err, "response body should be valid JSON") - - accessToken, ok := tokenResp["access_token"].(string) - require.True(t, ok, "access_token should be a non-empty string") - require.NotEmpty(t, accessToken, "access_token should not be empty") - - // Parse the JWT payload without signature verification to inspect claims. - parsedToken, err := josejwt.ParseSigned(accessToken, []jose.SignatureAlgorithm{jose.RS256}) - require.NoError(t, err, "access_token should be a parseable JWT") - - var claims map[string]any - err = parsedToken.UnsafeClaimsWithoutVerification(&claims) - require.NoError(t, err, "should be able to extract JWT claims without verification") - - // The sole AllowedAudience should have been granted automatically. - aud, ok := claims["aud"].([]any) - require.True(t, ok, "aud claim should be an array, got: %T %v", claims["aud"], claims["aud"]) - require.Len(t, aud, 1, "aud claim should contain exactly one entry") - assert.Equal(t, "https://api.example.com", aud[0], "aud should default to the sole AllowedAudience") -} - -func TestTokenHandler_EmptyResourceParamDefaultsAudience(t *testing.T) { - t.Parallel() - handler, storState, _ := handlerTestSetup(t) - - authorizeCode := simulateAuthorizeFlow(t, handler, storState) - - // Exchange the code with an explicit but empty resource parameter. - // This should be treated the same as omitting resource entirely and - // default to the sole AllowedAudience rather than granting aud:[\""]. - form := url.Values{ - "grant_type": {"authorization_code"}, - "client_id": {testAuthClientID}, - "redirect_uri": {testAuthRedirectURI}, - "code": {authorizeCode}, - "code_verifier": {testPKCEVerifier}, - "resource": {""}, // explicit empty value - } - req := httptest.NewRequest(http.MethodPost, "/oauth/token", strings.NewReader(form.Encode())) - req.Header.Set("Content-Type", "application/x-www-form-urlencoded") - rec := httptest.NewRecorder() - - handler.TokenHandler(rec, req) - - require.Equal(t, http.StatusOK, rec.Code, "expected 200 OK, got %d: %s", rec.Code, rec.Body.String()) - - var tokenResp map[string]any - err := json.NewDecoder(rec.Body).Decode(&tokenResp) - require.NoError(t, err, "response body should be valid JSON") - - accessToken, ok := tokenResp["access_token"].(string) - require.True(t, ok, "access_token should be a string") - require.NotEmpty(t, accessToken) - - parsedToken, err := josejwt.ParseSigned(accessToken, []jose.SignatureAlgorithm{jose.RS256}) - require.NoError(t, err) - - var claims map[string]any - err = parsedToken.UnsafeClaimsWithoutVerification(&claims) - require.NoError(t, err) - - aud, ok := claims["aud"].([]any) - require.True(t, ok, "aud claim should be an array, got: %T %v", claims["aud"], claims["aud"]) - require.Len(t, aud, 1) - assert.Equal(t, "https://api.example.com", aud[0], "explicit empty resource should default to sole AllowedAudience") } func TestTokenHandler_RouteRegistered(t *testing.T) {