Skip to content

Commit

Permalink
fix: include all creds in duplicate credential err
Browse files Browse the repository at this point in the history
  • Loading branch information
hperl committed Apr 19, 2024
1 parent eb67bed commit a987d5e
Show file tree
Hide file tree
Showing 5 changed files with 69 additions and 30 deletions.
43 changes: 25 additions & 18 deletions identity/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"context"
"encoding/json"
"reflect"
"slices"
"sort"

"go.opentelemetry.io/otel/trace"
Expand Down Expand Up @@ -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
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -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
}

Expand Down
37 changes: 36 additions & 1 deletion identity/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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)
})
}
Expand Down
1 change: 1 addition & 0 deletions internal/client-go/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down
2 changes: 1 addition & 1 deletion selfservice/strategy/webauthn/registration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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, your PassKey or a security key.", gjson.Get(actual, "ui.messages.0.text").String(), "%s", actual)
})
}
})
Expand Down
16 changes: 6 additions & 10 deletions text/message_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -279,25 +277,23 @@ 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...)
}
reason += fmt.Sprintf(" You can sign in using %s.", strings.Join(maps.Keys(humanReadable), ", "))
reason += fmt.Sprintf(" You can sign in using %s.", strings.Join(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, ", "))
Expand Down

0 comments on commit a987d5e

Please sign in to comment.