add: PasswordChecker interface
authorLucas Werkmeister <mail@lucaswerkmeister.de>
Sat, 13 Jan 2018 18:56:44 +0000 (19:56 +0100)
committerLucas Werkmeister <mail@lucaswerkmeister.de>
Sat, 9 Jun 2018 11:20:40 +0000 (13:20 +0200)
PasswordChecker is a generic version of the interface which
PasswordStrengthChecker currently offers. PasswordStrengthChecker is
changed to implement the new interface (currently the only
implementation, but others will be added in the future).

Using this interface instead of PasswordStrengthChecker directly in
other code will let us introduce other ways of checking password
strength as well, e. g. implementing #143.

The interface is placed in the new `passwords` subpackage, and the
PasswordStrengthChecker implementation is also moved there.

Change-Id: I2fb9dde216db7b14f3d4d45342bdc5c657c87233

src/club/wpia/gigi/dbObjects/User.java
src/club/wpia/gigi/pages/main/Signup.java
src/club/wpia/gigi/passwords/PasswordChecker.java [new file with mode: 0644]
src/club/wpia/gigi/passwords/PasswordStrengthChecker.java [moved from src/club/wpia/gigi/util/PasswordStrengthChecker.java with 54% similarity]
tests/club/wpia/gigi/passwords/TestPasswordStrengthChecker.java [moved from tests/club/wpia/gigi/util/TestPasswordStrengthChecker.java with 92% similarity]

index 3c2cd6b..55bb03f 100644 (file)
@@ -10,6 +10,7 @@ import java.util.LinkedList;
 import java.util.List;
 import java.util.Locale;
 import java.util.Set;
+import java.util.TreeSet;
 
 import club.wpia.gigi.GigiApiException;
 import club.wpia.gigi.database.GigiPreparedStatement;
@@ -21,11 +22,11 @@ import club.wpia.gigi.email.EmailProvider;
 import club.wpia.gigi.localisation.Language;
 import club.wpia.gigi.output.DateSelector;
 import club.wpia.gigi.pages.PasswordResetPage;
+import club.wpia.gigi.passwords.PasswordStrengthChecker;
 import club.wpia.gigi.util.CalendarUtil;
 import club.wpia.gigi.util.DayDate;
 import club.wpia.gigi.util.Notary;
 import club.wpia.gigi.util.PasswordHash;
-import club.wpia.gigi.util.PasswordStrengthChecker;
 import club.wpia.gigi.util.TimeConditions;
 
 /**
@@ -209,7 +210,17 @@ public class User extends CertificateOwner {
     }
 
     private void setPassword(String newPass) throws GigiApiException {
-        PasswordStrengthChecker.assertStrongPassword(newPass, getNames(), getEmail());
+        Name[] names = getNames();
+        TreeSet<String> nameParts = new TreeSet<>();
+        for (int i = 0; i < names.length; i++) {
+            for (NamePart string : names[i].getParts()) {
+                nameParts.add(string.getValue());
+            }
+        }
+        GigiApiException gaPassword = new PasswordStrengthChecker().checkPassword(newPass, nameParts.toArray(new String[nameParts.size()]), getEmail());
+        if (gaPassword != null) {
+            throw gaPassword;
+        }
         try (GigiPreparedStatement ps = new GigiPreparedStatement("UPDATE users SET `password`=? WHERE id=?")) {
             ps.setString(1, PasswordHash.hash(newPass));
             ps.setInt(2, getId());
index 5c68c38..b6585cb 100644 (file)
@@ -23,10 +23,10 @@ import club.wpia.gigi.output.template.SprintfCommand;
 import club.wpia.gigi.output.template.Template;
 import club.wpia.gigi.output.template.TranslateCommand;
 import club.wpia.gigi.pages.Page;
+import club.wpia.gigi.passwords.PasswordStrengthChecker;
 import club.wpia.gigi.util.CalendarUtil;
 import club.wpia.gigi.util.HTMLEncoder;
 import club.wpia.gigi.util.Notary;
-import club.wpia.gigi.util.PasswordStrengthChecker;
 import club.wpia.gigi.util.RateLimit.RateLimitException;
 
 public class Signup extends Form {
@@ -127,13 +127,13 @@ public class Signup extends Form {
         } else if ( !pw1.equals(pw2)) {
             ga.mergeInto(new GigiApiException("Passwords don't match"));
         }
-        int pwpoints = PasswordStrengthChecker.checkpw(pw1, ni.getNamePartsPlain(), email);
-        if (pwpoints < 3) {
-            ga.mergeInto(new GigiApiException(new SprintfCommand("The Password you submitted failed to contain enough differing characters and/or contained words from your name and/or email address. For the current requirements and to learn more, visit our {0}FAQ{1}.", Arrays.asList("!(/kb/goodPassword", "!'</a>'"))));
-        }
         if ( !ga.isEmpty()) {
             throw ga;
         }
+        GigiApiException gaPassword = new PasswordStrengthChecker().checkPassword(pw1, ni.getNamePartsPlain(), email);
+        if (gaPassword != null) {
+            throw gaPassword;
+        }
         GigiApiException ga2 = new GigiApiException();
         try (GigiPreparedStatement q1 = new GigiPreparedStatement("SELECT * FROM `emails` WHERE `email`=? AND `deleted` IS NULL"); GigiPreparedStatement q2 = new GigiPreparedStatement("SELECT * FROM `certOwners` INNER JOIN `users` ON `users`.`id`=`certOwners`.`id` WHERE `email`=? AND `deleted` IS NULL")) {
             q1.setString(1, email);
diff --git a/src/club/wpia/gigi/passwords/PasswordChecker.java b/src/club/wpia/gigi/passwords/PasswordChecker.java
new file mode 100644 (file)
index 0000000..822abf2
--- /dev/null
@@ -0,0 +1,23 @@
+package club.wpia.gigi.passwords;
+
+import club.wpia.gigi.GigiApiException;
+import club.wpia.gigi.dbObjects.Name;
+
+/**
+ * A strategy to check whether a password is acceptable for use or not.
+ */
+public interface PasswordChecker {
+
+    /**
+     * Checks if a password is acceptable for use.
+     * Most implementations judge a password’s strength in some way
+     * and reject weak passwords.
+     *
+     * @param password The password to check.
+     * @param nameParts The name parts of the user that wants to use this password.
+     * @param email The email address of the user that wants to use this password.
+     * @return {@code null} if the password is acceptable,
+     * otherwise a {@link GigiApiException} with an appropriate error message.
+     */
+    public GigiApiException checkPassword(String password, String[] nameParts, String email);
+}
@@ -1,4 +1,4 @@
-package club.wpia.gigi.util;
+package club.wpia.gigi.passwords;
 
 import java.util.Arrays;
 import java.util.TreeSet;
@@ -9,7 +9,7 @@ import club.wpia.gigi.dbObjects.Name;
 import club.wpia.gigi.dbObjects.NamePart;
 import club.wpia.gigi.output.template.SprintfCommand;
 
-public class PasswordStrengthChecker {
+public class PasswordStrengthChecker implements PasswordChecker {
 
     private static Pattern digits = Pattern.compile("\\d");
 
@@ -21,9 +21,13 @@ public class PasswordStrengthChecker {
 
     private static Pattern special = Pattern.compile("(?!\\s)\\W");
 
-    private PasswordStrengthChecker() {}
+    public PasswordStrengthChecker() {}
 
-    private static int checkpwlight(String pw) {
+    /**
+     * @param pw The password.
+     * @return Estimate of the password’s strength (positive).
+     */
+    private int ratePasswordStrength(String pw) {
         int points = 0;
         if (pw.length() > 15) {
             points++;
@@ -55,32 +59,39 @@ public class PasswordStrengthChecker {
         return points;
     }
 
-    public static int checkpw(String pw, String[] nameParts, String email) {
-        if (pw == null) {
-            return 0;
-        }
-        int light = checkpwlight(pw);
+    /**
+     * @param pw The password.
+     * @param nameParts The name parts of the user.
+     * @param email The email address of the user.
+     * @return Estimate of the password’s weakness (negative).
+     */
+    private int ratePasswordWeakness(String pw, String[] nameParts, String email) {
+        int points = 0;
         if (contained(pw, email)) {
-            light -= 2;
+            points -= 2;
         }
         for (int i = 0; i < nameParts.length; i++) {
             if (contained(pw, nameParts[i])) {
-                light -= 2;
+                points -= 2;
             }
         }
-        // TODO dictionary check
-        return light;
+        return points;
     }
 
-    public static void assertStrongPassword(String pw, Name[] names, String email) throws GigiApiException {
-        TreeSet<String> parts = new TreeSet<>();
-        for (int i = 0; i < names.length; i++) {
-            for (NamePart string : names[i].getParts()) {
-                parts.add(string.getValue());
-            }
-        }
-        if (checkpw(pw, parts.toArray(new String[parts.size()]), email) < 3) {
-            throw (new GigiApiException(new SprintfCommand("The Password you submitted failed to contain enough differing characters and/or contained words from your name and/or email address. For the current requirements and to learn more, visit our {0}FAQ{1}.", Arrays.asList("!(/kb/goodPassword", "!'</a>'"))));
+    public int ratePassword(String pw, String[] nameParts, String email) {
+        return ratePasswordStrength(pw) + ratePasswordWeakness(pw, nameParts, email);
+    }
+
+    @Override
+    public GigiApiException checkPassword(String password, String[] nameParts, String email) {
+        int points = ratePassword(password, nameParts, email);
+        if (points < 3) {
+            return new GigiApiException(new SprintfCommand(
+                "The Password you submitted failed to contain enough differing characters and/or contained words from your name and/or email address. For the current requirements and to learn more, visit our {0}FAQ{1}.",
+                Arrays.asList("!(/kb/goodPassword", "!'</a>'")
+            ));
+        } else {
+            return null;
         }
     }
 
@@ -1,11 +1,11 @@
-package club.wpia.gigi.util;
+package club.wpia.gigi.passwords;
 
 import static org.junit.Assert.*;
 
 import org.junit.Test;
 
+import club.wpia.gigi.passwords.PasswordStrengthChecker;
 import club.wpia.gigi.testUtils.ClientBusinessTest;
-import club.wpia.gigi.util.PasswordStrengthChecker;
 
 public class TestPasswordStrengthChecker extends ClientBusinessTest {
 
@@ -14,7 +14,7 @@ public class TestPasswordStrengthChecker extends ClientBusinessTest {
     public TestPasswordStrengthChecker() {}
 
     private int check(String pw) {
-        return PasswordStrengthChecker.checkpw(pw, new String[] {
+        return new PasswordStrengthChecker().ratePassword(pw, new String[] {
                 "fname", "lname", "mname", "suffix"
         }, e);
     }