Skip to content

Commit

Permalink
fix: lowercase oauth emails for account linking (supabase#1125)
Browse files Browse the repository at this point in the history
  • Loading branch information
kangmingtay authored Jun 3, 2023
1 parent 577a97e commit df22915
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 13 deletions.
2 changes: 1 addition & 1 deletion internal/api/external.go
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ func (a *API) createAccountFromExternalIdentity(tx *storage.Connection, r *http.

for _, email := range userData.Emails {
if email.Verified || config.Mailer.Autoconfirm {
emails = append(emails, email.Email)
emails = append(emails, strings.ToLower(email.Email))
}
}

Expand Down
4 changes: 2 additions & 2 deletions internal/models/linking.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,14 +74,14 @@ func DetermineAccountLinking(tx *storage.Connection, provider, sub string, email
var similarUsers []*User

if len(emails) > 0 {
if terr := tx.Q().Eager().Where("email in (?)", emails).All(&similarIdentities); terr != nil {
if terr := tx.Q().Eager().Where("email ilike any (?)", emails).All(&similarIdentities); terr != nil {
return AccountLinkingResult{}, terr
}

if !strings.HasPrefix(provider, "sso:") {
// there can be multiple user accounts with the same email when is_sso_user is true
// so we just do not consider those similar user accounts
if terr := tx.Q().Eager().Where("email in (?) and is_sso_user is false", emails).All(&similarUsers); terr != nil {
if terr := tx.Q().Eager().Where("email ilike any (?) and is_sso_user is false", emails).All(&similarUsers); terr != nil {
return AccountLinkingResult{}, terr
}
}
Expand Down
49 changes: 39 additions & 10 deletions internal/models/linking_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,12 +125,6 @@ func (ts *AccountLinkingTestSuite) TestLinkAccountExists() {
require.NoError(ts.T(), err)
require.NoError(ts.T(), ts.db.Create(identityA))

// link decision because the below described identity is in the default linking domain but uses "other-provider" instead of "provder"
decision, err := DetermineAccountLinking(ts.db, "other-provider", userA.ID.String(), []string{"[email protected]"})
require.NoError(ts.T(), err)

require.Equal(ts.T(), decision.Decision, LinkAccount)

userB, err := NewUser("", "[email protected]", "", "authenticated", nil)
require.NoError(ts.T(), err)
require.NoError(ts.T(), ts.db.Create(userB))
Expand All @@ -142,11 +136,46 @@ func (ts *AccountLinkingTestSuite) TestLinkAccountExists() {
require.NoError(ts.T(), err)
require.NoError(ts.T(), ts.db.Create(identityB))

// no link decision because the SSO linking domain is scoped to the provider unique ID
decision, err = DetermineAccountLinking(ts.db, "sso:f06f9e3d-ff92-4c47-a179-7acf1fda6387", userB.ID.String(), []string{"[email protected]"})
require.NoError(ts.T(), err)
cases := []struct {
desc string
email string
sub string
provider string
decision AccountLinkingDecision
}{
{
// link decision because the below described identity is in the default linking domain but uses "other-provider" instead of "provder"
desc: "same email address",
email: "[email protected]",
sub: userA.ID.String(),
provider: "other-provider",
decision: LinkAccount,
},
{
desc: "same email address in uppercase",
email: "[email protected]",
sub: userA.ID.String(),
provider: "other-provider",
decision: LinkAccount,
},
{
desc: "no link decision because the SSO linking domain is scoped to the provider unique ID",
email: "[email protected]",
sub: userB.ID.String(),
provider: "sso:f06f9e3d-ff92-4c47-a179-7acf1fda6387",
decision: AccountExists,
},
}

for _, c := range cases {
ts.Run(c.desc, func() {
decision, err := DetermineAccountLinking(ts.db, c.provider, c.sub, []string{c.email})
require.NoError(ts.T(), err)

require.Equal(ts.T(), decision.Decision, c.decision)
})
}

require.NotEqual(ts.T(), decision.Decision, LinkAccount)
}

func (ts *AccountLinkingTestSuite) TestMultipleAccounts() {
Expand Down

0 comments on commit df22915

Please sign in to comment.