diff --git a/plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/api/command/SAML2LoginAPIAuthenticatorCmd.java b/plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/api/command/SAML2LoginAPIAuthenticatorCmd.java index 6bb3e788a95..332e0602784 100644 --- a/plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/api/command/SAML2LoginAPIAuthenticatorCmd.java +++ b/plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/api/command/SAML2LoginAPIAuthenticatorCmd.java @@ -144,6 +144,14 @@ public class SAML2LoginAPIAuthenticatorCmd extends BaseCmd implements APIAuthent return responseObject; } + protected void checkAndFailOnMissingSAMLSignature(Signature signature) { + if (signature == null && SAML2AuthManager.SAMLCheckSignature.value()) { + s_logger.error("Failing SAML login due to missing signature in the SAML response and signature check is enforced. " + + "Please check and ensure the IDP configuration has signing certificate or relax the saml2.check.signature setting."); + throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, "Signature is missing from the SAML Response. Please contact the Administrator"); + } + } + @Override public String authenticate(final String command, final Map params, final HttpSession session, final InetAddress remoteAddress, final String responseType, final StringBuilder auditTrailSb, final HttpServletRequest req, final HttpServletResponse resp) throws ServerApiException { try { @@ -220,11 +228,13 @@ public class SAML2LoginAPIAuthenticatorCmd extends BaseCmd implements APIAuthent "Received SAML response for a SSO request that we may not have made or has expired, please try logging in again", params, responseType)); } + samlAuthManager.purgeToken(token); // Set IdpId for this session session.setAttribute(SAMLPluginConstants.SAML_IDPID, issuer.getValue()); Signature sig = processedSAMLResponse.getSignature(); + checkAndFailOnMissingSAMLSignature(sig); if (idpMetadata.getSigningCertificate() != null && sig != null) { BasicX509Credential credential = new BasicX509Credential(); credential.setEntityCertificate(idpMetadata.getSigningCertificate()); @@ -238,9 +248,8 @@ public class SAML2LoginAPIAuthenticatorCmd extends BaseCmd implements APIAuthent params, responseType)); } } - if (username == null) { - username = SAMLUtils.getValueFromAssertions(processedSAMLResponse.getAssertions(), SAML2AuthManager.SAMLUserAttributeName.value()); - } + + username = SAMLUtils.getValueFromAssertions(processedSAMLResponse.getAssertions(), SAML2AuthManager.SAMLUserAttributeName.value()); for (Assertion assertion: processedSAMLResponse.getAssertions()) { if (assertion!= null && assertion.getSubject() != null && assertion.getSubject().getNameID() != null) { @@ -272,6 +281,7 @@ public class SAML2LoginAPIAuthenticatorCmd extends BaseCmd implements APIAuthent continue; } Signature encSig = assertion.getSignature(); + checkAndFailOnMissingSAMLSignature(encSig); if (idpMetadata.getSigningCertificate() != null && encSig != null) { BasicX509Credential sigCredential = new BasicX509Credential(); sigCredential.setEntityCertificate(idpMetadata.getSigningCertificate()); diff --git a/plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/saml/SAML2AuthManager.java b/plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/saml/SAML2AuthManager.java index e52a7e32695..3a4030f9c0d 100644 --- a/plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/saml/SAML2AuthManager.java +++ b/plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/saml/SAML2AuthManager.java @@ -25,59 +25,63 @@ import java.util.Collection; public interface SAML2AuthManager extends PluggableAPIAuthenticator, PluggableService { - public static final ConfigKey SAMLIsPluginEnabled = new ConfigKey("Advanced", Boolean.class, "saml2.enabled", "false", + ConfigKey SAMLIsPluginEnabled = new ConfigKey("Advanced", Boolean.class, "saml2.enabled", "false", "Indicates whether SAML SSO plugin is enabled or not", true); - public static final ConfigKey SAMLServiceProviderID = new ConfigKey("Advanced", String.class, "saml2.sp.id", "org.apache.cloudstack", + ConfigKey SAMLServiceProviderID = new ConfigKey("Advanced", String.class, "saml2.sp.id", "org.apache.cloudstack", "SAML2 Service Provider Identifier String", true); - public static final ConfigKey SAMLServiceProviderContactPersonName = new ConfigKey("Advanced", String.class, "saml2.sp.contact.person", "CloudStack Developers", + ConfigKey SAMLServiceProviderContactPersonName = new ConfigKey("Advanced", String.class, "saml2.sp.contact.person", "CloudStack Developers", "SAML2 Service Provider Contact Person Name", true); - public static final ConfigKey SAMLServiceProviderContactEmail = new ConfigKey("Advanced", String.class, "saml2.sp.contact.email", "dev@cloudstack.apache.org", + ConfigKey SAMLServiceProviderContactEmail = new ConfigKey("Advanced", String.class, "saml2.sp.contact.email", "dev@cloudstack.apache.org", "SAML2 Service Provider Contact Email Address", true); - public static final ConfigKey SAMLServiceProviderOrgName = new ConfigKey("Advanced", String.class, "saml2.sp.org.name", "Apache CloudStack", + ConfigKey SAMLServiceProviderOrgName = new ConfigKey("Advanced", String.class, "saml2.sp.org.name", "Apache CloudStack", "SAML2 Service Provider Organization Name", true); - public static final ConfigKey SAMLServiceProviderOrgUrl = new ConfigKey("Advanced", String.class, "saml2.sp.org.url", "http://cloudstack.apache.org", + ConfigKey SAMLServiceProviderOrgUrl = new ConfigKey("Advanced", String.class, "saml2.sp.org.url", "http://cloudstack.apache.org", "SAML2 Service Provider Organization URL", true); - public static final ConfigKey SAMLServiceProviderSingleSignOnURL = new ConfigKey("Advanced", String.class, "saml2.sp.sso.url", "http://localhost:8080/client/api?command=samlSso", + ConfigKey SAMLServiceProviderSingleSignOnURL = new ConfigKey("Advanced", String.class, "saml2.sp.sso.url", "http://localhost:8080/client/api?command=samlSso", "SAML2 CloudStack Service Provider Single Sign On URL", true); - public static final ConfigKey SAMLServiceProviderSingleLogOutURL = new ConfigKey("Advanced", String.class, "saml2.sp.slo.url", "http://localhost:8080/client/", + ConfigKey SAMLServiceProviderSingleLogOutURL = new ConfigKey("Advanced", String.class, "saml2.sp.slo.url", "http://localhost:8080/client/", "SAML2 CloudStack Service Provider Single Log Out URL", true); - public static final ConfigKey SAMLCloudStackRedirectionUrl = new ConfigKey("Advanced", String.class, "saml2.redirect.url", "http://localhost:8080/client", + ConfigKey SAMLCloudStackRedirectionUrl = new ConfigKey("Advanced", String.class, "saml2.redirect.url", "http://localhost:8080/client", "The CloudStack UI url the SSO should redirected to when successful", true); - public static final ConfigKey SAMLUserAttributeName = new ConfigKey("Advanced", String.class, "saml2.user.attribute", "uid", + ConfigKey SAMLUserAttributeName = new ConfigKey("Advanced", String.class, "saml2.user.attribute", "uid", "Attribute name to be looked for in SAML response that will contain the username", true); - public static final ConfigKey SAMLIdentityProviderMetadataURL = new ConfigKey("Advanced", String.class, "saml2.idp.metadata.url", "https://openidp.feide.no/simplesaml/saml2/idp/metadata.php", + ConfigKey SAMLIdentityProviderMetadataURL = new ConfigKey("Advanced", String.class, "saml2.idp.metadata.url", "https://openidp.feide.no/simplesaml/saml2/idp/metadata.php", "SAML2 Identity Provider Metadata XML Url", true); - public static final ConfigKey SAMLDefaultIdentityProviderId = new ConfigKey("Advanced", String.class, "saml2.default.idpid", "https://openidp.feide.no", + ConfigKey SAMLDefaultIdentityProviderId = new ConfigKey("Advanced", String.class, "saml2.default.idpid", "https://openidp.feide.no", "The default IdP entity ID to use only in case of multiple IdPs", true); - public static final ConfigKey SAMLSignatureAlgorithm = new ConfigKey<>(String.class, "saml2.sigalg", "Advanced", "SHA1", + ConfigKey SAMLSignatureAlgorithm = new ConfigKey<>(String.class, "saml2.sigalg", "Advanced", "SHA1", "The algorithm to use to when signing a SAML request. Default is SHA1, allowed algorithms: SHA1, SHA256, SHA384, SHA512", true, ConfigKey.Scope.Global, null, null, null, null, null, ConfigKey.Kind.Select, "SHA1,SHA256,SHA384,SHA512"); - public static final ConfigKey SAMLAppendDomainSuffix = new ConfigKey("Advanced", Boolean.class, "saml2.append.idpdomain", "false", + ConfigKey SAMLAppendDomainSuffix = new ConfigKey("Advanced", Boolean.class, "saml2.append.idpdomain", "false", "If enabled, create account/user dialog with SAML SSO enabled will append the IdP domain to the user or account name in the UI dialog", true); - public static final ConfigKey SAMLTimeout = new ConfigKey("Advanced", Integer.class, "saml2.timeout", "1800", + ConfigKey SAMLTimeout = new ConfigKey("Advanced", Integer.class, "saml2.timeout", "1800", "SAML2 IDP Metadata refresh interval in seconds, minimum value is set to 300", true); - public SAMLProviderMetadata getSPMetadata(); - public SAMLProviderMetadata getIdPMetadata(String entityId); - public Collection getAllIdPMetadata(); + ConfigKey SAMLCheckSignature = new ConfigKey("Advanced", Boolean.class, "saml2.check.signature", "true", + "When enabled (default and recommended), SAML2 signature checks are enforced and lack of signature in the SAML SSO response will cause login exception. Disabling this is not advisable but provided for backward compatibility for users who are able to accept the risks.", false); - public boolean isUserAuthorized(Long userId, String entityId); - public boolean authorizeUser(Long userId, String entityId, boolean enable); + SAMLProviderMetadata getSPMetadata(); + SAMLProviderMetadata getIdPMetadata(String entityId); + Collection getAllIdPMetadata(); - public void saveToken(String authnId, String domain, String entity); - public SAMLTokenVO getToken(String authnId); - public void expireTokens(); + boolean isUserAuthorized(Long userId, String entityId); + boolean authorizeUser(Long userId, String entityId, boolean enable); + + void saveToken(String authnId, String domain, String entity); + SAMLTokenVO getToken(String authnId); + void purgeToken(SAMLTokenVO token); + void expireTokens(); } diff --git a/plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/saml/SAML2AuthManagerImpl.java b/plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/saml/SAML2AuthManagerImpl.java index ba85b151eea..dfa76414fb7 100644 --- a/plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/saml/SAML2AuthManagerImpl.java +++ b/plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/saml/SAML2AuthManagerImpl.java @@ -487,6 +487,13 @@ public class SAML2AuthManagerImpl extends AdapterBase implements SAML2AuthManage return _samlTokenDao.findByUuid(authnId); } + @Override + public void purgeToken(SAMLTokenVO token) { + if (token != null) { + _samlTokenDao.remove(token.getId()); + } + } + @Override public void expireTokens() { _samlTokenDao.expireTokens(); @@ -535,6 +542,6 @@ public class SAML2AuthManagerImpl extends AdapterBase implements SAML2AuthManage SAMLServiceProviderSingleSignOnURL, SAMLServiceProviderSingleLogOutURL, SAMLCloudStackRedirectionUrl, SAMLUserAttributeName, SAMLIdentityProviderMetadataURL, SAMLDefaultIdentityProviderId, - SAMLSignatureAlgorithm, SAMLAppendDomainSuffix, SAMLTimeout}; + SAMLSignatureAlgorithm, SAMLAppendDomainSuffix, SAMLTimeout, SAMLCheckSignature}; } } diff --git a/plugins/user-authenticators/saml2/src/test/java/org/apache/cloudstack/api/command/SAML2LoginAPIAuthenticatorCmdTest.java b/plugins/user-authenticators/saml2/src/test/java/org/apache/cloudstack/api/command/SAML2LoginAPIAuthenticatorCmdTest.java index 39c8c231bf0..48a3139052d 100644 --- a/plugins/user-authenticators/saml2/src/test/java/org/apache/cloudstack/api/command/SAML2LoginAPIAuthenticatorCmdTest.java +++ b/plugins/user-authenticators/saml2/src/test/java/org/apache/cloudstack/api/command/SAML2LoginAPIAuthenticatorCmdTest.java @@ -271,6 +271,30 @@ public class SAML2LoginAPIAuthenticatorCmdTest { verifyTestWhenFailToAuthenticateThrowExceptionOrRedirectToUrl(false, hasThrownServerApiException, 0, 0); } + private void overrideDefaultConfigValue(final ConfigKey configKey, final String name, final Object o) throws IllegalAccessException, NoSuchFieldException { + Field f = ConfigKey.class.getDeclaredField(name); + f.setAccessible(true); + f.set(configKey, o); + } + + @Test + public void testFailOnSAMLSignatureCheckWhenFalse() throws NoSuchFieldException, IllegalAccessException { + overrideDefaultConfigValue(SAML2AuthManager.SAMLCheckSignature, "_value", false); + SAML2LoginAPIAuthenticatorCmd cmd = new SAML2LoginAPIAuthenticatorCmd(); + try { + cmd.checkAndFailOnMissingSAMLSignature(null); + } catch(Exception e) { + Assert.fail("This shouldn't throw any exception"); + } + } + + @Test(expected = ServerApiException.class) + public void testFailOnSAMLSignatureCheckWhenTrue() throws NoSuchFieldException, IllegalAccessException { + overrideDefaultConfigValue(SAML2AuthManager.SAMLCheckSignature, "_value", true); + SAML2LoginAPIAuthenticatorCmd cmd = new SAML2LoginAPIAuthenticatorCmd(); + cmd.checkAndFailOnMissingSAMLSignature(null); + } + private UserAccountVO configureTestWhenFailToAuthenticateThrowExceptionOrRedirectToUrl(String entity, String configurationValue, Boolean isUserAuthorized) throws IOException { Mockito.when(samlAuthManager.isUserAuthorized(nullable(Long.class), nullable(String.class))).thenReturn(isUserAuthorized);