Return-Path: X-Original-To: apmail-cloudstack-commits-archive@www.apache.org Delivered-To: apmail-cloudstack-commits-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 891B1187F2 for ; Fri, 14 Aug 2015 06:54:43 +0000 (UTC) Received: (qmail 9062 invoked by uid 500); 14 Aug 2015 06:54:42 -0000 Delivered-To: apmail-cloudstack-commits-archive@cloudstack.apache.org Received: (qmail 8918 invoked by uid 500); 14 Aug 2015 06:54:42 -0000 Mailing-List: contact commits-help@cloudstack.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@cloudstack.apache.org Delivered-To: mailing list commits@cloudstack.apache.org Received: (qmail 8581 invoked by uid 99); 14 Aug 2015 06:54:42 -0000 Received: from git1-us-west.apache.org (HELO git1-us-west.apache.org) (140.211.11.23) by apache.org (qpsmtpd/0.29) with ESMTP; Fri, 14 Aug 2015 06:54:42 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id 72787E7143; Fri, 14 Aug 2015 06:54:42 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: bhaisaab@apache.org To: commits@cloudstack.apache.org Date: Fri, 14 Aug 2015 06:54:47 -0000 Message-Id: <921ad593952d42a2aa07880e880a4321@git.apache.org> In-Reply-To: <0529c891f5c34c64bd4810628d8ee35e@git.apache.org> References: <0529c891f5c34c64bd4810628d8ee35e@git.apache.org> X-Mailer: ASF-Git Admin Mailer Subject: [6/8] git commit: updated refs/heads/master to 869a83f CLOUDSTACK-8701: Add listandswitchsamlaccount API test and add boundary checks - Adds unit test for ListAndSwitchSAMLAccountCmd - Checks and logs in user only if they are enabled - If saml user switches to a locked account, send appropriate error message Signed-off-by: Rohit Yadav (cherry picked from commit b30977911dbfb1eae86d53ff1b848c5812b68c07) Signed-off-by: Rohit Yadav Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/25ccf412 Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/25ccf412 Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/25ccf412 Branch: refs/heads/master Commit: 25ccf4126d4729c1361855eaeef8e58acdebc70f Parents: fcbee60 Author: Rohit Yadav Authored: Wed Aug 12 14:04:08 2015 +0530 Committer: Rohit Yadav Committed: Fri Aug 14 12:00:04 2015 +0530 ---------------------------------------------------------------------- .../command/ListAndSwitchSAMLAccountCmd.java | 14 +- .../command/SAML2LoginAPIAuthenticatorCmd.java | 31 +-- .../ListAndSwitchSAMLAccountCmdTest.java | 201 +++++++++++++++++++ 3 files changed, 231 insertions(+), 15 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cloudstack/blob/25ccf412/plugins/user-authenticators/saml2/src/org/apache/cloudstack/api/command/ListAndSwitchSAMLAccountCmd.java ---------------------------------------------------------------------- diff --git a/plugins/user-authenticators/saml2/src/org/apache/cloudstack/api/command/ListAndSwitchSAMLAccountCmd.java b/plugins/user-authenticators/saml2/src/org/apache/cloudstack/api/command/ListAndSwitchSAMLAccountCmd.java index db0d6dc..80aee1b 100644 --- a/plugins/user-authenticators/saml2/src/org/apache/cloudstack/api/command/ListAndSwitchSAMLAccountCmd.java +++ b/plugins/user-authenticators/saml2/src/org/apache/cloudstack/api/command/ListAndSwitchSAMLAccountCmd.java @@ -19,6 +19,7 @@ package org.apache.cloudstack.api.command; import com.cloud.api.response.ApiResponseSerializer; import com.cloud.domain.Domain; import com.cloud.domain.dao.DomainDao; +import com.cloud.exception.CloudAuthenticationException; import com.cloud.user.Account; import com.cloud.user.User; import com.cloud.user.UserAccount; @@ -50,6 +51,7 @@ import javax.inject.Inject; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import javax.servlet.http.HttpSession; +import java.io.IOException; import java.util.ArrayList; import java.util.List; import java.util.Map; @@ -131,7 +133,14 @@ public class ListAndSwitchSAMLAccountCmd extends BaseCmd implements APIAuthentic final User user = _userDao.findByUuid(userUuid); final Domain domain = _domainDao.findByUuid(domainUuid); final UserAccount nextUserAccount = _accountService.getUserAccountById(user.getId()); - if (!nextUserAccount.getUsername().equals(currentUserAccount.getUsername()) + if (nextUserAccount != null && !nextUserAccount.getAccountState().equals(Account.State.enabled.toString())) { + throw new ServerApiException(ApiErrorCode.ACCOUNT_ERROR, _apiServer.getSerializedApiError(ApiErrorCode.PARAM_ERROR.getHttpCode(), + "The requested user account is locked and cannot be switched to, please contact your administrator.", + params, responseType)); + } + if (nextUserAccount == null + || !nextUserAccount.getAccountState().equals(Account.State.enabled.toString()) + || !nextUserAccount.getUsername().equals(currentUserAccount.getUsername()) || !nextUserAccount.getExternalEntity().equals(currentUserAccount.getExternalEntity()) || (nextUserAccount.getDomainId() != domain.getId()) || (nextUserAccount.getSource() != User.Source.SAML2)) { @@ -147,7 +156,8 @@ public class ListAndSwitchSAMLAccountCmd extends BaseCmd implements APIAuthentic resp.sendRedirect(SAML2AuthManager.SAMLCloudStackRedirectionUrl.value()); return ApiResponseSerializer.toSerializedString(loginResponse, responseType); } - } catch (final Exception ignored) { + } catch (CloudAuthenticationException | IOException exception) { + s_logger.debug("Failed to switch to request SAML user account due to: " + exception.getMessage()); } } else { List switchableAccounts = _userAccountDao.getAllUsersByNameAndEntity(currentUserAccount.getUsername(), currentUserAccount.getExternalEntity()); http://git-wip-us.apache.org/repos/asf/cloudstack/blob/25ccf412/plugins/user-authenticators/saml2/src/org/apache/cloudstack/api/command/SAML2LoginAPIAuthenticatorCmd.java ---------------------------------------------------------------------- diff --git a/plugins/user-authenticators/saml2/src/org/apache/cloudstack/api/command/SAML2LoginAPIAuthenticatorCmd.java b/plugins/user-authenticators/saml2/src/org/apache/cloudstack/api/command/SAML2LoginAPIAuthenticatorCmd.java index f2fcf59..7a1b83c 100644 --- a/plugins/user-authenticators/saml2/src/org/apache/cloudstack/api/command/SAML2LoginAPIAuthenticatorCmd.java +++ b/plugins/user-authenticators/saml2/src/org/apache/cloudstack/api/command/SAML2LoginAPIAuthenticatorCmd.java @@ -17,6 +17,7 @@ package org.apache.cloudstack.api.command; import com.cloud.api.response.ApiResponseSerializer; +import com.cloud.exception.CloudAuthenticationException; import com.cloud.user.Account; import com.cloud.user.DomainManager; import com.cloud.user.UserAccount; @@ -293,28 +294,32 @@ public class SAML2LoginAPIAuthenticatorCmd extends BaseCmd implements APIAuthent UserAccount userAccount = null; List possibleUserAccounts = _userAccountDao.getAllUsersByNameAndEntity(username, issuer.getValue()); if (possibleUserAccounts != null && possibleUserAccounts.size() > 0) { - // By default, log into the first user account + // Log into the first enabled user account // Users can switch to other allowed accounts later - userAccount = possibleUserAccounts.get(0); + for (UserAccountVO possibleUserAccount: possibleUserAccounts) { + if (possibleUserAccount.getAccountState().equals(Account.State.enabled.toString())) { + userAccount = possibleUserAccount; + break; + } + } } if (userAccount == null || userAccount.getExternalEntity() == null || !_samlAuthManager.isUserAuthorized(userAccount.getId(), issuer.getValue())) { throw new ServerApiException(ApiErrorCode.ACCOUNT_ERROR, _apiServer.getSerializedApiError(ApiErrorCode.ACCOUNT_ERROR.getHttpCode(), - "Your authenticated user is not authorized, please contact your administrator", + "Your authenticated user is not authorized for SAML Single Sign-On, please contact your administrator", params, responseType)); } - if (userAccount != null) { - try { - if (_apiServer.verifyUser(userAccount.getId())) { - LoginCmdResponse loginResponse = (LoginCmdResponse) _apiServer.loginUser(session, userAccount.getUsername(), userAccount.getUsername() + userAccount.getSource().toString(), - userAccount.getDomainId(), null, remoteAddress, params); - SAMLUtils.setupSamlUserCookies(loginResponse, resp); - resp.sendRedirect(SAML2AuthManager.SAMLCloudStackRedirectionUrl.value()); - return ApiResponseSerializer.toSerializedString(loginResponse, responseType); - } - } catch (final Exception ignored) { + try { + if (_apiServer.verifyUser(userAccount.getId())) { + LoginCmdResponse loginResponse = (LoginCmdResponse) _apiServer.loginUser(session, userAccount.getUsername(), userAccount.getUsername() + userAccount.getSource().toString(), + userAccount.getDomainId(), null, remoteAddress, params); + SAMLUtils.setupSamlUserCookies(loginResponse, resp); + resp.sendRedirect(SAML2AuthManager.SAMLCloudStackRedirectionUrl.value()); + return ApiResponseSerializer.toSerializedString(loginResponse, responseType); } + } catch (CloudAuthenticationException | IOException exception) { + s_logger.debug("SAML Login failed to log in the user due to: " + exception.getMessage()); } } } catch (IOException e) { http://git-wip-us.apache.org/repos/asf/cloudstack/blob/25ccf412/plugins/user-authenticators/saml2/test/org/apache/cloudstack/api/command/ListAndSwitchSAMLAccountCmdTest.java ---------------------------------------------------------------------- diff --git a/plugins/user-authenticators/saml2/test/org/apache/cloudstack/api/command/ListAndSwitchSAMLAccountCmdTest.java b/plugins/user-authenticators/saml2/test/org/apache/cloudstack/api/command/ListAndSwitchSAMLAccountCmdTest.java new file mode 100644 index 0000000..1423243 --- /dev/null +++ b/plugins/user-authenticators/saml2/test/org/apache/cloudstack/api/command/ListAndSwitchSAMLAccountCmdTest.java @@ -0,0 +1,201 @@ +/* + * 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 org.apache.cloudstack.api.command; + +import com.cloud.domain.DomainVO; +import com.cloud.domain.dao.DomainDao; +import com.cloud.user.Account; +import com.cloud.user.AccountService; +import com.cloud.user.User; +import com.cloud.user.UserAccountVO; +import com.cloud.user.UserVO; +import com.cloud.user.dao.UserAccountDao; +import com.cloud.user.dao.UserDao; +import com.cloud.utils.HttpUtils; +import junit.framework.TestCase; +import org.apache.cloudstack.api.ApiConstants; +import org.apache.cloudstack.api.ApiErrorCode; +import org.apache.cloudstack.api.ApiServerService; +import org.apache.cloudstack.api.BaseCmd; +import org.apache.cloudstack.api.ServerApiException; +import org.apache.cloudstack.api.auth.APIAuthenticationType; +import org.apache.cloudstack.api.response.LoginCmdResponse; +import org.apache.cloudstack.saml.SAML2AuthManager; +import org.junit.Assert; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.runners.MockitoJUnitRunner; + +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import javax.servlet.http.HttpSession; +import java.lang.reflect.Field; +import java.util.HashMap; +import java.util.Map; + +@RunWith(MockitoJUnitRunner.class) +public class ListAndSwitchSAMLAccountCmdTest extends TestCase { + @Mock + ApiServerService apiServer; + + @Mock + SAML2AuthManager samlAuthManager; + + @Mock + AccountService accountService; + + @Mock + UserAccountDao userAccountDao; + + @Mock + UserDao userDao; + + @Mock + DomainDao domainDao; + + @Mock + HttpSession session; + + @Mock + HttpServletResponse resp; + + @Mock + HttpServletRequest req; + + @Test + public void testListAndSwitchSAMLAccountCmd() throws Exception { + // Setup + final Map params = new HashMap(); + final String sessionKeyValue = "someSessionIDValue"; + Mockito.when(session.getAttribute(ApiConstants.SESSIONKEY)).thenReturn(sessionKeyValue); + Mockito.when(session.getAttribute("userid")).thenReturn(2L); + params.put(ApiConstants.USER_ID, new String[]{"2"}); + params.put(ApiConstants.DOMAIN_ID, new String[]{"1"}); + Mockito.when(userDao.findByUuid(Mockito.anyString())).thenReturn(new UserVO(2L)); + Mockito.when(domainDao.findByUuid(Mockito.anyString())).thenReturn(new DomainVO()); + + // Mock/field setup + ListAndSwitchSAMLAccountCmd cmd = new ListAndSwitchSAMLAccountCmd(); + Field apiServerField = ListAndSwitchSAMLAccountCmd.class.getDeclaredField("_apiServer"); + apiServerField.setAccessible(true); + apiServerField.set(cmd, apiServer); + + Field managerField = ListAndSwitchSAMLAccountCmd.class.getDeclaredField("_samlAuthManager"); + managerField.setAccessible(true); + managerField.set(cmd, samlAuthManager); + + Field accountServiceField = BaseCmd.class.getDeclaredField("_accountService"); + accountServiceField.setAccessible(true); + accountServiceField.set(cmd, accountService); + + Field userAccountDaoField = ListAndSwitchSAMLAccountCmd.class.getDeclaredField("_userAccountDao"); + userAccountDaoField.setAccessible(true); + userAccountDaoField.set(cmd, userAccountDao); + + Field userDaoField = ListAndSwitchSAMLAccountCmd.class.getDeclaredField("_userDao"); + userDaoField.setAccessible(true); + userDaoField.set(cmd, userDao); + + Field domainDaoField = ListAndSwitchSAMLAccountCmd.class.getDeclaredField("_domainDao"); + domainDaoField.setAccessible(true); + domainDaoField.set(cmd, domainDao); + + // invalid session test + try { + cmd.authenticate("command", params, null, "random", HttpUtils.RESPONSE_TYPE_JSON, new StringBuilder(), req, resp); + } catch (ServerApiException exception) { + assertEquals(exception.getErrorCode(), ApiErrorCode.UNAUTHORIZED); + } finally { + Mockito.verify(accountService, Mockito.times(0)).getUserAccountById(Mockito.anyLong()); + } + + // invalid sessionkey value test + params.put(ApiConstants.SESSIONKEY, new String[]{"someOtherValue"}); + try { + Mockito.when(session.isNew()).thenReturn(false); + cmd.authenticate("command", params, session, "random", HttpUtils.RESPONSE_TYPE_JSON, new StringBuilder(), req, resp); + } catch (ServerApiException exception) { + assertEquals(exception.getErrorCode(), ApiErrorCode.UNAUTHORIZED); + } finally { + Mockito.verify(accountService, Mockito.times(0)).getUserAccountById(Mockito.anyLong()); + } + + // valid sessionkey value test + params.put(ApiConstants.SESSIONKEY, new String[]{sessionKeyValue}); + try { + cmd.authenticate("command", params, session, "random", HttpUtils.RESPONSE_TYPE_JSON, new StringBuilder(), req, resp); + } catch (ServerApiException exception) { + assertEquals(exception.getErrorCode(), ApiErrorCode.ACCOUNT_ERROR); + } finally { + Mockito.verify(accountService, Mockito.times(1)).getUserAccountById(Mockito.anyLong()); + } + + // valid sessionkey, invalid useraccount type (non-saml) value test + UserAccountVO mockedUserAccount = new UserAccountVO(); + mockedUserAccount.setId(2L); + mockedUserAccount.setAccountState(Account.State.enabled.toString()); + mockedUserAccount.setUsername("someUsername"); + mockedUserAccount.setExternalEntity("some IDP ID"); + mockedUserAccount.setDomainId(0L); + mockedUserAccount.setSource(User.Source.UNKNOWN); + Mockito.when(accountService.getUserAccountById(Mockito.anyLong())).thenReturn(mockedUserAccount); + try { + cmd.authenticate("command", params, session, "random", HttpUtils.RESPONSE_TYPE_JSON, new StringBuilder(), req, resp); + } catch (ServerApiException exception) { + assertEquals(exception.getErrorCode(), ApiErrorCode.ACCOUNT_ERROR); + } finally { + // accountService should have been called twice by now, for this case and the case above + Mockito.verify(accountService, Mockito.times(2)).getUserAccountById(Mockito.anyLong()); + } + + // all valid test + mockedUserAccount.setSource(User.Source.SAML2); + Mockito.when(accountService.getUserAccountById(Mockito.anyLong())).thenReturn(mockedUserAccount); + Mockito.when(apiServer.verifyUser(Mockito.anyLong())).thenReturn(true); + LoginCmdResponse loginCmdResponse = new LoginCmdResponse(); + loginCmdResponse.setUserId("1"); + loginCmdResponse.setDomainId("1"); + loginCmdResponse.setType("1"); + loginCmdResponse.setUsername("userName"); + loginCmdResponse.setAccount("someAccount"); + loginCmdResponse.setFirstName("firstName"); + loginCmdResponse.setLastName("lastName"); + loginCmdResponse.setSessionKey("newSessionKeyString"); + Mockito.when(apiServer.loginUser(Mockito.any(HttpSession.class), Mockito.anyString(), Mockito.anyString(), + Mockito.anyLong(), Mockito.anyString(), Mockito.anyString(), Mockito.anyMap())).thenReturn(loginCmdResponse); + try { + cmd.authenticate("command", params, session, "random", HttpUtils.RESPONSE_TYPE_JSON, new StringBuilder(), req, resp); + } catch (ServerApiException exception) { + fail("SAML list and switch account API failed to pass for all valid data: " + exception.getMessage()); + } finally { + // accountService should have been called 4 times by now, for this case twice and 2 for cases above + Mockito.verify(accountService, Mockito.times(4)).getUserAccountById(Mockito.anyLong()); + Mockito.verify(resp, Mockito.times(1)).sendRedirect(Mockito.anyString()); + } + } + + @Test + public void testGetAPIType() { + Assert.assertTrue(new ListAndSwitchSAMLAccountCmd().getAPIType() == APIAuthenticationType.READONLY_API); + } + +}