Skip to content

Commit 2cfb541

Browse files
authored
saml: purge token after first response and improve setting description (#9377)
* saml: purge token after first response and improve setting description This improves the description of a saml signature checking global setting, and purges the SAML token upon handling the first SAML response. Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com> * fix failing unit test Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com> --------- Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>
1 parent 50586a9 commit 2cfb541

File tree

4 files changed

+20
-11
lines changed

4 files changed

+20
-11
lines changed

plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/api/command/SAML2LoginAPIAuthenticatorCmd.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,7 @@ public String authenticate(final String command, final Map<String, Object[]> par
228228
"Received SAML response for a SSO request that we may not have made or has expired, please try logging in again",
229229
params, responseType));
230230
}
231+
samlAuthManager.purgeToken(token);
231232

232233
// Set IdpId for this session
233234
session.setAttribute(SAMLPluginConstants.SAML_IDPID, issuer.getValue());

plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/saml/SAML2AuthManager.java

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -71,16 +71,17 @@ public interface SAML2AuthManager extends PluggableAPIAuthenticator, PluggableSe
7171
"SAML2 IDP Metadata refresh interval in seconds, minimum value is set to 300", true);
7272

7373
ConfigKey<Boolean> SAMLCheckSignature = new ConfigKey<Boolean>("Advanced", Boolean.class, "saml2.check.signature", "true",
74-
"Whether SAML2 signature must be checked, when enforced and when the SAML response does not have a signature would lead to login exception", true);
74+
"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);
7575

76-
public SAMLProviderMetadata getSPMetadata();
77-
public SAMLProviderMetadata getIdPMetadata(String entityId);
78-
public Collection<SAMLProviderMetadata> getAllIdPMetadata();
76+
SAMLProviderMetadata getSPMetadata();
77+
SAMLProviderMetadata getIdPMetadata(String entityId);
78+
Collection<SAMLProviderMetadata> getAllIdPMetadata();
7979

80-
public boolean isUserAuthorized(Long userId, String entityId);
81-
public boolean authorizeUser(Long userId, String entityId, boolean enable);
80+
boolean isUserAuthorized(Long userId, String entityId);
81+
boolean authorizeUser(Long userId, String entityId, boolean enable);
8282

83-
public void saveToken(String authnId, String domain, String entity);
84-
public SAMLTokenVO getToken(String authnId);
85-
public void expireTokens();
83+
void saveToken(String authnId, String domain, String entity);
84+
SAMLTokenVO getToken(String authnId);
85+
void purgeToken(SAMLTokenVO token);
86+
void expireTokens();
8687
}

plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/saml/SAML2AuthManagerImpl.java

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -487,6 +487,13 @@ public SAMLTokenVO getToken(String authnId) {
487487
return _samlTokenDao.findByUuid(authnId);
488488
}
489489

490+
@Override
491+
public void purgeToken(SAMLTokenVO token) {
492+
if (token != null) {
493+
_samlTokenDao.remove(token.getId());
494+
}
495+
}
496+
490497
@Override
491498
public void expireTokens() {
492499
_samlTokenDao.expireTokens();

plugins/user-authenticators/saml2/src/test/java/org/apache/cloudstack/api/command/SAML2LoginAPIAuthenticatorCmdTest.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -279,7 +279,7 @@ private void overrideDefaultConfigValue(final ConfigKey configKey, final String
279279

280280
@Test
281281
public void testFailOnSAMLSignatureCheckWhenFalse() throws NoSuchFieldException, IllegalAccessException {
282-
overrideDefaultConfigValue(SAML2AuthManager.SAMLCheckSignature, "_defaultValue", "false");
282+
overrideDefaultConfigValue(SAML2AuthManager.SAMLCheckSignature, "_value", false);
283283
SAML2LoginAPIAuthenticatorCmd cmd = new SAML2LoginAPIAuthenticatorCmd();
284284
try {
285285
cmd.checkAndFailOnMissingSAMLSignature(null);
@@ -290,7 +290,7 @@ public void testFailOnSAMLSignatureCheckWhenFalse() throws NoSuchFieldException,
290290

291291
@Test(expected = ServerApiException.class)
292292
public void testFailOnSAMLSignatureCheckWhenTrue() throws NoSuchFieldException, IllegalAccessException {
293-
overrideDefaultConfigValue(SAML2AuthManager.SAMLCheckSignature, "_defaultValue", "true");
293+
overrideDefaultConfigValue(SAML2AuthManager.SAMLCheckSignature, "_value", true);
294294
SAML2LoginAPIAuthenticatorCmd cmd = new SAML2LoginAPIAuthenticatorCmd();
295295
cmd.checkAndFailOnMissingSAMLSignature(null);
296296
}

0 commit comments

Comments
 (0)