fix: include all creds in duplicate credential err (#3881)

identity/manager.go

@@ -7,6 +7,7 @@ import (
 	"context"
 	"encoding/json"
 	"reflect"
+	"slices"
 	"sort"
 
 	"go.opentelemetry.io/otel/trace"
@@ -188,6 +189,8 @@ func (m *Manager) findExistingAuthMethod(ctx context.Context, e error, i *Identi
 		return creds[i].Type < creds[j].Type
 	})
 
+	duplicateCredErr := &ErrDuplicateCredentials{error: e}
+
 	for _, cred := range creds {
 		if cred.Config == nil {
 			continue
@@ -202,11 +205,8 @@ func (m *Manager) findExistingAuthMethod(ctx context.Context, e error, i *Identi
 			if len(cred.Identifiers) > 0 {
 				identifierHint = cred.Identifiers[0]
 			}
-			return &ErrDuplicateCredentials{
-				error:                e,
-				availableCredentials: []CredentialsType{cred.Type},
-				identifierHint:       identifierHint,
-			}
+			duplicateCredErr.AddCredentialsType(cred.Type)
+			duplicateCredErr.SetIdentifierHint(identifierHint)
 		case CredentialsTypeOIDC:
 			var cfg CredentialsOIDC
 			if err := json.Unmarshal(cred.Config, &cfg); err != nil {
@@ -218,12 +218,9 @@ func (m *Manager) findExistingAuthMethod(ctx context.Context, e error, i *Identi
 				available = append(available, provider.Provider)
 			}
 
-			return &ErrDuplicateCredentials{
-				error:                  e,
-				availableCredentials:   []CredentialsType{cred.Type},
-				availableOIDCProviders: available,
-				identifierHint:         foundConflictAddress,
-			}
+			duplicateCredErr.AddCredentialsType(cred.Type)
+			duplicateCredErr.SetIdentifierHint(foundConflictAddress)
+			duplicateCredErr.availableOIDCProviders = available
 		case CredentialsTypeWebAuthn:
 			var cfg CredentialsWebAuthnConfig
 			if err := json.Unmarshal(cred.Config, &cfg); err != nil {
@@ -237,18 +234,15 @@ func (m *Manager) findExistingAuthMethod(ctx context.Context, e error, i *Identi
 
 			for _, webauthn := range cfg.Credentials {
 				if webauthn.IsPasswordless {
-					return &ErrDuplicateCredentials{
-						error:                e,
-						availableCredentials: []CredentialsType{cred.Type},
-						identifierHint:       identifierHint,
-					}
+					duplicateCredErr.AddCredentialsType(cred.Type)
+					duplicateCredErr.SetIdentifierHint(identifierHint)
+					break
 				}
 			}
 		}
 	}
 
-	// Still not found? Return generic error.
-	return &ErrDuplicateCredentials{error: e}
+	return duplicateCredErr
 }
 
 type ErrDuplicateCredentials struct {
@@ -265,15 +259,28 @@ func (e *ErrDuplicateCredentials) Unwrap() error {
 	return e.error
 }
 
+func (e *ErrDuplicateCredentials) AddCredentialsType(ct CredentialsType) {
+	e.availableCredentials = append(e.availableCredentials, ct)
+}
+
+func (e *ErrDuplicateCredentials) SetIdentifierHint(hint string) {
+	if hint != "" {
+		e.identifierHint = hint
+	}
+}
+
 func (e *ErrDuplicateCredentials) AvailableCredentials() []string {
 	res := make([]string, len(e.availableCredentials))
 	for k, v := range e.availableCredentials {
 		res[k] = string(v)
 	}
+	slices.Sort(res)
+
 	return res
 }
 
 func (e *ErrDuplicateCredentials) AvailableOIDCProviders() []string {
+	slices.Sort(e.availableOIDCProviders)
 	return e.availableOIDCProviders
 }
 

identity/manager_test.go

@@ -222,6 +222,41 @@ func TestManager(t *testing.T) {
 					assert.Len(t, verr.AvailableOIDCProviders(), 0)
 					assert.Equal(t, verr.IdentifierHint(), email)
 				})
+
+				t.Run("type=password+oidc+webauthn", func(t *testing.T) {
+					email := uuid.Must(uuid.NewV4()).String() + "@ory.sh"
+					creds := map[identity.CredentialsType]identity.Credentials{
+						identity.CredentialsTypePassword: {
+							Type:        identity.CredentialsTypePassword,
+							Identifiers: []string{email},
+							Config:      sqlxx.JSONRawMessage(`{"hashed_password":"$2a$08$.cOYmAd.vCpDOoiVJrO5B.hjTLKQQ6cAK40u8uB.FnZDyPvVvQ9Q."}`),
+						},
+						identity.CredentialsTypeOIDC: {
+							Type: identity.CredentialsTypeOIDC,
+							// Identifiers in OIDC are not email addresses, but a unique user ID.
+							Identifiers: []string{"google:" + uuid.Must(uuid.NewV4()).String()},
+							Config:      sqlxx.JSONRawMessage(`{"providers":[{"provider": "google"},{"provider": "github"}]}`),
+						},
+						identity.CredentialsTypeWebAuthn: {
+							Type:        identity.CredentialsTypeWebAuthn,
+							Identifiers: []string{email},
+							Config:      sqlxx.JSONRawMessage(`{"credentials": [{"is_passwordless":true}]}`),
+						},
+					}
+
+					first := createIdentity(email, "email_creds", creds)
+					require.NoError(t, reg.IdentityManager().Create(context.Background(), first))
+
+					second := createIdentity(email, "email_creds", creds)
+					err := reg.IdentityManager().Create(context.Background(), second)
+					require.Error(t, err)
+
+					var verr = new(identity.ErrDuplicateCredentials)
+					assert.ErrorAs(t, err, &verr)
+					assert.ElementsMatch(t, []string{"password", "oidc", "webauthn"}, verr.AvailableCredentials())
+					assert.ElementsMatch(t, []string{"google", "github"}, verr.AvailableOIDCProviders())
+					assert.Equal(t, email, verr.IdentifierHint())
+				})
 			})
 
 			runAddress := func(t *testing.T, field string) {
@@ -278,7 +313,7 @@ func TestManager(t *testing.T) {
 					var verr = new(identity.ErrDuplicateCredentials)
 					assert.ErrorAs(t, err, &verr)
 					assert.EqualValues(t, []string{identity.CredentialsTypeOIDC.String()}, verr.AvailableCredentials())
-					assert.EqualValues(t, verr.AvailableOIDCProviders(), []string{"google", "github"})
+					assert.EqualValues(t, verr.AvailableOIDCProviders(), []string{"github", "google"})
 					assert.Equal(t, verr.IdentifierHint(), email)
 				})
 			}

internal/client-go/go.sum

@@ -4,6 +4,7 @@ github.com/golang/protobuf v1.2.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5y
 golang.org/x/net v0.0.0-20180724234803-3673e40ba225/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
 golang.org/x/net v0.0.0-20190108225652-1e06a53dbb7e h1:bRhVy7zSSasaqNksaRZiA5EEI+Ei4I1nO5Jh72wfHlg=
 golang.org/x/net v0.0.0-20190108225652-1e06a53dbb7e/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
+golang.org/x/oauth2 v0.0.0-20190604053449-0f29369cfe45/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw=
 golang.org/x/sync v0.0.0-20181221193216-37e7f081c4d4 h1:YUO/7uOKsKeq9UokNS62b8FYywz3ker1l1vDZRCRefw=
 golang.org/x/sync v0.0.0-20181221193216-37e7f081c4d4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
 golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=

selfservice/strategy/oidc/strategy_test.go

@@ -1260,7 +1260,7 @@ func TestStrategy(t *testing.T) {
 			var linkingLoginFlow struct{ ID string }
 			t.Run("step=should fail login and start a new login", func(t *testing.T) {
 				res, body := loginWithOIDC(t, client, loginFlow.ID, "valid")
-				assertUIError(t, res, body, "You tried signing in with existing-oidc-identity-1@ory.sh which is already in use by another account. You can sign in using social sign in. You can sign in using one of the following social sign in providers: Secondprovider.")
+				assertUIError(t, res, body, "You tried signing in with existing-oidc-identity-1@ory.sh which is already in use by another account. You can sign in using social sign in, or your password. You can sign in using one of the following social sign in providers: Secondprovider.")
 				linkingLoginFlow.ID = gjson.GetBytes(body, "id").String()
 				assert.NotEqual(t, loginFlow.ID.String(), linkingLoginFlow.ID, "should have started a new flow")
 			})

selfservice/strategy/webauthn/registration_test.go

@@ -438,7 +438,7 @@ func TestRegistration(t *testing.T) {
 					actual, _, _ = makeRegistration(t, f, values(email))
 					assert.Contains(t, gjson.Get(actual, "ui.action").String(), publicTS.URL+registration.RouteSubmitFlow, "%s", actual)
 					registrationhelpers.CheckFormContent(t, []byte(actual), node.WebAuthnRegisterTrigger, "csrf_token", "traits.username")
-					assert.Equal(t, "You tried signing in with "+email+" which is already in use by another account. You can sign in using your password.", gjson.Get(actual, "ui.messages.0.text").String(), "%s", actual)
+					assert.Equal(t, "You tried signing in with "+email+" which is already in use by another account. You can sign in using your password, or your passkey or a security key.", gjson.Get(actual, "ui.messages.0.text").String(), "%s", actual)
 				})
 			}
 		})

session/handler_test.go

@@ -184,18 +184,13 @@ func TestSessionWhoAmI(t *testing.T) {
 					assert.NotEmpty(t, res.Header.Get("X-Kratos-Authenticated-Identity-Id"))
 
 					if cacheEnabled {
+						var expectedSeconds int
 						if maxAge > 0 {
-							assert.Equal(t, fmt.Sprintf("%0.f", maxAge.Seconds()), res.Header.Get("Ory-Session-Cache-For"))
+							expectedSeconds = int(maxAge.Seconds())
 						} else {
-							// parse int to string from Ory-Session-Cache-For
-							parsed, err := strconv.Atoi(res.Header.Get("Ory-Session-Cache-For"))
-							require.NoError(t, err)
-							lifespan := conf.SessionLifespan(ctx).Seconds()
-							// We need to account for the time it takes to make the request, as depending on the system it might take a few more ms which leads to the value being off by a second or more.
-							assert.Condition(t, func() bool {
-								return parsed > int(lifespan-5) && parsed <= int(lifespan)
-							}, "Expected the value of the Ory-Session-Cache-For header to be roughly around the configured lifespan. Got parsed: %d, lifespan: %d", parsed, int(lifespan))
+							expectedSeconds = int(conf.SessionLifespan(ctx).Seconds())
 						}
+						assert.InDelta(t, expectedSeconds, x.Must(strconv.Atoi(res.Header.Get("Ory-Session-Cache-For"))), 5)
 					} else {
 						assert.Empty(t, res.Header.Get("Ory-Session-Cache-For"))
 					}

text/message_validation.go

@@ -9,8 +9,6 @@ import (
 
 	"golang.org/x/text/cases"
 	"golang.org/x/text/language"
-
-	"golang.org/x/exp/maps"
 )
 
 func NewValidationErrorGeneric(reason string) *Message {
@@ -279,25 +277,30 @@ func NewErrorValidationDuplicateCredentialsWithHints(availableCredentialTypes []
 
 	reason := fmt.Sprintf("You tried signing in with %s which is already in use by another account.", identifier)
 	if len(availableCredentialTypes) > 0 {
-		humanReadable := make(map[string]struct{}, len(availableCredentialTypes))
+		humanReadable := make([]string, 0, len(availableCredentialTypes))
 		for _, cred := range availableCredentialTypes {
 			switch cred {
 			case "password":
-				humanReadable["your password"] = struct{}{}
+				humanReadable = append(humanReadable, "your password")
 			case "oidc":
-				humanReadable["social sign in"] = struct{}{}
+				humanReadable = append(humanReadable, "social sign in")
 			case "webauthn":
-				humanReadable["your PassKey or a security key"] = struct{}{}
+				humanReadable = append(humanReadable, "your passkey or a security key")
 			}
 		}
 		if len(humanReadable) == 0 {
 			// show at least some hint
 			// also our example message generation tool runs into this case
-			for _, cred := range availableCredentialTypes {
-				humanReadable[cred] = struct{}{}
-			}
+			humanReadable = append(humanReadable, availableCredentialTypes...)
+		}
+
+		// Final format: "You can sign in using foo, bar, or baz."
+		if len(humanReadable) > 1 {
+			humanReadable[len(humanReadable)-1] = "or " + humanReadable[len(humanReadable)-1]
+		}
+		if len(humanReadable) > 0 {
+			reason += fmt.Sprintf(" You can sign in using %s.", strings.Join(humanReadable, ", "))
 		}
-		reason += fmt.Sprintf(" You can sign in using %s.", strings.Join(maps.Keys(humanReadable), ", "))
 	}
 	if len(oidcProviders) > 0 {
 		reason += fmt.Sprintf(" You can sign in using one of the following social sign in providers: %s.", strings.Join(oidcProviders, ", "))