diff --git a/core/src/main/java/hudson/model/AbstractItem.java b/core/src/main/java/hudson/model/AbstractItem.java index 3524444fea..5f298eb3d5 100644 --- a/core/src/main/java/hudson/model/AbstractItem.java +++ b/core/src/main/java/hudson/model/AbstractItem.java @@ -45,6 +45,8 @@ import jenkins.model.DirectlyModifiableTopLevelItemGroup; import jenkins.model.Jenkins; import jenkins.security.NotReallyRoleSensitiveCallable; import org.acegisecurity.Authentication; +import jenkins.util.xml.XMLUtils; + import org.apache.tools.ant.taskdefs.Copy; import org.apache.tools.ant.types.FileSet; import org.kohsuke.stapler.WebMethod; @@ -67,12 +69,11 @@ import org.kohsuke.stapler.HttpDeletable; import org.kohsuke.args4j.Argument; import org.kohsuke.args4j.CmdLineException; import org.kohsuke.stapler.interceptor.RequirePOST; +import org.xml.sax.SAXException; import javax.servlet.ServletException; import javax.xml.transform.Source; -import javax.xml.transform.Transformer; import javax.xml.transform.TransformerException; -import javax.xml.transform.TransformerFactory; import javax.xml.transform.stream.StreamResult; import javax.xml.transform.stream.StreamSource; @@ -627,25 +628,24 @@ public abstract class AbstractItem extends Actionable implements Item, HttpDelet } /** - * Updates Job by its XML definition. + * Updates an Item by its XML definition. + * @param source source of the Item's new definition. + * The source should be either a StreamSource or a SAXSource, other + * sources may not be handled. * @since 1.473 */ public void updateByXml(Source source) throws IOException { checkPermission(CONFIGURE); XmlFile configXmlFile = getConfigFile(); - AtomicFileWriter out = new AtomicFileWriter(configXmlFile.getFile()); + final AtomicFileWriter out = new AtomicFileWriter(configXmlFile.getFile()); try { try { - // this allows us to use UTF-8 for storing data, - // plus it checks any well-formedness issue in the submitted - // data - Transformer t = TransformerFactory.newInstance() - .newTransformer(); - t.transform(source, - new StreamResult(out)); + XMLUtils.safeTransform(source, new StreamResult(out)); out.close(); } catch (TransformerException e) { throw new IOException("Failed to persist config.xml", e); + } catch (SAXException e) { + throw new IOException("Failed to persist config.xml", e); } // try to reflect the changes by reloading @@ -667,6 +667,7 @@ public abstract class AbstractItem extends Actionable implements Item, HttpDelet // if everything went well, commit this new version out.commit(); SaveableListener.fireOnChange(this, getConfigFile()); + } finally { out.abort(); // don't leave anything behind } diff --git a/core/src/main/java/hudson/model/User.java b/core/src/main/java/hudson/model/User.java index c4190ee4af..52ca6005fb 100644 --- a/core/src/main/java/hudson/model/User.java +++ b/core/src/main/java/hudson/model/User.java @@ -35,6 +35,7 @@ import hudson.security.Permission; import hudson.security.SecurityRealm; import hudson.security.UserMayOrMayNotExistException; import hudson.util.FormApply; +import hudson.util.FormValidation; import hudson.util.RunList; import hudson.util.XStream2; import jenkins.model.IdStrategy; @@ -50,6 +51,8 @@ import org.acegisecurity.providers.UsernamePasswordAuthenticationToken; import org.acegisecurity.providers.anonymous.AnonymousAuthenticationToken; import org.acegisecurity.userdetails.UserDetails; import org.acegisecurity.userdetails.UsernameNotFoundException; +import org.kohsuke.accmod.Restricted; +import org.kohsuke.accmod.restrictions.NoExternalUse; import org.springframework.dao.DataAccessException; import org.kohsuke.stapler.StaplerRequest; import org.kohsuke.stapler.StaplerResponse; @@ -109,7 +112,12 @@ import javax.annotation.Nullable; */ @ExportedBean public class User extends AbstractModelObject implements AccessControlled, DescriptorByNameOwner, Saveable, Comparable, ModelObjectWithContextMenu { - + + /** + * These usernames should not be used by real users logging into Jenkins. Therefore, we prevent + * users with these names from being saved. + */ + public static final String[] ILLEGAL_PERSISTED_USERNAMES = new String[]{"anonymous", "system", "unknown"}; private transient final String id; private volatile String fullName; @@ -642,10 +650,36 @@ public class User extends AbstractModelObject implements AccessControlled, Descr return new File(Jenkins.getInstance().getRootDir(), "users"); } + /** + * Is the ID allowed? Some are prohibited for security reasons. See SECURITY-166. + *

+ * Note that this is only enforced when saving. These users are often created + * via the constructor (and even listed on /asynchPeople), but our goal is to + * prevent anyone from logging in as these users. Therefore, we prevent + * saving a User with one of these ids. + * + * @return true if the username or fullname is valid + */ + //TODO: Remove Restricted and add @since when merging to stable-rc + @Restricted(NoExternalUse.class) + public static boolean isIdOrFullnameAllowed(String id) { + for (String invalidId : ILLEGAL_PERSISTED_USERNAMES) { + if (id.equalsIgnoreCase(invalidId)) + return false; + } + return true; + } + /** * Save the settings to a file. */ - public synchronized void save() throws IOException { + public synchronized void save() throws IOException, FormValidation { + if (! isIdOrFullnameAllowed(id)) { + throw FormValidation.error(Messages.User_IllegalUsername(id)); + } + if (! isIdOrFullnameAllowed(fullName)) { + throw FormValidation.error(Messages.User_IllegalFullname(fullName)); + } if(BulkChange.contains(this)) return; getConfigFile().write(this); SaveableListener.fireOnChange(this, getConfigFile()); diff --git a/core/src/main/java/hudson/model/View.java b/core/src/main/java/hudson/model/View.java index d03727af6c..d30a80e876 100644 --- a/core/src/main/java/hudson/model/View.java +++ b/core/src/main/java/hudson/model/View.java @@ -58,6 +58,8 @@ import hudson.widgets.Widget; import jenkins.model.Jenkins; import jenkins.model.ModelObjectWithChildren; import jenkins.util.ProgressiveRendering; +import jenkins.util.xml.XMLUtils; + import net.sf.json.JSON; import net.sf.json.JSONArray; import net.sf.json.JSONObject; @@ -75,9 +77,7 @@ import org.kohsuke.stapler.interceptor.RequirePOST; import javax.servlet.ServletException; import javax.servlet.http.HttpServletResponse; import javax.xml.transform.Source; -import javax.xml.transform.Transformer; import javax.xml.transform.TransformerException; -import javax.xml.transform.TransformerFactory; import javax.xml.transform.stream.StreamResult; import javax.xml.transform.stream.StreamSource; import java.io.BufferedInputStream; @@ -106,6 +106,7 @@ import static javax.servlet.http.HttpServletResponse.SC_BAD_REQUEST; import static jenkins.model.Jenkins.*; import org.kohsuke.accmod.Restricted; import org.kohsuke.accmod.restrictions.NoExternalUse; +import org.xml.sax.SAXException; /** * Encapsulates the rendering of the list of {@link TopLevelItem}s @@ -1063,7 +1064,10 @@ public abstract class View extends AbstractModelObject implements AccessControll } /** - * Updates Job by its XML definition. + * Updates the View with the new XML definition. + * @param source source of the Item's new definition. + * The source should be either a StreamSource or SAXSource, other sources + * may not be handled. */ public void updateByXml(Source source) throws IOException { checkPermission(CONFIGURE); @@ -1072,13 +1076,12 @@ public abstract class View extends AbstractModelObject implements AccessControll // this allows us to use UTF-8 for storing data, // plus it checks any well-formedness issue in the submitted // data - Transformer t = TransformerFactory.newInstance() - .newTransformer(); - t.transform(source, - new StreamResult(out)); + XMLUtils.safeTransform(source, new StreamResult(out)); out.close(); } catch (TransformerException e) { throw new IOException("Failed to persist configuration.xml", e); + } catch (SAXException e) { + throw new IOException("Failed to persist configuration.xml", e); } // try to reflect the changes by reloading diff --git a/core/src/main/java/hudson/security/HudsonPrivateSecurityRealm.java b/core/src/main/java/hudson/security/HudsonPrivateSecurityRealm.java index 642ebac84b..546d611fcf 100644 --- a/core/src/main/java/hudson/security/HudsonPrivateSecurityRealm.java +++ b/core/src/main/java/hudson/security/HudsonPrivateSecurityRealm.java @@ -337,6 +337,14 @@ public class HudsonPrivateSecurityRealm extends AbstractPasswordBasedSecurityRea if(si.email==null || !si.email.contains("@")) si.errorMessage = Messages.HudsonPrivateSecurityRealm_CreateAccount_InvalidEmailAddress(); + if (! User.isIdOrFullnameAllowed(si.username)) { + si.errorMessage = hudson.model.Messages.User_IllegalUsername(si.username); + } + + if (! User.isIdOrFullnameAllowed(si.fullname)) { + si.errorMessage = hudson.model.Messages.User_IllegalFullname(si.fullname); + } + if(si.errorMessage!=null) { // failed. ask the user to try again. req.setAttribute("data",si); diff --git a/core/src/main/java/jenkins/util/xml/RestrictiveEntityResolver.java b/core/src/main/java/jenkins/util/xml/RestrictiveEntityResolver.java new file mode 100644 index 0000000000..87079b551e --- /dev/null +++ b/core/src/main/java/jenkins/util/xml/RestrictiveEntityResolver.java @@ -0,0 +1,31 @@ +package jenkins.util.xml; + +import org.xml.sax.EntityResolver; +import org.xml.sax.InputSource; +import org.xml.sax.SAXException; + +import java.io.IOException; + +/** + * An EntityResolver that will fail to resolve any entities. + * Useful in preventing External XML Entity injection attacks. + * + * @since TODO + */ +public final class RestrictiveEntityResolver implements EntityResolver { + + public final static RestrictiveEntityResolver INSTANCE = new RestrictiveEntityResolver(); + + private RestrictiveEntityResolver() { + // prevent multiple instantiation. + super(); + } + + /** + * Throws a SAXException if this tried to resolve any entity. + */ + @Override + public InputSource resolveEntity(String publicId, String systemId) throws SAXException, IOException { + throw new SAXException("Refusing to resolve entity with publicId(" + publicId + ") and systemId (" + systemId + ")"); + } +} diff --git a/core/src/main/java/jenkins/util/xml/XMLUtils.java b/core/src/main/java/jenkins/util/xml/XMLUtils.java new file mode 100644 index 0000000000..5aab9b5567 --- /dev/null +++ b/core/src/main/java/jenkins/util/xml/XMLUtils.java @@ -0,0 +1,96 @@ +package jenkins.util.xml; + +import org.xml.sax.InputSource; +import org.xml.sax.SAXException; +import org.xml.sax.XMLReader; +import org.xml.sax.helpers.XMLReaderFactory; + +import java.util.logging.Level; +import java.util.logging.LogManager; +import java.util.logging.Logger; +import javax.annotation.Nonnull; +import javax.xml.XMLConstants; +import javax.xml.transform.Result; +import javax.xml.transform.Source; +import javax.xml.transform.Transformer; +import javax.xml.transform.TransformerException; +import javax.xml.transform.TransformerFactory; +import javax.xml.transform.sax.SAXSource; +import javax.xml.transform.sax.SAXTransformerFactory; + +/** + * Utilities useful when working with various XML types. + */ +public final class XMLUtils { + + private final static Logger LOGGER = LogManager.getLogManager().getLogger(XMLUtils.class.getName()); + private final static String DISABLED_PROPERTY_NAME = XMLUtils.class.getName() + ".disableXXEPrevention"; + + /** + * Transform the source to the output in a manner that is protected against XXE attacks. + * If the transform can not be completed safely then an IOException is thrown. + * Note - to turn off safety set the system property disableXXEPrevention to true. + * @param source The XML input to transform. - This should be a StreamSource or a + * SAXSource in order to be able to prevent XXE attacks. + * @param out The Result of transforming the source. + */ + public static void safeTransform(@Nonnull Source source, @Nonnull Result out) throws TransformerException, + SAXException { + + InputSource src = SAXSource.sourceToInputSource(source); + if (src != null) { + SAXTransformerFactory stFactory = (SAXTransformerFactory) TransformerFactory.newInstance(); + stFactory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); + + XMLReader xmlReader = XMLReaderFactory.createXMLReader(); + try { + xmlReader.setFeature("http://xml.org/sax/features/external-general-entities", false); + } + catch (SAXException ignored) { /* ignored */ } + try { + xmlReader.setFeature("http://xml.org/sax/features/external-parameter-entities", false); + } + catch (SAXException ignored) { /* ignored */ } + // defend against XXE + // the above features should strip out entities - however the feature may not be supported depending + // on the xml implementation used and this is out of our control. + // So add a fallback plan if all else fails. + xmlReader.setEntityResolver(RestrictiveEntityResolver.INSTANCE); + SAXSource saxSource = new SAXSource(xmlReader, src); + _transform(saxSource, out); + } + else { + // for some reason we could not convert source + // this applies to DOMSource and StAXSource - and possibly 3rd party implementations... + // a DOMSource can already be compromised as it is parsed by the time it gets to us. + if (Boolean.getBoolean(DISABLED_PROPERTY_NAME)) { + LOGGER.log(Level.WARNING, "XML external entity (XXE) prevention has been disabled by the system " + + "property {0}=true Your system may be vulnerable to XXE attacks.", DISABLED_PROPERTY_NAME); + if (LOGGER.isLoggable(Level.FINE)) { + LOGGER.log(Level.FINE, "Caller stack trace: ", new Exception("XXE Prevention caller history")); + } + _transform(source, out); + } + else { + throw new TransformerException("Could not convert source of type " + source.getClass() + " and " + + "XXEPrevention is enabled."); + } + } + } + + /** + * potentially unsafe XML transformation. + * @param source The XML input to transform. + * @param out The Result of transforming the source. + */ + private static void _transform(Source source, Result out) throws TransformerException { + TransformerFactory factory = TransformerFactory.newInstance(); + factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); + + // this allows us to use UTF-8 for storing data, + // plus it checks any well-formedness issue in the submitted data. + Transformer t = factory.newTransformer(); + t.transform(source, out); + } + +} diff --git a/core/src/main/resources/hudson/model/Messages.properties b/core/src/main/resources/hudson/model/Messages.properties index 33dfb245cd..1629372412 100644 --- a/core/src/main/resources/hudson/model/Messages.properties +++ b/core/src/main/resources/hudson/model/Messages.properties @@ -356,3 +356,6 @@ Jenkins.CheckDisplayName.DisplayNameNotUniqueWarning=The display name, "{0}", is Jenkins.NotAllowedName="{0}" is not allowed name Jenkins.IsRestarting=Jenkins is restarting + +User.IllegalUsername="{0}" is prohibited as a username for security reasons. +User.IllegalFullname="{0}" is prohibited as a full name for security reasons. diff --git a/core/src/test/java/jenkins/xml/XMLUtilsTest.java b/core/src/test/java/jenkins/xml/XMLUtilsTest.java new file mode 100644 index 0000000000..18bd0052b3 --- /dev/null +++ b/core/src/test/java/jenkins/xml/XMLUtilsTest.java @@ -0,0 +1,101 @@ +/* + * The MIT License + * + * Copyright 2015 James Nord. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +package jenkins.xml; + +import jenkins.util.xml.XMLUtils; + +import org.apache.commons.io.output.NullOutputStream; +import org.junit.Test; + +import java.io.StringReader; +import java.io.StringWriter; +import javax.xml.transform.TransformerException; +import javax.xml.transform.stream.StreamResult; +import javax.xml.transform.stream.StreamSource; + +import static org.hamcrest.core.StringContains.containsString; +import static org.junit.Assert.assertThat; +import static org.junit.Assert.fail; + +public class XMLUtilsTest { + + @Test() + // SECURITY-167 + public void testSafeTransformDoesNotProcessForeignResources() throws Exception { + final String xml = "\n" + + "\n" + + "]>\n" + + "\n" + + " \n" + + " &foo;\n" + + " false\n" + + " \n" + + " \n" + + " true\n" + + " \n" + + " \n" + + " \n" + + " \n" + + ""; + + + StringWriter stringWriter = new StringWriter(); + try { + XMLUtils.safeTransform(new StreamSource(new StringReader(xml)), new StreamResult(stringWriter)); + // if no exception then JAXP is swallowing these - so there should be no entity in the description. + assertThat(stringWriter.toString(), containsString("")); + } catch (TransformerException ex) { + assertThat(ex.getMessage(), containsString("Refusing to resolve entity")); + } + + } + + + @Test() + // SECURITY-167 + public void testUpdateByXmlIDoesNotFail() throws Exception { + final String xml = "\n" + + "\n" + + " \n" + + " &\n" + + " false\n" + + " \n" + + " \n" + + " true\n" + + " \n" + + " \n" + + " \n" + + " \n" + + ""; + + StringWriter stringWriter = new StringWriter(); + + XMLUtils.safeTransform(new StreamSource(new StringReader(xml)), new StreamResult(stringWriter)); + // make sure that normal entities are retained. + assertThat(stringWriter.toString(), containsString("&")); + } + +} diff --git a/test/src/test/java/hudson/model/AbstractItemTest.java b/test/src/test/java/hudson/model/AbstractItemTest.java new file mode 100644 index 0000000000..6c186fd939 --- /dev/null +++ b/test/src/test/java/hudson/model/AbstractItemTest.java @@ -0,0 +1,113 @@ +/* + * The MIT License + * + * Copyright 2015 James Nord. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +package hudson.model; + +import hudson.model.FreeStyleProject; +import org.junit.Rule; +import org.junit.Test; +import org.jvnet.hudson.test.JenkinsRule; +import org.w3c.dom.Document; +import org.w3c.dom.Node; +import org.w3c.dom.NodeList; +import org.w3c.dom.Text; + +import javax.xml.parsers.DocumentBuilder; +import javax.xml.parsers.DocumentBuilderFactory; +import javax.xml.transform.dom.DOMSource; +import javax.xml.transform.stream.StreamSource; +import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.io.StringReader; + +import static org.hamcrest.core.Is.is; +import static org.hamcrest.core.IsNot.not; +import static org.hamcrest.core.IsNull.nullValue; +import static org.hamcrest.core.StringContains.containsString; +import static org.hamcrest.text.IsEmptyString.isEmptyOrNullString; +import static org.junit.Assert.assertThat; +import static org.junit.Assert.fail; + +public class AbstractItemTest { + + @Rule + public JenkinsRule jenkinsRule = new JenkinsRule(); + + @Test() + // SECURITY-167 + public void testUpdateByXmlIDoesNotProcessForeignResources() throws Exception { + final String xml = "\n" + + "\n" + + "]>\n" + + "\n" + + " \n" + + " &foo;\n" + + " false\n" + + " \n" + + " \n" + + " true\n" + + " \n" + + " \n" + + " \n" + + " \n" + + ""; + + FreeStyleProject project = jenkinsRule.createFreeStyleProject("security-167"); + project.setDescription("Wibble"); + try { + project.updateByXml(new StreamSource(new StringReader(xml))); + // if we didn't fail JAXP has thrown away the entity. + assertThat(project.getDescription(), isEmptyOrNullString()); + } catch (IOException ex) { + assertThat(ex.getCause(), not(nullValue())); + assertThat(ex.getCause().getMessage(), containsString("Refusing to resolve entity")); + } + + } + + + @Test() + // SECURITY-167 + public void testhamyXmlIDoesNotFail() throws Exception { + final String xml = "\n" + + "\n" + + " \n" + + " &\n" + + " false\n" + + " \n" + + " \n" + + " true\n" + + " \n" + + " \n" + + " \n" + + " \n" + + ""; + + FreeStyleProject project = jenkinsRule.createFreeStyleProject("security-167"); + project.updateByXml((StreamSource) new StreamSource(new StringReader(xml))); + assertThat(project.getDescription(), is("&")); // the entity is transformed + } + +} diff --git a/test/src/test/java/hudson/security/HudsonPrivateSecurityRealm2Test.java b/test/src/test/java/hudson/security/HudsonPrivateSecurityRealm2Test.java new file mode 100644 index 0000000000..0d87c08f45 --- /dev/null +++ b/test/src/test/java/hudson/security/HudsonPrivateSecurityRealm2Test.java @@ -0,0 +1,93 @@ +package hudson.security; + +import com.gargoylesoftware.htmlunit.html.HtmlPage; +import hudson.model.User; +import hudson.security.pages.SignupPage; +import org.junit.Rule; +import org.junit.Test; +import org.jvnet.hudson.test.JenkinsRule; + +import java.util.Collections; + +import static org.hamcrest.Matchers.containsString; +import static org.junit.Assert.*; + +/** + * Tests the signup page of {@link hudson.security.HudsonPrivateSecurityRealm} + * TODO: Add to {@link hudson.security.HudsonPrivateSecurityRealmTest} going forward + */ +public class HudsonPrivateSecurityRealm2Test { + + + @Rule + public JenkinsRule rule = new JenkinsRule(); + + @Test + public void signup() throws Exception { + HudsonPrivateSecurityRealm securityRealm = new HudsonPrivateSecurityRealm(true, false, null); + rule.jenkins.setSecurityRealm(securityRealm); + JenkinsRule.WebClient wc = rule.createWebClient(); + SignupPage signup = new SignupPage(wc.goTo("signup")); + signup.enterUsername("alice"); + signup.enterPassword("alice"); + signup.enterFullName("Alice User"); + signup.enterEmail("alice@nowhere.com"); + HtmlPage success = signup.submit(rule); + assertThat(success.getElementById("main-panel").getTextContent(), containsString("Success")); + assertThat(success.getElementById("login-field").getTextContent(), containsString("Alice User")); + + assertEquals("Alice User", securityRealm.getUser("alice").getDisplayName()); + + } + + // @Issue("SECURITY-166") + @Test + public void anonymousCantSignup() throws Exception { + HudsonPrivateSecurityRealm securityRealm = new HudsonPrivateSecurityRealm(true, false, null); + rule.jenkins.setSecurityRealm(securityRealm); + JenkinsRule.WebClient wc = rule.createWebClient(); + SignupPage signup = new SignupPage(wc.goTo("signup")); + signup.enterUsername("anonymous"); + signup.enterFullName("Bob"); + signup.enterPassword("nothing"); + signup.enterEmail("noone@nowhere.com"); + signup = new SignupPage(signup.submit(rule)); + signup.assertErrorContains("prohibited as a username"); + assertNull(User.get("anonymous", false, Collections.emptyMap())); + } + + // @Issue("SECURITY-166") + @Test + public void systemCantSignup() throws Exception { + HudsonPrivateSecurityRealm securityRealm = new HudsonPrivateSecurityRealm(true, false, null); + rule.jenkins.setSecurityRealm(securityRealm); + JenkinsRule.WebClient wc = rule.createWebClient(); + SignupPage signup = new SignupPage(wc.goTo("signup")); + signup.enterUsername("system"); + signup.enterFullName("Bob"); + signup.enterPassword("nothing"); + signup.enterEmail("noone@nowhere.com"); + signup = new SignupPage(signup.submit(rule)); + signup.assertErrorContains("prohibited as a username"); + assertNull(User.get("system",false, Collections.emptyMap())); + } + + /** + * We don't allow prohibited fullnames since this may encumber auditing. + */ + // @Issue("SECURITY-166") + @Test + public void fullNameOfUnknownCantSignup() throws Exception { + HudsonPrivateSecurityRealm securityRealm = new HudsonPrivateSecurityRealm(true, false, null); + rule.jenkins.setSecurityRealm(securityRealm); + JenkinsRule.WebClient wc = rule.createWebClient(); + SignupPage signup = new SignupPage(wc.goTo("signup")); + signup.enterUsername("unknown2"); + signup.enterPassword("unknown2"); + signup.enterFullName("unknown"); + signup.enterEmail("noone@nowhere.com"); + signup = new SignupPage(signup.submit(rule)); + signup.assertErrorContains("prohibited as a full name"); + assertNull(User.get("unknown2",false, Collections.emptyMap())); + } +} diff --git a/test/src/test/java/hudson/security/pages/SignupPage.java b/test/src/test/java/hudson/security/pages/SignupPage.java new file mode 100644 index 0000000000..5007bce1b5 --- /dev/null +++ b/test/src/test/java/hudson/security/pages/SignupPage.java @@ -0,0 +1,65 @@ +package hudson.security.pages; + +import com.gargoylesoftware.htmlunit.html.HtmlForm; +import com.gargoylesoftware.htmlunit.html.HtmlPage; +import org.jvnet.hudson.test.JenkinsRule; + +import static org.hamcrest.Matchers.containsString; +import static org.junit.Assert.*; + +/** + * The the signup page for {@link hudson.security.HudsonPrivateSecurityRealm} + */ +public class SignupPage { + + public HtmlForm signupForm; + public final HtmlPage signupPage; + + public SignupPage(HtmlPage signupPage) { + this.signupPage = signupPage; + + assertNotNull("The sign up page has a username field.", this.signupPage.getElementById("username")); + for (HtmlForm signupForm : this.signupPage.getForms()) { + if (signupForm.getInputsByName("username").size() == 0) + continue; + this.signupForm = signupForm; + } + + } + + + + public void enterUsername(String username) { + signupForm.getInputByName("username").setValueAttribute(username); + } + + /** + * Enters the password in password1 and password2. + * You can then call {@link #enterPassword2(String)} if you want them to be different. + * @param password + */ + public void enterPassword(String password) { + signupForm.getInputByName("password1").setValueAttribute(password); + signupForm.getInputByName("password2").setValueAttribute(password); + } + + public void enterPassword2(String password2) { + signupForm.getInputByName("password2").setValueAttribute(password2); + } + + public void enterFullName(String fullName) { + signupForm.getInputByName("fullname").setValueAttribute(fullName); + } + + public void enterEmail(String email) { + signupForm.getInputByName("email").setValueAttribute(email); + } + + public HtmlPage submit(JenkinsRule rule) throws Exception { + return rule.submit(signupForm); + } + + public void assertErrorContains(String msg) { + assertThat(signupForm.getElementById("main-panel").getTextContent(),containsString(msg)); + } +}