cloudstack-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From bhais...@apache.org
Subject [2/4] git commit: updated refs/heads/4.6.2.1-RC20160525T1218 to bc2e2cf
Date Wed, 25 May 2016 09:21:56 GMT
CLOUDSTACK-9369: Restrict default login to ldap/native users

- Restricts default login auth handler to ldap and native-cloudstack users
- Refactors and create re-usable method to find domain by id/path
- Adds unit test for refactored method in DomainManagerImpl
- Adds smoke test for login handler

Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>


Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo
Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/86ae03e3
Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/86ae03e3
Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/86ae03e3

Branch: refs/heads/4.6.2.1-RC20160525T1218
Commit: 86ae03e3d3b692bd9642505e32162fff3d1824aa
Parents: 44731ca
Author: Rohit Yadav <rohit.yadav@shapeblue.com>
Authored: Wed May 25 11:55:59 2016 +0530
Committer: Rohit Yadav <rohit.yadav@shapeblue.com>
Committed: Wed May 25 11:57:04 2016 +0530

----------------------------------------------------------------------
 api/src/com/cloud/user/DomainService.java       |  10 ++
 server/src/com/cloud/api/ApiServer.java         |  16 +-
 .../auth/DefaultLoginAPIAuthenticatorCmd.java   |  13 ++
 .../src/com/cloud/user/DomainManagerImpl.java   |  20 +++
 .../com/cloud/user/DomainManagerImplTest.java   | 137 ++++++++++++++++++
 .../com/cloud/user/MockDomainManagerImpl.java   |   5 +
 test/integration/smoke/test_login.py            | 145 +++++++++++++++++++
 7 files changed, 335 insertions(+), 11 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cloudstack/blob/86ae03e3/api/src/com/cloud/user/DomainService.java
----------------------------------------------------------------------
diff --git a/api/src/com/cloud/user/DomainService.java b/api/src/com/cloud/user/DomainService.java
index 4c1f93d..3ccfcbc 100644
--- a/api/src/com/cloud/user/DomainService.java
+++ b/api/src/com/cloud/user/DomainService.java
@@ -56,4 +56,14 @@ public interface DomainService {
      */
     Domain findDomainByPath(String domainPath);
 
+    /**
+     * finds the domain by either id or provided path
+     *
+     * @param id the domain id
+     * @param domainPath the domain path use to lookup a domain
+     *
+     * @return domainId the long value of the domain ID, or null if no domain id exists with
provided id/path
+     */
+    Domain findDomainByIdOrPath(Long id, String domainPath);
+
 }

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/86ae03e3/server/src/com/cloud/api/ApiServer.java
----------------------------------------------------------------------
diff --git a/server/src/com/cloud/api/ApiServer.java b/server/src/com/cloud/api/ApiServer.java
index 1459dc2..6600557 100644
--- a/server/src/com/cloud/api/ApiServer.java
+++ b/server/src/com/cloud/api/ApiServer.java
@@ -998,17 +998,11 @@ public class ApiServer extends ManagerBase implements HttpRequestHandler,
ApiSer
             final Map<String, Object[]> requestParameters) throws CloudAuthenticationException
{
         // We will always use domainId first. If that does not exist, we will use domain
name. If THAT doesn't exist
         // we will default to ROOT
-        if (domainId == null) {
-            if (domainPath == null || domainPath.trim().length() == 0) {
-                domainId = Domain.ROOT_DOMAIN;
-            } else {
-                final Domain domainObj = _domainMgr.findDomainByPath(domainPath);
-                if (domainObj != null) {
-                    domainId = domainObj.getId();
-                } else { // if an unknown path is passed in, fail the login call
-                    throw new CloudAuthenticationException("Unable to find the domain from
the path " + domainPath);
-                }
-            }
+        final Domain userDomain = _domainMgr.findDomainByIdOrPath(domainId, domainPath);
+        if (userDomain == null || userDomain.getId() < 1L) {
+            throw new CloudAuthenticationException("Unable to find the domain from the path
" + domainPath);
+        } else {
+            domainId = userDomain.getId();
         }
 
         final UserAccount userAcct = _accountMgr.authenticateUser(username, password, domainId,
loginIpAddress, requestParameters);

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/86ae03e3/server/src/com/cloud/api/auth/DefaultLoginAPIAuthenticatorCmd.java
----------------------------------------------------------------------
diff --git a/server/src/com/cloud/api/auth/DefaultLoginAPIAuthenticatorCmd.java b/server/src/com/cloud/api/auth/DefaultLoginAPIAuthenticatorCmd.java
index d6eac78..c83e708 100644
--- a/server/src/com/cloud/api/auth/DefaultLoginAPIAuthenticatorCmd.java
+++ b/server/src/com/cloud/api/auth/DefaultLoginAPIAuthenticatorCmd.java
@@ -16,6 +16,9 @@
 // under the License.
 package com.cloud.api.auth;
 
+import com.cloud.domain.Domain;
+import com.cloud.user.User;
+import com.cloud.user.UserAccount;
 import org.apache.cloudstack.api.ApiServerService;
 import com.cloud.api.response.ApiResponseSerializer;
 import com.cloud.exception.CloudAuthenticationException;
@@ -156,6 +159,16 @@ public class DefaultLoginAPIAuthenticatorCmd extends BaseCmd implements
APIAuthe
         if (username != null) {
             final String pwd = ((password == null) ? null : password[0]);
             try {
+                final Domain userDomain = _domainService.findDomainByIdOrPath(domainId, domain);
+                if (userDomain != null) {
+                    domainId = userDomain.getId();
+                } else {
+                    throw new CloudAuthenticationException("Unable to find the domain from
the path " + domain);
+                }
+                final UserAccount userAccount = _accountService.getActiveUserAccount(username[0],
domainId);
+                if (userAccount == null || !(User.Source.UNKNOWN.equals(userAccount.getSource())
|| User.Source.LDAP.equals(userAccount.getSource()))) {
+                    throw new CloudAuthenticationException("User is not allowed CloudStack
login");
+                }
                 return ApiResponseSerializer.toSerializedString(_apiServer.loginUser(session,
username[0], pwd, domainId, domain, remoteAddress, params),
                         responseType);
             } catch (final CloudAuthenticationException ex) {

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/86ae03e3/server/src/com/cloud/user/DomainManagerImpl.java
----------------------------------------------------------------------
diff --git a/server/src/com/cloud/user/DomainManagerImpl.java b/server/src/com/cloud/user/DomainManagerImpl.java
index fbbe0c2..5df4283 100644
--- a/server/src/com/cloud/user/DomainManagerImpl.java
+++ b/server/src/com/cloud/user/DomainManagerImpl.java
@@ -24,6 +24,7 @@ import java.util.UUID;
 import javax.ejb.Local;
 import javax.inject.Inject;
 
+import com.google.common.base.Strings;
 import org.apache.log4j.Logger;
 import org.springframework.stereotype.Component;
 
@@ -221,6 +222,25 @@ public class DomainManagerImpl extends ManagerBase implements DomainManager,
Dom
     }
 
     @Override
+    public Domain findDomainByIdOrPath(final Long id, final String domainPath) {
+        Long domainId = id;
+        if (domainId == null || domainId < 1L) {
+            if (Strings.isNullOrEmpty(domainPath) || domainPath.trim().isEmpty()) {
+                domainId = Domain.ROOT_DOMAIN;
+            } else {
+                final Domain domainVO = findDomainByPath(domainPath.trim());
+                if (domainVO != null) {
+                    return domainVO;
+                }
+            }
+        }
+        if (domainId != null && domainId > 0L) {
+            return _domainDao.findById(domainId);
+        }
+        return null;
+    }
+
+    @Override
     public Set<Long> getDomainParentIds(long domainId) {
         return _domainDao.getDomainParentIds(domainId);
     }

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/86ae03e3/server/test/com/cloud/user/DomainManagerImplTest.java
----------------------------------------------------------------------
diff --git a/server/test/com/cloud/user/DomainManagerImplTest.java b/server/test/com/cloud/user/DomainManagerImplTest.java
new file mode 100644
index 0000000..82d5491
--- /dev/null
+++ b/server/test/com/cloud/user/DomainManagerImplTest.java
@@ -0,0 +1,137 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package com.cloud.user;
+
+import com.cloud.configuration.dao.ResourceCountDao;
+import com.cloud.configuration.dao.ResourceLimitDao;
+import com.cloud.dc.dao.DedicatedResourceDao;
+import com.cloud.domain.DomainVO;
+import com.cloud.domain.dao.DomainDao;
+import com.cloud.network.dao.NetworkDomainDao;
+import com.cloud.projects.ProjectManager;
+import com.cloud.projects.dao.ProjectDao;
+import com.cloud.service.dao.ServiceOfferingDao;
+import com.cloud.storage.dao.DiskOfferingDao;
+import com.cloud.user.dao.AccountDao;
+import org.apache.cloudstack.engine.orchestration.service.NetworkOrchestrationService;
+import org.apache.cloudstack.framework.messagebus.MessageBus;
+import org.apache.cloudstack.region.RegionManager;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.Mockito;
+import org.mockito.runners.MockitoJUnitRunner;
+
+import javax.inject.Inject;
+import java.lang.reflect.Field;
+
+@RunWith(MockitoJUnitRunner.class)
+public class DomainManagerImplTest {
+    @Mock
+    DomainDao _domainDao;
+    @Mock
+    AccountManager _accountMgr;
+    @Mock
+    ResourceCountDao _resourceCountDao;
+    @Mock
+    AccountDao _accountDao;
+    @Mock
+    DiskOfferingDao _diskOfferingDao;
+    @Mock
+    ServiceOfferingDao _offeringsDao;
+    @Mock
+    ProjectDao _projectDao;
+    @Mock
+    ProjectManager _projectMgr;
+    @Mock
+    RegionManager _regionMgr;
+    @Mock
+    ResourceLimitDao _resourceLimitDao;
+    @Mock
+    DedicatedResourceDao _dedicatedDao;
+    @Mock
+    NetworkOrchestrationService _networkMgr;
+    @Mock
+    NetworkDomainDao _networkDomainDao;
+    @Mock
+    MessageBus _messageBus;
+
+    DomainManagerImpl domainManager;
+
+    @Before
+    public void setup() throws NoSuchFieldException, SecurityException,
+            IllegalArgumentException, IllegalAccessException {
+        domainManager = new DomainManagerImpl();
+        for (Field field : DomainManagerImpl.class.getDeclaredFields()) {
+            if (field.getAnnotation(Inject.class) != null) {
+                field.setAccessible(true);
+                try {
+                    Field mockField = this.getClass().getDeclaredField(
+                            field.getName());
+                    field.set(domainManager, mockField.get(this));
+                } catch (Exception ignored) {
+                }
+            }
+        }
+    }
+
+    @Test
+    public void testFindDomainByIdOrPathNullOrEmpty() {
+        final DomainVO domain = new DomainVO("someDomain", 123, 1L, "network.domain");
+        Mockito.when(_domainDao.findById(1L)).thenReturn(domain);
+        Assert.assertEquals(domain, domainManager.findDomainByIdOrPath(null, null));
+        Assert.assertEquals(domain, domainManager.findDomainByIdOrPath(0L, ""));
+        Assert.assertEquals(domain, domainManager.findDomainByIdOrPath(-1L, " "));
+        Assert.assertEquals(domain, domainManager.findDomainByIdOrPath(null, "       "));
+    }
+
+    @Test
+    public void testFindDomainByIdOrPathValidPathAndInvalidId() {
+        final DomainVO domain = new DomainVO("someDomain", 123, 1L, "network.domain");
+        Mockito.when(_domainDao.findDomainByPath(Mockito.eq("/someDomain/"))).thenReturn(domain);
+        Assert.assertEquals(domain, domainManager.findDomainByIdOrPath(null, "/someDomain/"));
+        Assert.assertEquals(domain, domainManager.findDomainByIdOrPath(0L, " /someDomain/"));
+        Assert.assertEquals(domain, domainManager.findDomainByIdOrPath(-1L, "/someDomain/
"));
+        Assert.assertEquals(domain, domainManager.findDomainByIdOrPath(null, "   /someDomain/
  "));
+    }
+
+    @Test
+    public void testFindDomainByIdOrPathInvalidPathAndInvalidId() {
+        Mockito.when(_domainDao.findDomainByPath(Mockito.anyString())).thenReturn(null);
+        Assert.assertNull(domainManager.findDomainByIdOrPath(null, "/nonExistingDomain/"));
+        Assert.assertNull(domainManager.findDomainByIdOrPath(0L, " /nonExistingDomain/"));
+        Assert.assertNull(domainManager.findDomainByIdOrPath(-1L, "/nonExistingDomain/ "));
+        Assert.assertNull(domainManager.findDomainByIdOrPath(null, "   /nonExistingDomain/
  "));
+    }
+
+
+    @Test
+    public void testFindDomainByIdOrPathValidId() {
+        final DomainVO domain = new DomainVO("someDomain", 123, 1L, "network.domain");
+        Mockito.when(_domainDao.findById(1L)).thenReturn(domain);
+        Mockito.when(_domainDao.findDomainByPath(Mockito.eq("/validDomain/"))).thenReturn(new
DomainVO());
+        Assert.assertEquals(domain, domainManager.findDomainByIdOrPath(1L, null));
+        Assert.assertEquals(domain, domainManager.findDomainByIdOrPath(1L, ""));
+        Assert.assertEquals(domain, domainManager.findDomainByIdOrPath(1L, " "));
+        Assert.assertEquals(domain, domainManager.findDomainByIdOrPath(1L, "       "));
+        Assert.assertEquals(domain, domainManager.findDomainByIdOrPath(1L, "/validDomain/"));
+    }
+
+}

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/86ae03e3/server/test/com/cloud/user/MockDomainManagerImpl.java
----------------------------------------------------------------------
diff --git a/server/test/com/cloud/user/MockDomainManagerImpl.java b/server/test/com/cloud/user/MockDomainManagerImpl.java
index 7dddefb..f44ab08 100644
--- a/server/test/com/cloud/user/MockDomainManagerImpl.java
+++ b/server/test/com/cloud/user/MockDomainManagerImpl.java
@@ -94,6 +94,11 @@ public class MockDomainManagerImpl extends ManagerBase implements DomainManager,
     }
 
     @Override
+    public DomainVO findDomainByIdOrPath(Long id, String domainPath) {
+        return null;
+    }
+
+    @Override
     public Set<Long> getDomainParentIds(long domainId) {
         // TODO Auto-generated method stub
         return null;

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/86ae03e3/test/integration/smoke/test_login.py
----------------------------------------------------------------------
diff --git a/test/integration/smoke/test_login.py b/test/integration/smoke/test_login.py
new file mode 100644
index 0000000..5855fea
--- /dev/null
+++ b/test/integration/smoke/test_login.py
@@ -0,0 +1,145 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+from nose.plugins.attrib import attr
+from marvin.cloudstackTestCase import *
+from marvin.cloudstackAPI import *
+from marvin.lib.utils import *
+from marvin.lib.base import *
+from marvin.lib.common import *
+
+import requests
+
+
+class TestLogin(cloudstackTestCase):
+    """
+        Tests default login API handler
+    """
+
+    def setUp(self):
+        self.apiclient = self.testClient.getApiClient()
+        self.dbclient = self.testClient.getDbConnection()
+        self.server_details = self.config.__dict__["mgtSvr"][0].__dict__
+        self.server_url = "http://%s:8080/client/api" % self.server_details['mgtSvrIp']
+        self.testdata = {
+            "account": {
+                "email": "login-user@test.cloud",
+                "firstname": "TestLoginFirstName",
+                "lastname": "TestLoginLastName",
+                "username": "testloginuser-",
+                "password": "password123",
+            }
+        }
+        self.cleanup = []
+
+
+    def tearDown(self):
+        try:
+            cleanup_resources(self.apiclient, self.cleanup)
+        except Exception as e:
+            raise Exception("Warning: Exception during cleanup : %s" % e)
+
+
+    def login(self, username, password, domain="/"):
+        """
+            Logs in and returns a session to be used for subsequent API calls
+        """
+        args = {}
+        args["command"] = 'login'
+        args["username"] = username
+        args["password"] = password
+        args["domain"] = domain
+        args["response"] = "json"
+
+        session = requests.Session()
+
+        try:
+            resp = session.post(self.server_url, params=args, verify=False)
+        except requests.exceptions.ConnectionError, e:
+            self.fail("Failed to attempt login request to mgmt server")
+            return None, None
+
+        return resp, session
+
+
+    @attr(tags = ["devcloud", "advanced", "advancedns", "advancedsg", "smoke",
+                  "basic", "sg"], required_hardware="false")
+    def login_test_saml_user(self):
+        """
+            Tests that SAML users are not allowed CloudStack local log in
+
+            Creates account across various account types and converts them to
+            a SAML user and tests that they are not able to log in; then
+            converts them back as a CloudStack user account and verifies that
+            they are allowed to log in and make API requests
+        """
+        # Tests across various account types: 0=User, 1=Root Admin, 2=Domain Admin
+        for account_type in range(0, 3):
+            account = Account.create(
+                self.apiclient,
+                self.testdata['account'],
+                admin=account_type
+            )
+            self.cleanup.append(account)
+
+            username = account.user[0].username
+            password = self.testdata['account']['password']
+
+            # Convert newly created account user to SAML user
+            user_id = self.dbclient.execute("select id from user where uuid='%s'" % account.user[0].id)[0][0]
+            self.dbclient.execute("update user set source='SAML2' where id=%d" % user_id)
+
+            response, session = self.login(username, password)
+            self.assertEqual(
+                response.json()['loginresponse']['errorcode'],
+                531,
+                "SAML user should not be allowed to log in, error code 531 not returned"
+            )
+            self.assertEqual(
+                response.json()['loginresponse']['errortext'],
+                "User is not allowed CloudStack login",
+                "Invalid error message returned, SAML user should not be allowed to log in"
+            )
+
+            # Convert newly created account user back to normal source
+            self.dbclient.execute("update user set source='UNKNOWN' where id=%d" % user_id)
+
+            response, session = self.login(username, password)
+            self.assertEqual(
+                response.status_code,
+                200,
+                "Login response code was not 200"
+            )
+            self.assertTrue(
+                len(response.json()['loginresponse']['sessionkey']) > 0,
+                "Invalid session key received"
+            )
+
+            args = {}
+            args["command"] = 'listUsers'
+            args["listall"] = 'true'
+            args["response"] = "json"
+            response = session.get(self.server_url, params=args)
+            self.assertEqual(
+                response.status_code,
+                200,
+                "listUsers response code was not 200"
+            )
+            self.assertTrue(
+                len(response.json()['listusersresponse']['user']) > 0,
+                "listUsers list is empty or zero"
+            )


Mime
View raw message