Merge pull request #47 from pwm-project/disallowed-attribute-thresholds

Added size restrictions to "Disallowed Attributes"

src/main/java/password/pwm/config/profile/PwmPasswordPolicy.java

@@ -39,6 +39,7 @@ import password.pwm.config.UserPermission;
 import password.pwm.config.option.ADPolicyComplexity;
 import password.pwm.health.HealthMessage;
 import password.pwm.health.HealthRecord;
+import password.pwm.util.Helper;
 import password.pwm.util.logging.PwmLogger;
 import password.pwm.util.macro.MacroMachine;
 
@@ -289,6 +290,8 @@ public class PwmPasswordPolicy implements Profile,Serializable {
 // -------------------------- INNER CLASSES --------------------------
 
     public static class RuleHelper {
+        public enum Flag { KeepThresholds }
+
         private final PwmPasswordPolicy passwordPolicy;
         private final PasswordRuleHelper chaiRuleHelper;
 
@@ -301,8 +304,30 @@ public class PwmPasswordPolicy implements Profile,Serializable {
             return chaiRuleHelper.getDisallowedValues();
         }
 
-        public List<String> getDisallowedAttributes() {
-            return chaiRuleHelper.getDisallowedAttributes();
+        public List<String> getDisallowedAttributes(Flag ... flags) {
+            final List<String> disallowedAttributes = chaiRuleHelper.getDisallowedAttributes();
+
+            if (Helper.enumArrayContainsValue(flags, Flag.KeepThresholds)) {
+                return disallowedAttributes;
+            } else {
+                // Strip off any thresholds from attribute (specified as: "attributeName:N", where N is a numeric value).
+                final List<String> strippedDisallowedAttributes = new ArrayList<String>();
+
+                if (disallowedAttributes != null) {
+                    for (final String disallowedAttribute : disallowedAttributes) {
+                        if (disallowedAttribute != null) {
+                            final int indexOfColon = disallowedAttribute.indexOf(':');
+                            if (indexOfColon > 0) {
+                                strippedDisallowedAttributes.add(disallowedAttribute.substring(0, indexOfColon));
+                            } else {
+                                strippedDisallowedAttributes.add(disallowedAttribute);
+                            }
+                        }
+                    }
+                }
+
+                return strippedDisallowedAttributes;
+            }
         }
 
         public List<Pattern> getRegExMatch() {

src/main/java/password/pwm/util/PwmPasswordRuleValidator.java

@@ -38,6 +38,7 @@ import password.pwm.config.Configuration;
 import password.pwm.config.PwmSetting;
 import password.pwm.config.option.ADPolicyComplexity;
 import password.pwm.config.profile.PwmPasswordPolicy;
+import password.pwm.config.profile.PwmPasswordPolicy.RuleHelper;
 import password.pwm.config.profile.PwmPasswordRule;
 import password.pwm.error.*;
 import password.pwm.http.PwmSession;
@@ -51,6 +52,9 @@ import password.pwm.ws.client.rest.RestClientHelper;
 import java.util.*;
 import java.util.regex.Pattern;
 
+import org.apache.commons.lang3.StringUtils;
+import org.apache.commons.lang3.math.NumberUtils;
+
 public class PwmPasswordRuleValidator {
 
     private static final PwmLogger LOGGER = PwmLogger.forClass(PwmPasswordRuleValidator.class);
@@ -237,23 +241,19 @@ public class PwmPasswordRuleValidator {
 
         // check disallowed attributes.
         if (!policy.getRuleHelper().getDisallowedAttributes().isEmpty()) {
-            final List paramConfigs = policy.getRuleHelper().getDisallowedAttributes();
+            final List<String> paramConfigs = policy.getRuleHelper().getDisallowedAttributes(RuleHelper.Flag.KeepThresholds);
             if (uiBean != null) {
                 final Map<String,String> userValues = uiBean.getCachedPasswordRuleAttributes();
-                final String lcasePwd = passwordString.toLowerCase();
-                for (final Object paramConfig : paramConfigs) {
-                    final String attr = (String) paramConfig;
-                    final String userValue = userValues.get(attr) == null ? "" : userValues.get(attr).toLowerCase();
-
-                    // if the password is greater then 1 char and the value is contained within it then disallow
-                    if (userValue.length() > 1 && lcasePwd.contains(userValue)) {
-                        LOGGER.trace("password rejected, same as user attr " + attr);
-                        errorList.add(new ErrorInformation(PwmError.PASSWORD_SAMEASATTR));
-                    }
 
-                    // if the password is 1 char and the value is the same then disallow
-                    if (lcasePwd.equalsIgnoreCase(userValue)) {
-                        LOGGER.trace("password rejected, same as user attr " + attr);
+                for (final String paramConfig : paramConfigs) {
+                    final String[] parts = paramConfig.split(":");
+
+                    final String attrName = parts[0];
+                    final String disallowedValue = StringUtils.defaultString(userValues.get(attrName));
+                    final int threshold = parts.length > 1 ? NumberUtils.toInt(parts[1]) : 0;
+
+                    if (containsDisallowedValue(passwordString, disallowedValue, threshold)) {
+                        LOGGER.trace("password rejected, same as user attr " + attrName);
                         errorList.add(new ErrorInformation(PwmError.PASSWORD_SAMEASATTR));
                     }
                 }
@@ -371,7 +371,25 @@ public class PwmPasswordRuleValidator {
         return errorList;
     }
 
+    static boolean containsDisallowedValue(final String password, final String disallowedValue, final int threshold) {
+        if (StringUtils.isNotBlank(disallowedValue)) {
+            if (threshold > 0) {
+                if (disallowedValue.length() >= threshold) {
+                    final String[] disallowedValueChunks = StringUtil.createStringChunks(disallowedValue, threshold);
+                    for (final String chunk : disallowedValueChunks) {
+                        if (StringUtils.containsIgnoreCase(password, chunk)) {
+                            return true;
+                        }
+                    }
+                }
+            } else {
+                // No threshold?  Then the password can't contain the whole disallowed value
+                return StringUtils.containsIgnoreCase(password, disallowedValue);
+            }
+        }
 
+        return false;
+    }
 
     /**
      * Check a supplied password for it's validity according to AD complexity rules.

src/main/java/password/pwm/util/StringUtil.java

@@ -25,6 +25,7 @@ package password.pwm.util;
 import org.apache.commons.codec.binary.Base32;
 import org.apache.commons.lang3.StringEscapeUtils;
 import org.apache.commons.lang3.StringUtils;
+
 import password.pwm.PwmConstants;
 import password.pwm.util.logging.PwmLogger;
 
@@ -266,4 +267,19 @@ public abstract class StringUtil {
         final String[] splitValues = input.trim().split("\\s+");
         return Arrays.asList(splitValues);
     }
+
+    public static String[] createStringChunks(String str, int size) {
+        if (size <= 0 || str == null || str.length() <= size) {
+            return new String[] { str };
+        }
+
+        int numOfChunks = str.length() - size + 1;
+        Set<String> chunks = new HashSet<>(numOfChunks);
+
+        for (int i=0; i<numOfChunks; i++) {
+            chunks.add(StringUtils.substring(str, i, i+size));
+        }
+
+        return chunks.toArray(new String[numOfChunks]);
+    }
 }

src/main/resources/password/pwm/config/PwmSetting.xml

@@ -1113,7 +1113,7 @@
     </setting>
     <setting hidden="false" key="password.policy.disallowedAttributes" level="2">
         <ldapPermission actor="proxy" access="read"/>
-        <regex>^[a-zA-Z][a-zA-Z0-9-]*$</regex>
+        <regex>^[a-zA-Z][a-zA-Z0-9-]*(:[0-9]+)*$</regex>
         <default>
             <value><![CDATA[cn]]></value>
             <value><![CDATA[givenName]]></value>

src/main/resources/password/pwm/i18n/ConfigEditor.properties

@@ -453,7 +453,7 @@ Setting_Description_password.policy.charGroup.minimumMatch=
 Setting_Description_password.policy.charGroup.regExValues=
 Setting_Description_password.policy.checkWordlist=Check the password against the configured Wordlist.
 Setting_Description_password.policy.disallowCurrent=Prohibit current password from being used as new password.  Note that this can only be enforced if the login method permits the user's password to be known.
-Setting_Description_password.policy.disallowedAttributes=A list of attributes not allowed to be used as passwords.  For a given user, the values will be read and will not be permitted to be used as part of the password value.  This check is case insensitive.
+Setting_Description_password.policy.disallowedAttributes=A list of attributes not allowed to be used as passwords.  For a given user, the values will be read and will not be permitted to be used as part of the password value.  This check is case insensitive.  Note: specifying a number after the attribute name will restrict how many consecutive characters in the value are disallowed (i.e. "Language:4" means the password can't contain: "Engl", "ngli", "glis", or "lish", for English speaking users).
 Setting_Description_password.policy.disallowedValues=A case insensitive list of values not allowed to be used as passwords.
 Setting_Description_password.policy.maximumAlpha=Maximum amount of alphabetic characters required.  A value of zero disables this check.
 Setting_Description_password.policy.maximumConsecutive=Maximum amount of characters in a sequence such as <i>0123456789</i> or <i>abcdefghijk</i>.  More specifically character sequence is defined by unicode character order of each character after the entire value is converted to lowercase.   A value of 0 disables this check.

src/test/java/password/pwm/util/PwmPasswordRuleValidatorTest.java

@@ -0,0 +1,62 @@
+package password.pwm.util;
+
+import static org.assertj.core.api.Assertions.*;
+import static password.pwm.util.PwmPasswordRuleValidator.*;
+
+import org.junit.Test;
+
+public class PwmPasswordRuleValidatorTest {
+    @Test
+    public void testContainsDisallowedValue() throws Exception {
+        // containsDisallowedValue([new password], [disallowed value], [character match threshold])
+
+        assertThat(containsDisallowedValue("n", "n", 0)).isTrue();
+        assertThat(containsDisallowedValue("N", "n", 0)).isTrue();
+        assertThat(containsDisallowedValue("n", "N", 0)).isTrue();
+        assertThat(containsDisallowedValue("novell", "N", 0)).isTrue();
+        assertThat(containsDisallowedValue("novell", "o", 0)).isTrue();
+        assertThat(containsDisallowedValue("novell", "V", 0)).isTrue();
+        assertThat(containsDisallowedValue("novell", "e", 0)).isTrue();
+        assertThat(containsDisallowedValue("novell", "l", 0)).isTrue();
+        assertThat(containsDisallowedValue("n", "n", 10)).isFalse(); // TODO: Need to verify this with Jason
+
+        assertThat(containsDisallowedValue("novell", "novell", 0)).isTrue();
+        assertThat(containsDisallowedValue("novell", "novell", 5)).isTrue();
+        assertThat(containsDisallowedValue("novell", "novell", 6)).isTrue();
+        assertThat(containsDisallowedValue("novell", "novell", 7)).isFalse(); // TODO: Need to verify this with Jason
+        assertThat(containsDisallowedValue("novell", "foo", 0)).isFalse();
+        assertThat(containsDisallowedValue("novell", "", 0)).isFalse();
+
+        assertThat(containsDisallowedValue("love", "novell", 1)).isTrue();
+        assertThat(containsDisallowedValue("love", "novell", 2)).isTrue();
+        assertThat(containsDisallowedValue("love", "novell", 3)).isTrue();
+        assertThat(containsDisallowedValue("love", "novell", 4)).isFalse();
+        assertThat(containsDisallowedValue("love", "novell", 5)).isFalse();
+        assertThat(containsDisallowedValue("love", "novell", 6)).isFalse();
+
+        // Case shouldn't matter
+        assertThat(containsDisallowedValue("LOVE", "novell", 1)).isTrue();
+        assertThat(containsDisallowedValue("LOVE", "novell", 2)).isTrue();
+        assertThat(containsDisallowedValue("LOVE", "novell", 3)).isTrue();
+        assertThat(containsDisallowedValue("LOVE", "novell", 4)).isFalse();
+        assertThat(containsDisallowedValue("LOVE", "novell", 5)).isFalse();
+        assertThat(containsDisallowedValue("LOVE", "novell", 6)).isFalse();
+        assertThat(containsDisallowedValue("love", "NOVELL", 1)).isTrue();
+        assertThat(containsDisallowedValue("love", "NOVELL", 2)).isTrue();
+        assertThat(containsDisallowedValue("love", "NOVELL", 3)).isTrue();
+        assertThat(containsDisallowedValue("love", "NOVELL", 4)).isFalse();
+        assertThat(containsDisallowedValue("love", "NOVELL", 5)).isFalse();
+        assertThat(containsDisallowedValue("love", "NOVELL", 6)).isFalse();
+
+        // Play around the threshold boundaries
+        assertThat(containsDisallowedValue("foo-nove-bar", "novell", 4)).isTrue();
+        assertThat(containsDisallowedValue("foo-ovel-bar", "novell", 4)).isTrue();
+        assertThat(containsDisallowedValue("foo-vell-bar", "novell", 4)).isTrue();
+        assertThat(containsDisallowedValue("foo-nove", "novell", 4)).isTrue();
+        assertThat(containsDisallowedValue("foo-ovel", "novell", 4)).isTrue();
+        assertThat(containsDisallowedValue("foo-vell", "novell", 4)).isTrue();
+        assertThat(containsDisallowedValue("nove-bar", "novell", 4)).isTrue();
+        assertThat(containsDisallowedValue("ovel-bar", "novell", 4)).isTrue();
+        assertThat(containsDisallowedValue("vell-bar", "novell", 4)).isTrue();
+    }
+}