From 41d0976ba2dd022250db907bffa6f96c6d52a692 Mon Sep 17 00:00:00 2001 From: Josh Soref Date: Mon, 8 Apr 2019 16:23:22 -0400 Subject: [PATCH 01/10] Pre-security cleanup: removing trailing whitespace --- .../test/java/hudson/PluginSEC925Test.java | 6 ++--- .../java/hudson/model/QueueSEC891Test.java | 26 +++++++++---------- .../hudson/model/UserRestartSEC897Test.java | 16 ++++++------ ...udsonPrivateSecurityRealm2SEC1158Test.java | 14 +++++----- ...HudsonPrivateSecurityRealmSEC1245Test.java | 2 +- ...kenBasedRememberMeServices2SEC868Test.java | 8 +++--- ...kenBasedRememberMeServices2SEC996Test.java | 22 ++++++++-------- 7 files changed, 47 insertions(+), 47 deletions(-) diff --git a/test/src/test/java/hudson/PluginSEC925Test.java b/test/src/test/java/hudson/PluginSEC925Test.java index ab3fb94243..beeca3183d 100644 --- a/test/src/test/java/hudson/PluginSEC925Test.java +++ b/test/src/test/java/hudson/PluginSEC925Test.java @@ -15,10 +15,10 @@ import org.junit.Ignore; //TODO merge it within PluginTest after the security release public class PluginSEC925Test { - + @Rule public JenkinsRule r = new JenkinsRule(); - + @Ignore("TODO observed to fail in CI with 404 due to external UC issues") @Test @Issue("SECURITY-925") @@ -26,7 +26,7 @@ public class PluginSEC925Test { // impossible to use installDetachedPlugin("credentials") since we want to have it exploded like with WAR Jenkins.getInstance().getUpdateCenter().getSites().get(0).updateDirectlyNow(false); List> pluginInstalled = r.jenkins.pluginManager.install(Arrays.asList("credentials"), true); - + for (Future job : pluginInstalled) { job.get(); } diff --git a/test/src/test/java/hudson/model/QueueSEC891Test.java b/test/src/test/java/hudson/model/QueueSEC891Test.java index 573065ae7b..97d5104d77 100644 --- a/test/src/test/java/hudson/model/QueueSEC891Test.java +++ b/test/src/test/java/hudson/model/QueueSEC891Test.java @@ -22,52 +22,52 @@ import static org.junit.Assert.assertTrue; //TODO merge into QueueTest after security patch public class QueueSEC891Test { - + @Rule public JenkinsRule r = new NodeProvisionerRule(-1, 0, 10); - + @Test public void doCancelItem_PermissionIsChecked() throws Exception { checkCancelOperationUsingUrl(item -> "queue/cancelItem?id=" + item.getId()); } - + @Test public void doCancelQueue_PermissionIsChecked() throws Exception { checkCancelOperationUsingUrl(item -> "queue/item/" + item.getId() + "/cancelQueue"); } - + private void checkCancelOperationUsingUrl(Function urlProvider) throws Exception { Queue q = r.jenkins.getQueue(); - + r.jenkins.setCrumbIssuer(null); r.jenkins.setSecurityRealm(r.createDummySecurityRealm()); r.jenkins.setAuthorizationStrategy(new MockAuthorizationStrategy() .grant(Jenkins.READ, Item.CANCEL).everywhere().to("admin") .grant(Jenkins.READ).everywhere().to("user") ); - + // prevent execution to push stuff into the queue r.jenkins.setNumExecutors(0); assertThat(q.getItems().length, equalTo(0)); - + FreeStyleProject testProject = r.createFreeStyleProject("test"); testProject.scheduleBuild(new UserIdCause()); - + Queue.Item[] items = q.getItems(); assertThat(items.length, equalTo(1)); Queue.Item currentOne = items[0]; assertFalse(currentOne.getFuture().isCancelled()); - + WebRequest request = new WebRequest(new URL(r.getURL() + urlProvider.apply(currentOne)), HttpMethod.POST); - + { // user without right cannot cancel JenkinsRule.WebClient wc = r.createWebClient() .withRedirectEnabled(false) .withThrowExceptionOnFailingStatusCode(false); wc.login("user"); Page p = wc.getPage(request); - // currently the endpoint return a redirection to the previously visited page, none in our case + // currently the endpoint return a redirection to the previously visited page, none in our case // (so force no redirect to avoid false positive error) assertThat(p.getWebResponse().getStatusCode(), lessThan(400)); - + assertFalse(currentOne.getFuture().isCancelled()); } { // user with right can @@ -77,7 +77,7 @@ public class QueueSEC891Test { wc.login("admin"); Page p = wc.getPage(request); assertThat(p.getWebResponse().getStatusCode(), lessThan(400)); - + assertTrue(currentOne.getFuture().isCancelled()); } } diff --git a/test/src/test/java/hudson/model/UserRestartSEC897Test.java b/test/src/test/java/hudson/model/UserRestartSEC897Test.java index 4763504de0..8315abfa3b 100644 --- a/test/src/test/java/hudson/model/UserRestartSEC897Test.java +++ b/test/src/test/java/hudson/model/UserRestartSEC897Test.java @@ -17,24 +17,24 @@ import static org.junit.Assert.assertThat; //TODO after the security fix, it could be merged inside UserRestartTest public class UserRestartSEC897Test { - + @Rule public RestartableJenkinsRule rr = new RestartableJenkinsRule(); - + @Test public void legacyConfigMoveCannotEscapeUserFolder() { rr.addStep(new Statement() { @Override public void evaluate() throws Throwable { rr.j.jenkins.setSecurityRealm(rr.j.createDummySecurityRealm()); assertThat(rr.j.jenkins.isUseSecurity(), equalTo(true)); - + // in order to create the folder "users" User.getById("admin", true).save(); - + { // attempt with ".." JenkinsRule.WebClient wc = rr.j.createWebClient() .withThrowExceptionOnFailingStatusCode(false); - + WebRequest request = new WebRequest(new URL(rr.j.jenkins.getRootUrl() + "whoAmI/api/xml")); request.setAdditionalHeader("Authorization", base64("..", "any-password")); wc.getPage(request); @@ -42,12 +42,12 @@ public class UserRestartSEC897Test { { // attempt with "../users/.." JenkinsRule.WebClient wc = rr.j.createWebClient() .withThrowExceptionOnFailingStatusCode(false); - + WebRequest request = new WebRequest(new URL(rr.j.jenkins.getRootUrl() + "whoAmI/api/xml")); request.setAdditionalHeader("Authorization", base64("../users/..", "any-password")); wc.getPage(request); } - + // security is still active assertThat(rr.j.jenkins.isUseSecurity(), equalTo(true)); // but, the config file was moved @@ -64,7 +64,7 @@ public class UserRestartSEC897Test { } }); } - + private String base64(String login, String password) { return "Basic " + Base64.getEncoder().encodeToString((login + ":" + password).getBytes(StandardCharsets.UTF_8)); } diff --git a/test/src/test/java/hudson/security/HudsonPrivateSecurityRealm2SEC1158Test.java b/test/src/test/java/hudson/security/HudsonPrivateSecurityRealm2SEC1158Test.java index fca41a931d..ad0cb99660 100644 --- a/test/src/test/java/hudson/security/HudsonPrivateSecurityRealm2SEC1158Test.java +++ b/test/src/test/java/hudson/security/HudsonPrivateSecurityRealm2SEC1158Test.java @@ -38,22 +38,22 @@ import static org.junit.Assert.assertThat; //TODO to be merged into HudsonPrivateSecurityRealm2Test after security release public class HudsonPrivateSecurityRealm2SEC1158Test { - + @Rule public JenkinsRule rule = new JenkinsRule(); - + @Test @Issue("SECURITY-1158") public void singupNoLongerVulnerableToSessionFixation() throws Exception { HudsonPrivateSecurityRealm securityRealm = new HudsonPrivateSecurityRealm(true, false, null); rule.jenkins.setSecurityRealm(securityRealm); JenkinsRule.WebClient wc = rule.createWebClient(); - + // to trigger the creation of a session wc.goTo(""); Cookie sessionBefore = wc.getCookieManager().getCookie("JSESSIONID"); String sessionIdBefore = sessionBefore.getValue(); - + SignupPage signup = new SignupPage(wc.goTo("signup")); signup.enterUsername("alice"); signup.enterPassword("alice"); @@ -62,12 +62,12 @@ public class HudsonPrivateSecurityRealm2SEC1158Test { HtmlPage success = signup.submit(rule); assertThat(success.getElementById("main-panel").getTextContent(), containsString("Success")); assertThat(success.getAnchorByHref("/jenkins/user/alice").getTextContent(), containsString("Alice User")); - + assertEquals("Alice User", securityRealm.getUser("alice").getDisplayName()); - + Cookie sessionAfter = wc.getCookieManager().getCookie("JSESSIONID"); String sessionIdAfter = sessionAfter.getValue(); - + assertNotEquals(sessionIdAfter, sessionIdBefore); } } diff --git a/test/src/test/java/hudson/security/HudsonPrivateSecurityRealmSEC1245Test.java b/test/src/test/java/hudson/security/HudsonPrivateSecurityRealmSEC1245Test.java index 1f2eece510..bcda8c4c92 100644 --- a/test/src/test/java/hudson/security/HudsonPrivateSecurityRealmSEC1245Test.java +++ b/test/src/test/java/hudson/security/HudsonPrivateSecurityRealmSEC1245Test.java @@ -141,7 +141,7 @@ public class HudsonPrivateSecurityRealmSEC1245Test { UserSeedProperty.DISABLE_USER_SEED = previousConfig; } } - + private User prepareRealmAndAlice() throws Exception { j.jenkins.setDisableRememberMe(false); HudsonPrivateSecurityRealm securityRealm = new HudsonPrivateSecurityRealm(false, false, null); diff --git a/test/src/test/java/hudson/security/TokenBasedRememberMeServices2SEC868Test.java b/test/src/test/java/hudson/security/TokenBasedRememberMeServices2SEC868Test.java index 8f8031b79b..3be48adfd7 100644 --- a/test/src/test/java/hudson/security/TokenBasedRememberMeServices2SEC868Test.java +++ b/test/src/test/java/hudson/security/TokenBasedRememberMeServices2SEC868Test.java @@ -122,7 +122,7 @@ public class TokenBasedRememberMeServices2SEC868Test { { // a malicious cookie with expiration too far in the future should not work JenkinsRule.WebClient wc = j.createWebClient(); - // by default we have 14 days of validity, + // by default we have 14 days of validity, // here we increase artificially the duration of validity, that could be used to have permanent access long oneDay = TimeUnit.DAYS.toMillis(1); Cookie cookie = createRememberMeCookie(tokenService, oneDay, alice); @@ -135,7 +135,7 @@ public class TokenBasedRememberMeServices2SEC868Test { { // a hand crafted cookie with regular expiration duration works JenkinsRule.WebClient wc = j.createWebClient(); - // by default we have 14 days of validity, + // by default we have 14 days of validity, // here we reduce a bit the expiration date to simulate an "old" cookie (regular usage) long minusFiveMinutes = TimeUnit.MINUTES.toMillis(-5); Cookie cookie = createRememberMeCookie(tokenService, minusFiveMinutes, alice); @@ -165,7 +165,7 @@ public class TokenBasedRememberMeServices2SEC868Test { { // a malicious cookie with expiration too far in the future should not work JenkinsRule.WebClient wc = j.createWebClient(); - // by default we have 14 days of validity, + // by default we have 14 days of validity, // here we increase artificially the duration of validity, that could be used to have permanent access long oneDay = TimeUnit.DAYS.toMillis(1); Cookie cookie = createRememberMeCookie(tokenService, oneDay, alice); @@ -178,7 +178,7 @@ public class TokenBasedRememberMeServices2SEC868Test { { // a hand crafted cookie with regular expiration duration works JenkinsRule.WebClient wc = j.createWebClient(); - // by default we have 14 days of validity, + // by default we have 14 days of validity, // here we reduce a bit the expiration date to simulate an "old" cookie (regular usage) long minusFiveMinutes = TimeUnit.MINUTES.toMillis(-5); Cookie cookie = createRememberMeCookie(tokenService, minusFiveMinutes, alice); diff --git a/test/src/test/java/hudson/security/TokenBasedRememberMeServices2SEC996Test.java b/test/src/test/java/hudson/security/TokenBasedRememberMeServices2SEC996Test.java index 1abecfbfc8..bcb7ddea16 100644 --- a/test/src/test/java/hudson/security/TokenBasedRememberMeServices2SEC996Test.java +++ b/test/src/test/java/hudson/security/TokenBasedRememberMeServices2SEC996Test.java @@ -40,48 +40,48 @@ import static org.junit.Assert.assertThat; public class TokenBasedRememberMeServices2SEC996Test { @Rule public JenkinsRule j = new JenkinsRule(); - + @Test @Issue("SECURITY-996") public void rememberMeToken_shouldNotBeRead_ifOptionIsDisabled() throws Exception { j.jenkins.setDisableRememberMe(false); j.jenkins.setSecurityRealm(j.createDummySecurityRealm()); - + Cookie rememberMeCookie = null; { JenkinsRule.WebClient wc = j.createWebClient(); wc.login("alice", "alice", true); - - + + // we should see a remember me cookie rememberMeCookie = getRememberMeCookie(wc); assertNotNull(rememberMeCookie); assertThat(rememberMeCookie.getValue(), not(isEmptyString())); } - + j.jenkins.setDisableRememberMe(true); { JenkinsRule.WebClient wc = j.createWebClient(); - + wc.getCookieManager().addCookie(rememberMeCookie); - + // the application should not use the cookie to connect XmlPage page = (XmlPage) wc.goTo("whoAmI/api/xml", "application/xml"); assertThat(page, hasXPath("//name", not(is("alice")))); } - + j.jenkins.setDisableRememberMe(false); { JenkinsRule.WebClient wc = j.createWebClient(); - + wc.getCookieManager().addCookie(rememberMeCookie); - + // if we reactivate the remember me feature, it's ok XmlPage page = (XmlPage) wc.goTo("whoAmI/api/xml", "application/xml"); assertThat(page, hasXPath("//name", is("alice"))); } } - + private Cookie getRememberMeCookie(JenkinsRule.WebClient wc) { return wc.getCookieManager().getCookie(TokenBasedRememberMeServices2.ACEGI_SECURITY_HASHED_REMEMBER_ME_COOKIE_KEY); } From d636d1e100f3fbc882e54b505f840ee86cbd59f8 Mon Sep 17 00:00:00 2001 From: Josh Soref Date: Mon, 8 Apr 2019 16:28:57 -0400 Subject: [PATCH 02/10] Pre-security cleanup: adding Issue tags --- test/src/test/java/hudson/model/QueueSEC891Test.java | 8 ++++++-- .../src/test/java/hudson/model/UserRestartSEC897Test.java | 4 +++- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/test/src/test/java/hudson/model/QueueSEC891Test.java b/test/src/test/java/hudson/model/QueueSEC891Test.java index 97d5104d77..b87c4c544c 100644 --- a/test/src/test/java/hudson/model/QueueSEC891Test.java +++ b/test/src/test/java/hudson/model/QueueSEC891Test.java @@ -26,11 +26,15 @@ public class QueueSEC891Test { @Rule public JenkinsRule r = new NodeProvisionerRule(-1, 0, 10); - @Test public void doCancelItem_PermissionIsChecked() throws Exception { + @Test + @Issue("SECURITY-891") + public void doCancelItem_PermissionIsChecked() throws Exception { checkCancelOperationUsingUrl(item -> "queue/cancelItem?id=" + item.getId()); } - @Test public void doCancelQueue_PermissionIsChecked() throws Exception { + @Test + @Issue("SECURITY-891") + public void doCancelQueue_PermissionIsChecked() throws Exception { checkCancelOperationUsingUrl(item -> "queue/item/" + item.getId() + "/cancelQueue"); } diff --git a/test/src/test/java/hudson/model/UserRestartSEC897Test.java b/test/src/test/java/hudson/model/UserRestartSEC897Test.java index 8315abfa3b..a7121591f9 100644 --- a/test/src/test/java/hudson/model/UserRestartSEC897Test.java +++ b/test/src/test/java/hudson/model/UserRestartSEC897Test.java @@ -21,7 +21,9 @@ public class UserRestartSEC897Test { @Rule public RestartableJenkinsRule rr = new RestartableJenkinsRule(); - @Test public void legacyConfigMoveCannotEscapeUserFolder() { + @Test + @Issue("SECURITY-897") + public void legacyConfigMoveCannotEscapeUserFolder() { rr.addStep(new Statement() { @Override public void evaluate() throws Throwable { From c14a0edd794fc3579a9872826fee5f806a77ca69 Mon Sep 17 00:00:00 2001 From: Josh Soref Date: Tue, 9 Apr 2019 13:12:32 -0400 Subject: [PATCH 03/10] Pre-security cleanup: use full package name to avoid collision with hudson.model.User --- .../hudson/security/TokenBasedRememberMeServices2Test.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/src/test/java/hudson/security/TokenBasedRememberMeServices2Test.java b/test/src/test/java/hudson/security/TokenBasedRememberMeServices2Test.java index 35bea92686..7fbad7afe3 100644 --- a/test/src/test/java/hudson/security/TokenBasedRememberMeServices2Test.java +++ b/test/src/test/java/hudson/security/TokenBasedRememberMeServices2Test.java @@ -12,7 +12,6 @@ import org.acegisecurity.BadCredentialsException; import org.acegisecurity.GrantedAuthority; import org.acegisecurity.GrantedAuthorityImpl; import org.acegisecurity.ui.rememberme.TokenBasedRememberMeServices; -import org.acegisecurity.userdetails.User; import org.acegisecurity.userdetails.UserDetails; import org.acegisecurity.userdetails.UsernameNotFoundException; import static org.junit.Assert.*; @@ -73,7 +72,7 @@ public class TokenBasedRememberMeServices2Test { @Override protected UserDetails authenticate(String username, String password) throws AuthenticationException { if (username.equals(password)) { - return new User(username, password, true, new GrantedAuthority[] {new GrantedAuthorityImpl("myteam")}); + return new org.acegisecurity.userdetails.User(username, password, true, new GrantedAuthority[] {new GrantedAuthorityImpl("myteam")}); } throw new BadCredentialsException(username); } From 7b4649657f90e98a5564cf5f0892deaa5fee0454 Mon Sep 17 00:00:00 2001 From: Josh Soref Date: Mon, 25 Mar 2019 14:12:24 -0400 Subject: [PATCH 04/10] [SECURITY-868] Fold tests back into normal files --- ...kenBasedRememberMeServices2SEC868Test.java | 217 ------------------ .../TokenBasedRememberMeServices2Test.java | 183 +++++++++++++++ 2 files changed, 183 insertions(+), 217 deletions(-) delete mode 100644 test/src/test/java/hudson/security/TokenBasedRememberMeServices2SEC868Test.java diff --git a/test/src/test/java/hudson/security/TokenBasedRememberMeServices2SEC868Test.java b/test/src/test/java/hudson/security/TokenBasedRememberMeServices2SEC868Test.java deleted file mode 100644 index 3be48adfd7..0000000000 --- a/test/src/test/java/hudson/security/TokenBasedRememberMeServices2SEC868Test.java +++ /dev/null @@ -1,217 +0,0 @@ -/* - * The MIT License - * - * Copyright (c) 2018, CloudBees, Inc. - * - * 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.security; - -import com.gargoylesoftware.htmlunit.CookieManager; -import com.gargoylesoftware.htmlunit.util.Cookie; -import com.gargoylesoftware.htmlunit.xml.XmlPage; -import hudson.model.User; -import jenkins.security.seed.UserSeedProperty; -import org.apache.commons.codec.binary.Base64; -import org.junit.Rule; -import org.junit.Test; -import org.jvnet.hudson.test.For; -import org.jvnet.hudson.test.Issue; -import org.jvnet.hudson.test.JenkinsRule; - -import java.util.concurrent.TimeUnit; - -import static org.hamcrest.Matchers.is; -import static org.hamcrest.Matchers.not; -import static org.hamcrest.xml.HasXPath.hasXPath; -import static org.junit.Assert.assertThat; - -@For(TokenBasedRememberMeServices2.class) -public class TokenBasedRememberMeServices2SEC868Test { - @Rule - public JenkinsRule j = new JenkinsRule(); - - @Test - @Issue("SECURITY-868") - @For(UserSeedProperty.class) - public void rememberMeToken_invalid_afterUserSeedReset() throws Exception { - j.jenkins.setDisableRememberMe(false); - - HudsonPrivateSecurityRealm realm = new HudsonPrivateSecurityRealm(false, false, null); - j.jenkins.setSecurityRealm(realm); - - String username = "alice"; - User alice = realm.createAccount(username, username); - - JenkinsRule.WebClient wc = j.createWebClient(); - - wc.login(username, username, true); - CookieManager cm = wc.getCookieManager(); - - cm.removeCookie(cm.getCookie("JSESSIONID")); - assertUserConnected(wc, username); - - alice.getProperty(UserSeedProperty.class).renewSeed(); - - cm.removeCookie(cm.getCookie("JSESSIONID")); - assertUserNotConnected(wc, username); - } - - @Test - @Issue("SECURITY-868") - @For(UserSeedProperty.class) - public void rememberMeToken_stillValid_afterUserSeedReset_ifUserSeedDisabled() throws Exception { - boolean currentStatus = UserSeedProperty.DISABLE_USER_SEED; - try { - UserSeedProperty.DISABLE_USER_SEED = true; - - j.jenkins.setDisableRememberMe(false); - - HudsonPrivateSecurityRealm realm = new HudsonPrivateSecurityRealm(false, false, null); - j.jenkins.setSecurityRealm(realm); - - String username = "alice"; - User alice = realm.createAccount(username, username); - - JenkinsRule.WebClient wc = j.createWebClient(); - - wc.login(username, username, true); - CookieManager cm = wc.getCookieManager(); - - cm.removeCookie(cm.getCookie("JSESSIONID")); - assertUserConnected(wc, username); - - alice.getProperty(UserSeedProperty.class).renewSeed(); - - cm.removeCookie(cm.getCookie("JSESSIONID")); - // as userSeed disabled, no care about the renew - assertUserConnected(wc, username); - } finally { - UserSeedProperty.DISABLE_USER_SEED = currentStatus; - } - } - - @Test - @Issue("SECURITY-868") - public void rememberMeToken_shouldNotAccept_expirationDurationLargerThanConfigured() throws Exception { - j.jenkins.setDisableRememberMe(false); - - HudsonPrivateSecurityRealm realm = new HudsonPrivateSecurityRealm(false, false, null); - TokenBasedRememberMeServices2 tokenService = (TokenBasedRememberMeServices2) realm.getSecurityComponents().rememberMe; - j.jenkins.setSecurityRealm(realm); - - String username = "alice"; - User alice = realm.createAccount(username, username); - - { // a malicious cookie with expiration too far in the future should not work - JenkinsRule.WebClient wc = j.createWebClient(); - - // by default we have 14 days of validity, - // here we increase artificially the duration of validity, that could be used to have permanent access - long oneDay = TimeUnit.DAYS.toMillis(1); - Cookie cookie = createRememberMeCookie(tokenService, oneDay, alice); - wc.getCookieManager().addCookie(cookie); - - // the application should not use the cookie to connect - assertUserNotConnected(wc, username); - } - - { // a hand crafted cookie with regular expiration duration works - JenkinsRule.WebClient wc = j.createWebClient(); - - // by default we have 14 days of validity, - // here we reduce a bit the expiration date to simulate an "old" cookie (regular usage) - long minusFiveMinutes = TimeUnit.MINUTES.toMillis(-5); - Cookie cookie = createRememberMeCookie(tokenService, minusFiveMinutes, alice); - wc.getCookieManager().addCookie(cookie); - - // if we reactivate the remember me feature, it's ok - assertUserConnected(wc, username); - } - } - - @Test - @Issue("SECURITY-868") - public void rememberMeToken_skipExpirationCheck() throws Exception { - boolean previousConfig = TokenBasedRememberMeServices2.SKIP_TOO_FAR_EXPIRATION_DATE_CHECK; - try { - TokenBasedRememberMeServices2.SKIP_TOO_FAR_EXPIRATION_DATE_CHECK = true; - - j.jenkins.setDisableRememberMe(false); - - HudsonPrivateSecurityRealm realm = new HudsonPrivateSecurityRealm(false, false, null); - TokenBasedRememberMeServices2 tokenService = (TokenBasedRememberMeServices2) realm.getSecurityComponents().rememberMe; - j.jenkins.setSecurityRealm(realm); - - String username = "alice"; - User alice = realm.createAccount(username, username); - - { // a malicious cookie with expiration too far in the future should not work - JenkinsRule.WebClient wc = j.createWebClient(); - - // by default we have 14 days of validity, - // here we increase artificially the duration of validity, that could be used to have permanent access - long oneDay = TimeUnit.DAYS.toMillis(1); - Cookie cookie = createRememberMeCookie(tokenService, oneDay, alice); - wc.getCookieManager().addCookie(cookie); - - // the application should not use the cookie to connect - assertUserConnected(wc, username); - } - - { // a hand crafted cookie with regular expiration duration works - JenkinsRule.WebClient wc = j.createWebClient(); - - // by default we have 14 days of validity, - // here we reduce a bit the expiration date to simulate an "old" cookie (regular usage) - long minusFiveMinutes = TimeUnit.MINUTES.toMillis(-5); - Cookie cookie = createRememberMeCookie(tokenService, minusFiveMinutes, alice); - wc.getCookieManager().addCookie(cookie); - - // if we reactivate the remember me feature, it's ok - assertUserConnected(wc, username); - } - } finally { - TokenBasedRememberMeServices2.SKIP_TOO_FAR_EXPIRATION_DATE_CHECK = previousConfig; - } - } - - private Cookie createRememberMeCookie(TokenBasedRememberMeServices2 tokenService, long deltaDuration, User user) throws Exception { - long tokenValiditySeconds = tokenService.getTokenValiditySeconds(); - long expiryTime = System.currentTimeMillis() + TimeUnit.SECONDS.toMillis(tokenValiditySeconds); - - // the hack - expiryTime += deltaDuration; - - String signatureValue = tokenService.makeTokenSignature(expiryTime, user.getProperty(HudsonPrivateSecurityRealm.Details.class)); - String tokenValue = user.getId() + ":" + expiryTime + ":" + signatureValue; - String tokenValueBase64 = new String(Base64.encodeBase64(tokenValue.getBytes())); - return new Cookie(j.getURL().getHost(), tokenService.getCookieName(), tokenValueBase64); - } - - private void assertUserConnected(JenkinsRule.WebClient wc, String expectedUsername) throws Exception { - XmlPage page = (XmlPage) wc.goTo("whoAmI/api/xml", "application/xml"); - assertThat(page, hasXPath("//name", is(expectedUsername))); - } - - private void assertUserNotConnected(JenkinsRule.WebClient wc, String notExpectedUsername) throws Exception { - XmlPage page = (XmlPage) wc.goTo("whoAmI/api/xml", "application/xml"); - assertThat(page, hasXPath("//name", not(is(notExpectedUsername)))); - } -} diff --git a/test/src/test/java/hudson/security/TokenBasedRememberMeServices2Test.java b/test/src/test/java/hudson/security/TokenBasedRememberMeServices2Test.java index 7fbad7afe3..e4612e5aa5 100644 --- a/test/src/test/java/hudson/security/TokenBasedRememberMeServices2Test.java +++ b/test/src/test/java/hudson/security/TokenBasedRememberMeServices2Test.java @@ -1,11 +1,16 @@ package hudson.security; +import com.gargoylesoftware.htmlunit.CookieManager; import com.gargoylesoftware.htmlunit.util.Cookie; +import com.gargoylesoftware.htmlunit.xml.XmlPage; import com.google.common.collect.ImmutableList; import java.util.Arrays; import static java.util.logging.Level.FINEST; import java.util.stream.Collectors; + import jenkins.model.Jenkins; +import jenkins.security.seed.UserSeedProperty; + import org.acegisecurity.Authentication; import org.acegisecurity.AuthenticationException; import org.acegisecurity.BadCredentialsException; @@ -14,14 +19,25 @@ import org.acegisecurity.GrantedAuthorityImpl; import org.acegisecurity.ui.rememberme.TokenBasedRememberMeServices; import org.acegisecurity.userdetails.UserDetails; import org.acegisecurity.userdetails.UsernameNotFoundException; +import org.apache.commons.codec.binary.Base64; + import static org.junit.Assert.*; import org.junit.Before; import org.junit.Rule; import org.junit.Test; +import org.jvnet.hudson.test.For; +import org.jvnet.hudson.test.Issue; import org.jvnet.hudson.test.JenkinsRule; import org.jvnet.hudson.test.LoggerRule; import org.springframework.dao.DataAccessException; +import java.util.concurrent.TimeUnit; + +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.not; +import static org.hamcrest.xml.HasXPath.hasXPath; +import static org.junit.Assert.assertThat; + public class TokenBasedRememberMeServices2Test { @Rule @@ -126,4 +142,171 @@ public class TokenBasedRememberMeServices2Test { } } + @Test + @Issue("SECURITY-868") + @For(UserSeedProperty.class) + public void rememberMeToken_invalid_afterUserSeedReset() throws Exception { + j.jenkins.setDisableRememberMe(false); + + HudsonPrivateSecurityRealm realm = new HudsonPrivateSecurityRealm(false, false, null); + j.jenkins.setSecurityRealm(realm); + + String username = "alice"; + hudson.model.User alice = realm.createAccount(username, username); + + JenkinsRule.WebClient wc = j.createWebClient(); + + wc.login(username, username, true); + CookieManager cm = wc.getCookieManager(); + + cm.removeCookie(cm.getCookie("JSESSIONID")); + assertUserConnected(wc, username); + + alice.getProperty(UserSeedProperty.class).renewSeed(); + + cm.removeCookie(cm.getCookie("JSESSIONID")); + assertUserNotConnected(wc, username); + } + + @Test + @Issue("SECURITY-868") + @For(UserSeedProperty.class) + public void rememberMeToken_stillValid_afterUserSeedReset_ifUserSeedDisabled() throws Exception { + boolean currentStatus = UserSeedProperty.DISABLE_USER_SEED; + try { + UserSeedProperty.DISABLE_USER_SEED = true; + + j.jenkins.setDisableRememberMe(false); + + HudsonPrivateSecurityRealm realm = new HudsonPrivateSecurityRealm(false, false, null); + j.jenkins.setSecurityRealm(realm); + + String username = "alice"; + hudson.model.User alice = realm.createAccount(username, username); + + JenkinsRule.WebClient wc = j.createWebClient(); + + wc.login(username, username, true); + CookieManager cm = wc.getCookieManager(); + + cm.removeCookie(cm.getCookie("JSESSIONID")); + assertUserConnected(wc, username); + + alice.getProperty(UserSeedProperty.class).renewSeed(); + + cm.removeCookie(cm.getCookie("JSESSIONID")); + // as userSeed disabled, no care about the renew + assertUserConnected(wc, username); + } finally { + UserSeedProperty.DISABLE_USER_SEED = currentStatus; + } + } + + @Test + @Issue("SECURITY-868") + public void rememberMeToken_shouldNotAccept_expirationDurationLargerThanConfigured() throws Exception { + j.jenkins.setDisableRememberMe(false); + + HudsonPrivateSecurityRealm realm = new HudsonPrivateSecurityRealm(false, false, null); + TokenBasedRememberMeServices2 tokenService = (TokenBasedRememberMeServices2) realm.getSecurityComponents().rememberMe; + j.jenkins.setSecurityRealm(realm); + + String username = "alice"; + hudson.model.User alice = realm.createAccount(username, username); + + { // a malicious cookie with expiration too far in the future should not work + JenkinsRule.WebClient wc = j.createWebClient(); + + // by default we have 14 days of validity, + // here we increase artificially the duration of validity, that could be used to have permanent access + long oneDay = TimeUnit.DAYS.toMillis(1); + Cookie cookie = createRememberMeCookie(tokenService, oneDay, alice); + wc.getCookieManager().addCookie(cookie); + + // the application should not use the cookie to connect + assertUserNotConnected(wc, username); + } + + { // a hand crafted cookie with regular expiration duration works + JenkinsRule.WebClient wc = j.createWebClient(); + + // by default we have 14 days of validity, + // here we reduce a bit the expiration date to simulate an "old" cookie (regular usage) + long minusFiveMinutes = TimeUnit.MINUTES.toMillis(-5); + Cookie cookie = createRememberMeCookie(tokenService, minusFiveMinutes, alice); + wc.getCookieManager().addCookie(cookie); + + // if we reactivate the remember me feature, it's ok + assertUserConnected(wc, username); + } + } + + @Test + @Issue("SECURITY-868") + public void rememberMeToken_skipExpirationCheck() throws Exception { + boolean previousConfig = TokenBasedRememberMeServices2.SKIP_TOO_FAR_EXPIRATION_DATE_CHECK; + try { + TokenBasedRememberMeServices2.SKIP_TOO_FAR_EXPIRATION_DATE_CHECK = true; + + j.jenkins.setDisableRememberMe(false); + + HudsonPrivateSecurityRealm realm = new HudsonPrivateSecurityRealm(false, false, null); + TokenBasedRememberMeServices2 tokenService = (TokenBasedRememberMeServices2) realm.getSecurityComponents().rememberMe; + j.jenkins.setSecurityRealm(realm); + + String username = "alice"; + hudson.model.User alice = realm.createAccount(username, username); + + { // a malicious cookie with expiration too far in the future should not work + JenkinsRule.WebClient wc = j.createWebClient(); + + // by default we have 14 days of validity, + // here we increase artificially the duration of validity, that could be used to have permanent access + long oneDay = TimeUnit.DAYS.toMillis(1); + Cookie cookie = createRememberMeCookie(tokenService, oneDay, alice); + wc.getCookieManager().addCookie(cookie); + + // the application should not use the cookie to connect + assertUserConnected(wc, username); + } + + { // a hand crafted cookie with regular expiration duration works + JenkinsRule.WebClient wc = j.createWebClient(); + + // by default we have 14 days of validity, + // here we reduce a bit the expiration date to simulate an "old" cookie (regular usage) + long minusFiveMinutes = TimeUnit.MINUTES.toMillis(-5); + Cookie cookie = createRememberMeCookie(tokenService, minusFiveMinutes, alice); + wc.getCookieManager().addCookie(cookie); + + // if we reactivate the remember me feature, it's ok + assertUserConnected(wc, username); + } + } finally { + TokenBasedRememberMeServices2.SKIP_TOO_FAR_EXPIRATION_DATE_CHECK = previousConfig; + } + } + + private Cookie createRememberMeCookie(TokenBasedRememberMeServices2 tokenService, long deltaDuration, hudson.model.User user) throws Exception { + long tokenValiditySeconds = tokenService.getTokenValiditySeconds(); + long expiryTime = System.currentTimeMillis() + TimeUnit.SECONDS.toMillis(tokenValiditySeconds); + + // the hack + expiryTime += deltaDuration; + + String signatureValue = tokenService.makeTokenSignature(expiryTime, user.getProperty(HudsonPrivateSecurityRealm.Details.class)); + String tokenValue = user.getId() + ":" + expiryTime + ":" + signatureValue; + String tokenValueBase64 = new String(Base64.encodeBase64(tokenValue.getBytes())); + return new Cookie(j.getURL().getHost(), tokenService.getCookieName(), tokenValueBase64); + } + + private void assertUserConnected(JenkinsRule.WebClient wc, String expectedUsername) throws Exception { + XmlPage page = (XmlPage) wc.goTo("whoAmI/api/xml", "application/xml"); + assertThat(page, hasXPath("//name", is(expectedUsername))); + } + + private void assertUserNotConnected(JenkinsRule.WebClient wc, String notExpectedUsername) throws Exception { + XmlPage page = (XmlPage) wc.goTo("whoAmI/api/xml", "application/xml"); + assertThat(page, hasXPath("//name", not(is(notExpectedUsername)))); + } } From ca174a6c7d41f632253ad8c37716b801c5dcad14 Mon Sep 17 00:00:00 2001 From: Josh Soref Date: Mon, 25 Mar 2019 17:03:42 -0400 Subject: [PATCH 05/10] [SECURITY-891] Fold tests back into normal files --- .../java/hudson/model/QueueSEC891Test.java | 88 ------------------- .../src/test/java/hudson/model/QueueTest.java | 68 ++++++++++++++ 2 files changed, 68 insertions(+), 88 deletions(-) delete mode 100644 test/src/test/java/hudson/model/QueueSEC891Test.java diff --git a/test/src/test/java/hudson/model/QueueSEC891Test.java b/test/src/test/java/hudson/model/QueueSEC891Test.java deleted file mode 100644 index b87c4c544c..0000000000 --- a/test/src/test/java/hudson/model/QueueSEC891Test.java +++ /dev/null @@ -1,88 +0,0 @@ -package hudson.model; - -import com.gargoylesoftware.htmlunit.HttpMethod; -import com.gargoylesoftware.htmlunit.Page; -import com.gargoylesoftware.htmlunit.WebRequest; -import hudson.model.Cause.UserIdCause; -import hudson.slaves.NodeProvisionerRule; -import jenkins.model.Jenkins; -import org.junit.Rule; -import org.junit.Test; -import org.jvnet.hudson.test.JenkinsRule; -import org.jvnet.hudson.test.MockAuthorizationStrategy; - -import java.net.URL; -import java.util.function.Function; - -import static org.hamcrest.Matchers.equalTo; -import static org.hamcrest.Matchers.lessThan; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertThat; -import static org.junit.Assert.assertTrue; - -//TODO merge into QueueTest after security patch -public class QueueSEC891Test { - - @Rule - public JenkinsRule r = new NodeProvisionerRule(-1, 0, 10); - - @Test - @Issue("SECURITY-891") - public void doCancelItem_PermissionIsChecked() throws Exception { - checkCancelOperationUsingUrl(item -> "queue/cancelItem?id=" + item.getId()); - } - - @Test - @Issue("SECURITY-891") - public void doCancelQueue_PermissionIsChecked() throws Exception { - checkCancelOperationUsingUrl(item -> "queue/item/" + item.getId() + "/cancelQueue"); - } - - private void checkCancelOperationUsingUrl(Function urlProvider) throws Exception { - Queue q = r.jenkins.getQueue(); - - r.jenkins.setCrumbIssuer(null); - r.jenkins.setSecurityRealm(r.createDummySecurityRealm()); - r.jenkins.setAuthorizationStrategy(new MockAuthorizationStrategy() - .grant(Jenkins.READ, Item.CANCEL).everywhere().to("admin") - .grant(Jenkins.READ).everywhere().to("user") - ); - - // prevent execution to push stuff into the queue - r.jenkins.setNumExecutors(0); - assertThat(q.getItems().length, equalTo(0)); - - FreeStyleProject testProject = r.createFreeStyleProject("test"); - testProject.scheduleBuild(new UserIdCause()); - - Queue.Item[] items = q.getItems(); - assertThat(items.length, equalTo(1)); - Queue.Item currentOne = items[0]; - assertFalse(currentOne.getFuture().isCancelled()); - - WebRequest request = new WebRequest(new URL(r.getURL() + urlProvider.apply(currentOne)), HttpMethod.POST); - - { // user without right cannot cancel - JenkinsRule.WebClient wc = r.createWebClient() - .withRedirectEnabled(false) - .withThrowExceptionOnFailingStatusCode(false); - wc.login("user"); - Page p = wc.getPage(request); - // currently the endpoint return a redirection to the previously visited page, none in our case - // (so force no redirect to avoid false positive error) - assertThat(p.getWebResponse().getStatusCode(), lessThan(400)); - - assertFalse(currentOne.getFuture().isCancelled()); - } - { // user with right can - JenkinsRule.WebClient wc = r.createWebClient() - .withRedirectEnabled(false) - .withThrowExceptionOnFailingStatusCode(false); - wc.login("admin"); - Page p = wc.getPage(request); - assertThat(p.getWebResponse().getStatusCode(), lessThan(400)); - - assertTrue(currentOne.getFuture().isCancelled()); - } - } -} diff --git a/test/src/test/java/hudson/model/QueueTest.java b/test/src/test/java/hudson/model/QueueTest.java index ed550b06be..63d6a1deb1 100644 --- a/test/src/test/java/hudson/model/QueueTest.java +++ b/test/src/test/java/hudson/model/QueueTest.java @@ -23,6 +23,9 @@ */ package hudson.model; +import com.gargoylesoftware.htmlunit.HttpMethod; +import com.gargoylesoftware.htmlunit.Page; +import com.gargoylesoftware.htmlunit.WebRequest; import com.gargoylesoftware.htmlunit.html.DomElement; import com.gargoylesoftware.htmlunit.html.DomNode; import com.gargoylesoftware.htmlunit.html.HtmlFileInput; @@ -93,6 +96,7 @@ import org.junit.Rule; import org.junit.Test; import org.jvnet.hudson.test.Issue; import org.jvnet.hudson.test.JenkinsRule; +import org.jvnet.hudson.test.MockAuthorizationStrategy; import org.jvnet.hudson.test.MockQueueItemAuthenticator; import org.jvnet.hudson.test.SequenceLock; import org.jvnet.hudson.test.SleepBuilder; @@ -107,6 +111,7 @@ import javax.servlet.http.HttpServletResponse; import java.io.File; import java.io.IOException; import java.net.HttpURLConnection; +import java.net.URL; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -121,9 +126,12 @@ import java.util.concurrent.ScheduledThreadPoolExecutor; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicInteger; +import java.util.function.Function; import java.util.logging.Level; import static org.hamcrest.Matchers.contains; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.lessThan; import static org.hamcrest.Matchers.nullValue; import static org.junit.Assert.*; import org.junit.Ignore; @@ -1067,4 +1075,64 @@ public class QueueTest { } } } + + @Test + @Issue("SECURITY-891") + public void doCancelItem_PermissionIsChecked() throws Exception { + checkCancelOperationUsingUrl(item -> "queue/cancelItem?id=" + item.getId()); + } + + @Test + @Issue("SECURITY-891") + public void doCancelQueue_PermissionIsChecked() throws Exception { + checkCancelOperationUsingUrl(item -> "queue/item/" + item.getId() + "/cancelQueue"); + } + + private void checkCancelOperationUsingUrl(Function urlProvider) throws Exception { + Queue q = r.jenkins.getQueue(); + + r.jenkins.setCrumbIssuer(null); + r.jenkins.setSecurityRealm(r.createDummySecurityRealm()); + r.jenkins.setAuthorizationStrategy(new MockAuthorizationStrategy() + .grant(Jenkins.READ, Item.CANCEL).everywhere().to("admin") + .grant(Jenkins.READ).everywhere().to("user") + ); + + // prevent execution to push stuff into the queue + r.jenkins.setNumExecutors(0); + assertThat(q.getItems().length, equalTo(0)); + + FreeStyleProject testProject = r.createFreeStyleProject("test"); + testProject.scheduleBuild(new UserIdCause()); + + Queue.Item[] items = q.getItems(); + assertThat(items.length, equalTo(1)); + Queue.Item currentOne = items[0]; + assertFalse(currentOne.getFuture().isCancelled()); + + WebRequest request = new WebRequest(new URL(r.getURL() + urlProvider.apply(currentOne)), HttpMethod.POST); + + { // user without right cannot cancel + JenkinsRule.WebClient wc = r.createWebClient() + .withRedirectEnabled(false) + .withThrowExceptionOnFailingStatusCode(false); + wc.login("user"); + Page p = wc.getPage(request); + // currently the endpoint return a redirection to the previously visited page, none in our case + // (so force no redirect to avoid false positive error) + assertThat(p.getWebResponse().getStatusCode(), lessThan(400)); + + assertFalse(currentOne.getFuture().isCancelled()); + } + { // user with right can + JenkinsRule.WebClient wc = r.createWebClient() + .withRedirectEnabled(false) + .withThrowExceptionOnFailingStatusCode(false); + wc.login("admin"); + Page p = wc.getPage(request); + assertThat(p.getWebResponse().getStatusCode(), lessThan(400)); + + assertTrue(currentOne.getFuture().isCancelled()); + } + } } From f7a889df3f62ed9140488ace6702f080f9a68c6a Mon Sep 17 00:00:00 2001 From: Josh Soref Date: Mon, 25 Mar 2019 17:13:51 -0400 Subject: [PATCH 06/10] [SECURITY-897] Fold tests back into normal files --- .../hudson/model/UserRestartSEC897Test.java | 73 ------------------- .../java/hudson/model/UserRestartTest.java | 66 ++++++++++++++++- 2 files changed, 63 insertions(+), 76 deletions(-) delete mode 100644 test/src/test/java/hudson/model/UserRestartSEC897Test.java diff --git a/test/src/test/java/hudson/model/UserRestartSEC897Test.java b/test/src/test/java/hudson/model/UserRestartSEC897Test.java deleted file mode 100644 index a7121591f9..0000000000 --- a/test/src/test/java/hudson/model/UserRestartSEC897Test.java +++ /dev/null @@ -1,73 +0,0 @@ -package hudson.model; - -import com.gargoylesoftware.htmlunit.WebRequest; -import hudson.FilePath; -import org.junit.Rule; -import org.junit.Test; -import org.junit.runners.model.Statement; -import org.jvnet.hudson.test.JenkinsRule; -import org.jvnet.hudson.test.RestartableJenkinsRule; - -import java.net.URL; -import java.nio.charset.StandardCharsets; -import java.util.Base64; - -import static org.hamcrest.core.IsEqual.equalTo; -import static org.junit.Assert.assertThat; - -//TODO after the security fix, it could be merged inside UserRestartTest -public class UserRestartSEC897Test { - - @Rule - public RestartableJenkinsRule rr = new RestartableJenkinsRule(); - - @Test - @Issue("SECURITY-897") - public void legacyConfigMoveCannotEscapeUserFolder() { - rr.addStep(new Statement() { - @Override - public void evaluate() throws Throwable { - rr.j.jenkins.setSecurityRealm(rr.j.createDummySecurityRealm()); - assertThat(rr.j.jenkins.isUseSecurity(), equalTo(true)); - - // in order to create the folder "users" - User.getById("admin", true).save(); - - { // attempt with ".." - JenkinsRule.WebClient wc = rr.j.createWebClient() - .withThrowExceptionOnFailingStatusCode(false); - - WebRequest request = new WebRequest(new URL(rr.j.jenkins.getRootUrl() + "whoAmI/api/xml")); - request.setAdditionalHeader("Authorization", base64("..", "any-password")); - wc.getPage(request); - } - { // attempt with "../users/.." - JenkinsRule.WebClient wc = rr.j.createWebClient() - .withThrowExceptionOnFailingStatusCode(false); - - WebRequest request = new WebRequest(new URL(rr.j.jenkins.getRootUrl() + "whoAmI/api/xml")); - request.setAdditionalHeader("Authorization", base64("../users/..", "any-password")); - wc.getPage(request); - } - - // security is still active - assertThat(rr.j.jenkins.isUseSecurity(), equalTo(true)); - // but, the config file was moved - FilePath rootPath = rr.j.jenkins.getRootPath(); - assertThat(rootPath.child("config.xml").exists(), equalTo(true)); - } - }); - rr.addStep(new Statement() { - @Override - public void evaluate() throws Throwable { - assertThat(rr.j.jenkins.isUseSecurity(), equalTo(true)); - FilePath rootPath = rr.j.jenkins.getRootPath(); - assertThat(rootPath.child("config.xml").exists(), equalTo(true)); - } - }); - } - - private String base64(String login, String password) { - return "Basic " + Base64.getEncoder().encodeToString((login + ":" + password).getBytes(StandardCharsets.UTF_8)); - } -} diff --git a/test/src/test/java/hudson/model/UserRestartTest.java b/test/src/test/java/hudson/model/UserRestartTest.java index 2f6cec0750..d1d8253064 100644 --- a/test/src/test/java/hudson/model/UserRestartTest.java +++ b/test/src/test/java/hudson/model/UserRestartTest.java @@ -24,15 +24,26 @@ package hudson.model; +import com.gargoylesoftware.htmlunit.WebRequest; +import hudson.FilePath; import hudson.tasks.Mailer; -import static org.hamcrest.Matchers.containsString; -import static org.hamcrest.Matchers.not; -import static org.junit.Assert.*; import org.junit.Rule; import org.junit.Test; +import org.junit.runners.model.Statement; + import org.jvnet.hudson.test.Issue; +import org.jvnet.hudson.test.JenkinsRule; import org.jvnet.hudson.test.RestartableJenkinsRule; +import java.net.URL; +import java.nio.charset.StandardCharsets; +import java.util.Base64; + +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.not; +import static org.hamcrest.core.IsEqual.equalTo; +import static org.junit.Assert.*; + public class UserRestartTest { @Rule @@ -81,4 +92,53 @@ public class UserRestartTest { } } + @Test + @Issue("SECURITY-897") + public void legacyConfigMoveCannotEscapeUserFolder() { + rr.addStep(new Statement() { + @Override + public void evaluate() throws Throwable { + rr.j.jenkins.setSecurityRealm(rr.j.createDummySecurityRealm()); + assertThat(rr.j.jenkins.isUseSecurity(), equalTo(true)); + + // in order to create the folder "users" + User.getById("admin", true).save(); + + { // attempt with ".." + JenkinsRule.WebClient wc = rr.j.createWebClient() + .withThrowExceptionOnFailingStatusCode(false); + + WebRequest request = new WebRequest(new URL(rr.j.jenkins.getRootUrl() + "whoAmI/api/xml")); + request.setAdditionalHeader("Authorization", base64("..", "any-password")); + wc.getPage(request); + } + { // attempt with "../users/.." + JenkinsRule.WebClient wc = rr.j.createWebClient() + .withThrowExceptionOnFailingStatusCode(false); + + WebRequest request = new WebRequest(new URL(rr.j.jenkins.getRootUrl() + "whoAmI/api/xml")); + request.setAdditionalHeader("Authorization", base64("../users/..", "any-password")); + wc.getPage(request); + } + + // security is still active + assertThat(rr.j.jenkins.isUseSecurity(), equalTo(true)); + // but, the config file was moved + FilePath rootPath = rr.j.jenkins.getRootPath(); + assertThat(rootPath.child("config.xml").exists(), equalTo(true)); + } + }); + rr.addStep(new Statement() { + @Override + public void evaluate() throws Throwable { + assertThat(rr.j.jenkins.isUseSecurity(), equalTo(true)); + FilePath rootPath = rr.j.jenkins.getRootPath(); + assertThat(rootPath.child("config.xml").exists(), equalTo(true)); + } + }); + } + + private String base64(String login, String password) { + return "Basic " + Base64.getEncoder().encodeToString((login + ":" + password).getBytes(StandardCharsets.UTF_8)); + } } From ad15e52937df15b1da85f11245b7ad6c411fdc6c Mon Sep 17 00:00:00 2001 From: Josh Soref Date: Mon, 25 Mar 2019 17:14:18 -0400 Subject: [PATCH 07/10] [SECURITY-925] Fold tests back into normal files --- .../test/java/hudson/PluginSEC925Test.java | 35 ------------------- test/src/test/java/hudson/PluginTest.java | 21 +++++++++++ 2 files changed, 21 insertions(+), 35 deletions(-) delete mode 100644 test/src/test/java/hudson/PluginSEC925Test.java diff --git a/test/src/test/java/hudson/PluginSEC925Test.java b/test/src/test/java/hudson/PluginSEC925Test.java deleted file mode 100644 index beeca3183d..0000000000 --- a/test/src/test/java/hudson/PluginSEC925Test.java +++ /dev/null @@ -1,35 +0,0 @@ -package hudson; - -import hudson.model.UpdateCenter; -import jenkins.model.Jenkins; -import org.junit.Rule; -import org.junit.Test; -import org.jvnet.hudson.test.Issue; -import org.jvnet.hudson.test.JenkinsRule; - -import javax.servlet.http.HttpServletResponse; -import java.util.Arrays; -import java.util.List; -import java.util.concurrent.Future; -import org.junit.Ignore; - -//TODO merge it within PluginTest after the security release -public class PluginSEC925Test { - - @Rule - public JenkinsRule r = new JenkinsRule(); - - @Ignore("TODO observed to fail in CI with 404 due to external UC issues") - @Test - @Issue("SECURITY-925") - public void preventTimestamp2_toBeServed() throws Exception { - // impossible to use installDetachedPlugin("credentials") since we want to have it exploded like with WAR - Jenkins.getInstance().getUpdateCenter().getSites().get(0).updateDirectlyNow(false); - List> pluginInstalled = r.jenkins.pluginManager.install(Arrays.asList("credentials"), true); - - for (Future job : pluginInstalled) { - job.get(); - } - r.createWebClient().assertFails("plugin/credentials/.timestamp2", HttpServletResponse.SC_BAD_REQUEST); - } -} diff --git a/test/src/test/java/hudson/PluginTest.java b/test/src/test/java/hudson/PluginTest.java index 17f7329e00..d3301f3dfa 100644 --- a/test/src/test/java/hudson/PluginTest.java +++ b/test/src/test/java/hudson/PluginTest.java @@ -24,6 +24,14 @@ package hudson; +import hudson.model.UpdateCenter; +import jenkins.model.Jenkins; + +import java.util.Arrays; +import java.util.List; +import java.util.concurrent.Future; +import org.junit.Ignore; + import javax.servlet.http.HttpServletResponse; import org.junit.Test; import org.junit.Rule; @@ -57,4 +65,17 @@ public class PluginTest { r.createWebClient().assertFails("plugin/credentials/meta-inf/manifest.mf", HttpServletResponse.SC_BAD_REQUEST); } + @Ignore("TODO observed to fail in CI with 404 due to external UC issues") + @Test + @Issue("SECURITY-925") + public void preventTimestamp2_toBeServed() throws Exception { + // impossible to use installDetachedPlugin("credentials") since we want to have it exploded like with WAR + Jenkins.getInstance().getUpdateCenter().getSites().get(0).updateDirectlyNow(false); + List> pluginInstalled = r.jenkins.pluginManager.install(Arrays.asList("credentials"), true); + + for (Future job : pluginInstalled) { + job.get(); + } + r.createWebClient().assertFails("plugin/credentials/.timestamp2", HttpServletResponse.SC_BAD_REQUEST); + } } From 24d350d8a6f033bf32c94b3f7cca2d1ab2f9df03 Mon Sep 17 00:00:00 2001 From: Josh Soref Date: Mon, 25 Mar 2019 14:44:48 -0400 Subject: [PATCH 08/10] [SECURITY-996] Fold tests back into normal files --- ...kenBasedRememberMeServices2SEC996Test.java | 88 ------------------- .../TokenBasedRememberMeServices2Test.java | 43 +++++++++ 2 files changed, 43 insertions(+), 88 deletions(-) delete mode 100644 test/src/test/java/hudson/security/TokenBasedRememberMeServices2SEC996Test.java diff --git a/test/src/test/java/hudson/security/TokenBasedRememberMeServices2SEC996Test.java b/test/src/test/java/hudson/security/TokenBasedRememberMeServices2SEC996Test.java deleted file mode 100644 index bcb7ddea16..0000000000 --- a/test/src/test/java/hudson/security/TokenBasedRememberMeServices2SEC996Test.java +++ /dev/null @@ -1,88 +0,0 @@ -/* - * The MIT License - * - * Copyright (c) 2018, CloudBees, Inc. - * - * 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.security; - -import com.gargoylesoftware.htmlunit.util.Cookie; -import com.gargoylesoftware.htmlunit.xml.XmlPage; -import org.junit.Rule; -import org.junit.Test; -import org.jvnet.hudson.test.Issue; -import org.jvnet.hudson.test.JenkinsRule; - -import static org.hamcrest.Matchers.is; -import static org.hamcrest.Matchers.not; -import static org.hamcrest.text.IsEmptyString.isEmptyString; -import static org.hamcrest.xml.HasXPath.hasXPath; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertThat; - -public class TokenBasedRememberMeServices2SEC996Test { - @Rule - public JenkinsRule j = new JenkinsRule(); - - @Test - @Issue("SECURITY-996") - public void rememberMeToken_shouldNotBeRead_ifOptionIsDisabled() throws Exception { - j.jenkins.setDisableRememberMe(false); - j.jenkins.setSecurityRealm(j.createDummySecurityRealm()); - - Cookie rememberMeCookie = null; - { - JenkinsRule.WebClient wc = j.createWebClient(); - wc.login("alice", "alice", true); - - - // we should see a remember me cookie - rememberMeCookie = getRememberMeCookie(wc); - assertNotNull(rememberMeCookie); - assertThat(rememberMeCookie.getValue(), not(isEmptyString())); - } - - j.jenkins.setDisableRememberMe(true); - { - JenkinsRule.WebClient wc = j.createWebClient(); - - wc.getCookieManager().addCookie(rememberMeCookie); - - // the application should not use the cookie to connect - XmlPage page = (XmlPage) wc.goTo("whoAmI/api/xml", "application/xml"); - assertThat(page, hasXPath("//name", not(is("alice")))); - } - - j.jenkins.setDisableRememberMe(false); - { - JenkinsRule.WebClient wc = j.createWebClient(); - - wc.getCookieManager().addCookie(rememberMeCookie); - - // if we reactivate the remember me feature, it's ok - XmlPage page = (XmlPage) wc.goTo("whoAmI/api/xml", "application/xml"); - assertThat(page, hasXPath("//name", is("alice"))); - } - } - - private Cookie getRememberMeCookie(JenkinsRule.WebClient wc) { - return wc.getCookieManager().getCookie(TokenBasedRememberMeServices2.ACEGI_SECURITY_HASHED_REMEMBER_ME_COOKIE_KEY); - } -} diff --git a/test/src/test/java/hudson/security/TokenBasedRememberMeServices2Test.java b/test/src/test/java/hudson/security/TokenBasedRememberMeServices2Test.java index e4612e5aa5..e98eb4354a 100644 --- a/test/src/test/java/hudson/security/TokenBasedRememberMeServices2Test.java +++ b/test/src/test/java/hudson/security/TokenBasedRememberMeServices2Test.java @@ -35,7 +35,9 @@ import java.util.concurrent.TimeUnit; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.not; +import static org.hamcrest.text.IsEmptyString.isEmptyString; import static org.hamcrest.xml.HasXPath.hasXPath; +import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertThat; public class TokenBasedRememberMeServices2Test { @@ -309,4 +311,45 @@ public class TokenBasedRememberMeServices2Test { XmlPage page = (XmlPage) wc.goTo("whoAmI/api/xml", "application/xml"); assertThat(page, hasXPath("//name", not(is(notExpectedUsername)))); } + + @Test + @Issue("SECURITY-996") + public void rememberMeToken_shouldNotBeRead_ifOptionIsDisabled() throws Exception { + j.jenkins.setDisableRememberMe(false); + j.jenkins.setSecurityRealm(j.createDummySecurityRealm()); + + Cookie rememberMeCookie = null; + { + JenkinsRule.WebClient wc = j.createWebClient(); + wc.login("alice", "alice", true); + + + // we should see a remember me cookie + rememberMeCookie = getRememberMeCookie(wc); + assertNotNull(rememberMeCookie); + assertThat(rememberMeCookie.getValue(), not(isEmptyString())); + } + + j.jenkins.setDisableRememberMe(true); + { + JenkinsRule.WebClient wc = j.createWebClient(); + + wc.getCookieManager().addCookie(rememberMeCookie); + + // the application should not use the cookie to connect + XmlPage page = (XmlPage) wc.goTo("whoAmI/api/xml", "application/xml"); + assertThat(page, hasXPath("//name", not(is("alice")))); + } + + j.jenkins.setDisableRememberMe(false); + { + JenkinsRule.WebClient wc = j.createWebClient(); + + wc.getCookieManager().addCookie(rememberMeCookie); + + // if we reactivate the remember me feature, it's ok + XmlPage page = (XmlPage) wc.goTo("whoAmI/api/xml", "application/xml"); + assertThat(page, hasXPath("//name", is("alice"))); + } + } } From 279b9616fc14fe5ae28df03747d84911ab77eb7d Mon Sep 17 00:00:00 2001 From: Josh Soref Date: Mon, 25 Mar 2019 15:00:28 -0400 Subject: [PATCH 09/10] [SECURITY-1158] Fold tests back into normal files --- ...udsonPrivateSecurityRealm2SEC1158Test.java | 73 ------------------- .../HudsonPrivateSecurityRealmTest.java | 30 ++++++++ 2 files changed, 30 insertions(+), 73 deletions(-) delete mode 100644 test/src/test/java/hudson/security/HudsonPrivateSecurityRealm2SEC1158Test.java diff --git a/test/src/test/java/hudson/security/HudsonPrivateSecurityRealm2SEC1158Test.java b/test/src/test/java/hudson/security/HudsonPrivateSecurityRealm2SEC1158Test.java deleted file mode 100644 index ad0cb99660..0000000000 --- a/test/src/test/java/hudson/security/HudsonPrivateSecurityRealm2SEC1158Test.java +++ /dev/null @@ -1,73 +0,0 @@ -/* - * The MIT License - * - * Copyright (c) 2015, CloudBees, Inc - * - * 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.security; - -import com.gargoylesoftware.htmlunit.html.HtmlPage; -import com.gargoylesoftware.htmlunit.util.Cookie; -import hudson.security.pages.SignupPage; -import org.junit.Rule; -import org.junit.Test; -import org.jvnet.hudson.test.Issue; -import org.jvnet.hudson.test.JenkinsRule; - -import static org.hamcrest.Matchers.containsString; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotEquals; -import static org.junit.Assert.assertThat; - -//TODO to be merged into HudsonPrivateSecurityRealm2Test after security release -public class HudsonPrivateSecurityRealm2SEC1158Test { - - @Rule - public JenkinsRule rule = new JenkinsRule(); - - @Test - @Issue("SECURITY-1158") - public void singupNoLongerVulnerableToSessionFixation() throws Exception { - HudsonPrivateSecurityRealm securityRealm = new HudsonPrivateSecurityRealm(true, false, null); - rule.jenkins.setSecurityRealm(securityRealm); - JenkinsRule.WebClient wc = rule.createWebClient(); - - // to trigger the creation of a session - wc.goTo(""); - Cookie sessionBefore = wc.getCookieManager().getCookie("JSESSIONID"); - String sessionIdBefore = sessionBefore.getValue(); - - 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.getAnchorByHref("/jenkins/user/alice").getTextContent(), containsString("Alice User")); - - assertEquals("Alice User", securityRealm.getUser("alice").getDisplayName()); - - Cookie sessionAfter = wc.getCookieManager().getCookie("JSESSIONID"); - String sessionIdAfter = sessionAfter.getValue(); - - assertNotEquals(sessionIdAfter, sessionIdBefore); - } -} diff --git a/test/src/test/java/hudson/security/HudsonPrivateSecurityRealmTest.java b/test/src/test/java/hudson/security/HudsonPrivateSecurityRealmTest.java index 557dbbea4e..2f24ea3ae7 100644 --- a/test/src/test/java/hudson/security/HudsonPrivateSecurityRealmTest.java +++ b/test/src/test/java/hudson/security/HudsonPrivateSecurityRealmTest.java @@ -27,6 +27,7 @@ package hudson.security; import com.gargoylesoftware.htmlunit.FailingHttpStatusCodeException; import com.gargoylesoftware.htmlunit.HttpMethod; import com.gargoylesoftware.htmlunit.WebRequest; +import com.gargoylesoftware.htmlunit.util.Cookie; import com.gargoylesoftware.htmlunit.html.HtmlForm; import com.gargoylesoftware.htmlunit.html.HtmlPage; import com.gargoylesoftware.htmlunit.util.NameValuePair; @@ -605,4 +606,33 @@ public class HudsonPrivateSecurityRealmTest { assertThat(success.getElementById("main-panel").getTextContent(), not(containsString("Success"))); assertThat(success.getElementById("main-panel").getTextContent(), containsString(regex)); } + + @Test + @Issue("SECURITY-1158") + public void singupNoLongerVulnerableToSessionFixation() throws Exception { + HudsonPrivateSecurityRealm securityRealm = new HudsonPrivateSecurityRealm(true, false, null); + j.jenkins.setSecurityRealm(securityRealm); + JenkinsRule.WebClient wc = j.createWebClient(); + + // to trigger the creation of a session + wc.goTo(""); + Cookie sessionBefore = wc.getCookieManager().getCookie("JSESSIONID"); + String sessionIdBefore = sessionBefore.getValue(); + + 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(j); + assertThat(success.getElementById("main-panel").getTextContent(), containsString("Success")); + assertThat(success.getAnchorByHref("/jenkins/user/alice").getTextContent(), containsString("Alice User")); + + assertEquals("Alice User", securityRealm.getUser("alice").getDisplayName()); + + Cookie sessionAfter = wc.getCookieManager().getCookie("JSESSIONID"); + String sessionIdAfter = sessionAfter.getValue(); + + assertNotEquals(sessionIdAfter, sessionIdBefore); + } } From d9b920183e5038071b49dcf98250669405a319d0 Mon Sep 17 00:00:00 2001 From: Josh Soref Date: Mon, 25 Mar 2019 15:07:52 -0400 Subject: [PATCH 10/10] [SECURITY-1245] Fold tests back into normal files --- ...HudsonPrivateSecurityRealmSEC1245Test.java | 162 ------------------ .../HudsonPrivateSecurityRealmTest.java | 122 ++++++++++++- 2 files changed, 119 insertions(+), 165 deletions(-) delete mode 100644 test/src/test/java/hudson/security/HudsonPrivateSecurityRealmSEC1245Test.java diff --git a/test/src/test/java/hudson/security/HudsonPrivateSecurityRealmSEC1245Test.java b/test/src/test/java/hudson/security/HudsonPrivateSecurityRealmSEC1245Test.java deleted file mode 100644 index bcda8c4c92..0000000000 --- a/test/src/test/java/hudson/security/HudsonPrivateSecurityRealmSEC1245Test.java +++ /dev/null @@ -1,162 +0,0 @@ -/* - * The MIT License - * - * Copyright (c) 2015, CloudBees, Inc. and others - * - * 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.security; - -import com.gargoylesoftware.htmlunit.html.HtmlForm; -import com.gargoylesoftware.htmlunit.html.HtmlPage; -import com.gargoylesoftware.htmlunit.html.HtmlPasswordInput; -import com.gargoylesoftware.htmlunit.xml.XmlPage; -import hudson.model.User; -import jenkins.security.seed.UserSeedProperty; -import org.junit.Rule; -import org.junit.Test; -import org.jvnet.hudson.test.For; -import org.jvnet.hudson.test.Issue; -import org.jvnet.hudson.test.JenkinsRule; -import org.jvnet.hudson.test.JenkinsRule.WebClient; - -import static org.hamcrest.Matchers.is; -import static org.hamcrest.Matchers.not; -import static org.hamcrest.xml.HasXPath.hasXPath; -import static org.junit.Assert.assertThat; - -@For({UserSeedProperty.class, HudsonPrivateSecurityRealm.class}) -public class HudsonPrivateSecurityRealmSEC1245Test { - - @Rule - public JenkinsRule j = new JenkinsRule(); - - @Test - @Issue("SECURITY-1245") - public void changingPassword_mustInvalidateAllSessions() throws Exception { - User alice = prepareRealmAndAlice(); - String initialSeed = alice.getProperty(UserSeedProperty.class).getSeed(); - - WebClient wc = j.createWebClient(); - WebClient wc_anotherTab = j.createWebClient(); - - wc.login(alice.getId()); - assertUserConnected(wc, alice.getId()); - - wc_anotherTab.login(alice.getId()); - assertUserConnected(wc_anotherTab, alice.getId()); - - HtmlPage configurePage = wc.goTo(alice.getUrl() + "/configure"); - HtmlPasswordInput password1 = configurePage.getElementByName("user.password"); - HtmlPasswordInput password2 = configurePage.getElementByName("user.password2"); - - password1.setText("alice2"); - password2.setText("alice2"); - - HtmlForm form = configurePage.getFormByName("config"); - j.submit(form); - - assertUserNotConnected(wc, alice.getId()); - assertUserNotConnected(wc_anotherTab, alice.getId()); - - String seedAfter = alice.getProperty(UserSeedProperty.class).getSeed(); - assertThat(seedAfter, not(is(initialSeed))); - } - - @Test - @Issue("SECURITY-1245") - public void notChangingPassword_hasNoImpactOnSeed() throws Exception { - User alice = prepareRealmAndAlice(); - String initialSeed = alice.getProperty(UserSeedProperty.class).getSeed(); - - WebClient wc = j.createWebClient(); - WebClient wc_anotherTab = j.createWebClient(); - - wc.login(alice.getId()); - assertUserConnected(wc, alice.getId()); - - wc_anotherTab.login(alice.getId()); - assertUserConnected(wc_anotherTab, alice.getId()); - - HtmlPage configurePage = wc.goTo(alice.getUrl() + "/configure"); - // not changing password this time - HtmlForm form = configurePage.getFormByName("config"); - j.submit(form); - - assertUserConnected(wc, alice.getId()); - assertUserConnected(wc_anotherTab, alice.getId()); - - String seedAfter = alice.getProperty(UserSeedProperty.class).getSeed(); - assertThat(seedAfter, is(initialSeed)); - } - - @Test - @Issue("SECURITY-1245") - public void changingPassword_withSeedDisable_hasNoImpact() throws Exception { - boolean previousConfig = UserSeedProperty.DISABLE_USER_SEED; - try { - UserSeedProperty.DISABLE_USER_SEED = true; - - User alice = prepareRealmAndAlice(); - - WebClient wc = j.createWebClient(); - WebClient wc_anotherTab = j.createWebClient(); - - wc.login(alice.getId()); - assertUserConnected(wc, alice.getId()); - - wc_anotherTab.login(alice.getId()); - assertUserConnected(wc_anotherTab, alice.getId()); - - HtmlPage configurePage = wc.goTo(alice.getUrl() + "/configure"); - HtmlPasswordInput password1 = configurePage.getElementByName("user.password"); - HtmlPasswordInput password2 = configurePage.getElementByName("user.password2"); - - password1.setText("alice2"); - password2.setText("alice2"); - - HtmlForm form = configurePage.getFormByName("config"); - j.submit(form); - - assertUserConnected(wc, alice.getId()); - assertUserConnected(wc_anotherTab, alice.getId()); - } finally { - UserSeedProperty.DISABLE_USER_SEED = previousConfig; - } - } - - private User prepareRealmAndAlice() throws Exception { - j.jenkins.setDisableRememberMe(false); - HudsonPrivateSecurityRealm securityRealm = new HudsonPrivateSecurityRealm(false, false, null); - j.jenkins.setSecurityRealm(securityRealm); - - return securityRealm.createAccount("alice", "alice"); - } - - private void assertUserConnected(JenkinsRule.WebClient wc, String expectedUsername) throws Exception { - XmlPage page = (XmlPage) wc.goTo("whoAmI/api/xml", "application/xml"); - assertThat(page, hasXPath("//name", is(expectedUsername))); - } - - private void assertUserNotConnected(JenkinsRule.WebClient wc, String notExpectedUsername) throws Exception { - XmlPage page = (XmlPage) wc.goTo("whoAmI/api/xml", "application/xml"); - assertThat(page, hasXPath("//name", not(is(notExpectedUsername)))); - } -} diff --git a/test/src/test/java/hudson/security/HudsonPrivateSecurityRealmTest.java b/test/src/test/java/hudson/security/HudsonPrivateSecurityRealmTest.java index 2f24ea3ae7..dec921ea24 100644 --- a/test/src/test/java/hudson/security/HudsonPrivateSecurityRealmTest.java +++ b/test/src/test/java/hudson/security/HudsonPrivateSecurityRealmTest.java @@ -30,6 +30,7 @@ import com.gargoylesoftware.htmlunit.WebRequest; import com.gargoylesoftware.htmlunit.util.Cookie; import com.gargoylesoftware.htmlunit.html.HtmlForm; import com.gargoylesoftware.htmlunit.html.HtmlPage; +import com.gargoylesoftware.htmlunit.html.HtmlPasswordInput; import com.gargoylesoftware.htmlunit.util.NameValuePair; import com.gargoylesoftware.htmlunit.xml.XmlPage; import hudson.ExtensionList; @@ -46,15 +47,16 @@ import java.util.Collections; import java.util.List; import jenkins.security.ApiTokenProperty; +import jenkins.security.SecurityListener; +import jenkins.security.apitoken.ApiTokenPropertyConfiguration; +import jenkins.security.apitoken.ApiTokenTestHelper; +import jenkins.security.seed.UserSeedProperty; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.not; import static org.hamcrest.xml.HasXPath.hasXPath; import static org.junit.Assert.*; -import jenkins.security.apitoken.ApiTokenPropertyConfiguration; -import jenkins.security.SecurityListener; -import jenkins.security.apitoken.ApiTokenTestHelper; import org.apache.commons.lang.StringUtils; import java.lang.reflect.Field; @@ -62,6 +64,7 @@ import java.lang.reflect.Field; import org.junit.Before; import org.junit.Rule; import org.junit.Test; +import org.jvnet.hudson.test.For; import org.jvnet.hudson.test.Issue; import org.jvnet.hudson.test.JenkinsRule; import org.jvnet.hudson.test.JenkinsRule.WebClient; @@ -71,6 +74,7 @@ import org.mindrot.jbcrypt.BCrypt; import javax.annotation.Nonnull; +@For({UserSeedProperty.class, HudsonPrivateSecurityRealm.class}) public class HudsonPrivateSecurityRealmTest { @Rule @@ -635,4 +639,116 @@ public class HudsonPrivateSecurityRealmTest { assertNotEquals(sessionIdAfter, sessionIdBefore); } + + @Test + @Issue("SECURITY-1245") + public void changingPassword_mustInvalidateAllSessions() throws Exception { + User alice = prepareRealmAndAlice(); + String initialSeed = alice.getProperty(UserSeedProperty.class).getSeed(); + + WebClient wc = j.createWebClient(); + WebClient wc_anotherTab = j.createWebClient(); + + wc.login(alice.getId()); + assertUserConnected(wc, alice.getId()); + + wc_anotherTab.login(alice.getId()); + assertUserConnected(wc_anotherTab, alice.getId()); + + HtmlPage configurePage = wc.goTo(alice.getUrl() + "/configure"); + HtmlPasswordInput password1 = configurePage.getElementByName("user.password"); + HtmlPasswordInput password2 = configurePage.getElementByName("user.password2"); + + password1.setText("alice2"); + password2.setText("alice2"); + + HtmlForm form = configurePage.getFormByName("config"); + j.submit(form); + + assertUserNotConnected(wc, alice.getId()); + assertUserNotConnected(wc_anotherTab, alice.getId()); + + String seedAfter = alice.getProperty(UserSeedProperty.class).getSeed(); + assertThat(seedAfter, not(is(initialSeed))); + } + + @Test + @Issue("SECURITY-1245") + public void notChangingPassword_hasNoImpactOnSeed() throws Exception { + User alice = prepareRealmAndAlice(); + String initialSeed = alice.getProperty(UserSeedProperty.class).getSeed(); + + WebClient wc = j.createWebClient(); + WebClient wc_anotherTab = j.createWebClient(); + + wc.login(alice.getId()); + assertUserConnected(wc, alice.getId()); + + wc_anotherTab.login(alice.getId()); + assertUserConnected(wc_anotherTab, alice.getId()); + + HtmlPage configurePage = wc.goTo(alice.getUrl() + "/configure"); + // not changing password this time + HtmlForm form = configurePage.getFormByName("config"); + j.submit(form); + + assertUserConnected(wc, alice.getId()); + assertUserConnected(wc_anotherTab, alice.getId()); + + String seedAfter = alice.getProperty(UserSeedProperty.class).getSeed(); + assertThat(seedAfter, is(initialSeed)); + } + + @Test + @Issue("SECURITY-1245") + public void changingPassword_withSeedDisable_hasNoImpact() throws Exception { + boolean previousConfig = UserSeedProperty.DISABLE_USER_SEED; + try { + UserSeedProperty.DISABLE_USER_SEED = true; + + User alice = prepareRealmAndAlice(); + + WebClient wc = j.createWebClient(); + WebClient wc_anotherTab = j.createWebClient(); + + wc.login(alice.getId()); + assertUserConnected(wc, alice.getId()); + + wc_anotherTab.login(alice.getId()); + assertUserConnected(wc_anotherTab, alice.getId()); + + HtmlPage configurePage = wc.goTo(alice.getUrl() + "/configure"); + HtmlPasswordInput password1 = configurePage.getElementByName("user.password"); + HtmlPasswordInput password2 = configurePage.getElementByName("user.password2"); + + password1.setText("alice2"); + password2.setText("alice2"); + + HtmlForm form = configurePage.getFormByName("config"); + j.submit(form); + + assertUserConnected(wc, alice.getId()); + assertUserConnected(wc_anotherTab, alice.getId()); + } finally { + UserSeedProperty.DISABLE_USER_SEED = previousConfig; + } + } + + private User prepareRealmAndAlice() throws Exception { + j.jenkins.setDisableRememberMe(false); + HudsonPrivateSecurityRealm securityRealm = new HudsonPrivateSecurityRealm(false, false, null); + j.jenkins.setSecurityRealm(securityRealm); + + return securityRealm.createAccount("alice", "alice"); + } + + private void assertUserConnected(JenkinsRule.WebClient wc, String expectedUsername) throws Exception { + XmlPage page = (XmlPage) wc.goTo("whoAmI/api/xml", "application/xml"); + assertThat(page, hasXPath("//name", is(expectedUsername))); + } + + private void assertUserNotConnected(JenkinsRule.WebClient wc, String notExpectedUsername) throws Exception { + XmlPage page = (XmlPage) wc.goTo("whoAmI/api/xml", "application/xml"); + assertThat(page, hasXPath("//name", not(is(notExpectedUsername)))); + } }