Skip to content

Commit

Permalink
feat: update automatic linking algorithm
Browse files Browse the repository at this point in the history
  • Loading branch information
@kangmingtay
kangmingtay committed Nov 13, 2023
1 parent fcb44cb commit 72765b0
Show file tree
Hide file tree
Showing 3 changed files with 146 additions and 94 deletions.
31 changes: 9 additions & 22 deletions internal/api/external.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -262,28 +262,14 @@ func (a *API) createAccountFromExternalIdentity(tx *storage.Connection, r *http.
aud := a.requestAud(ctx, r)
config := a.config

var terr error

var user *models.User
var identity *models.Identity
var identityData map[string]interface{}
if userData.Metadata != nil {
identityData = structs.Map(userData.Metadata)
}

var emailData provider.Email
var emails []string
// an oauth identity with an unverified email will not have an email present
for _, email := range userData.Emails {
if email.Verified || config.Mailer.Autoconfirm {
if email.Primary {
emailData = email
}
emails = append(emails, strings.ToLower(email.Email))
}
}

decision, terr := models.DetermineAccountLinking(tx, providerType, userData.Metadata.Subject, emails)
decision, terr := models.DetermineAccountLinking(tx, userData.Emails, aud, providerType, userData.Metadata.Subject)
if terr != nil {
return nil, terr
}
Expand All @@ -307,15 +293,17 @@ func (a *API) createAccountFromExternalIdentity(tx *storage.Connection, r *http.

params := &SignupParams{
Provider: providerType,
Email: emailData.Email,
Email: decision.CandidateEmail.Email,
Aud: aud,
Data: identityData,
}

isSSOUser := strings.HasPrefix(providerType, "sso:")
isSSOUser := false
if decision.LinkingDomain == "sso" {
isSSOUser = true
}

user, terr = a.signupNewUser(ctx, tx, params, isSSOUser)
if terr != nil {
if user, terr = a.signupNewUser(ctx, tx, params, isSSOUser); terr != nil {
return nil, terr
}

Expand Down Expand Up @@ -357,7 +345,7 @@ func (a *API) createAccountFromExternalIdentity(tx *storage.Connection, r *http.
if terr = user.RemoveUnconfirmedIdentities(tx, identity); terr != nil {
return nil, internalServerError("Error updating user").WithInternalError(terr)
}
if emailData.Verified || config.Mailer.Autoconfirm {
if decision.CandidateEmail.Verified || config.Mailer.Autoconfirm {
if terr := models.NewAuditLogEntry(r, tx, user, models.UserSignedUpAction, "", map[string]interface{}{
"provider": providerType,
}); terr != nil {
Expand All @@ -372,8 +360,7 @@ func (a *API) createAccountFromExternalIdentity(tx *storage.Connection, r *http.
return nil, internalServerError("Error updating user").WithInternalError(terr)
}
} else {
if user.Email != "" {
// an oauth identity with an unverified email will not have an email present
if decision.CandidateEmail.Email != "" {
mailer := a.Mailer(ctx)
referrer := utilities.GetReferrer(r, config)
externalURL := getExternalHost(ctx)
Expand Down
138 changes: 81 additions & 57 deletions internal/models/linking.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package models
import (
"strings"

p "github.com/supabase/gotrue/internal/api/provider"
"github.com/supabase/gotrue/internal/storage"
)

Expand Down Expand Up @@ -34,12 +35,11 @@ const (
)

type AccountLinkingResult struct {
Decision AccountLinkingDecision

User *User
Identities []*Identity

LinkingDomain string
Decision AccountLinkingDecision
User *User
Identities []*Identity
LinkingDomain string
CandidateEmail p.Email
}

// DetermineAccountLinking uses the provided data and database state to compute a decision on whether:
Expand All @@ -49,7 +49,19 @@ type AccountLinkingResult struct {
// - It's not possible to decide due to data inconsistency (MultipleAccounts) and the caller should decide
//
// Errors signal failure in processing only, like database access errors.
func DetermineAccountLinking(tx *storage.Connection, provider, sub string, emails []string) (AccountLinkingResult, error) {
func DetermineAccountLinking(tx *storage.Connection, emails []p.Email, aud, provider, sub string) (AccountLinkingResult, error) {
var verifiedEmails []string
var candidateEmail p.Email
for _, email := range emails {
if email.Verified {
verifiedEmails = append(verifiedEmails, strings.ToLower(email.Email))
}
if email.Primary {
candidateEmail = email
candidateEmail.Email = strings.ToLower(email.Email)
}
}

if identity, terr := FindIdentityByIdAndProvider(tx, sub, provider); terr == nil {
// account exists

Expand All @@ -59,58 +71,65 @@ func DetermineAccountLinking(tx *storage.Connection, provider, sub string, email
}

return AccountLinkingResult{
Decision: AccountExists,
User: user,
Identities: []*Identity{identity},
LinkingDomain: GetAccountLinkingDomain(provider),
Decision: AccountExists,
User: user,
Identities: []*Identity{identity},
LinkingDomain: GetAccountLinkingDomain(provider),
CandidateEmail: candidateEmail,
}, nil
} else if !IsNotFoundError(terr) {
return AccountLinkingResult{}, terr
}

// account does not exist, identity and user not immediately
// identifiable, look for similar identities based on email
var similarIdentities []*Identity
var similarUsers []*User
// the identity does not exist, so we need to check if we should create a new account
// or link to an existing one

if len(emails) > 0 {
if terr := tx.Q().Eager().Where("email ilike any (?)", emails).All(&similarIdentities); terr != nil {
// this is the linking domain for the new identity
candidateLinkingDomain := GetAccountLinkingDomain(provider)
if len(verifiedEmails) == 0 {
// if there are no verified emails, we always decide to create a new account
user, terr := IsDuplicatedEmail(tx, candidateEmail.Email, aud, nil)
if 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 ilike any (?) and is_sso_user is false", emails).All(&similarUsers); terr != nil {
return AccountLinkingResult{}, terr
if user != nil {
candidateEmail = p.Email{
Email: "",
Verified: false,
Primary: false,
}
}
}

// TODO: determine linking behavior over phone too
if len(similarIdentities) == 0 && len(similarUsers) == 0 {
// there are no similar identities, clearly we have to create a new account

return AccountLinkingResult{
Decision: CreateAccount,
LinkingDomain: GetAccountLinkingDomain(provider),
Decision: CreateAccount,
LinkingDomain: candidateLinkingDomain,
CandidateEmail: candidateEmail,
}, nil
}

// there are some similar identities, we now need to proceed in
// identifying whether this supposed new identity should be assigned to
// an existing user or to create a new user, according to the automatic
// linking rules
var similarIdentities []*Identity
var similarUsers []*User
// look for similar identities and users based on email
if terr := tx.Q().Eager().Where("email ilike any (?)", verifiedEmails).All(&similarIdentities); terr != nil {
return AccountLinkingResult{}, terr
}

// this is the linking domain for the new identity
newAccountLinkingDomain := GetAccountLinkingDomain(provider)
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 ilike any (?) and is_sso_user is false", verifiedEmails).All(&similarUsers); terr != nil {
return AccountLinkingResult{}, terr
}
}

// Need to check if the new identity should be assigned to an
// existing user or to create a new user, according to the automatic
// linking rules
var linkingIdentities []*Identity

// now let's see if there are any existing and similar identities in
// the same linking domain
for _, identity := range similarIdentities {
if GetAccountLinkingDomain(identity.Provider) == newAccountLinkingDomain {
if GetAccountLinkingDomain(identity.Provider) == candidateLinkingDomain {
linkingIdentities = append(linkingIdentities, identity)
}
}
Expand All @@ -121,41 +140,45 @@ func DetermineAccountLinking(tx *storage.Connection, provider, sub string, email
// so we link this new identity to the user
// TODO: Backfill the missing identity for the user
return AccountLinkingResult{
Decision: LinkAccount,
User: similarUsers[0],
Identities: linkingIdentities,
LinkingDomain: newAccountLinkingDomain,
Decision: LinkAccount,
User: similarUsers[0],
Identities: linkingIdentities,
LinkingDomain: candidateLinkingDomain,
CandidateEmail: candidateEmail,
}, nil
} else if len(similarUsers) > 1 {
// this shouldn't happen since there is a partial unique index on (email and is_sso_user = false)
return AccountLinkingResult{
Decision: MultipleAccounts,
Identities: linkingIdentities,
LinkingDomain: newAccountLinkingDomain,
Decision: MultipleAccounts,
Identities: linkingIdentities,
LinkingDomain: candidateLinkingDomain,
CandidateEmail: candidateEmail,
}, nil
} else {
// there are no identities in the linking domain, we have to
// create a new identity and new user
return AccountLinkingResult{
Decision: CreateAccount,
LinkingDomain: newAccountLinkingDomain,
Decision: CreateAccount,
LinkingDomain: candidateLinkingDomain,
CandidateEmail: candidateEmail,
}, nil
}
}

// there is at least one identity in the linking domain let's do a
// sanity check to see if all of the identities in the domain share the
// same user ID

linkingUserId := linkingIdentities[0].UserID
for _, identity := range linkingIdentities {
if identity.UserID != linkingIdentities[0].UserID {
if identity.UserID != linkingUserId {
// ok this linking domain has more than one user account
// caller should decide what to do

return AccountLinkingResult{
Decision: MultipleAccounts,
Identities: linkingIdentities,
LinkingDomain: newAccountLinkingDomain,
Decision: MultipleAccounts,
Identities: linkingIdentities,
LinkingDomain: candidateLinkingDomain,
CandidateEmail: candidateEmail,
}, nil
}
}
Expand All @@ -166,14 +189,15 @@ func DetermineAccountLinking(tx *storage.Connection, provider, sub string, email
var user *User
var terr error

if user, terr = FindUserByID(tx, linkingIdentities[0].UserID); terr != nil {
if user, terr = FindUserByID(tx, linkingUserId); terr != nil {
return AccountLinkingResult{}, terr
}

return AccountLinkingResult{
Decision: LinkAccount,
User: user,
Identities: linkingIdentities,
LinkingDomain: newAccountLinkingDomain,
Decision: LinkAccount,
User: user,
Identities: linkingIdentities,
LinkingDomain: candidateLinkingDomain,
CandidateEmail: candidateEmail,
}, nil
}
Loading

0 comments on commit 72765b0

Please sign in to comment.