feat: support federated OIDC login through the OAuth2 Authorization Server#24050
Draft
netroms wants to merge 6 commits into
Draft
feat: support federated OIDC login through the OAuth2 Authorization Server#24050netroms wants to merge 6 commits into
netroms wants to merge 6 commits into
Conversation
The OAuth2 Authorization Server stores the authenticated principal in the authorization attributes as JSON. After an external-OIDC login that principal is a DhisOidcUser wrapping a UserDetailsImpl, neither of which is on Spring Security's SecurityJackson2Modules deserialization allowlist, so any read of such an authorization (e.g. the /oauth2/token exchange) failed with HTTP 500: "... is not in the allowlist". Serialization succeeded, which is why the failure only showed up on read. Register Dhis2OAuth2PrincipalJackson2Module on the authorization service's ObjectMapper to allowlist and (de)serialize both types, and make UserDetailsImpl round-trip cleanly through its Lombok builder (@Jacksonized) instead of the previous hand-written, never-exercised @JsonCreator factory. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The new OIDC-principal regression test uses OAuth2AuthenticationToken from spring-security-oauth2-client, which dhis-test-integration only pulled in transitively. Declare it explicitly at test scope so the dependency:analyze check (run by the unit-test and sonarqube jobs) passes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…dule - Drop @JsonTypeInfo from the DhisOidcUser mix-in (S4544): the principal's static type is the OAuth2User interface, so the allowlist default typing already writes the type id; round-trip re-verified. - Make the UserDetailsImpl allowlist marker an interface (S1610, S2094). - Suppress S1610 on the DhisOidcUser mix-in, which must stay a class to carry a @JsonCreator constructor. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
netroms
changed the title
Supersedes the custom Jackson-mixin approach. The authorization persistence layer now swaps a heavyweight DHIS2 principal (DhisOidcUser from a federated OIDC login, or UserDetailsImpl from a form login) for a lean Spring-native UsernamePasswordAuthenticationToken carrying just the DHIS2 username and authorities, at the toEntity serialization boundary. The persisted attributes then contain no DHIS2-custom types and round-trip without any custom mixins, so the /oauth2/token read no longer trips Spring Security's SecurityJackson2Modules allowlist. Spring AS builds the token-context principal from the persisted principal attribute, so the token customizer sees the lean principal: getName() is the DHIS2 username, which becomes the JWT username claim the resource server resolves the user by. This makes the federated OIDC flow work end to end (the username/email claims previously carried the IdP sub). - Delete Dhis2OAuth2PrincipalJackson2Module and the speculative Jackson annotations on UserDetailsImpl (no longer serialized into the authorization). - Add a leanPrincipal unit test, update the service integration test to assert the lean principal, and add a federated-OIDC web e2e (authorization_code -> /oauth2/token -> /api/me). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
FederatedOidcTokenControllerTest uses PasswordEncoder (spring-security-crypto) to encode the client secret; dhis-test-web-api only pulled it in transitively. Declare it at test scope so the dependency:analyze check (run by the unit-test and sonarqube jobs) passes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…test SonarCloud S5977 — tests should not depend on randomly generated values. The test runs in isolation, so a fixed client id and authorization code are safe and deterministic. Co-Authored-By: Claude Opus 4.8 (1M context) <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
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.
Goal
Support federated OIDC login through DHIS2's OAuth2 Authorization Server: a client (e.g. the Android app) runs
authorization_code(+PKCE) against DHIS2, the user authenticates via an external IdP ("Log in with Google"), and DHIS2's Authorization Server issues its own tokens. The client only ever talks to DHIS2; the IdP is used solely to establish the user's identity — federated identity, not token pass-through.This path was broken: after an OIDC login the authorization's principal is a
DhisOidcUserwrapping aUserDetailsImpl, which Spring Security'sSecurityJackson2Modulesallowlist refuses to deserialize, so the/oauth2/tokenread returned HTTP 500 (reported by the Android team).Approach: store a lean principal
Rather than persist the heavyweight OIDC principal and register custom Jackson mixins to allowlist it, the persistence layer stores a lean, Spring-native principal. At the
toEntityserialization boundary, anAuthenticationwhose principal is a DHIS2UserDetails(DhisOidcUserfor OIDC,UserDetailsImplfor form login) is swapped for aUsernamePasswordAuthenticationTokencarrying just the DHIS2 username + authorities.Why this works (verified against the Spring AS 1.5.5 sources):
OAuth2AuthorizationCodeAuthenticationProviderandOAuth2RefreshTokenAuthenticationProviderbuild the token-context principal fromauthorization.getAttribute(Principal.class.getName())— the persisted attribute. So the token customizer sees the lean principal;getName()is the DHIS2 username, which becomes the JWTusernameclaim that the resource server (Dhis2JwtAuthenticationManagerResolver, internal providermapping_claim = username) resolves the user by.principalName(which the consent store keys on) is left untouched.This deletes more than it adds: the custom
Dhis2OAuth2PrincipalJackson2Moduleand the speculative Jackson annotations onUserDetailsImplare removed; the change is one ~12-line helper at the persistence boundary. It also fixes the federated flow end to end — theusername/emailclaims previously carried the IdPsub, which broke both token issuance (NPE on the email scope) and resolution.Tests
LeanPrincipalTest— unit test of the swap (no-op for client_credentials / non-UserDetailsprincipals; form-login and federated principals leaned).Dhis2OAuth2AuthorizationServiceIntegrationTest(Postgres) — a federated authorization persists and reads back as a lean principal with the DHIS2 username + authorities.FederatedOidcTokenControllerTest(web e2e) — aDhisOidcUser-originatedauthorization_codeexchange at/oauth2/tokenissues a JWT whoseusername/emailclaims are the DHIS2 user's (not the IdPsub), and/api/mewith that token resolves to the user.AI Assisted