make sure pre saml login is enabled after saml is disabled

Author: Daan Hoogland

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<String, SAMLProviderMetadata> _idpMetadataMap = new HashMap<String, SAMLProviderMetadata>();
 
@@ -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;

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() {