Skip to content

Commit 46ca0dd

Browse files
author
Daan Hoogland
committed
make password an optional parameter/field in createAccount
1 parent 5e5ae0c commit 46ca0dd

4 files changed

Lines changed: 37 additions & 17 deletions

File tree

api/src/main/java/org/apache/cloudstack/api/command/admin/account/CreateAccountCmd.java

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737

3838
import com.cloud.user.Account;
3939
import com.cloud.user.UserAccount;
40+
import com.cloud.utils.PasswordGenerator;
4041

4142

4243
@APICommand(name = "createAccount", description = "Creates an account", responseObject = AccountResponse.class, entityType = {Account.class},
@@ -75,8 +76,8 @@ public class CreateAccountCmd extends BaseCmd {
7576

7677
@Parameter(name = ApiConstants.PASSWORD,
7778
type = CommandType.STRING,
78-
required = true,
79-
description = "Clear text password (Default hashed to SHA256SALT). If you wish to use any other hashing algorithm, you would need to write a custom authentication adapter See Docs section.")
79+
description = "Clear text password (Default hashed to SHA256SALT). If you wish to use any other hashing algorithm, you would need to write a custom authentication adapter See Docs section. "
80+
+ "If omitted, a random password is generated, e.g. for an account that will only ever authenticate externally via SAML/LDAP.")
8081
private String password;
8182

8283
@Parameter(name = ApiConstants.TIMEZONE,
@@ -191,10 +192,13 @@ public void execute() {
191192

192193
/**
193194
* TODO: this should be done through a validator. for now replicating the validation logic in create account and user
195+
*
196+
* <p>A blank password generates a random one instead of failing, since an account that will
197+
* only ever authenticate externally (SAML/LDAP) has no need for the admin to set one.
194198
*/
195199
private void validateParams() {
196-
if(StringUtils.isEmpty(getPassword())) {
197-
throw new ServerApiException(ApiErrorCode.PARAM_ERROR, "Empty passwords are not allowed");
200+
if (StringUtils.isEmpty(getPassword())) {
201+
password = PasswordGenerator.generateRandomPassword(12);
198202
}
199203
if (getAccountType() == null && (getRoleId() == null || getRoleId() < 1L)) {
200204
throw new ServerApiException(ApiErrorCode.PARAM_ERROR, "Neither account type and role ID are not provided");

api/src/test/java/org/apache/cloudstack/api/command/admin/account/CreateAccountCmdTest.java

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919
package org.apache.cloudstack.api.command.admin.account;
2020

2121
import org.apache.cloudstack.acl.RoleService;
22-
import org.apache.cloudstack.api.ApiErrorCode;
2322
import org.apache.cloudstack.api.ServerApiException;
2423
import org.apache.cloudstack.context.CallContext;
2524
import org.apache.logging.log4j.Logger;
@@ -81,28 +80,26 @@ public void testExecuteWithNotBlankPassword() {
8180
}
8281

8382
@Test
84-
public void testExecuteWithNullPassword() {
83+
public void testExecuteWithNullPasswordGeneratesOne() {
8584
ReflectionTestUtils.setField(createAccountCmd, "password", null);
8685
try {
8786
createAccountCmd.execute();
88-
Assert.fail("should throw exception for a null password");
8987
} catch (ServerApiException e) {
90-
Assert.assertEquals(ApiErrorCode.PARAM_ERROR, e.getErrorCode());
91-
Assert.assertEquals("Empty passwords are not allowed", e.getMessage());
88+
Assert.assertTrue("Received exception as the mock accountService createUserAccount returns null user", true);
9289
}
93-
Mockito.verify(accountService, Mockito.never()).createUserAccount(createAccountCmd);
90+
Assert.assertNotNull("a password should be generated for accounts that authenticate externally", createAccountCmd.getPassword());
91+
Mockito.verify(accountService, Mockito.times(1)).createUserAccount(createAccountCmd);
9492
}
9593

9694
@Test
97-
public void testExecuteWithEmptyPassword() {
95+
public void testExecuteWithEmptyPasswordGeneratesOne() {
9896
ReflectionTestUtils.setField(createAccountCmd, "password", "");
9997
try {
10098
createAccountCmd.execute();
101-
Assert.fail("should throw exception for a empty password");
10299
} catch (ServerApiException e) {
103-
Assert.assertEquals(ApiErrorCode.PARAM_ERROR, e.getErrorCode());
104-
Assert.assertEquals("Empty passwords are not allowed", e.getMessage());
100+
Assert.assertTrue("Received exception as the mock accountService createUserAccount returns null user", true);
105101
}
106-
Mockito.verify(accountService, Mockito.never()).createUserAccount(createAccountCmd);
102+
Assert.assertNotNull("a password should be generated for accounts that authenticate externally", createAccountCmd.getPassword());
103+
Mockito.verify(accountService, Mockito.times(1)).createUserAccount(createAccountCmd);
107104
}
108105
}

ui/public/locales/en.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3982,6 +3982,7 @@
39823982
"message.restart.vpc": "Please confirm that you want to restart the VPC.",
39833983
"message.restart.vpc.remark": "Please confirm that you want to restart the VPC <p><i>Remark: making a non-redundant VPC redundant will force a clean up. The Networks will not be available for a couple of minutes</i>.</p>",
39843984
"message.running.custom.action": "Running action",
3985+
"message.saml.account.no.password": "This account will authenticate via SAML SSO, so no password is needed — one will be generated automatically.",
39853986
"message.scale.processing": "Scale in progress",
39863987
"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.",
39873988
"message.scaledown.policy.continue": "Please add at least condition to ScaleDown policy to continue",

ui/src/views/iam/AddAccount.vue

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@
5353
v-model:value="form.username"
5454
:placeholder="apiParams.username.description" />
5555
</a-form-item>
56-
<a-row :gutter="12">
56+
<a-row :gutter="12" v-if="!form.samlenable">
5757
<a-col :md="24" :lg="12">
5858
<a-form-item ref="password" name="password">
5959
<template #label>
@@ -75,6 +75,12 @@
7575
</a-form-item>
7676
</a-col>
7777
</a-row>
78+
<a-alert
79+
v-else
80+
type="info"
81+
show-icon
82+
:message="$t('message.saml.account.no.password')"
83+
style="margin-bottom: 12px;" />
7884
<a-form-item ref="email" name="email">
7985
<template #label>
8086
<tooltip-label :title="$t('label.email')" :tooltip="apiParams.email.description"/>
@@ -250,6 +256,14 @@ export default {
250256
}
251257
},
252258
immediate: false
259+
},
260+
'form.samlenable' (samlEnabled) {
261+
// a SAML-authenticated account never logs in with a native password
262+
this.rules.password = samlEnabled ? [] : [{ required: true, message: this.$t('message.error.required.input') }]
263+
this.rules.confirmpassword = samlEnabled ? [] : [
264+
{ required: true, message: this.$t('message.error.required.input') },
265+
{ validator: this.validateConfirmPassword }
266+
]
253267
}
254268
},
255269
methods: {
@@ -408,12 +422,16 @@ export default {
408422
const params = {
409423
roleid: values.roleid,
410424
username: values.username,
411-
password: values.password,
412425
email: values.email,
413426
firstname: values.firstname,
414427
lastname: values.lastname,
415428
domainid: values.domainid
416429
}
430+
if (!values.samlenable) {
431+
// SAML-authenticated accounts never log in with a native password; let the API
432+
// generate one rather than asking the admin to set one that will never be used
433+
params.password = values.password
434+
}
417435
if (this.isValidValueForKey(values, 'account') && values.account.length > 0) {
418436
params.account = values.account
419437
}

0 commit comments

Comments
 (0)