diff --git a/cmd/admin_cmd.go b/cmd/admin_cmd.go index ba4f088b1..aac4a8a33 100644 --- a/cmd/admin_cmd.go +++ b/cmd/admin_cmd.go @@ -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) diff --git a/internal/api/admin.go b/internal/api/admin.go index ad50b81e5..b8f970c07 100644 --- a/internal/api/admin.go +++ b/internal/api/admin.go @@ -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) diff --git a/internal/api/mail.go b/internal/api/mail.go index dc7b8a58d..46959dd67 100644 --- a/internal/api/mail.go +++ b/internal/api/mail.go @@ -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) diff --git a/internal/api/signup.go b/internal/api/signup.go index fcc3877ed..4ebcb56fd 100644 --- a/internal/api/signup.go +++ b/internal/api/signup.go @@ -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") diff --git a/internal/api/user.go b/internal/api/user.go index 723bce144..35aaf0396 100644 --- a/internal/api/user.go +++ b/internal/api/user.go @@ -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) diff --git a/internal/conf/configuration.go b/internal/conf/configuration.go index 3e397be69..ed7c6c64a 100644 --- a/internal/conf/configuration.go +++ b/internal/conf/configuration.go @@ -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"` } diff --git a/internal/models/linking.go b/internal/models/linking.go index c058c0baa..4b4ef9353 100644 --- a/internal/models/linking.go +++ b/internal/models/linking.go @@ -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" + } + // 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 } @@ -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 @@ -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 } @@ -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) } } diff --git a/internal/models/linking_test.go b/internal/models/linking_test.go index 05d4a8c32..49f5c34b0 100644 --- a/internal/models/linking_test.go +++ b/internal/models/linking_test.go @@ -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") +} diff --git a/internal/models/user.go b/internal/models/user.go index 3c706b80e..bd47f49fe 100644 --- a/internal/models/user.go +++ b/internal/models/user.go @@ -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 { @@ -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 } } diff --git a/internal/models/user_test.go b/internal/models/user_test.go index 28e9f9cc6..25df39de1 100644 --- a/internal/models/user_test.go +++ b/internal/models/user_test.go @@ -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") }

AltStyle によって変換されたページ (->オリジナル) /