From 05b5771c89ccb9cb1b357d14fba21e2013420b12 Mon Sep 17 00:00:00 2001 From: Ludy Date: Sat, 9 Aug 2025 16:09:50 +0200 Subject: [PATCH] fix(saml): correct ClassPathResource handling for IdP metadata and add null-guard for privateKey (#4157) ## Description of Changes **What was changed** - In `getIdpMetadataUri()`, use `idpMetadataUri.substring("classpath:".length())` so the `classpath:` scheme (including the colon) is stripped correctly before creating the `ClassPathResource`. - In `getPrivateKey()`, add a null check (`if (privateKey == null) return null;`) to avoid a potential `NullPointerException` when the property is unset. **Why the change was made** - The previous substring used `"classpath".length()` (without the colon), leaving a leading `:` in the path (e.g., `:/saml/idp.xml`) which breaks `ClassPathResource` resolution and can prevent SAML bootstrapping when `idpMetadataUri` uses the `classpath:` scheme. - The null-guard aligns the method with defensive coding practices and prevents runtime errors when no private key is configured. --- ## Checklist ### General - [x] I have read the [Contribution Guidelines](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/CONTRIBUTING.md) - [x] I have read the [Stirling-PDF Developer Guide](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/devGuide/DeveloperGuide.md) (if applicable) - [ ] I have read the [How to add new languages to Stirling-PDF](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/devGuide/HowToAddNewLanguage.md) (if applicable) - [x] I have performed a self-review of my own code - [x] My changes generate no new warnings ### Documentation - [ ] I have updated relevant docs on [Stirling-PDF's doc repo](https://github.com/Stirling-Tools/Stirling-Tools.github.io/blob/main/docs/) (if functionality has heavily changed) - [ ] I have read the section [Add New Translation Tags](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/devGuide/HowToAddNewLanguage.md#add-new-translation-tags) (for new translation tags only) ### UI Changes (if applicable) - [ ] Screenshots or videos demonstrating the UI changes are attached (e.g., as comments or direct attachments in the PR) ### Testing (if applicable) - [ ] I have tested my changes locally. Refer to the [Testing Guide](https://github.com/Stirling-Tools/Stirling-PDF/blob/main/devGuide/DeveloperGuide.md#6-testing) for more details. --- .../common/model/ApplicationProperties.java | 3 +- ...opertiesDynamicYamlPropertySourceTest.java | 59 +++++ .../model/ApplicationPropertiesLogicTest.java | 248 ++++++++++++++++++ .../ApplicationPropertiesSaml2HttpTest.java | 80 ++++++ ...pplicationPropertiesSaml2ResourceTest.java | 55 ++++ app/common/src/test/resources/saml/dummy.txt | 1 + build.gradle | 19 +- 7 files changed, 463 insertions(+), 2 deletions(-) create mode 100644 app/common/src/test/java/stirling/software/common/model/ApplicationPropertiesDynamicYamlPropertySourceTest.java create mode 100644 app/common/src/test/java/stirling/software/common/model/ApplicationPropertiesLogicTest.java create mode 100644 app/common/src/test/java/stirling/software/common/model/ApplicationPropertiesSaml2HttpTest.java create mode 100644 app/common/src/test/java/stirling/software/common/model/ApplicationPropertiesSaml2ResourceTest.java create mode 100644 app/common/src/test/resources/saml/dummy.txt 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 fb93ef345..ee893c575 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 @@ -197,7 +197,7 @@ public class ApplicationProperties { @JsonIgnore public InputStream getIdpMetadataUri() throws IOException { if (idpMetadataUri.startsWith("classpath:")) { - return new ClassPathResource(idpMetadataUri.substring("classpath".length())) + return new ClassPathResource(idpMetadataUri.substring("classpath:".length())) .getInputStream(); } try { @@ -233,6 +233,7 @@ public class ApplicationProperties { @JsonIgnore public Resource getPrivateKey() { + if (privateKey == null) return null; if (privateKey.startsWith("classpath:")) { return new ClassPathResource(privateKey.substring("classpath:".length())); } else { diff --git a/app/common/src/test/java/stirling/software/common/model/ApplicationPropertiesDynamicYamlPropertySourceTest.java b/app/common/src/test/java/stirling/software/common/model/ApplicationPropertiesDynamicYamlPropertySourceTest.java new file mode 100644 index 000000000..71d3997be --- /dev/null +++ b/app/common/src/test/java/stirling/software/common/model/ApplicationPropertiesDynamicYamlPropertySourceTest.java @@ -0,0 +1,59 @@ +package stirling.software.common.model; + +import static org.junit.jupiter.api.Assertions.*; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; + +import org.junit.jupiter.api.Test; +import org.mockito.MockedStatic; +import org.mockito.Mockito; +import org.springframework.core.env.ConfigurableEnvironment; +import org.springframework.core.env.StandardEnvironment; + +import stirling.software.common.configuration.InstallationPathConfig; + +class ApplicationPropertiesDynamicYamlPropertySourceTest { + + @Test + void loads_yaml_into_environment() throws Exception { + // YAML-Config in Temp-Datei schreiben + String yaml = + "" + + "ui:\n" + + " appName: \"My App\"\n" + + "system:\n" + + " enableAnalytics: true\n"; + Path tmp = Files.createTempFile("spdf-settings-", ".yml"); + Files.writeString(tmp, yaml); + + // Pfad per statischem Mock liefern + try (MockedStatic mocked = + Mockito.mockStatic(InstallationPathConfig.class)) { + mocked.when(InstallationPathConfig::getSettingsPath).thenReturn(tmp.toString()); + + ConfigurableEnvironment env = new StandardEnvironment(); + ApplicationProperties props = new ApplicationProperties(); + + props.dynamicYamlPropertySource(env); // fügt PropertySource an erster Stelle ein + + assertEquals("My App", env.getProperty("ui.appName")); + assertEquals("true", env.getProperty("system.enableAnalytics")); + } + } + + @Test + void throws_when_settings_file_missing() throws Exception { + String missing = "/path/does/not/exist/spdf.yml"; + try (MockedStatic mocked = + Mockito.mockStatic(InstallationPathConfig.class)) { + mocked.when(InstallationPathConfig::getSettingsPath).thenReturn(missing); + + ConfigurableEnvironment env = new StandardEnvironment(); + ApplicationProperties props = new ApplicationProperties(); + + assertThrows(IOException.class, () -> props.dynamicYamlPropertySource(env)); + } + } +} diff --git a/app/common/src/test/java/stirling/software/common/model/ApplicationPropertiesLogicTest.java b/app/common/src/test/java/stirling/software/common/model/ApplicationPropertiesLogicTest.java new file mode 100644 index 000000000..da83fd462 --- /dev/null +++ b/app/common/src/test/java/stirling/software/common/model/ApplicationPropertiesLogicTest.java @@ -0,0 +1,248 @@ +package stirling.software.common.model; + +import static org.junit.jupiter.api.Assertions.*; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; + +import org.junit.jupiter.api.Test; + +import stirling.software.common.model.ApplicationProperties.Driver; +import stirling.software.common.model.ApplicationProperties.Premium; +import stirling.software.common.model.ApplicationProperties.Security; +import stirling.software.common.model.exception.UnsupportedProviderException; + +class ApplicationPropertiesLogicTest { + + @Test + void system_isAnalyticsEnabled_null_false_true() { + ApplicationProperties.System sys = new ApplicationProperties.System(); + + sys.setEnableAnalytics(null); + assertFalse(sys.isAnalyticsEnabled()); + + sys.setEnableAnalytics(Boolean.FALSE); + assertFalse(sys.isAnalyticsEnabled()); + + sys.setEnableAnalytics(Boolean.TRUE); + assertTrue(sys.isAnalyticsEnabled()); + } + + @Test + void tempFileManagement_defaults_and_overrides() { + ApplicationProperties.TempFileManagement tfm = + new ApplicationProperties.TempFileManagement(); + + String expectedBase = + java.lang.System.getProperty("java.io.tmpdir").replaceAll("/+$", "") + + "/stirling-pdf"; + assertEquals(expectedBase, tfm.getBaseTmpDir()); + + String expectedLibre = expectedBase + "/libreoffice"; + assertEquals(expectedLibre, tfm.getLibreofficeDir()); + + tfm.setBaseTmpDir("/custom/base"); + assertEquals("/custom/base", tfm.getBaseTmpDir()); + + tfm.setLibreofficeDir("/opt/libre"); + assertEquals("/opt/libre", tfm.getLibreofficeDir()); + } + + @Test + void oauth2_scope_parsing_and_validity() { + Security.OAUTH2 oauth2 = new Security.OAUTH2(); + oauth2.setIssuer("https://issuer"); + oauth2.setClientId("client"); + oauth2.setClientSecret("secret"); + oauth2.setUseAsUsername("email"); + oauth2.setScopes("openid, profile ,email"); + assertTrue(oauth2.isSettingsValid()); + } + + @Test + void security_login_method_flags() { + Security sec = new Security(); + + sec.getOauth2().setEnabled(true); + sec.getSaml2().setEnabled(true); + + assertTrue(sec.isUserPass()); + assertTrue(sec.isOauth2Active()); + assertTrue(sec.isSaml2Active()); + + sec.setLoginMethod(Security.LoginMethods.NORMAL.toString()); + assertTrue(sec.isUserPass()); + assertFalse(sec.isOauth2Active()); + assertFalse(sec.isSaml2Active()); + } + + @Test + void security_isAltLogin_reflects_oauth2_or_saml2() { + Security sec = new Security(); + + assertFalse(sec.isAltLogin()); + + sec.getOauth2().setEnabled(true); + sec.getSaml2().setEnabled(false); + assertTrue(sec.isAltLogin()); + + sec.getOauth2().setEnabled(false); + sec.getSaml2().setEnabled(true); + assertTrue(sec.isAltLogin()); + + sec.getOauth2().setEnabled(true); + sec.getSaml2().setEnabled(true); + assertTrue(sec.isAltLogin()); + } + + @Test + void oauth2_client_provider_mapping_and_unsupported() throws UnsupportedProviderException { + Security.OAUTH2.Client client = new Security.OAUTH2.Client(); + + assertNotNull(client.get("google")); + assertNotNull(client.get("github")); + assertNotNull(client.get("keycloak")); + + UnsupportedProviderException ex = + assertThrows(UnsupportedProviderException.class, () -> client.get("unknown")); + assertTrue(ex.getMessage().toLowerCase().contains("not supported")); + } + + @Test + void premium_google_drive_getters_return_empty_string_on_null_or_blank() { + Premium.ProFeatures.GoogleDrive gd = new Premium.ProFeatures.GoogleDrive(); + + assertEquals("", gd.getClientId()); + assertEquals("", gd.getApiKey()); + assertEquals("", gd.getAppId()); + + gd.setClientId(" id "); + gd.setApiKey(" key "); + gd.setAppId(" app "); + assertEquals(" id ", gd.getClientId()); + assertEquals(" key ", gd.getApiKey()); + assertEquals(" app ", gd.getAppId()); + } + + @Test + void ui_getters_return_null_for_blank() { + ApplicationProperties.Ui ui = new ApplicationProperties.Ui(); + ui.setAppName(" "); + ui.setHomeDescription(""); + ui.setAppNameNavbar(null); + + assertNull(ui.getAppName()); + assertNull(ui.getHomeDescription()); + assertNull(ui.getAppNameNavbar()); + + ui.setAppName("Stirling-PDF"); + ui.setHomeDescription("Home"); + ui.setAppNameNavbar("Nav"); + assertEquals("Stirling-PDF", ui.getAppName()); + assertEquals("Home", ui.getHomeDescription()); + assertEquals("Nav", ui.getAppNameNavbar()); + } + + @Test + void driver_toString_contains_driver_name() { + assertTrue(Driver.H2.toString().contains("h2")); + assertTrue(Driver.POSTGRESQL.toString().contains("postgresql")); + } + + @Test + void session_limits_and_timeouts_have_reasonable_defaults() { + ApplicationProperties.ProcessExecutor pe = new ApplicationProperties.ProcessExecutor(); + + ApplicationProperties.ProcessExecutor.SessionLimit s = pe.getSessionLimit(); + assertEquals(2, s.getQpdfSessionLimit()); + assertEquals(1, s.getTesseractSessionLimit()); + assertEquals(1, s.getLibreOfficeSessionLimit()); + assertEquals(1, s.getPdfToHtmlSessionLimit()); + assertEquals(8, s.getPythonOpenCvSessionLimit()); + assertEquals(16, s.getWeasyPrintSessionLimit()); + assertEquals(1, s.getInstallAppSessionLimit()); + assertEquals(1, s.getCalibreSessionLimit()); + assertEquals(8, s.getGhostscriptSessionLimit()); + assertEquals(2, s.getOcrMyPdfSessionLimit()); + + ApplicationProperties.ProcessExecutor.TimeoutMinutes t = pe.getTimeoutMinutes(); + assertEquals(30, t.getTesseractTimeoutMinutes()); + assertEquals(30, t.getQpdfTimeoutMinutes()); + assertEquals(30, t.getLibreOfficeTimeoutMinutes()); + assertEquals(20, t.getPdfToHtmlTimeoutMinutes()); + assertEquals(30, t.getPythonOpenCvTimeoutMinutes()); + assertEquals(30, t.getWeasyPrintTimeoutMinutes()); + assertEquals(60, t.getInstallAppTimeoutMinutes()); + assertEquals(30, t.getCalibreTimeoutMinutes()); + assertEquals(30, t.getGhostscriptTimeoutMinutes()); + assertEquals(30, t.getOcrMyPdfTimeoutMinutes()); + } + + @Deprecated + @Test + void enterprise_metadata_defaults() { + ApplicationProperties.EnterpriseEdition ee = new ApplicationProperties.EnterpriseEdition(); + ApplicationProperties.EnterpriseEdition.CustomMetadata eMeta = ee.getCustomMetadata(); + eMeta.setCreator(" "); + eMeta.setProducer(null); + assertEquals("Stirling-PDF", eMeta.getCreator()); + assertEquals("Stirling-PDF", eMeta.getProducer()); + } + + @Test + void premium_metadata_defaults() { + Premium.ProFeatures pf = new Premium.ProFeatures(); + Premium.ProFeatures.CustomMetadata pMeta = pf.getCustomMetadata(); + pMeta.setCreator(""); + pMeta.setProducer(""); + assertEquals("Stirling-PDF", pMeta.getCreator()); + assertEquals("Stirling-PDF", pMeta.getProducer()); + } + + @Test + void premium_metadata_awesome() { + Premium.ProFeatures pf = new Premium.ProFeatures(); + Premium.ProFeatures.CustomMetadata pMeta = pf.getCustomMetadata(); + pMeta.setCreator("Awesome PDF Tool"); + pMeta.setProducer("Awesome PDF Tool"); + assertEquals("Awesome PDF Tool", pMeta.getCreator()); + assertEquals("Awesome PDF Tool", pMeta.getProducer()); + } + + @Test + void string_isValid_handles_null_empty_blank_and_trimmed() { + ApplicationProperties.Security.OAUTH2 oauth2 = new ApplicationProperties.Security.OAUTH2(); + + assertFalse(oauth2.isValid((String) null, "issuer")); + assertFalse(oauth2.isValid("", "issuer")); + assertFalse(oauth2.isValid(" ", "issuer")); + + assertTrue(oauth2.isValid("x", "issuer")); + assertTrue(oauth2.isValid(" x ", "issuer")); // trimmt intern + } + + @Test + void collection_isValid_handles_null_and_empty() { + ApplicationProperties.Security.OAUTH2 oauth2 = new ApplicationProperties.Security.OAUTH2(); + + Collection nullColl = null; + Collection empty = List.of(); + + assertFalse(oauth2.isValid(nullColl, "scopes")); + assertFalse(oauth2.isValid(empty, "scopes")); + } + + @Test + void collection_isValid_true_when_non_empty_even_if_element_is_blank() { + ApplicationProperties.Security.OAUTH2 oauth2 = new ApplicationProperties.Security.OAUTH2(); + + // Aktuelles Verhalten: prüft NUR !isEmpty(), nicht Inhalt + Collection oneBlank = new ArrayList<>(); + oneBlank.add(" "); + + assertTrue( + oauth2.isValid(oneBlank, "scopes"), + "Dokumentiert aktuelles Verhalten: nicht-leere Liste gilt als gültig, auch wenn Element leer/blank ist"); + } +} diff --git a/app/common/src/test/java/stirling/software/common/model/ApplicationPropertiesSaml2HttpTest.java b/app/common/src/test/java/stirling/software/common/model/ApplicationPropertiesSaml2HttpTest.java new file mode 100644 index 000000000..3fa8299ca --- /dev/null +++ b/app/common/src/test/java/stirling/software/common/model/ApplicationPropertiesSaml2HttpTest.java @@ -0,0 +1,80 @@ +package stirling.software.common.model; + +import static org.junit.jupiter.api.Assertions.*; + +import java.io.IOException; +import java.io.InputStream; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; + +import org.junit.jupiter.api.Test; +import org.springframework.core.io.FileSystemResource; +import org.springframework.core.io.Resource; + +import okhttp3.mockwebserver.MockResponse; +import okhttp3.mockwebserver.MockWebServer; + +class ApplicationPropertiesSaml2HttpTest { + + @Test + void idpMetadataUri_http_is_resolved_via_mockwebserver() throws Exception { + try (MockWebServer server = new MockWebServer()) { + server.enqueue( + new MockResponse() + .setResponseCode(200) + .addHeader("Content-Type", "application/xml") + .setBody("")); + server.start(); + + String url = server.url("/meta").toString(); + + var s = new ApplicationProperties.Security.SAML2(); + s.setIdpMetadataUri(url); + + try (InputStream in = s.getIdpMetadataUri()) { + String body = new String(in.readAllBytes(), StandardCharsets.UTF_8); + assertTrue(body.contains("EntityDescriptor")); + } + } + } + + @Test + void idpMetadataUri_invalidUri_triggers_catch_and_throwsIOException() { + // Ungültige URI -> new URI(...) wirft URISyntaxException -> catch -> IOException + var s = new ApplicationProperties.Security.SAML2(); + s.setIdpMetadataUri("http:##invalid uri"); // absichtlich kaputt (Leerzeichen + ##) + + assertThrows(IOException.class, s::getIdpMetadataUri); + } + + @Test + void spCert_else_branch_returns_FileSystemResource_for_filesystem_path() throws Exception { + var s = new ApplicationProperties.Security.SAML2(); + + // temporäre Datei simuliert "Filesystem"-Pfad (-> else-Zweig) + Path tmp = Files.createTempFile("spdf-spcert-", ".crt"); + Files.writeString(tmp, "CERT"); + + s.setSpCert(tmp.toString()); + Resource r = s.getSpCert(); + + assertNotNull(r); + assertTrue(r instanceof FileSystemResource, "Expected FileSystemResource for FS path"); + assertTrue(r.exists(), "Temp file should exist"); + } + + @Test + void idpCert_else_branch_returns_FileSystemResource_even_if_missing() { + var s = new ApplicationProperties.Security.SAML2(); + + // bewusst nicht existierender Pfad -> else-Zweig wird trotzdem genommen + String missing = "/this/path/does/not/exist/idp.crt"; + s.setIdpCert(missing); + Resource r = s.getIdpCert(); + + assertNotNull(r); + assertTrue(r instanceof FileSystemResource, "Expected FileSystemResource for FS path"); + assertFalse(r.exists(), "Resource should not exist for missing file"); + } +} diff --git a/app/common/src/test/java/stirling/software/common/model/ApplicationPropertiesSaml2ResourceTest.java b/app/common/src/test/java/stirling/software/common/model/ApplicationPropertiesSaml2ResourceTest.java new file mode 100644 index 000000000..efc266561 --- /dev/null +++ b/app/common/src/test/java/stirling/software/common/model/ApplicationPropertiesSaml2ResourceTest.java @@ -0,0 +1,55 @@ +package stirling.software.common.model; + +import static org.junit.jupiter.api.Assertions.*; + +import java.io.InputStream; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; + +import org.junit.jupiter.api.Test; +import org.springframework.core.io.Resource; + +class ApplicationPropertiesSaml2ResourceTest { + + @Test + void idpMetadataUri_classpath_is_resolved() throws Exception { + var s = new ApplicationProperties.Security.SAML2(); + s.setIdpMetadataUri("classpath:saml/dummy.txt"); + + try (InputStream in = s.getIdpMetadataUri()) { + assertNotNull(in, "Classpath InputStream should not be null"); + String txt = new String(in.readAllBytes(), StandardCharsets.UTF_8); + assertTrue(txt.contains("ok")); + } + } + + @Test + void spCert_idpCert_privateKey_null_classpath_and_filesystem() throws Exception { + var s = new ApplicationProperties.Security.SAML2(); + + s.setSpCert(null); + s.setIdpCert(null); + s.setPrivateKey(null); + assertNull(s.getSpCert()); + assertNull(s.getIdpCert()); + assertNull(s.getPrivateKey()); + + s.setSpCert("classpath:saml/dummy.txt"); + s.setIdpCert("classpath:saml/dummy.txt"); + s.setPrivateKey("classpath:saml/dummy.txt"); + Resource sp = s.getSpCert(); + Resource idp = s.getIdpCert(); + Resource pk = s.getPrivateKey(); + assertTrue(sp.exists()); + assertTrue(idp.exists()); + assertTrue(pk.exists()); + + Path tmp = Files.createTempFile("spdf-key-", ".pem"); + Files.writeString(tmp, "KEY"); + s.setPrivateKey(tmp.toString()); + Resource pkFs = s.getPrivateKey(); + assertNotNull(pkFs); + assertTrue(pkFs.exists()); + } +} diff --git a/app/common/src/test/resources/saml/dummy.txt b/app/common/src/test/resources/saml/dummy.txt new file mode 100644 index 000000000..9766475a4 --- /dev/null +++ b/app/common/src/test/resources/saml/dummy.txt @@ -0,0 +1 @@ +ok diff --git a/build.gradle b/build.gradle index e54c58e7d..2c151d11b 100644 --- a/build.gradle +++ b/build.gradle @@ -69,7 +69,7 @@ allprojects { tasks.register('writeVersion', WriteProperties) { outputFile = layout.projectDirectory.file('app/common/src/main/resources/version.properties') println "Writing version.properties to ${outputFile.path}" - comment "${new Date()}" + comment = "${new Date()}" property 'version', project.provider { project.version.toString() } } @@ -128,6 +128,9 @@ subprojects { testImplementation 'org.springframework.boot:spring-boot-starter-test' testRuntimeOnly 'org.mockito:mockito-inline:5.2.0' testRuntimeOnly "org.junit.platform:junit-platform-launcher:$junitPlatformVersion" + + testImplementation platform("com.squareup.okhttp3:okhttp-bom:5.1.0") + testImplementation "com.squareup.okhttp3:mockwebserver" } tasks.withType(JavaCompile).configureEach { @@ -153,6 +156,17 @@ subprojects { } } + jacocoTestCoverageVerification { + dependsOn jacocoTestReport + violationRules { + rule { + limit { + minimum = 0.0 + } + } + } + } + tasks.named("processResources") { dependsOn(rootProject.tasks.writeVersion) } @@ -569,6 +583,9 @@ dependencies { testImplementation 'org.springframework.boot:spring-boot-starter-test' testRuntimeOnly "org.junit.platform:junit-platform-launcher:$junitPlatformVersion" + + testImplementation platform("com.squareup.okhttp3:okhttp-bom:5.1.0") + testImplementation "com.squareup.okhttp3:mockwebserver" } tasks.named("test") {