feat: add option to skip email verification for SSO registrations - #179
Open
heudev wants to merge 2 commits into
Open
feat: add option to skip email verification for SSO registrations#179heudev wants to merge 2 commits into
heudev wants to merge 2 commits into
Conversation
trustEmailVerified only confirms an email when the provider actually sends an email_verified claim. Providers that omit it entirely, such as Microsoft's OIDC userinfo endpoint, leave every SSO user unverified with no way to opt out.
Matching an incoming address against an existing account is an account takeover vector when the provider has not actually verified it, so that path stays keyed off a genuine email_verified claim.
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
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.
Adds a per-strategy "Skip email verification for people who register using SSO?" option, mirroring the setting the first-party SSO plugins expose.
Why
trustEmailVerifiedonly confirms an email when the provider sends anemail_verifiedclaim that is truthy:Not every provider sends that claim. Microsoft's
https://graph.microsoft.com/oidc/userinfois one — a real response containssub,givenname,familyname,email,localeandpicture, and noemail_verifiedat all. The address is there and is the one the account is registered with, buttrustEmailVerifiedcan never fire, so every user arriving through that strategy stays unverified and hits the "posting in some categories is enabled once your email is confirmed" wall.There is currently no way to express "I trust this provider's addresses outright". Loosening
trustEmailVerifiedto treat a missing claim as verified would silently change behaviour for existing installs, so this adds an explicit opt-in instead and leavestrustEmailVerifiedexactly as it was.What changed
skipEmailVerificationcheckbox on the strategy editor, registered ineditStrategy's checkbox list so it is normalised to1/0like its siblings.OAuth.isEmailTrusted(strategy, payload): the new option short-circuits totrue, otherwise the existingtrustEmailVerifiedbehaviour is unchanged.OAuth.loginreturns early for users who already have an association, which meant the email was only ever confirmed at registration time. Turning the option on therefore did nothing for accounts that had already signed in once.confirmEmailIfTrustedcloses that gap, and refuses to act when the account is already confirmed or when the stored address differs from the one the provider just sent.The email fallback is deliberately left alone
OAuth.loginmatches an incoming address against existing users before creating an account:Routing the new option through that same flag would mean "skip email verification" also silently grants "attach this provider to whatever account already owns the address it claims" — an account takeover vector for any provider that does not actually verify its addresses, up to and including an admin's account. So the two decisions are separated:
isEmailTrusted(which the new option relaxes) governs confirmation, whileisEmailVerifiedByProviderstill demands a genuineemail_verifiedclaim before any account is matched by email.Testing
Verified on a live NodeBB 4.14.10 forum against a Microsoft strategy. With the option enabled, an account that had signed in previously moved to
email:confirmed = 1and joinedverified-userson its next login; with the option off, behaviour is identical to before.npx eslint .is clean.