Skip to content

feat: add DangerousSSOAutoLinking config flag for automatic SSO ident… #2351

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
yuvalkarmi wants to merge 1 commit into
base: master
Choose a base branch
from
Open

feat: add DangerousSSOAutoLinking config flag for automatic SSO ident… #2351

Show file tree
Hide file tree
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
2 changes: 1 addition & 1 deletion cmd/admin_cmd.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ func adminCreateUser(config *conf.GlobalConfiguration, args []string) {
defer db.Close()

aud := getAudience(config)
if user, err := models.IsDuplicatedEmail(db, args[0], aud, nil, config.Experimental.ProvidersWithOwnLinkingDomain); user != nil {
if user, err := models.IsDuplicatedEmail(db, args[0], aud, nil, config.Experimental.ProvidersWithOwnLinkingDomain, config.Security.DangerousSSOAutoLinking); user != nil {
logrus.Fatalf("Error creating new user: user already exists")
} else if err != nil {
logrus.Fatalf("Error checking user email: %+v", err)
Expand Down
2 changes: 1 addition & 1 deletion internal/api/admin.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,7 @@ func (a *API) adminUserCreate(w http.ResponseWriter, r *http.Request) error {
if err != nil {
return err
}
if user, err := models.IsDuplicatedEmail(db, params.Email, aud, nil, config.Experimental.ProvidersWithOwnLinkingDomain); err != nil {
if user, err := models.IsDuplicatedEmail(db, params.Email, aud, nil, config.Experimental.ProvidersWithOwnLinkingDomain, config.Security.DangerousSSOAutoLinking); err != nil {
return apierrors.NewInternalServerError("Database error checking email").WithInternalError(err)
} else if user != nil {
return apierrors.NewUnprocessableEntityError(apierrors.ErrorCodeEmailExists, DuplicateEmailMsg)
Expand Down
2 changes: 1 addition & 1 deletion internal/api/mail.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ func (a *API) adminGenerateLink(w http.ResponseWriter, r *http.Request) error {
if terr != nil {
return terr
}
if duplicateUser, terr := models.IsDuplicatedEmail(tx, params.NewEmail, user.Aud, user, config.Experimental.ProvidersWithOwnLinkingDomain); terr != nil {
if duplicateUser, terr := models.IsDuplicatedEmail(tx, params.NewEmail, user.Aud, user, config.Experimental.ProvidersWithOwnLinkingDomain, config.Security.DangerousSSOAutoLinking); terr != nil {
return apierrors.NewInternalServerError("Database error checking email").WithInternalError(terr)
} else if duplicateUser != nil {
return apierrors.NewUnprocessableEntityError(apierrors.ErrorCodeEmailExists, DuplicateEmailMsg)
Expand Down
2 changes: 1 addition & 1 deletion internal/api/signup.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ func (a *API) Signup(w http.ResponseWriter, r *http.Request) error {
if err != nil {
return err
}
user, err = models.IsDuplicatedEmail(db, params.Email, params.Aud, nil, config.Experimental.ProvidersWithOwnLinkingDomain)
user, err = models.IsDuplicatedEmail(db, params.Email, params.Aud, nil, config.Experimental.ProvidersWithOwnLinkingDomain, config.Security.DangerousSSOAutoLinking)
case "phone":
if !config.External.Phone.Enabled {
return apierrors.NewBadRequestError(apierrors.ErrorCodePhoneProviderDisabled, "Phone signups are disabled")
Expand Down
2 changes: 1 addition & 1 deletion internal/api/user.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ func (a *API) UserUpdate(w http.ResponseWriter, r *http.Request) error {
}

if params.Email != "" && user.GetEmail() != params.Email {
if duplicateUser, err := models.IsDuplicatedEmail(db, params.Email, aud, user, config.Experimental.ProvidersWithOwnLinkingDomain); err != nil {
if duplicateUser, err := models.IsDuplicatedEmail(db, params.Email, aud, user, config.Experimental.ProvidersWithOwnLinkingDomain, config.Security.DangerousSSOAutoLinking); err != nil {
return apierrors.NewInternalServerError("Database error checking email").WithInternalError(err)
} else if duplicateUser != nil {
return apierrors.NewUnprocessableEntityError(apierrors.ErrorCodeEmailExists, DuplicateEmailMsg)
Expand Down
6 changes: 6 additions & 0 deletions internal/conf/configuration.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -733,6 +733,12 @@ type SecurityConfiguration struct {
ManualLinkingEnabled bool `json:"manual_linking_enabled" split_words:"true" default:"false"`
SbForwardedForEnabled bool `json:"sb_forwarded_for_enabled" split_words:"true" default:"false"`

// DangerousSSOAutoLinking enables automatic identity linking for SSO providers.
// When enabled, SSO identities will be linked to existing accounts based on email
// matching, just like OAuth providers. WARNING: Only enable if you trust your SSO
// IdP to verify email ownership, as a malicious IdP could claim any email.
DangerousSSOAutoLinking bool `json:"dangerous_sso_auto_linking" split_words:"true" default:"false"`

DBEncryption DatabaseEncryptionConfiguration `json:"database_encryption" split_words:"true"`
}

Expand Down
26 changes: 18 additions & 8 deletions internal/models/linking.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,21 @@ import (
// _should_ generally fall under the same User entity. It's just a runtime
// string, and is not typically persisted in the database. This value can vary
// across time.
func GetAccountLinkingDomain(provider string, ownLinkingDomains []string) string {
if strings.HasPrefix(provider, "sso:") || slices.Contains(ownLinkingDomains, provider) {
// when the provider ID is a SSO provider, then the linking
// domain is the provider itself i.e. there can only be one
// user + identity per identity provider
func GetAccountLinkingDomain(provider string, ownLinkingDomains []string, dangerousSSOAutoLinking bool) string {
if strings.HasPrefix(provider, "sso:") {
if dangerousSSOAutoLinking {
// When dangerous SSO auto-linking is enabled, treat SSO providers
// like OAuth providers - they join the default linking domain and
// will be linked to existing accounts based on email matching.
return "default"
Copy link
Contributor

@depthfirst-app depthfirst-app bot Jan 25, 2026

Choose a reason for hiding this comment

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

🟠 Severity: HIGH

Authentication Bypass via Malicious SSO IdP: When DangerousSSOAutoLinking is enabled, SSO providers automatically trust the IdP's email verification (hardcoded EmailVerified = true in samlacs.go). A compromised or malicious SAML IdP can claim ANY email address and automatically link to/takeover existing accounts. Unlike globally trusted OAuth providers, SSO IdPs are customer-configured and may lack proper email verification controls.
Helpful? Add 👍 / 👎

💡 Fix Suggestion

Suggestion: This is a complex architectural security issue that requires multiple complementary mitigations:

  1. Implement an SSO Provider Allowlist: Add configuration to explicitly specify which SSO providers are trusted for auto-linking (e.g., GOTRUE_SECURITY_SSO_AUTO_LINKING_ALLOWED_PROVIDERS). Modify GetAccountLinkingDomain() to only return 'default' for SSO providers that are on this allowlist.

  2. Add Account Linking Confirmation Flow: Before completing the account link, send a confirmation email to the existing account's verified email address with a time-limited token. Only complete the link after user confirmation. This requires changes to the DetermineAccountLinking flow and new database tables for pending links.

  3. Implement Enhanced Logging and Alerting: Add comprehensive audit logging for all SSO auto-linking events, including the IdP identifier, email being linked, and user IDs involved. Configure alerts for suspicious patterns (e.g., multiple linking attempts, links to privileged accounts).

  4. Multi-Factor Attribute Matching: Beyond email matching, require additional IdP-provided attributes to match (e.g., name, employee ID) before allowing auto-linking. Add configuration for required matching attributes.

  5. IdP Certificate Validation: Ensure SAML signature validation is enforced and add monitoring for certificate changes on SSO providers to detect potential compromises.

These changes span multiple files (linking.go, configuration.go, samlacs.go, audit logging system, and database migrations) and require careful consideration of backward compatibility and migration paths for existing deployments.

}
// Default behavior: SSO providers get their own isolated linking domain,
// meaning they will always create new user accounts.
return provider
}

if slices.Contains(ownLinkingDomains, provider) {
// Providers explicitly configured to have their own linking domain
return provider
}

Expand Down Expand Up @@ -65,7 +75,7 @@ func DetermineAccountLinking(tx *storage.Connection, config *conf.GlobalConfigur
}

// this is the linking domain for the new identity
candidateLinkingDomain := GetAccountLinkingDomain(providerName, config.Experimental.ProvidersWithOwnLinkingDomain)
candidateLinkingDomain := GetAccountLinkingDomain(providerName, config.Experimental.ProvidersWithOwnLinkingDomain, config.Security.DangerousSSOAutoLinking)

if identity, terr := FindIdentityByIdAndProvider(tx, sub, providerName); terr == nil {
// account exists
Expand Down Expand Up @@ -93,7 +103,7 @@ func DetermineAccountLinking(tx *storage.Connection, config *conf.GlobalConfigur
// or link to an existing one
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, config.Experimental.ProvidersWithOwnLinkingDomain)
user, terr := IsDuplicatedEmail(tx, candidateEmail.Email, aud, nil, config.Experimental.ProvidersWithOwnLinkingDomain, config.Security.DangerousSSOAutoLinking)
if terr != nil {
return AccountLinkingResult{}, terr
}
Expand Down Expand Up @@ -131,7 +141,7 @@ func DetermineAccountLinking(tx *storage.Connection, config *conf.GlobalConfigur
// 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, config.Experimental.ProvidersWithOwnLinkingDomain) == candidateLinkingDomain {
if GetAccountLinkingDomain(identity.Provider, config.Experimental.ProvidersWithOwnLinkingDomain, config.Security.DangerousSSOAutoLinking) == candidateLinkingDomain {
linkingIdentities = append(linkingIdentities, identity)
}
}
Expand Down
109 changes: 109 additions & 0 deletions internal/models/linking_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -312,3 +312,112 @@ func (ts *AccountLinkingTestSuite) TestMultipleAccounts() {

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

func (ts *AccountLinkingTestSuite) TestGetAccountLinkingDomainSSO() {
ssoProvider := "sso:f06f9e3d-ff92-4c47-a179-7acf1fda6387"

// When DangerousSSOAutoLinking is disabled, SSO providers get their own isolated domain
domain := GetAccountLinkingDomain(ssoProvider, nil, false)
require.Equal(ts.T(), ssoProvider, domain, "SSO should get its own domain when DangerousSSOAutoLinking is disabled")

// When DangerousSSOAutoLinking is enabled, SSO providers join the default domain
domain = GetAccountLinkingDomain(ssoProvider, nil, true)
require.Equal(ts.T(), "default", domain, "SSO should join default domain when DangerousSSOAutoLinking is enabled")

// Regular OAuth providers always get default domain regardless of the flag
domain = GetAccountLinkingDomain("google", nil, false)
require.Equal(ts.T(), "default", domain)

domain = GetAccountLinkingDomain("google", nil, true)
require.Equal(ts.T(), "default", domain)
}

func (ts *AccountLinkingTestSuite) TestSSOAutoLinkingEnabled() {
// Create an existing user with email/password
existingUser, err := NewUser("", "test@example.com", "password", ts.config.JWT.Aud, nil)
require.NoError(ts.T(), err)
require.NoError(ts.T(), ts.db.Create(existingUser))
existingIdentity, err := NewIdentity(existingUser, "email", map[string]interface{}{
"sub": existingUser.ID.String(),
"email": "test@example.com",
})
require.NoError(ts.T(), err)
require.NoError(ts.T(), ts.db.Create(existingIdentity))

ssoProvider := "sso:f06f9e3d-ff92-4c47-a179-7acf1fda6387"

// Enable DangerousSSOAutoLinking
ts.config.Security.DangerousSSOAutoLinking = true
defer func() {
ts.config.Security.DangerousSSOAutoLinking = false
}()

// Now when SSO user logs in with same email, they should be linked to existing user
decision, err := DetermineAccountLinking(ts.db, ts.config, []provider.Email{
{
Email: "test@example.com",
Verified: true,
Primary: true,
},
}, ts.config.JWT.Aud, ssoProvider, "sso-user-subject-id")
require.NoError(ts.T(), err)

require.Equal(ts.T(), LinkAccount, decision.Decision, "SSO should link to existing user when DangerousSSOAutoLinking is enabled")
require.Equal(ts.T(), "default", decision.LinkingDomain, "SSO should be in default linking domain")
require.Equal(ts.T(), existingUser.ID, decision.User.ID, "SSO should link to existing user")
}

func (ts *AccountLinkingTestSuite) TestSSOAutoLinkingCreatesNewUserWhenNoMatch() {
// Enable DangerousSSOAutoLinking
ts.config.Security.DangerousSSOAutoLinking = true
defer func() {
ts.config.Security.DangerousSSOAutoLinking = false
}()

ssoProvider := "sso:f06f9e3d-ff92-4c47-a179-7acf1fda6387"

// When no matching user exists, SSO should still create a new account
decision, err := DetermineAccountLinking(ts.db, ts.config, []provider.Email{
{
Email: "new-sso-user@example.com",
Verified: true,
Primary: true,
},
}, ts.config.JWT.Aud, ssoProvider, "new-sso-subject-id")
require.NoError(ts.T(), err)

require.Equal(ts.T(), CreateAccount, decision.Decision, "SSO should create new account when no matching user exists")
require.Equal(ts.T(), "default", decision.LinkingDomain, "SSO should be in default linking domain even when creating account")
}

func (ts *AccountLinkingTestSuite) TestSSOAutoLinkingDisabledCreatesNewUser() {
// Create an existing user with email/password
existingUser, err := NewUser("", "test-disabled@example.com", "password", ts.config.JWT.Aud, nil)
require.NoError(ts.T(), err)
require.NoError(ts.T(), ts.db.Create(existingUser))
existingIdentity, err := NewIdentity(existingUser, "email", map[string]interface{}{
"sub": existingUser.ID.String(),
"email": "test-disabled@example.com",
})
require.NoError(ts.T(), err)
require.NoError(ts.T(), ts.db.Create(existingIdentity))

ssoProvider := "sso:f06f9e3d-ff92-4c47-a179-7acf1fda6387"

// Ensure DangerousSSOAutoLinking is disabled (default)
ts.config.Security.DangerousSSOAutoLinking = false

// When SSO user logs in with same email AND DangerousSSOAutoLinking is disabled,
// they should get a new account (not linked)
decision, err := DetermineAccountLinking(ts.db, ts.config, []provider.Email{
{
Email: "test-disabled@example.com",
Verified: true,
Primary: true,
},
}, ts.config.JWT.Aud, ssoProvider, "sso-user-subject-disabled")
require.NoError(ts.T(), err)

require.Equal(ts.T(), CreateAccount, decision.Decision, "SSO should create new account when DangerousSSOAutoLinking is disabled")
require.Equal(ts.T(), ssoProvider, decision.LinkingDomain, "SSO should be in its own isolated linking domain")
}
6 changes: 3 additions & 3 deletions internal/models/user.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -777,9 +777,9 @@ func FindUsersInAudience(tx *storage.Connection, aud string, pageParams *Paginat

// IsDuplicatedEmail returns whether a user exists with a matching email and
// audience importantly in the *default* identity linking domain (meaning SSO
// accounts and similar are not considered).
// accounts and similar are not considered unless dangerousSSOAutoLinking is enabled).
// If a currentUser is provided, we will need to filter out any identities that belong to the current user.
func IsDuplicatedEmail(tx *storage.Connection, email, aud string, currentUser *User, ownDomainProviders []string) (*User, error) {
func IsDuplicatedEmail(tx *storage.Connection, email, aud string, currentUser *User, ownDomainProviders []string, dangerousSSOAutoLinking bool) (*User, error) {
var identities []Identity

if err := tx.Eager().Q().Where("email = ?", strings.ToLower(email)).All(&identities); err != nil {
Expand All @@ -793,7 +793,7 @@ func IsDuplicatedEmail(tx *storage.Connection, email, aud string, currentUser *U
userIDs := make(map[string]uuid.UUID)
for _, identity := range identities {
if _, ok := userIDs[identity.UserID.String()]; !ok {
if GetAccountLinkingDomain(identity.Provider, ownDomainProviders) == "default" {
if GetAccountLinkingDomain(identity.Provider, ownDomainProviders, dangerousSSOAutoLinking) == "default" {
userIDs[identity.UserID.String()] = identity.UserID
}
}
Expand Down
12 changes: 6 additions & 6 deletions internal/models/user_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,19 +167,19 @@ func (ts *UserTestSuite) TestFindUserWithRefreshToken() {
func (ts *UserTestSuite) TestIsDuplicatedEmail() {
_ = ts.createUserWithEmail("david.calavera@netlify.com")

e, err := IsDuplicatedEmail(ts.db, "david.calavera@netlify.com", "test", nil, nil)
e, err := IsDuplicatedEmail(ts.db, "david.calavera@netlify.com", "test", nil, nil, false)
require.NoError(ts.T(), err)
require.NotNil(ts.T(), e, "expected email to be duplicated")

e, err = IsDuplicatedEmail(ts.db, "davidcalavera@netlify.com", "test", nil, nil)
e, err = IsDuplicatedEmail(ts.db, "davidcalavera@netlify.com", "test", nil, nil, false)
require.NoError(ts.T(), err)
require.Nil(ts.T(), e, "expected email to not be duplicated", nil, nil)
require.Nil(ts.T(), e, "expected email to not be duplicated")

e, err = IsDuplicatedEmail(ts.db, "david@netlify.com", "test", nil, nil)
e, err = IsDuplicatedEmail(ts.db, "david@netlify.com", "test", nil, nil, false)
require.NoError(ts.T(), err)
require.Nil(ts.T(), e, "expected same email to not be duplicated", nil, nil)
require.Nil(ts.T(), e, "expected same email to not be duplicated")

e, err = IsDuplicatedEmail(ts.db, "david.calavera@netlify.com", "other-aud", nil, nil)
e, err = IsDuplicatedEmail(ts.db, "david.calavera@netlify.com", "other-aud", nil, nil, false)
require.NoError(ts.T(), err)
require.Nil(ts.T(), e, "expected same email to not be duplicated")
}
Expand Down