Skip to content

Commit

Permalink
[SECURITY-786]
Browse files Browse the repository at this point in the history
  • Loading branch information
@Wadeck @daniel-beck
Wadeck authored and daniel-beck committed Apr 25, 2018
1 parent 6eea1e9 commit de7aaab
Show file tree
Hide file tree
Showing 3 changed files with 154 additions and 1 deletion.
24 changes: 24 additions & 0 deletions core/src/main/java/hudson/security/HudsonPrivateSecurityRealm.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@
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;
Expand Down Expand Up @@ -95,6 +96,15 @@
* @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>
Expand Down Expand Up @@ -338,6 +348,12 @@ private User createAccount(StaplerRequest req, StaplerResponse rsp, boolean self

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);
Expand Down Expand Up @@ -385,6 +401,14 @@ private User createAccount(StaplerRequest req, StaplerResponse rsp, boolean self
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 {
Expand Down
2 changes: 2 additions & 0 deletions core/src/main/resources/hudson/security/Messages.properties
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
129 changes: 128 additions & 1 deletion test/src/test/java/hudson/security/HudsonPrivateSecurityRealmTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,21 @@

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;
Expand All @@ -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.
Expand Down Expand Up @@ -170,5 +183,119 @@ private static final String basicHeader(String user, String pass) throws Unsuppo
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));
}
}

0 comments on commit de7aaab

Please sign in to comment.