Repository navigation
Custom OIDC identity providers parse email_verified from validated tokens when not pre-populated in context - #54268
Conversation
There was a problem hiding this comment.
🟢 Approval recommended
Static review found no blocking issues in the focused claim-handling changes or regression tests; tests were not executed.
No open issues 🎉
What changed in this PR
Fixes #54245 by preserving the ID token’s email_verified claim when userinfo or custom OIDC provider context omits it, while keeping explicit context values authoritative.
Changes:
- Falls back to the validated ID token when the context has no email-verification value.
- Preserves the token’s value when userinfo supplies an email without
email_verified. - Adds unit tests for fallback and precedence, plus an integration regression test.
| File | Description |
|---|---|
| tests/base/src/test/java/org/keycloak/tests/broker/oidc/KcOidcBrokerTest.java | Tests preservation of the token’s false value when userinfo omits verification. |
| services/src/test/java/org/keycloak/test/broker/oidc/OIDCIdentityProviderTest.java | Tests token fallback and explicit context precedence. |
| services/src/main/java/org/keycloak/broker/oidc/OIDCIdentityProvider.java | Implements token fallback and preserves verification during userinfo extraction. |
Review effort: Balanced
There was a problem hiding this comment.
Just a nitpick for the test. The rest LGTM.
There was a problem hiding this comment.
Thanks @msdaly200! Just one comment, as the equals returns true when null I think we should check the token email is not null. In the other path we are checking the userInfoEmail is not null previously.
|
This one is ready but we need to wait to the next patch and move the notes to the proper version when everything is released. Thanks @msdaly200! |
There was a problem hiding this comment.
🟡 Changes recommended
The custom-extractor fallback still lets Trust Email verify a new user's email when the token has email_verified=false but no email, and the unit test currently expects this incorrect behavior.
1 open issue
Review effort: Balanced
There was a problem hiding this comment.
🔵 Needs a closer look
The guard still ignores email_verified=false when an overridden extractor supplies the email but the ID token omits it, allowing Trust Email to incorrectly verify the profile email; the new test does not cover this path.
Review effort: Balanced
…eycloak#54245 Signed-off-by: Marie Daly <marie.daly1@ibm.com>
There was a problem hiding this comment.
@msdaly200 I don't see any issue with this approach. LGTM, thanks!
There was a problem hiding this comment.
Thanks @msdaly200 and @mabartos!
Closes #54245
When a provider like GitLab overrides extractIdentity() and stores VALIDATED_ID_TOKEN but not the EMAIL_VERIFIED context entry, an explicit email_verified=false from the ID token is silently ignored. With "Trust Email" enabled, setEmailVerified() finds a null context entry, falls through to the superclass default, and marks the email as verified, the opposite of what the token declared.
Fix:
Tests: