This is an automated email from the ASF dual-hosted git repository.

rohit pushed a commit to branch 4.19
in repository https://gitbox.apache.org/repos/asf/cloudstack.git


The following commit(s) were added to refs/heads/4.19 by this push:
     new 2cfb541a1d7 saml: purge token after first response and improve setting 
description (#9377)
2cfb541a1d7 is described below

commit 2cfb541a1d7875d1beb25b2199303683d472ddcd
Author: Rohit Yadav <rohit.ya...@shapeblue.com>
AuthorDate: Mon Jul 15 09:45:28 2024 +0530

    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.ya...@shapeblue.com>
    
    * fix failing unit test
    
    Signed-off-by: Rohit Yadav <rohit.ya...@shapeblue.com>
    
    ---------
    
    Signed-off-by: Rohit Yadav <rohit.ya...@shapeblue.com>
---
 .../api/command/SAML2LoginAPIAuthenticatorCmd.java    |  1 +
 .../org/apache/cloudstack/saml/SAML2AuthManager.java  | 19 ++++++++++---------
 .../apache/cloudstack/saml/SAML2AuthManagerImpl.java  |  7 +++++++
 .../command/SAML2LoginAPIAuthenticatorCmdTest.java    |  4 ++--
 4 files changed, 20 insertions(+), 11 deletions(-)

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 fd4da7a59b2..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
@@ -228,6 +228,7 @@ 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());
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 27f17cee6b1..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
@@ -71,16 +71,17 @@ public interface SAML2AuthManager extends 
PluggableAPIAuthenticator, PluggableSe
             "SAML2 IDP Metadata refresh interval in seconds, minimum value is 
set to 300", true);
 
     ConfigKey<Boolean> SAMLCheckSignature = new ConfigKey<Boolean>("Advanced", 
Boolean.class, "saml2.check.signature", "true",
-            "Whether SAML2 signature must be checked, when enforced and when 
the SAML response does not have a signature would lead to login exception", 
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 SAMLProviderMetadata getSPMetadata();
-    public SAMLProviderMetadata getIdPMetadata(String entityId);
-    public Collection<SAMLProviderMetadata> getAllIdPMetadata();
+    SAMLProviderMetadata getSPMetadata();
+    SAMLProviderMetadata getIdPMetadata(String entityId);
+    Collection<SAMLProviderMetadata> getAllIdPMetadata();
 
-    public boolean isUserAuthorized(Long userId, String entityId);
-    public boolean authorizeUser(Long userId, String entityId, boolean enable);
+    boolean isUserAuthorized(Long userId, String entityId);
+    boolean authorizeUser(Long userId, String entityId, boolean enable);
 
-    public void saveToken(String authnId, String domain, String entity);
-    public SAMLTokenVO getToken(String authnId);
-    public void expireTokens();
+    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 3ecebf4d185..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();
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 ebdc5be8fe9..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
@@ -279,7 +279,7 @@ public class SAML2LoginAPIAuthenticatorCmdTest {
 
     @Test
     public void testFailOnSAMLSignatureCheckWhenFalse() throws 
NoSuchFieldException, IllegalAccessException {
-        overrideDefaultConfigValue(SAML2AuthManager.SAMLCheckSignature, 
"_defaultValue", "false");
+        overrideDefaultConfigValue(SAML2AuthManager.SAMLCheckSignature, 
"_value", false);
         SAML2LoginAPIAuthenticatorCmd cmd = new 
SAML2LoginAPIAuthenticatorCmd();
         try {
             cmd.checkAndFailOnMissingSAMLSignature(null);
@@ -290,7 +290,7 @@ public class SAML2LoginAPIAuthenticatorCmdTest {
 
     @Test(expected = ServerApiException.class)
     public void testFailOnSAMLSignatureCheckWhenTrue() throws 
NoSuchFieldException, IllegalAccessException {
-        overrideDefaultConfigValue(SAML2AuthManager.SAMLCheckSignature, 
"_defaultValue", "true");
+        overrideDefaultConfigValue(SAML2AuthManager.SAMLCheckSignature, 
"_value", true);
         SAML2LoginAPIAuthenticatorCmd cmd = new 
SAML2LoginAPIAuthenticatorCmd();
         cmd.checkAndFailOnMissingSAMLSignature(null);
     }

Reply via email to