Skip to content

Commit d66b276

Browse files
Merge pull request #5381 from ImalshaD/fix/api-gate-access-token-only
Accept only access tokens as bearer credentials at the API gate
2 parents ad852dd + ea1a675 commit d66b276

8 files changed

Lines changed: 518 additions & 54 deletions

File tree

backend/internal/system/mcp/auth/token_verifier_test.go

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,9 @@ import (
1919
"github.com/stretchr/testify/suite"
2020

2121
"github.com/thunder-id/thunderid/internal/system/config"
22+
"github.com/thunder-id/thunderid/internal/system/jose/jwt"
2223
"github.com/thunder-id/thunderid/internal/system/security"
24+
tidcommon "github.com/thunder-id/thunderid/pkg/thunderidengine/common"
2325
"github.com/thunder-id/thunderid/tests/mocks/jose/jwtmock"
2426
)
2527

@@ -60,10 +62,14 @@ func TestTokenVerifierTestSuite(t *testing.T) {
6062
suite.Run(t, new(TokenVerifierTestSuite))
6163
}
6264

65+
// encodeTestToken builds a self-issued access token. The RFC 9068 typ header is required: a
66+
// self-issued token that is not an access token is rejected before verification.
6367
func encodeTestToken(payload map[string]interface{}) string {
68+
headerJSON, _ := json.Marshal(map[string]interface{}{"alg": "RS256", "typ": jwt.TokenTypeAccessToken})
6469
payloadJSON, _ := json.Marshal(payload)
70+
headerB64 := base64.RawURLEncoding.EncodeToString(headerJSON)
6571
payloadB64 := base64.RawURLEncoding.EncodeToString(payloadJSON)
66-
return "header." + payloadB64 + ".signature"
72+
return headerB64 + "." + payloadB64 + ".signature"
6773
}
6874

6975
func (suite *TokenVerifierTestSuite) newVerifier() auth.TokenVerifier {
@@ -103,9 +109,13 @@ func (suite *TokenVerifierTestSuite) TestNewTokenVerifier_Success() {
103109
}
104110

105111
func (suite *TokenVerifierTestSuite) TestNewTokenVerifier_JWTVerificationFailed() {
106-
testToken := "invalid.token.here"
112+
testToken := encodeTestToken(map[string]interface{}{"sub": "user123"})
107113

108-
suite.mockJWT.On("VerifyJWT", mock.Anything, testToken, testMCPURL, "").Return(nil)
114+
suite.mockJWT.On("VerifyJWT", mock.Anything, testToken, testMCPURL, "").Return(&tidcommon.ServiceError{
115+
Type: tidcommon.ServerErrorType,
116+
Code: "INVALID_SIGNATURE",
117+
Error: tidcommon.I18nMessage{DefaultValue: "Invalid signature"},
118+
})
109119

110120
verifier := suite.newVerifier()
111121
req := httptest.NewRequest(http.MethodGet, "/mcp", nil)
@@ -137,7 +147,8 @@ func (suite *TokenVerifierTestSuite) TestNewTokenVerifier_RevokedTokenRejected()
137147
}
138148

139149
func (suite *TokenVerifierTestSuite) TestNewTokenVerifier_InvalidPayload() {
140-
testToken := "header.invalid-payload.signature"
150+
headerJSON, _ := json.Marshal(map[string]interface{}{"alg": "RS256", "typ": jwt.TokenTypeAccessToken})
151+
testToken := base64.RawURLEncoding.EncodeToString(headerJSON) + ".invalid-payload.signature"
141152

142153
suite.mockJWT.On("VerifyJWT", mock.Anything, testToken, testMCPURL, "").Return(nil)
143154

backend/internal/system/security/jwt_authenticator.go

Lines changed: 28 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -155,9 +155,9 @@ func AuthenticateBearerToken(
155155
// verifyToken verifies the bearer token by routing on its iss claim against
156156
// an explicit allowlist of accepted issuers. Tokens from the configured
157157
// trusted issuer (when set) are verified against its JWKS. Tokens whose iss
158-
// matches this server's own JWT issuer are verified with the local signing
159-
// key, and against expectedAud if it is non-empty. Any other iss is rejected.
160-
// There is no cross-issuer fallback.
158+
// matches this server's own JWT issuer must be access tokens, and are verified
159+
// with the local signing key and against expectedAud if it is non-empty. Any
160+
// other iss is rejected. There is no cross-issuer fallback.
161161
func (h *jwtAuthenticator) verifyToken(ctx context.Context, token, expectedAud string) error {
162162
trustedIssuer := config.GetServerRuntime().Config.Server.SecurityConfig.TrustedIssuer
163163
iss := extractIssuer(token)
@@ -167,6 +167,9 @@ func (h *jwtAuthenticator) verifyToken(ctx context.Context, token, expectedAud s
167167
return errInvalidToken
168168
}
169169
case iss == config.GetServerRuntime().Config.JWT.Issuer:
170+
if err := requireAccessTokenType(token); err != nil {
171+
return err
172+
}
170173
if err := h.jwtService.VerifyJWT(ctx, token, expectedAud, ""); err != nil {
171174
return errInvalidToken
172175
}
@@ -216,6 +219,24 @@ func (h *jwtAuthenticator) verifyFederatedToken(ctx context.Context, token strin
216219
return true
217220
}
218221

222+
// requireAccessTokenType enforces the RFC 9068 typ header on a self-issued token, so that only an
223+
// access token authenticates. Every other JWT this server mints — the flow's auth assertion, ID
224+
// tokens, magic link, OTP, consent and flow tokens — carries the same issuer and signing key and
225+
// would otherwise be indistinguishable from one here. RFC 9068 §4 requires both the compact and the
226+
// media-type spelling to be accepted, compared case-insensitively.
227+
func requireAccessTokenType(token string) error {
228+
header, err := jwt.DecodeJWTHeader(token)
229+
if err != nil {
230+
return errInvalidToken
231+
}
232+
typ, _ := header["typ"].(string)
233+
if !strings.EqualFold(typ, jwt.TokenTypeAccessToken) &&
234+
!strings.EqualFold(typ, jwt.TokenTypeAccessTokenWithPrefix) {
235+
return errInvalidToken
236+
}
237+
return nil
238+
}
239+
219240
// extractToken extracts the Bearer token from the Authorization header.
220241
func extractToken(authHeader string) (string, error) {
221242
if !utils.HasPrefixFold(authHeader, constants.AuthSchemeBearer) {
@@ -237,8 +258,10 @@ func extractIssuer(token string) string {
237258
}
238259

239260
// extractScopes extracts permissions from JWT claims.
240-
// Permissions can be in "scope" (string with space-separated values), "scopes" (array) claim,
241-
// or "authorized_permissions" (server-specific) claim.
261+
// Permissions can be in "scope" (string with space-separated values) or in the "scopes" (array)
262+
// claim. The "authorized_permissions" claim of an auth assertion is deliberately not consulted: an
263+
// assertion is not an access token and no longer authenticates here, and the claim is not one the
264+
// access token builder owns, so a subject attribute of that name must not confer permissions.
242265
func extractScopes(attributes map[string]interface{}) []string {
243266
// Try "scope" claim (OAuth2 standard - space-separated string)
244267
if scopeStr, ok := attributes["scope"].(string); ok && scopeStr != "" {
@@ -261,11 +284,6 @@ func extractScopes(attributes map[string]interface{}) []string {
261284
}
262285
}
263286

264-
// Try "authorized_permissions" from the server assertion
265-
if permsStr, ok := attributes["authorized_permissions"].(string); ok && permsStr != "" {
266-
return strings.Fields(permsStr)
267-
}
268-
269287
return []string{}
270288
}
271289

backend/internal/system/security/jwt_authenticator_test.go

Lines changed: 151 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"github.com/stretchr/testify/suite"
2222

2323
"github.com/thunder-id/thunderid/internal/system/config"
24+
"github.com/thunder-id/thunderid/internal/system/jose/jwt"
2425
"github.com/thunder-id/thunderid/tests/mocks/jose/jwtmock"
2526
)
2627

@@ -97,10 +98,18 @@ func (suite *JWTAuthenticatorTestSuite) TestCanHandle() {
9798
}
9899

99100
func (suite *JWTAuthenticatorTestSuite) TestAuthenticate() {
100-
// Valid JWT token with attributes (simplified representation)
101-
// Payload: {"sub":"user123","scope":"system users:read","ouId":"ou1","app_id":"app1"}
102-
//nolint:gosec,lll // Test data, not a real credential
103-
validToken := "eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiJ1c2VyMTIzIiwic2NvcGUiOiJzeXN0ZW0gdXNlcnM6cmVhZCIsIm91SWQiOiJvdTEiLCJhcHBfaWQiOiJhcHAxIn0.signature"
101+
// A self-issued token only authenticates as an access token, so every fixture that is meant to
102+
// reach signature verification carries the RFC 9068 typ header.
103+
validToken := buildFakeJWT(
104+
accessTokenHeader(),
105+
map[string]interface{}{
106+
"sub": "user123", "scope": "system users:read", "ouId": "ou1", "app_id": "app1",
107+
},
108+
)
109+
badSignatureToken := buildFakeJWT(accessTokenHeader(), map[string]interface{}{"sub": "user123"})
110+
expiredToken := buildFakeJWT(accessTokenHeader(), map[string]interface{}{"sub": "expired-user"})
111+
malformedBase64PayloadToken := accessTokenHeaderB64() + ".invalid!base64!payload.signature"
112+
malformedJSONPayloadToken := accessTokenHeaderB64() + ".bm90X3ZhbGlkX2pzb24.signature"
104113

105114
tests := []struct {
106115
name string
@@ -142,9 +151,9 @@ func (suite *JWTAuthenticatorTestSuite) TestAuthenticate() {
142151
},
143152
{
144153
name: "Invalid JWT signature",
145-
authHeader: "Bearer invalid.jwt.token",
154+
authHeader: "Bearer " + badSignatureToken,
146155
setupMock: func(m *jwtmock.JWTServiceInterfaceMock) {
147-
m.On("VerifyJWT", mock.Anything, "invalid.jwt.token", "", "").Return(&tidcommon.ServiceError{
156+
m.On("VerifyJWT", mock.Anything, badSignatureToken, "", "").Return(&tidcommon.ServiceError{
148157
Type: tidcommon.ServerErrorType,
149158
Code: "INVALID_SIGNATURE",
150159
Error: tidcommon.I18nMessage{DefaultValue: "Invalid signature"},
@@ -155,9 +164,9 @@ func (suite *JWTAuthenticatorTestSuite) TestAuthenticate() {
155164
},
156165
{
157166
name: "Expired JWT token",
158-
authHeader: "Bearer expired.jwt.token",
167+
authHeader: "Bearer " + expiredToken,
159168
setupMock: func(m *jwtmock.JWTServiceInterfaceMock) {
160-
m.On("VerifyJWT", mock.Anything, "expired.jwt.token", "", "").Return(&tidcommon.ServiceError{
169+
m.On("VerifyJWT", mock.Anything, expiredToken, "", "").Return(&tidcommon.ServiceError{
161170
Type: tidcommon.ClientErrorType,
162171
Code: "JWT-60005",
163172
Error: tidcommon.I18nMessage{DefaultValue: "Token has expired"},
@@ -167,28 +176,26 @@ func (suite *JWTAuthenticatorTestSuite) TestAuthenticate() {
167176
expectedError: errInvalidToken,
168177
},
169178
{
170-
name: "Invalid JWT format - decoding error",
171-
authHeader: "Bearer invalidjwtformat", // Not 3 parts separated by dots
172-
setupMock: func(m *jwtmock.JWTServiceInterfaceMock) {
173-
m.On("VerifyJWT", mock.Anything, "invalidjwtformat", "", "").Return(nil)
174-
},
179+
// Not 3 parts separated by dots, so the header cannot be decoded and the token is
180+
// rejected as not an access token, before any verifier is consulted.
181+
name: "Invalid JWT format - decoding error",
182+
authHeader: "Bearer invalidjwtformat",
183+
setupMock: func(m *jwtmock.JWTServiceInterfaceMock) {},
175184
expectedError: errInvalidToken,
176185
},
177186
{
178187
name: "Invalid JWT payload - malformed base64",
179-
authHeader: "Bearer eyJhbGciOiJIUzI1NiJ9.invalid!base64!payload.signature",
188+
authHeader: "Bearer " + malformedBase64PayloadToken,
180189
setupMock: func(m *jwtmock.JWTServiceInterfaceMock) {
181-
const tok = "eyJhbGciOiJIUzI1NiJ9.invalid!base64!payload.signature"
182-
m.On("VerifyJWT", mock.Anything, tok, "", "").Return(nil)
190+
m.On("VerifyJWT", mock.Anything, malformedBase64PayloadToken, "", "").Return(nil)
183191
},
184192
expectedError: errInvalidToken,
185193
},
186194
{
187-
name: "Invalid JWT payload - malformed JSON",
188-
authHeader: "Bearer eyJhbGciOiJIUzI1NiJ9.bm90X3ZhbGlkX2pzb24.signature", // "not_valid_json" base64 encoded
195+
name: "Invalid JWT payload - malformed JSON", // "not_valid_json" base64 encoded
196+
authHeader: "Bearer " + malformedJSONPayloadToken,
189197
setupMock: func(m *jwtmock.JWTServiceInterfaceMock) {
190-
const tok = "eyJhbGciOiJIUzI1NiJ9.bm90X3ZhbGlkX2pzb24.signature"
191-
m.On("VerifyJWT", mock.Anything, tok, "", "").Return(nil)
198+
m.On("VerifyJWT", mock.Anything, malformedJSONPayloadToken, "", "").Return(nil)
192199
},
193200
expectedError: errInvalidToken,
194201
},
@@ -233,7 +240,7 @@ func (suite *JWTAuthenticatorTestSuite) TestAuthenticate() {
233240
// resource-indicator audience check; the REST gate's own jwtAuthenticator always passes "" here.
234241
func (suite *JWTAuthenticatorTestSuite) TestAuthenticate_DoesNotValidateAudience() {
235242
token := buildFakeJWT(
236-
map[string]interface{}{"alg": "RS256", "kid": "test-kid"},
243+
accessTokenHeader(),
237244
map[string]interface{}{"sub": "user123", "aud": "https://some-other-resource/mcp"},
238245
)
239246

@@ -299,11 +306,13 @@ func (suite *JWTAuthenticatorTestSuite) TestExtractPermissionsFromJWTClaims() {
299306
expectedPermissions: []string{"users:read"},
300307
},
301308
{
302-
name: "ThunderID assertion authorized_permissions attribute",
309+
// An assertion's authorized_permissions never becomes a caller's permissions. Only an
310+
// access token authenticates, and its scopes are carried in scope.
311+
name: "Assertion authorized_permissions attribute is ignored",
303312
attributes: map[string]interface{}{
304313
"authorized_permissions": "perm1 perm2 perm3",
305314
},
306-
expectedPermissions: []string{"perm1", "perm2", "perm3"},
315+
expectedPermissions: []string{},
307316
},
308317
}
309318

@@ -480,6 +489,19 @@ const (
480489
)
481490

482491
// buildFakeJWT creates a fake JWT string with the given header and payload claims.
492+
// accessTokenHeader returns the header of a self-issued access token, the only self-issued token the
493+
// gate authenticates.
494+
func accessTokenHeader() map[string]interface{} {
495+
return map[string]interface{}{"alg": "RS256", "kid": "test-kid", "typ": jwt.TokenTypeAccessToken}
496+
}
497+
498+
// accessTokenHeaderB64 returns that header already encoded, for building tokens whose payload is
499+
// deliberately malformed and so cannot go through buildFakeJWT.
500+
func accessTokenHeaderB64() string {
501+
headerJSON, _ := json.Marshal(accessTokenHeader())
502+
return base64.RawURLEncoding.EncodeToString(headerJSON)
503+
}
504+
483505
func buildFakeJWT(header, payload map[string]interface{}) string {
484506
headerJSON, _ := json.Marshal(header)
485507
payloadJSON, _ := json.Marshal(payload)
@@ -892,7 +914,7 @@ func (suite *JWTAuthenticatorTestSuite) TestAuthenticate_SelfIssuedTokenUnderFed
892914
_ = config.InitializeServerRuntime("", federatedConfigWithLocalIssuer())
893915

894916
token := buildFakeJWT(
895-
map[string]interface{}{"alg": "RS256", "kid": "local-kid"},
917+
map[string]interface{}{"alg": "RS256", "kid": "local-kid", "typ": jwt.TokenTypeAccessToken},
896918
map[string]interface{}{
897919
"sub": "service-app",
898920
"access_token_sub": "user-123",
@@ -929,7 +951,7 @@ func (suite *JWTAuthenticatorTestSuite) TestAuthenticate_SelfIssuedTokenInvalidU
929951
_ = config.InitializeServerRuntime("", federatedConfigWithLocalIssuer())
930952

931953
token := buildFakeJWT(
932-
map[string]interface{}{"alg": "RS256", "kid": "local-kid"},
954+
map[string]interface{}{"alg": "RS256", "kid": "local-kid", "typ": jwt.TokenTypeAccessToken},
933955
map[string]interface{}{"sub": "service-app", "iss": testLocalIssuer},
934956
)
935957

@@ -976,3 +998,107 @@ func (suite *JWTAuthenticatorTestSuite) TestAuthenticate_UnknownIssuerUnderFeder
976998
mockJWT.AssertNotCalled(suite.T(), "VerifyJWT")
977999
mockJWT.AssertNotCalled(suite.T(), "VerifyJWTWithJWKS")
9781000
}
1001+
1002+
// TestAuthenticate_RequiresAccessTokenTypeForSelfIssuedToken asserts the gate accepts only an access
1003+
// token (RFC 9068 typ) on the self-issued branch, so no other JWT this server signs with the same key
1004+
// — an auth assertion, ID token, magic link, OTP, consent or flow token — passes as an API credential.
1005+
func (suite *JWTAuthenticatorTestSuite) TestAuthenticate_RequiresAccessTokenTypeForSelfIssuedToken() {
1006+
tests := []struct {
1007+
name string
1008+
typ interface{}
1009+
accepted bool
1010+
}{
1011+
{name: "at+jwt is accepted", typ: jwt.TokenTypeAccessToken, accepted: true},
1012+
{name: "media type form is accepted", typ: jwt.TokenTypeAccessTokenWithPrefix, accepted: true},
1013+
{name: "typ is compared case insensitively", typ: "AT+JWT", accepted: true},
1014+
{name: "plain JWT is rejected", typ: jwt.TokenTypeJWT, accepted: false},
1015+
{name: "ID-JAG is rejected", typ: jwt.TokenTypeIDJAG, accepted: false},
1016+
{name: "missing typ is rejected", typ: nil, accepted: false},
1017+
}
1018+
1019+
for _, tt := range tests {
1020+
suite.Run(tt.name, func() {
1021+
header := map[string]interface{}{"alg": "RS256", "kid": "test-kid"}
1022+
if tt.typ != nil {
1023+
header["typ"] = tt.typ
1024+
}
1025+
token := buildFakeJWT(header, map[string]interface{}{"sub": "user123"})
1026+
1027+
mockJWT := jwtmock.NewJWTServiceInterfaceMock(suite.T())
1028+
if tt.accepted {
1029+
mockJWT.On("VerifyJWT", mock.Anything, token, "", "").Return(nil)
1030+
}
1031+
auth := newJWTAuthenticator(mockJWT)
1032+
1033+
req := httptest.NewRequest(http.MethodGet, "/users", nil)
1034+
req.Header.Set("Authorization", "Bearer "+token)
1035+
1036+
authCtx, err := auth.Authenticate(req)
1037+
1038+
if tt.accepted {
1039+
assert.NoError(suite.T(), err)
1040+
assert.NotNil(suite.T(), authCtx)
1041+
} else {
1042+
assert.ErrorIs(suite.T(), err, errInvalidToken)
1043+
assert.Nil(suite.T(), authCtx)
1044+
// The type check must short-circuit before signature verification.
1045+
mockJWT.AssertNotCalled(suite.T(), "VerifyJWT")
1046+
}
1047+
mockJWT.AssertExpectations(suite.T())
1048+
})
1049+
}
1050+
}
1051+
1052+
// TestAuthenticate_RejectsFlowAuthAssertion covers the concrete confusion the type check closes. The
1053+
// assertion the sign-in flow returns is self-issued and carries authorized_permissions, which the
1054+
// gate read as the caller's permissions; it is meant only for exchange at the token endpoint.
1055+
func (suite *JWTAuthenticatorTestSuite) TestAuthenticate_RejectsFlowAuthAssertion() {
1056+
assertion := buildFakeJWT(
1057+
map[string]interface{}{"alg": "RS256", "kid": "test-kid", "typ": jwt.TokenTypeJWT},
1058+
map[string]interface{}{
1059+
"sub": "user123",
1060+
"aud": "app1",
1061+
"assurance": map[string]interface{}{"aal": "AAL1", "ial": "IAL1"},
1062+
"authorized_permissions": "users:read users:write",
1063+
},
1064+
)
1065+
1066+
mockJWT := jwtmock.NewJWTServiceInterfaceMock(suite.T())
1067+
auth := newJWTAuthenticator(mockJWT)
1068+
1069+
req := httptest.NewRequest(http.MethodGet, "/users", nil)
1070+
req.Header.Set("Authorization", "Bearer "+assertion)
1071+
1072+
authCtx, err := auth.Authenticate(req)
1073+
1074+
assert.ErrorIs(suite.T(), err, errInvalidToken)
1075+
assert.Nil(suite.T(), authCtx)
1076+
}
1077+
1078+
// TestAuthenticate_FederatedTokenTypeNotRestricted pins the access-token type check to self-issued
1079+
// tokens only. A trusted issuer may be a generic OIDC provider that stamps typ JWT on its access
1080+
// tokens, and those must keep authenticating.
1081+
func (suite *JWTAuthenticatorTestSuite) TestAuthenticate_FederatedTokenTypeNotRestricted() {
1082+
config.ResetServerRuntime()
1083+
defer config.ResetServerRuntime()
1084+
_ = config.InitializeServerRuntime("", federatedConfigWithLocalIssuer())
1085+
1086+
token := buildFakeJWT(
1087+
map[string]interface{}{"alg": "RS256", "kid": "test-kid", "typ": jwt.TokenTypeJWT},
1088+
map[string]interface{}{"sub": "federated-user", "iss": testFederatedIssuer},
1089+
)
1090+
1091+
mockJWT := jwtmock.NewJWTServiceInterfaceMock(suite.T())
1092+
mockJWT.On("VerifyJWTWithJWKS", mock.Anything, token,
1093+
testFederatedJWKSURL, testFederatedAudience, testFederatedIssuer).Return(nil)
1094+
auth := newJWTAuthenticator(mockJWT)
1095+
1096+
req := httptest.NewRequest(http.MethodGet, "/users", nil)
1097+
req.Header.Set("Authorization", "Bearer "+token)
1098+
1099+
authCtx, err := auth.Authenticate(req)
1100+
1101+
assert.NoError(suite.T(), err)
1102+
assert.NotNil(suite.T(), authCtx)
1103+
mockJWT.AssertExpectations(suite.T())
1104+
}

0 commit comments

Comments
 (0)