sonarqube

Author: Daan Hoogland

plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/saml/SAML2AuthManagerImpl.java

@@ -455,29 +455,36 @@ public boolean isUserAuthorized(Long userId, String entityId) {
     @Override
     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(getPreSamlSource(user.getId()));
-                    } else {
-                        user.setSource(User.Source.SAML2DISABLED);
-                    }
-                } else {
-                    return false;
-                }
-            }
-            _userDao.update(user.getId(), user);
-            return true;
+        if (user == null) {
+            return false;
         }
-        return false;
+        if (enable) {
+            enableSAMLForUser(user, entityId);
+        } else if (!disableSAMLForUser(user)) {
+            return false;
+        }
+        _userDao.update(user.getId(), user);
+        return true;
+    }
+
+    private void enableSAMLForUser(UserVO user, String entityId) {
+        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);
+    }
+
+    private boolean disableSAMLForUser(UserVO user) {
+        if (!user.getSource().equals(User.Source.SAML2)) {
+            return false;
+        }
+        if (SAML2AuthManager.EnableLoginAfterSAMLDisable.value()) {
+            user.setSource(getPreSamlSource(user.getId()));
+        } else {
+            user.setSource(User.Source.SAML2DISABLED);
+        }
+        return true;
     }
 
     /**

plugins/user-authenticators/saml2/src/test/java/org/apache/cloudstack/SAML2AuthManagerImplTest.java

@@ -19,6 +19,10 @@
 
 package org.apache.cloudstack;
 
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
 import java.lang.reflect.Field;
 import java.lang.reflect.Method;
 
@@ -132,11 +136,11 @@ public void testAuthorizeUserStoresPreSamlSourceOnEnable() {
         UserVO user = new UserVO(200L);
         user.setUsername("someuser");
         user.setSource(User.Source.LDAP);
-        Mockito.when(userDao.getUser(Mockito.anyLong())).thenReturn(user);
+        when(userDao.getUser(Mockito.anyLong())).thenReturn(user);
 
         saml2AuthManager.authorizeUser(200L, "someID", true);
 
-        Mockito.verify(userDetailsDao).addDetail(200L, "PreSamlSource", "LDAP", false);
+        verify(userDetailsDao).addDetail(200L, "PreSamlSource", "LDAP", false);
         assertEquals(User.Source.SAML2, user.getSource());
     }
 
@@ -145,30 +149,30 @@ public void testAuthorizeUserDoesNotRestorePreSamlSourceWhenAlreadyAuthorized()
         UserVO user = new UserVO(200L);
         user.setUsername("someuser");
         user.setSource(User.Source.SAML2);
-        Mockito.when(userDao.getUser(Mockito.anyLong())).thenReturn(user);
+        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());
+        verify(userDetailsDao, 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"));
+        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);
+        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"));
+        when(userDetailsDao.findDetail(200L, "PreSamlSource")).thenReturn(new UserDetailVO(200L, "PreSamlSource", "not-a-real-source"));
 
         assertEquals(User.Source.UNKNOWN, invokeGetPreSamlSource(200L));
     }