mirror of https://github.com/apache/cloudstack.git
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>
This commit is contained in:
Rohit Yadav
Will Stevens
parent
d9429f6add
commit
566e7d9fac
|
|
@ -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);
|
||||
|
||||
}
|
||||
|
|
|
|||
|
|
@ -993,17 +993,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);
|
||||
|
|
|
|||
|
|
@ -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) {
|
||||
|
|
|
|||
|
|
@ -73,6 +73,7 @@ import com.cloud.utils.exception.CloudRuntimeException;
|
|||
import com.cloud.utils.net.NetUtils;
|
||||
import com.cloud.vm.ReservationContext;
|
||||
import com.cloud.vm.ReservationContextImpl;
|
||||
import com.google.common.base.Strings;
|
||||
|
||||
@Component
|
||||
public class DomainManagerImpl extends ManagerBase implements DomainManager, DomainService {
|
||||
|
|
@ -218,6 +219,25 @@ public class DomainManagerImpl extends ManagerBase implements DomainManager, Dom
|
|||
return _domainDao.findDomainByPath(domainPath);
|
||||
}
|
||||
|
||||
@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);
|
||||
|
|
|
|||
|
|
@ -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/"));
|
||||
}
|
||||
|
||||
}
|
||||
|
|
@ -91,6 +91,11 @@ public class MockDomainManagerImpl extends ManagerBase implements DomainManager,
|
|||
return null;
|
||||
}
|
||||
|
||||
@Override
|
||||
public DomainVO findDomainByIdOrPath(Long id, String domainPath) {
|
||||
return null;
|
||||
}
|
||||
|
||||
@Override
|
||||
public Set<Long> getDomainParentIds(long domainId) {
|
||||
// TODO Auto-generated method stub
|
||||
|
|
|
|||
|
|
@ -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"
|
||||
)
|
||||
Loading…
Reference in New Issue