Skip to content

Commit bfde12d

Browse files
authored
Fall back to request-token claims for opaque upstream tokens (#5147)
VirtualMCPServer (Cedar incoming authz) denied every request when the embedded auth servers upstream provider issues opaque OAuth 2.0 access tokens (Googles ya29.*, GitHubs gho_*). resolveClaims tried to JWT-parse the upstream token unconditionally and returned the parse error verbatim, so every authorization check failed and the gateway skipped every tool. Discriminate by token shape: if the upstream token is not three dot- separated segments it cannot be a JWT, so fall back to identity.Claims (the request-token claims). The embedded auth server already mirrors the upstream OIDC sub, email and name into its issued AS token (see pkg/authserver/server/session/session.go), so policies referencing standard OIDC claims continue to evaluate correctly. JWT-shaped tokens (three segments) that fail to parse still return the error: a tampered or corrupted upstream JWT must not silently degrade to fallback claims. Closes #5146 Signed-off-by: Cody J. Hanson <cjohnhanson@users.noreply.github.com> Co-authored-by: Cody J. Hanson <cjohnhanson@users.noreply.github.com>
1 parent 5a8692d commit bfde12d

2 files changed

Lines changed: 71 additions & 1 deletion

File tree

pkg/authz/authorizers/cedar/core.go

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -492,6 +492,30 @@ func (a *Authorizer) resolveClaims(identity *auth.Identity) (jwt.MapClaims, erro
492492
}
493493
parsedClaims, err := parseUpstreamJWTClaims(upstreamToken)
494494
if err != nil {
495+
// Distinguish "not JWT-shaped" (opaque OAuth 2.0 access token —
496+
// Google's ya29.*, GitHub's gho_*, etc.) from "JWT-shaped but
497+
// malformed/tampered" (a JWT with three segments that fails to
498+
// parse). Only fall back for the former; preserve the deny for
499+
// the latter so a tampered upstream JWT cannot bypass policy.
500+
//
501+
// The embedded auth server already mirrors the upstream OIDC
502+
// sub/email/name claims into its issued AS token (see
503+
// pkg/authserver/server/session/session.go). For opaque-token
504+
// providers, falling back to identity.Claims preserves identity
505+
// for policies referencing standard OIDC claims; policies that
506+
// reference upstream-only claims (groups, hd, custom namespaced
507+
// claims) will see those attributes as absent and must be
508+
// authored defensively (`has(claim_groups) && ...`).
509+
if !looksLikeJWT(upstreamToken) {
510+
// The Warn shares claimKeyLog with logClaimKeys below so a busy
511+
// Google/GitHub deployment does not emit one line per tool call.
512+
a.claimKeyLog.Do(func() {
513+
slog.Warn("upstream token is not a JWT; falling back to request-token claims for Cedar evaluation",
514+
"provider", a.primaryUpstreamProvider)
515+
})
516+
a.logClaimKeys("token-fallback", jwt.MapClaims(identity.Claims))
517+
return jwt.MapClaims(identity.Claims), nil
518+
}
495519
return nil, fmt.Errorf("failed to parse upstream token for provider %q: %w",
496520
a.primaryUpstreamProvider, err)
497521
}
@@ -504,6 +528,13 @@ func (a *Authorizer) resolveClaims(identity *auth.Identity) (jwt.MapClaims, erro
504528
return claims, nil
505529
}
506530

531+
// looksLikeJWT returns true when the token has the three-segment shape of a
532+
// JOSE-compact-serialized JWT (`header.payload.signature`). It does not
533+
// validate the contents; the parser handles that.
534+
func looksLikeJWT(tokenStr string) bool {
535+
return strings.Count(tokenStr, ".") == 2
536+
}
537+
507538
// logClaimKeys emits a rate-limited DEBUG log listing the JWT claim keys
508539
// available for Cedar policy evaluation.
509540
func (a *Authorizer) logClaimKeys(source string, claims jwt.MapClaims) {

pkg/authz/authorizers/cedar/core_test.go

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1296,7 +1296,7 @@ func TestAuthorizeWithJWTClaims_UpstreamProvider(t *testing.T) {
12961296
errContains: "upstream token for provider",
12971297
},
12981298
{
1299-
name: "upstream_token_opaque_not_parseable",
1299+
name: "upstream_token_opaque_falls_back_to_request_claims_denied",
13001300
identity: &auth.Identity{
13011301
PrincipalInfo: auth.PrincipalInfo{
13021302
Subject: "thv-user",
@@ -1306,6 +1306,45 @@ func TestAuthorizeWithJWTClaims_UpstreamProvider(t *testing.T) {
13061306
providerName: "opaque-token-cannot-be-parsed",
13071307
},
13081308
},
1309+
// Opaque upstream tokens (Google's ya29.*, GitHub's gho_*, etc.)
1310+
// trigger the fallback to identity.Claims. Here the request-token
1311+
// sub does not match the policy, so authorization is correctly
1312+
// denied based on policy evaluation rather than a parse-time error.
1313+
wantAuthorize: false,
1314+
},
1315+
{
1316+
name: "upstream_token_opaque_falls_back_to_request_claims_permitted",
1317+
identity: &auth.Identity{
1318+
PrincipalInfo: auth.PrincipalInfo{
1319+
Subject: "upstream-user",
1320+
Claims: map[string]any{"sub": "upstream-user"},
1321+
},
1322+
UpstreamTokens: map[string]string{
1323+
providerName: "opaque-token-cannot-be-parsed",
1324+
},
1325+
},
1326+
// When the upstream token is not a JWT, Cedar evaluates against
1327+
// the request-token claims. The embedded auth server already
1328+
// mirrors the upstream OIDC sub/email/name into its issued token,
1329+
// so a policy referencing claim_sub still matches the user.
1330+
wantAuthorize: true,
1331+
},
1332+
{
1333+
name: "upstream_token_jwt_shaped_but_malformed_still_errors",
1334+
identity: &auth.Identity{
1335+
PrincipalInfo: auth.PrincipalInfo{
1336+
Subject: "thv-user",
1337+
Claims: map[string]any{"sub": "thv-user"},
1338+
},
1339+
UpstreamTokens: map[string]string{
1340+
// Three-segment shape (looks like a JWT) but the segments are
1341+
// not valid base64-encoded JSON — i.e. a tampered or
1342+
// corrupted JWT. The fallback path MUST NOT trigger here:
1343+
// silently degrading a tampered upstream JWT to fallback
1344+
// claims would be a security regression.
1345+
providerName: "not-base64.not-base64.not-base64",
1346+
},
1347+
},
13091348
wantErr: true,
13101349
errContains: "failed to parse upstream token",
13111350
},

0 commit comments

Comments
 (0)