diff --git a/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java b/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java index aabbfddd..9d0bf922 100644 --- a/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java +++ b/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java @@ -1230,7 +1230,7 @@ public boolean handleTokenExpiration(HttpServletRequest httpRequest, HttpServlet if (serverConfiguration.isUseRefreshTokens() && !Strings.isNullOrEmpty(credentials.getRefreshToken())) { return refreshExpiredToken(user.getId(), credentials, httpRequest, httpResponse); } else if (!isTokenExpirationCheckDisabled()) { - redirectOrRejectRequest(httpRequest, httpResponse); + redirectToLoginUrl(httpRequest, httpResponse); return false; } } @@ -1238,14 +1238,12 @@ public boolean handleTokenExpiration(HttpServletRequest httpRequest, HttpServlet return true; } - private void redirectOrRejectRequest(HttpServletRequest req, HttpServletResponse res) + private void redirectToLoginUrl(HttpServletRequest req, HttpServletResponse res) 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()); } public boolean isExpired(OicCredentials credentials) { @@ -1279,7 +1277,7 @@ private boolean refreshExpiredToken( return handleTokenRefreshResponse(flow, expectedUsername, credentials, tokenResponse, httpResponse); } catch (TokenResponseException e) { - handleTokenRefreshException(e, httpResponse); + handleTokenRefreshException(e, httpRequest, httpResponse); return false; } } @@ -1349,14 +1347,19 @@ private boolean handleTokenRefreshResponse( 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 { + redirectToLoginUrl(httpRequest, httpResponse); + } catch (ServletException ex) { + httpResponse.sendError(HttpServletResponse.SC_UNAUTHORIZED, "Token expired"); + } } } else { LOGGER.warning("Token response error: " + details.getError() + ", error description: " diff --git a/src/test/java/org/jenkinsci/plugins/oic/PluginTest.java b/src/test/java/org/jenkinsci/plugins/oic/PluginTest.java index 94348b6c..d4f0ea80 100644 --- a/src/test/java/org/jenkinsci/plugins/oic/PluginTest.java +++ b/src/test/java/org/jenkinsci/plugins/oic/PluginTest.java @@ -84,7 +84,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/") @@ -486,7 +486,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"))); } @@ -1076,7 +1076,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);