[SECURITY-786]

core/src/main/java/hudson/security/HudsonPrivateSecurityRealm.java

@@ -64,6 +64,7 @@ import org.kohsuke.stapler.interceptor.RequirePOST;
 import org.mindrot.jbcrypt.BCrypt;
 import org.springframework.dao.DataAccessException;
 
+import javax.annotation.Nonnull;
 import javax.servlet.Filter;
 import javax.servlet.FilterChain;
 import javax.servlet.FilterConfig;
@@ -95,6 +96,15 @@ import org.kohsuke.accmod.restrictions.NoExternalUse;
  * @author Kohsuke Kawaguchi
  */
 public class HudsonPrivateSecurityRealm extends AbstractPasswordBasedSecurityRealm implements ModelObject, AccessControlled {
+    private static /* not final */ String ID_REGEX = System.getProperty(HudsonPrivateSecurityRealm.class.getName() + ".ID_REGEX");
+    
+    /**
+     * Default REGEX for the user ID check in case the ID_REGEX is not set
+     * It allows A-Za-z0-9 + "_-"
+     * in Java {@code \w} is equivalent to {@code [A-Za-z0-9_]} (take care of "_")
+     */
+    private static final String DEFAULT_ID_REGEX = "^[\\w-]+$";
+    
     /**
      * If true, sign up is not allowed.
      * <p>
@@ -338,6 +348,12 @@ public class HudsonPrivateSecurityRealm extends AbstractPasswordBasedSecurityRea
 
         if(si.username==null || si.username.length()==0)
             si.errorMessage = Messages.HudsonPrivateSecurityRealm_CreateAccount_UserNameRequired();
+        else if(!containsOnlyAcceptableCharacters(si.username))
+            if(ID_REGEX == null){
+                si.errorMessage = Messages.HudsonPrivateSecurityRealm_CreateAccount_UserNameInvalidCharacters();
+            }else{
+                si.errorMessage = Messages.HudsonPrivateSecurityRealm_CreateAccount_UserNameInvalidCharactersCustom(ID_REGEX);
+            }
         else {
             // do not create the user - we just want to check if the user already exists but is not a "login" user.
             User user = User.getById(si.username, false); 
@@ -385,6 +401,14 @@ public class HudsonPrivateSecurityRealm extends AbstractPasswordBasedSecurityRea
         return user;
     }
     
+    private boolean containsOnlyAcceptableCharacters(@Nonnull String value){
+        if(ID_REGEX == null){
+            return value.matches(DEFAULT_ID_REGEX);
+        }else{
+            return value.matches(ID_REGEX);
+        }
+    }
+    
     @Restricted(NoExternalUse.class)
     public boolean isMailerPluginPresent() {
         try {

core/src/main/resources/hudson/security/Messages.properties

@@ -37,6 +37,8 @@ HudsonPrivateSecurityRealm.CreateAccount.TextNotMatchWordInImage=Text didn't mat
 HudsonPrivateSecurityRealm.CreateAccount.PasswordNotMatch=Password didn't match
 HudsonPrivateSecurityRealm.CreateAccount.PasswordRequired=Password is required
 HudsonPrivateSecurityRealm.CreateAccount.UserNameRequired=User name is required
+HudsonPrivateSecurityRealm.CreateAccount.UserNameInvalidCharacters=User name must only contain alphanumeric characters, underscore and dash
+HudsonPrivateSecurityRealm.CreateAccount.UserNameInvalidCharactersCustom=User name must match the following expression: {0}
 HudsonPrivateSecurityRealm.CreateAccount.InvalidEmailAddress=Invalid e-mail address
 HudsonPrivateSecurityRealm.CreateAccount.UserNameAlreadyTaken=User name is already taken
 

test/src/test/java/hudson/security/HudsonPrivateSecurityRealmTest.java

@@ -2,15 +2,21 @@ package hudson.security;
 
 import static hudson.security.HudsonPrivateSecurityRealm.CLASSIC;
 import static hudson.security.HudsonPrivateSecurityRealm.PASSWORD_ENCODER;
+import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.not;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.assertThat;
 import static org.hamcrest.Matchers.is;
 import static org.hamcrest.xml.HasXPath.hasXPath;
 
 import java.io.UnsupportedEncodingException;
+import java.lang.reflect.Field;
 
+import hudson.security.pages.SignupPage;
+import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 import org.jvnet.hudson.test.Issue;
@@ -34,6 +40,13 @@ public class HudsonPrivateSecurityRealmTest {
     @Rule
     public JenkinsRule j = new JenkinsRule();
 
+    @Before
+    public void setup() throws Exception {
+        Field field = HudsonPrivateSecurityRealm.class.getDeclaredField("ID_REGEX");
+        field.setAccessible(true);
+        field.set(null, null);
+    }
+
     /**
      * Tests the data compatibility with Hudson before 1.283.
      * Starting 1.283, passwords are now stored hashed.
@@ -170,5 +183,119 @@ public class HudsonPrivateSecurityRealmTest {
         String authHeader = "Basic " + auth;
         return authHeader;
     }
-
+    
+    @Issue("SECURITY-786")
+    @Test
+    public void controlCharacterAreNoMoreValid() throws Exception {
+        HudsonPrivateSecurityRealm securityRealm = new HudsonPrivateSecurityRealm(true, false, null);
+        j.jenkins.setSecurityRealm(securityRealm);
+        
+        String password = "testPwd";
+        String email = "test@test.com";
+        int i = 0;
+        
+        // regular case = only accepting a-zA-Z0-9 + "-_"
+        checkUserCanBeCreatedWith(securityRealm, "test" + i, password, "Test" + i, email);
+        assertNotNull(User.getById("test" + i, false));
+        i++;
+        checkUserCanBeCreatedWith(securityRealm, "te-st_123" + i, password, "Test" + i, email);
+        assertNotNull(User.getById("te-st_123" + i, false));
+        i++;
+        {// user id that contains invalid characters
+            checkUserCannotBeCreatedWith(securityRealm, "test " + i, password, "Test" + i, email);
+            i++;
+            checkUserCannotBeCreatedWith(securityRealm, "te@st" + i, password, "Test" + i, email);
+            i++;
+            checkUserCannotBeCreatedWith(securityRealm, "test.com" + i, password, "Test" + i, email);
+            i++;
+            checkUserCannotBeCreatedWith(securityRealm, "test,com" + i, password, "Test" + i, email);
+            i++;
+            checkUserCannotBeCreatedWith(securityRealm, "test,com" + i, password, "Test" + i, email);
+            i++;
+            checkUserCannotBeCreatedWith(securityRealm, "testécom" + i, password, "Test" + i, email);
+            i++;
+            checkUserCannotBeCreatedWith(securityRealm, "Stargåte" + i, password, "Test" + i, email);
+            i++;
+            checkUserCannotBeCreatedWith(securityRealm, "te\u0000st" + i, password, "Test" + i, email);
+            i++;
+        }
+    }
+    
+    @Issue("SECURITY-786")
+    @Test
+    public void controlCharacterAreNoMoreValid_CustomRegex() throws Exception {
+        HudsonPrivateSecurityRealm securityRealm = new HudsonPrivateSecurityRealm(true, false, null);
+        j.jenkins.setSecurityRealm(securityRealm);
+        
+        String currentRegex = "^[A-Z]+[0-9]*$";
+        
+        Field field = HudsonPrivateSecurityRealm.class.getDeclaredField("ID_REGEX");
+        field.setAccessible(true);
+        field.set(null, currentRegex);
+        
+        String password = "testPwd";
+        String email = "test@test.com";
+        int i = 0;
+        
+        // regular case = only accepting a-zA-Z0-9 + "-_"
+        checkUserCanBeCreatedWith(securityRealm, "TEST" + i, password, "Test" + i, email);
+        assertNotNull(User.getById("TEST" + i, false));
+        i++;
+        checkUserCanBeCreatedWith(securityRealm, "TEST123" + i, password, "Test" + i, email);
+        assertNotNull(User.getById("TEST123" + i, false));
+        i++;
+        {// user id that do not follow custom regex
+            checkUserCannotBeCreatedWith_custom(securityRealm, "test " + i, password, "Test" + i, email, currentRegex);
+            i++;
+            checkUserCannotBeCreatedWith_custom(securityRealm, "@" + i, password, "Test" + i, email, currentRegex);
+            i++;
+            checkUserCannotBeCreatedWith_custom(securityRealm, "T2A" + i, password, "Test" + i, email, currentRegex);
+            i++;
+        }
+        { // we can even change regex on the fly
+            currentRegex = "^[0-9]*$";
+            field.set(null, currentRegex);
+    
+            checkUserCanBeCreatedWith(securityRealm, "125213" + i, password, "Test" + i, email);
+            assertNotNull(User.getById("125213" + i, false));
+            i++;
+            checkUserCannotBeCreatedWith_custom(securityRealm, "TEST12" + i, password, "Test" + i, email, currentRegex);
+            i++;
+        }
+    }
+    
+    private void checkUserCanBeCreatedWith(HudsonPrivateSecurityRealm securityRealm, String id, String password, String fullName, String email) throws Exception {
+        JenkinsRule.WebClient wc = j.createWebClient();
+        SignupPage signup = new SignupPage(wc.goTo("signup"));
+        signup.enterUsername(id);
+        signup.enterPassword(password);
+        signup.enterFullName(fullName);
+        signup.enterEmail(email);
+        HtmlPage success = signup.submit(j);
+        assertThat(success.getElementById("main-panel").getTextContent(), containsString("Success"));
+    }
+    
+    private void checkUserCannotBeCreatedWith(HudsonPrivateSecurityRealm securityRealm, String id, String password, String fullName, String email) throws Exception {
+        JenkinsRule.WebClient wc = j.createWebClient();
+        SignupPage signup = new SignupPage(wc.goTo("signup"));
+        signup.enterUsername(id);
+        signup.enterPassword(password);
+        signup.enterFullName(fullName);
+        signup.enterEmail(email);
+        HtmlPage success = signup.submit(j);
+        assertThat(success.getElementById("main-panel").getTextContent(), not(containsString("Success")));
+        assertThat(success.getElementById("main-panel").getTextContent(), containsString(Messages.HudsonPrivateSecurityRealm_CreateAccount_UserNameInvalidCharacters()));
+    }
+    
+    private void checkUserCannotBeCreatedWith_custom(HudsonPrivateSecurityRealm securityRealm, String id, String password, String fullName, String email, String regex) throws Exception {
+        JenkinsRule.WebClient wc = j.createWebClient();
+        SignupPage signup = new SignupPage(wc.goTo("signup"));
+        signup.enterUsername(id);
+        signup.enterPassword(password);
+        signup.enterFullName(fullName);
+        signup.enterEmail(email);
+        HtmlPage success = signup.submit(j);
+        assertThat(success.getElementById("main-panel").getTextContent(), not(containsString("Success")));
+        assertThat(success.getElementById("main-panel").getTextContent(), containsString(regex));
+    }
 }