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 <[email protected]> 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() {
