Merge pull request #395 from eva-mueller-coremedia/eva-mueller-coreme…

…dia/redirect-on-token-expiry Redirect to login page in case token is expired instead of showing an error page
jenkinsci · Oct 8, 2024 · c7c0c06 · c7c0c06
2 parents b0ebf34 + 89afbcf
commit c7c0c06
Show file tree

Hide file tree

Showing 2 changed files with 14 additions and 11 deletions.
diff --git a/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java b/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java
@@ -1230,22 +1230,20 @@ 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;
             }
         }
 
         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
@@ -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);