Skip to content

Commit 197cfe2

Browse files
committed
fix(sso): serialize concurrent SAML account creation for the same email
1 parent 7f3c529 commit 197cfe2

2 files changed

Lines changed: 66 additions & 0 deletions

File tree

‎internal/api/external.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -311,6 +311,12 @@ func (a *API) createAccountFromExternalIdentity(tx *storage.Connection, r *http.
311311
identityData = structs.Map(userData.Metadata)
312312
}
313313

314+
if strings.HasPrefix(providerType, "sso:") && userData.Metadata.Email != "" {
315+
if terr := models.LockAccountLinking(tx, providerType, userData.Metadata.Email); terr != nil {
316+
return 0, nil, terr
317+
}
318+
}
319+
314320
decision, terr := models.DetermineAccountLinking(tx, config, userData.Emails, aud, providerType, userData.Metadata.Subject)
315321
if terr != nil {
316322
return 0, nil, terr

‎internal/api/external_test.go‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616
"github.com/supabase/auth/internal/api/provider"
1717
"github.com/supabase/auth/internal/conf"
1818
"github.com/supabase/auth/internal/models"
19+
"github.com/supabase/auth/internal/storage"
1920
)
2021

2122
type ExternalTestSuite struct {
@@ -92,6 +93,65 @@ func (ts *ExternalTestSuite) TestAutomaticLinkIdentityWritesAuditLog() {
9293
require.Len(ts.T(), logs, 1, "signing in with an existing identity must not emit another audit log")
9394
}
9495

96+
func (ts *ExternalTestSuite) TestSSOConcurrentCreateSameEmailLinksToOneUser() {
97+
ssoProvider := &models.SSOProvider{}
98+
require.NoError(ts.T(), ts.API.db.Create(ssoProvider))
99+
providerType := "sso:" + ssoProvider.ID.String()
100+
101+
userData := func(sub string) *provider.UserProvidedData {
102+
return &provider.UserProvidedData{
103+
Metadata: &provider.Claims{
104+
Subject: sub,
105+
Email: "sso-race@example.com",
106+
EmailVerified: true,
107+
},
108+
Emails: []provider.Email{{
109+
Email: "sso-race@example.com",
110+
Primary: true,
111+
Verified: true,
112+
}},
113+
}
114+
}
115+
r := httptest.NewRequest(http.MethodPost, "/sso/saml/acs", nil)
116+
117+
subs := []string{"sub-a", "sub-b"}
118+
decisions := make([]models.AccountLinkingDecision, len(subs))
119+
errs := make([]error, len(subs))
120+
121+
var wg sync.WaitGroup
122+
start := make(chan struct{})
123+
for i, sub := range subs {
124+
wg.Add(1)
125+
go func(i int, sub string) {
126+
defer wg.Done()
127+
<-start
128+
errs[i] = ts.API.db.Transaction(func(tx *storage.Connection) error {
129+
decision, _, terr := ts.API.createAccountFromExternalIdentity(tx, r, userData(sub), providerType, false)
130+
decisions[i] = decision
131+
return terr
132+
})
133+
}(i, sub)
134+
}
135+
close(start)
136+
wg.Wait()
137+
138+
for _, err := range errs {
139+
require.NoError(ts.T(), err)
140+
}
141+
142+
created := 0
143+
for _, decision := range decisions {
144+
if decision == models.CreateAccount {
145+
created++
146+
}
147+
}
148+
require.Equal(ts.T(), 1, created, "the lock must serialize concurrent SSO creates for the same email so only one account is created")
149+
150+
count, err := ts.API.db.Q().Where("email = ?", "sso-race@example.com").Count(&models.User{})
151+
require.NoError(ts.T(), err)
152+
require.EqualValues(ts.T(), 1, count)
153+
}
154+
95155
func (ts *ExternalTestSuite) createUser(providerId string, email string, name string, avatar string, confirmationToken string) (*models.User, error) {
96156
// Cleanup existing user, if they already exist
97157
if u, _ := models.FindUserByEmailAndAudience(ts.API.db, email, ts.Config.JWT.Aud); u != nil {

0 commit comments

Comments
 (0)