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 9f8101b867d4..0994571ec84e 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.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 @@ @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 _idpMetadataMap = new HashMap(); @@ -115,6 +121,9 @@ public String getSAMLIdentityProviderMetadataURL(){ @Inject private UserDao _userDao; + @Inject + private UserDetailsDao userDetailsDao; + @Inject DomainManager _domainMgr; @@ -448,13 +457,16 @@ public boolean authorizeUser(Long userId, String entityId, boolean enable) { 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 boolean authorizeUser(Long userId, String entityId, boolean enable) { 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 81594db1009f..5b06072620b6 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 void setUp() throws NoSuchFieldException, IllegalAccessException { 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 void testAuthorizeUser() { 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() {