chore: harden core identity provider authentication and sync#122
Merged
Conversation
Compact reference covering the coreIdProvider concept, configuration, data model, lifecycle (login adoption, conflict guard, patchCoreAuthUser, keepalive refresh), the idp-flag restrictions, member role/department auto-sync, and pointers to the cross-site / email-trust trust boundaries. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
A user pre-existing as a local-password account that subsequently logs in via a core identity provider was adopted into that provider but kept its password (and 2FA) intact, leaving the password channel as a parallel authentication path that the schema description on coreIdProvider explicitly forbids. Revoking access at the IdP would refresh-fail the SSO session but the password still worked. Two-pronged fix: - patchCoreAuthUser now clears password, passwordUpdate and 2FA at binding time (mongo patchUser translates null -> $unset). - /auth/password and /auth/passwordless reject any target user with coreIdProvider set, returning the existing enumeration-resistant shape (badCredentials 400 / silent 204). Defense in depth against any residual credential data from older accounts. Tested by an api spec that creates a password account, sanity-checks password login, triggers OIDC core adoption on the same email, and asserts the second password attempt is now refused. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
patchCoreAuthUser auto-applied memberships returned by the core IdP via storage.addMember on every login. Under multiRoles=true, addMember keys memberships by (org, dep, role) — so when a provider-asserted role changed between two logins of the same user, a duplicate membership was appended instead of the existing one being updated. Memberships the provider no longer asserted at all stayed on the user record forever. Read-only memberships are only ever set via this auto-sync path (authProviderMemberInfo flips readOnly=true when a core IdP also configures memberRole or memberDepartment), so dropping all readOnly entries before re-applying the current set is safe and gives the IdP authoritative control over the slice of the user's memberships it owns. Surfaced by the existing oidc.api.spec.ts:50:3 test, which asserts a single replaced membership on second login but was producing two stacked entries until now. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.
Fixes some gaps in core id provider promises and actual implementation.