From 5de8cb179218be97842993d38365c3902a160328 Mon Sep 17 00:00:00 2001 From: jeff Date: Thu, 28 Jan 2016 14:23:10 +0000 Subject: [PATCH 1/3] Add missing license header to ActionEventUtilsTest. --- .../com/cloud/event/ActionEventUtilsTest.java | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/server/test/com/cloud/event/ActionEventUtilsTest.java b/server/test/com/cloud/event/ActionEventUtilsTest.java index a7fcf53f61..17d3909e6c 100644 --- a/server/test/com/cloud/event/ActionEventUtilsTest.java +++ b/server/test/com/cloud/event/ActionEventUtilsTest.java @@ -1,3 +1,19 @@ +// 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.event; import java.lang.reflect.Field; From 828cadb8b44563b6dd8130a0493dbf068288205f Mon Sep 17 00:00:00 2001 From: Rohit Yadav Date: Wed, 27 Apr 2016 00:02:11 +0530 Subject: [PATCH 2/3] 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 Signed-off-by: Rohit Yadav --- 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/MockDomainManagerImpl.java | 5 +++++ 5 files changed, 53 insertions(+), 11 deletions(-) diff --git a/api/src/com/cloud/user/DomainService.java b/api/src/com/cloud/user/DomainService.java index 4c1f93d073..3ccfcbcea4 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); + } diff --git a/server/src/com/cloud/api/ApiServer.java b/server/src/com/cloud/api/ApiServer.java index 1459dc2832..66005570a3 100644 --- a/server/src/com/cloud/api/ApiServer.java +++ b/server/src/com/cloud/api/ApiServer.java @@ -998,17 +998,11 @@ public ResponseObject loginUser(final HttpSession session, final String username final Map 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); diff --git a/server/src/com/cloud/api/auth/DefaultLoginAPIAuthenticatorCmd.java b/server/src/com/cloud/api/auth/DefaultLoginAPIAuthenticatorCmd.java index d6eac7864d..c83e7080ed 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 String authenticate(String command, Map params, HttpSes 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) { diff --git a/server/src/com/cloud/user/DomainManagerImpl.java b/server/src/com/cloud/user/DomainManagerImpl.java index fbbe0c2870..aa54412ae0 100644 --- a/server/src/com/cloud/user/DomainManagerImpl.java +++ b/server/src/com/cloud/user/DomainManagerImpl.java @@ -24,6 +24,7 @@ import javax.ejb.Local; import javax.inject.Inject; +import com.google.common.base.Strings; import org.apache.log4j.Logger; import org.springframework.stereotype.Component; @@ -220,6 +221,25 @@ public DomainVO findDomainByPath(String domainPath) { return _domainDao.findDomainByPath(domainPath); } + @Override + public Domain findDomainByIdOrPath(final Long id, final String domainPath) { + Long domainId = id; + if (domainId == null) { + if (Strings.isNullOrEmpty(domainPath) || domainPath.trim().isEmpty()) { + domainId = Domain.ROOT_DOMAIN; + } else { + final Domain domainVO = findDomainByPath(domainPath); + if (domainVO != null) { + domainId = domainVO.getId(); + } + } + } + if (domainId != null) { + return _domainDao.findById(domainId); + } + return null; + } + @Override public Set getDomainParentIds(long domainId) { return _domainDao.getDomainParentIds(domainId); diff --git a/server/test/com/cloud/user/MockDomainManagerImpl.java b/server/test/com/cloud/user/MockDomainManagerImpl.java index 7dddefbcf3..f44ab08d5d 100644 --- a/server/test/com/cloud/user/MockDomainManagerImpl.java +++ b/server/test/com/cloud/user/MockDomainManagerImpl.java @@ -93,6 +93,11 @@ public DomainVO findDomainByPath(String domainPath) { return null; } + @Override + public DomainVO findDomainByIdOrPath(Long id, String domainPath) { + return null; + } + @Override public Set getDomainParentIds(long domainId) { // TODO Auto-generated method stub From d9f5cc72175ab5ab98d8f0eb4ca87a94d42eb40f Mon Sep 17 00:00:00 2001 From: Rohit Yadav Date: Wed, 25 May 2016 11:52:58 +0530 Subject: [PATCH 3/3] CLOUDSTACK-9376: Restrict listTemplates API with filter=all for root admin Restricts use of listemplates API with templatefilter=all for root admin only. Signed-off-by: Rohit Yadav --- .../com/cloud/api/query/QueryManagerImpl.java | 4 +- test/integration/component/test_templates.py | 76 +++++++++++++++++++ 2 files changed, 78 insertions(+), 2 deletions(-) diff --git a/server/src/com/cloud/api/query/QueryManagerImpl.java b/server/src/com/cloud/api/query/QueryManagerImpl.java index d64b2ac71e..b2edd2e47b 100644 --- a/server/src/com/cloud/api/query/QueryManagerImpl.java +++ b/server/src/com/cloud/api/query/QueryManagerImpl.java @@ -3055,9 +3055,9 @@ private Pair, Integer> searchForTemplatesInternal(ListTempl boolean listAll = false; if (templateFilter != null && templateFilter == TemplateFilter.all) { - if (caller.getType() == Account.ACCOUNT_TYPE_NORMAL) { + if (caller.getType() != Account.ACCOUNT_TYPE_ADMIN) { throw new InvalidParameterValueException("Filter " + TemplateFilter.all - + " can be specified by admin only"); + + " can be specified by root admin only"); } listAll = true; } diff --git a/test/integration/component/test_templates.py b/test/integration/component/test_templates.py index b1e7e7c654..c8384d97c8 100644 --- a/test/integration/component/test_templates.py +++ b/test/integration/component/test_templates.py @@ -22,6 +22,7 @@ from marvin.cloudstackAPI import listZones from marvin.lib.utils import (cleanup_resources) from marvin.lib.base import (Account, + Domain, Template, ServiceOffering, VirtualMachine, @@ -51,6 +52,7 @@ def __init__(self): # username "password": "password", }, + "testdomain": {"name": "test"}, "service_offering": { "name": "Tiny Instance", "displaytext": "Tiny Instance", @@ -602,3 +604,77 @@ def test_04_template_from_snapshot(self): "Check the state of VM created from Template" ) return + + +class TestListTemplate(cloudstackTestCase): + + def setUp(self): + self.apiclient = self.testClient.getApiClient() + self.hypervisor = self.testClient.getHypervisorInfo() + self.dbclient = self.testClient.getDbConnection() + self.cleanup = [] + + self.services = Services().services + # Get Zone, Domain and templates + self.domain = get_domain(self.apiclient) + self.account = Account.create( + self.apiclient, + self.services["account"], + domainid=self.domain.id + ) + self.newdomain = Domain.create( + self.apiclient, + self.services["testdomain"], + parentdomainid=self.domain.id + ) + self.newdomain_account = Account.create( + self.apiclient, + self.services["account"], + admin=True, + domainid=self.newdomain.id + ) + self.cleanup = [ + self.account, + self.newdomain_account, + self.newdomain, + ] + + + def tearDown(self): + try: + # Clean up, terminate the created templates + cleanup_resources(self.apiclient, self.cleanup) + except Exception as e: + raise Exception("Warning: Exception during cleanup : %s" % e) + + + @attr(tags=["devcloud", "advanced", "advancedns", "smoke", "basic", "sg"], required_hardware="false") + def test_01_list_templates_with_templatefilter_all_normal_user(self): + """ + Test list templates with templatefilter=all is not permitted for normal user + """ + + user_api_client = self.testClient.getUserApiClient( + UserName=self.account.name, + DomainName=self.account.domain) + try: + list_template_response = Template.list(self.user_api_client, templatefilter='all') + self.fail("Regular User is able to use templatefilter='all' in listTemplates API call") + except Exception as e: + self.debug("ListTemplates API with templatefilter='all' is not permitted for normal user") + + + @attr(tags=["devcloud", "advanced", "advancedns", "smoke", "basic", "sg"], required_hardware="false") + def test_02_list_templates_with_templatefilter_all_domain_admin(self): + """ + Test list templates with templatefilter=all is not permitted for domain admin + """ + + domain_user_api_client = self.testClient.getUserApiClient( + UserName=self.newdomain_account.name, + DomainName=self.newdomain_account.domain) + try: + list_template_response = Template.list(self.domain_user_api_client, templatefilter='all') + self.fail("Domain admin is able to use templatefilter='all' in listTemplates API call") + except Exception as e: + self.debug("ListTemplates API with templatefilter='all' is not permitted for domain admin user")