Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Redirect to login page in case token is expired instead of showing an error page #395

Merged
merged 4 commits into from
Oct 8, 2024
Merged

Redirect to login page in case token is expired instead of showing an error page #395

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
58674c5
Redirect to login page in case token is expired
eva-mueller-coremedia Sep 12, 2024
0dc8084
Fix formatting
eva-mueller-coremedia Sep 12, 2024
0a20ea7
Fix PluginTest
eva-mueller-coremedia Sep 12, 2024
89afbcf
Rename method
eva-mueller-coremedia Sep 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 9 additions & 6 deletions src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1450,10 +1450,8 @@
throws IOException, ServletException {
if (req.getSession(false) != null || Strings.isNullOrEmpty(req.getHeader("Authorization"))) {
req.getSession().invalidate();
res.sendRedirect(Jenkins.get().getSecurityRealm().getLoginUrl());
} else {
res.sendError(HttpServletResponse.SC_UNAUTHORIZED, "Token expired");
}
res.sendRedirect(Jenkins.get().getSecurityRealm().getLoginUrl());
eva-mueller-coremedia marked this conversation as resolved.
Show resolved Hide resolved
}

public boolean isExpired(OicCredentials credentials) {
Expand Down Expand Up @@ -1487,7 +1485,7 @@

return handleTokenRefreshResponse(flow, expectedUsername, credentials, tokenResponse, httpResponse);
} catch (TokenResponseException e) {
handleTokenRefreshException(e, httpResponse);
handleTokenRefreshException(e, httpRequest, httpResponse);
return false;
}
}
Expand Down Expand Up @@ -1557,14 +1555,19 @@
return true;
}

private void handleTokenRefreshException(TokenResponseException e, HttpServletResponse httpResponse)
private void handleTokenRefreshException(
TokenResponseException e, HttpServletRequest httpRequest, HttpServletResponse httpResponse)
throws IOException {
TokenErrorResponse details = e.getDetails();

if ("invalid_grant".equals(details.getError())) {
// RT expired or session terminated
if (!isTokenExpirationCheckDisabled()) {
httpResponse.sendError(HttpServletResponse.SC_UNAUTHORIZED, "Token expired");
try {
redirectOrRejectRequest(httpRequest, httpResponse);
} catch (ServletException ex) {

Check warning on line 1568 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 1568 is not covered by tests
httpResponse.sendError(HttpServletResponse.SC_UNAUTHORIZED, "Token expired");

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

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java#L1568-L1569 

Added lines #L1568 - L1569 were not covered by tests
}
}
} else {
LOGGER.warning("Token response error: " + details.getError() + ", error description: "
Expand Down
6 changes: 3 additions & 3 deletions src/test/java/org/jenkinsci/plugins/oic/PluginTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@

/**
* goes through a login scenario, the openid provider is mocked and always returns state. We aren't checking
* if if openid connect or if the openid connect implementation works. Rather we are only
* if openid connect or if the openid connect implementation works. Rather we are only
* checking if the jenkins interaction works and if the plugin code works.
*/
@Url("https://jenkins.io/blog/2018/01/13/jep-200/")
Expand Down Expand Up @@ -480,7 +480,7 @@ public void testRefreshTokenAndTokenExpiration_expiredRefreshToken() throws Exce
.withHeader("Content-Type", "application/json")
.withBody("{ \"error\": \"invalid_grant\" }")));
expire();
webClient.assertFails(jenkins.getSearchUrl(), 401);
webClient.assertFails(jenkins.getSearchUrl(), 500);

verify(postRequestedFor(urlPathEqualTo("/token")).withRequestBody(containing("grant_type=refresh_token")));
}
Expand Down Expand Up @@ -1018,7 +1018,7 @@ public void testAccessUsingJenkinsApiTokens() throws Exception {
// the default behavior expects there to be a valid oic session, so token based
// access should now fail (unauthorized)
rsp = getPageWithGet(TEST_USER_USERNAME, token, "/whoAmI/api/xml");
MatcherAssert.assertThat("response should have been 401\n" + rsp.body(), rsp.statusCode(), is(401));
MatcherAssert.assertThat("response should have been 302\n" + rsp.body(), rsp.statusCode(), is(302));

// enable "traditional api token access"
testRealm.setAllowTokenAccessWithoutOicSession(true);
Expand Down