Minor code refactoring. Ensure auth cookie invalidation resets CSRF token & that it happens immediately on first usage of cookie

dspace-server-webapp/src/main/java/org/dspace/app/rest/security/RestAuthenticationService.java

@@ -71,8 +71,7 @@ public interface RestAuthenticationService {
      * existing authentication data/token is destroyed/invalidated and cannot be reused in later requests.
      * <P>
      * In other words, this method invalidates the authentication data created by addAuthenticationDataForUser().
-     * This also should include clearing any Cookie created by that method, usually by calling the separate
-     * invalidateAuthenticationCookie() method in this same class.
+     *
      * @param request current request
      * @param response current response
      * @param context current DSpace Context.
@@ -103,8 +102,9 @@ public interface RestAuthenticationService {
      * addAuthenticationDataForUser()). It's useful for those services to immediately *remove/discard* the Cookie after
      * it has been used. This ensures the auth Cookie is temporary in nature, and is destroyed as soon as it is no
      * longer needed.
+     * @param request current request
      * @param res current response (where Cookie should be destroyed)
      */
-    void invalidateAuthenticationCookie(HttpServletResponse res);
+    void invalidateAuthenticationCookie(HttpServletRequest request, HttpServletResponse res);
 
 }

dspace-server-webapp/src/main/java/org/dspace/app/rest/security/StatelessAuthenticationFilter.java

@@ -39,7 +39,8 @@ import org.springframework.security.web.authentication.www.BasicAuthenticationFi
 
 /**
  * Custom Spring authentication filter for Stateless authentication, intercepts requests to check for valid
- * authentication
+ * authentication. This runs before *every* request in the DSpace backend to see if any authentication data
+ * is passed in that request. If so, it authenticates the EPerson in the current Context.
  *
  * @author Frederic Van Reet (frederic dot vanreet at atmire dot com)
  * @author Tom Desair (tom dot desair at atmire dot com)

dspace-server-webapp/src/main/java/org/dspace/app/rest/security/StatelessLoginFilter.java

@@ -23,7 +23,10 @@ import org.springframework.security.web.authentication.AbstractAuthenticationPro
 import org.springframework.security.web.util.matcher.AntPathRequestMatcher;
 
 /**
- * This class will filter login requests to try and authenticate them
+ * This class will filter /api/authn/login requests to try and authenticate them. Keep in mind, this filter runs *after*
+ * StatelessAuthenticationFilter (which looks for authentication data in the request itself). So, in some scenarios
+ * (e.g. after a Shibboleth login) the StatelessAuthenticationFilter does the actual authentication, and this Filter
+ * just ensures the auth token (JWT) is sent back in an Authorization header.
  *
  * @author Frederic Van Reet (frederic dot vanreet at atmire dot com)
  * @author Tom Desair (tom dot desair at atmire dot com)

dspace-server-webapp/src/main/java/org/dspace/app/rest/security/jwt/JWTTokenRestAuthenticationServiceImpl.java

@@ -157,7 +157,6 @@ public class JWTTokenRestAuthenticationServiceImpl implements RestAuthentication
     public void invalidateAuthenticationData(HttpServletRequest request, HttpServletResponse response,
                                              Context context) throws Exception {
         String token = getLoginToken(request, response);
-        invalidateAuthenticationCookie(response);
         loginJWTTokenHandler.invalidateToken(token, request, context);
 
         // Reset our CSRF token, generating a new one
@@ -166,16 +165,20 @@ public class JWTTokenRestAuthenticationServiceImpl implements RestAuthentication
 
     /**
      * Invalidate our temporary authentication cookie by overwriting it in the response.
+     * @param request
      * @param response
      */
     @Override
-    public void invalidateAuthenticationCookie(HttpServletResponse response) {
+    public void invalidateAuthenticationCookie(HttpServletRequest request, HttpServletResponse response) {
         // Re-send the same cookie (as addTokenToResponse()) with no value and a Max-Age of 0 seconds
         ResponseCookie cookie = ResponseCookie.from(AUTHORIZATION_COOKIE, "")
                                               .maxAge(0).httpOnly(true).secure(true).sameSite("None").build();
 
         // Write the cookie to the Set-Cookie header in order to send it
         response.addHeader(HttpHeaders.SET_COOKIE, cookie.toString());
+
+        // Reset our CSRF token, generating a new one
+        resetCSRFToken(request, response);
     }
 
     @Override
@@ -254,7 +257,6 @@ public class JWTTokenRestAuthenticationServiceImpl implements RestAuthentication
      * Get the Login token (JWT) in the current request. First we check the Authorization header.
      * If not found there, we check for a temporary authentication cookie and use that.
      * @param request current request
-     * @param request current response
      * @return authentication token (if found), or null
      */
     private String getLoginToken(HttpServletRequest request, HttpServletResponse response) {
@@ -265,12 +267,10 @@ public class JWTTokenRestAuthenticationServiceImpl implements RestAuthentication
             tokenValue = authHeader.replace(AUTHORIZATION_TYPE, "").trim();
         } else if (StringUtils.isNotBlank(authCookie)) {
             tokenValue = authCookie;
-            // If auth cookie is read/used, then we immediately remove the cookie & reset the CSRF token.
-            // This is because we treat auth cookies as temporary in nature, only to be used once & only used by
-            // specific auth plugins (like Shibboleth) which require them. So, an auth cookie should only ever be
-            // used for an initial login, after which Headers are used.
-            invalidateAuthenticationCookie(response);
-            resetCSRFToken(request, response);
+            // After auth cookie is read, immediately invalidate it (and this also resets the CSRF token).
+            // This ensures the auth cookie is temporary in nature, only to be used once (usually on initial login,
+            // e.g. via Shibboleth). After that, we assume Authorization Header will be used.
+            invalidateAuthenticationCookie(request, response);
         }
 
         return tokenValue;
@@ -292,6 +292,7 @@ public class JWTTokenRestAuthenticationServiceImpl implements RestAuthentication
             for (Cookie cookie : cookies) {
                 if (cookie.getName().equals(AUTHORIZATION_COOKIE) && StringUtils.isNotEmpty(cookie.getValue())) {
                     authCookie = cookie.getValue();
+                    break;
                 }
             }
         }