An omitted password generates a random one instead of failing, since an account that will + * only ever authenticate externally (SAML/LDAP) has no need for the admin to set one. A password + * that is passed but blank is still rejected, as that is more likely a client error. */ private void validateParams() { - if(StringUtils.isEmpty(getPassword())) { - throw new ServerApiException(ApiErrorCode.PARAM_ERROR, "Empty passwords are not allowed"); + if (getPassword() == null) { + password = PasswordGenerator.generateRandomPassword(GENERATED_PASSWORD_LENGTH); + } else if (StringUtils.isBlank(getPassword())) { + throw new ServerApiException(ApiErrorCode.PARAM_ERROR, "Blank passwords are not allowed"); } if (getAccountType() == null && (getRoleId() == null || getRoleId() < 1L)) { throw new ServerApiException(ApiErrorCode.PARAM_ERROR, "Neither account type and role ID are not provided"); diff --git a/api/src/test/java/org/apache/cloudstack/api/command/admin/account/CreateAccountCmdTest.java b/api/src/test/java/org/apache/cloudstack/api/command/admin/account/CreateAccountCmdTest.java index 365646de7a33..b72062b5d59b 100644 --- a/api/src/test/java/org/apache/cloudstack/api/command/admin/account/CreateAccountCmdTest.java +++ b/api/src/test/java/org/apache/cloudstack/api/command/admin/account/CreateAccountCmdTest.java @@ -20,7 +20,10 @@ import org.apache.cloudstack.acl.RoleService; import org.apache.cloudstack.api.ApiErrorCode; +import org.apache.cloudstack.api.ResponseGenerator; +import org.apache.cloudstack.api.ResponseObject.ResponseView; import org.apache.cloudstack.api.ServerApiException; +import org.apache.cloudstack.api.response.AccountResponse; import org.apache.cloudstack.context.CallContext; import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.LogManager; @@ -37,6 +40,7 @@ import com.cloud.user.Account; import com.cloud.user.AccountService; import com.cloud.user.User; +import com.cloud.user.UserAccount; public class CreateAccountCmdTest { protected Logger logger = LogManager.getLogger(getClass()); @@ -45,6 +49,8 @@ public class CreateAccountCmdTest { private AccountService accountService; @Mock private RoleService roleService; + @Mock + private ResponseGenerator responseGenerator; @InjectMocks private CreateAccountCmd createAccountCmd = new CreateAccountCmd(); @@ -81,28 +87,33 @@ public void testExecuteWithNotBlankPassword() { } @Test - public void testExecuteWithNullPassword() { + public void testExecuteWithNullPasswordGeneratesOne() { 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(createAccountCmd); + UserAccount userAccount = Mockito.mock(UserAccount.class); + Mockito.when(accountService.createUserAccount(createAccountCmd)).thenReturn(userAccount); + Mockito.when(responseGenerator.createUserAccountResponse(ResponseView.Full, userAccount)).thenReturn(Mockito.mock(AccountResponse.class)); + + createAccountCmd.execute(); + + Assert.assertEquals("a password should be generated for accounts that authenticate externally", + 12, createAccountCmd.getPassword().length()); + Mockito.verify(accountService, Mockito.times(1)).createUserAccount(createAccountCmd); } @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()); - } + ServerApiException e = Assert.assertThrows(ServerApiException.class, () -> createAccountCmd.execute()); + Assert.assertEquals(ApiErrorCode.PARAM_ERROR, e.getErrorCode()); + Assert.assertEquals("Blank passwords are not allowed", e.getMessage()); + Mockito.verify(accountService, Mockito.never()).createUserAccount(createAccountCmd); + } + + @Test + public void testExecuteWithWhitespacePassword() { + ReflectionTestUtils.setField(createAccountCmd, "password", " "); + ServerApiException e = Assert.assertThrows(ServerApiException.class, () -> createAccountCmd.execute()); + Assert.assertEquals(ApiErrorCode.PARAM_ERROR, e.getErrorCode()); Mockito.verify(accountService, Mockito.never()).createUserAccount(createAccountCmd); } } diff --git a/ui/public/locales/en.json b/ui/public/locales/en.json index 0acafc3b8015..c9dc2a9f6afa 100644 --- a/ui/public/locales/en.json +++ b/ui/public/locales/en.json @@ -4077,6 +4077,7 @@ "message.restart.vpc": "Please confirm that you want to restart the VPC.", "message.restart.vpc.remark": "Please confirm that you want to restart the VPC
Remark: making a non-redundant VPC redundant will force a clean up. The Networks will not be available for a couple of minutes.
", "message.running.custom.action": "Running action", +"message.saml.account.no.password": "This account will authenticate via SAML SSO, so no password is needed — one will be generated automatically.", "message.scale.processing": "Scale in progress", "message.scaledown.policies": "Please add at least a ScaleDown policy. The AutoScale Group will be scaled down when all conditions in a ScaleDown policy are matched. ScaleDown policies will be checked after ScaleUp policies.", "message.scaledown.policy.continue": "Please add at least condition to ScaleDown policy to continue", diff --git a/ui/src/views/iam/AddAccount.vue b/ui/src/views/iam/AddAccount.vue index a211fd2d8c3d..f95836362d43 100644 --- a/ui/src/views/iam/AddAccount.vue +++ b/ui/src/views/iam/AddAccount.vue @@ -53,7 +53,7 @@ v-model:value="form.username" :placeholder="apiParams.username.description" /> -