Skip to content

Commit

Permalink
SECURITY-3473
Browse files Browse the repository at this point in the history
  • Loading branch information
jmdesprez committed Oct 30, 2024
1 parent 3649a04 commit 5422614
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 0 deletions.
4 changes: 4 additions & 0 deletions src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java
Original file line number Diff line number Diff line change
Expand Up @@ -1269,6 +1269,10 @@ public void doFinishLogin(StaplerRequest request, StaplerResponse response) thro
// Jenkins stuff correctly
// also should have its own URL to make the code easier to follow :)

if (!sessionStore.renewSession(webContext)) {

Check warning on line 1272 in src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line

Line 1272 is only partially covered, one branch is missing
throw new TechnicalException("Could not create a new session");

Check warning on line 1273 in src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered line

Line 1273 is not covered by tests
}

Credentials credentials = client.getCredentials(webContext, sessionStore)
.orElseThrow(() -> new Failure("Could not extract credentials from request"));

Expand Down
50 changes: 50 additions & 0 deletions src/test/java/org/jenkinsci/plugins/oic/PluginTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.function.Consumer;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
Expand All @@ -46,7 +47,9 @@
import jenkins.security.ApiTokenProperty;
import jenkins.security.LastGrantedAuthoritiesProperty;
import org.hamcrest.MatcherAssert;
import org.htmlunit.CookieManager;
import org.htmlunit.html.HtmlPage;
import org.htmlunit.util.Cookie;
import org.junit.Before;
import org.junit.Ignore;
import org.junit.Rule;
Expand Down Expand Up @@ -83,6 +86,7 @@
import static org.jenkinsci.plugins.oic.TestRealm.GROUPS_FIELD;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertThrows;
Expand Down Expand Up @@ -173,6 +177,52 @@ public void testLoginWithDefaultsUntrustedTLSPassesWhenTLSChecksDisabled() throw
assertTestUserIsMemberOfTestGroups(user);
}

@Test
@Issue("SECURITY-3473")
public void testSessionRefresh() throws Exception {
String cookieName = "JSESSIONID";
String cookieHost = jenkinsRule.getURL().getHost();
// Dev memo: jenkinsRule.getURL().getPath() has a trailing / which breaks Cookie#equals
String cookiePath = jenkinsRule.contextPath;
String previousSession = "fakesessionid0123456789";
CookieManager cookieManager = webClient.getCookieManager();
Cookie jSessionIDCookie = new Cookie(cookieHost, cookieName, previousSession, cookiePath, null, false, true);

mockAuthorizationRedirectsToFinishLogin();
mockTokenReturnsIdTokenWithGroup();
configureTestRealm(sc -> {});

// Not yet logged in
assertAnonymous();
assertEquals(
"No session cookie should be present",
0,
cookieManager.getCookies().stream()
.filter(c -> Objects.equals(c.getName(), cookieName))
.count());

// Set a JSESSIONID cookie value before the first login is attempted.
cookieManager.addCookie(jSessionIDCookie);

browseLoginPage();

// Multiple JSESSIONID can exist if, for example, the path is different
assertEquals(
"Only one session cookie should be present",
1,
cookieManager.getCookies().stream()
.filter(c -> Objects.equals(c.getName(), cookieName))
.count());

String firstLoginSession = cookieManager.getCookie(cookieName).getValue();
assertNotEquals("The previous session should be replaced with a new one", previousSession, firstLoginSession);

browseLoginPage();

String secondLoginSession = cookieManager.getCookie(cookieName).getValue();
assertNotEquals("The session should be renewed when the user log in", firstLoginSession, secondLoginSession);
}

private void browseLoginPage() throws IOException, SAXException {
webClient.goTo(jenkins.getSecurityRealm().getLoginUrl());
}
Expand Down

0 comments on commit 5422614

Please sign in to comment.