Skip to content

Commit

Permalink
fix: improve password error resilience on settings flow
Browse files Browse the repository at this point in the history
  • Loading branch information
aeneasr committed Feb 26, 2022
1 parent afb7aaf commit e614f6e
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 17 deletions.
16 changes: 8 additions & 8 deletions selfservice/strategy/password/registration.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,16 +136,16 @@ func (s *Strategy) validateCredentials(ctx context.Context, i *identity.Identity
// This should never happen
return errors.WithStack(x.PseudoPanic.WithReasonf("identity object did not provide the %s CredentialType unexpectedly", identity.CredentialsTypePassword))
} else if len(c.Identifiers) == 0 {
return schema.NewMissingIdentifierError()
}

// Sometimes, no identifier is set, but we still want to validate the password!
ids := c.Identifiers
if len(ids) == 0 {
ids = []string{""}
if err := s.d.PasswordValidator().Validate(ctx, "", pw); err != nil {
if _, ok := errorsx.Cause(err).(*herodot.DefaultError); ok {
return err
}
return schema.NewPasswordPolicyViolationError("#/password", err.Error())
}
return nil
}

for _, id := range ids {
for _, id := range c.Identifiers {
if err := s.d.PasswordValidator().Validate(ctx, id, pw); err != nil {
if _, ok := errorsx.Cause(err).(*herodot.DefaultError); ok {
return err
Expand Down
1 change: 0 additions & 1 deletion selfservice/strategy/password/settings.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,6 @@ func (s *Strategy) continueSettingsFlow(
}

ctxUpdate.UpdateIdentity(i)

return nil
}

Expand Down
28 changes: 21 additions & 7 deletions selfservice/strategy/password/settings_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,15 @@ func newEmptyIdentity() *identity.Identity {
}
}

func newIdentityWithoutCredentials(email string) *identity.Identity {
return &identity.Identity{
ID: x.NewUUID(),
State: identity.StateActive,
Traits: identity.Traits(`{"email":"` + email + `"}`),
SchemaID: config.DefaultIdentityTraitsSchemaID,
}
}

func TestSettings(t *testing.T) {
conf, reg := internal.NewFastRegistryWithMocks(t)
conf.MustSet(config.ViperKeySelfServiceBrowserDefaultReturnTo, "https://www.ory.sh/")
Expand Down Expand Up @@ -321,6 +330,7 @@ func TestSettings(t *testing.T) {
}

t.Run("description=should update the password even if no password was set before", func(t *testing.T) {
email := fmt.Sprintf("test+%[email protected]", x.NewUUID())
var check = func(t *testing.T, actual string, id *identity.Identity) {
assert.Equal(t, "success", gjson.Get(actual, "state").String(), "%s", actual)
assert.Empty(t, gjson.Get(actual, "ui.nodes.#(name==password).attributes.value").String(), "%s", actual)
Expand All @@ -330,27 +340,31 @@ func TestSettings(t *testing.T) {
cfg := string(actualIdentity.Credentials[identity.CredentialsTypePassword].Config)
assert.Contains(t, cfg, "hashed_password", "%+v", actualIdentity.Credentials)
require.Len(t, actualIdentity.Credentials[identity.CredentialsTypePassword].Identifiers, 1)
assert.Contains(t, actualIdentity.Credentials[identity.CredentialsTypePassword].Identifiers[0], "-4")
assert.Equal(t, email, actualIdentity.Credentials[identity.CredentialsTypePassword].Identifiers[0])
}

var payload = func(v url.Values) {
v.Set("method", "password")
v.Set("password", randx.MustString(16, randx.AlphaNum))
}

id := newIdentityWithoutCredentials(email)
browserUser := testhelpers.NewHTTPClientWithIdentitySessionCookie(t, reg, id)
apiUser := testhelpers.NewHTTPClientWithIdentitySessionToken(t, reg, id)

t.Run("type=api", func(t *testing.T) {
actual := expectSuccess(t, true, false, apiUser2, payload)
check(t, actual, apiIdentity2)
actual := expectSuccess(t, true, false, apiUser, payload)
check(t, actual, id)
})

t.Run("type=spa", func(t *testing.T) {
actual := expectSuccess(t, false, true, browserUser2, payload)
check(t, actual, browserIdentity2)
actual := expectSuccess(t, false, true, browserUser, payload)
check(t, actual, id)
})

t.Run("type=browser", func(t *testing.T) {
actual := expectSuccess(t, false, false, browserUser2, payload)
check(t, actual, browserIdentity2)
actual := expectSuccess(t, false, false, browserUser, payload)
check(t, actual, id)
})
})

Expand Down
2 changes: 1 addition & 1 deletion selfservice/strategy/password/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ func (s *DefaultPasswordValidator) Validate(ctx context.Context, identifier, pas
return errors.Errorf("password length must be at least %d characters but only got %d", passwordPolicyConfig.MinPasswordLength, len(password))
}

if passwordPolicyConfig.IdentifierSimilarityCheckEnabled {
if passwordPolicyConfig.IdentifierSimilarityCheckEnabled && len(identifier) > 0 {
compIdentifier, compPassword := strings.ToLower(identifier), strings.ToLower(password)
dist := levenshtein.Distance(compIdentifier, compPassword)
lcs := float32(lcsLength(compIdentifier, compPassword)) / float32(len(compPassword))
Expand Down

0 comments on commit e614f6e

Please sign in to comment.