Skip to content

fix(signup): return 409 instead of raw 500 on concurrent signup race (fixes #2675) #2712

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
deepshekhardas wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix(signup): return 409 instead of raw 500 on concurrent signup race (fixes #2675) #2712

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions internal/api/signup.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"github.com/supabase/auth/internal/metering"
"github.com/supabase/auth/internal/models"
"github.com/supabase/auth/internal/storage"
"github.com/supabase/auth/internal/utilities"
)

// SignupParams are the parameters the Signup endpoint accepts
Expand Down Expand Up @@ -383,6 +384,10 @@ func (a *API) signupNewUser(conn *storage.Connection, user *models.User) (*model
err := conn.Transaction(func(tx *storage.Connection) error {
var terr error
if terr = tx.Create(user); terr != nil {
if pgErr := utilities.NewPostgresError(terr); pgErr != nil && pgErr.IsUniqueConstraintViolated() {
// A concurrent request created the same user first
return apierrors.NewConflictError(apierrors.ErrorCodeUserAlreadyExists, "User already exists").WithInternalError(terr)
Comment on lines +388 to +389

@depthfirst-app depthfirst-app Bot Aug 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Severity: MEDIUM

An unauthenticated caller controls the signup email and X-JWT-AUD; when the audience-scoped lookup misses an email belonging to another audience, the global users_email_partial_key reaches this branch and returns user_already_exists. On structured-error API versions, that response exposes cross-audience account existence.
Helpful? Add 👍 / 👎

💡 Fix Suggestion

Suggestion: In signupNewUser, the unique-constraint violation on tx.Create(user) is currently mapped unconditionally to 409 user_already_exists. This leaks cross-audience email existence because IsDuplicatedEmail filters by audience and misses a user in a different audience, but the global users_email_partial_key index still fires.

The fix is to add a same-audience confirmation lookup before returning user_already_exists. After the constraint violation, tx (and the closure's conn, since they may be the same aborted connection when called from within an outer transaction) cannot be used for subsequent queries. Instead, use a.db — the non-transactional connection held by the API struct — to call models.FindUserByEmailAndAudience(a.db, user.GetEmail(), user.Aud). Only return the user_already_exists 409 if a user is confirmed to exist in the SAME audience (a legitimate concurrent race). Otherwise fall through to the existing generic NewInternalServerError on line 391, which avoids exposing cross-audience account existence to the unauthenticated caller.

⚠️ Experimental Feature: This code suggestion is automatically generated. Please review carefully.

Suggested change
// A concurrent request created the same user first
return apierrors.NewConflictError(apierrors.ErrorCodeUserAlreadyExists, "User already exists").WithInternalError(terr)
// Only surface user_already_exists for a same-audience concurrent race;
// a cross-audience email collision must not reveal cross-audience existence.
// Use a.db (non-transactional) because conn/tx is aborted after the violation.
if existing, _ := models.FindUserByEmailAndAudience(a.db, user.GetEmail(), user.Aud); existing != nil {
return apierrors.NewConflictError(apierrors.ErrorCodeUserAlreadyExists, "User already exists").WithInternalError(terr)
}

}
return apierrors.NewInternalServerError("Database error saving new user").WithInternalError(terr)
}
if terr = user.SetRole(tx, config.JWT.DefaultGroupName); terr != nil {
Expand Down