Conversation
* KnoxIDF - Initial commit * KnoxIDF - multi OP support * KnoxIDF - make token endpoint configurable during discovery * KnoxIDF - Code cleanup and bug fixes * KnoxIDF - Multi OP enablement improvements and code adoption to Larry's recent changes * KnoxIDF - Add REFRESH_TOKEN support * KnoxIDF - Automatically enable JdbcFederatedIdentityService when KnoxIDF is present in any topology * KnoxIDF - Added Docker-based integration tests * KnoxIDF: configurable user params provider (only LDAP for now) * KnoxIDF: add support for auth code flow with PKCE * KnoxIDF: fix an issue with the empty user params provider implementation * KnoxIDF: Refactor Docker build to use local Maven artifacts and unify CI/Dev workflows
* KNOX-3355 - Add TrustedOidcIssuerService schema and interface Co-authored-by: Harrison <hsheinblatt@cloudera.com>
…d TrustedOidcIssuerServiceFactory (#1315)
…d the missing methods (#1322)
…limit-reached instead of 500 (#1328)
…Authz (#1337) Remove redundant admin URL paths, still allowing separate ACLs for different knox idf admin APIs using PathAclAuthz. Co-authored-by: Harrison <hsheinblatt@cloudera.com>
…change (#1339) * KNOX-3408 - Regression tests for subject handling in JWTFederationFilter.handleTokenExchange and TokenExchangePrincipal handling in AbstractIdentityAssertionFilter#continueChainAsPrincipal handling Only unit tests are added for existing functionality. * KNOX-3408 - Allow no actor token in JWTFederationFilter.handleTokenExchange. --------- Co-authored-by: Harrison <hsheinblatt@cloudera.com>
…hangeHandlerTest; drop redundant JWTFederationFilterHandleTokenExchangeTest
…bute on token-exchange (#1342)
Fixes a batch of security and correctness findings from reviewing the squashed "Knox as OIDC Provider" feature. Reviewed and tested together. Tier 1 (critical): - 1.1 Authenticate the client on the auth-code token exchange. The JWTFederationFilter Bearer path forwards to the token endpoint without checking client_secret, so a stolen code could be redeemed by anyone holding some valid Knox JWT. The endpoint now independently binds redemption to the client: PKCE code_verifier (S256) when a challenge was stored, else a constant-time client_secret check. - 1.2 Validate the federated id_token (signature via the OP JWKS, expected issuer, audience, exp/nbf) before trusting any claim; fail closed when jwks.endpoint/issuer are not configured. - 1.3 Add knoxidf.client.registration.anonymous.allowed (default false): dynamic client registration refuses anonymous callers unless explicitly enabled. Sample knoxidf-ldap topology opts in to preserve open reg. - 1.4 Stop leaking custom claims across users: build a per-request copy of the claim map instead of mutating the shared singleton field. Tier 2 (high): - 2.1 Default issueTime to now in JWTokenAttributesBuilder so every token gets a correct iat (fixes iat=1970 on KnoxSSO cookies/assertions). - 2.2 Set SCOPE_ATTRIBUTE to the scope value (was a double getClaim -> null). - 2.3 Match redirect_uri on parsed URI components with a path/host boundary (fixes wildcard open-redirect via startsWith). - 2.5 Require S256 PKCE; reject plain. - 2.6 Use the Knox truststore for federated OP HTTP calls. - 2.7 Treat auto_consent as a server-side policy, not a client bypass. - 2.8 Escape username in LDAP DN/filter (Rdn.escapeValue). - 2.9 Require and validate state on the authorize flow. - 2.10 Null-guard the federated callback / registration and return 4xx. Tests: TokenResourceClientAuthTest, RegistrationResourceAnonymousGuardTest, FederatedOpConfigurationTest, JWTTokenTest iat assertion. Deferred to follow-up PRs: 1.5 (secrets at rest via AliasService), 2.4 + 2.11 (atomic single-use auth-code consume + schema migration across dialects; the auth-code replay window remains until 2.4 lands), Tier 3/4. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…nce (1.5, 2.4, 2.11) Three KnoxIDF security/correctness hardening findings from the OIDC-provider review, landed together. 1.5 - Secrets at rest - Federated OP client_secret is now resolvable via AliasService. New optional topology param federated.op.<name>.clientSecret.alias (FederatedOpConfiguration). AuthorizeResource.resolveClientSecret prefers the alias (getPasswordFromAliasForCluster, cluster from GATEWAY_CLUSTER_ATTRIBUTE with NO_CLUSTER_NAME fallback); fails closed if a configured alias is unresolvable rather than leaking the plaintext param. Plaintext clientSecret retained only as a fallback when no alias is set. - Federated OP access token is no longer persisted at rest: decorateAuthCodeToken stores only the FEDERATED_IDENTITY_ID pointer. Removed the now-dead split/join/chunk token helpers and FEDERATED_*_TOKEN_PREFIX constants. 2.4 - Single-use authorization codes (close the replay window) - SPI TokenStateService: new default consumeToken(String) - atomic single-use consume; exactly one concurrent caller receives true, an absent token is false (never throws). - DefaultTokenStateService: overrides consumeToken with a true atomic claim (tokenExpirations.remove(id) != null), evicting the remaining per-token state. - JDBCTokenStateService: overrides consumeToken using the primary-key DELETE as the atomic arbiter, evicts the in-memory cache, and fails closed on SQLException (the inherited removeToken swallows it and would falsely report a win). Derby inherits this. - idf TokenResource: validateAuthCode now returns the metadata it already reads; handleAuthorizationCodeFlow validates, then atomically consumes the code BEFORE issuing any token, stashing the metadata in a per-request attribute consumed by the issuance steps (getAuthCodeMetadata). The revoke-in-finally is removed. A code that fails validation is deliberately NOT consumed, so replaying with bad params cannot burn a victim's still-valid code. 2.11 - Persistence hardening - Consent key reshaped to fit KNOX_TOKEN_METADATA.md_name VARCHAR(32): AuthorizeResource.consentMetadataKey(subject) = "consent_" + first-20-hex(SHA-256(subject)) (28 chars), used by both hasConsent and markConsentAccepted so read and write agree. No schema change to md_name. - Derby DDL parity: added the missing NOT NULL constraints to the federated identity and attribute tables (the UNIQUE index was already present). - TOCTOU: JdbcFederatedIdentityService.addFederatedIdentity now inserts and catches instead of check-then-insert; a unique-constraint violation (SQLIntegrityConstraintViolationException or SQLState class 23, walked up the cause chain) is treated as a benign already-exists. The unique index is the arbiter. - Transaction boundary: FederatedIdentityDatabase.addFederatedIdentity writes the core row and attribute rows on a single connection with autocommit off, then commits (rollback on failure) so an identity is never persisted without its attributes. - Double-checked locking: added the missing inner recheck in JdbcFederatedIdentityService.init; corrected its misleading exception message. - SELECT * replaced with the explicit column list in the by-provider/issuer/subject query; KnoxDatabase resolves DDL via getClass().getClassLoader() so each subclass loads its own create*.sql. Tests - DefaultTokenStateServiceTest: +3 consumeToken tests (single-use, state removed, unknown id). - New TokenResourceAuthCodeReplayTest: a losing (already-consumed) redemption yields invalid_grant with no issuance; a winning redemption issues exactly once. - New ConsentMetadataKeyTest: key fits VARCHAR(32) for realistic subjects, deterministic, distinct subjects -> distinct keys. - FederatedOpConfigurationTest: +2 (clientSecret alias read / absent by default). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Tier 3 (hardening): - Registration redirect_uris require HTTPS (plain HTTP only for loopback, RFC 8252) - XSS: escape unknown scopes in AuthConsentServlet; rebuild knoxauth.js OP links via the DOM API instead of raw HTML / inline onclick interpolation - URL-encode clientId and federated-OP redirect params in AuthorizeResource / KnoxIDFUtils - Single-use consent/federation state: add KnoxIDFArtifactStore.remove() and invalidate state after use in authCallback and consentAccepted - Reject disabled refresh tokens (isEnabled check) in TokenResource - Remove hardcoded LDAP "admin-password" fallback; fail fast without a configured alias - DiscoveryResource: literal String.replace instead of regex replaceAll for topology name - error() maps OAuth error codes to correct HTTP status per RFC 6749 5.2 (no longer always 401) - RedirectToUrlFilter: treat a blank fedOpSid the same as absent Tier 4 (polish): - Fix public-API typos: EmptyFederatedIdentityService, BASE_RESOURCE_PATH - Make ALLOWED_RESPONSE_TYPES / DEFAULT_SCOPES immutable; copy at mutating call sites - Drop nonce from the UserInfo response (id_token only) - Document the KnoxIDFArtifactStore ttl*2 grace window Tests: KnoxIDFUtilsErrorStatusTest, RegistrationRedirectUriPolicyTest, KnoxIDFArtifactStoreTest (+18); TokenResourceAuthCodeReplayTest updated 401->400. All affected modules green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…g fixes and changes
…ated-identity persistence Adds an opt-in end-to-end test that stands up a real Keycloak as the external OpenID Provider and drives the full broker flow through Knox (register client, /authorize -> SSOCookie redirect -> Keycloak login -> callback -> token exchange). It is layered in via docker-compose.knoxidf-federation.yml and is NOT part of the default test run (the base compose ignores test_knoxidf_federation.py). Surfacing that path exposed a production bug baked into the OIDC-provider squash: federated-identity persistence never activated. The service factories decided whether to use the JDBC-backed store by calling TopologyService.getTopologies() at gateway-service-init time, but topologies are not loaded yet at that point, so the no-op EmptyFederatedIdentityService was chosen in every deployment and the token exchange failed with "Federated identity not found". Fixes: - Move isKnoxIdfEnabledInAnyTopology into AbstractServiceFactory and add a topology-directory disk-scan fallback for when getTopologies() is still empty at init time; match both KNOXIDF and KNOXIDF_ADMIN roles. TrustedOidcIssuerServiceFactory now delegates to the shared helper. - Add a self-provisioning DerbyDBFederatedIdentityService (mirrors DerbyDBTokenStateService) and 3-way backend auto-selection: explicit impl wins; otherwise an operator-configured external DB -> JDBC, else the embedded Derby default so federation works out of the box with no extra infrastructure. - SQL DDL: the create-table runner executes a single statement per file, so fold the federated-identity UNIQUE constraint inline and drop trailing semicolons. - Validate federated OP id_tokens with a JWS type verifier that accepts typ=JWT and an absent typ (Keycloak and most OPs set typ=JWT; the shared token authority rejects any typ'd token when no verifier is supplied). - getPrefix() now returns "knoxidf." (trailing dot) so knoxidf.knox.token.* topology params map onto the base KNOXTOKEN params; add the same override to AuthorizeResource so its callback-minted tokens honor the per-user limit/ttl configured on the topology. Covered by FederatedIdentityServiceFactoryTest, DerbyDBFederatedIdentityServiceTest, and the 3 federation E2E tests (all green); default KnoxIDF E2E and the knoxidf/ knoxtoken JUnit suites remain green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… finding H1) validateRefreshTokenGrant matched only the client_id carried on the refresh token and never proved client identity. Because the JWTFederationFilter Bearer path forwards a request to the token endpoint without checking client_secret, matching client_id alone would let anyone holding a stolen refresh token redeem and rotate it. Add a constant-time client_secret check (isValidClientSecret, MAC over tokenId+issueTime+userName+rawPasscode) after the client_id match, mirroring the client authentication the authorization_code grant already does. Covered by TokenResourceRefreshTokenClientAuthTest (valid secret passes; missing / wrong / bound-to-a-different-client secret rejected). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…iew finding H2) The federated broker leg sent no nonce to the external OP and never checked one on the callback, so a signature-valid id_token minted for a different (or attacker-initiated) authorization request could be injected/replayed at the callback. Add the OIDC nonce binding (OIDC Core 3.1.2.1): - New JVM-singleton FederatedNonceStore (mirrors FederatedOpConfigurationStore), single-use, keyed by the federated login-session id (== the state echoed by the OP). - WebSSOResource.federatedOpLogin mints a per-request nonce, stashes it, and passes it to the redirect builder. - KnoxIDFUtils.buildFederatedOpAuthRedirect now appends &nonce=<value> (percent-encoded). - AuthorizeResource.authCallback retrieves and single-use-removes the expected nonce and, only after the id_token's signature/issuer/audience are verified, requires its nonce claim to match (verifyFederatedNonce). Missing expected nonce or a mismatch fails the flow. Covered by AuthorizeResourceFederatedNonceTest (match passes; mismatch, missing claim, and missing expected nonce all rejected). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ew finding M1) handleRefreshToken revoked the presented refresh token with revokeToken() only after validation, then minted the replacement. On DefaultTokenStateService revokeToken is check-then-act, so two concurrent redemptions of the same refresh token could both pass validation and both mint a new access/refresh pair. (The JDBC/Derby path is already atomic via a PK DELETE; this is the in-memory gap.) Replace revokeToken with the atomic consumeToken as a single-use claim performed BEFORE issuance: exactly one concurrent caller wins and rotates, the losers get invalid_grant and mint nothing. Mirrors the consume-before-issue guard already on the authorization_code grant (finding 2.4). Covered by TokenResourceRefreshTokenRotationTest (a lost consume yields invalid_grant with no issuance). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…eview finding M2) The federated callback derives the Knox subject and the federated-identity primary key from the OP id_token's sub claim, and the identity tables declare external_subject NOT NULL. A broken or hostile OP that returns a verified id_token with no sub therefore drove a NOT NULL INSERT failure -> HTTP 500 on every callback through that OP (a targeted DoS). sub is REQUIRED by OIDC Core 2. Enforce its presence on the (already signature/issuer/audience-verified) id_token via requireFederatedSubject and return invalid_request when it is absent/blank, before any persistence happens. The iss claim is already covered: the configured expectedIssuer is non-blank, so a blank issuer fails the existing issuer-mismatch check. Covered by AuthorizeResourceFederatedSubjectTest. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…finding M3) matchesRedirectUri did a startsWith on the un-normalized requested path in the wildcard branch, so a registration of https://app.example/callback/* matched https://app.example/callback/../admin (path "/callback/../admin" starts with "/callback") and the authorization code was delivered to /admin -- a same-host open redirect. The exact, non-wildcard branch was already safe. Normalize both the base and requested paths (URI.normalize()) before the prefix compare so "/callback/../admin" collapses to "/admin" and no longer matches the "/callback" prefix. Genuine sub-paths under the wildcard prefix still match. Covered by AuthorizeResourceRedirectUriMatchTest. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…esolvable (review finding M4) resolveClientSecret returned null when a configured client-secret alias could not be resolved. That null flowed into the token-request form as a BasicNameValuePair, which the URL encoder serialized to a literal client_secret=null and sent to the OP -- the opposite of fail-closed: instead of refusing, Knox made a back-channel call with a bogus secret. Make it truly fail closed: requireResolvedAliasSecret throws ClientSecretResolutionException when a declared alias resolves to nothing (null or empty), and authCallback catches it and returns a clear server_error before any HTTP request to the OP. The plaintext clientSecret fallback (no alias configured) is unchanged for backward compatibility. Covered by AuthorizeResourceClientSecretResolutionTest. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…Service.init (review finding M5) init() tested initialized only at the outer (pre-lock) check and was missing the inner re-check under the lock that JdbcFederatedIdentityService has. A startup race where two threads both observed initialized==false could therefore both enter the critical section sequentially and initialize twice, the second run overwriting the already-built database and discoveryHelper references. Add the inner if (!initialized.get()) recheck inside the lock so a thread that blocked while another was initializing becomes a no-op. Mirrors the sibling JdbcFederatedIdentityService. Covered by JdbcTrustedOidcIssuerServiceTest#testConcurrentInitDoesNotReinitialize, which deterministically drives the race via the service's own init lock queue and asserts the database reference is not rebuilt once initialized. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…arer token (review finding M6) getTokenMetadata throws UnknownTokenException for an expired/revoked/unknown token, and doGet rethrew it as an unmapped RuntimeException -> HTTP 500. RFC 6750 requires a protected resource to answer such a token with 401 and a WWW-Authenticate: Bearer error="invalid_token" challenge. Catch UnknownTokenException in getUserInfo and return the RFC 6750 challenge via the new invalidToken() helper. Also handle a token that references a now-missing federated identity the same way (the token can no longer be honored) instead of throwing. doGet no longer wraps-and-rethrows. Covered by UserInfoResourceInvalidTokenTest. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… a query (review finding M7) redirectToAuthSuccess appended "?code=...&state=..." unconditionally. When the registered redirect_uri already carried a query string (e.g. https://app.example/cb?ui=dark), the result had two "?" separators, so the client parsed neither code nor state and the authorization-code flow silently failed for those clients. Extract buildSuccessRedirect, which uses "&" when the redirect_uri already contains a query string and "?" otherwise, preserving any pre-existing params. Covered by AuthorizeResourceSuccessRedirectTest. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…pported (review finding M8) The discovery document listed ["plain","S256"] for code_challenge_methods_supported, but AuthorizeResource rejects every code_challenge_method other than S256. A client that trusted discovery and used plain PKCE was then rejected at /authorize. Drop "plain" so discovery matches enforcement. Covered by DiscoveryResourcePkceMethodsTest. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… (J2) Add audit-log completeness for the KnoxIDF authorization server. Every security-relevant decision on the Authorize, Token, Registration and UserInfo endpoints is now recorded through Knox's existing Auditor/ AuditService framework with a consistent action/outcome/resource shape: - AuthorizeResource: authorization request rejected, authorization code issued / issuance failed, consent granted / denied / invalid-state, and the federated-OP callback outcome (single try/finally over all exits). - TokenResource: authorization_code and refresh_token grant outcomes (issued/rotated vs. replayed/validation-failed) and unsupported grant. - RegistrationResource: client-registration outcome incl. anonymous-denied and invalid_request reasons. - UserInfoResource: /userinfo access outcome incl. invalid_token and unknown-federated-identity reasons. Emission is centralized in a new KnoxIDFAudit holder so all records share one Auditor, a single masking rule and a consistent shape. Credentials and full tokens are NEVER logged: token ids/JWTs pass through mask() (prefix/ suffix only) and client_secret/code_verifier/raw refresh tokens are never recorded. The Auditor field is package-private/non-final for test injection, mirroring TrustedOidcIssuersResource. Covered by KnoxIDFAuditTest: a representative SUCCESS (rotated refresh grant) and FAILURE (unsupported grant) assert the emitted record, plus the mask() never-leaks invariant. Full knoxidf suite: 88/88 green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…tedOidcIssuerService default JDBCUtils.tableExists passed the table name verbatim for uppercase-storing engines (Derby), so a lowercase constant like federated_identity never matched Derby's uppercased FEDERATED_IDENTITY metadata. On restart tableExists returned false, createTableIfNotExists re-ran the CREATE, and Derby failed with "Table/View 'FEDERATED_IDENTITY' already exists". Uppercase-constant callers (KNOX_TOKENS, KNOX_PROVIDERS, TRUSTED_OIDC_ISSUERS) accidentally worked. Extract a normalizeIdentifier() helper that upper-cases for uppercase-storing engines and lower-cases for lowercase-storing ones. Also add DerbyDBTrustedOidcIssuerService, a self-provisioning embedded-Derby default mirroring DerbyDBFederatedIdentityService, and wire TrustedOidcIssuerServiceFactory to auto-select Derby vs JDBC based on external DB config instead of always using JDBC. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- JWTFederationFilter: add AuthCode TokenType; forward authorization_code grant to the token endpoint with an anonymous subject so public PKCE clients (no client_secret) are not rejected - TokenResource: gate auth-code handling behind isAuthCodeFlow() and fall back to super.doPost() for other grants (no separate KNOXTOKEN needed) - RegistrationResource: add knoxidf.custom.loopback.hosts allowlist for plain-HTTP redirect_uri hosts beyond localhost/127.0.0.1/::1 - DiscoveryResource: append /register to advertised registration_endpoint - TokenResourceV2: make RESOURCE_PATH public - Tests for AuthCode pass-through and the loopback-host policy - Docs: Polaris and Polaris Console integration guides; ignore built site/ Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
buildResponseMap gated id_token + refresh-token issuance behind isAuthCodeFlow() only, but the refresh_token grant also flows through it, so token rotation stopped returning a new refresh_token. Run that block for both authorization_code and refresh_token grants, still excluding client_credentials. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
doPost() now delegates unrecognized grant types to super.doPost() (so the KnoxIDF endpoint doubles as the Knox-token endpoint; no separate KNOXTOKEN service). Unknown grants are therefore no longer rejected with an unsupported_grant_type audit. Repoint the representative FAILURE-audit test at a still-existing failure path (a rejected refresh-token grant). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Convert testEmptyResolvedSecretFailsClosed to @test(expected=...) since its catch block makes no assertions on the exception. The sibling test that inspects the exception message keeps the try/fail/catch form. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
smolnar82
requested review from
bonampak,
hanicz,
lmccay,
moresandeep and
pzampino
August 13, 2026 19:31
Contributor
Author
Contributor
Author
|
Closing as I forgot to apply |
Test Results49 tests 49 ✅ 10s ⏱️ Results for commit d34a447. ♻️ This comment has been updated with latest results. |
…esource implement getPrefix()
smolnar82
changed the title
Close LDAP NamingEnumeration resource leaks in LdapUserParamsProvider by closing every search cursor in finally blocks. Enforce HTTPS on discovery-supplied dynamic JWKS URIs in JWTFederationFilter, with knox.token.exchange.dynamic.jwks.allow.http to opt out per provider. Return the canonical persisted FederatedIdentity on a concurrent first-login race so downstream auth codes are keyed to a real id instead of a phantom UUID. Bound the KnoxIDF federated-OP token-exchange HTTP client with configurable connect/read timeouts so an unresponsive external OP cannot pin request threads. Escape the OAuth state value in a double-quoted attribute in AuthConsentServlet to prevent single-quote attribute breakout XSS. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…token rotation Persist FEDERATED_IDENTITY_ID onto the rotated refresh token and restore it on the refresh_token grant so id_token generation keeps emitting federated profile claims after the first rotation (previously fell through to a local id_token carrying only sub+aud). Review by thanicz Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ire POST consentAccepted now verifies the current effective principal matches the subject that initiated the authorization request (rejecting a replayed consent-state URL from a different user with 403), and is a POST endpoint so it cannot be triggered by passive GET navigation; the consent form posts directly to the endpoints. Review by thanicz Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ng its body The federated token exchange now throws a dedicated FederatedTokenExchangeException carrying only the OP HTTP status; authCallback catches it, audits the real cause, and returns a generic server_error instead of letting a RuntimeException carrying the OP response body escape as a 500. Review by thanicz Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Reflect the consent-flow changes: the accept/deny endpoints are POST-only and consentAccepted is bound to the initiating subject (endpoints.md consent-page section + endpoint table; security.md Consent section). Review by thanicz Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… for federated login The login-session id used as the OIDC state sent to the external OP (and as the key for the in-flight authorize/OP-config/nonce stores) is now a per-flow UUID rather than request.getSession().getId(). This stops the JSESSIONID leaking to the OP via the state parameter and fixes last-writer-wins collision between concurrent authorization flows sharing one browser session. Review by thanicz Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A dynamically-registered client could previously put any scope in its allowed_scopes, letting it self-assign a privileged scope name (e.g. 'admin') that a downstream service might trust. Bound the registerable scopes by a server-side whitelist (knoxidf.registration.allowed.scopes): an explicit value is authoritative, a blank/unset value defaults to the OIDC-standard scope set, and 'openid' is always registerable. A blank allowed_scopes request receives the built-in defaults intersected with the whitelist so the default grant can never exceed operator policy. Review by thanicz Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…pAuthFilterTest The JWKS-URI HTTPS-enforcement hardening added a getInitParameter call for knox.token.exchange.dynamic.jwks.allow.http in JWTFederationFilter.init(). The EasyMock-based HadoopAuthFilterTest replays a strict mock and failed with an unexpected-method-call on that lookup. Add the missing expectation (returning null, i.e. HTTPS enforcement stays on) to the JWT-supported setup. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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
2 participants
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.
What changes were proposed in this pull request?
This PR introduces KnoxIDF, turning Apache Knox into an OAuth 2.0 / OpenID Connect Provider (OP) in its own right, while retaining Knox's existing role as a federation client. Historically Knox has only ever been a relying party (delegating auth to CAS/SAML/OIDC via pac4j); with KnoxIDF, Knox can now issue its own signed access and ID tokens, and optionally broker login to external OPs (Keycloak, Okta, Azure AD, Auth0, …) and re-issue Knox-signed tokens. Downstream services integrate with Knox exactly as they would with any standard OIDC provider.
Highlights
KnoxIDF is a new Knox service (role
KNOXIDF, with an admin counterpartKNOXIDF_ADMIN) that attaches to any topology and exposes standard OIDC endpoints:.well-known/openid-configuration),authorize,token,userinfo,jwks, and dynamic client registration.client_secretfor confidential clients.id_token, and re-issue Knox tokens; stable federated subject via UUIDv5.Main areas touched
gateway-service-knoxidf— the OIDC endpoint resources (AuthorizeResource,TokenResource,UserInfoResource,DiscoveryResource,JwksResource,RegistrationResource, consent servlet), audit, user-param providers, and deployment contributors.gateway-server— new services and JDBC-backed stores:TrustedOidcIssuerService,FederatedIdentityService(with Derby/JDBC/empty implementations and service factories), OIDC discovery helper, DB schema/DDL for trusted issuers and federated identity, and gateway-site config wiring.gateway-spi/gateway-util-common— federation and trusted-issuer SPI types,KnoxIDF*utility/constants/artifact-store classes,JWTokenAttributesextensions, new auditAction/ResourceTypevalues.gateway-provider-security-jwt) —JWTFederationFilter/AbstractJWTFilterextended for dynamic JWKS and theissattribute on token-exchange (KNOX-3405/3408), plusSSOCookieProvidersupport for the federated login page.WebSSOResource,TokenResource,JWKSResourceupdated to publish multiple JWKs and select the verification key bykid; knoxauth login page gains the "Continue with " federated sign-in option.mkdocs.ymlnav and architecture/screenshot assets.This branch also rolls up the incremental KnoxIDF sub-tasks that landed along the way (KNOX-3355, KNOX-3368, KNOX-3390, KNOX-3396, KNOX-3405, KNOX-3408) and a subsequent security-hardening pass (single-use auth codes, constant-time client-secret checks, fail-closed secret/alias resolution, redirect-URI normalization, OIDC nonce binding, refresh-token client auth).
How was this patch tested?
Unit tests. Extensive JUnit coverage was added across the new and modified modules, including:
TrustedOidcIssuerService(JDBC/Derby/empty + schema),FederatedIdentityService(Derby/JDBC + factory),OIDCDiscoveryHelper,DefaultTokenAuthorityServicemulti-JWK selection, token-state services.JWTFederationFiltertoken-exchange (dynamic JWKS,iss), OAuth-flows federation, SSO cookie provider.Docker-based integration tests. New end-to-end suites run against a real Knox distribution under Docker Compose:
test_knoxidf.py— exercises the KnoxIDF OIDC endpoints (discovery, client registration, Client Credentials and Authorization Code + PKCE flows, token/userinfo/JWKS) against theknoxidf-ldap,knoxidf-sso, andknoxidf-tokenCI topologies.test_knoxidf_federation.py— the federation case: stands up a real Keycloak as an external OpenID Provider (realm imported fromcompose/keycloak/realm.json) and drives the full broker flow — Knox delegates login to Keycloak, validates the OPid_token, and re-issues Knox tokens.Manual testing with Apache Polaris. I verified real downstream consumption of Knox-issued tokens by integrating KnoxIDF with Apache Polaris, documented under Integrations in the book:
internal/external/mixedrealms (machine-to-machine).Integration Tests
New integration tests were added under
.github/workflows/tests:test_knoxidf.py— KnoxIDF OIDC endpoint / flow coverage (runs on every PR).test_knoxidf_federation.py— full Keycloak-broker federation E2E (opt-in, see below).Supporting CI assets: new topologies (
knoxidf-ldap.xml,knoxidf-sso.xml,knoxidf-token.xml,knoxsso.xml),docker-compose.knoxidf-federation.yml, and the Keycloak realm import.Opt-in test suites (PR labels)
Some integration suites are expensive and are not run on every PR. Add the corresponding label to this PR to run them:
test-federationtest_knoxidf_federation.py). Stands up a real Keycloak as an external OpenID Provider and drives the full broker flow. Adds a few minutes (image pull + realm import).skip-testsWhen to add the label: these labels only take effect on runs triggered by opening the PR, pushing a commit, or reopening the PR — the workflow does not run on a label change. Add the
test-federationlabel before opening the PR (or before your next push) to exercise the federation suite.UI changes
The KnoxAuth login page (
knoxauth) gains an optional "Continue with <OP>" federated sign-in button (shown below an "Or" separator) when a topology fronts/authorizewith an SSO cookie provider and has one or more federated OPs enabled, plus a one-time consent page for the Authorization Code flow. See the Federation and Endpoints → consent page chapters for screenshots.