Merge pull request #769 from metabase/less-strict-passwords

Less strict password requirements

src/metabase/config.clj

@@ -21,7 +21,7 @@
    :mb-jetty-port "3000"
    ;; Other Application Settings
    :mb-password-complexity "normal"
-   :mb-password-length "8"
+   ;:mb-password-length "8"
    :max-session-age "20160"})                    ; session length in minutes (14 days)
 
 

src/metabase/driver/mongo/util.clj

@@ -12,9 +12,9 @@
    forever, especially when called with bad details. This translates to our tests
    taking longer and the DB setup API endpoints seeming sluggish.
 
-   Don't set the timeout too low -- I've have Circle fail when the timeout was 250ms
-   on one occasion."
-  1000)
+   Don't set the timeout too low -- I've have Circle fail when the timeout was 1000ms
+   on *one* occasion."
+  1500)
 
 (defn- details-map->connection-string
   [{:keys [user pass host port dbname]}]

src/metabase/util/password.clj

@@ -4,28 +4,54 @@
 
 
 (defn- count-occurrences
-  "Takes in a Character predicate function which is applied to all characters in the supplied string and uses
-   map/reduce to count the number of characters which return `true` for the given predicate function."
-  [f s]
-  {:pre [(fn? f)
-         (string? s)]}
-  (reduce + (map #(if (true? (f %)) 1 0) s)))
-
-(defn is-complex?
-  "Check if a given password meets complexity standards for the application."
+  "Return a map of the counts of each class of character for PASSWORD.
+
+    (count-occurrences \"GoodPw!!\")
+      -> {:total  8, :lower 4, :upper 2, :letter 6, :digit 0, :special 2}"
   [password]
-  {:pre [(string? password)]}
-  (let [complexity (config/config-kw :mb-password-complexity)
-        length     (config/config-int :mb-password-length)
-        lowers     (count-occurrences #(Character/isLowerCase ^Character %) password)
-        uppers     (count-occurrences #(Character/isUpperCase ^Character %) password)
-        digits     (count-occurrences #(Character/isDigit ^Character %) password)
-        specials   (count-occurrences #(not (Character/isLetterOrDigit ^Character %)) password)]
-    (if-not (>= (count password) length) false
-      (case complexity
-        :weak   (and (> lowers 0) (> digits 0) (> uppers 0))                    ; weak   = 1 lower, 1 digit, 1 uppercase
-        :normal (and (> lowers 0) (> digits 0) (> uppers 0) (> specials 0))     ; normal = 1 lower, 1 digit, 1 uppercase, 1 special
-        :strong (and (> lowers 1) (> digits 0) (> uppers 1) (> specials 0)))))) ; strong = 2 lower, 1 digit, 2 uppercase, 1 special
+  (loop [[^Character c & more] password, {:keys [total, lower, upper, letter, digit, special], :as counts} {:total 0, :lower 0, :upper 0, :letter 0, :digit 0, :special 0}]
+    (if-not c counts
+      (recur more (merge (update counts :total inc)
+                         (cond
+                           (Character/isLowerCase c) {:lower   (inc lower), :letter (inc letter)}
+                           (Character/isUpperCase c) {:upper   (inc upper), :letter (inc letter)}
+                           (Character/isDigit     c) {:digit   (inc digit)}
+                           :else                     {:special (inc special)}))))))
+
+(def ^:private ^:const complexity->char-type->min
+  "Minimum counts of each class of character a password should have for a given password complexity level."
+  {:weak   {:total   6} ; total here effectively means the same thing as a minimum password length
+   :normal {:total   6
+            :digit   1}
+   :strong {:total   8
+            :lower   2
+            :upper   2
+            :digit   1
+            :special 1}})
+
+(defn- password-has-char-counts?
+  "Check that PASSWORD satisfies the minimum count requirements for each character class.
+
+    (password-has-char-counts? {:total 6, :lower 1, :upper 1, :digit 1, :special 1} \"abc\")
+      -> false
+    (password-has-char-counts? {:total 6, :lower 1, :upper 1, :digit 1, :special 1} \"passworD1!\")
+      -> true"
+  [char-type->min password]
+  {:pre [(map? char-type->min)
+         (string? password)]}
+  (let [occurances (count-occurrences password)]
+    (boolean (loop [[[char-type min-count] & more] (seq char-type->min)]
+               (if-not char-type true
+                 (when (>= (occurances char-type) min-count)
+                   (recur more)))))))
+
+(def ^{:arglists '([password])} is-complex?
+  "Check if a given password meets complexity standards for the application."
+  (partial password-has-char-counts? (merge (complexity->char-type->min (config/config-kw :mb-password-complexity))
+                                            ;; Setting MB_PASSWORD_LENGTH overrides the default :total for a given password complexity class
+                                            (when-let [min-len (config/config-int :mb-password-length)]
+                                              {:total min-len}))))
+
 
 (defn verify-password
   "Verify if a given unhashed password + salt matches the supplied hashed-password.  Returns true if matched, false otherwise."

test/metabase/util/password_test.clj

 (ns metabase.util.password-test
   (:require [expectations :refer :all]
+            [metabase.test.util :refer [resolve-private-fns]]
             [metabase.util.password :refer :all]))
 
 
 ;; Password Complexity testing
-;; TODO - need a way to test other complexity scenarios.  DI on the config would make this easier.
-
-; fail due to being too short (min 8 chars)
-(expect false (is-complex? "god"))
-(expect false (is-complex? "god12"))
-(expect false (is-complex? "god4!"))
-(expect false (is-complex? "Agod4!"))
-
-; fail due to missing complexity
-(expect false (is-complex? "password"))
-(expect false (is-complex? "password1"))
-(expect false (is-complex? "password1!"))
-(expect false (is-complex? "passworD!"))
-(expect false (is-complex? "passworD1"))
-
-; these passwords should be good
-(expect true (is-complex? "passworD1!"))
-(expect true (is-complex? "paSS&&word1"))
-(expect true (is-complex? "passW0rd))"))
-(expect true (is-complex? "^^Wut4nG^^"))
+
+(resolve-private-fns metabase.util.password count-occurrences password-has-char-counts?)
+
+;; Check that password occurance counting works
+(expect {:total  3, :lower 3, :upper 0, :letter 3, :digit 0, :special 0} (count-occurrences "abc"))
+(expect {:total  8, :lower 0, :upper 8, :letter 8, :digit 0, :special 0} (count-occurrences "PASSWORD"))
+(expect {:total  3, :lower 0, :upper 0, :letter 0, :digit 3, :special 0} (count-occurrences "123"))
+(expect {:total  8, :lower 4, :upper 2, :letter 6, :digit 0, :special 2} (count-occurrences "GoodPw!!"))
+(expect {:total  9, :lower 7, :upper 1, :letter 8, :digit 1, :special 0} (count-occurrences "passworD1"))
+(expect {:total 10, :lower 3, :upper 2, :letter 5, :digit 1, :special 4} (count-occurrences "^^Wut4nG^^"))
+
+;; Check that password length complexity applies
+(expect true  (password-has-char-counts? {:total 3} "god1"))
+(expect true  (password-has-char-counts? {:total 4} "god1"))
+(expect false (password-has-char-counts? {:total 5} "god1"))
+
+;; Check that testing password character type complexity works
+(expect true  (password-has-char-counts? {} "ABC"))
+(expect false (password-has-char-counts? {:lower 1} "ABC"))
+(expect true  (password-has-char-counts? {:lower 1} "abc"))
+(expect false (password-has-char-counts? {:digit 1} "abc"))
+(expect true  (password-has-char-counts? {:digit 1, :special 2} "!0!"))
+
+;; Do some tests that combine both requirements
+(expect false (password-has-char-counts? {:total 6, :lower 1, :upper 1, :digit 1, :special 1} "^aA2"))
+(expect false (password-has-char-counts? {:total 6, :lower 1, :upper 1, :digit 1, :special 1} "password"))
+(expect false (password-has-char-counts? {:total 6, :lower 1, :upper 1, :digit 1, :special 1} "password1"))
+(expect false (password-has-char-counts? {:total 6, :lower 1, :upper 1, :digit 1, :special 1} "password1!"))
+(expect false (password-has-char-counts? {:total 6, :lower 1, :upper 1, :digit 1, :special 1} "passworD!"))
+(expect false (password-has-char-counts? {:total 6, :lower 1, :upper 1, :digit 1, :special 1} "passworD1"))
+(expect true  (password-has-char-counts? {:total 6, :lower 1, :upper 1, :digit 1, :special 1} "passworD1!"))
+(expect true  (password-has-char-counts? {:total 6, :lower 1, :upper 1, :digit 1, :special 1} "paSS&&word1"))
+(expect true  (password-has-char-counts? {:total 6, :lower 1, :upper 1, :digit 1, :special 1} "passW0rd))"))
+(expect true  (password-has-char-counts? {:total 6, :lower 1, :upper 1, :digit 1, :special 1} "^^Wut4nG^^"))
+
+;; Do some tests with the default (:normal) password requirements
+(expect false (is-complex? "ABC"))
+(expect false (is-complex? "ABCDEF"))
+(expect true  (is-complex? "ABCDE1"))
+(expect true  (is-complex? "123456"))