SOLR-18211 Migrate jwt-auth module from bc-jose4j to nimbus-jose-jwt#4334
Draft
janhoy wants to merge 12 commits intoapache:mainfrom
Draft
SOLR-18211 Migrate jwt-auth module from bc-jose4j to nimbus-jose-jwt#4334janhoy wants to merge 12 commits intoapache:mainfrom
janhoy wants to merge 12 commits intoapache:mainfrom
Conversation
The old name reflected the jose4j VerificationKeyResolver interface it used to implement. The class now implements JWSKeySelector<SecurityContext> from nimbus-jose-jwt, with per-issuer key lookup as its key feature. Also add JIRA link to changelog entry.
github-actions
Bot
added
dependencies
The refreshReprieveThreshold was stored on HttpsJwksFactory but never passed to JwkSetFetcher, making it dead code. Repeated unknown-kid requests would trigger a remote refresh on every auth attempt. Wire the threshold through to JwkSetFetcher and track lastRefreshTime: forced refresh() calls within the reprieve window are now skipped.
…ormat
asConfig() was storing the JWK as a flat List<Map>, but setJsonWebKeySet(Object)
expects a Map (JWK Set with 'keys' field or single JWK map). A config edit
round-trip would cause a ClassCastException on reload.
Wrap the key list in the standard JWK Set format {"keys": [...]} so it
round-trips correctly through parseJwkSet().
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
requireIssuer controls whether the iss claim must be present in the token. It should not control whether a mismatching iss value is silently accepted when the claim is present. A token whose iss doesn't match any configured issuer should always be rejected if issuers are explicitly configured.
…path SolrException (unchecked) thrown inside selectJWSKeys() escapes the Nimbus DefaultJWTProcessor without being caught, producing an unexpected 500 instead of a JWT validation failure. Use KeySourceException so it propagates as a JOSEException and is handled cleanly.
13 new tests across IssuerAwareJWSKeySelectorTest, JWTAuthPluginTest, and JWTIssuerConfigTest covering: all resolveIssuerConfig() branches, EC key materialisation, requireIssuer=false semantics, scope-as-array, asConfig() JWK round-trip, clock-skew tolerance, JwkSetFetcher cache expiry, and refresh reprieve suppression. Doc fixes in jwt-authentication-plugin.adoc: - blockUnknown default corrected from true to false (code has always used getOrDefault(..., false); the wrong default was introduced by a docs-only change in apache/lucene-solr#805 / SOLR-13649) - aud default corrected: no clientId fallback exists in the code; audience validation is simply skipped when aud is not configured - adminUiScope: document the second fallback to hardcoded "solr" - requireExp: document the 30-second clock-skew tolerance
The table entry now clearly states the default is false and explains both use cases (enforcement vs. open configuration). The intro paragraph is rewritten as a single sentence that leads with the default behaviour.
github-actions
Bot
added
the
documentation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
https://issues.apache.org/jira/browse/SOLR-18211
Motivation
org.bitbucket.b_c:jose4jis unmaintained — its last release was March 2024 and it is a single-maintainer project.com.nimbusds:nimbus-jose-jwtis a well-maintained, widely-used JOSE/JWT library that was already present as atest dependency in this module.
Changes
JWTIssuerConfigJsonWebKeySet/JsonWebKeywith nimbusJWKSet/JWKHttpsJwkswith a new inner classJwkSetFetcherwrapping nimbus'sResourceRetrieverwith synchronized caching and a configurable TTLResourceRetrieverimplementation preserves existing behaviour: trusted SSL certificates and hostname verification bypass for localhost endpointsIssuerAwareJWSKeySelector(previouslyJWTVerificationkeyResolver)JWSKeySelector<SecurityContext>)IssuerContext implements SecurityContextto thread the JWT's (unverified) issuer claim from the payload to the key selector, enabling per-issuer key lookupJWKSelector+JWKMatcher.forJWSHeader()to match onkidand algorithmJWTAuthPluginJwtConsumer/JwtConsumerBuilderwithDefaultJWTProcessor<SecurityContext>+DefaultJWTClaimsVerifierissvalue, the token's issuer must matchTesting
All 65 existing tests pass unchanged (1 skipped). Passes
forbiddenApis,ecjLint,spotlessCheck, andvalidateLogCalls.To further lower the risk of this migration, 13 new unit tests are added across the three test classes:
IssuerAwareJWSKeySelectorTest— covers all previously untested branches ofresolveIssuerConfig(): null issuer with single multiple issuers, unrecognised issuer with single/multiple issuers, and correct materialisation of EC keys asECPublicKeyJWTAuthPluginTest— covers:requireIssuer=falsedoes not bypass issuer value validation;requireIssuer=falsewith no issuer in token or config authenticates successfully;scopeclaim expressed as a JSON array;asConfig()JWK round-trip produces a functional plugin; clock-skew tolerance (25 s expired → authenticated; 35 s expired →JWT_EXPIRED)JWTIssuerConfigTest— covers:parseJwkSetwith a bare single-JWK map (no"keys"wrapper);JwkSetFetcherrefresh reprieve suppressing a second call within the window;JwkSetFetchercache expiry triggering a re-fetchDocs
While reviewing the code for this migration, several bugs were found in
jwt-authentication-plugin.adocand fixed:blockUnknowndefault isfalse, nottrue— the ref guide has documented the default astruesince apache/lucene-solr#805 (SOLR-13649), a docs-only PR merged in 9.0 that appears to have introduced the wrong default. The code has always usedgetOrDefault(PARAM_BLOCK_UNKNOWN, false), meaning unauthenticated requests pass through unlessblockUnknownis explicitly set totrue. The intro paragraph ("The plugin will by default require a valid JWT for all traffic") has also been corrected to match.auddefault — documented as "UsesclientIdif configured"; the code has no such fallback. Ifaudis not set, audience validation is skipped entirely.adminUiScopefallback — documented as falling back to the firstscopeentry; the code has a second fallback to the hardcoded string"solr"that was not mentioned.exptolerance is applied by the plugin but was not documented anywhere; added a note to therequireExprow.Disclaimer: This migration is done with help of Claude Code.