KEYCLOAK-1807 Fix bug with commas in regex password policies by using alternate way to declare multiple regex patterns

docbook/reference/en/en-US/modules/security-vulnerabilities.xml

@@ -130,8 +130,8 @@
             A password has to match all policies. The password policies that can be configured are hash iterations, length, digits,
             lowercase, uppercase, special characters, not username, regex patterns, password history and force expired password update. 
             Force expired password update policy forces or requires password updates after specified span of time. Password history policy 
-            restricts a user from resetting his password to N old expired passwords. Multiple regex patterns, separated by comma, 
+            restricts a user from resetting his password to N old expired passwords. Multiple regex patterns can be specified.
-            can be specified in regex pattern policy. If there's more than one regex added, password has to match all fully.
+            If there's more than one regex added, password has to match all fully.
             Increasing number of Hash Iterations (n) does not worsen anything (and certainly not the cipher) and it greatly increases the 
             resistance to dictionary attacks. However the drawback to increasing n is that it has some cost (CPU usage, energy, delay) for 
             the legitimate parties. Increasing n also slightly increases the odds that a random password gives the same result as the right 

forms/common-themes/src/main/resources/theme/base/admin/resources/js/controllers/realm.js

@@ -414,6 +414,14 @@ module.controller('RealmPasswordPolicyCtrl', function($scope, Realm, realm, $htt
         if (!$scope.policy) {
             $scope.policy = [];
         }
+        if (policy.name === 'regexPattern') {
+            for (var i in $scope.allPolicies) {
+                var p = $scope.allPolicies[i];
+                if (p.name === 'regexPattern') {
+                    $scope.allPolicies[i] = { name: 'regexPattern', value: '' };
+                }
+            }
+        }
         $scope.policy.push(policy);
     }
 

forms/common-themes/src/main/resources/theme/base/admin/resources/js/services.js

@@ -1063,7 +1063,7 @@ module.factory('PasswordPolicy', function() {
         upperCase:      	"Minimal number (integer type) of uppercase characters in password. Default value is 1.",
         specialChars:   	"Minimal number (integer type) of special characters in password. Default value is 1.",
         notUsername:    	"Block passwords that are equal to the username",
-        regexPatterns:  	"Block passwords that do not match all of the regex patterns (string type).",
+        regexPattern:  	    "Block passwords that do not match the regex pattern (string type).",
         passwordHistory:  	"Block passwords that are equal to previous passwords. Default value is 3.",
         forceExpiredPasswordChange:  	"Force password change when password credential is expired. Default value is 365 days."
     }
@@ -1076,7 +1076,7 @@ module.factory('PasswordPolicy', function() {
         { name: 'upperCase', value: 1 },
         { name: 'specialChars', value: 1 },
         { name: 'notUsername', value: 1 },
-        { name: 'regexPatterns', value: ''},
+        { name: 'regexPattern', value: ''},
         { name: 'passwordHistory', value: 3 },
         { name: 'forceExpiredPasswordChange', value: 365 }
     ];
@@ -1094,7 +1094,7 @@ module.factory('PasswordPolicy', function() {
         for (var i = 0; i < policyArray.length; i ++){
             var policyToken = policyArray[i];
             
-            if(policyToken.indexOf('regexPatterns') === 0) {
+            if(policyToken.indexOf('regexPattern') === 0) {
             	re = /(\w+)\((.*)\)/;
             	policyEntry = re.exec(policyToken);
                 if (null !== policyEntry) {
@@ -1134,6 +1134,25 @@ module.factory('PasswordPolicy', function() {
     return p;
 });
 
+module.filter('removeSelectedPolicies', function() {
+    return function(policies, selectedPolicies) {
+        var result = [];
+        for(var i in policies) {
+            var policy = policies[i];
+            var policyAvailable = true;
+            for(var j in selectedPolicies) {
+                if(policy.name === selectedPolicies[j].name && policy.name !== 'regexPattern') {
+                    policyAvailable = false;
+                }
+            }
+            if(policyAvailable) {
+                result.push(policy);
+            }
+        }
+        return result;
+    }
+});
+
 module.factory('IdentityProvider', function($resource) {
     return $resource(authUrl + '/admin/realms/:realm/identity-provider/instances/:alias', {
         realm : '@realm',

forms/common-themes/src/main/resources/theme/base/admin/resources/partials/password-policy.html

@@ -7,12 +7,12 @@
         <table class="table table-striped table-bordered">
             <caption class="hidden">Table of Password Policies</caption>
             <thead>
-            <tr ng-show="(allPolicies|remove:policy:'name').length > 0">
+            <tr ng-show="(allPolicies|removeSelectedPolicies:policy).length > 0">
                 <th colspan="5" class="kc-table-actions">
                     <div class="pull-right">
                         <div>
                             <select class="form-control" ng-model="selectedPolicy"
-                                    ng-options="(p.name|capitalize) for p in (allPolicies|remove:policy:'name')"
+                                    ng-options="(p.name|capitalize) for p in (allPolicies|removeSelectedPolicies:policy)"
                                     data-ng-change="addPolicy(selectedPolicy); selectedPolicy = null">
                                 <option value="" disabled selected>Add policy...</option>
                             </select>
@@ -51,4 +51,4 @@
 </div>
 
 
-<kc-menu></kc-menu>
+<kc-menu></kc-menu>

model/api/src/main/java/org/keycloak/models/PasswordPolicy.java

@@ -46,42 +46,37 @@ public class PasswordPolicy implements Serializable {
             policy = policy.trim();
 
             String name;
-            String[] args = null;
+            String arg = null;
 
             int i = policy.indexOf('(');
             if (i == -1) {
                 name = policy.trim();
             } else {
                 name = policy.substring(0, i).trim();
-                args = policy.substring(i + 1, policy.length() - 1).split(",");
+                arg = policy.substring(i + 1, policy.length() - 1);
-                for (int j = 0; j < args.length; j++) {
-                    args[j] = args[j].trim();
-                }
             }
 
             if (name.equals(Length.NAME)) {
-                list.add(new Length(args));
+                list.add(new Length(arg));
             } else if (name.equals(Digits.NAME)) {
-                list.add(new Digits(args));
+                list.add(new Digits(arg));
             } else if (name.equals(LowerCase.NAME)) {
-                list.add(new LowerCase(args));
+                list.add(new LowerCase(arg));
             } else if (name.equals(UpperCase.NAME)) {
-                list.add(new UpperCase(args));
+                list.add(new UpperCase(arg));
             } else if (name.equals(SpecialChars.NAME)) {
-                list.add(new SpecialChars(args));
+                list.add(new SpecialChars(arg));
             } else if (name.equals(NotUsername.NAME)) {
-                list.add(new NotUsername(args));
+                list.add(new NotUsername(arg));
             } else if (name.equals(HashIterations.NAME)) {
-                list.add(new HashIterations(args));
+                list.add(new HashIterations(arg));
             } else if (name.equals(RegexPatterns.NAME)) {
-                for (String regexPattern : args) {
+                Pattern.compile(arg);
-                    Pattern.compile(regexPattern);
+                list.add(new RegexPatterns(arg));
-                }
-                list.add(new RegexPatterns(args));
             } else if (name.equals(PasswordHistory.NAME)) {
-                list.add(new PasswordHistory(args));
+                list.add(new PasswordHistory(arg));
             } else if (name.equals(ForceExpiredPasswordChange.NAME)) {
-                list.add(new ForceExpiredPasswordChange(args));
+                list.add(new ForceExpiredPasswordChange(arg));
             }
         }
         return list;
@@ -182,11 +177,10 @@ public class PasswordPolicy implements Serializable {
         private static final String NAME = "hashIterations";
         private int iterations;
 
-        public HashIterations(String[] args) {
+        public HashIterations(String arg) {
-            iterations = intArg(NAME, 1, args);
+            iterations = intArg(NAME, 1, arg);
         }
         
-
         @Override
         public Error validate(String user, String password) {
             return null;
@@ -201,7 +195,7 @@ public class PasswordPolicy implements Serializable {
     private static class NotUsername implements Policy {
         private static final String NAME = "notUsername";
 
-        public NotUsername(String[] args) {
+        public NotUsername(String arg) {
         }
 
         @Override
@@ -219,8 +213,9 @@ public class PasswordPolicy implements Serializable {
         private static final String NAME = "length";
         private int min;
 
-        public Length(String[] args) {
+        public Length(String arg)
-            min = intArg(NAME, 8, args);
+        {
+            min = intArg(NAME, 8, arg);
         }
         
 
@@ -239,8 +234,9 @@ public class PasswordPolicy implements Serializable {
         private static final String NAME = "digits";
         private int min;
 
-        public Digits(String[] args) {
+        public Digits(String arg)
-            min = intArg(NAME, 1, args);
+        {
+            min = intArg(NAME, 1, arg);
         }
         
 
@@ -265,8 +261,9 @@ public class PasswordPolicy implements Serializable {
         private static final String NAME = "lowerCase";
         private int min;
 
-        public LowerCase(String[] args) {
+        public LowerCase(String arg)
-            min = intArg(NAME, 1, args);
+        {
+            min = intArg(NAME, 1, arg);
         }
         
         @Override
@@ -290,8 +287,8 @@ public class PasswordPolicy implements Serializable {
         private static final String NAME = "upperCase";
         private int min;
 
-        public UpperCase(String[] args) {
+        public UpperCase(String arg) {
-            min = intArg(NAME, 1, args);
+            min = intArg(NAME, 1, arg);
         }
 
         @Override
@@ -315,8 +312,9 @@ public class PasswordPolicy implements Serializable {
         private static final String NAME = "specialChars";
         private int min;
 
-        public SpecialChars(String[] args) {
+        public SpecialChars(String arg)
-            min = intArg(NAME, 1, args);
+        {
+            min = intArg(NAME, 1, arg);
         }
         
         @Override
@@ -337,23 +335,20 @@ public class PasswordPolicy implements Serializable {
     }
 
     private static class RegexPatterns implements Policy {
-        private static final String NAME = "regexPatterns";
+        private static final String NAME = "regexPattern";
-        private String regexPatterns[];
+        private String regexPattern;
 
-        public RegexPatterns(String[] args) {
+        public RegexPatterns(String arg)
-            regexPatterns = args;
+        {
+            regexPattern = arg;
         }
 
         @Override
         public Error validate(String username, String password) {
-            Pattern pattern = null;
+            Pattern pattern = Pattern.compile(regexPattern);
-            Matcher matcher = null;
+            Matcher matcher = pattern.matcher(password);
-            for (String regexPattern : regexPatterns) {
+            if (!matcher.matches()) {
-                pattern = Pattern.compile(regexPattern);
+                return new Error(INVALID_PASSWORD_REGEX_PATTERN, (Object) regexPattern);
-                matcher = pattern.matcher(password);
-                if (!matcher.matches()) {
-                    return new Error(INVALID_PASSWORD_REGEX_PATTERN, (Object) regexPatterns);
-                }
             }
             return null;
         }
@@ -368,8 +363,9 @@ public class PasswordPolicy implements Serializable {
         private static final String NAME = "passwordHistory";
         private int passwordHistoryPolicyValue;
 
-        public PasswordHistory(String[] args) {
+        public PasswordHistory(String arg)
-            passwordHistoryPolicyValue = intArg(NAME, 3, args);
+        {
+            passwordHistoryPolicyValue = intArg(NAME, 3, arg);
         }
         
         @Override
@@ -442,8 +438,8 @@ public class PasswordPolicy implements Serializable {
         private static final String NAME = "forceExpiredPasswordChange";
         private int daysToExpirePassword;
 
-        public ForceExpiredPasswordChange(String[] args) {
+        public ForceExpiredPasswordChange(String arg) {
-            daysToExpirePassword = intArg(NAME, 365, args);
+            daysToExpirePassword = intArg(NAME, 365, arg);
         }
 
         @Override
@@ -457,13 +453,11 @@ public class PasswordPolicy implements Serializable {
         }
     }
     
-    private static int intArg(String policy, int defaultValue, String... args) {
+    private static int intArg(String policy, int defaultValue, String arg) {
-        if (args == null || args.length == 0) {
+        if (arg == null) {
             return defaultValue;
-        } else if (args.length == 1) {
-            return Integer.parseInt(args[0]);
         } else {
-            throw new IllegalArgumentException("Invalid arguments to " + policy + ", expect no argument or single integer");
+            return Integer.parseInt(arg);
         }
     }
 

model/api/src/test/java/org/keycloak/models/PasswordPolicyTest.java

@@ -88,41 +88,41 @@ public class PasswordPolicyTest {
     public void testRegexPatterns() {
         PasswordPolicy policy = null;
         try {
-            policy = new PasswordPolicy("regexPatterns");
+            policy = new PasswordPolicy("regexPattern");
-            fail("Expected NullPointerEXception: Regex Pattern cannot be null.");
+            fail("Expected NullPointerException: Regex Pattern cannot be null.");
         } catch (NullPointerException e) {
             // Expected NPE as regex pattern is null.
         }
         
         try {
-            policy = new PasswordPolicy("regexPatterns(*)");
+            policy = new PasswordPolicy("regexPattern(*)");
             fail("Expected PatternSyntaxException: Regex Pattern cannot be null.");
         } catch (PatternSyntaxException e) {
             // Expected PSE as regex pattern(or any of its token) is not quantifiable.
         }
         
         try {
-            policy = new PasswordPolicy("regexPatterns(*,**)");
+            policy = new PasswordPolicy("regexPattern(*,**)");
             fail("Expected PatternSyntaxException: Regex Pattern cannot be null.");
         } catch (PatternSyntaxException e) {
             // Expected PSE as regex pattern(or any of its token) is not quantifiable.
         }
         
         //Fails to match one of the regex pattern
-        policy = new PasswordPolicy("regexPatterns(jdoe,j*d)");
+        policy = new PasswordPolicy("regexPattern(jdoe) and regexPattern(j*d)");
         Assert.assertEquals("invalidPasswordRegexPatternMessage", policy.validate("jdoe", "jdoe").getMessage());
         
         ////Fails to match all of the regex patterns
-        policy = new PasswordPolicy("regexPatterns(j*p,j*d,adoe)");
+        policy = new PasswordPolicy("regexPattern(j*p) and regexPattern(j*d) and regexPattern(adoe)");
         Assert.assertEquals("invalidPasswordRegexPatternMessage", policy.validate("jdoe", "jdoe").getMessage());
         
-        policy = new PasswordPolicy("regexPatterns([a-z][a-z][a-z][a-z][0-9])");
+        policy = new PasswordPolicy("regexPattern([a-z][a-z][a-z][a-z][0-9])");
         Assert.assertEquals("invalidPasswordRegexPatternMessage", policy.validate("jdoe", "jdoe").getMessage());
         
-        policy = new PasswordPolicy("regexPatterns(jdoe)");
+        policy = new PasswordPolicy("regexPattern(jdoe)");
         Assert.assertNull(policy.validate("jdoe", "jdoe"));
         
-        policy = new PasswordPolicy("regexPatterns([a-z][a-z][a-z][a-z][0-9])");
+        policy = new PasswordPolicy("regexPattern([a-z][a-z][a-z][a-z][0-9])");
         Assert.assertNull(policy.validate("jdoe", "jdoe0"));
     }