fix(auth): use standard OIDC email claim for subscriber email#1689
Open
kvdb wants to merge 1 commit into
Open
Conversation
The generic `oidc` auth scheme derived the subscriber email from
`preferred_username`, assuming `preferred_username == email`. That only
holds for Thunderbird Accounts / Firefox Accounts, where the username is
the email. For any other OIDC IdP (Keycloak, Kanidm, ...),
`preferred_username` is a display/login handle, not an email, so the
subscriber was created with a bogus email (the short username), breaking
host notification emails.
Per OpenID Connect Core 1.0 §5.1 (Standard Claims), `preferred_username`
is the "Shorthand name by which the End-User wishes to be referred to ...
It MUST NOT be relied upon to be unique by the RP" - i.e. a display
handle, explicitly not an email. The dedicated `email` claim (provided by
the `email` scope, §5.4) is the authoritative email source.
Prefer the `email` claim, falling back to `preferred_username`/`username`
to preserve Thunderbird Accounts compatibility:
email = token_data.get('email') or token_data.get('preferred_username') or token_data.get('username')
Add integration tests covering both paths: when the introspected token
carries an `email` claim the subscriber email comes from it (not from
`preferred_username`); when `email` is absent it falls back to
`preferred_username`.
Ref: https://openid.net/specs/openid-connect-core-1_0.html#StandardClaims
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.
What changed?
The OIDC token endpoint (
backend/src/appointment/routes/auth.py) now derives anew subscriber's email from the standard OIDC
emailclaim, falling back topreferred_username/usernameonly when it's absent.Adds two integration tests in
backend/test/integration/test_auth.pyfor bothpaths (email claim present; falls back to
preferred_username).Why?
The scheme derived the email from
preferred_username, assumingpreferred_username == email. That only holds for Thunderbird/Firefox Accounts.For other IdPs (Keycloak, Kanidm, ...)
preferred_usernameis a display handle,not an email, so subscribers got a bogus email, breaking host notification
emails. Per OIDC Core 1.0 §5.1
preferred_username"MUST NOT be relied upon" asan identifier; the
emailclaim (§5.4) is the authoritative source.Ref: https://openid.net/specs/openid-connect-core-1_0.html#StandardClaims
Limitations and Notes
Trusts the introspected
emailclaim without checkingemail_verified(matchingprior behavior). Verified locally: full backend suite passes (403 passed, 1
skipped),
ruff checkclean.