From 460f2d9adef3060d283fde4c9ec89e11f87965d2 Mon Sep 17 00:00:00 2001 From: Dario Ghunney Ware Date: Fri, 4 Jul 2025 18:46:32 +0100 Subject: [PATCH] Fixed JWT auth flow Adding tests --- .gitignore | 3 + .../common/model/ApplicationProperties.java | 8 +-- .../src/main/resources/settings.yml.template | 11 ++-- .../CustomAuthenticationSuccessHandler.java | 4 +- .../security/CustomLogoutSuccessHandler.java | 16 ++---- .../configuration/SecurityConfiguration.java | 57 ++++++++++--------- .../filter/UserAuthenticationFilter.java | 14 ++--- .../CustomLogoutSuccessHandlerTest.java | 24 ++++++++ 8 files changed, 76 insertions(+), 61 deletions(-) diff --git a/.gitignore b/.gitignore index 6ebd87c35..55603719c 100644 --- a/.gitignore +++ b/.gitignore @@ -200,3 +200,6 @@ id_ed25519.pub # node_modules node_modules/ + +# Claude +CLAUDE.md diff --git a/app/common/src/main/java/stirling/software/common/model/ApplicationProperties.java b/app/common/src/main/java/stirling/software/common/model/ApplicationProperties.java index 9e2705b3e..c3abe0c26 100644 --- a/app/common/src/main/java/stirling/software/common/model/ApplicationProperties.java +++ b/app/common/src/main/java/stirling/software/common/model/ApplicationProperties.java @@ -306,7 +306,6 @@ public class ApplicationProperties { @Data public static class JWT { private Boolean enabled = false; - @ToString.Exclude private String secretKey; private Long expiration = 3600000L; // Default 1 hour in milliseconds private String algorithm = "HS256"; // Default HMAC algorithm private String issuer = "Stirling-PDF"; // Default issuer @@ -314,12 +313,7 @@ public class ApplicationProperties { private Long refreshTokenExpiration = 86400000L; // Default 24 hours public boolean isSettingsValid() { - return enabled != null - && enabled - && secretKey != null - && !secretKey.trim().isEmpty() - && expiration != null - && expiration > 0; + return enabled != null && enabled && expiration != null && expiration > 0; } } } diff --git a/app/core/src/main/resources/settings.yml.template b/app/core/src/main/resources/settings.yml.template index 91a6e4f4f..1aae02f2d 100644 --- a/app/core/src/main/resources/settings.yml.template +++ b/app/core/src/main/resources/settings.yml.template @@ -11,14 +11,14 @@ ############################################################################################################# security: - enableLogin: false # set to 'true' to enable login - csrfDisabled: false # set to 'true' to disable CSRF protection (not recommended for production) + enableLogin: true # set to 'true' to enable login + csrfDisabled: true # set to 'true' to disable CSRF protection (not recommended for production) loginAttemptCount: 5 # lock user account after 5 tries; when using e.g. Fail2Ban you can deactivate the function with -1 loginResetTimeMinutes: 120 # lock account for 2 hours after x attempts loginMethod: all # Accepts values like 'all' and 'normal'(only Login with Username/Password), 'oauth2'(only Login with OAuth2) or 'saml2'(only Login with SAML2) initialLogin: - username: '' # initial username for the first login - password: '' # initial password for the first login + username: 'admin' # initial username for the first login + password: 'stirling' # initial password for the first login oauth2: enabled: false # set to 'true' to enable login (Note: enableLogin must also be 'true' for this to work) client: @@ -61,12 +61,9 @@ security: spCert: classpath:saml-public-cert.crt # Your signing certificate. Generated from your keypair jwt: enabled: true # set to 'true' to enable JWT authentication - secretKey: 'Uz4BgfMySCz2Uplhp1x9ij19vVV2bXYktROtrlw3CC0=' # secret expiration: 3600000 # Expiration time in milliseconds. Default is 1 hour (3600000 ms) algorithm: HS256 # JWT signing algorithm. Default is HS256 issuer: Stirling-PDF # Issuer of the JWT token. Default is 'Stirling-PDF' - refreshTokenEnabled: false # Set to 'true' to enable refresh tokens - refreshTokenExpiration: 86400000 # Expiration time for refresh tokens in milliseconds. Default is 1 day (86400000 ms) premium: key: 00000000-0000-0000-0000-000000000000 diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/security/CustomAuthenticationSuccessHandler.java b/app/proprietary/src/main/java/stirling/software/proprietary/security/CustomAuthenticationSuccessHandler.java index ea115c4ef..63bb87613 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/security/CustomAuthenticationSuccessHandler.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/security/CustomAuthenticationSuccessHandler.java @@ -53,11 +53,11 @@ public class CustomAuthenticationSuccessHandler // Generate JWT token if JWT authentication is enabled boolean jwtEnabled = jwtService.isJwtEnabled(); - if (jwtService != null && jwtEnabled) { + if (jwtEnabled) { try { String jwt = jwtService.generateToken(authentication); jwtService.addTokenToResponse(response, jwt); - log.debug("JWT token generated and added to response for user: {}", userName); + log.debug("JWT generated for user: {}", userName); } catch (Exception e) { log.error("Failed to generate JWT token for user: {}", userName, e); } diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/security/CustomLogoutSuccessHandler.java b/app/proprietary/src/main/java/stirling/software/proprietary/security/CustomLogoutSuccessHandler.java index 2f19fedca..8249d31f5 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/security/CustomLogoutSuccessHandler.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/security/CustomLogoutSuccessHandler.java @@ -53,17 +53,6 @@ public class CustomLogoutSuccessHandler extends SimpleUrlLogoutSuccessHandler { HttpServletRequest request, HttpServletResponse response, Authentication authentication) throws IOException { - // Clear JWT token if JWT authentication is enabled - if (jwtService != null && jwtService.isJwtEnabled()) { - try { - jwtService.clearTokenFromResponse(response); - log.debug("JWT token cleared from response during logout"); - } catch (Exception e) { - log.error("Failed to clear JWT token during logout", e); - // Continue with normal logout flow even if JWT clearing fails - } - } - if (!response.isCommitted()) { if (authentication != null) { if (authentication instanceof Saml2Authentication samlAuthentication) { @@ -82,6 +71,11 @@ public class CustomLogoutSuccessHandler extends SimpleUrlLogoutSuccessHandler { authentication.getClass().getSimpleName()); getRedirectStrategy().sendRedirect(request, response, LOGOUT_PATH); } + } else if (jwtService.isJwtEnabled()) { + // Clear JWT token if JWT authentication is enabled + jwtService.clearTokenFromResponse(response); + log.debug("Cleared JWT from response"); + getRedirectStrategy().sendRedirect(request, response, LOGOUT_PATH); } else { // Redirect to login page after logout String path = checkForErrors(request); diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/security/configuration/SecurityConfiguration.java b/app/proprietary/src/main/java/stirling/software/proprietary/security/configuration/SecurityConfiguration.java index 5185ac1ab..8090ced3b 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/security/configuration/SecurityConfiguration.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/security/configuration/SecurityConfiguration.java @@ -38,6 +38,7 @@ import stirling.software.common.model.ApplicationProperties; import stirling.software.proprietary.security.CustomAuthenticationFailureHandler; import stirling.software.proprietary.security.CustomAuthenticationSuccessHandler; import stirling.software.proprietary.security.CustomLogoutSuccessHandler; +import stirling.software.proprietary.security.JWTAuthenticationEntryPoint; import stirling.software.proprietary.security.database.repository.JPATokenRepositoryImpl; import stirling.software.proprietary.security.database.repository.PersistentLoginRepository; import stirling.software.proprietary.security.filter.FirstLoginFilter; @@ -74,6 +75,7 @@ public class SecurityConfiguration { private final UserAuthenticationFilter userAuthenticationFilter; private final JWTAuthenticationFilter jwtAuthenticationFilter; private final JWTServiceInterface jwtService; + private final JWTAuthenticationEntryPoint jwtAuthenticationEntryPoint; private final LoginAttemptService loginAttemptService; private final FirstLoginFilter firstLoginFilter; private final SessionPersistentRegistry sessionRegistry; @@ -93,6 +95,7 @@ public class SecurityConfiguration { UserAuthenticationFilter userAuthenticationFilter, JWTAuthenticationFilter jwtAuthenticationFilter, JWTServiceInterface jwtService, + JWTAuthenticationEntryPoint jwtAuthenticationEntryPoint, LoginAttemptService loginAttemptService, FirstLoginFilter firstLoginFilter, SessionPersistentRegistry sessionRegistry, @@ -110,6 +113,7 @@ public class SecurityConfiguration { this.userAuthenticationFilter = userAuthenticationFilter; this.jwtAuthenticationFilter = jwtAuthenticationFilter; this.jwtService = jwtService; + this.jwtAuthenticationEntryPoint = jwtAuthenticationEntryPoint; this.loginAttemptService = loginAttemptService; this.firstLoginFilter = firstLoginFilter; this.sessionRegistry = sessionRegistry; @@ -136,16 +140,19 @@ public class SecurityConfiguration { if (loginEnabledValue) { if (jwtEnabled && jwtAuthenticationFilter != null) { http.addFilterBefore( - jwtAuthenticationFilter, UsernamePasswordAuthenticationFilter.class); - // .addFilterAfter( - // jwtAuthenticationFilter, - // userAuthenticationFilter.getClass()); + jwtAuthenticationFilter, UsernamePasswordAuthenticationFilter.class) + .exceptionHandling( + exceptionHandling -> + exceptionHandling.authenticationEntryPoint( + jwtAuthenticationEntryPoint)); } else { http.addFilterBefore( - userAuthenticationFilter, UsernamePasswordAuthenticationFilter.class); + userAuthenticationFilter, + UsernamePasswordAuthenticationFilter.class) + .addFilterBefore(userAuthenticationFilter, firstLoginFilter.getClass()); } - http.addFilterAfter(firstLoginFilter, UsernamePasswordAuthenticationFilter.class) - .addFilterAfter(rateLimitingFilter(), firstLoginFilter.getClass()); + http.addFilterAfter(rateLimitingFilter(), UsernamePasswordAuthenticationFilter.class); + if (!securityProperties.getCsrfDisabled()) { CookieCsrfTokenRepository cookieRepo = CookieCsrfTokenRepository.withHttpOnlyFalse(); @@ -198,7 +205,6 @@ public class SecurityConfiguration { }); http.authenticationProvider(daoAuthenticationProvider()); http.requestCache(requestCache -> requestCache.requestCache(new NullRequestCache())); - // Configure logout behavior based on JWT setting http.logout( logout -> logout.logoutRequestMatcher( @@ -211,24 +217,21 @@ public class SecurityConfiguration { .invalidateHttpSession(true) .deleteCookies( "JSESSIONID", "remember-me", "STIRLING_JWT_TOKEN")); - // Only configure remember-me if JWT is not enabled (stateless) todo: check if remember-me can be used with JWT - if (!jwtEnabled) { - http.rememberMe( - rememberMeConfigurer -> // Use the configurator directly - rememberMeConfigurer - .tokenRepository(persistentTokenRepository()) - .tokenValiditySeconds( // 14 days - 14 * 24 * 60 * 60) - .userDetailsService( // Your existing UserDetailsService - userDetailsService) - .useSecureCookie( // Enable secure cookie - true) - .rememberMeParameter( // Form parameter name - "remember-me") - .rememberMeCookieName( // Cookie name - "remember-me") - .alwaysRemember(false)); - } + http.rememberMe( + rememberMeConfigurer -> // Use the configurator directly + rememberMeConfigurer + .tokenRepository(persistentTokenRepository()) + .tokenValiditySeconds( // 14 days + 14 * 24 * 60 * 60) + .userDetailsService( // Your existing UserDetailsService + userDetailsService) + .useSecureCookie( // Enable secure cookie + true) + .rememberMeParameter( // Form parameter name + "remember-me") + .rememberMeCookieName( // Cookie name + "remember-me") + .alwaysRemember(false)); http.authorizeHttpRequests( authz -> authz.requestMatchers( @@ -253,6 +256,7 @@ public class SecurityConfiguration { || trimmedUri.startsWith("/css/") || trimmedUri.startsWith("/fonts/") || trimmedUri.startsWith("/js/") + || trimmedUri.startsWith("/favicon") || trimmedUri.startsWith( "/api/v1/info/status"); }) @@ -343,7 +347,6 @@ public class SecurityConfiguration { return http.build(); } - // todo: check if this is needed @Bean public AuthenticationManager authenticationManager(AuthenticationConfiguration configuration) throws Exception { diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/security/filter/UserAuthenticationFilter.java b/app/proprietary/src/main/java/stirling/software/proprietary/security/filter/UserAuthenticationFilter.java index e9addd239..971cd4859 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/security/filter/UserAuthenticationFilter.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/security/filter/UserAuthenticationFilter.java @@ -117,18 +117,18 @@ public class UserAuthenticationFilter extends OncePerRequestFilter { if ("GET".equalsIgnoreCase(method) && !(contextPath + "/login").equals(requestURI)) { response.sendRedirect(contextPath + "/login"); // redirect to the login page - return; } else { response.setStatus(HttpStatus.UNAUTHORIZED.value()); response.getWriter() .write( - "Authentication required. Please provide a X-API-KEY in request" - + " header.\n" - + "This is found in Settings -> Account Settings -> API Key\n" - + "Alternatively you can disable authentication if this is" - + " unexpected"); - return; + """ + Authentication required. Please provide a X-API-KEY in request\ + header. + This is found in Settings -> Account Settings -> API Key + Alternatively you can disable authentication if this is\ + unexpected"""); } + return; } // Check if the authenticated user is disabled and invalidate their session if so diff --git a/app/proprietary/src/test/java/stirling/software/proprietary/security/CustomLogoutSuccessHandlerTest.java b/app/proprietary/src/test/java/stirling/software/proprietary/security/CustomLogoutSuccessHandlerTest.java index 5a392f369..4e4bf8552 100644 --- a/app/proprietary/src/test/java/stirling/software/proprietary/security/CustomLogoutSuccessHandlerTest.java +++ b/app/proprietary/src/test/java/stirling/software/proprietary/security/CustomLogoutSuccessHandlerTest.java @@ -9,7 +9,9 @@ import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; import org.springframework.security.oauth2.client.authentication.OAuth2AuthenticationToken; +import stirling.software.common.configuration.AppConfig; import stirling.software.common.model.ApplicationProperties; +import stirling.software.proprietary.security.service.JWTServiceInterface; import static org.mockito.Mockito.*; @ExtendWith(MockitoExtension.class) @@ -17,6 +19,10 @@ class CustomLogoutSuccessHandlerTest { @Mock private ApplicationProperties.Security securityProperties; + @Mock private AppConfig appConfig; + + @Mock private JWTServiceInterface jwtService; + @InjectMocks private CustomLogoutSuccessHandler customLogoutSuccessHandler; @Test @@ -26,6 +32,7 @@ class CustomLogoutSuccessHandlerTest { String logoutPath = "logout=true"; when(response.isCommitted()).thenReturn(false); + when(jwtService.isJwtEnabled()).thenReturn(false); when(request.getContextPath()).thenReturn(""); when(response.encodeRedirectURL(logoutPath)).thenReturn(logoutPath); @@ -34,6 +41,23 @@ class CustomLogoutSuccessHandlerTest { verify(response).sendRedirect(logoutPath); } + @Test + void testSuccessfulLogoutViaJWT() throws IOException { + HttpServletRequest request = mock(HttpServletRequest.class); + HttpServletResponse response = mock(HttpServletResponse.class); + String logoutPath = "/login?logout=true"; + + when(response.isCommitted()).thenReturn(false); + when(jwtService.isJwtEnabled()).thenReturn(true); + when(request.getContextPath()).thenReturn(""); + when(response.encodeRedirectURL(logoutPath)).thenReturn(logoutPath); + + customLogoutSuccessHandler.onLogoutSuccess(request, response, null); + + verify(response).sendRedirect(logoutPath); + verify(jwtService).clearTokenFromResponse(response); + } + @Test void testSuccessfulLogoutViaOAuth2() throws IOException { HttpServletRequest request = mock(HttpServletRequest.class);