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 C3C6617D26 for ; Fri, 11 Sep 2015 12:54:49 +0000 (UTC) Received: (qmail 30406 invoked by uid 500); 11 Sep 2015 12:54:49 -0000 Delivered-To: apmail-cloudstack-commits-archive@cloudstack.apache.org Received: (qmail 30366 invoked by uid 500); 11 Sep 2015 12:54:49 -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 30348 invoked by uid 99); 11 Sep 2015 12:54:49 -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, 11 Sep 2015 12:54:49 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id 689B1DFFBF; Fri, 11 Sep 2015 12:54:49 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: widodh@apache.org To: commits@cloudstack.apache.org Date: Fri, 11 Sep 2015 12:54:49 -0000 Message-Id: <09a15dda1c254098af76ce22ec4da539@git.apache.org> X-Mailer: ASF-Git Admin Mailer Subject: [1/2] git commit: updated refs/heads/master to 5812f71 Repository: cloudstack Updated Branches: refs/heads/master 6330ef023 -> 5812f714f unittests to verify empty password is not allowed during account create Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/1865433e Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/1865433e Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/1865433e Branch: refs/heads/master Commit: 1865433e69fe9f0e639a51340406517496cb92cc Parents: 2d90f18 Author: Rajani Karuturi Authored: Tue Apr 1 11:32:33 2014 +0530 Committer: Rajani Karuturi Committed: Fri Sep 11 15:52:38 2015 +0530 ---------------------------------------------------------------------- .../command/admin/account/CreateAccountCmd.java | 15 ++- .../api/command/admin/user/CreateUserCmd.java | 13 ++- .../admin/account/CreateAccountCmdTest.java | 100 +++++++++++++++++++ .../command/admin/user/CreateUserCmdTest.java | 96 ++++++++++++++++++ .../com/cloud/user/AccountManagerImplTest.java | 43 ++++++++ 5 files changed, 260 insertions(+), 7 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cloudstack/blob/1865433e/api/src/org/apache/cloudstack/api/command/admin/account/CreateAccountCmd.java ---------------------------------------------------------------------- diff --git a/api/src/org/apache/cloudstack/api/command/admin/account/CreateAccountCmd.java b/api/src/org/apache/cloudstack/api/command/admin/account/CreateAccountCmd.java index ec3090f..b55ce71 100644 --- a/api/src/org/apache/cloudstack/api/command/admin/account/CreateAccountCmd.java +++ b/api/src/org/apache/cloudstack/api/command/admin/account/CreateAccountCmd.java @@ -19,6 +19,7 @@ package org.apache.cloudstack.api.command.admin.account; import java.util.Collection; import java.util.Map; +import org.apache.commons.lang.StringUtils; import org.apache.log4j.Logger; import org.apache.cloudstack.api.APICommand; @@ -31,7 +32,6 @@ import org.apache.cloudstack.api.ServerApiException; import org.apache.cloudstack.api.response.AccountResponse; import org.apache.cloudstack.api.response.DomainResponse; import org.apache.cloudstack.context.CallContext; -import org.apache.commons.lang.StringUtils; import com.cloud.user.Account; import com.cloud.user.UserAccount; @@ -175,9 +175,7 @@ public class CreateAccountCmd extends BaseCmd { @Override public void execute() { - if (StringUtils.isEmpty(getPassword())) { - throw new ServerApiException(ApiErrorCode.PARAM_ERROR, "Empty passwords are not allowed"); - } + validateParams(); CallContext.current().setEventDetails("Account Name: " + getAccountName() + ", Domain Id:" + getDomainId()); UserAccount userAccount = _accountService.createUserAccount(getUsername(), getPassword(), getFirstName(), getLastName(), getEmail(), getTimeZone(), getAccountName(), getAccountType(), @@ -190,4 +188,13 @@ public class CreateAccountCmd extends BaseCmd { throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, "Failed to create a user account"); } } + + /** + * TODO: this should be done through a validator. for now replicating the validation logic in create account and user + */ + private void validateParams() { + if(StringUtils.isEmpty(getPassword())) { + throw new ServerApiException(ApiErrorCode.PARAM_ERROR, "Empty passwords are not allowed"); + } + } } http://git-wip-us.apache.org/repos/asf/cloudstack/blob/1865433e/api/src/org/apache/cloudstack/api/command/admin/user/CreateUserCmd.java ---------------------------------------------------------------------- diff --git a/api/src/org/apache/cloudstack/api/command/admin/user/CreateUserCmd.java b/api/src/org/apache/cloudstack/api/command/admin/user/CreateUserCmd.java index 122fd43..71d6a66 100644 --- a/api/src/org/apache/cloudstack/api/command/admin/user/CreateUserCmd.java +++ b/api/src/org/apache/cloudstack/api/command/admin/user/CreateUserCmd.java @@ -150,9 +150,7 @@ public class CreateUserCmd extends BaseCmd { @Override public void execute() { - if (StringUtils.isEmpty(getPassword())) { - throw new ServerApiException(ApiErrorCode.PARAM_ERROR, "Empty passwords are not allowed"); - } + validateParams(); CallContext.current().setEventDetails("UserName: " + getUserName() + ", FirstName :" + getFirstName() + ", LastName: " + getLastName()); User user = _accountService.createUser(getUserName(), getPassword(), getFirstName(), getLastName(), getEmail(), getTimezone(), getAccountName(), getDomainId(), @@ -165,4 +163,13 @@ public class CreateUserCmd extends BaseCmd { throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, "Failed to create a user"); } } + + /** + * TODO: this should be done through a validator. for now replicating the validation logic in create account and user + */ + private void validateParams() { + if(StringUtils.isEmpty(getPassword())) { + throw new ServerApiException(ApiErrorCode.PARAM_ERROR, "Empty passwords are not allowed"); + } + } } http://git-wip-us.apache.org/repos/asf/cloudstack/blob/1865433e/api/test/org/apache/cloudstack/api/command/admin/account/CreateAccountCmdTest.java ---------------------------------------------------------------------- diff --git a/api/test/org/apache/cloudstack/api/command/admin/account/CreateAccountCmdTest.java b/api/test/org/apache/cloudstack/api/command/admin/account/CreateAccountCmdTest.java new file mode 100644 index 0000000..c0a046d --- /dev/null +++ b/api/test/org/apache/cloudstack/api/command/admin/account/CreateAccountCmdTest.java @@ -0,0 +1,100 @@ +/* + * 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.admin.account; + +import org.apache.cloudstack.api.ApiErrorCode; +import org.apache.cloudstack.api.ServerApiException; +import org.apache.cloudstack.context.CallContext; +import org.apache.log4j.Logger; +import org.junit.After; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.MockitoAnnotations; +import org.springframework.test.util.ReflectionTestUtils; + +import com.cloud.user.Account; +import com.cloud.user.AccountService; +import com.cloud.user.User; + +public class CreateAccountCmdTest { + public static final Logger s_logger = Logger.getLogger(CreateAccountCmdTest.class.getName()); + + @Mock + private AccountService accountService; + + @InjectMocks + private CreateAccountCmd createAccountCmd = new CreateAccountCmd(); + + private short accountType = 1; + private Long domainId = 1L; + + @Before + public void setUp() throws Exception { + MockitoAnnotations.initMocks(this); + ReflectionTestUtils.setField(createAccountCmd, "domainId", domainId); + ReflectionTestUtils.setField(createAccountCmd, "accountType", accountType); + CallContext.register(Mockito.mock(User.class), Mockito.mock(Account.class)); + } + + @After + public void tearDown() throws Exception { + CallContext.unregister(); + } + + @Test + public void testExecuteWithNotBlankPassword() { + ReflectionTestUtils.setField(createAccountCmd, "password", "Test"); + try { + createAccountCmd.execute(); + } catch (ServerApiException e) { + Assert.assertTrue("Received exception as the mock accountService createUserAccount returns null user", true); + } + Mockito.verify(accountService, Mockito.times(1)).createUserAccount(null, "Test", null, null, null, null, null, accountType, domainId, null, null, null, null); + } + + @Test + public void testExecuteWithNullPassword() { + ReflectionTestUtils.setField(createAccountCmd, "password", null); + try { + createAccountCmd.execute(); + Assert.fail("should throw exception for a null password"); + } catch (ServerApiException e) { + Assert.assertEquals(ApiErrorCode.PARAM_ERROR, e.getErrorCode()); + Assert.assertEquals("Empty passwords are not allowed", e.getMessage()); + } + Mockito.verify(accountService, Mockito.never()).createUserAccount(null, null, null, null, null, null, null, accountType, domainId, null, null, null, null); + } + + @Test + public void testExecuteWithEmptyPassword() { + ReflectionTestUtils.setField(createAccountCmd, "password", ""); + try { + createAccountCmd.execute(); + Assert.fail("should throw exception for a empty password"); + } catch (ServerApiException e) { + Assert.assertEquals(ApiErrorCode.PARAM_ERROR, e.getErrorCode()); + Assert.assertEquals("Empty passwords are not allowed", e.getMessage()); + } + Mockito.verify(accountService, Mockito.never()).createUserAccount(null, null, null, null, null, null, null, accountType, domainId, null, null, null, null); + } +} http://git-wip-us.apache.org/repos/asf/cloudstack/blob/1865433e/api/test/org/apache/cloudstack/api/command/admin/user/CreateUserCmdTest.java ---------------------------------------------------------------------- diff --git a/api/test/org/apache/cloudstack/api/command/admin/user/CreateUserCmdTest.java b/api/test/org/apache/cloudstack/api/command/admin/user/CreateUserCmdTest.java new file mode 100644 index 0000000..cde3b2d --- /dev/null +++ b/api/test/org/apache/cloudstack/api/command/admin/user/CreateUserCmdTest.java @@ -0,0 +1,96 @@ +/* + * 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.admin.user; + +import org.apache.cloudstack.api.ApiErrorCode; +import org.apache.cloudstack.api.ServerApiException; +import org.apache.cloudstack.context.CallContext; +import org.apache.log4j.Logger; +import org.junit.After; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.MockitoAnnotations; +import org.springframework.test.util.ReflectionTestUtils; + +import com.cloud.user.Account; +import com.cloud.user.AccountService; +import com.cloud.user.User; + +public class CreateUserCmdTest { + public static final Logger s_logger = Logger.getLogger(CreateUserCmdTest.class.getName()); + + @Mock + private AccountService accountService; + + @InjectMocks + private CreateUserCmd createUserCmd = new CreateUserCmd(); + + @Before + public void setUp() throws Exception { + MockitoAnnotations.initMocks(this); + CallContext.register(Mockito.mock(User.class), Mockito.mock(Account.class)); + } + + @After + public void tearDown() throws Exception { + CallContext.unregister(); + } + + @Test + public void testExecuteWithNotBlankPassword() { + ReflectionTestUtils.setField(createUserCmd, "password", "Test"); + try { + createUserCmd.execute(); + } catch (ServerApiException e) { + Assert.assertTrue("Received exception as the mock accountService createUser returns null user", true); + } + Mockito.verify(accountService, Mockito.times(1)).createUser(null, "Test", null, null, null, null, null, null, null); + } + + @Test + public void testExecuteWithNullPassword() { + ReflectionTestUtils.setField(createUserCmd, "password", null); + try { + createUserCmd.execute(); + Assert.fail("should throw exception for a null password"); + } catch (ServerApiException e) { + Assert.assertEquals(ApiErrorCode.PARAM_ERROR,e.getErrorCode()); + Assert.assertEquals("Empty passwords are not allowed", e.getMessage()); + } + Mockito.verify(accountService, Mockito.never()).createUser(null, null, null, null, null, null, null, null, null); + } + + @Test + public void testExecuteWithEmptyPassword() { + ReflectionTestUtils.setField(createUserCmd, "password", ""); + try { + createUserCmd.execute(); + Assert.fail("should throw exception for a empty password"); + } catch (ServerApiException e) { + Assert.assertEquals(ApiErrorCode.PARAM_ERROR,e.getErrorCode()); + Assert.assertEquals("Empty passwords are not allowed", e.getMessage()); + } + Mockito.verify(accountService, Mockito.never()).createUser(null, null, null, null, null, null, null, null, null); + } +} http://git-wip-us.apache.org/repos/asf/cloudstack/blob/1865433e/server/test/com/cloud/user/AccountManagerImplTest.java ---------------------------------------------------------------------- diff --git a/server/test/com/cloud/user/AccountManagerImplTest.java b/server/test/com/cloud/user/AccountManagerImplTest.java index f70aa39..a895ec2 100644 --- a/server/test/com/cloud/user/AccountManagerImplTest.java +++ b/server/test/com/cloud/user/AccountManagerImplTest.java @@ -17,11 +17,15 @@ package com.cloud.user; import java.lang.reflect.Field; +import java.net.InetAddress; +import java.net.UnknownHostException; import java.util.Arrays; import java.util.ArrayList; import javax.inject.Inject; +import com.cloud.server.auth.UserAuthenticator; +import com.cloud.utils.Pair; import org.junit.After; import org.junit.Assert; import org.junit.Before; @@ -88,6 +92,7 @@ import com.cloud.vm.dao.DomainRouterDao; import com.cloud.vm.dao.InstanceGroupDao; import com.cloud.vm.dao.UserVmDao; import com.cloud.vm.dao.VMInstanceDao; +import org.springframework.test.util.ReflectionTestUtils; @RunWith(MockitoJUnitRunner.class) public class AccountManagerImplTest { @@ -197,6 +202,9 @@ public class AccountManagerImplTest { @Mock SecurityChecker securityChecker; + @Mock + private UserAuthenticator userAuthenticator; + @Before public void setup() throws NoSuchFieldException, SecurityException, IllegalArgumentException, IllegalAccessException { @@ -213,6 +221,7 @@ public class AccountManagerImplTest { } } } + ReflectionTestUtils.setField(accountManager, "_userAuthenticators", Arrays.asList(userAuthenticator)); accountManager.setSecurityCheckers(Arrays.asList(securityChecker)); CallContext.register(callingUser, callingAccount); } @@ -312,4 +321,38 @@ public class AccountManagerImplTest { Mockito.verify(_accountDao, Mockito.atLeastOnce()).markForCleanup( Mockito.eq(42l)); } + + + @Test + public void testAuthenticateUser() throws UnknownHostException { + Pair successAuthenticationPair = new Pair<>(true, null); + Pair failureAuthenticationPair = new Pair<>(false, + UserAuthenticator.ActionOnFailedAuthentication.INCREMENT_INCORRECT_LOGIN_ATTEMPT_COUNT); + + UserAccountVO userAccountVO = new UserAccountVO(); + userAccountVO.setSource(User.Source.UNKNOWN); + userAccountVO.setState(Account.State.disabled.toString()); + Mockito.when(_userAccountDao.getUserAccount("test", 1L)).thenReturn(userAccountVO); + Mockito.when(userAuthenticator.authenticate("test", "fail", 1L, null)).thenReturn(failureAuthenticationPair); + Mockito.when(userAuthenticator.authenticate("test", null, 1L, null)).thenReturn(successAuthenticationPair); + Mockito.when(userAuthenticator.authenticate("test", "", 1L, null)).thenReturn(successAuthenticationPair); + + //Test for incorrect password. authentication should fail + UserAccount userAccount = accountManager.authenticateUser("test", "fail", 1L, InetAddress.getByName("127.0.0.1"), null); + Assert.assertNull(userAccount); + + //Test for null password. authentication should fail + userAccount = accountManager.authenticateUser("test", null, 1L, InetAddress.getByName("127.0.0.1"), null); + Assert.assertNull(userAccount); + + //Test for empty password. authentication should fail + userAccount = accountManager.authenticateUser("test", "", 1L, InetAddress.getByName("127.0.0.1"), null); + Assert.assertNull(userAccount); + + //Verifying that the authentication method is only called when password is specified + Mockito.verify(userAuthenticator, Mockito.times(1)).authenticate("test", "fail", 1L, null); + Mockito.verify(userAuthenticator, Mockito.never()).authenticate("test", null, 1L, null); + Mockito.verify(userAuthenticator, Mockito.never()).authenticate("test", "", 1L, null); + + } }