KEYCLOAK-4889

Improve error messages for password policies

server-spi-private/src/main/java/org/keycloak/policy/HashAlgorithmPasswordPolicyProviderFactory.java

@@ -91,7 +91,7 @@ public class HashAlgorithmPasswordPolicyProviderFactory implements PasswordPolic
         String providerId = value != null && value.length() > 0 ? value : PasswordPolicy.HASH_ALGORITHM_DEFAULT;
         PasswordHashProvider provider = session.getProvider(PasswordHashProvider.class, providerId);
         if (provider == null) {
-            throw new ModelException("Password hashing provider not found");
+            throw new PasswordPolicyConfigException("Password hashing provider not found");
         }
         return providerId;
     }

server-spi-private/src/main/java/org/keycloak/policy/HistoryPasswordPolicyProvider.java

@@ -73,7 +73,7 @@ public class HistoryPasswordPolicyProvider implements PasswordPolicyProvider {
 
     @Override
     public Object parseConfig(String value) {
-        return value != null ? Integer.parseInt(value) : HistoryPasswordPolicyProviderFactory.DEFAULT_VALUE;
+        return parseInteger(value, HistoryPasswordPolicyProviderFactory.DEFAULT_VALUE);
     }
 
     @Override

server-spi-private/src/main/java/org/keycloak/policy/LengthPasswordPolicyProvider.java

@@ -47,7 +47,7 @@ public class LengthPasswordPolicyProvider implements PasswordPolicyProvider {
 
     @Override
     public Object parseConfig(String value) {
-        return value != null ? Integer.parseInt(value) : 8;
+        return parseInteger(value, 8);
     }
 
     @Override

server-spi-private/src/main/java/org/keycloak/policy/LowerCasePasswordPolicyProvider.java

@@ -53,7 +53,7 @@ public class LowerCasePasswordPolicyProvider implements PasswordPolicyProvider {
 
     @Override
     public Object parseConfig(String value) {
-        return value != null ? Integer.parseInt(value) : 1;
+        return parseInteger(value, 1);
     }
 
     @Override

server-spi-private/src/main/java/org/keycloak/policy/RegexPatternsPasswordPolicyProvider.java

@@ -23,6 +23,7 @@ import org.keycloak.models.UserModel;
 
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
+import java.util.regex.PatternSyntaxException;
 
 /**
  * @author <a href="mailto:sthorger@redhat.com">Stian Thorgersen</a>
@@ -55,9 +56,13 @@ public class RegexPatternsPasswordPolicyProvider implements PasswordPolicyProvid
     @Override
     public Object parseConfig(String value) {
         if (value == null) {
-            throw new IllegalArgumentException("Config required");
+            throw new PasswordPolicyConfigException("Config required");
+        }
+        try {
+            return Pattern.compile(value);
+        } catch (PatternSyntaxException e) {
+            throw new PasswordPolicyConfigException("Not a valid regular expression");
         }
-        return Pattern.compile(value);
     }
 
     @Override

server-spi-private/src/main/java/org/keycloak/policy/SpecialCharsPasswordPolicyProvider.java

@@ -53,7 +53,7 @@ public class SpecialCharsPasswordPolicyProvider implements PasswordPolicyProvide
 
     @Override
     public Object parseConfig(String value) {
-        return value != null ? Integer.parseInt(value) : 1;
+        return parseInteger(value, 1);
     }
 
     @Override

server-spi-private/src/main/java/org/keycloak/policy/UpperCasePasswordPolicyProvider.java

@@ -53,7 +53,7 @@ public class UpperCasePasswordPolicyProvider implements PasswordPolicyProvider {
 
     @Override
     public Object parseConfig(String value) {
-        return value != null ? Integer.parseInt(value) : 1;
+        return parseInteger(value, 1);
     }
 
     @Override

server-spi/src/main/java/org/keycloak/models/PasswordPolicy.java

@@ -17,6 +17,7 @@
 
 package org.keycloak.models;
 
+import org.keycloak.policy.PasswordPolicyConfigException;
 import org.keycloak.policy.PasswordPolicyProvider;
 
 import java.io.Serializable;
@@ -68,10 +69,17 @@ public class PasswordPolicy implements Serializable {
 
                 PasswordPolicyProvider provider = session.getProvider(PasswordPolicyProvider.class, key);
                 if (provider == null) {
-                    throw new IllegalArgumentException("Unsupported policy");
+                    throw new PasswordPolicyConfigException("Password policy not found");
                 }
 
-                policyConfig.put(key, provider.parseConfig(config));
+                Object o;
+                try {
+                    o = provider.parseConfig(config);
+                } catch (PasswordPolicyConfigException e) {
+                    throw new ModelException("Invalid config for " + key + ": " + e.getMessage());
+                }
+
+                policyConfig.put(key, o);
             }
         }
 

server-spi/src/main/java/org/keycloak/policy/PasswordPolicyConfigException.java

@@ -0,0 +1,14 @@
+package org.keycloak.policy;
+
+import org.keycloak.models.ModelException;
+
+/**
+ * Created by st on 23/05/17.
+ */
+public class PasswordPolicyConfigException extends ModelException {
+
+    public PasswordPolicyConfigException(String message) {
+        super(message);
+    }
+
+}

server-spi/src/main/java/org/keycloak/policy/PasswordPolicyProvider.java

@@ -33,4 +33,12 @@ public interface PasswordPolicyProvider extends Provider {
     PolicyError validate(String user, String password);
     Object parseConfig(String value);
 
+    default Integer parseInteger(String value, Integer defaultValue) {
+        try {
+            return value != null ? Integer.parseInt(value) : defaultValue;
+        } catch (NumberFormatException e) {
+            throw new PasswordPolicyConfigException("Not a valid number");
+        }
+    }
+
 }

services/src/main/java/org/keycloak/services/resources/admin/RealmAdminResource.java

@@ -325,8 +325,6 @@ public class RealmAdminResource {
             }
             
             return Response.noContent().build();
-        } catch (PatternSyntaxException e) {
-            return ErrorResponse.error("Specified regex pattern(s) is invalid.", Response.Status.BAD_REQUEST);
         } catch (ModelDuplicateException e) {
             return ErrorResponse.exists("Realm with same name exists");
         } catch (ModelException e) {

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/PasswordHashingTest.java

@@ -97,7 +97,7 @@ public class PasswordHashingTest extends AbstractTestRealmKeycloakTest {
             fail("Expected error");
         } catch (BadRequestException e) {
             ErrorRepresentation error = e.getResponse().readEntity(ErrorRepresentation.class);
-            assertEquals("Password hashing provider not found", error.getErrorMessage());
+            assertEquals("Invalid config for hashAlgorithm: Password hashing provider not found", error.getErrorMessage());
         }
     }
 

testsuite/integration/src/test/java/org/keycloak/testsuite/model/PasswordPolicyTest.java

@@ -20,12 +20,14 @@ package org.keycloak.testsuite.model;
 import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
+import org.keycloak.models.ModelException;
 import org.keycloak.models.PasswordPolicy;
 import org.keycloak.models.RealmModel;
 import org.keycloak.policy.PasswordPolicyManagerProvider;
 
 import java.util.regex.PatternSyntaxException;
 
+import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.fail;
 
 /**
@@ -123,7 +125,8 @@ public class PasswordPolicyTest extends AbstractModelTest {
         try {
             realmModel.setPasswordPolicy(PasswordPolicy.parse(session, "noSuchPolicy"));
             Assert.fail("Expected exception");
-        } catch (IllegalArgumentException e) {
+        } catch (ModelException e) {
+            assertEquals("Password policy not found", e.getMessage());
         }
     }
 
@@ -133,22 +136,22 @@ public class PasswordPolicyTest extends AbstractModelTest {
         try {
             realmModel.setPasswordPolicy(PasswordPolicy.parse(session, "regexPattern"));
             fail("Expected NullPointerException: Regex Pattern cannot be null.");
-        } catch (IllegalArgumentException e) {
+        } catch (ModelException e) {
-            // Expected NPE as regex pattern is null.
+            assertEquals("Invalid config for regexPattern: Config required", e.getMessage());
         }
 
         try {
             realmModel.setPasswordPolicy(PasswordPolicy.parse(session, "regexPattern(*)"));
             fail("Expected PatternSyntaxException: Regex Pattern cannot be null.");
-        } catch (PatternSyntaxException e) {
+        } catch (ModelException e) {
-            // Expected PSE as regex pattern(or any of its token) is not quantifiable.
+            assertEquals("Invalid config for regexPattern: Not a valid regular expression", e.getMessage());
         }
 
         try {
             realmModel.setPasswordPolicy(PasswordPolicy.parse(session, "regexPattern(*,**)"));
             fail("Expected PatternSyntaxException: Regex Pattern cannot be null.");
-        } catch (PatternSyntaxException e) {
+        } catch (ModelException e) {
-            // Expected PSE as regex pattern(or any of its token) is not quantifiable.
+            assertEquals("Invalid config for regexPattern: Not a valid regular expression", e.getMessage());
         }
 
         //Fails to match one of the regex pattern