diff --git a/cmd/clidoc/main.go b/cmd/clidoc/main.go index 834373d52e6d..6010768ecd1f 100644 --- a/cmd/clidoc/main.go +++ b/cmd/clidoc/main.go @@ -125,7 +125,7 @@ func init() { "NewInfoLoginTOTP": text.NewInfoLoginTOTP(), "NewInfoLoginLookup": text.NewInfoLoginLookup(), "NewInfoLoginVerify": text.NewInfoLoginVerify(), - "NewInfoLoginWith": text.NewInfoLoginWith("{provider}"), + "NewInfoLoginWith": text.NewInfoLoginWith("{provider}", "{providerID}"), "NewInfoLoginWithAndLink": text.NewInfoLoginWithAndLink("{provider}"), "NewErrorValidationLoginFlowExpired": text.NewErrorValidationLoginFlowExpired(aSecondAgo), "NewErrorValidationLoginNoStrategyFound": text.NewErrorValidationLoginNoStrategyFound(), @@ -135,7 +135,7 @@ func init() { "NewErrorValidationVerificationNoStrategyFound": text.NewErrorValidationVerificationNoStrategyFound(), "NewInfoSelfServiceLoginWebAuthn": text.NewInfoSelfServiceLoginWebAuthn(), "NewInfoRegistration": text.NewInfoRegistration(), - "NewInfoRegistrationWith": text.NewInfoRegistrationWith("{provider}"), + "NewInfoRegistrationWith": text.NewInfoRegistrationWith("{provider}", "{providerID}"), "NewInfoRegistrationContinue": text.NewInfoRegistrationContinue(), "NewInfoRegistrationBack": text.NewInfoRegistrationBack(), "NewInfoSelfServiceChooseCredentials": text.NewInfoSelfServiceChooseCredentials(), diff --git a/identity/manager.go b/identity/manager.go index 2429df260fbe..d05fd83a4a7f 100644 --- a/identity/manager.go +++ b/identity/manager.go @@ -214,6 +214,15 @@ func (m *Manager) findExistingAuthMethod(ctx context.Context, e error, i *Identi } duplicateCredErr.AddCredentialsType(cred.Type) + + case CredentialsTypeCodeAuth: + identifierHint := foundConflictAddress + if len(cred.Identifiers) > 0 { + identifierHint = cred.Identifiers[0] + } + + duplicateCredErr.SetIdentifierHint(identifierHint) + duplicateCredErr.AddCredentialsType(cred.Type) case CredentialsTypeOIDC: var cfg CredentialsOIDC if err := json.Unmarshal(cred.Config, &cfg); err != nil { @@ -312,6 +321,7 @@ func (e *ErrDuplicateCredentials) AvailableOIDCProviders() []string { func (e *ErrDuplicateCredentials) IdentifierHint() string { return e.identifierHint } + func (e *ErrDuplicateCredentials) HasHints() bool { return len(e.availableCredentials) > 0 || len(e.availableOIDCProviders) > 0 || len(e.identifierHint) > 0 } diff --git a/identity/manager_test.go b/identity/manager_test.go index dcd2b2762c8f..bd9e67782ea9 100644 --- a/identity/manager_test.go +++ b/identity/manager_test.go @@ -229,7 +229,7 @@ func TestManager(t *testing.T) { err := reg.IdentityManager().Create(ctx, second) require.Error(t, err) - var verr = new(identity.ErrDuplicateCredentials) + verr := new(identity.ErrDuplicateCredentials) assert.ErrorAs(t, err, &verr) assert.EqualValues(t, []string{identity.CredentialsTypePassword.String()}, verr.AvailableCredentials()) assert.Len(t, verr.AvailableOIDCProviders(), 0) @@ -253,7 +253,7 @@ func TestManager(t *testing.T) { err := reg.IdentityManager().Create(ctx, second) require.Error(t, err) - var verr = new(identity.ErrDuplicateCredentials) + verr := new(identity.ErrDuplicateCredentials) assert.ErrorAs(t, err, &verr) assert.EqualValues(t, []string{identity.CredentialsTypeWebAuthn.String()}, verr.AvailableCredentials()) assert.Len(t, verr.AvailableOIDCProviders(), 0) @@ -278,7 +278,7 @@ func TestManager(t *testing.T) { err := reg.IdentityManager().Create(ctx, second) require.Error(t, err) - var verr = new(identity.ErrDuplicateCredentials) + verr := new(identity.ErrDuplicateCredentials) assert.ErrorAs(t, err, &verr) assert.ElementsMatch(t, []string{"oidc"}, verr.AvailableCredentials()) assert.ElementsMatch(t, []string{"google", "github"}, verr.AvailableOIDCProviders()) @@ -313,12 +313,36 @@ func TestManager(t *testing.T) { err := reg.IdentityManager().Create(ctx, second) require.Error(t, err) - var verr = new(identity.ErrDuplicateCredentials) + 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()) }) + + t.Run("type=code", func(t *testing.T) { + email := uuid.Must(uuid.NewV4()).String() + "@ory.sh" + creds := map[identity.CredentialsType]identity.Credentials{ + identity.CredentialsTypeCodeAuth: { + Type: identity.CredentialsTypeCodeAuth, + Identifiers: []string{email}, + Config: sqlxx.JSONRawMessage(`{}`), + }, + } + + first := createIdentity(email, "email_creds", creds) + require.NoError(t, reg.IdentityManager().Create(ctx, first)) + + second := createIdentity(email, "email_creds", creds) + err := reg.IdentityManager().Create(ctx, second) + require.Error(t, err) + + verr := new(identity.ErrDuplicateCredentials) + assert.ErrorAs(t, err, &verr) + assert.EqualValues(t, []string{identity.CredentialsTypeCodeAuth.String()}, verr.AvailableCredentials()) + assert.Len(t, verr.AvailableOIDCProviders(), 0) + assert.Equal(t, verr.IdentifierHint(), email) + }) }) runAddress := func(t *testing.T, field string) { @@ -343,7 +367,7 @@ func TestManager(t *testing.T) { err := reg.IdentityManager().Create(ctx, second) require.Error(t, err) - var verr = new(identity.ErrDuplicateCredentials) + verr := new(identity.ErrDuplicateCredentials) assert.ErrorAs(t, err, &verr) assert.EqualValues(t, []string{identity.CredentialsTypePassword.String()}, verr.AvailableCredentials()) assert.Len(t, verr.AvailableOIDCProviders(), 0) @@ -372,7 +396,7 @@ func TestManager(t *testing.T) { err := reg.IdentityManager().Create(ctx, second) require.Error(t, err) - var verr = new(identity.ErrDuplicateCredentials) + verr := new(identity.ErrDuplicateCredentials) assert.ErrorAs(t, err, &verr) assert.EqualValues(t, []string{identity.CredentialsTypeOIDC.String()}, verr.AvailableCredentials()) assert.EqualValues(t, verr.AvailableOIDCProviders(), []string{"github", "google"}) diff --git a/internal/testhelpers/sdk.go b/internal/testhelpers/sdk.go index 849f7a73304e..1e55cda04665 100644 --- a/internal/testhelpers/sdk.go +++ b/internal/testhelpers/sdk.go @@ -9,7 +9,6 @@ import ( "net/http/httptest" "net/url" - "github.com/ory/kratos/text" "github.com/ory/kratos/ui/node" "github.com/ory/kratos/x" @@ -88,26 +87,6 @@ func NewSDKEmailNode(group string) *kratos.UiNode { } } -func NewSDKOIDCNode(name, provider string) *kratos.UiNode { - t := text.NewInfoRegistrationWith(provider) - return &kratos.UiNode{ - Group: node.OpenIDConnectGroup.String(), - Type: "input", - Attributes: kratos.UiNodeInputAttributesAsUiNodeAttributes(&kratos.UiNodeInputAttributes{ - Name: name, - Type: "submit", - Value: provider, - }), - Meta: kratos.UiNodeMeta{ - Label: &kratos.UiText{ - Id: int64(t.ID), - Text: t.Text, - Type: string(t.Type), - }, - }, - } -} - func NewMethodSubmit(group, value string) *kratos.UiNode { return &kratos.UiNode{ Type: "input", diff --git a/selfservice/strategy/idfirst/strategy_login_test.go b/selfservice/strategy/idfirst/strategy_login_test.go index 4b69a2b44fd4..d8c31bdeb786 100644 --- a/selfservice/strategy/idfirst/strategy_login_test.go +++ b/selfservice/strategy/idfirst/strategy_login_test.go @@ -172,7 +172,7 @@ func TestCompleteLogin(t *testing.T) { }) t.Run("case=should return an error because the request is expired", func(t *testing.T) { - conf.MustSet(ctx, config.ViperKeySelfServiceLoginRequestLifespan, time.Millisecond*10) + conf.MustSet(ctx, config.ViperKeySelfServiceLoginRequestLifespan, time.Millisecond*30) conf.MustSet(ctx, config.ViperKeySecurityAccountEnumerationMitigate, true) t.Cleanup(func() { conf.MustSet(ctx, config.ViperKeySelfServiceLoginRequestLifespan, time.Hour) diff --git a/selfservice/strategy/oidc/.snapshots/TestFormHydration-method=PopulateLoginMethodFirstFactor.json b/selfservice/strategy/oidc/.snapshots/TestFormHydration-method=PopulateLoginMethodFirstFactor.json index 9ce35531c24a..2939b127e281 100644 --- a/selfservice/strategy/oidc/.snapshots/TestFormHydration-method=PopulateLoginMethodFirstFactor.json +++ b/selfservice/strategy/oidc/.snapshots/TestFormHydration-method=PopulateLoginMethodFirstFactor.json @@ -29,7 +29,8 @@ "text": "Sign in with test-provider", "type": "info", "context": { - "provider": "test-provider" + "provider": "test-provider", + "provider_id": "test-provider" } } } diff --git a/selfservice/strategy/oidc/.snapshots/TestFormHydration-method=PopulateLoginMethodFirstFactorRefresh.json b/selfservice/strategy/oidc/.snapshots/TestFormHydration-method=PopulateLoginMethodFirstFactorRefresh.json index 9ce35531c24a..2939b127e281 100644 --- a/selfservice/strategy/oidc/.snapshots/TestFormHydration-method=PopulateLoginMethodFirstFactorRefresh.json +++ b/selfservice/strategy/oidc/.snapshots/TestFormHydration-method=PopulateLoginMethodFirstFactorRefresh.json @@ -29,7 +29,8 @@ "text": "Sign in with test-provider", "type": "info", "context": { - "provider": "test-provider" + "provider": "test-provider", + "provider_id": "test-provider" } } } diff --git a/selfservice/strategy/oidc/.snapshots/TestFormHydration-method=PopulateLoginMethodIdentifierFirstCredentials-case=WithIdentityHint-case=account_enumeration_mitigation_disabled-case=identity_has_oidc.json b/selfservice/strategy/oidc/.snapshots/TestFormHydration-method=PopulateLoginMethodIdentifierFirstCredentials-case=WithIdentityHint-case=account_enumeration_mitigation_disabled-case=identity_has_oidc.json index 29f5e9aa1061..a2c08bd8d1b9 100644 --- a/selfservice/strategy/oidc/.snapshots/TestFormHydration-method=PopulateLoginMethodIdentifierFirstCredentials-case=WithIdentityHint-case=account_enumeration_mitigation_disabled-case=identity_has_oidc.json +++ b/selfservice/strategy/oidc/.snapshots/TestFormHydration-method=PopulateLoginMethodIdentifierFirstCredentials-case=WithIdentityHint-case=account_enumeration_mitigation_disabled-case=identity_has_oidc.json @@ -16,7 +16,8 @@ "text": "Sign in with test-provider", "type": "info", "context": { - "provider": "test-provider" + "provider": "test-provider", + "provider_id": "test-provider" } } } diff --git a/selfservice/strategy/oidc/.snapshots/TestFormHydration-method=PopulateLoginMethodIdentifierFirstIdentification.json b/selfservice/strategy/oidc/.snapshots/TestFormHydration-method=PopulateLoginMethodIdentifierFirstIdentification.json index 9ce35531c24a..2939b127e281 100644 --- a/selfservice/strategy/oidc/.snapshots/TestFormHydration-method=PopulateLoginMethodIdentifierFirstIdentification.json +++ b/selfservice/strategy/oidc/.snapshots/TestFormHydration-method=PopulateLoginMethodIdentifierFirstIdentification.json @@ -29,7 +29,8 @@ "text": "Sign in with test-provider", "type": "info", "context": { - "provider": "test-provider" + "provider": "test-provider", + "provider_id": "test-provider" } } } diff --git a/selfservice/strategy/oidc/.snapshots/TestStrategy-case=should_fail_to_register_and_return_fresh_login_flow_if_email_is_already_being_used_by_password_credentials-case=should_fail_login.json b/selfservice/strategy/oidc/.snapshots/TestStrategy-case=should_fail_to_register_and_return_fresh_login_flow_if_email_is_already_being_used_by_password_credentials-case=should_fail_login.json new file mode 100644 index 000000000000..c87e844c54e3 --- /dev/null +++ b/selfservice/strategy/oidc/.snapshots/TestStrategy-case=should_fail_to_register_and_return_fresh_login_flow_if_email_is_already_being_used_by_password_credentials-case=should_fail_login.json @@ -0,0 +1,185 @@ +{ + "organization_id": null, + "type": "browser", + "active": "oidc", + "ui": { + "method": "POST", + "nodes": [ + { + "type": "input", + "group": "oidc", + "attributes": { + "name": "provider", + "type": "submit", + "value": "claimsViaUserInfo", + "disabled": false, + "node_type": "input" + }, + "messages": [], + "meta": { + "label": { + "id": 1010018, + "text": "Confirm with claimsViaUserInfo", + "type": "info", + "context": { + "provider": "claimsViaUserInfo" + } + } + } + }, + { + "type": "input", + "group": "oidc", + "attributes": { + "name": "provider", + "type": "submit", + "value": "invalid-issuer", + "disabled": false, + "node_type": "input" + }, + "messages": [], + "meta": { + "label": { + "id": 1010018, + "text": "Confirm with invalid-issuer", + "type": "info", + "context": { + "provider": "invalid-issuer" + } + } + } + }, + { + "type": "input", + "group": "oidc", + "attributes": { + "name": "provider", + "type": "submit", + "value": "secondProvider", + "disabled": false, + "node_type": "input" + }, + "messages": [], + "meta": { + "label": { + "id": 1010018, + "text": "Confirm with secondProvider", + "type": "info", + "context": { + "provider": "secondProvider" + } + } + } + }, + { + "type": "input", + "group": "oidc", + "attributes": { + "name": "provider", + "type": "submit", + "value": "valid2", + "disabled": false, + "node_type": "input" + }, + "messages": [], + "meta": { + "label": { + "id": 1010018, + "text": "Confirm with valid2", + "type": "info", + "context": { + "provider": "valid2" + } + } + } + }, + { + "type": "input", + "group": "default", + "attributes": { + "name": "csrf_token", + "type": "hidden", + "required": true, + "disabled": false, + "node_type": "input" + }, + "messages": [], + "meta": {} + }, + { + "type": "input", + "group": "default", + "attributes": { + "name": "identifier", + "type": "hidden", + "value": "email-exist-with-password-strategy@ory.sh", + "required": true, + "disabled": false, + "node_type": "input" + }, + "messages": [], + "meta": { + "label": { + "id": 1070004, + "text": "ID", + "type": "info" + } + } + }, + { + "type": "input", + "group": "password", + "attributes": { + "name": "password", + "type": "password", + "required": true, + "autocomplete": "current-password", + "disabled": false, + "node_type": "input" + }, + "messages": [], + "meta": { + "label": { + "id": 1070001, + "text": "Password", + "type": "info" + } + } + }, + { + "type": "input", + "group": "password", + "attributes": { + "name": "method", + "type": "submit", + "value": "password", + "disabled": false, + "node_type": "input" + }, + "messages": [], + "meta": { + "label": { + "id": 1010022, + "text": "Sign in with password", + "type": "info" + } + } + } + ], + "messages": [ + { + "id": 1010016, + "text": "You tried to sign in with \"email-exist-with-password-strategy@ory.sh\", but that email is already used by another account. Sign in to your account with one of the options below to add your account \"email-exist-with-password-strategy@ory.sh\" at \"generic\" as another way to sign in.", + "type": "info", + "context": { + "duplicateIdentifier": "email-exist-with-password-strategy@ory.sh", + "provider": "generic" + } + } + ] + }, + "refresh": false, + "requested_aal": "aal1", + "state": "choose_method" +} + diff --git a/selfservice/strategy/oidc/.snapshots/TestStrategy-case=should_fail_to_register_and_return_fresh_login_flow_if_email_is_already_being_used_by_password_credentials-case=should_fail_registration.json b/selfservice/strategy/oidc/.snapshots/TestStrategy-case=should_fail_to_register_and_return_fresh_login_flow_if_email_is_already_being_used_by_password_credentials-case=should_fail_registration.json new file mode 100644 index 000000000000..c87e844c54e3 --- /dev/null +++ b/selfservice/strategy/oidc/.snapshots/TestStrategy-case=should_fail_to_register_and_return_fresh_login_flow_if_email_is_already_being_used_by_password_credentials-case=should_fail_registration.json @@ -0,0 +1,185 @@ +{ + "organization_id": null, + "type": "browser", + "active": "oidc", + "ui": { + "method": "POST", + "nodes": [ + { + "type": "input", + "group": "oidc", + "attributes": { + "name": "provider", + "type": "submit", + "value": "claimsViaUserInfo", + "disabled": false, + "node_type": "input" + }, + "messages": [], + "meta": { + "label": { + "id": 1010018, + "text": "Confirm with claimsViaUserInfo", + "type": "info", + "context": { + "provider": "claimsViaUserInfo" + } + } + } + }, + { + "type": "input", + "group": "oidc", + "attributes": { + "name": "provider", + "type": "submit", + "value": "invalid-issuer", + "disabled": false, + "node_type": "input" + }, + "messages": [], + "meta": { + "label": { + "id": 1010018, + "text": "Confirm with invalid-issuer", + "type": "info", + "context": { + "provider": "invalid-issuer" + } + } + } + }, + { + "type": "input", + "group": "oidc", + "attributes": { + "name": "provider", + "type": "submit", + "value": "secondProvider", + "disabled": false, + "node_type": "input" + }, + "messages": [], + "meta": { + "label": { + "id": 1010018, + "text": "Confirm with secondProvider", + "type": "info", + "context": { + "provider": "secondProvider" + } + } + } + }, + { + "type": "input", + "group": "oidc", + "attributes": { + "name": "provider", + "type": "submit", + "value": "valid2", + "disabled": false, + "node_type": "input" + }, + "messages": [], + "meta": { + "label": { + "id": 1010018, + "text": "Confirm with valid2", + "type": "info", + "context": { + "provider": "valid2" + } + } + } + }, + { + "type": "input", + "group": "default", + "attributes": { + "name": "csrf_token", + "type": "hidden", + "required": true, + "disabled": false, + "node_type": "input" + }, + "messages": [], + "meta": {} + }, + { + "type": "input", + "group": "default", + "attributes": { + "name": "identifier", + "type": "hidden", + "value": "email-exist-with-password-strategy@ory.sh", + "required": true, + "disabled": false, + "node_type": "input" + }, + "messages": [], + "meta": { + "label": { + "id": 1070004, + "text": "ID", + "type": "info" + } + } + }, + { + "type": "input", + "group": "password", + "attributes": { + "name": "password", + "type": "password", + "required": true, + "autocomplete": "current-password", + "disabled": false, + "node_type": "input" + }, + "messages": [], + "meta": { + "label": { + "id": 1070001, + "text": "Password", + "type": "info" + } + } + }, + { + "type": "input", + "group": "password", + "attributes": { + "name": "method", + "type": "submit", + "value": "password", + "disabled": false, + "node_type": "input" + }, + "messages": [], + "meta": { + "label": { + "id": 1010022, + "text": "Sign in with password", + "type": "info" + } + } + } + ], + "messages": [ + { + "id": 1010016, + "text": "You tried to sign in with \"email-exist-with-password-strategy@ory.sh\", but that email is already used by another account. Sign in to your account with one of the options below to add your account \"email-exist-with-password-strategy@ory.sh\" at \"generic\" as another way to sign in.", + "type": "info", + "context": { + "duplicateIdentifier": "email-exist-with-password-strategy@ory.sh", + "provider": "generic" + } + } + ] + }, + "refresh": false, + "requested_aal": "aal1", + "state": "choose_method" +} + diff --git a/selfservice/strategy/oidc/.snapshots/TestStrategy-method=TestPopulateSignUpMethod.json b/selfservice/strategy/oidc/.snapshots/TestStrategy-method=TestPopulateSignUpMethod.json index 6b0a9ba98cf8..4177f37350e2 100644 --- a/selfservice/strategy/oidc/.snapshots/TestStrategy-method=TestPopulateSignUpMethod.json +++ b/selfservice/strategy/oidc/.snapshots/TestStrategy-method=TestPopulateSignUpMethod.json @@ -31,7 +31,31 @@ "text": "Sign up with valid", "type": "info", "context": { - "provider": "valid" + "provider": "valid", + "provider_id": "valid" + } + } + } + }, + { + "type": "input", + "group": "oidc", + "attributes": { + "name": "provider", + "type": "submit", + "value": "valid2", + "disabled": false, + "node_type": "input" + }, + "messages": [], + "meta": { + "label": { + "id": 1040002, + "text": "Sign up with valid2", + "type": "info", + "context": { + "provider": "valid2", + "provider_id": "valid2" } } } @@ -53,7 +77,8 @@ "text": "Sign up with secondProvider", "type": "info", "context": { - "provider": "secondProvider" + "provider": "secondProvider", + "provider_id": "secondProvider" } } } @@ -75,7 +100,8 @@ "text": "Sign up with claimsViaUserInfo", "type": "info", "context": { - "provider": "claimsViaUserInfo" + "provider": "claimsViaUserInfo", + "provider_id": "claimsViaUserInfo" } } } @@ -97,7 +123,8 @@ "text": "Sign up with invalid-issuer", "type": "info", "context": { - "provider": "invalid-issuer" + "provider": "invalid-issuer", + "provider_id": "invalid-issuer" } } } diff --git a/selfservice/strategy/oidc/strategy.go b/selfservice/strategy/oidc/strategy.go index e6733f0f0019..84e84a6ccf8e 100644 --- a/selfservice/strategy/oidc/strategy.go +++ b/selfservice/strategy/oidc/strategy.go @@ -416,7 +416,7 @@ func (s *Strategy) HandleCallback(w http.ResponseWriter, r *http.Request, ps htt return } - provider, err := s.provider(r.Context(), r, pid) + provider, err := s.provider(r.Context(), pid) if err != nil { s.forwardError(w, r, req, s.handleError(ctx, w, r, req, pid, nil, err)) return @@ -535,7 +535,7 @@ func (s *Strategy) ExchangeCode(ctx context.Context, provider Provider, code str } } -func (s *Strategy) populateMethod(r *http.Request, f flow.Flow, message func(provider string) *text.Message) error { +func (s *Strategy) populateMethod(r *http.Request, f flow.Flow, message func(provider string, providerId string) *text.Message) error { conf, err := s.Config(r.Context()) if err != nil { return err @@ -561,7 +561,7 @@ func (s *Strategy) Config(ctx context.Context) (*ConfigurationCollection, error) return &c, nil } -func (s *Strategy) provider(ctx context.Context, r *http.Request, id string) (Provider, error) { +func (s *Strategy) provider(ctx context.Context, id string) (Provider, error) { if c, err := s.Config(ctx); err != nil { return nil, err } else if provider, err := c.Provider(id, s.d); err != nil { @@ -588,7 +588,7 @@ func (s *Strategy) forwardError(w http.ResponseWriter, r *http.Request, f flow.F } } -func (s *Strategy) handleError(ctx context.Context, w http.ResponseWriter, r *http.Request, f flow.Flow, providerID string, traits []byte, err error) error { +func (s *Strategy) handleError(ctx context.Context, w http.ResponseWriter, r *http.Request, f flow.Flow, usedProviderID string, traits []byte, err error) error { switch rf := f.(type) { case *login.Flow: return err @@ -608,7 +608,7 @@ func (s *Strategy) handleError(ctx context.Context, w http.ResponseWriter, r *ht rf.UI.Messages.Add(text.NewErrorValidationDuplicateCredentialsOnOIDCLink()) } - lf, err := s.registrationToLogin(w, r, rf, providerID) + lf, err := s.registrationToLogin(w, r, rf, usedProviderID) if err != nil { return err } @@ -628,33 +628,8 @@ func (s *Strategy) handleError(ctx context.Context, w http.ResponseWriter, r *ht } if dc, err := flow.DuplicateCredentials(lf); err == nil && dc != nil { redirectURL = urlx.CopyWithQuery(redirectURL, url.Values{"no_org_ui": {"true"}}) - - for i, n := range lf.UI.Nodes { - if n.Meta == nil || n.Meta.Label == nil { - continue - } - switch n.Meta.Label.ID { - case text.InfoSelfServiceLogin: - lf.UI.Nodes[i].Meta.Label = text.NewInfoLoginAndLink() - case text.InfoSelfServiceLoginWith: - p := gjson.GetBytes(n.Meta.Label.Context, "provider").String() - lf.UI.Nodes[i].Meta.Label = text.NewInfoLoginWithAndLink(p) - } - } - - newLoginURL := s.d.Config().SelfServiceFlowLoginUI(r.Context()).String() - providerLabel := providerID - provider, _ := s.provider(r.Context(), r, providerID) - if provider != nil && provider.Config() != nil { - providerLabel = provider.Config().Label - if providerLabel == "" { - providerLabel = provider.Config().Provider - } - } - lf.UI.Messages.Add(text.NewInfoLoginLinkMessage(dc.DuplicateIdentifier, providerLabel, newLoginURL)) - - err := s.d.LoginFlowPersister().UpdateLoginFlow(r.Context(), lf) - if err != nil { + s.populateAccountLinkingUI(r.Context(), lf, usedProviderID, dc.DuplicateIdentifier, dup.AvailableCredentials()) + if err := s.d.LoginFlowPersister().UpdateLoginFlow(r.Context(), lf); err != nil { return err } } @@ -667,7 +642,7 @@ func (s *Strategy) handleError(ctx context.Context, w http.ResponseWriter, r *ht // Adds the "Continue" button rf.UI.SetCSRF(s.d.GenerateCSRFToken(r)) - AddProvider(rf.UI, providerID, text.NewInfoRegistrationContinue()) + AddProvider(rf.UI, usedProviderID, text.NewInfoRegistrationContinue()) if traits != nil { ds, err := s.d.Config().DefaultIdentityTraitsSchemaURL(r.Context()) @@ -692,6 +667,69 @@ func (s *Strategy) handleError(ctx context.Context, w http.ResponseWriter, r *ht return err } +func (s *Strategy) populateAccountLinkingUI(ctx context.Context, lf *login.Flow, usedProviderID string, duplicateIdentifier string, availableCredentials []string) { + newLoginURL := s.d.Config().SelfServiceFlowLoginUI(ctx).String() + usedProviderLabel := usedProviderID + provider, _ := s.provider(ctx, usedProviderID) + if provider != nil && provider.Config() != nil { + usedProviderLabel = provider.Config().Label + if usedProviderLabel == "" { + usedProviderLabel = provider.Config().Provider + } + } + nodes := []*node.Node{} + for _, n := range lf.UI.Nodes { + // We don't want to touch nodes unecessary nodes + if n.Meta == nil || n.Meta.Label == nil || n.Group == "default" { + nodes = append(nodes, n) + continue + } + + // Skip the provider that was used to get here (in case they used an OIDC provider) + pID := gjson.GetBytes(n.Meta.Label.Context, "provider_id").String() + if n.Group == node.OpenIDConnectGroup && pID == usedProviderID { + continue + } + + // Replace some labels to make it easier for the user to understand what's going on. + switch n.Meta.Label.ID { + case text.InfoSelfServiceLogin: + n.Meta.Label = text.NewInfoLoginAndLink() + case text.InfoSelfServiceLoginWith: + p := gjson.GetBytes(n.Meta.Label.Context, "provider").String() + n.Meta.Label = text.NewInfoLoginWithAndLink(p) + } + + // This can happen, if login hints are disabled. In that case, we need to make sure to show all credential options. + // It could in theory also happen due to a mis-configuration, and in that case, we should make sure to not delete the entire flow. + if len(availableCredentials) == 0 { + nodes = append(nodes, n) + } else { + // Hide nodes from credentials that are not relevant for the user + for _, ct := range availableCredentials { + if ct == string(n.Group) { + nodes = append(nodes, n) + break + } + } + } + } + + // Hide the "primary" identifier field present for Password, webauthn or passwordless, as we already know the identifier + identifierNode := lf.UI.Nodes.Find("identifier") + if identifierNode != nil { + if attributes, ok := identifierNode.Attributes.(*node.InputAttributes); ok { + attributes.Type = node.InputAttributeTypeHidden + attributes.SetValue(duplicateIdentifier) + identifierNode.Attributes = attributes + } + } + + lf.UI.Nodes = nodes + lf.UI.Messages.Clear() + lf.UI.Messages.Add(text.NewInfoLoginLinkMessage(duplicateIdentifier, usedProviderLabel, newLoginURL)) +} + func (s *Strategy) NodeGroup() node.UiNodeGroup { return node.OpenIDConnectGroup } diff --git a/selfservice/strategy/oidc/strategy_login.go b/selfservice/strategy/oidc/strategy_login.go index 23e96eeb1504..eb004c534621 100644 --- a/selfservice/strategy/oidc/strategy_login.go +++ b/selfservice/strategy/oidc/strategy_login.go @@ -227,7 +227,7 @@ func (s *Strategy) Login(w http.ResponseWriter, r *http.Request, f *login.Flow, return nil, s.handleError(ctx, w, r, f, pid, nil, err) } - provider, err := s.provider(ctx, r, pid) + provider, err := s.provider(ctx, pid) if err != nil { return nil, s.handleError(ctx, w, r, f, pid, nil, err) } @@ -383,7 +383,7 @@ func (s *Strategy) PopulateLoginMethodIdentifierFirstCredentials(r *http.Request for _, l := range linked { lc := l.Config() - AddProvider(f.UI, lc.ID, text.NewInfoLoginWith(stringsx.Coalesce(lc.Label, lc.ID))) + AddProvider(f.UI, lc.ID, text.NewInfoLoginWith(stringsx.Coalesce(lc.Label, lc.ID), lc.ID)) } } diff --git a/selfservice/strategy/oidc/strategy_registration.go b/selfservice/strategy/oidc/strategy_registration.go index f756e8caf156..2c229fc91704 100644 --- a/selfservice/strategy/oidc/strategy_registration.go +++ b/selfservice/strategy/oidc/strategy_registration.go @@ -181,7 +181,7 @@ func (s *Strategy) Register(w http.ResponseWriter, r *http.Request, f *registrat return s.handleError(ctx, w, r, f, pid, nil, err) } - provider, err := s.provider(ctx, r, pid) + provider, err := s.provider(ctx, pid) if err != nil { return s.handleError(ctx, w, r, f, pid, nil, err) } diff --git a/selfservice/strategy/oidc/strategy_settings.go b/selfservice/strategy/oidc/strategy_settings.go index 82df2298692c..5cba6f304bf8 100644 --- a/selfservice/strategy/oidc/strategy_settings.go +++ b/selfservice/strategy/oidc/strategy_settings.go @@ -369,7 +369,7 @@ func (s *Strategy) initLinkProvider(w http.ResponseWriter, r *http.Request, ctxU return s.handleSettingsError(w, r, ctxUpdate, p, errors.WithStack(settings.NewFlowNeedsReAuth())) } - provider, err := s.provider(ctx, r, p.Link) + provider, err := s.provider(ctx, p.Link) if err != nil { return s.handleSettingsError(w, r, ctxUpdate, p, err) } diff --git a/selfservice/strategy/oidc/strategy_test.go b/selfservice/strategy/oidc/strategy_test.go index 244232f638d2..eda0fa53c18b 100644 --- a/selfservice/strategy/oidc/strategy_test.go +++ b/selfservice/strategy/oidc/strategy_test.go @@ -82,6 +82,7 @@ func TestStrategy(t *testing.T) { t, conf, newOIDCProvider(t, ts, remotePublic, remoteAdmin, "valid"), + newOIDCProvider(t, ts, remotePublic, remoteAdmin, "valid2"), newOIDCProvider(t, ts, remotePublic, remoteAdmin, "secondProvider"), newOIDCProvider(t, ts, remotePublic, remoteAdmin, "claimsViaUserInfo", func(c *oidc.Configuration) { c.ClaimsSource = oidc.ClaimsSourceUserInfo @@ -130,14 +131,13 @@ func TestStrategy(t *testing.T) { assert.Equal(t, "POST", config.Method) - var found bool - for _, field := range config.Nodes { - if strings.Contains(field.ID(), "provider") && field.GetValue() == provider { - found = true - break + var providers []interface{} + for _, nodes := range config.Nodes { + if strings.Contains(nodes.ID(), "provider") { + providers = append(providers, nodes.GetValue()) } } - require.True(t, found, "%+v", assertx.PrettifyJSONPayload(t, config)) + require.Contains(t, providers, provider, "%+v", assertx.PrettifyJSONPayload(t, config)) return config.Action } @@ -737,7 +737,7 @@ func TestStrategy(t *testing.T) { require.NotEmpty(t, returnedFlow, "flow query param was empty in the return_to URL") loginFlow, err := reg.LoginFlowPersister().GetLoginFlow(ctx, uuid.FromStringOrNil(returnedFlow)) require.NoError(t, err) - assert.Equal(t, text.ErrorValidationDuplicateCredentials, loginFlow.UI.Messages[0].ID) + assert.Equal(t, text.InfoSelfServiceLoginLink, loginFlow.UI.Messages[0].ID) }) }) @@ -1072,7 +1072,6 @@ func TestStrategy(t *testing.T) { }) }) } - }) t.Run("case=should fail to register and return fresh login flow if email is already being used by password credentials", func(t *testing.T) { @@ -1092,16 +1091,15 @@ func TestStrategy(t *testing.T) { t.Run("case=should fail registration", func(t *testing.T) { r := newBrowserRegistrationFlow(t, returnTS.URL, time.Minute) action := assertFormValues(t, r.ID, "valid") - res, body := makeRequest(t, "valid", action, url.Values{}) - assertUIError(t, res, body, "An account with the same identifier (email, phone, username, ...) exists already.") - require.Contains(t, gjson.GetBytes(body, "ui.action").String(), "/self-service/login") + _, body := makeRequest(t, "valid", action, url.Values{}) + snapshotx.SnapshotTJSON(t, body, snapshotx.ExceptPaths("expires_at", "updated_at", "issued_at", "id", "created_at", "ui.action", "ui.nodes.4.attributes.value", "request_url"), snapshotx.ExceptNestedKeys("newLoginUrl")) }) t.Run("case=should fail login", func(t *testing.T) { r := newBrowserLoginFlow(t, returnTS.URL, time.Minute) action := assertFormValues(t, r.ID, "valid") - res, body := makeRequest(t, "valid", action, url.Values{}) - assertUIError(t, res, body, "An account with the same identifier (email, phone, username, ...) exists already.") + _, body := makeRequest(t, "valid", action, url.Values{}) + snapshotx.SnapshotTJSON(t, body, snapshotx.ExceptPaths("expires_at", "updated_at", "issued_at", "id", "created_at", "ui.action", "ui.nodes.4.attributes.value", "request_url"), snapshotx.ExceptNestedKeys("newLoginUrl")) }) }) @@ -1347,9 +1345,10 @@ func TestStrategy(t *testing.T) { t.Run("step=should fail login and start a new flow", func(t *testing.T) { res, body := loginWithOIDC(t, client, loginFlow.ID, "valid") assert.True(t, res.Request.URL.Query().Has("no_org_ui")) - assertUIError(t, res, body, "You tried signing in with new-login-if-email-exist-with-password-strategy@ory.sh which is already in use by another account. You can sign in using your password.") - assert.Equal(t, "password", gjson.GetBytes(body, "ui.messages.#(id==4000028).context.available_credential_types.0").String()) - assert.Equal(t, "new-login-if-email-exist-with-password-strategy@ory.sh", gjson.GetBytes(body, "ui.messages.#(id==4000028).context.credential_identifier_hint").String()) + assertUIError(t, res, body, "You tried to sign in with \"new-login-if-email-exist-with-password-strategy@ory.sh\", but that email is already used by another account. Sign in to your account with one of the options below to add your account \"new-login-if-email-exist-with-password-strategy@ory.sh\" at \"generic\" as another way to sign in.") + assert.True(t, gjson.GetBytes(body, "ui.nodes.#(attributes.name==identifier)").Exists(), "%s", body) + assert.True(t, gjson.GetBytes(body, "ui.nodes.#(attributes.name==password)").Exists(), "%s", body) + assert.Equal(t, "new-login-if-email-exist-with-password-strategy@ory.sh", gjson.GetBytes(body, "ui.messages.#(id==1010016).context.duplicateIdentifier").String()) linkingLoginFlow.ID = gjson.GetBytes(body, "id").String() linkingLoginFlow.UIAction = gjson.GetBytes(body, "ui.action").String() linkingLoginFlow.CSRFToken = gjson.GetBytes(body, `ui.nodes.#(attributes.name=="csrf_token").attributes.value`).String() @@ -1416,14 +1415,15 @@ func TestStrategy(t *testing.T) { loginFlow := newLoginFlow(t, returnTS.URL, time.Minute, flow.TypeBrowser) 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.") + res, body := loginWithOIDC(t, client, loginFlow.ID, "valid2") + assertUIError(t, res, body, "You tried to sign in with \"existing-oidc-identity-1@ory.sh\", but that email is already used by another account. Sign in to your account with one of the options below to add your account \"existing-oidc-identity-1@ory.sh\" at \"generic\" as another way to sign in.") linkingLoginFlow.ID = gjson.GetBytes(body, "id").String() assert.NotEqual(t, loginFlow.ID.String(), linkingLoginFlow.ID, "should have started a new flow") }) subject = email2 t.Run("step=should fail login if existing identity identifier doesn't match", func(t *testing.T) { + require.NotNil(t, linkingLoginFlow.ID) res, body := loginWithOIDC(t, client, uuid.Must(uuid.FromString(linkingLoginFlow.ID)), "valid") assertUIError(t, res, body, "Linked credentials do not match.") }) diff --git a/selfservice/strategy/oidc/types.go b/selfservice/strategy/oidc/types.go index 4460c35ce67b..768e20d23531 100644 --- a/selfservice/strategy/oidc/types.go +++ b/selfservice/strategy/oidc/types.go @@ -18,10 +18,9 @@ type FlowMethod struct { *container.Container } -func AddProviders(c *container.Container, providers []Configuration, message func(provider string) *text.Message) { +func AddProviders(c *container.Container, providers []Configuration, message func(provider string, providerId string) *text.Message) { for _, p := range providers { - AddProvider(c, p.ID, message( - stringsx.Coalesce(p.Label, p.ID))) + AddProvider(c, p.ID, message(stringsx.Coalesce(p.Label, p.ID), p.ID)) } } diff --git a/test/e2e/cypress/integration/profiles/oidc/login/success.spec.ts b/test/e2e/cypress/integration/profiles/oidc/login/success.spec.ts index 153b3332dd81..853c165df949 100644 --- a/test/e2e/cypress/integration/profiles/oidc/login/success.spec.ts +++ b/test/e2e/cypress/integration/profiles/oidc/login/success.spec.ts @@ -60,7 +60,6 @@ context("Social Sign In Successes", () => { cy.noSession() // Log in with the same identifier through the login flow. This should link the accounts. - cy.get(`${appPrefix(app)}input[name="identifier"]`).type(email) cy.get('input[name="password"]').type(password) cy.submitPasswordForm() cy.location("pathname").should("not.contain", "/login") diff --git a/test/e2e/cypress/integration/profiles/oidc/registration/success.spec.ts b/test/e2e/cypress/integration/profiles/oidc/registration/success.spec.ts index 370d5f0ff253..54adac4bdd16 100644 --- a/test/e2e/cypress/integration/profiles/oidc/registration/success.spec.ts +++ b/test/e2e/cypress/integration/profiles/oidc/registration/success.spec.ts @@ -252,7 +252,6 @@ context("Social Sign Up Successes", () => { cy.location("href").should("contain", "/login") - cy.get("[name='provider'][value='hydra']").should("be.visible") cy.get("[name='provider'][value='google']").should("be.visible") cy.get("[name='provider'][value='github']").should("be.visible") @@ -260,7 +259,6 @@ context("Social Sign Up Successes", () => { cy.get("[data-testid='forgot-password-link']").should("be.visible") } - cy.get("input[name='identifier']").type(email) cy.get("input[name='password']").type(password) cy.submitPasswordForm() cy.getSession() diff --git a/test/e2e/cypress/integration/profiles/oidc/settings/success.spec.ts b/test/e2e/cypress/integration/profiles/oidc/settings/success.spec.ts index 5f22ae886338..8eaec262b303 100644 --- a/test/e2e/cypress/integration/profiles/oidc/settings/success.spec.ts +++ b/test/e2e/cypress/integration/profiles/oidc/settings/success.spec.ts @@ -44,7 +44,7 @@ context("Social Sign In Settings Success", () => { cy.get('[data-testid="ui/message/1010016"]').should( "contain.text", - "Signing in will link your account", + "as another way to sign in.", ) cy.noSession() diff --git a/text/message_login.go b/text/message_login.go index 0a6ca5684181..a7a58118c813 100644 --- a/text/message_login.go +++ b/text/message_login.go @@ -61,7 +61,7 @@ func NewInfoLoginLinkMessage(dupIdentifier, provider, newLoginURL string) *Messa ID: InfoSelfServiceLoginLink, Type: Info, Text: fmt.Sprintf( - "Signing in will link your account to %q at provider %q. If you do not wish to link that account, please start a new login flow.", + "You tried to sign in with %q, but that email is already used by another account. Sign in to your account with one of the options below to add your account %[1]q at %q as another way to sign in.", dupIdentifier, provider, ), @@ -113,13 +113,14 @@ func NewInfoLoginVerify() *Message { } } -func NewInfoLoginWith(provider string) *Message { +func NewInfoLoginWith(provider string, providerId string) *Message { return &Message{ ID: InfoSelfServiceLoginWith, Text: fmt.Sprintf("Sign in with %s", provider), Type: Info, Context: context(map[string]any{ - "provider": provider, + "provider": provider, + "provider_id": providerId, }), } } @@ -127,7 +128,7 @@ func NewInfoLoginWith(provider string) *Message { func NewInfoLoginWithAndLink(provider string) *Message { return &Message{ ID: InfoSelfServiceLoginWithAndLink, - Text: fmt.Sprintf("Sign in with %s and link credential", provider), + Text: fmt.Sprintf("Confirm with %s", provider), Type: Info, Context: context(map[string]any{ "provider": provider, diff --git a/text/message_registration.go b/text/message_registration.go index e0d62bde2610..443d234cfda7 100644 --- a/text/message_registration.go +++ b/text/message_registration.go @@ -16,13 +16,14 @@ func NewInfoRegistration() *Message { } } -func NewInfoRegistrationWith(provider string) *Message { +func NewInfoRegistrationWith(provider string, providerID string) *Message { return &Message{ ID: InfoSelfServiceRegistrationWith, Text: fmt.Sprintf("Sign up with %s", provider), Type: Info, Context: context(map[string]any{ - "provider": provider, + "provider": provider, + "provider_id": providerID, }), } }