Skip to content

Add test coverage for jwt-auth module#4338

Merged
janhoy merged 3 commits intoapache:mainfrom
janhoy:feature/jwt-auth-tests-premigration
Apr 26, 2026
Merged

Add test coverage for jwt-auth module#4338
janhoy merged 3 commits intoapache:mainfrom
janhoy:feature/jwt-auth-tests-premigration

Conversation

@janhoy
Copy link
Copy Markdown
Contributor

@janhoy janhoy commented Apr 26, 2026

Used AI coding assistant to find gaps in test coverage for the jwt-auth module. With this PR we cover more code paths with tests. This makes us more prepared to migrate to a new JWT library in #4334 (which has the same tests added)

janhoy added 2 commits April 26, 2026 02:46
Increases test coverage for key resolution, issuer fallback, clock skew
tolerance, scope handling, and JWK parsing — all using the existing jose4j
API so tests pass on main before the nimbus migration is merged.

New tests in JWTVerificationkeyResolverTest:
- noIssRequireIssuerFalseSingleIssuerFallback: null iss + single issuer falls back
- noIssRequireIssuerFalseMultipleIssuersThrows: null iss + multiple issuers → SolrException
- issMismatchSingleIssuerBackCompatFallback: unrecognised iss + single issuer falls back
- issMismatchMultipleIssuersThrows: unrecognised iss + multiple issuers → UnresolvableKeyException
- ecKeyTypeMaterialisedCorrectly: EC key resolves as ECPublicKey

New tests in JWTAuthPluginTest:
- requireIssuerFalseButIssPresentAndMismatches: mismatched iss → JWT_VALIDATION_EXCEPTION
- requireIssuerFalseNoIssInTokenOrConfig: absent iss + requireIss=false → authenticated
- scopeClaimAsJsonArray: scope as JSON array is parsed and filtered correctly
- tokenExpiredWithinClockSkewIsAuthenticated: exp=now-25s → authenticated (30s skew)
- tokenExpiredBeyondClockSkewIsRejected: exp=now-35s → JWT_EXPIRED

New test in JWTIssuerConfigTest:
- parseJwkSetSingleBareJwk: bare JWK map (no "keys" wrapper) → JWKSet with 1 key
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds additional unit tests to improve branch/edge-case coverage in the jwt-auth module, helping reduce risk ahead of the planned JWT library migration in #4334.

Changes:

  • Extend JWTVerificationkeyResolverTest to cover issuer-selection edge cases (missing/unknown iss) and EC key materialization.
  • Extend JWTAuthPluginTest to cover requireIss=false behavior nuances, scope claim as JSON array, and clock-skew tolerance around expiry.
  • Extend JWTIssuerConfigTest to cover parsing a single bare JWK map (no "keys" wrapper).

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
solr/modules/jwt-auth/src/test/org/apache/solr/security/jwt/JWTVerificationkeyResolverTest.java Adds resolver tests for missing/unknown issuer handling and EC key type resolution.
solr/modules/jwt-auth/src/test/org/apache/solr/security/jwt/JWTIssuerConfigTest.java Adds test for parseJwkSet bare single-JWK map branch.
solr/modules/jwt-auth/src/test/org/apache/solr/security/jwt/JWTAuthPluginTest.java Adds tests for requireIss=false semantics, scope-as-array parsing, and expiry clock-skew behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

…ew tests

Avoids forbidden API (System#currentTimeMillis) flagged by forbiddenApisTest.
@janhoy janhoy merged commit a7056c0 into apache:main Apr 26, 2026
5 checks passed
@janhoy janhoy deleted the feature/jwt-auth-tests-premigration branch April 26, 2026 11:17
janhoy added a commit that referenced this pull request Apr 26, 2026
janhoy added a commit that referenced this pull request Apr 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants