KNOX-3405 - Extend JWTFederationFilter for dynamic JWKS and iss attribute on token-exchange - #1342
Open
hsheinblatt wants to merge 4 commits into
Open
KNOX-3405 - Extend JWTFederationFilter for dynamic JWKS and iss attribute on token-exchange#1342hsheinblatt wants to merge 4 commits into
hsheinblatt wants to merge 4 commits into
Conversation
added 4 commits
August 10, 2026 18:33
…bute on token-exchange Test issues to improve: Several tests use a helper rather than a mock, DynamicJwksPassTokenAuthority. This causes ambiguity in what method precisely was called and what failed. Additionally, fixing that becomes more complex because the token exchange requests use both a subject token and an actor token, so both tokens are validated, and it's either ambiguous or complex to ensure that each token validation path is correct. EasyMock should allow a range of times(0, 1) to be called, so we can make the appropriate signature verification optional for negative tests. That is, we can write the tests so that the order of enforcement for each condition is arbitrary: all the other conditions would evaluate to true if executed first. Once the actor token is made optional, we can simplify the negative test cases to use only a subject token, and then specify the mocks precisely so each negative test case validates the correct methods are called, if called, and no extra methods are called, the test can be insensitive to the order of validation checks, and we can remove the DynamicJwksPassTokenAuthority helper.
…alidation independent, and ensure that both the single subject token and the subject plus actor token request paths are covered.
TokenExchangeHandler is not public, and likely it shouldn't be. But these constants need to be used outside the package, at least for testing, and are generic OIDC constants, so likely will need wider scope in the future. Moved them up a level to JWTFederationFilter so they are public without making TokenExchangeHandler public.
JWTFederationFilter so that the tests outside the package, namely JWTFederationFilterTokenExchangeTest, can read the values. Update JWTFederationFilterTokenExchangeTest to mock the subject token and actor token types given that they are now required.
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
1 participant
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.
KNOX-3405 - Extend JWTFederationFilter for dynamic JWKS and iss attribute on token-exchange
What changes were proposed in this pull request?
JWTFederationFilter and the base AbstractJWTFilter are modified to use dynamically
discovered JWKS for validating token signatures for RFC 8693 token exchange.
AbstractJWTFilter.validateToken first checks if the issuer is an expected issuer.
That is, defined in the static topology constant list of allowed issuers. This is
the pre-existing path. If the token issuer is in the expected issuer list, then the
logic is unchanged, the token validate proceeds as it had.
If the token issuer is not part of the expected list, then it is checked against the
TrustedOIDCIssuer registry, if it's enabled. If it matches, then the JWKS dynamically
discovered for that issuer is used to validate the token signature. The rest of the token
validation is the same as for the existing case. That is, the other fields like expiry and nbf
remain validated in the path with the same logic.
JWTFederationFilter was also modified to store the validated token's iss claim as a request
attribute for downstream handlers.
Some RFC 8693 token exchange related constants were moved from TokenExchangeHandler
to JWTFederationFilter so that the tests outside the package, namely
JWTFederationFilterTokenExchangeTest, can read the values.
How was this patch tested?
Unit tests were added for the JWKS dynamic discovery for trusted OIDC issuers paths.
These include success paths where the token signature is validated using the dynamically
discovered JWKS and a similar test with both actor and subject tokens where one is from
an issuer in the configured list and the other is discovered dynamically. Negative tests
for invalid tokens are added to show that signature validation must pass, and the same
checks conducted for the static issuer path is enforced in the dynamic issuer path as well,
such as expiry and nbf. Tests are added for paths where the token issuer is not in the
static list, but also not valid for dynamic discovery. These include an unregistered issuer--
one not registered as a trusted OIDC issuer--, when the TrustedOIDCIssuer
service is not configured, and when the request is not a token exchange request. Unit
tests were also added to show that token validation for an issuer that is statically defined
will not fail over to a dynamically registered issuer path, the existing logic is used when
the issuer is a statically defined issuer regardless of whether or not it's also a dynamically
trusted issuer, a sanity that the new dynamic path is not called in a successful
validation using the existing statically defined issuer path, and the logic for a non-token
exchange request does not use the new path.
A unit test was added for the iss request claim added.
Integration Tests
No integration tests were added. They will be once the full flow is implemented.
UI changes
N/A