Skip to content

Commit 35d2b2d

Browse files
kofoworolakofoworola
authored
[TT-15359]improve backwards compatibility of Jwt claim validation (#7294)
### **User description** <details open> <summary><a href="https://tyktech.atlassian.net/browse/TT-15359" title="TT-15359" target="_blank">TT-15359</a></summary> <br /> <table> <tr> <th>Summary</th> <td> Core Registered Claims Validation</td> </tr> <tr> <th>Type</th> <td> <img alt="Story" src="https://tyktech.atlassian.net/rest/api/2/universal_avatar/view/type/issuetype/avatar/10315?size=medium" /> Story </td> </tr> <tr> <th>Status</th> <td>In Code Review</td> </tr> <tr> <th>Points</th> <td>N/A</td> </tr> <tr> <th>Labels</th> <td><a href="https://tyktech.atlassian.net/issues?jql=project%20%3D%20TT%20AND%20labels%20%3D%20jira_escalated%20ORDER%20BY%20created%20DESC" title="jira_escalated">jira_escalated</a></td> </tr> </table> </details> <!-- do not remove this marker as it will break jira-lint's functionality. added_by_jira_lint --> --- <!-- Provide a general summary of your changes in the Title above --> ## Description [TT-15359](https://tyktech.atlassian.net/browse/TT-15359) <!-- Describe your changes in detail --> This pull request improves backward compatibility for JWT claim validation in the Tyk Gateway by: - Adding fallback logic in the OAS (OpenAPI Specification) JWT configuration: - Uses PolicyFieldName if BasePolicyClaims is empty. - Uses IdentityBaseField if SubjectClaims is empty. - Uses Scopes.ClaimName if Scopes.Claims is empty. - Extending and reorganizing unit tests to cover these new fallback behaviors. - Ensuring that new logic preserves existing backward-compatible behavior when parsing JWTs. <!-- This project only accepts pull requests related to open issues. --> <!-- If suggesting a new feature or change, please discuss it in an issue first. --> <!-- If fixing a bug, there should be an issue describing it with steps to reproduce. --> <!-- OSS: Please link to the issue here. Tyk: please create/link the JIRA ticket. --> ## Motivation and Context <!-- Why is this change required? What problem does it solve? --> ## How This Has Been Tested <!-- Please describe in detail how you tested your changes --> <!-- Include details of your testing environment, and the tests --> <!-- you ran to see how your change affects other areas of the code, etc. --> <!-- This information is helpful for reviewers and QA. --> ## Screenshots (if appropriate) ## Types of changes <!-- What types of changes does your code introduce? Put an `x` in all the boxes that apply: --> - [ ] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [ ] Refactoring or add test (improvements in base code or adds test coverage to functionality) ## Checklist <!-- Go over all the following points, and put an `x` in all the boxes that apply --> <!-- If there are no documentation updates required, mark the item as checked. --> <!-- Raise up any additional concerns not covered by the checklist. --> - [ ] I ensured that the documentation is up to date - [ ] I explained why this PR updates go.mod in detail with reasoning why it's required - [ ] I would like a code coverage CI quality gate exception and have explained why [TT-15359]: https://tyktech.atlassian.net/browse/TT-15359?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ ___ ### **PR Type** Enhancement, Tests ___ ### **Description** - Add OAS fallbacks for deprecated fields - Prefer single-field configs when arrays empty - Expand unit tests for OAS JWT parsing - Preserve backward compatibility behavior ___ ### Diagram Walkthrough ```mermaid flowchart LR OASCfg["OAS JWT config"] -- "BasePolicyClaims empty, use PolicyFieldName" --> PolicyID["Resolve policy ID"] OASCfg -- "SubjectClaims empty, use IdentityBaseField" --> UserID["Resolve user ID"] OASCfg -- "Scopes.Claims empty, use Scopes.ClaimName" --> ScopeClaim["Resolve scope claim name"] Tests["Unit tests"] -- "cover new fallbacks" --> Verification["Behavior verified"] ``` <details> <summary><h3> File Walkthrough</h3></summary> <table><thead><tr><th></th><th align="left">Relevant files</th></tr></thead><tbody><tr><td><strong>Enhancement</strong></td><td><table> <tr> <td> <details> <summary><strong>mw_jwt.go</strong><dd><code>Add OAS JWT claim fallback logic</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></summary> <hr> gateway/mw_jwt.go <ul><li>Fallback to <code>PolicyFieldName</code> when <code>BasePolicyClaims</code> empty<br> <li> Fallback to <code>IdentityBaseField</code> when <code>SubjectClaims</code> empty<br> <li> Fallback to <code>Scopes.ClaimName</code> when <code>Scopes.Claims</code> empty</ul> </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/7294/files#diff-e8bce0f6790c8c56b30e24dbeebb0fc4aa0879ab5ea5f6ef6dbe68931410e237">+9/-0</a>&nbsp; &nbsp; &nbsp; </td> </tr> </table></td></tr><tr><td><strong>Tests</strong></td><td><table> <tr> <td> <details> <summary><strong>mw_jwt_test.go</strong><dd><code>Extend tests for OAS JWT claim fallbacks</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></summary> <hr> gateway/mw_jwt_test.go <ul><li>Add tests for policy fallback via <code>PolicyFieldName</code><br> <li> Split and expand OAS user ID tests with <code>IdentityBaseField</code><br> <li> Add tests for scopes fallback via <code>Scopes.ClaimName</code></ul> </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/7294/files#diff-406bf8fdb6c0cc77f04c6245c70abfc592ddb1525aa843200d850e14d135ebfc">+140/-104</a></td> </tr> </table></td></tr></tr></tbody></table> </details> ___
1 parent 8cd87da commit 35d2b2d

File tree

2 files changed

+149
-104
lines changed

2 files changed

+149
-104
lines changed

‎gateway/mw_jwt.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -384,6 +384,9 @@ func (k *JWTMiddleware) getPolicyIDFromToken(claims jwt.MapClaims) (string, bool
384384
policyID := ""
385385
found := false
386386
fieldNames := k.Spec.OAS.GetJWTConfiguration().BasePolicyClaims
387+
if len(fieldNames) == 0 && k.Spec.OAS.GetJWTConfiguration().PolicyFieldName != "" {
388+
fieldNames = append(fieldNames, k.Spec.OAS.GetJWTConfiguration().PolicyFieldName)
389+
}
387390
for _, c := range fieldNames {
388391
policyID, found = claims[c].(string)
389392
if found {
@@ -451,6 +454,9 @@ func (k *JWTMiddleware) getUserIdFromClaim(claims jwt.MapClaims) (string, error)
451454

452455
func (k *JWTMiddleware) getUserIDFromClaimOAS(claims jwt.MapClaims) (string, error) {
453456
identityBaseFields := k.Spec.OAS.GetJWTConfiguration().SubjectClaims
457+
if len(identityBaseFields) == 0 && k.Spec.OAS.GetJWTConfiguration().IdentityBaseField != "" {
458+
identityBaseFields = append(identityBaseFields, k.Spec.OAS.GetJWTConfiguration().IdentityBaseField)
459+
}
454460
checkedSub := false
455461
for _, identityBaseField := range identityBaseFields {
456462
if identityBaseField == SUB {
@@ -832,6 +838,9 @@ func (k *JWTMiddleware) processCentralisedJWT(r *http.Request, token *jwt.Token)
832838

833839
func (k *JWTMiddleware) getScopeClaimNameOAS(claims jwt.MapClaims) string {
834840
claimNames := k.Spec.OAS.GetJWTConfiguration().Scopes.Claims
841+
if len(claimNames) == 0 && k.Spec.OAS.GetJWTConfiguration().Scopes.ClaimName != "" {
842+
claimNames = []string{k.Spec.OAS.GetJWTConfiguration().Scopes.ClaimName}
843+
}
835844
for _, claimName := range claimNames {
836845
for k := range claims {
837846
if k == claimName {

‎gateway/mw_jwt_test.go

Lines changed: 140 additions & 104 deletions
Original file line numberDiff line numberDiff line change
@@ -1211,6 +1211,19 @@ func TestGetPolicyIDFromToken(t *testing.T) {
12111211
spec.OAS.ExtractTo(spec.APIDefinition)
12121212
},
12131213
},
1214+
{
1215+
name: "is oas set policyFieldName, no BasePolicyClaim",
1216+
claims: jwt.MapClaims{
1217+
"policy": "mainpolicy",
1218+
},
1219+
expectedBool: true,
1220+
expected: "mainpolicy",
1221+
modifySpec: func(spec *APISpec) {
1222+
spec.IsOAS = true
1223+
spec.OAS.GetJWTConfiguration().PolicyFieldName = "policy"
1224+
spec.OAS.ExtractTo(spec.APIDefinition)
1225+
},
1226+
},
12141227
{
12151228
name: "is oas second",
12161229
claims: jwt.MapClaims{
@@ -2907,119 +2920,131 @@ func TestGetUserIDFromClaim(t *testing.T) {
29072920
assert.Equal(t, identity, userID)
29082921
})
29092922

2910-
t.Run("is OAS", func(t *testing.T) {
2911-
testCases := []struct {
2912-
name string
2913-
expectedErr error
2914-
expected string
2915-
claims jwt.MapClaims
2916-
subjectClaims []string
2917-
}{
2918-
{
2919-
name: "identity base field exists",
2920-
claims: jwt.MapClaims{
2921-
"user_id": userID,
2922-
"iss": "example.com",
2923-
},
2924-
subjectClaims: []string{"user_id"},
2925-
expected: userID,
2926-
},
2927-
{
2928-
name: "second identity base field exists",
2929-
claims: jwt.MapClaims{
2930-
"backup_user_id": userID,
2931-
"iss": "example.com",
2932-
},
2933-
subjectClaims: []string{"user_id", "backup_user_id"},
2934-
expected: userID,
2923+
}
2924+
2925+
func TestGetUserIDFromClaimOAS(t *testing.T) {
2926+
userID := "123"
2927+
testCases := []struct {
2928+
name string
2929+
expectedErr error
2930+
expected string
2931+
claims jwt.MapClaims
2932+
subjectClaims []string
2933+
identityBaseField string
2934+
}{
2935+
{
2936+
name: "identity base field exists",
2937+
claims: jwt.MapClaims{
2938+
"user_id": userID,
2939+
"iss": "example.com",
29352940
},
2936-
{
2937-
name: "no identity base field exists, fallback to sub",
2938-
claims: jwt.MapClaims{
2939-
"iss": "example.com",
2940-
"sub": userID,
2941-
},
2942-
subjectClaims: []string{"user_id", "backup_user_id"},
2943-
expected: userID,
2941+
subjectClaims: []string{"user_id"},
2942+
expected: userID,
2943+
},
2944+
{
2945+
name: "use identity base instead of subject base fields",
2946+
claims: jwt.MapClaims{
2947+
"user_id": userID,
2948+
"iss": "example.com",
29442949
},
2945-
{
2946-
name: "sub in identity base fields",
2947-
subjectClaims: []string{"user_id", "sub"},
2948-
expected: userID,
2949-
claims: jwt.MapClaims{
2950-
"iss": "example.com",
2951-
"sub": userID,
2952-
},
2950+
identityBaseField: "user_id",
2951+
expected: userID,
2952+
},
2953+
{
2954+
name: "second identity base field exists",
2955+
claims: jwt.MapClaims{
2956+
"backup_user_id": userID,
2957+
"iss": "example.com",
29532958
},
2954-
{
2955-
name: "sub in identity base fields, but not in claims",
2956-
subjectClaims: []string{"user_id", "sub"},
2957-
claims: jwt.MapClaims{},
2958-
expectedErr: ErrNoSuitableUserIDClaimFound,
2959+
subjectClaims: []string{"user_id", "backup_user_id"},
2960+
expected: userID,
2961+
},
2962+
{
2963+
name: "no identity base field exists, fallback to sub",
2964+
claims: jwt.MapClaims{
2965+
"iss": "example.com",
2966+
"sub": userID,
29592967
},
2960-
{
2961-
name: "no identity base fields and no sub",
2962-
subjectClaims: []string{"user_id", "backup_user_id"},
2963-
expectedErr: ErrNoSuitableUserIDClaimFound,
2964-
claims: jwt.MapClaims{
2965-
"iss": "example.com",
2966-
},
2968+
subjectClaims: []string{"user_id", "backup_user_id"},
2969+
expected: userID,
2970+
},
2971+
{
2972+
name: "sub in identity base fields",
2973+
subjectClaims: []string{"user_id", "sub"},
2974+
expected: userID,
2975+
claims: jwt.MapClaims{
2976+
"iss": "example.com",
2977+
"sub": userID,
29672978
},
2968-
{
2969-
name: "no configured base fields and sub",
2970-
claims: jwt.MapClaims{
2971-
"sub": userID,
2972-
},
2973-
expected: userID,
2979+
},
2980+
{
2981+
name: "sub in identity base fields, but not in claims",
2982+
subjectClaims: []string{"user_id", "sub"},
2983+
claims: jwt.MapClaims{},
2984+
expectedErr: ErrNoSuitableUserIDClaimFound,
2985+
},
2986+
{
2987+
name: "no identity base fields and no sub",
2988+
subjectClaims: []string{"user_id", "backup_user_id"},
2989+
expectedErr: ErrNoSuitableUserIDClaimFound,
2990+
claims: jwt.MapClaims{
2991+
"iss": "example.com",
29742992
},
2975-
{
2976-
name: "empty identity base field",
2977-
subjectClaims: []string{"user_id", "backup_user_id"},
2978-
claims: jwt.MapClaims{
2979-
"iss": "example.com",
2980-
"user_id": "",
2981-
},
2982-
expectedErr: ErrEmptyUserIDInClaim,
2993+
},
2994+
{
2995+
name: "no configured base fields and sub",
2996+
claims: jwt.MapClaims{
2997+
"sub": userID,
29832998
},
2984-
{
2985-
name: "no configured base field and no sub",
2986-
claims: jwt.MapClaims{},
2987-
expectedErr: ErrNoSuitableUserIDClaimFound,
2999+
expected: userID,
3000+
},
3001+
{
3002+
name: "empty identity base field",
3003+
subjectClaims: []string{"user_id", "backup_user_id"},
3004+
claims: jwt.MapClaims{
3005+
"iss": "example.com",
3006+
"user_id": "",
29883007
},
2989-
}
2990-
for _, tc := range testCases {
2991-
t.Run(tc.name, func(t *testing.T) {
2992-
var api apidef.APIDefinition
2993-
api.EnableJWT = true
2994-
api.AuthConfigs = map[string]apidef.AuthConfig{
2995-
apidef.JWTType: {
2996-
Name: "jwtAuth",
2997-
AuthHeaderName: "Authorization",
2998-
},
2999-
}
3000-
api.IsOAS = true
3001-
3002-
var o oas.OAS
3003-
o.Fill(api)
3004-
o.GetJWTConfiguration().SubjectClaims = tc.subjectClaims
3005-
middleware := JWTMiddleware{&BaseMiddleware{Spec: &APISpec{
3006-
OAS: o,
3007-
APIDefinition: &api,
3008-
}}}
3009-
3010-
identity, err := middleware.getUserIdFromClaim(tc.claims)
3011-
if tc.expectedErr != nil {
3012-
assert.ErrorIs(t, err, tc.expectedErr)
3013-
} else {
3014-
assert.NoError(t, err)
3015-
assert.Equal(t, identity, tc.expected)
3016-
}
3017-
})
3018-
}
3019-
})
3008+
expectedErr: ErrEmptyUserIDInClaim,
3009+
},
3010+
{
3011+
name: "no configured base field and no sub",
3012+
claims: jwt.MapClaims{},
3013+
expectedErr: ErrNoSuitableUserIDClaimFound,
3014+
},
3015+
}
30203016

3021-
}
3017+
for _, tc := range testCases {
3018+
t.Run(tc.name, func(t *testing.T) {
3019+
var api apidef.APIDefinition
3020+
api.EnableJWT = true
3021+
api.AuthConfigs = map[string]apidef.AuthConfig{
3022+
apidef.JWTType: {
3023+
Name: "jwtAuth",
3024+
AuthHeaderName: "Authorization",
3025+
},
3026+
}
3027+
api.IsOAS = true
30223028

3029+
var o oas.OAS
3030+
o.Fill(api)
3031+
o.GetJWTConfiguration().SubjectClaims = tc.subjectClaims
3032+
o.GetJWTConfiguration().IdentityBaseField = tc.identityBaseField
3033+
middleware := JWTMiddleware{&BaseMiddleware{Spec: &APISpec{
3034+
OAS: o,
3035+
APIDefinition: &api,
3036+
}}}
3037+
3038+
identity, err := middleware.getUserIdFromClaim(tc.claims)
3039+
if tc.expectedErr != nil {
3040+
assert.ErrorIs(t, err, tc.expectedErr)
3041+
} else {
3042+
assert.NoError(t, err)
3043+
assert.Equal(t, identity, tc.expected)
3044+
}
3045+
})
3046+
}
3047+
}
30233048
func TestJWTMiddleware_getSecretToVerifySignature_JWKNoKID(t *testing.T) {
30243049
const jwkURL = "https://jwk.com"
30253050

@@ -3459,6 +3484,7 @@ func TestJWTMiddleware_getScopeClaimNameOAS(t *testing.T) {
34593484
tests := []struct {
34603485
name string
34613486
claimNames []string
3487+
claimName string
34623488
claims jwt.MapClaims
34633489
want string
34643490
}{
@@ -3476,6 +3502,15 @@ func TestJWTMiddleware_getScopeClaimNameOAS(t *testing.T) {
34763502
},
34773503
want: "scope",
34783504
},
3505+
{
3506+
name: "claim exists in deprecated claimName field",
3507+
claimNames: []string{},
3508+
claims: jwt.MapClaims{
3509+
"scope": "read write",
3510+
},
3511+
claimName: "scope",
3512+
want: "scope",
3513+
},
34793514
{
34803515
name: "claim exists - multiple claim names, first match",
34813516
claimNames: []string{"scp", "scope", "permissions"},
@@ -3529,7 +3564,8 @@ func TestJWTMiddleware_getScopeClaimNameOAS(t *testing.T) {
35293564
o.SetTykExtension(&oas.XTykAPIGateway{})
35303565
o.Fill(api)
35313566
o.GetJWTConfiguration().Scopes = &oas.Scopes{
3532-
Claims: tt.claimNames,
3567+
Claims: tt.claimNames,
3568+
ClaimName: tt.claimName,
35333569
}
35343570
mw := JWTMiddleware{
35353571
BaseMiddleware: &BaseMiddleware{

0 commit comments

Comments
 (0)