You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by wi...@apache.org on 2015/09/11 14:54:49 UTC
[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 <ra...@gmail.com>
Authored: Tue Apr 1 11:32:33 2014 +0530
Committer: Rajani Karuturi <ra...@citrix.com>
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<Boolean, UserAuthenticator.ActionOnFailedAuthentication> successAuthenticationPair = new Pair<>(true, null);
+ Pair<Boolean, UserAuthenticator.ActionOnFailedAuthentication> 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);
+
+ }
}
[2/2] git commit: updated refs/heads/master to 5812f71
Posted by wi...@apache.org.
Merge pull request #807 from karuturi/createuser-unittests
unittests to verify empty password is not allowed during account/user create
* pr/807:
unittests to verify empty password is not allowed during account create
Signed-off-by: Wido den Hollander <wi...@widodh.nl>
Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo
Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/5812f714
Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/5812f714
Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/5812f714
Branch: refs/heads/master
Commit: 5812f714fcb96566ada96736b81857017a9ef0ca
Parents: 6330ef0 1865433
Author: Wido den Hollander <wi...@widodh.nl>
Authored: Fri Sep 11 14:54:47 2015 +0200
Committer: Wido den Hollander <wi...@widodh.nl>
Committed: Fri Sep 11 14:54:47 2015 +0200
----------------------------------------------------------------------
.../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(-)
----------------------------------------------------------------------