From ce7bac26d2b5fdb9399e9886a4608d85fd5dbdc4 Mon Sep 17 00:00:00 2001 From: Dario Ghunney Ware Date: Tue, 22 Jul 2025 17:46:42 +0100 Subject: [PATCH] Cleanup --- .claude/settings.local.json | 4 +- .github/workflows/build.yml | 121 +++++++- .gitignore | 3 - .../common/model/ApplicationProperties.java | 7 + .../software/common/util/RequestUriUtils.java | 2 + .../src/main/resources/application.properties | 2 +- .../src/main/resources/settings.yml.template | 32 ++- .../configuration/SecurityConfiguration.java | 10 +- .../repository/JwtSigningKeyRepository.java | 18 ++ .../filter/JwtAuthenticationFilter.java | 52 +--- .../filter/UserAuthenticationFilter.java | 12 +- .../security/model/AuthenticationType.java | 2 +- .../security/model/JwtSigningKey.java | 62 +++++ ...tomOAuth2AuthenticationSuccessHandler.java | 3 +- ...tSaml2AuthenticationRequestRepository.java | 2 +- .../security/service/JwtKeystoreService.java | 238 ++++++++++++++++ .../service/JwtKeystoreServiceInterface.java | 17 ++ .../security/service/JwtService.java | 70 ++++- .../security/service/UserService.java | 7 +- .../filter/JwtAuthenticationFilterTest.java | 100 +------ .../JwtKeystoreServiceInterfaceTest.java | 258 ++++++++++++++++++ .../security/service/JwtServiceTest.java | 107 +++++++- 22 files changed, 935 insertions(+), 194 deletions(-) create mode 100644 app/proprietary/src/main/java/stirling/software/proprietary/security/database/repository/JwtSigningKeyRepository.java create mode 100644 app/proprietary/src/main/java/stirling/software/proprietary/security/model/JwtSigningKey.java create mode 100644 app/proprietary/src/main/java/stirling/software/proprietary/security/service/JwtKeystoreService.java create mode 100644 app/proprietary/src/main/java/stirling/software/proprietary/security/service/JwtKeystoreServiceInterface.java create mode 100644 app/proprietary/src/test/java/stirling/software/proprietary/security/service/JwtKeystoreServiceInterfaceTest.java diff --git a/.claude/settings.local.json b/.claude/settings.local.json index 13d8d8350..bc5358b85 100644 --- a/.claude/settings.local.json +++ b/.claude/settings.local.json @@ -9,7 +9,9 @@ "Bash(find:*)", "Bash(grep:*)", "Bash(rg:*)", - "Bash(strings:*)" + "Bash(strings:*)", + "Bash(pkill:*)", + "Bash(true)" ], "deny": [] } diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index f00b927c9..db847f570 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -1,8 +1,9 @@ -name: Build repo +name: Build and Test Workflow on: - push: - branches: ["main"] + workflow_dispatch: + # push: + # branches: ["main"] pull_request: branches: ["main"] @@ -22,6 +23,24 @@ permissions: contents: read jobs: + files-changed: + name: detect what files changed + runs-on: ubuntu-latest + timeout-minutes: 3 + # Map a step output to a job output + outputs: + build: ${{ steps.changes.outputs.build }} + app: ${{ steps.changes.outputs.app }} + project: ${{ steps.changes.outputs.project }} + openapi: ${{ steps.changes.outputs.openapi }} + steps: + - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + + - name: Check for file changes + uses: dorny/paths-filter@de90cc6fb38fc0963ad72b210f1f284cd68cea36 # v3.0.2 + id: changes + with: + filters: ".github/config/.files.yaml" build: runs-on: ubuntu-latest @@ -37,7 +56,7 @@ jobs: steps: - name: Harden Runner - uses: step-security/harden-runner@6c439dc8bdf85cadbbce9ed30d1c7b959517bc49 # v2.12.2 + uses: step-security/harden-runner@ec9f2d5744a09debf3a187a3f4f675c53b671911 # v2.13.0 with: egress-policy: audit @@ -50,6 +69,11 @@ jobs: java-version: ${{ matrix.jdk-version }} distribution: "temurin" + - name: Setup Gradle + uses: gradle/actions/setup-gradle@ac638b010cf58a27ee6c972d7336334ccaf61c96 # v4.4.1 + with: + gradle-version: 8.14 + - name: Build with Gradle and spring security ${{ matrix.spring-security }} run: ./gradlew clean build env: @@ -100,14 +124,17 @@ jobs: if-no-files-found: warn check-generateOpenApiDocs: + if: needs.files-changed.outputs.openapi == 'true' + needs: [files-changed, build] runs-on: ubuntu-latest steps: - name: Harden Runner - uses: step-security/harden-runner@6c439dc8bdf85cadbbce9ed30d1c7b959517bc49 # v2.12.2 + uses: step-security/harden-runner@ec9f2d5744a09debf3a187a3f4f675c53b671911 # v2.13.0 with: egress-policy: audit - - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + - name: Checkout repository + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 - name: Set up JDK 17 uses: actions/setup-java@c5195efecf7bdfc987ee8bae7a71cb8b11521c00 # v4.7.1 @@ -115,7 +142,8 @@ jobs: java-version: "17" distribution: "temurin" - - uses: gradle/actions/setup-gradle@ac638b010cf58a27ee6c972d7336334ccaf61c96 # v4.4.1 + - name: Setup Gradle + uses: gradle/actions/setup-gradle@ac638b010cf58a27ee6c972d7336334ccaf61c96 # v4.4.1 - name: Generate OpenAPI documentation run: ./gradlew :stirling-pdf:generateOpenApiDocs @@ -127,10 +155,12 @@ jobs: path: ./SwaggerDoc.json check-licence: + if: needs.files-changed.outputs.build == 'true' + needs: [files-changed, build] runs-on: ubuntu-latest steps: - name: Harden Runner - uses: step-security/harden-runner@6c439dc8bdf85cadbbce9ed30d1c7b959517bc49 # v2.12.2 + uses: step-security/harden-runner@ec9f2d5744a09debf3a187a3f4f675c53b671911 # v2.13.0 with: egress-policy: audit @@ -141,7 +171,7 @@ jobs: uses: actions/setup-java@c5195efecf7bdfc987ee8bae7a71cb8b11521c00 # v4.7.1 with: java-version: "17" - distribution: "adopt" + distribution: "temurin" - name: check the licenses for compatibility run: ./gradlew clean checkLicense @@ -156,6 +186,8 @@ jobs: retention-days: 3 docker-compose-tests: + if: needs.files-changed.outputs.project == 'true' + needs: files-changed # if: github.event_name == 'push' && github.ref == 'refs/heads/main' || # (github.event_name == 'pull_request' && # contains(github.event.pull_request.labels.*.name, 'licenses') == false && @@ -174,7 +206,7 @@ jobs: steps: - name: Harden Runner - uses: step-security/harden-runner@6c439dc8bdf85cadbbce9ed30d1c7b959517bc49 # v2.12.2 + uses: step-security/harden-runner@ec9f2d5744a09debf3a187a3f4f675c53b671911 # v2.13.0 with: egress-policy: audit @@ -185,7 +217,7 @@ jobs: uses: actions/setup-java@c5195efecf7bdfc987ee8bae7a71cb8b11521c00 # v4.7.1 with: java-version: "17" - distribution: "adopt" + distribution: "temurin" - name: Set up Docker Buildx uses: docker/setup-buildx-action@e468171a9de216ec08956ac3ada2f0791b6bd435 # v3.11.1 @@ -200,6 +232,7 @@ jobs: with: python-version: "3.12" cache: 'pip' # caching pip dependencies + cache-dependency-path: ./testing/cucumber/requirements.txt - name: Pip requirements run: | @@ -211,3 +244,69 @@ jobs: chmod +x ./testing/test.sh chmod +x ./testing/test_disabledEndpoints.sh ./testing/test.sh + + test-build-docker-images: + if: github.event_name == 'pull_request' && needs.files-changed.outputs.project == 'true' + needs: [files-changed, build, check-generateOpenApiDocs, check-licence] + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + docker-rev: ["Dockerfile", "Dockerfile.ultra-lite", "Dockerfile.fat"] + steps: + - name: Harden Runner + uses: step-security/harden-runner@ec9f2d5744a09debf3a187a3f4f675c53b671911 # v2.13.0 + with: + egress-policy: audit + + - name: Checkout Repository + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + + - name: Set up JDK 17 + uses: actions/setup-java@c5195efecf7bdfc987ee8bae7a71cb8b11521c00 # v4.7.1 + with: + java-version: "17" + distribution: "temurin" + + - name: Set up Gradle + uses: gradle/actions/setup-gradle@ac638b010cf58a27ee6c972d7336334ccaf61c96 # v4.4.1 + with: + gradle-version: 8.14 + + - name: Build application + run: ./gradlew clean build + env: + DISABLE_ADDITIONAL_FEATURES: true + STIRLING_PDF_DESKTOP_UI: false + + - name: Set up QEMU + uses: docker/setup-qemu-action@29109295f81e9208d7d86ff1c6c12d2833863392 # v3.6.0 + + - name: Set up Docker Buildx + id: buildx + uses: docker/setup-buildx-action@e468171a9de216ec08956ac3ada2f0791b6bd435 # v3.11.1 + + - name: Build ${{ matrix.docker-rev }} + uses: docker/build-push-action@263435318d21b8e681c14492fe198d362a7d2c83 # v6.18.0 + with: + builder: ${{ steps.buildx.outputs.name }} + context: . + file: ./${{ matrix.docker-rev }} + push: false + cache-from: type=gha + cache-to: type=gha,mode=max + platforms: linux/amd64,linux/arm64/v8 + provenance: true + sbom: true + + - name: Upload Reports + if: always() + uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 + with: + name: reports-docker-${{ matrix.docker-rev }} + path: | + build/reports/tests/ + build/test-results/ + build/reports/problems/ + retention-days: 3 + if-no-files-found: warn diff --git a/.gitignore b/.gitignore index 55603719c..6ebd87c35 100644 --- a/.gitignore +++ b/.gitignore @@ -200,6 +200,3 @@ 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 802a55831..aeb220dae 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 @@ -119,6 +119,7 @@ public class ApplicationProperties { private long loginResetTimeMinutes; private String loginMethod = "all"; private String customGlobalAPIKey; + private Jwt jwt = new Jwt(); public Boolean isAltLogin() { return saml2.getEnabled() || oauth2.getEnabled(); @@ -297,6 +298,12 @@ public class ApplicationProperties { } } } + + @Data + public static class Jwt { + private boolean enableKeystore = true; + private boolean enableKeyRotation = false; + } } @Data diff --git a/app/common/src/main/java/stirling/software/common/util/RequestUriUtils.java b/app/common/src/main/java/stirling/software/common/util/RequestUriUtils.java index 654c78fe9..239976b66 100644 --- a/app/common/src/main/java/stirling/software/common/util/RequestUriUtils.java +++ b/app/common/src/main/java/stirling/software/common/util/RequestUriUtils.java @@ -14,8 +14,10 @@ public class RequestUriUtils { || requestURI.startsWith(contextPath + "/images/") || requestURI.startsWith(contextPath + "/public/") || requestURI.startsWith(contextPath + "/pdfjs/") + || requestURI.startsWith(contextPath + "/pdfjs-legacy/") || requestURI.startsWith(contextPath + "/login") || requestURI.startsWith(contextPath + "/error") + || requestURI.startsWith(contextPath + "/favicon") || requestURI.endsWith(".svg") || requestURI.endsWith(".png") || requestURI.endsWith(".ico") diff --git a/app/core/src/main/resources/application.properties b/app/core/src/main/resources/application.properties index e0273eb5a..ec3a0a390 100644 --- a/app/core/src/main/resources/application.properties +++ b/app/core/src/main/resources/application.properties @@ -50,4 +50,4 @@ spring.main.allow-bean-definition-overriding=true java.io.tmpdir=${stirling.tempfiles.directory:${java.io.tmpdir}/stirling-pdf} # V2 features -v2=false +v2=true diff --git a/app/core/src/main/resources/settings.yml.template b/app/core/src/main/resources/settings.yml.template index 07e0fb7db..0814a6924 100644 --- a/app/core/src/main/resources/settings.yml.template +++ b/app/core/src/main/resources/settings.yml.template @@ -11,7 +11,7 @@ ############################################################################################################# security: - enableLogin: true # set to 'true' to enable login + enableLogin: false # set to 'true' to enable login csrfDisabled: false # 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 @@ -31,7 +31,7 @@ security: google: clientId: '' # client ID for Google OAuth2 clientSecret: '' # client secret for Google OAuth2 - scopes: email, profile # scopes for Google OAuth2 + scopes: https://www.googleapis.com/auth/userinfo.email, https://www.googleapis.com/auth/userinfo.profile # scopes for Google OAuth2 useAsUsername: email # field to use as the username for Google OAuth2. Available options are: [email | name | given_name | family_name] github: clientId: '' # client ID for GitHub OAuth2 @@ -47,24 +47,26 @@ security: scopes: openid, profile, email # specify the scopes for which the application will request permissions provider: google # set this to your OAuth Provider's name, e.g., 'google' or 'keycloak' saml2: - enabled: true # Only enabled for paid enterprise clients (enterpriseEdition.enabled must be true) - provider: authentik # The name of your Provider + enabled: false # Only enabled for paid enterprise clients (enterpriseEdition.enabled must be true) + provider: '' # The name of your Provider autoCreateUser: true # set to 'true' to allow auto-creation of non-existing users blockRegistration: false # set to 'true' to deny login with SSO without prior registration by an admin - registrationId: stirling # The name of your Service Provider (SP) app name. Should match the name in the path for your SSO & SLO URLs - idpMetadataUri: https://dev-XXXXXXXX.okta.com/app/externalKey/sso/saml/metadata # The uri for your Provider's metadata - idpSingleLoginUrl: https://dev-XXXXXXXX.okta.com/app/dev-XXXXXXXX_stirlingpdf_1/externalKey/sso/saml # The URL for initiating SSO. Provided by your Provider - idpSingleLogoutUrl: https://dev-XXXXXXXX.okta.com/app/dev-XXXXXXXX_stirlingpdf_1/externalKey/slo/saml # The URL for initiating SLO. Provided by your Provider + registrationId: stirlingpdf-dario-saml # The name of your Service Provider (SP) app name. Should match the name in the path for your SSO & SLO URLs + idpMetadataUri: https://authentik.dev.stirlingpdf.com/api/v3/providers/saml/5/metadata/ # The uri for your Provider's metadata + idpSingleLoginUrl: https://authentik.dev.stirlingpdf.com/application/saml/stirlingpdf-dario-saml/sso/binding/post/ # The URL for initiating SSO. Provided by your Provider + idpSingleLogoutUrl: https://authentik.dev.stirlingpdf.com/application/saml/stirlingpdf-dario-saml/slo/binding/post/ # The URL for initiating SLO. Provided by your Provider idpIssuer: authentik # The ID of your Provider - idpCert: classpath:authentik-Self_Signed_Certificate.pem # The certificate your Provider will use to authenticate your app's SAML authentication requests. Provided by your Provider - privateKey: classpath:private-key.key # Your private key. Generated from your keypair - spCert: classpath:cert.crt # Your signing certificate. Generated from your keypair + idpCert: classpath:authentik-Self-signed_Certificate_certificate.pem # The certificate your Provider will use to authenticate your app's SAML authentication requests. Provided by your Provider + privateKey: classpath:private_key.key # Your private key. Generated from your keypair + spCert: classpath:certificate.crt # Your signing certificate. Generated from your keypair + jwt: + enableKeyStore: true # Set to 'true' to enable JWT key store + enableKeyRotation: true # Set to 'true' to enable JWT key rotation premium: - key: 00000000-0000-0000-0000-000000000000 - enabled: true # Enable license key checks for pro/enterprise features + key: 3R3T-WFPY-UNRW-LJFA-MMXM-YVJK-WCKY-PCRT # fixme: remove + enabled: false # Enable license key checks for pro/enterprise features proFeatures: - database: false # Enable database features SSOAutoLogin: false CustomMetadata: autoUpdateMetadata: false @@ -100,7 +102,7 @@ legal: system: defaultLocale: en-US # set the default language (e.g. 'de-DE', 'fr-FR', etc) googlevisibility: false # 'true' to allow Google visibility (via robots.txt), 'false' to disallow - enableAlphaFunctionality: true # set to enable functionality which might need more testing before it fully goes live (this feature might make no changes) + enableAlphaFunctionality: false # set to enable functionality which might need more testing before it fully goes live (this feature might make no changes) showUpdate: false # see when a new update is available showUpdateOnlyAdmin: false # only admins can see when a new update is available, depending on showUpdate it must be set to 'true' customHTMLFiles: false # enable to have files placed in /customFiles/templates override the existing template HTML files 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 094a7986b..a24b109c9 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 @@ -135,7 +135,7 @@ public class SecurityConfiguration { boolean v2Enabled = appConfig.v2Enabled(); if (v2Enabled) { - http.addFilterAt( + http.addFilterBefore( jwtAuthenticationFilter(), UsernamePasswordAuthenticationFilter.class) .exceptionHandling( @@ -145,8 +145,8 @@ public class SecurityConfiguration { } http.addFilterBefore( userAuthenticationFilter, UsernamePasswordAuthenticationFilter.class) - .addFilterAfter(rateLimitingFilter(), userAuthenticationFilter.getClass()) - .addFilterAfter(firstLoginFilter, rateLimitingFilter().getClass()); + .addFilterAfter(rateLimitingFilter(), UserAuthenticationFilter.class) + .addFilterAfter(firstLoginFilter, UsernamePasswordAuthenticationFilter.class); if (!securityProperties.getCsrfDisabled()) { CookieCsrfTokenRepository cookieRepo = @@ -252,7 +252,9 @@ public class SecurityConfiguration { || trimmedUri.startsWith("/js/") || trimmedUri.startsWith("/favicon") || trimmedUri.startsWith( - "/api/v1/info/status"); + "/api/v1/info/status") + || trimmedUri.startsWith("/v1/api-docs") + || uri.contains("/v1/api-docs"); }) .permitAll() .anyRequest() diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/security/database/repository/JwtSigningKeyRepository.java b/app/proprietary/src/main/java/stirling/software/proprietary/security/database/repository/JwtSigningKeyRepository.java new file mode 100644 index 000000000..cde43ff69 --- /dev/null +++ b/app/proprietary/src/main/java/stirling/software/proprietary/security/database/repository/JwtSigningKeyRepository.java @@ -0,0 +1,18 @@ +package stirling.software.proprietary.security.database.repository; + +import java.util.Optional; + +import org.springframework.data.jpa.repository.JpaRepository; +import org.springframework.stereotype.Repository; + +import stirling.software.proprietary.security.model.JwtSigningKey; + +@Repository +public interface JwtSigningKeyRepository extends JpaRepository { + + Optional findByIsActiveTrue(); + + Optional findByKeyId(String keyId); + + Optional findByKeyIdAndIsActiveTrue(String keyId); +} diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/security/filter/JwtAuthenticationFilter.java b/app/proprietary/src/main/java/stirling/software/proprietary/security/filter/JwtAuthenticationFilter.java index eaa333a76..6aea4739a 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/security/filter/JwtAuthenticationFilter.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/security/filter/JwtAuthenticationFilter.java @@ -1,5 +1,6 @@ package stirling.software.proprietary.security.filter; +import static stirling.software.common.util.RequestUriUtils.isStaticResource; import static stirling.software.proprietary.security.model.AuthenticationType.*; import static stirling.software.proprietary.security.model.AuthenticationType.SAML2; @@ -62,7 +63,7 @@ public class JwtAuthenticationFilter extends OncePerRequestFilter { filterChain.doFilter(request, response); return; } - if (shouldNotFilter(request)) { + if (isStaticResource(request.getContextPath(), request.getRequestURI())) { filterChain.doFilter(request, response); return; } @@ -87,6 +88,8 @@ public class JwtAuthenticationFilter extends OncePerRequestFilter { try { jwtService.validateToken(jwtToken); } catch (AuthenticationFailureException e) { + // Clear invalid tokens from response + jwtService.clearTokenFromResponse(response); handleAuthenticationFailure(request, response, e); return; } @@ -128,7 +131,9 @@ public class JwtAuthenticationFilter extends OncePerRequestFilter { authToken.setDetails(new WebAuthenticationDetailsSource().buildDetails(request)); SecurityContextHolder.getContext().setAuthentication(authToken); - log.debug("JWT authentication successful for user: {}", username); + log.info( + "JWT authentication successful for user: {} - Authentication set in SecurityContext", + username); } else { throw new UsernameNotFoundException("User not found: " + username); @@ -160,49 +165,6 @@ public class JwtAuthenticationFilter extends OncePerRequestFilter { } } - @Override - protected boolean shouldNotFilter(HttpServletRequest request) { - String uri = request.getRequestURI(); - String method = request.getMethod(); - - // Skip JWT processing for logout requests to prevent token refresh during logout - if ("/logout".equals(uri)) { - return true; - } - - // Allow login POST requests to be processed - if ("/login".equals(uri) && "POST".equalsIgnoreCase(method)) { - return true; - } - - String[] permitAllPatterns = { - "/login", - "/register", - "/error", - "/images/", - "/public/", - "/css/", - "/fonts/", - "/js/", - "/pdfjs/", - "/pdfjs-legacy/", - "/api/v1/info/status", - "/site.webmanifest", - "/favicon" - }; - - for (String pattern : permitAllPatterns) { - if (uri.startsWith(pattern) - || uri.endsWith(".svg") - || uri.endsWith(".png") - || uri.endsWith(".ico")) { - return true; - } - } - - return false; - } - private void handleAuthenticationFailure( HttpServletRequest request, HttpServletResponse response, 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 8a148e931..70c9e233e 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 @@ -63,7 +63,15 @@ public class UserAuthenticationFilter extends OncePerRequestFilter { return; } String requestURI = request.getRequestURI(); + Authentication authentication = SecurityContextHolder.getContext().getAuthentication(); + log.info( + "UserAuthenticationFilter - Authentication from SecurityContext: {}", + authentication != null + ? authentication.getClass().getSimpleName() + + " for " + + authentication.getName() + : "null"); // Check for session expiration (unsure if needed) // if (authentication != null && authentication.isAuthenticated()) { @@ -220,11 +228,12 @@ public class UserAuthenticationFilter extends OncePerRequestFilter { } @Override - protected boolean shouldNotFilter(HttpServletRequest request) throws ServletException { + protected boolean shouldNotFilter(HttpServletRequest request) { String uri = request.getRequestURI(); String contextPath = request.getContextPath(); String[] permitAllPatterns = { contextPath + "/login", + contextPath + "/signup", contextPath + "/register", contextPath + "/error", contextPath + "/images/", @@ -241,6 +250,7 @@ public class UserAuthenticationFilter extends OncePerRequestFilter { for (String pattern : permitAllPatterns) { if (uri.startsWith(pattern) || uri.endsWith(".svg") + || uri.endsWith(".mjs") || uri.endsWith(".png") || uri.endsWith(".ico")) { return true; diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/security/model/AuthenticationType.java b/app/proprietary/src/main/java/stirling/software/proprietary/security/model/AuthenticationType.java index cf9f15e35..0ef0a9235 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/security/model/AuthenticationType.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/security/model/AuthenticationType.java @@ -2,7 +2,7 @@ package stirling.software.proprietary.security.model; public enum AuthenticationType { WEB, - @Deprecated(since = "1.0.2") SSO, + SSO, OAUTH2, SAML2 } diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/security/model/JwtSigningKey.java b/app/proprietary/src/main/java/stirling/software/proprietary/security/model/JwtSigningKey.java new file mode 100644 index 000000000..d1b78d8a9 --- /dev/null +++ b/app/proprietary/src/main/java/stirling/software/proprietary/security/model/JwtSigningKey.java @@ -0,0 +1,62 @@ +package stirling.software.proprietary.security.model; + +import java.io.Serializable; +import java.time.LocalDateTime; + +import jakarta.persistence.Column; +import jakarta.persistence.Entity; +import jakarta.persistence.GeneratedValue; +import jakarta.persistence.GenerationType; +import jakarta.persistence.Id; +import jakarta.persistence.Table; + +import lombok.EqualsAndHashCode; +import lombok.Getter; +import lombok.NoArgsConstructor; +import lombok.Setter; +import lombok.ToString; + +@Entity +@Getter +@Setter +@NoArgsConstructor +@Table(name = "signing_keys") +@ToString(onlyExplicitlyIncluded = true) +@EqualsAndHashCode(onlyExplicitlyIncluded = true) +public class JwtSigningKey implements Serializable { + + private static final long serialVersionUID = 1L; + + @Id + @GeneratedValue(strategy = GenerationType.IDENTITY) + @Column(name = "signing_key_id") + @EqualsAndHashCode.Include + @ToString.Include + private Long id; + + @Column(name = "key_id", nullable = false, unique = true) + @ToString.Include + private String keyId; + + @Column(name = "signing_key", columnDefinition = "TEXT", nullable = false) + private String signingKey; + + @Column(name = "algorithm", nullable = false) + private String algorithm = "RS256"; + + @Column(name = "created_at", nullable = false) + @ToString.Include + private LocalDateTime createdAt; + + @Column(name = "is_active", nullable = false) + @ToString.Include + private Boolean isActive = true; + + public JwtSigningKey(String keyId, String signingKey, String algorithm) { + this.keyId = keyId; + this.signingKey = signingKey; + this.algorithm = algorithm; + this.createdAt = LocalDateTime.now(); + this.isActive = true; + } +} diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/security/oauth2/CustomOAuth2AuthenticationSuccessHandler.java b/app/proprietary/src/main/java/stirling/software/proprietary/security/oauth2/CustomOAuth2AuthenticationSuccessHandler.java index 71227b618..dc489ffd2 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/security/oauth2/CustomOAuth2AuthenticationSuccessHandler.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/security/oauth2/CustomOAuth2AuthenticationSuccessHandler.java @@ -85,7 +85,8 @@ public class CustomOAuth2AuthenticationSuccessHandler } if (userService.usernameExistsIgnoreCase(username) && userService.hasPassword(username) - && !userService.isAuthenticationTypeByUsername(username, SSO) + && (!userService.isAuthenticationTypeByUsername(username, SSO) + || !userService.isAuthenticationTypeByUsername(username, OAUTH2)) && oauth2Properties.getAutoCreateUser()) { response.sendRedirect(contextPath + "/logout?oAuth2AuthenticationErrorWeb=true"); return; diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/security/saml2/JwtSaml2AuthenticationRequestRepository.java b/app/proprietary/src/main/java/stirling/software/proprietary/security/saml2/JwtSaml2AuthenticationRequestRepository.java index f1f2da6b1..68c190e64 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/security/saml2/JwtSaml2AuthenticationRequestRepository.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/security/saml2/JwtSaml2AuthenticationRequestRepository.java @@ -39,7 +39,7 @@ public class JwtSaml2AuthenticationRequestRepository HttpServletRequest request, HttpServletResponse response) { if (!jwtService.isJwtEnabled()) { - log.warn("SAML2 v2 is not enabled, skipping saveAuthenticationRequest"); + log.debug("V2 is not enabled, skipping SAMLRequest token storage"); return; } diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/security/service/JwtKeystoreService.java b/app/proprietary/src/main/java/stirling/software/proprietary/security/service/JwtKeystoreService.java new file mode 100644 index 000000000..72fbaba92 --- /dev/null +++ b/app/proprietary/src/main/java/stirling/software/proprietary/security/service/JwtKeystoreService.java @@ -0,0 +1,238 @@ +package stirling.software.proprietary.security.service; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.security.KeyFactory; +import java.security.KeyPair; +import java.security.KeyPairGenerator; +import java.security.NoSuchAlgorithmException; +import java.security.PrivateKey; +import java.security.PublicKey; +import java.security.spec.InvalidKeySpecException; +import java.security.spec.PKCS8EncodedKeySpec; +import java.security.spec.X509EncodedKeySpec; +import java.time.LocalDateTime; +import java.time.format.DateTimeFormatter; +import java.util.Base64; +import java.util.Optional; + +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.stereotype.Service; +import org.springframework.transaction.annotation.Transactional; + +import jakarta.annotation.PostConstruct; + +import lombok.extern.slf4j.Slf4j; + +import stirling.software.common.configuration.InstallationPathConfig; +import stirling.software.common.model.ApplicationProperties; +import stirling.software.proprietary.security.database.repository.JwtSigningKeyRepository; +import stirling.software.proprietary.security.model.JwtSigningKey; + +@Service +@Slf4j +public class JwtKeystoreService implements JwtKeystoreServiceInterface { + + public static final String KEY_SUFFIX = ".key"; + private final JwtSigningKeyRepository repository; + private final ApplicationProperties.Security.Jwt jwtProperties; + private final Path privateKeyDirectory; + + private volatile KeyPair currentKeyPair; + private volatile String currentKeyId; + + @Autowired + public JwtKeystoreService( + JwtSigningKeyRepository repository, ApplicationProperties applicationProperties) { + this.repository = repository; + this.jwtProperties = applicationProperties.getSecurity().getJwt(); + this.privateKeyDirectory = Paths.get(InstallationPathConfig.getConfigPath(), "jwt-keys"); + } + + @PostConstruct + public void initializeKeystore() { + if (!isKeystoreEnabled()) { + log.info("JWT keystore is disabled, using in-memory key generation"); + return; + } + + try { + ensurePrivateKeyDirectoryExists(); + loadOrGenerateKeypair(); + } catch (Exception e) { + log.error("Failed to initialize JWT keystore, falling back to in-memory generation", e); + } + } + + @Override + public KeyPair getActiveKeypair() { + if (!isKeystoreEnabled() || currentKeyPair == null) { + return generateRSAKeypair(); + } + return currentKeyPair; + } + + @Override + public Optional getKeypairByKeyId(String keyId) { + if (!isKeystoreEnabled()) { + return Optional.empty(); + } + + try { + Optional signingKey = repository.findByKeyId(keyId); + if (signingKey.isEmpty()) { + return Optional.empty(); + } + + PrivateKey privateKey = loadPrivateKey(keyId); + PublicKey publicKey = decodePublicKey(signingKey.get().getSigningKey()); + + return Optional.of(new KeyPair(publicKey, privateKey)); + } catch (Exception e) { + log.error("Failed to load keypair for keyId: {}", keyId, e); + return Optional.empty(); + } + } + + @Override + public String getActiveKeyId() { + return currentKeyId; + } + + @Override + @Transactional + public void rotateKeypair() { + if (!isKeystoreEnabled()) { + log.warn("Cannot rotate keypair when keystore is disabled"); + return; + } + + try { + repository + .findByIsActiveTrue() + .ifPresent( + key -> { + key.setIsActive(false); + repository.save(key); + }); + + generateAndStoreKeypair(); + log.info("Successfully rotated JWT keypair"); + } catch (Exception e) { + log.error("Failed to rotate JWT keypair", e); + throw new RuntimeException("Keypair rotation failed", e); + } + } + + @Override + public boolean isKeystoreEnabled() { + return jwtProperties.isEnableKeystore(); + } + + private void loadOrGenerateKeypair() { + Optional activeKey = repository.findByIsActiveTrue(); + + if (activeKey.isPresent()) { + try { + currentKeyId = activeKey.get().getKeyId(); + PrivateKey privateKey = loadPrivateKey(currentKeyId); + PublicKey publicKey = decodePublicKey(activeKey.get().getSigningKey()); + currentKeyPair = new KeyPair(publicKey, privateKey); + log.info("Loaded existing JWT keypair with keyId: {}", currentKeyId); + } catch (Exception e) { + log.error("Failed to load existing keypair, generating new one", e); + generateAndStoreKeypair(); + } + } else { + generateAndStoreKeypair(); + } + } + + private void generateAndStoreKeypair() { + try { + KeyPair keyPair = generateRSAKeypair(); + String keyId = generateKeyId(); + + storePrivateKey(keyId, keyPair.getPrivate()); + + JwtSigningKey signingKey = + new JwtSigningKey(keyId, encodePublicKey(keyPair.getPublic()), "RS256"); + repository.save(signingKey); + currentKeyPair = keyPair; + currentKeyId = keyId; + + log.info("Generated and stored new JWT keypair with keyId: {}", keyId); + } catch (Exception e) { + log.error("Failed to generate and store keypair", e); + throw new RuntimeException("Keypair generation failed", e); + } + } + + private KeyPair generateRSAKeypair() { + KeyPairGenerator keyPairGenerator = null; + + try { + keyPairGenerator = KeyPairGenerator.getInstance("RSA"); + keyPairGenerator.initialize(2048); + } catch (NoSuchAlgorithmException e) { + throw new RuntimeException("Failed to initialize RSA key pair generator", e); + } + + return keyPairGenerator.generateKeyPair(); + } + + private String generateKeyId() { + return "jwt-key-" + + LocalDateTime.now().format(DateTimeFormatter.ofPattern("yyyy-MM-dd-HHmmss")); + } + + private void ensurePrivateKeyDirectoryExists() throws IOException { + if (!Files.exists(privateKeyDirectory)) { + Files.createDirectories(privateKeyDirectory); + log.info("Created JWT private key directory: {}", privateKeyDirectory); + } + } + + private void storePrivateKey(String keyId, PrivateKey privateKey) throws IOException { + Path keyFile = privateKeyDirectory.resolve(keyId + KEY_SUFFIX); + String encodedKey = Base64.getEncoder().encodeToString(privateKey.getEncoded()); + Files.writeString(keyFile, encodedKey); + + // Set read/write to only the owner + try { + keyFile.toFile().setReadable(true, true); + keyFile.toFile().setWritable(true, true); + keyFile.toFile().setExecutable(false, false); + } catch (Exception e) { + log.warn("Failed to set permissions on private key file: {}", keyFile, e); + } + } + + private PrivateKey loadPrivateKey(String keyId) + throws IOException, NoSuchAlgorithmException, InvalidKeySpecException { + Path keyFile = privateKeyDirectory.resolve(keyId + KEY_SUFFIX); + if (!Files.exists(keyFile)) { + throw new IOException("Private key file not found: " + keyFile); + } + + String encodedKey = Files.readString(keyFile); + byte[] keyBytes = Base64.getDecoder().decode(encodedKey); + PKCS8EncodedKeySpec keySpec = new PKCS8EncodedKeySpec(keyBytes); + KeyFactory keyFactory = KeyFactory.getInstance("RSA"); + return keyFactory.generatePrivate(keySpec); + } + + private String encodePublicKey(PublicKey publicKey) { + return Base64.getEncoder().encodeToString(publicKey.getEncoded()); + } + + private PublicKey decodePublicKey(String encodedKey) + throws NoSuchAlgorithmException, InvalidKeySpecException { + byte[] keyBytes = Base64.getDecoder().decode(encodedKey); + X509EncodedKeySpec keySpec = new X509EncodedKeySpec(keyBytes); + KeyFactory keyFactory = KeyFactory.getInstance("RSA"); + return keyFactory.generatePublic(keySpec); + } +} diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/security/service/JwtKeystoreServiceInterface.java b/app/proprietary/src/main/java/stirling/software/proprietary/security/service/JwtKeystoreServiceInterface.java new file mode 100644 index 000000000..4df7d552d --- /dev/null +++ b/app/proprietary/src/main/java/stirling/software/proprietary/security/service/JwtKeystoreServiceInterface.java @@ -0,0 +1,17 @@ +package stirling.software.proprietary.security.service; + +import java.security.KeyPair; +import java.util.Optional; + +public interface JwtKeystoreServiceInterface { + + KeyPair getActiveKeypair(); + + Optional getKeypairByKeyId(String keyId); + + String getActiveKeyId(); + + void rotateKeypair(); + + boolean isKeystoreEnabled(); +} diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/security/service/JwtService.java b/app/proprietary/src/main/java/stirling/software/proprietary/security/service/JwtService.java index 2ae2197b8..b903767ff 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/security/service/JwtService.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/security/service/JwtService.java @@ -4,8 +4,10 @@ import java.security.KeyPair; import java.util.Date; import java.util.HashMap; import java.util.Map; +import java.util.Optional; import java.util.function.Function; +import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Qualifier; import org.springframework.http.ResponseCookie; import org.springframework.security.core.Authentication; @@ -40,12 +42,15 @@ public class JwtService implements JwtServiceInterface { private static final String ISSUER = "Stirling PDF"; private static final long EXPIRATION = 3600000; - private final KeyPair keyPair; + private final JwtKeystoreServiceInterface keystoreService; private final boolean v2Enabled; - public JwtService(@Qualifier("v2Enabled") boolean v2Enabled) { + @Autowired + public JwtService( + @Qualifier("v2Enabled") boolean v2Enabled, + JwtKeystoreServiceInterface keystoreService) { this.v2Enabled = v2Enabled; - keyPair = Jwts.SIG.RS256.keyPair().build(); + this.keystoreService = keystoreService; } @Override @@ -66,14 +71,23 @@ public class JwtService implements JwtServiceInterface { @Override public String generateToken(String username, Map claims) { - return Jwts.builder() - .claims(claims) - .subject(username) - .issuer(ISSUER) - .issuedAt(new Date()) - .expiration(new Date(System.currentTimeMillis() + EXPIRATION)) - .signWith(keyPair.getPrivate(), Jwts.SIG.RS256) - .compact(); + KeyPair keyPair = keystoreService.getActiveKeypair(); + + var builder = + Jwts.builder() + .claims(claims) + .subject(username) + .issuer(ISSUER) + .issuedAt(new Date()) + .expiration(new Date(System.currentTimeMillis() + EXPIRATION)) + .signWith(keyPair.getPrivate(), Jwts.SIG.RS256); + + String keyId = keystoreService.getActiveKeyId(); + if (keyId != null) { + builder.header().keyId(keyId); + } + + return builder.compact(); } @Override @@ -112,6 +126,25 @@ public class JwtService implements JwtServiceInterface { private Claims extractAllClaimsFromToken(String token) { try { + // Extract key ID from token header if present + String keyId = extractKeyIdFromToken(token); + KeyPair keyPair; + + if (keyId != null) { + Optional specificKeyPair = keystoreService.getKeypairByKeyId(keyId); + if (specificKeyPair.isPresent()) { + keyPair = specificKeyPair.get(); + } else { + log.warn( + "Key ID {} not found in keystore, token may have been signed with a rotated key", + keyId); + throw new AuthenticationFailureException( + "JWT token signed with unknown key ID: " + keyId); + } + } else { + keyPair = keystoreService.getActiveKeypair(); + } + return Jwts.parser() .verifyWith(keyPair.getPublic()) .build() @@ -191,4 +224,19 @@ public class JwtService implements JwtServiceInterface { public boolean isJwtEnabled() { return v2Enabled; } + + private String extractKeyIdFromToken(String token) { + try { + return (String) + Jwts.parser() + .unsecured() + .build() + .parseUnsecuredClaims(token) + .getHeader() + .get("kid"); + } catch (Exception e) { + log.debug("Failed to extract key ID from token header: {}", e.getMessage()); + return null; + } + } } diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/security/service/UserService.java b/app/proprietary/src/main/java/stirling/software/proprietary/security/service/UserService.java index 8cddebcbf..982f551ca 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/security/service/UserService.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/security/service/UserService.java @@ -1,5 +1,8 @@ package stirling.software.proprietary.security.service; +import static stirling.software.proprietary.security.model.AuthenticationType.OAUTH2; +import static stirling.software.proprietary.security.model.AuthenticationType.SSO; + import java.sql.SQLException; import java.util.ArrayList; import java.util.Collection; @@ -63,10 +66,10 @@ public class UserService implements UserServiceInterface { @Transactional public void migrateOauth2ToSSO() { userRepository - .findByAuthenticationTypeIgnoreCase("OAUTH2") + .findByAuthenticationTypeIgnoreCase(OAUTH2.toString()) .forEach( user -> { - user.setAuthenticationType(AuthenticationType.SSO); + user.setAuthenticationType(SSO); userRepository.save(user); }); } diff --git a/app/proprietary/src/test/java/stirling/software/proprietary/security/filter/JwtAuthenticationFilterTest.java b/app/proprietary/src/test/java/stirling/software/proprietary/security/filter/JwtAuthenticationFilterTest.java index ecb84122a..7e67a9106 100644 --- a/app/proprietary/src/test/java/stirling/software/proprietary/security/filter/JwtAuthenticationFilterTest.java +++ b/app/proprietary/src/test/java/stirling/software/proprietary/security/filter/JwtAuthenticationFilterTest.java @@ -88,12 +88,11 @@ class JwtAuthenticationFilterTest { void shouldNotFilterWhenPageIsLogin() throws ServletException, IOException { when(jwtService.isJwtEnabled()).thenReturn(true); when(request.getRequestURI()).thenReturn("/login"); - when(request.getMethod()).thenReturn("POST"); + when(request.getContextPath()).thenReturn("/login"); jwtAuthenticationFilter.doFilterInternal(request, response, filterChain); - verify(filterChain).doFilter(request, response); - verify(jwtService, never()).extractTokenFromRequest(any()); + verify(filterChain, never()).doFilter(request, response); } @Test @@ -104,8 +103,8 @@ class JwtAuthenticationFilterTest { Map claims = Map.of("sub", username, "authType", "WEB"); when(jwtService.isJwtEnabled()).thenReturn(true); + when(request.getContextPath()).thenReturn("/"); when(request.getRequestURI()).thenReturn("/protected"); - when(request.getMethod()).thenReturn("GET"); when(jwtService.extractTokenFromRequest(request)).thenReturn(token); doNothing().when(jwtService).validateToken(token); when(jwtService.extractAllClaims(token)).thenReturn(claims); @@ -151,7 +150,7 @@ class JwtAuthenticationFilterTest { when(jwtService.isJwtEnabled()).thenReturn(true); when(request.getRequestURI()).thenReturn("/protected"); - when(request.getMethod()).thenReturn("GET"); + when(request.getContextPath()).thenReturn("/"); when(jwtService.extractTokenFromRequest(request)).thenReturn(token); doThrow(new AuthenticationFailureException("Invalid token")).when(jwtService).validateToken(token); @@ -168,7 +167,7 @@ class JwtAuthenticationFilterTest { when(jwtService.isJwtEnabled()).thenReturn(true); when(request.getRequestURI()).thenReturn("/protected"); - when(request.getMethod()).thenReturn("GET"); + when(request.getContextPath()).thenReturn("/"); when(jwtService.extractTokenFromRequest(request)).thenReturn(token); doThrow(new AuthenticationFailureException("The token has expired")).when(jwtService).validateToken(token); @@ -187,7 +186,7 @@ class JwtAuthenticationFilterTest { when(jwtService.isJwtEnabled()).thenReturn(true); when(request.getRequestURI()).thenReturn("/protected"); - when(request.getMethod()).thenReturn("GET"); + when(request.getContextPath()).thenReturn("/"); when(jwtService.extractTokenFromRequest(request)).thenReturn(token); doNothing().when(jwtService).validateToken(token); when(jwtService.extractAllClaims(token)).thenReturn(claims); @@ -205,96 +204,11 @@ class JwtAuthenticationFilterTest { } } - @Test - void shouldNotFilterLoginPost() { - when(request.getRequestURI()).thenReturn("/login"); - when(request.getMethod()).thenReturn("POST"); - - assertTrue(jwtAuthenticationFilter.shouldNotFilter(request)); - } - - @Test - void shouldNotFilterLoginGet() { - when(request.getRequestURI()).thenReturn("/login"); - when(request.getMethod()).thenReturn("GET"); - - assertTrue(jwtAuthenticationFilter.shouldNotFilter(request)); - } - - @Test - void shouldNotFilterPublicPaths() { - String[] publicPaths = { - "/register", - "/error", - "/images/logo.png", - "/public/file.txt", - "/css/style.css", - "/fonts/font.ttf", - "/js/script.js", - "/pdfjs/viewer.js", - "/pdfjs-legacy/viewer.js", - "/api/v1/info/status", - "/site.webmanifest", - "/favicon.ico" - }; - - for (String path : publicPaths) { - when(request.getRequestURI()).thenReturn(path); - when(request.getMethod()).thenReturn("GET"); - - assertTrue(jwtAuthenticationFilter.shouldNotFilter(request), - "Should not filter path: " + path); - } - } - - @Test - void shouldNotFilterStaticFiles() { - String[] staticFiles = { - "/some/path/file.svg", - "/another/path/image.png", - "/path/to/icon.ico" - }; - - for (String file : staticFiles) { - when(request.getRequestURI()).thenReturn(file); - when(request.getMethod()).thenReturn("GET"); - - assertTrue(jwtAuthenticationFilter.shouldNotFilter(request), - "Should not filter file: " + file); - } - } - - @Test - void shouldFilterProtectedPaths() { - String[] protectedPaths = { - "/protected", - "/api/v1/user/profile", - "/admin", - "/dashboard" - }; - - for (String path : protectedPaths) { - when(request.getRequestURI()).thenReturn(path); - when(request.getMethod()).thenReturn("GET"); - - assertFalse(jwtAuthenticationFilter.shouldNotFilter(request), - "Should filter path: " + path); - } - } - - @Test - void shouldFilterRootPath() { - when(request.getRequestURI()).thenReturn("/"); - when(request.getMethod()).thenReturn("GET"); - - assertFalse(jwtAuthenticationFilter.shouldNotFilter(request)); - } - @Test void testAuthenticationEntryPointCalledWithCorrectException() throws ServletException, IOException { when(jwtService.isJwtEnabled()).thenReturn(true); when(request.getRequestURI()).thenReturn("/protected"); - when(request.getMethod()).thenReturn("GET"); + when(request.getContextPath()).thenReturn("/"); when(jwtService.extractTokenFromRequest(request)).thenReturn(null); jwtAuthenticationFilter.doFilterInternal(request, response, filterChain); diff --git a/app/proprietary/src/test/java/stirling/software/proprietary/security/service/JwtKeystoreServiceInterfaceTest.java b/app/proprietary/src/test/java/stirling/software/proprietary/security/service/JwtKeystoreServiceInterfaceTest.java new file mode 100644 index 000000000..98e2b836d --- /dev/null +++ b/app/proprietary/src/test/java/stirling/software/proprietary/security/service/JwtKeystoreServiceInterfaceTest.java @@ -0,0 +1,258 @@ +package stirling.software.proprietary.security.service; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.*; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.security.KeyPair; +import java.security.KeyPairGenerator; +import java.security.NoSuchAlgorithmException; +import java.util.Base64; +import java.util.Optional; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.api.io.TempDir; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; +import org.mockito.Mock; +import org.mockito.MockedStatic; +import org.mockito.junit.jupiter.MockitoExtension; + +import stirling.software.common.configuration.InstallationPathConfig; +import stirling.software.common.model.ApplicationProperties; +import stirling.software.proprietary.security.database.repository.JwtSigningKeyRepository; +import stirling.software.proprietary.security.model.JwtSigningKey; + +@ExtendWith(MockitoExtension.class) +class JwtKeystoreServiceInterfaceTest { + + @Mock + private JwtSigningKeyRepository repository; + + @Mock + private ApplicationProperties applicationProperties; + + @Mock + private ApplicationProperties.Security security; + + @Mock + private ApplicationProperties.Security.Jwt jwtConfig; + + @TempDir + Path tempDir; + + private JwtKeystoreService keystoreService; + private KeyPair testKeyPair; + + @BeforeEach + void setUp() throws NoSuchAlgorithmException { + KeyPairGenerator keyPairGenerator = KeyPairGenerator.getInstance("RSA"); + keyPairGenerator.initialize(2048); + testKeyPair = keyPairGenerator.generateKeyPair(); + + when(applicationProperties.getSecurity()).thenReturn(security); + when(security.getJwt()).thenReturn(jwtConfig); + when(jwtConfig.isEnableKeystore()).thenReturn(true); + } + + @ParameterizedTest + @ValueSource(booleans = {true, false}) + void testKeystoreEnabled(boolean keystoreEnabled) { + when(jwtConfig.isEnableKeystore()).thenReturn(keystoreEnabled); + + try (MockedStatic mockedStatic = mockStatic(InstallationPathConfig.class)) { + mockedStatic.when(InstallationPathConfig::getConfigPath).thenReturn(tempDir.toString()); + keystoreService = new JwtKeystoreService(repository, applicationProperties); + + assertEquals(keystoreEnabled, keystoreService.isKeystoreEnabled()); + } + } + + @Test + void testGetActiveKeypairWhenKeystoreDisabled() { + when(jwtConfig.isEnableKeystore()).thenReturn(false); + + try (MockedStatic mockedStatic = mockStatic(InstallationPathConfig.class)) { + mockedStatic.when(InstallationPathConfig::getConfigPath).thenReturn(tempDir.toString()); + keystoreService = new JwtKeystoreService(repository, applicationProperties); + + KeyPair result = keystoreService.getActiveKeypair(); + + assertNotNull(result); + assertNotNull(result.getPublic()); + assertNotNull(result.getPrivate()); + } + } + + @Test + void testGetActiveKeypairWhenNoActiveKeyExists() { + when(repository.findByIsActiveTrue()).thenReturn(Optional.empty()); + + try (MockedStatic mockedStatic = mockStatic(InstallationPathConfig.class)) { + mockedStatic.when(InstallationPathConfig::getConfigPath).thenReturn(tempDir.toString()); + keystoreService = new JwtKeystoreService(repository, applicationProperties); + keystoreService.initializeKeystore(); + + KeyPair result = keystoreService.getActiveKeypair(); + + assertNotNull(result); + verify(repository).save(any(JwtSigningKey.class)); + } + } + + @Test + void testGetActiveKeypairWithExistingKey() throws Exception { + String keyId = "test-key-2024-01-01-120000"; + String publicKeyBase64 = Base64.getEncoder().encodeToString(testKeyPair.getPublic().getEncoded()); + String privateKeyBase64 = Base64.getEncoder().encodeToString(testKeyPair.getPrivate().getEncoded()); + + JwtSigningKey existingKey = new JwtSigningKey(keyId, publicKeyBase64, "RS256"); + when(repository.findByIsActiveTrue()).thenReturn(Optional.of(existingKey)); + + Path keyFile = tempDir.resolve("jwt-keys").resolve(keyId + ".key"); + Files.createDirectories(keyFile.getParent()); + Files.writeString(keyFile, privateKeyBase64); + + try (MockedStatic mockedStatic = mockStatic(InstallationPathConfig.class)) { + mockedStatic.when(InstallationPathConfig::getConfigPath).thenReturn(tempDir.toString()); + keystoreService = new JwtKeystoreService(repository, applicationProperties); + keystoreService.initializeKeystore(); + + KeyPair result = keystoreService.getActiveKeypair(); + + assertNotNull(result); + assertEquals(keyId, keystoreService.getActiveKeyId()); + } + } + + @Test + void testGetKeypairByKeyId() throws Exception { + String keyId = "test-key-123"; + String publicKeyBase64 = Base64.getEncoder().encodeToString(testKeyPair.getPublic().getEncoded()); + String privateKeyBase64 = Base64.getEncoder().encodeToString(testKeyPair.getPrivate().getEncoded()); + + JwtSigningKey signingKey = new JwtSigningKey(keyId, publicKeyBase64, "RS256"); + when(repository.findByKeyId(keyId)).thenReturn(Optional.of(signingKey)); + + Path keyFile = tempDir.resolve("jwt-keys").resolve(keyId + ".key"); + Files.createDirectories(keyFile.getParent()); + Files.writeString(keyFile, privateKeyBase64); + + try (MockedStatic mockedStatic = mockStatic(InstallationPathConfig.class)) { + mockedStatic.when(InstallationPathConfig::getConfigPath).thenReturn(tempDir.toString()); + keystoreService = new JwtKeystoreService(repository, applicationProperties); + + Optional result = keystoreService.getKeypairByKeyId(keyId); + + assertTrue(result.isPresent()); + assertNotNull(result.get().getPublic()); + assertNotNull(result.get().getPrivate()); + } + } + + @Test + void testGetKeypairByKeyIdNotFound() { + String keyId = "non-existent-key"; + when(repository.findByKeyId(keyId)).thenReturn(Optional.empty()); + + try (MockedStatic mockedStatic = mockStatic(InstallationPathConfig.class)) { + mockedStatic.when(InstallationPathConfig::getConfigPath).thenReturn(tempDir.toString()); + keystoreService = new JwtKeystoreService(repository, applicationProperties); + + Optional result = keystoreService.getKeypairByKeyId(keyId); + + assertFalse(result.isPresent()); + } + } + + @Test + void testGetKeypairByKeyIdWhenKeystoreDisabled() { + when(jwtConfig.isEnableKeystore()).thenReturn(false); + + try (MockedStatic mockedStatic = mockStatic(InstallationPathConfig.class)) { + mockedStatic.when(InstallationPathConfig::getConfigPath).thenReturn(tempDir.toString()); + keystoreService = new JwtKeystoreService(repository, applicationProperties); + + Optional result = keystoreService.getKeypairByKeyId("any-key"); + + assertFalse(result.isPresent()); + } + } + + @Test + void testRotateKeypair() { + String oldKeyId = "old-key-123"; + JwtSigningKey oldKey = new JwtSigningKey(oldKeyId, "old-public-key", "RS256"); + when(repository.findByIsActiveTrue()).thenReturn(Optional.of(oldKey)); + + try (MockedStatic mockedStatic = mockStatic(InstallationPathConfig.class)) { + mockedStatic.when(InstallationPathConfig::getConfigPath).thenReturn(tempDir.toString()); + keystoreService = new JwtKeystoreService(repository, applicationProperties); + + keystoreService.initializeKeystore(); + + keystoreService.rotateKeypair(); + + assertFalse(oldKey.getIsActive()); + verify(repository, atLeast(2)).save(any(JwtSigningKey.class)); // At least one for deactivation, one for new key + + assertNotNull(keystoreService.getActiveKeyId()); + assertNotEquals(oldKeyId, keystoreService.getActiveKeyId()); + } + } + + @Test + void testRotateKeypairWhenKeystoreDisabled() { + when(jwtConfig.isEnableKeystore()).thenReturn(false); + + try (MockedStatic mockedStatic = mockStatic(InstallationPathConfig.class)) { + mockedStatic.when(InstallationPathConfig::getConfigPath).thenReturn(tempDir.toString()); + keystoreService = new JwtKeystoreService(repository, applicationProperties); + + assertDoesNotThrow(() -> keystoreService.rotateKeypair()); + + verify(repository, never()).save(any()); + } + } + + @Test + void testInitializeKeystoreCreatesDirectory() throws IOException { + when(repository.findByIsActiveTrue()).thenReturn(Optional.empty()); + + try (MockedStatic mockedStatic = mockStatic(InstallationPathConfig.class)) { + mockedStatic.when(InstallationPathConfig::getConfigPath).thenReturn(tempDir.toString()); + keystoreService = new JwtKeystoreService(repository, applicationProperties); + keystoreService.initializeKeystore(); + + Path jwtKeysDir = tempDir.resolve("jwt-keys"); + assertTrue(Files.exists(jwtKeysDir)); + assertTrue(Files.isDirectory(jwtKeysDir)); + } + } + + @Test + void testLoadExistingKeypairWithMissingPrivateKeyFile() { + String keyId = "test-key-missing-file"; + String publicKeyBase64 = Base64.getEncoder().encodeToString(testKeyPair.getPublic().getEncoded()); + + JwtSigningKey existingKey = new JwtSigningKey(keyId, publicKeyBase64, "RS256"); + when(repository.findByIsActiveTrue()).thenReturn(Optional.of(existingKey)); + + try (MockedStatic mockedStatic = mockStatic(InstallationPathConfig.class)) { + mockedStatic.when(InstallationPathConfig::getConfigPath).thenReturn(tempDir.toString()); + keystoreService = new JwtKeystoreService(repository, applicationProperties); + keystoreService.initializeKeystore(); + + KeyPair result = keystoreService.getActiveKeypair(); + assertNotNull(result); + + verify(repository).save(any(JwtSigningKey.class)); + } + } + +} diff --git a/app/proprietary/src/test/java/stirling/software/proprietary/security/service/JwtServiceTest.java b/app/proprietary/src/test/java/stirling/software/proprietary/security/service/JwtServiceTest.java index d108d7db9..f5017a329 100644 --- a/app/proprietary/src/test/java/stirling/software/proprietary/security/service/JwtServiceTest.java +++ b/app/proprietary/src/test/java/stirling/software/proprietary/security/service/JwtServiceTest.java @@ -3,7 +3,11 @@ package stirling.software.proprietary.security.service; import jakarta.servlet.http.Cookie; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; +import java.security.KeyPair; +import java.security.KeyPairGenerator; +import java.security.NoSuchAlgorithmException; import java.util.Collections; +import java.util.Optional; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -24,8 +28,10 @@ import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.atLeast; import static org.mockito.Mockito.contains; import static org.mockito.Mockito.eq; +import static org.mockito.Mockito.lenient; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -48,19 +54,28 @@ class JwtServiceTest { @Mock private HttpServletResponse response; + @Mock + private JwtKeystoreServiceInterface keystoreService; + private JwtService jwtService; + private KeyPair testKeyPair; @BeforeEach - void setUp() { - jwtService = new JwtService(true); + void setUp() throws NoSuchAlgorithmException { + // Generate a test keypair + KeyPairGenerator keyPairGenerator = KeyPairGenerator.getInstance("RSA"); + keyPairGenerator.initialize(2048); + testKeyPair = keyPairGenerator.generateKeyPair(); + + jwtService = new JwtService(true, keystoreService); } @Test void testGenerateTokenWithAuthentication() { String username = "testuser"; - when(authentication.getPrincipal()).thenReturn(userDetails); - when(userDetails.getUsername()).thenReturn(username); + when(keystoreService.getActiveKeypair()).thenReturn(testKeyPair); + when(keystoreService.getActiveKeyId()).thenReturn("test-key-id"); when(authentication.getPrincipal()).thenReturn(userDetails); when(userDetails.getUsername()).thenReturn(username); @@ -78,6 +93,8 @@ class JwtServiceTest { claims.put("role", "admin"); claims.put("department", "IT"); + when(keystoreService.getActiveKeypair()).thenReturn(testKeyPair); + when(keystoreService.getActiveKeyId()).thenReturn("test-key-id"); when(authentication.getPrincipal()).thenReturn(userDetails); when(userDetails.getUsername()).thenReturn(username); @@ -94,6 +111,11 @@ class JwtServiceTest { @Test void testValidateTokenSuccess() { + when(keystoreService.getActiveKeypair()).thenReturn(testKeyPair); + when(keystoreService.getActiveKeyId()).thenReturn("test-key-id"); + when(authentication.getPrincipal()).thenReturn(userDetails); + when(userDetails.getUsername()).thenReturn("testuser"); + String token = jwtService.generateToken(authentication, new HashMap<>()); assertDoesNotThrow(() -> jwtService.validateToken(token)); @@ -101,6 +123,8 @@ class JwtServiceTest { @Test void testValidateTokenWithInvalidToken() { + when(keystoreService.getActiveKeypair()).thenReturn(testKeyPair); + assertThrows(AuthenticationFailureException.class, () -> { jwtService.validateToken("invalid-token"); }); @@ -108,6 +132,8 @@ class JwtServiceTest { @Test void testValidateTokenWithMalformedToken() { + when(keystoreService.getActiveKeypair()).thenReturn(testKeyPair); + AuthenticationFailureException exception = assertThrows(AuthenticationFailureException.class, () -> { jwtService.validateToken("malformed.token"); }); @@ -117,6 +143,8 @@ class JwtServiceTest { @Test void testValidateTokenWithEmptyToken() { + when(keystoreService.getActiveKeypair()).thenReturn(testKeyPair); + AuthenticationFailureException exception = assertThrows(AuthenticationFailureException.class, () -> { jwtService.validateToken(""); }); @@ -130,6 +158,8 @@ class JwtServiceTest { User user = mock(User.class); Map claims = Map.of("sub", "testuser", "authType", "WEB"); + when(keystoreService.getActiveKeypair()).thenReturn(testKeyPair); + when(keystoreService.getActiveKeyId()).thenReturn("test-key-id"); when(authentication.getPrincipal()).thenReturn(user); when(user.getUsername()).thenReturn(username); @@ -140,6 +170,8 @@ class JwtServiceTest { @Test void testExtractUsernameWithInvalidToken() { + when(keystoreService.getActiveKeypair()).thenReturn(testKeyPair); + assertThrows(AuthenticationFailureException.class, () -> jwtService.extractUsername("invalid-token")); } @@ -148,6 +180,8 @@ class JwtServiceTest { String username = "testuser"; Map claims = Map.of("role", "admin", "department", "IT"); + when(keystoreService.getActiveKeypair()).thenReturn(testKeyPair); + when(keystoreService.getActiveKeyId()).thenReturn("test-key-id"); when(authentication.getPrincipal()).thenReturn(userDetails); when(userDetails.getUsername()).thenReturn(username); @@ -162,6 +196,8 @@ class JwtServiceTest { @Test void testExtractAllClaimsWithInvalidToken() { + when(keystoreService.getActiveKeypair()).thenReturn(testKeyPair); + assertThrows(AuthenticationFailureException.class, () -> jwtService.extractAllClaims("invalid-token")); } @@ -228,4 +264,67 @@ class JwtServiceTest { verify(response).addHeader(eq("Set-Cookie"), contains("stirling_jwt=")); verify(response).addHeader(eq("Set-Cookie"), contains("Max-Age=0")); } + + @Test + void testGenerateTokenWithKeyId() { + String username = "testuser"; + Map claims = new HashMap<>(); + + when(keystoreService.getActiveKeypair()).thenReturn(testKeyPair); + when(keystoreService.getActiveKeyId()).thenReturn("test-key-id"); + when(authentication.getPrincipal()).thenReturn(userDetails); + when(userDetails.getUsername()).thenReturn(username); + + String token = jwtService.generateToken(authentication, claims); + + assertNotNull(token); + assertFalse(token.isEmpty()); + // Verify that the keystore service was called + verify(keystoreService).getActiveKeypair(); + verify(keystoreService).getActiveKeyId(); + } + + @Test + void testTokenVerificationWithSpecificKeyId() throws NoSuchAlgorithmException { + String username = "testuser"; + Map claims = new HashMap<>(); + + when(keystoreService.getActiveKeypair()).thenReturn(testKeyPair); + when(keystoreService.getActiveKeyId()).thenReturn("test-key-id"); + when(authentication.getPrincipal()).thenReturn(userDetails); + when(userDetails.getUsername()).thenReturn(username); + + // Generate token with key ID + String token = jwtService.generateToken(authentication, claims); + + // Mock extraction of key ID and verification (lenient to avoid unused stubbing) + lenient().when(keystoreService.getKeypairByKeyId("test-key-id")).thenReturn(Optional.of(testKeyPair)); + + // Verify token can be validated + assertDoesNotThrow(() -> jwtService.validateToken(token)); + assertEquals(username, jwtService.extractUsername(token)); + } + + @Test + void testTokenVerificationFallsBackToActiveKeyWhenKeyIdNotFound() { + String username = "testuser"; + Map claims = new HashMap<>(); + + when(keystoreService.getActiveKeypair()).thenReturn(testKeyPair); + when(keystoreService.getActiveKeyId()).thenReturn("test-key-id"); + when(authentication.getPrincipal()).thenReturn(userDetails); + when(userDetails.getUsername()).thenReturn(username); + + String token = jwtService.generateToken(authentication, claims); + + // Mock scenario where specific key ID is not found (lenient to avoid unused stubbing) + lenient().when(keystoreService.getKeypairByKeyId("test-key-id")).thenReturn(Optional.empty()); + + // Should still work using active keypair + assertDoesNotThrow(() -> jwtService.validateToken(token)); + assertEquals(username, jwtService.extractUsername(token)); + + // Verify fallback to active keypair was used (called multiple times during token operations) + verify(keystoreService, atLeast(1)).getActiveKeypair(); + } }