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:31 UTC
(cloudstack) 01/01: make sure pre saml login is enabled after saml is disabled
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() {