From d30f1e53ab17b9ee60f3eb26de45e332bf929e61 Mon Sep 17 00:00:00 2001 From: Kees van den Broek Date: Fri, 29 May 2026 12:40:21 +0200 Subject: [PATCH] fix(auth): use standard OIDC email claim for subscriber email MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- backend/src/appointment/routes/auth.py | 8 +++- backend/test/integration/test_auth.py | 63 ++++++++++++++++++++++++++ 2 files changed, 69 insertions(+), 2 deletions(-) diff --git a/backend/src/appointment/routes/auth.py b/backend/src/appointment/routes/auth.py index 35d21a836..fb47c8e2d 100644 --- a/backend/src/appointment/routes/auth.py +++ b/backend/src/appointment/routes/auth.py @@ -581,8 +581,12 @@ def oidc_token( oidc_id = token_data.get('sub') name = token_data.get('name') - # preferred_username is the thundermail address (@thundermail.com) - email = token_data.get('preferred_username', token_data.get('username')) + # Per OIDC Core 1.0 §5.1, the standard `email` claim is the authoritative email + # address, while `preferred_username` is only a display/login handle (it MUST NOT + # be relied upon to hold an email). Prefer `email`, falling back to + # `preferred_username`/`username` for Thunderbird Accounts, where the username is + # the (@thundermail.com) email address. + email = token_data.get('email') or token_data.get('preferred_username') or token_data.get('username') # the local part of the email should already be unique # so we can strip out the domain portion for a better Booking Link URL diff --git a/backend/test/integration/test_auth.py b/backend/test/integration/test_auth.py index 82b22c74a..b65ceb56a 100644 --- a/backend/test/integration/test_auth.py +++ b/backend/test/integration/test_auth.py @@ -1091,3 +1091,66 @@ def test_oidc_token_strips_domain_from_username(self, with_db, with_client, fake subscriber = repo.subscriber.get_by_email(db, email) assert subscriber is not None assert subscriber.username == expected_username + + def test_oidc_token_prefers_email_claim(self, with_db, with_client, faker): + """A new subscriber's email comes from the standard `email` claim, not `preferred_username`. + + Per OIDC Core 1.0 §5.1, `preferred_username` is a display/login handle and is not + guaranteed to be an email address, so the dedicated `email` claim must win. + """ + os.environ['AUTH_SCHEME'] = 'oidc' + + email = faker.email() + preferred_username = faker.user_name() # a display handle, not an email + oidc_id = 'new-oidc-id-prefers-email' + + with patch('appointment.controller.apis.oidc_client.OIDCClient.introspect_token') as mock_introspect: + mock_introspect.return_value = { + 'sub': oidc_id, + 'email': email, + 'preferred_username': preferred_username, + 'username': preferred_username, + 'name': 'OIDC User', + } + + response = with_client.post( + '/oidc/token', json={'access_token': 'valid_token', 'timezone': 'America/Vancouver'} + ) + + assert response.status_code == 200, response.text + assert response.json() is True + + with with_db() as db: + subscriber = repo.subscriber.get_by_email(db, email) + assert subscriber is not None + assert subscriber.email == email.lower() + + def test_oidc_token_falls_back_to_preferred_username(self, with_db, with_client, faker): + """When the `email` claim is absent, fall back to `preferred_username`. + + This preserves Thunderbird Accounts compatibility, where `preferred_username` + is the (@thundermail.com) email address. + """ + os.environ['AUTH_SCHEME'] = 'oidc' + + email = faker.email() # supplied via preferred_username, no `email` claim + oidc_id = 'new-oidc-id-fallback-preferred-username' + + with patch('appointment.controller.apis.oidc_client.OIDCClient.introspect_token') as mock_introspect: + mock_introspect.return_value = { + 'sub': oidc_id, + 'preferred_username': email, + 'name': 'OIDC User', + } + + response = with_client.post( + '/oidc/token', json={'access_token': 'valid_token', 'timezone': 'America/Vancouver'} + ) + + assert response.status_code == 200, response.text + assert response.json() is True + + with with_db() as db: + subscriber = repo.subscriber.get_by_email(db, email) + assert subscriber is not None + assert subscriber.email == email.lower()