From 58674c5742cf4b63950ec6d725d4ac1b5d19c0e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Eva=20M=C3=BCller?= Date: Thu, 12 Sep 2024 08:35:31 +0200 Subject: [PATCH 1/4] Redirect to login page in case token is expired Instead of showing an error page --- .../jenkinsci/plugins/oic/OicSecurityRealm.java | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java b/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java index 8d922da2..1403877b 100644 --- a/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java +++ b/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java @@ -1450,10 +1450,8 @@ private void redirectOrRejectRequest(HttpServletRequest req, HttpServletResponse 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) { @@ -1487,7 +1485,7 @@ private boolean refreshExpiredToken( return handleTokenRefreshResponse(flow, expectedUsername, credentials, tokenResponse, httpResponse); } catch (TokenResponseException e) { - handleTokenRefreshException(e, httpResponse); + handleTokenRefreshException(e, httpRequest, httpResponse); return false; } } @@ -1557,14 +1555,18 @@ 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 { + redirectOrRejectRequest(httpRequest, httpResponse); + } catch (ServletException ex) { + httpResponse.sendError(HttpServletResponse.SC_UNAUTHORIZED, "Token expired"); + } } } else { LOGGER.warning("Token response error: " + details.getError() + ", error description: " From 0dc808497ff5b703dd326438823e7ca77570c3b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Eva=20M=C3=BCller?= Date: Thu, 12 Sep 2024 09:15:09 +0200 Subject: [PATCH 2/4] Fix formatting --- .../java/org/jenkinsci/plugins/oic/OicSecurityRealm.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java b/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java index 1403877b..44a5599c 100644 --- a/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java +++ b/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java @@ -1555,7 +1555,8 @@ private boolean handleTokenRefreshResponse( return true; } - private void handleTokenRefreshException(TokenResponseException e, HttpServletRequest httpRequest, HttpServletResponse httpResponse) + private void handleTokenRefreshException( + TokenResponseException e, HttpServletRequest httpRequest, HttpServletResponse httpResponse) throws IOException { TokenErrorResponse details = e.getDetails(); @@ -1563,9 +1564,9 @@ private void handleTokenRefreshException(TokenResponseException e, HttpServletRe // RT expired or session terminated if (!isTokenExpirationCheckDisabled()) { try { - redirectOrRejectRequest(httpRequest, httpResponse); + redirectOrRejectRequest(httpRequest, httpResponse); } catch (ServletException ex) { - httpResponse.sendError(HttpServletResponse.SC_UNAUTHORIZED, "Token expired"); + httpResponse.sendError(HttpServletResponse.SC_UNAUTHORIZED, "Token expired"); } } } else { From 0a20ea7f020774f711bc4b6e7a8f3b189518c5d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Eva=20M=C3=BCller?= Date: Thu, 12 Sep 2024 09:21:55 +0200 Subject: [PATCH 3/4] Fix PluginTest --- src/test/java/org/jenkinsci/plugins/oic/PluginTest.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/test/java/org/jenkinsci/plugins/oic/PluginTest.java b/src/test/java/org/jenkinsci/plugins/oic/PluginTest.java index 244c6526..1f679da5 100644 --- a/src/test/java/org/jenkinsci/plugins/oic/PluginTest.java +++ b/src/test/java/org/jenkinsci/plugins/oic/PluginTest.java @@ -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/") @@ -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"))); } @@ -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); From 89afbcf290b2758f048cffcf54f640022587173e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Eva=20M=C3=BCller?= Date: Thu, 19 Sep 2024 08:22:37 +0200 Subject: [PATCH 4/4] Rename method --- .../java/org/jenkinsci/plugins/oic/OicSecurityRealm.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java b/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java index 44a5599c..4375c0af 100644 --- a/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java +++ b/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java @@ -1438,7 +1438,7 @@ public boolean handleTokenExpiration(HttpServletRequest httpRequest, HttpServlet if (isUseRefreshTokens() && !Strings.isNullOrEmpty(credentials.getRefreshToken())) { return refreshExpiredToken(user.getId(), credentials, httpRequest, httpResponse); } else if (!isTokenExpirationCheckDisabled()) { - redirectOrRejectRequest(httpRequest, httpResponse); + redirectToLoginUrl(httpRequest, httpResponse); return false; } } @@ -1446,7 +1446,7 @@ 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(); @@ -1564,7 +1564,7 @@ private void handleTokenRefreshException( // RT expired or session terminated if (!isTokenExpirationCheckDisabled()) { try { - redirectOrRejectRequest(httpRequest, httpResponse); + redirectToLoginUrl(httpRequest, httpResponse); } catch (ServletException ex) { httpResponse.sendError(HttpServletResponse.SC_UNAUTHORIZED, "Token expired"); }