fix: use relative threshold to judge longest common substring in pass…

…word policy (#585) Closes #581
ory · Jul 17, 2020 · 3e9f8cc · 3e9f8cc
1 parent 7c7d59e
commit 3e9f8cc
Show file tree

Hide file tree

Showing 2 changed files with 16 additions and 11 deletions.
diff --git a/selfservice/strategy/password/validator.go b/selfservice/strategy/password/validator.go
@@ -52,18 +52,18 @@ type DefaultPasswordValidator struct {
  maxBreachesThreshold int64
  ignoreNetworkErrors bool
 
- minIdentifierPasswordDist int
- maxIdentifierPasswordSubstr int
+ minIdentifierPasswordDist  int
+ maxIdentifierPasswordSubstrThreshold float32
 }
 
 func NewDefaultPasswordValidatorStrategy() *DefaultPasswordValidator {
  return &DefaultPasswordValidator{
- c: httpx.NewResilientClientLatencyToleranceMedium(nil),
- maxBreachesThreshold: 0,
- hashes: map[string]int64{},
- ignoreNetworkErrors: true,
- minIdentifierPasswordDist: 5,
- maxIdentifierPasswordSubstr: 3,
+ c:  httpx.NewResilientClientLatencyToleranceMedium(nil),
+ maxBreachesThreshold:  0,
+ hashes:  map[string]int64{},
+ ignoreNetworkErrors:  true,
+ minIdentifierPasswordDist:  5,
+ maxIdentifierPasswordSubstrThreshold: 0.5,
  }
 }
 
@@ -154,7 +154,9 @@ func (s *DefaultPasswordValidator) Validate(identifier, password string) error {
  }
 
  compIdentifier, compPassword := strings.ToLower(identifier), strings.ToLower(password)
- if levenshtein.Distance(compIdentifier, compPassword) < s.minIdentifierPasswordDist || lcsLength(compIdentifier, compPassword) > s.maxIdentifierPasswordSubstr {
+ dist := levenshtein.Distance(compIdentifier, compPassword)
+ lcs := float32(lcsLength(compIdentifier, compPassword)) / float32(len(compPassword))
+ if dist < s.minIdentifierPasswordDist || lcs > s.maxIdentifierPasswordSubstrThreshold {
  return errors.Errorf("the password is too similar to the user identifier")
  }
 

diff --git a/selfservice/strategy/password/validator_test.go b/selfservice/strategy/password/validator_test.go
@@ -62,13 +62,16 @@ func TestDefaultPasswordValidationStrategy(t *testing.T) {
  {id: "abcd", pw: "9d3c8a1b", pass: true},
  {id: "a", pw: "kjOkla", pass: true},
  {id: "ab", pw: "0000ab0000", pass: true},
+ // longest common substring with long password
+ {id: "d4f6090b-5a84", pw: "d4f6090b-5a84-2184-4404-8d1b-8da3eb00ebbe", pass: true},
+ {id: "asdflasdflasdf", pw: "asdflasdflpiuhefnciluaksdzuföfhg", pass: true},
  } {
  t.Run(fmt.Sprintf("case=%d", k), func(t *testing.T) {
  err := s.Validate(tc.id, tc.pw)
  if tc.pass {
- require.NoError(t, err, "%+v", err)
+ require.NoError(t, err, "err: %+v, id: %s, pw: %s", err, tc.id, tc.pw)
  } else {
- require.Error(t, err)
+ require.Error(t, err, "id: %s, pw: %s", tc.id, tc.pw)
  }
  })
  }