fix(auth): handle concurrent OAuth callback creating the same auth service - #2780
Open
mroderick wants to merge 1 commit into
Open
fix(auth): handle concurrent OAuth callback creating the same auth service#2780mroderick wants to merge 1 commit into
mroderick wants to merge 1 commit into
Conversation
…rvice A double-fired OAuth callback can pass the initial service lookup concurrently, then collide on the unique (uid, provider) index (or the auth_service uniqueness validation) during member.save!, erroring the request. Rescue RecordInvalid and RecordNotUnique and reuse the already-created auth_service, guarded by find_by! so a non-race failure still surfaces. Toggle can_log_in only on actual creation so the losing callback does not flip the winner flag back.
mroderick
marked this pull request as ready for review
August 4, 2026 11:51
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.
Problem
A double-fired OAuth callback can pass the initial
AuthService.find_by(provider:, uid:)lookup concurrently (both see nothing), then both enter the create branch and collide whilemember.save!saves the nested auth service. The unique(uid, provider)index (already live in prod) plus the auth_service uniqueness validation make one request error —RecordNotUniqueon a true TOCTOU race, orRecordInvalidvia the app validation when the duplicate is already visible. Result: an errored login request.What this does
In
auth_services_controller#create, wrapmember.save!in a rescue of bothRecordInvalidandRecordNotUnique, and on collision reuse the already-created auth service:AuthService.find_by!(provider:, uid:)acts as the guard — if no winner exists this isn't a race, so it raises the real error instead of swallowing it.member.toggle!(:can_log_in)only runs on actual creation (created), so the losing callback doesn't flip the winner's flag back.Verification
spec/requests/auth_services_callback_spec.rb) deterministically exercises the rescue and asserts the loser reuses the winner's member/service and does not togglecan_log_in.Why a separate PR from #2779
The
auth_services (uid, provider)index already shipped to prod in the first partial run of the unique-index migration, so this fix is independently urgent and not gated on #2779 — it gets focused auth review on its own clock.