From 4c3977caab5cc82d8f40fbbfb35c689af346d6ff Mon Sep 17 00:00:00 2001 From: Zipeng WU Date: Thu, 14 Jul 2022 10:58:55 +0200 Subject: [PATCH] SONAR-16492 Log clear message when private key is not in PKCS8 format --- .../sonar/auth/saml/SamlIdentityProvider.java | 10 ++++++---- .../org/sonar/auth/saml/SamlSettings.java | 2 +- .../auth/saml/SamlIdentityProviderTest.java | 19 +++++++++++++++++++ 3 files changed, 26 insertions(+), 5 deletions(-) diff --git a/server/sonar-auth-saml/src/main/java/org/sonar/auth/saml/SamlIdentityProvider.java b/server/sonar-auth-saml/src/main/java/org/sonar/auth/saml/SamlIdentityProvider.java index 1bdc071fe54..1a8751e9152 100644 --- a/server/sonar-auth-saml/src/main/java/org/sonar/auth/saml/SamlIdentityProvider.java +++ b/server/sonar-auth-saml/src/main/java/org/sonar/auth/saml/SamlIdentityProvider.java @@ -209,10 +209,12 @@ public class SamlIdentityProvider implements OAuth2IdentityProvider { // During callback, the callback URL is by definition not needed, but the Saml2Settings does never allow this setting to be empty... samlData.put("onelogin.saml2.sp.assertion_consumer_service.url", callbackUrl != null ? callbackUrl : ANY_URL); - SettingsBuilder builder = new SettingsBuilder(); - return builder - .fromValues(samlData) - .build(); + + var saml2Settings = new SettingsBuilder().fromValues(samlData).build(); + if (samlSettings.getServiceProviderPrivateKey().isPresent() && saml2Settings.getSPkey() == null) { + LOGGER.error("Error in parsing service provider private key, please make sure that it is in PKCS 8 format."); + } + return saml2Settings; } private static HttpServletRequest useProxyHeadersInRequest(HttpServletRequest request) { diff --git a/server/sonar-auth-saml/src/main/java/org/sonar/auth/saml/SamlSettings.java b/server/sonar-auth-saml/src/main/java/org/sonar/auth/saml/SamlSettings.java index 3ce35fc7376..e654958327a 100644 --- a/server/sonar-auth-saml/src/main/java/org/sonar/auth/saml/SamlSettings.java +++ b/server/sonar-auth-saml/src/main/java/org/sonar/auth/saml/SamlSettings.java @@ -214,7 +214,7 @@ public class SamlSettings { .build(), PropertyDefinition.builder(SERVICE_PROVIDER_CERTIFICATE) .name("Service provider certificate") - .description("X.509 certificate for the service provider.") + .description("X.509 certificate for the service provider, used for signing the requests.") .category(CATEGORY) .subCategory(SUBCATEGORY) .type(PASSWORD) diff --git a/server/sonar-auth-saml/src/test/java/org/sonar/auth/saml/SamlIdentityProviderTest.java b/server/sonar-auth-saml/src/test/java/org/sonar/auth/saml/SamlIdentityProviderTest.java index df41b12abf3..76ebc901b26 100644 --- a/server/sonar-auth-saml/src/test/java/org/sonar/auth/saml/SamlIdentityProviderTest.java +++ b/server/sonar-auth-saml/src/test/java/org/sonar/auth/saml/SamlIdentityProviderTest.java @@ -38,6 +38,8 @@ import org.sonar.api.server.authentication.OAuth2IdentityProvider; import org.sonar.api.server.authentication.UnauthorizedException; import org.sonar.api.server.authentication.UserIdentity; import org.sonar.api.utils.System2; +import org.sonar.api.utils.log.LogAndArguments; +import org.sonar.api.utils.log.LogTester; import org.sonar.db.DbTester; import static org.assertj.core.api.Assertions.assertThat; @@ -47,6 +49,7 @@ import static org.mockito.ArgumentMatchers.argThat; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import static org.sonar.api.utils.log.LoggerLevel.ERROR; public class SamlIdentityProviderTest { private static final String SQ_CALLBACK_URL = "http://localhost:9000/oauth2/callback/saml"; @@ -62,6 +65,8 @@ public class SamlIdentityProviderTest { @Rule public DbTester db = DbTester.create(); + @Rule + public LogTester log = new LogTester(); private final MapSettings settings = new MapSettings(new PropertyDefinitions(System2.INSTANCE, SamlSettings.definitions())); private final SamlIdentityProvider underTest = new SamlIdentityProvider(new SamlSettings(settings.asConfig()), new SamlMessageIdChecker(db.getDbClient())); @@ -218,6 +223,20 @@ public class SamlIdentityProviderTest { assertThat(callbackContext.userIdentity.getGroups()).isEmpty(); } + @Test + public void log_clear_error_when_private_key_is_not_pkcs8() { + var WRONG_FORMAT_PRIVATE_KEY = "MIIEpAIBAAKCAQEA0haE9+9QtP5JWbj4LymxiLZJk2n+QsHjVy/Lt/PXffGjl0aQ0O5mk13Vf1vlXC/L1FoPMhup5/AkcMHmJxkzXZ/VAHYYJJ78UHt6atxMDicpbOBaYqumE4fg0H4mVEIs4xYwzq/xhuTtpTb00ZCOF2Y/151o0alWcLbYgObZtbCpLxncjkJY8J+CY2v1WyE3VfBgErpseugYpf8GpkrKiM5tkY7DjhkrH+VY2FD8A6G6bkn/o+Ay7Vo3pv/byMyNsHBN9LgvCwQIDtyQJSEvTUK3IY0vcYTCUaITqIrWkj5qrDsuw9gR9Ie11+kzYL/1mE5AWOHmT2qNz7C9gOgJoQIDAQABAoIBAQCY9dRyQEfev5XgQZBRpmWgSDhhoDaDnG9Nt3r3wA4RoLGfHr2poSoF+bfMNrhT2mjpf3i43vNh77JYdpR/uxVvAUQwRctmPmsunfiPfT3SwCilIOQuGxOb/L5ujqqRhmzwGeQHWIrd0ChGtjChtEIAP24UKoN6w3QwNLCFiY7RfTC7+yyO05tKoIhzirCNDBARZPmaIm4ClmIf3Z3xg0Vsgtz7qntd63UoXjSWFA0f9EDJHfxCTaS1r9OrpPsPMNpVOEYDek4le+mWVCBmQABxYDBCycrILQUwpDkGOa6D7tezpjGYyn8Z4HiHzcneNqt/0+g5lG28DWHz9h0TKIjBAoGBAOklXNZmeMpMrQvTOta2cE/ts17SSrnUFWtKqU7ZH9Gs50ZpwzrnNWy+3sCiCCKfa+RylbXjukioKR5qz/qpr28GdD8+dWYNDHraEpk/ZtOfff7TlNpOjiNPXb0OF2t3HDeQs5etUPx5DHgCA58vDK4RlQcIWpZROCeH5vzo7lStAoGBAOauiFYKmbX3FdWsyBerjhbem5X59eNs68KtHxd/Y6INn/uI12gSsOi+Gj9B7yC/N1AKxqaWGN9fPeGy2+mC+BI0tiTWwATNlZyaSXeKBqKThONhgmZWUaX++dczqbWADdtzRUroy5X2lHzG8q6iG0RQtgwnczU1OdBk+UgF1/NFAoGBAJcr0Lx8CQozGWk3d0lNVhmdaNasyCMh7xl4ebtUcZtE31j6rsn8rNlsEYcaCOhaMl0YJxafKGSAFNlSLLS9XbFBoBJ57ylSgKsPx0tynrvNCKc4jaXXlbYzefZhsrHNs5Ab1Tcd/AsYegs+UxbeLPyZDeZXdlVNKHoJVq7aYd6pAoGAC7M2fwaynSQXG2tUCr9MyaQoyAaRjiNsIceeGBcB+qouPxfFtSWdi3B47FRvyH1qVMj3ImPihxHRlaz4snNOGb5KrrulqZizyemZaFK722sYBmBfuMkQAxdXnK6mIOqJyWOjVBVSnhyPk3STwn++WkytrxghI8W7VPKKIjkJpvECgYBBnvWXa8Ez/azEN+Y2Lc1PnU9OpNa/QRPUPApq15dB8Cu9e2Vm6F+CdGKBY8WQvDw7DJd6eOjCfN6ymy1O9vLiooNQJGaO/znncU1r3s42dfpQ8owthILl24GNXnEgth2yYfYPr/EVLoVsbgO0WKFvdsVbSo/upeLzGnVT+DMqfA=="; + setSettings(true); + settings.setProperty("sonar.auth.saml.sp.privateKey.secured", WRONG_FORMAT_PRIVATE_KEY); + DumbCallbackContext callbackContext = new DumbCallbackContext(request, response, "encoded_minimal_response.txt", SQ_CALLBACK_URL); + + underTest.callback(callbackContext); + + assertThat(log.getLogs(ERROR)) + .extracting(LogAndArguments::getFormattedMsg) + .contains("Error in parsing service provider private key, please make sure that it is in PKCS 8 format."); + } + @Test public void callback_does_not_sync_group_when_group_setting_is_not_set() { setSettings(true); -- 2.39.5