oauth2: OIDC login nonce and per-attempt CSRF state - #14296
Draft
nagaboinaramgopal wants to merge 6 commits into
Draft
nagaboinaramgopal wants to merge 6 commits into
nagaboinaramgopal wants to merge 6 commits into
Conversation
The OAuth2 plugin resolves a login to a UserOAuth2Authenticator through a fixed provider-name to Spring-bean map, and each OIDC vendor is its own bean running the same authorization-code flow with no vendor-specific logic. A new IdP needs a new class, and since provider is both the display name and the routing key, a domain can register only one keycloak. This adds a type column to oauth_provider. OAuth2AuthManagerImpl looks the name up in the bean map first and only on a miss falls back to the registration's type, so provider becomes an admin-chosen label and type selects the implementation. One bean then serves any number of registrations under arbitrary names. Existing google, github and keycloak rows carry a null type and dispatch by name as before. GenericOIDCOAuth2Provider is registered under type oidc and configured with the issuer URL. It reads the token and JWKS endpoints from the issuer's discovery document (the issuer must match), and validates the id_token before trusting it: signature against the JWKS key named by the token kid, then issuer, audience and expiry, using the CXF JOSE library already on the classpath. It holds no token state between logins, and a code verified through verifyOAuthCodeAndGetUser is redeemed once so the following oauthlogin does not re-present it. ListOAuthProvidersCmd and UpdateOAuthProviderCmd derived the response enabled flag from a name-to-bean check, which reported every generically-named registration as disabled; both now also accept a registration whose type resolves to a plugin. The schema change adds type and issuer_url to oauth_provider in the 4.23.0.0 to 24.0.0 upgrade file. Login.vue renders the OAuth buttons from the registered provider list so a generic oidc provider gets a Sign in with <label> button and starts the flow from the issuer's authorize endpoint, resolved from discovery.
- fall back to the id_token header algorithm when a published JWK omits alg (Entra ID does this), so verification no longer fails for those providers - cache the JWKS per issuer like the discovery document, refetching once on an unknown key id for rotation, instead of fetching on every login - reject registering an OIDC type under a name reserved by a built-in provider, where the name would shadow the type during dispatch - log the provider's raw token-exchange error server side and return a generic message to the caller
- require a verified email (email_verified) in the id_token before using it as the account identity, so an unverified address cannot claim a user - key the single-use verified-email cache by domain as well, so a code verified for one domain cannot be consumed for another - store the provider type lowercased so list, update and the UI agree with the case-insensitive dispatch lookup - correct the since metadata on the new type/issuerUrl parameters and response fields from 4.24.0 to 24.0.0
Binds the generic OIDC login to the authorization request: - the UI generates a random nonce and state per attempt, sends the nonce in the authorization request, and rejects the callback when the returned state does not match before the code is exchanged - the id_token nonce claim is verified against the nonce from the request (generic OIDC only; the built-in providers ignore it) Follow-up to apache#14205.
…n page - discover and store the authorization endpoint at registration so the UI redirects without reading the discovery document from the browser, which removed the IdP CORS requirement; the UI falls back to discovery when the endpoint is not stored - build the authorize URL with the URL API so an endpoint that already has a query component is preserved - restore the global generic provider list when the domain field is cleared so stale domain-specific buttons are not shown
Align the existing provider and command tests with the hardened behavior: verifyUser resolves through the nonce-aware path, the verified-email tests stub the four-argument resolveEmail, the token helper sets email_verified, and the command test mocks the nonce overload. All 110 oauth2 tests pass.
This branch has not been deployed
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.
Description
Follow-up to #14205. Finishes and hardens the generic OIDC login, from the review on #14205.
cloudstackbefore).nonceAPI parameter. Only the generic OIDC provider checks it and the state check only applies to the OIDC flow, so the built-in google, github and keycloak logins are unchanged.Depends on #14205.
Types of changes
How Has This Been Tested?
All 110 tests in the oauth2 plugin pass, built and run on a Linux build host with Java 17. The provider tests use real RS256 signing and verification against a JWKS: a matching nonce is accepted and a mismatched one rejected, an unverified email is rejected, a key without alg falls back to the token header, the keys are cached across logins, and the usual signature, issuer, audience and expiry checks hold.
The frontend state and nonce round trip has been reviewed by hand but not run in a browser, since that needs a live identity provider.