You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by da...@apache.org on 2026/08/22 19:33:30 UTC

(cloudstack) branch ghi12595-saml-disable-ldap-fallback created (now bc42cd18470)

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

DaanHoogland pushed a change to branch ghi12595-saml-disable-ldap-fallback
in repository https://gitbox.apache.org/repos/asf/cloudstack.git


      at bc42cd18470 make sure pre saml login is enabled after saml is disabled

This branch includes the following new commits:

     new bc42cd18470 make sure pre saml login is enabled after saml is disabled

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.



(cloudstack) 01/01: make sure pre saml login is enabled after saml is disabled

Posted by da...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

DaanHoogland pushed a commit to branch ghi12595-saml-disable-ldap-fallback
in repository https://gitbox.apache.org/repos/asf/cloudstack.git

commit bc42cd18470581e503d40626c6eacbc8aef9284b
Author: Daan Hoogland <da...@apache.org>
AuthorDate: Sat Aug 22 21:32:47 2026 +0200

    make sure pre saml login is enabled after saml is disabled
---
 .../cloudstack/saml/SAML2AuthManagerImpl.java      | 30 ++++++++++-
 .../cloudstack/SAML2AuthManagerImplTest.java       | 60 ++++++++++++++++++++++
 2 files changed, 89 insertions(+), 1 deletion(-)

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 9f8101b867d..0994571ec84 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
@@ -54,6 +54,8 @@ import org.apache.cloudstack.framework.config.ConfigKey;
 import org.apache.cloudstack.framework.config.Configurable;
 import org.apache.cloudstack.framework.security.keystore.KeystoreDao;
 import org.apache.cloudstack.framework.security.keystore.KeystoreVO;
+import org.apache.cloudstack.resourcedetail.UserDetailVO;
+import org.apache.cloudstack.resourcedetail.dao.UserDetailsDao;
 import org.apache.cloudstack.utils.security.CertUtils;
 import org.apache.commons.codec.binary.Base64;
 import org.apache.commons.httpclient.HttpClient;
@@ -92,6 +94,10 @@ import com.cloud.utils.component.AdapterBase;
 @Component
 public class SAML2AuthManagerImpl extends AdapterBase implements SAML2AuthManager, Configurable {
 
+    /** Remembers the user's Source (e.g. LDAP) from before SAML was authorized, so disabling
+     * SAML can fall back to it instead of always defaulting to {@link User.Source#UNKNOWN}. */
+    private static final String PRE_SAML_SOURCE_DETAIL_KEY = "PreSamlSource";
+
     private SAMLProviderMetadata _spMetadata = new SAMLProviderMetadata();
     private Map<String, SAMLProviderMetadata> _idpMetadataMap = new HashMap<String, SAMLProviderMetadata>();
 
@@ -115,6 +121,9 @@ public class SAML2AuthManagerImpl extends AdapterBase implements SAML2AuthManage
     @Inject
     private UserDao _userDao;
 
+    @Inject
+    private UserDetailsDao userDetailsDao;
+
     @Inject
     DomainManager _domainMgr;
 
@@ -448,13 +457,16 @@ public class SAML2AuthManagerImpl extends AdapterBase implements SAML2AuthManage
         UserVO user = _userDao.getUser(userId);
         if (user != null) {
             if (enable) {
+                if (user.getSource() != null && !User.Source.SAML2.equals(user.getSource()) && !User.Source.SAML2DISABLED.equals(user.getSource())) {
+                    userDetailsDao.addDetail(user.getId(), PRE_SAML_SOURCE_DETAIL_KEY, user.getSource().toString(), false);
+                }
                 user.setExternalEntity(entityId);
                 user.setSource(User.Source.SAML2);
             } else {
                 boolean enableLoginAfterSAMLDisable =  SAML2AuthManager.EnableLoginAfterSAMLDisable.value();
                 if (user.getSource().equals(User.Source.SAML2)) {
                     if(enableLoginAfterSAMLDisable) {
-                        user.setSource(User.Source.UNKNOWN);
+                        user.setSource(getPreSamlSource(user.getId()));
                     } else {
                         user.setSource(User.Source.SAML2DISABLED);
                     }
@@ -468,6 +480,22 @@ public class SAML2AuthManagerImpl extends AdapterBase implements SAML2AuthManage
         return false;
     }
 
+    /**
+     * The Source (e.g. LDAP) the user had before SAML was authorized for them, so disabling
+     * SAML can restore it instead of always falling back to {@link User.Source#UNKNOWN}.
+     */
+    private User.Source getPreSamlSource(long userId) {
+        UserDetailVO preSamlSource = userDetailsDao.findDetail(userId, PRE_SAML_SOURCE_DETAIL_KEY);
+        if (preSamlSource != null) {
+            try {
+                return User.Source.valueOf(preSamlSource.getValue());
+            } catch (IllegalArgumentException e) {
+                logger.warn("Unrecognized pre-SAML source '{}' stored for user {}; falling back to UNKNOWN", preSamlSource.getValue(), userId);
+            }
+        }
+        return User.Source.UNKNOWN;
+    }
+
     @Override
     public void saveToken(String authnId, String domainPath, String entity) {
         Long domainId = null;
diff --git a/plugins/user-authenticators/saml2/src/test/java/org/apache/cloudstack/SAML2AuthManagerImplTest.java b/plugins/user-authenticators/saml2/src/test/java/org/apache/cloudstack/SAML2AuthManagerImplTest.java
index 81594db1009..5b06072620b 100644
--- a/plugins/user-authenticators/saml2/src/test/java/org/apache/cloudstack/SAML2AuthManagerImplTest.java
+++ b/plugins/user-authenticators/saml2/src/test/java/org/apache/cloudstack/SAML2AuthManagerImplTest.java
@@ -20,8 +20,11 @@
 package org.apache.cloudstack;
 
 import java.lang.reflect.Field;
+import java.lang.reflect.Method;
 
 import org.apache.cloudstack.framework.security.keystore.KeystoreDao;
+import org.apache.cloudstack.resourcedetail.UserDetailVO;
+import org.apache.cloudstack.resourcedetail.dao.UserDetailsDao;
 import org.apache.cloudstack.saml.SAML2AuthManagerImpl;
 import org.apache.cloudstack.saml.SAMLTokenDao;
 import org.apache.cloudstack.saml.SAMLTokenVO;
@@ -50,6 +53,9 @@ public class SAML2AuthManagerImplTest extends TestCase {
     @Mock
     private UserDao userDao;
 
+    @Mock
+    private UserDetailsDao userDetailsDao;
+
     @Mock
     DomainManager domainMgr;
 
@@ -72,6 +78,10 @@ public class SAML2AuthManagerImplTest extends TestCase {
         userDaoField.setAccessible(true);
         userDaoField.set(saml2AuthManager, userDao);
 
+        Field userDetailsDaoField = SAML2AuthManagerImpl.class.getDeclaredField("userDetailsDao");
+        userDetailsDaoField.setAccessible(true);
+        userDetailsDaoField.set(saml2AuthManager, userDetailsDao);
+
         Field domainMgrField = SAML2AuthManagerImpl.class.getDeclaredField("_domainMgr");
         domainMgrField.setAccessible(true);
         domainMgrField.set(saml2AuthManager, domainMgr);
@@ -117,7 +127,57 @@ public class SAML2AuthManagerImplTest extends TestCase {
         Mockito.verify(userDao, Mockito.atLeastOnce()).update(Mockito.anyLong(), Mockito.any(user.getClass()));
     }
 
+    @Test
+    public void testAuthorizeUserStoresPreSamlSourceOnEnable() {
+        UserVO user = new UserVO(200L);
+        user.setUsername("someuser");
+        user.setSource(User.Source.LDAP);
+        Mockito.when(userDao.getUser(Mockito.anyLong())).thenReturn(user);
+
+        saml2AuthManager.authorizeUser(200L, "someID", true);
+
+        Mockito.verify(userDetailsDao).addDetail(200L, "PreSamlSource", "LDAP", false);
+        assertEquals(User.Source.SAML2, user.getSource());
+    }
+
+    @Test
+    public void testAuthorizeUserDoesNotRestorePreSamlSourceWhenAlreadyAuthorized() {
+        UserVO user = new UserVO(200L);
+        user.setUsername("someuser");
+        user.setSource(User.Source.SAML2);
+        Mockito.when(userDao.getUser(Mockito.anyLong())).thenReturn(user);
+
+        saml2AuthManager.authorizeUser(200L, "someID", true);
+
+        Mockito.verify(userDetailsDao, Mockito.never()).addDetail(Mockito.anyLong(), Mockito.anyString(), Mockito.anyString(), Mockito.anyBoolean());
+    }
+
+    @Test
+    public void testGetPreSamlSourceRestoresStoredSource() throws Exception {
+        Mockito.when(userDetailsDao.findDetail(200L, "PreSamlSource")).thenReturn(new UserDetailVO(200L, "PreSamlSource", "LDAP"));
+
+        assertEquals(User.Source.LDAP, invokeGetPreSamlSource(200L));
+    }
+
+    @Test
+    public void testGetPreSamlSourceDefaultsToUnknownWhenNothingStored() throws Exception {
+        Mockito.when(userDetailsDao.findDetail(200L, "PreSamlSource")).thenReturn(null);
+
+        assertEquals(User.Source.UNKNOWN, invokeGetPreSamlSource(200L));
+    }
+
+    @Test
+    public void testGetPreSamlSourceDefaultsToUnknownOnGarbageValue() throws Exception {
+        Mockito.when(userDetailsDao.findDetail(200L, "PreSamlSource")).thenReturn(new UserDetailVO(200L, "PreSamlSource", "not-a-real-source"));
+
+        assertEquals(User.Source.UNKNOWN, invokeGetPreSamlSource(200L));
+    }
 
+    private User.Source invokeGetPreSamlSource(long userId) throws Exception {
+        Method method = SAML2AuthManagerImpl.class.getDeclaredMethod("getPreSamlSource", long.class);
+        method.setAccessible(true);
+        return (User.Source) method.invoke(saml2AuthManager, userId);
+    }
 
     @Test
     public void testSaveToken() {