feat(oidc): persist and expose email_verified from OAuth providers#166
feat(oidc): persist and expose email_verified from OAuth providers#166
Conversation
- Add User.EmailVerified column so ID Token and UserInfo can report per-user verification status - Set EmailVerified on OAuth auto-registration using the provider's flag - Promote EmailVerified when linking a trusted provider to an existing account - Replace the hardcoded false in ID Token and UserInfo email_verified claim - Cover the new behaviour with handler, token-exchange and OAuth service tests
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Pull request overview
This PR adds a persisted User.EmailVerified flag and wires OIDC email_verified (in both ID Tokens and the UserInfo endpoint) to mirror the stored user state, with OAuth flows populating/promoting the flag based on upstream provider verification.
Changes:
- Add
EmailVerifiedtomodels.User(GORM column defaultfalse) and set it on OAuth user creation/linking. - Emit
email_verifiedfromUser.EmailVerifiedin ID Tokens (token_exchange.go) and UserInfo responses (oidc.go). - Add/extend tests covering new behavior for OAuth registration/linking and OIDC claim generation.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| internal/models/user.go | Adds EmailVerified field/column to User. |
| internal/services/user.go | Persists EmailVerified on OAuth user creation and promotes it on linking. |
| internal/services/token_exchange.go | Sets ID Token email_verified from User.EmailVerified. |
| internal/handlers/oidc.go | Sets UserInfo email_verified from User.EmailVerified. |
| internal/services/user_oauth_test.go | Adds tests for OAuth registration/link promotion behavior. |
| internal/services/token_test.go | Adds ID Token test asserting email_verified mirrors user state. |
| internal/handlers/oidc_test.go | Updates/adds tests for UserInfo email_verified. |
| docs/ARCHITECTURE.md | Updates docs to reflect non-constant email_verified behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Clear User.EmailVerified in UpdateUserProfile when an admin edits the email - Clear User.EmailVerified in UpsertExternalUser when an external sync changes the stored email - Cover both downgrade paths and unchanged-email preservation with regression tests
There was a problem hiding this comment.
Pull request overview
Adds persistent User.EmailVerified state and uses it to populate the OIDC email_verified claim consistently across ID Tokens and the UserInfo endpoint, primarily for OAuth-authenticated users.
Changes:
- Add
EmailVerifiedto themodels.Userschema and propagate it during OAuth user creation/linking. - Drive
email_verifiedin ID Token generation and/oauth/userinfofromUser.EmailVerified. - Downgrade
EmailVerifiedtofalsewhen an email address changes via external sync or admin edits; add/adjust tests and docs.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/models/user.go | Adds EmailVerified field with DB default. |
| internal/services/user.go | Propagates/promotes/downgrades EmailVerified across OAuth create/link and admin email edits. |
| internal/services/token_exchange.go | Sets ID Token email_verified from persisted user state. |
| internal/handlers/oidc.go | Sets UserInfo email_verified from persisted user state. |
| internal/store/user.go | Clears EmailVerified when external sync changes a user’s email. |
| internal/store/store_test.go | Adds store-level tests for EmailVerified downgrade/preserve behavior on external sync. |
| internal/services/user_test.go | Adds service-level tests for admin email-change downgrade/preserve behavior. |
| internal/services/user_oauth_test.go | Extends OAuth service tests to assert propagation/promotion behavior. |
| internal/services/token_test.go | Adds test to assert ID Token email_verified mirrors stored user value. |
| internal/handlers/oidc_test.go | Updates/adds tests to assert UserInfo email_verified mirrors stored user value. |
| docs/ARCHITECTURE.md | Updates documentation for email_verified semantics. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Promote User.EmailVerified during updateOAuthConnectionAndGetUser when the provider reports a verified email that still matches the stored address, so pre-migration users and earlier update failures recover on next login - Skip the promotion on email mismatch to keep a drifted provider account from asserting verification of an address it does not own - Cover the steady-state migration case and the mismatch guard with regression tests - Pick up automatic table-width reformatting in docs/ARCHITECTURE.md
There was a problem hiding this comment.
Pull request overview
Adds persistent User.EmailVerified state and uses it to emit accurate email_verified claims in OIDC ID Tokens and UserInfo responses, reflecting verification guarantees from upstream OAuth providers.
Changes:
- Add
EmailVerifiedcolumn tomodels.User(defaultfalse) and propagate it during OAuth user creation/linking/self-healing. - Drive
email_verifiedin ID Token issuance and/oauth/userinfofromUser.EmailVerifiedinstead of hardcodingfalse. - Add/extend tests to cover persistence, promotion, and downgrade behavior when emails change.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| internal/models/user.go | Adds EmailVerified field with GORM default/tagging. |
| internal/services/token_exchange.go | Sets ID token email_verified from user.EmailVerified. |
| internal/handlers/oidc.go | Sets UserInfo email_verified from user.EmailVerified. |
| internal/services/user.go | Persists EmailVerified on OAuth create; promotes on verified provider match; clears on admin email changes. |
| internal/store/user.go | Clears EmailVerified when external sync changes the stored email. |
| internal/services/token_test.go | Adds ID token test asserting claim mirrors User.EmailVerified. |
| internal/handlers/oidc_test.go | Updates/adds tests to assert UserInfo email_verified mirrors user field. |
| internal/services/user_oauth_test.go | Adds tests for new-user propagation and promotion behavior across linking/logins. |
| internal/services/user_test.go | Adds tests ensuring admin email edits clear/preserve EmailVerified appropriately. |
| internal/store/store_test.go | Adds tests ensuring external upsert email changes clear/preserve EmailVerified. |
| docs/ARCHITECTURE.md | Updates documentation for email_verified semantics under the email scope. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Trim req.Email and req.FullName in UpdateUserProfile so accidental whitespace does not downgrade EmailVerified or masquerade as a real email change - Match the existing normalization that CreateUserAdmin already performs - Cover the whitespace-only edit case with a regression test
There was a problem hiding this comment.
Pull request overview
Adds persistent per-user email verification state and uses it to drive the OIDC email_verified claim so relying parties can trust the value when upstream OAuth providers vouch for the address.
Changes:
- Add
User.EmailVerified(GORM column, defaultfalse) and propagate/promote it from OAuth logins/linking. - Emit
email_verifiedin ID Tokens and UserInfo based onUser.EmailVerified(instead of alwaysfalse). - Add/adjust tests and update architecture docs to reflect the new claim behavior.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| internal/models/user.go | Adds persisted EmailVerified field to the user model. |
| internal/store/user.go | Clears EmailVerified when external sync changes the stored email. |
| internal/store/store_test.go | Adds tests for clearing/preserving EmailVerified during external upsert. |
| internal/services/user.go | Promotes EmailVerified on trusted OAuth link/login; clears it on admin email change; trims admin inputs. |
| internal/services/user_test.go | Adds tests for admin update clearing/preserving EmailVerified (incl. whitespace normalization). |
| internal/services/user_oauth_test.go | Adds OAuth service tests for propagation/promotion and email mismatch safety. |
| internal/services/token_exchange.go | Sets ID token email_verified from user.EmailVerified. |
| internal/services/token_test.go | Adds coverage that ID token email_verified mirrors persisted user state. |
| internal/handlers/oidc.go | Sets UserInfo email_verified from user.EmailVerified. |
| internal/handlers/oidc_test.go | Updates/adds tests asserting UserInfo email_verified mirrors user state. |
| docs/ARCHITECTURE.md | Updates documentation of email_verified claim semantics. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- TrimSpace username, email, and full name at the start of UpsertExternalUser so incidental whitespace from upstream providers does not downgrade EmailVerified or pollute the stored email - Matches the normalization already performed by CreateUserAdmin and UpdateUserProfile - Cover the whitespace-only email case with a regression test
There was a problem hiding this comment.
Pull request overview
Adds persistence and correct OIDC exposure of email_verified by introducing User.EmailVerified and wiring it through OAuth registration/linking plus ID Token/UserInfo claim building.
Changes:
- Add
EmailVerifiedto theUsermodel (defaultfalse) and use it foremail_verifiedin ID Tokens and UserInfo. - Propagate
OAuthUserInfo.EmailVerifiedinto newly created OAuth users and promoteEmailVerifiedwhen linking/authing via verifying providers. - Add store/service logic + tests to downgrade
EmailVerifiedon real email changes and avoid whitespace-only downgrades.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/models/user.go | Adds persisted EmailVerified field with default false. |
| internal/store/user.go | Normalizes external-user inputs; downgrades EmailVerified when external sync changes email. |
| internal/store/store_test.go | Adds tests covering downgrade/preserve behavior for external upsert email changes/whitespace. |
| internal/services/user.go | Promotes EmailVerified from OAuth info; normalizes admin profile updates; downgrades on admin email change. |
| internal/services/user_test.go | Adds tests ensuring admin email edits downgrade/preserve EmailVerified correctly. |
| internal/services/user_oauth_test.go | Adds tests for OAuth creation/link/self-heal promotion behavior and email mismatch guard. |
| internal/services/token_exchange.go | Sources email_verified claim from User.EmailVerified during token exchange. |
| internal/services/token_test.go | Adds ID token test asserting email_verified mirrors User.EmailVerified. |
| internal/handlers/oidc.go | Sources UserInfo email_verified from User.EmailVerified. |
| internal/handlers/oidc_test.go | Updates/extends UserInfo tests for email_verified mirroring. |
| docs/ARCHITECTURE.md | Updates docs to reflect email_verified semantics under the email scope. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Trim oauthUserInfo fields once at the top of AuthenticateWithOAuth so downstream lookups, promotion checks, and persistence all use normalized values - Re-trim inside createUserWithOAuth as defense in depth for direct callers and store the normalized email/username/full_name plus duplicate-error text - Compare trimmed values in updateOAuthConnectionAndGetUser, UpdateUserProfile, and UpsertExternalUser so pre-existing rows with incidental whitespace self-heal instead of triggering spurious EmailVerified downgrades - Keep UpsertExternalUser backward compatible for HTTP API callbacks that omit email/full name by preserving the stored values instead of blanking them; still reject empty username always and empty email on create - Add ErrExternalUserMissingIdentity and cover the new behavior with service/store regression tests
There was a problem hiding this comment.
Pull request overview
Adds persisted email verification state to users and uses it to populate the OIDC email_verified claim, based on whether a trusted upstream OAuth provider verified the address.
Changes:
- Add
User.EmailVerifiedDB column (defaultfalse) and use it to driveemail_verifiedin ID Tokens and/oauth/userinfo. - Propagate/persist
OAuthUserInfo.EmailVerifiedon OAuth auto-registration, and promote the flag when linking/logging in with a verifying provider. - Normalize/trims identity fields (username/email/full name) in several paths and add extensive regression tests + docs updates.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/models/user.go | Adds EmailVerified field with DB defaults. |
| internal/services/token_exchange.go | Sets ID token email_verified from user.EmailVerified. |
| internal/handlers/oidc.go | Sets UserInfo email_verified from user.EmailVerified. |
| internal/services/user.go | Normalizes OAuth inputs; promotes/downgrades EmailVerified on link/update/admin email edits. |
| internal/store/user.go | Normalizes external user inputs; preserves fields when upstream omits them; downgrades on email change. |
| internal/store/errors.go | Introduces ErrExternalUserMissingIdentity for external upsert validation. |
| internal/services/token_test.go | Adds test asserting ID token email_verified mirrors user field. |
| internal/handlers/oidc_test.go | Updates/adds tests for UserInfo email_verified. |
| internal/services/user_oauth_test.go | Adds tests for OAuth create/link/promotion behavior. |
| internal/services/user_test.go | Adds tests for admin email edits downgrading/preserving verification with whitespace normalization. |
| internal/store/store_test.go | Adds tests for external-user upsert email-change/whitespace/empty-field behavior. |
| docs/ARCHITECTURE.md | Updates docs to reflect email_verified semantics. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Trim externalID and authSource in UpsertExternalUser and reject blank values so the (external_id, auth_source) lookup key cannot collapse unrelated accounts onto the same row - Trim the lookup argument in GetUserByEmail and add a TRIM(email) fallback so legacy rows with incidental whitespace still match — preventing OAuth logins from creating duplicate accounts for pre-existing users - Cover both behaviors with regression tests for empty externalID/authSource and the legacy-whitespace fallback match
There was a problem hiding this comment.
Pull request overview
Adds persisted email verification state to users and uses it to populate the OIDC email_verified claim consistently across ID Tokens and UserInfo responses, based on trusted upstream OAuth providers.
Changes:
- Add
User.EmailVerified(defaultfalse) and wire it into OIDC ID token + UserInfoemail_verifiedclaim generation. - Propagate and promote
EmailVerifiedduring OAuth registration/linking; downgrade it on email changes; normalize upstream/admin inputs to avoid whitespace-related drift. - Expand test coverage around verification promotion/demotion and whitespace/legacy data handling.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/models/user.go | Adds persisted EmailVerified field with default false. |
| internal/services/token_exchange.go | Sets ID token email_verified from user.EmailVerified. |
| internal/handlers/oidc.go | Sets UserInfo email_verified from user.EmailVerified. |
| internal/services/user.go | Normalizes OAuth/admin inputs; promotes/downgrades EmailVerified appropriately. |
| internal/store/user.go | Normalizes external-user upserts; adds trimmed-email fallback lookup. |
| internal/store/errors.go | Introduces ErrExternalUserMissingIdentity. |
| internal/store/store_test.go | Adds tests for upsert behavior, whitespace normalization, and email lookup fallback. |
| internal/services/user_test.go | Adds tests ensuring admin email edits downgrade/preserve EmailVerified correctly. |
| internal/services/user_oauth_test.go | Adds tests for OAuth verification propagation and promotion logic. |
| internal/services/token_test.go | Adds test asserting ID token email_verified mirrors stored user field. |
| internal/handlers/oidc_test.go | Updates/adds tests for UserInfo email_verified mirroring user field. |
| docs/ARCHITECTURE.md | Updates documentation to reflect new email_verified behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Refresh ErrExternalUserMissingIdentity's comment and message to cover all required identity fields (username, external_id, auth_source, email on create) - Add ErrAmbiguousEmail; GetUserByEmail now fetches up to two fallback matches and returns the new error when legacy whitespace-variant duplicates collide, so the OAuth auto-link path never binds to a non-deterministic row - Propagate ErrAmbiguousEmail out of AuthenticateWithOAuth so ambiguous local duplicates block auto-link and auto-register until operators deduplicate - Cover both store-level ambiguity detection and the service-level OAuth refusal with regression tests
There was a problem hiding this comment.
Pull request overview
Adds persistence and correct OIDC exposure of the email_verified claim by storing verification state on models.User and propagating/promoting it through OAuth flows and profile updates.
Changes:
- Add
User.EmailVerified(DB column via GORM) and use it to populateemail_verifiedin ID Tokens and/oauth/userinfo. - Normalize/trims upstream and admin-provided identity fields; adjust external-user upsert behavior to avoid blanking stored emails and to reset verification on real email changes.
- Add new error types and extensive tests covering promotion/downgrade behavior, whitespace legacy rows, and ambiguous normalized-email cases.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/models/user.go | Adds EmailVerified field with default false. |
| internal/services/token_exchange.go | Sets ID token email_verified from user.EmailVerified. |
| internal/handlers/oidc.go | Sets UserInfo email_verified from user.EmailVerified. |
| internal/store/user.go | Trims external inputs; preserves stored email when upstream omits it; adds whitespace-tolerant GetUserByEmail fallback and ambiguity error. |
| internal/store/errors.go | Introduces ErrExternalUserMissingIdentity and ErrAmbiguousEmail. |
| internal/services/user.go | Normalizes OAuth/admin inputs; promotes EmailVerified on trusted OAuth link/reauth; surfaces ambiguous-email error to callers. |
| internal/store/store_test.go | Adds tests for email change downgrade, whitespace normalization, and ambiguous email handling in the store. |
| internal/services/user_test.go | Adds admin profile update tests ensuring downgrade/self-heal semantics. |
| internal/services/user_oauth_test.go | Adds OAuth tests for persistence/promotion and ambiguous-email refusal. |
| internal/services/token_test.go | Adds ID token claim test ensuring email_verified mirrors user field. |
| internal/handlers/oidc_test.go | Updates/adds UserInfo claim tests for email_verified. |
| docs/ARCHITECTURE.md | Updates docs to reflect new email_verified semantics. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…dits - Run GetUserByEmail's lookup against TRIM(email) on every call so a legacy whitespace duplicate is detected even when a properly normalized exact match exists, preventing the OAuth auto-link path from silently binding to an unambiguous-looking row while duplicates lurk - Compare raw user.Email to req.Email in UpdateUserProfile so a pure normalization edit still runs the uniqueness check; map ErrAmbiguousEmail to ErrEmailConflict for admin callers so the DB never returns a raw unique-constraint error - Cover both cases with regression tests
There was a problem hiding this comment.
Pull request overview
Persists upstream email verification state on users and exposes it via OIDC (email_verified claim) so relying parties can trust the claim when the email was verified by a trusted OAuth provider.
Changes:
- Add
User.EmailVerified(defaultfalse) and propagate/promote it during OAuth registration/linking and admin email edits. - Drive
email_verifiedin both ID Tokens and/oauth/userinfofromUser.EmailVerified. - Harden email/external user handling with whitespace normalization and ambiguity detection (
ErrAmbiguousEmail) plus comprehensive new tests.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/models/user.go | Adds EmailVerified field persisted in DB. |
| internal/store/user.go | Normalizes external-user inputs; adds email self-heal/downgrade rules; makes GetUserByEmail whitespace-tolerant with ambiguity detection. |
| internal/store/errors.go | Adds ErrExternalUserMissingIdentity and ErrAmbiguousEmail. |
| internal/store/store_test.go | Adds store-level tests for email verification downgrade/preservation and ambiguous email lookup behavior. |
| internal/services/user.go | Normalizes OAuth fields, promotes EmailVerified on verified providers, trims admin inputs, handles ambiguous email in OAuth path. |
| internal/services/user_test.go | Adds admin update tests to ensure email changes downgrade verification and whitespace normalization behaves correctly. |
| internal/services/user_oauth_test.go | Adds OAuth tests for verified/unverified new users, promotion on linking/reauth, and ambiguous-email rejection. |
| internal/services/token_exchange.go | Sets ID Token email_verified claim from User.EmailVerified. |
| internal/services/token_test.go | Adds coverage ensuring ID Token claim mirrors stored user field. |
| internal/handlers/oidc.go | Sets UserInfo email_verified from User.EmailVerified. |
| internal/handlers/oidc_test.go | Updates/adds tests ensuring UserInfo claim mirrors stored user field. |
| docs/ARCHITECTURE.md | Updates documentation of /oauth/userinfo email_verified semantics and table formatting. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Look up exact email via the UNIQUE index first, falling back to a bounded TRIM(email) scan only when no exact match is found or when confirming no legacy whitespace duplicate exists - Cap the ambiguity confirmation to a single additional row so clean deployments pay an O(log n) index lookup plus an early-stopping check instead of a full table scan per call - Document the AutoMigrate production caveat in docs/DEVELOPMENT.md with an example out-of-band ALTER TABLE for the new email_verified column
There was a problem hiding this comment.
Pull request overview
Adds persisted email_verified state to users and wires OIDC claims to reflect upstream OAuth provider verification instead of always returning false, while also hardening email lookups/normalization to avoid unsafe auto-linking when legacy whitespace duplicates exist.
Changes:
- Add
User.EmailVerified(defaultfalse) and propagate/promote it during OAuth registration/linking and certain updates. - Drive OIDC
email_verifiedclaim in both ID Token and UserInfo fromUser.EmailVerified. - Normalize/trust email lookups (trim + ambiguity detection) and add extensive tests + documentation notes.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/models/user.go | Adds EmailVerified field with GORM default. |
| internal/store/errors.go | Introduces ErrExternalUserMissingIdentity and ErrAmbiguousEmail. |
| internal/store/user.go | Normalizes external-user upserts; makes GetUserByEmail whitespace-tolerant and ambiguity-aware. |
| internal/store/store_test.go | Adds test coverage for external-user upsert behavior and ambiguous email handling. |
| internal/services/user.go | Normalizes OAuth/admin inputs; promotes/downgrades EmailVerified; surfaces ambiguous-email errors for OAuth. |
| internal/services/user_test.go | Tests admin profile update behavior around normalization + EmailVerified downgrades/preservation. |
| internal/services/user_oauth_test.go | Tests OAuth new-user, linking promotion, and ambiguous email rejection scenarios. |
| internal/services/token_exchange.go | Sets ID token email_verified from User.EmailVerified. |
| internal/services/token_test.go | Validates ID token email_verified mirrors stored user field. |
| internal/handlers/oidc.go | Sets UserInfo email_verified from User.EmailVerified. |
| internal/handlers/oidc_test.go | Validates UserInfo email_verified mirrors stored user field. |
| docs/DEVELOPMENT.md | Documents AutoMigrate production caveat re: adding NOT NULL + default columns. |
| docs/ARCHITECTURE.md | Updates docs to reflect conditional email_verified semantics. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Treat ErrAmbiguousEmail as ErrEmailConflict in the admin create path's uniqueness check and race-condition re-query so legacy whitespace duplicates surface a clear conflict instead of a generic failure or a misleading username-conflict - Document the ASCII-vs-Unicode whitespace divergence between Go's strings.TrimSpace and SQL TRIM() in GetUserByEmail as a known limitation, since OAuth/admin writes already normalize on insert so only pre-existing exotic-whitespace rows are affected
There was a problem hiding this comment.
Pull request overview
Adds persistent email_verified support to AuthGate’s user model and OIDC outputs, so ID Tokens and UserInfo reflect whether a trusted upstream OAuth provider verified the user’s email.
Changes:
- Introduces
User.EmailVerified(defaultfalse) and uses it to populateemail_verifiedin ID Token generation and/oauth/userinfo. - Propagates and promotes verification state during OAuth registration/linking, with normalization + safeguards for legacy whitespace email duplicates.
- Expands test coverage across store, user service, token exchange, and OIDC handlers; updates docs to reflect the new behavior and migration considerations.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/models/user.go | Adds EmailVerified field to persist verification state. |
| internal/store/errors.go | Adds store-level errors for missing external identity and ambiguous normalized email. |
| internal/store/user.go | Normalizes external upsert inputs; enhances GetUserByEmail to be whitespace-tolerant and detect ambiguity. |
| internal/store/store_test.go | Adds store tests for email normalization, ambiguity detection, and verification downgrade rules. |
| internal/services/user.go | Normalizes OAuth/admin inputs; promotes/downgrades EmailVerified appropriately; surfaces ambiguous-email sign-in error. |
| internal/services/user_test.go | Adds tests ensuring admin email edits normalize and correctly downgrade/preserve EmailVerified. |
| internal/services/user_oauth_test.go | Adds OAuth tests for persisting EmailVerified on create/link/re-auth and rejecting ambiguous legacy duplicates. |
| internal/services/token_exchange.go | Sets ID token email_verified from User.EmailVerified when email scope is present. |
| internal/services/token_test.go | Adds ID token claim test asserting email_verified mirrors user state. |
| internal/handlers/oidc.go | Sets UserInfo email_verified from User.EmailVerified when email scope is present. |
| internal/handlers/oidc_test.go | Updates/adds tests asserting UserInfo email_verified mirrors user state. |
| docs/DEVELOPMENT.md | Documents AutoMigrate operational caveats and example migration for users.email_verified. |
| docs/ARCHITECTURE.md | Updates endpoint documentation to describe non-always-false email_verified semantics. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Restore GetUserByEmail to a purely indexed `email = ?` lookup so hot-path callers (admin uniqueness checks, OAuth session resolution) no longer pay for a per-request TRIM scan - Add FindUserByNormalizedEmail for the OAuth auto-link flow, which still performs the whitespace-tolerant scan with ambiguity detection to prevent binding a verified provider to the wrong legacy row - Point AuthenticateWithOAuth at the new normalized lookup; drop the now-unreachable ErrAmbiguousEmail branches from the admin create/update paths - Reword the AutoMigrate caveat in docs/DEVELOPMENT.md to clarify that ACCESS EXCLUSIVE is Postgres-specific and describe other engines as potentially taking a comparable table lock
There was a problem hiding this comment.
Pull request overview
This PR adds a persisted users.email_verified state and wires OIDC email_verified claims (ID token + UserInfo) to reflect that persisted value, with OAuth flows promoting/downgrading the flag based on trusted provider signals and email changes.
Changes:
- Add
User.EmailVerified(defaultfalse) and use it to driveemail_verifiedin ID tokens and/oauth/userinfo. - Normalize/trim upstream OAuth + external-auth inputs; add a whitespace-tolerant email lookup (
FindUserByNormalizedEmail) for safe OAuth auto-linking with ambiguity detection. - Add/adjust tests across store, services, token exchange, and OIDC claims to cover promotion/downgrade and legacy whitespace behavior.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/models/user.go | Adds persisted EmailVerified field to User model with default false. |
| internal/services/token_exchange.go | Sets ID token email_verified from user.EmailVerified when email scope is granted. |
| internal/handlers/oidc.go | Sets UserInfo email_verified from user.EmailVerified when email scope is granted. |
| internal/store/user.go | Trims inputs in UpsertExternalUser, adds FindUserByNormalizedEmail, and refines email update semantics to preserve verification unless email truly changes. |
| internal/store/errors.go | Adds new store errors for missing external identity and ambiguous normalized email matches. |
| internal/core/store.go | Extends UserReader with FindUserByNormalizedEmail. |
| internal/mocks/mock_store.go | Regenerates mocks to include FindUserByNormalizedEmail. |
| internal/services/user.go | Normalizes OAuth userinfo fields, uses normalized email lookup for auto-link, promotes EmailVerified on trusted linking/re-auth, and downgrades it on admin email change. |
| internal/store/store_test.go | Adds store-level tests for email trimming, verification downgrade rules, and normalized-email ambiguity detection. |
| internal/services/user_test.go | Adds tests ensuring admin email changes downgrade EmailVerified and normalization edits behave deterministically. |
| internal/services/user_oauth_test.go | Adds tests for OAuth registration/linking behavior around EmailVerified promotion and ambiguity rejection. |
| internal/services/token_test.go | Adds tests ensuring ID token email_verified mirrors the stored user field. |
| internal/handlers/oidc_test.go | Updates/adds tests ensuring UserInfo email_verified mirrors the stored user field. |
| docs/DEVELOPMENT.md | Documents operational caveat for AutoMigrate adding NOT NULL DEFAULT columns, with an example for email_verified. |
| docs/ARCHITECTURE.md | Updates documentation to reflect that email_verified is no longer always false. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…split - Reword the ErrAmbiguousEmail godoc to reference FindUserByNormalizedEmail (the only caller that can now return it) instead of the old GetUserByEmail fallback - Rename the three TestGetUserByEmail_* tests that actually exercise FindUserByNormalizedEmail so CI output points at the correct API
There was a problem hiding this comment.
Pull request overview
This PR adds persistent email verification state to users and uses it to populate the OIDC email_verified claim accurately for both ID Tokens and the UserInfo endpoint, based on trusted upstream OAuth providers.
Changes:
- Add
User.EmailVerified(defaultfalse) and propagate/promote it during OAuth registration/linking flows. - Drive
email_verifiedin ID Token minting and/oauth/userinforesponses from the persisted user field. - Add whitespace-tolerant email lookup (
FindUserByNormalizedEmail) to make OAuth auto-link safe in the presence of legacy whitespace variants, plus extensive tests.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| internal/models/user.go | Adds persisted EmailVerified column with default false. |
| internal/store/user.go | Normalizes external-user inputs, avoids blanking email/full name on update, adds normalized-email lookup with ambiguity detection. |
| internal/store/errors.go | Adds store-level errors for missing external identity and ambiguous normalized email. |
| internal/core/store.go | Extends UserReader with FindUserByNormalizedEmail. |
| internal/mocks/mock_store.go | Updates gomock stubs for the new store interface method. |
| internal/services/user.go | Trims OAuth inputs, uses normalized-email lookup for safe auto-link, promotes EmailVerified on trusted link/re-auth, downgrades on admin email change. |
| internal/services/user_test.go | Adds tests around admin email edits, normalization self-heal, and EmailVerified downgrades. |
| internal/services/user_oauth_test.go | Adds tests for verified/unverified OAuth, promotion on linking, and ambiguous legacy duplicates. |
| internal/services/token_exchange.go | Sets ID token email_verified claim from User.EmailVerified. |
| internal/services/token_test.go | Validates ID token email_verified mirrors stored user state. |
| internal/handlers/oidc.go | Sets UserInfo email_verified from User.EmailVerified. |
| internal/handlers/oidc_test.go | Updates/extends UserInfo claim tests for email_verified. |
| internal/store/store_test.go | Adds store-level tests for UpsertExternalUser behavior and normalized-email lookup. |
| docs/DEVELOPMENT.md | Documents AutoMigrate production caveat and an example manual migration for email_verified. |
| docs/ARCHITECTURE.md | Updates endpoint/claim documentation to reflect new email_verified semantics. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…andler - Add a dedicated case for services.ErrAmbiguousEmail in the OAuth callback so legacy whitespace-duplicate local accounts surface an actionable "contact your administrator" message at 409 Conflict instead of a generic 500 error
There was a problem hiding this comment.
Pull request overview
This PR persists per-user email verification state (User.EmailVerified) and exposes it via OIDC email_verified in both ID Tokens and UserInfo, using upstream OAuth provider verification where available.
Changes:
- Add
EmailVerifiedtomodels.User(DB-backed, defaultfalse) and wire OIDC claims to mirror it. - Normalize/validate external & OAuth user identity inputs; add whitespace-tolerant email lookup (
FindUserByNormalizedEmail) with ambiguity detection to protect OAuth auto-linking. - Extend OAuth flows to set/promote/downgrade
EmailVerifiedappropriately; add extensive test coverage and update docs.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| internal/models/user.go | Adds persisted EmailVerified field with default false. |
| internal/services/token_exchange.go | Sets ID Token email_verified from user.EmailVerified. |
| internal/handlers/oidc.go | Sets UserInfo email_verified from user.EmailVerified. |
| internal/store/user.go | Normalizes external-user inputs; adds FindUserByNormalizedEmail; trims GetUserByEmail input. |
| internal/store/errors.go | Introduces ErrExternalUserMissingIdentity and ErrAmbiguousEmail. |
| internal/core/store.go | Extends UserReader with FindUserByNormalizedEmail. |
| internal/services/user.go | Normalizes OAuth fields; uses normalized email lookup; promotes/downgrades EmailVerified; surfaces ambiguity to UI. |
| internal/handlers/oauth_handler.go | Renders a dedicated error page/status for ambiguous-email OAuth sign-in. |
| internal/mocks/mock_store.go | Regenerates mocks to include FindUserByNormalizedEmail. |
| internal/store/store_test.go | Adds tests for external upsert trimming/identity validation and normalized email ambiguity. |
| internal/services/user_test.go | Adds admin profile-update tests for normalization and EmailVerified downgrade behavior. |
| internal/services/user_oauth_test.go | Adds OAuth tests for verified/unverified providers, promotion behavior, and ambiguity rejection. |
| internal/services/token_test.go | Adds test ensuring ID Token email_verified mirrors User.EmailVerified. |
| internal/handlers/oidc_test.go | Updates/adds tests ensuring UserInfo email_verified mirrors User.EmailVerified. |
| docs/DEVELOPMENT.md | Documents AutoMigrate operational caveat and example migration for email_verified. |
| docs/ARCHITECTURE.md | Updates docs to reflect email_verified semantics. |
Comments suppressed due to low confidence (1)
internal/services/user.go:408
- AuthenticateWithOAuth treats any error from FindUserByNormalizedEmail as “user not found” unless it happens to be store.ErrAmbiguousEmail. That means unexpected DB errors (connection issues, SQL errors, etc.) can fall through into the auto-registration path, masking the real failure and potentially attempting to create/link users in an unhealthy state. Handle non-RecordNotFound errors explicitly (both from GetOAuthConnection and FindUserByNormalizedEmail) by returning an error instead of continuing; only proceed to auto-register when the lookup error is gorm.ErrRecordNotFound.
// 1. Check if OAuth connection exists
connection, err := s.store.GetOAuthConnection(provider, oauthUserInfo.ProviderUserID)
if err == nil {
// Connection exists: update token and return user
return s.updateOAuthConnectionAndGetUser(ctx, connection, oauthUserInfo, token)
}
// 2. Check if user exists with same email. Use the whitespace-tolerant
// normalized lookup so legacy rows are still found and any ambiguity
// between duplicate whitespace variants blocks auto-link instead of
// letting the provider bind to a non-deterministic row.
user, err := s.store.FindUserByNormalizedEmail(oauthUserInfo.Email)
if err == nil {
if !user.IsActive {
return nil, ErrAccountDisabled
}
// Only auto-link when the provider has verified the email address.
// Without this check, an attacker who controls an OAuth account with
// a victim's email could take over the victim's AuthGate account.
if oauthUserInfo.EmailVerified {
return s.linkOAuthToExistingUser(ctx, user, provider, oauthUserInfo, token)
}
log.Printf(
"[OAuth] Skipping auto-link for user=%s provider=%s: email not verified by provider",
user.Username,
provider,
)
// Fall through to auto-register check — treat as new user
if !s.oauthAutoRegister {
return nil, ErrOAuthEmailNotVerified
}
return s.createUserWithOAuth(ctx, provider, oauthUserInfo, token)
}
// Surface ambiguous-email errors explicitly: auto-registering would create
// yet another duplicate, and silently linking is unsafe because we cannot
// tell which of the existing rows the provider is vouching for.
if errors.Is(err, store.ErrAmbiguousEmail) {
log.Printf(
"[OAuth] Ambiguous email for provider=%s email=%s — manual dedup required",
provider,
oauthUserInfo.Email,
)
return nil, ErrAmbiguousEmail
}
// 3. Check if auto-registration is enabled
if !s.oauthAutoRegister {
return nil, ErrOAuthAutoRegisterDisabled
}
// 4. Create new user with OAuth
return s.createUserWithOAuth(ctx, provider, oauthUserInfo, token)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
User.EmailVerifiedcolumn (GORM AutoMigrate, defaultfalse) so per-user verification state can be persisted.email_verifiedclaim in both ID Token (token_exchange.go) and UserInfo (oidc.go) off that field instead of always returningfalse.OAuthUserInfo.EmailVerifiedon OAuth auto-registration, and promote the flag totruewhen linking a trusted provider to an existing account.Behaviour by source
false(unchanged default).true.false(those APIs don't expose verification status).true.Why
Previously every ID Token advertised
email_verified: falseregardless of how the user signed in, which made the claim effectively useless for relying parties that want to decide whether to trust the email. Now it accurately reflects whether a trusted upstream OAuth provider confirmed the address.Test plan
make generatemake fmtmake lint(0 issues)make test(all packages pass; addedTestBuildUserInfoClaims_EmailVerifiedMirrorsUserField,TestExchangeAuthorizationCode_IDToken_EmailVerifiedMirrorsUser,TestAuthenticateWithOAuth_NewUser_UnverifiedEmail,TestAuthenticateWithOAuth_LinkPromotesEmailVerified)make buildemail_verified=truein the issued ID Token //oauth/userinfo; sign in via local admin and confirm it staysfalse.🤖 Generated with Claude Code