Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@

import com.cloud.user.Account;
import com.cloud.user.UserAccount;
import com.cloud.utils.PasswordGenerator;


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

@Parameter(name = ApiConstants.PASSWORD,
type = CommandType.STRING,
required = true,
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.")
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. "

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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. "
description = "Clear text password (Default hashed to SHA256SALT). If you wish to use any other hashing algorithm, you will need to write a custom authentication adapter See Docs section. "

"If you wish... you would need..." sounds weird IMO

+ "If omitted, a random password is generated, e.g. for an account that will only ever authenticate externally via SAML/LDAP.")
private String password;

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

/**
* TODO: this should be done through a validator. for now replicating the validation logic in create account and user
*
* <p>A blank 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.
*/
private void validateParams() {
if(StringUtils.isEmpty(getPassword())) {
throw new ServerApiException(ApiErrorCode.PARAM_ERROR, "Empty passwords are not allowed");
if (StringUtils.isEmpty(getPassword())) {
password = PasswordGenerator.generateRandomPassword(12);
}
if (getAccountType() == null && (getRoleId() == null || getRoleId() < 1L)) {
throw new ServerApiException(ApiErrorCode.PARAM_ERROR, "Neither account type and role ID are not provided");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
package org.apache.cloudstack.api.command.admin.account;

import org.apache.cloudstack.acl.RoleService;
import org.apache.cloudstack.api.ApiErrorCode;
import org.apache.cloudstack.api.ServerApiException;
import org.apache.cloudstack.context.CallContext;
import org.apache.logging.log4j.Logger;
Expand Down Expand Up @@ -81,28 +80,26 @@
}

@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());
Assert.assertTrue("Received exception as the mock accountService createUserAccount returns null user", true);
}
Mockito.verify(accountService, Mockito.never()).createUserAccount(createAccountCmd);
Assert.assertNotNull("a password should be generated for accounts that authenticate externally", createAccountCmd.getPassword());
Mockito.verify(accountService, Mockito.times(1)).createUserAccount(createAccountCmd);

Check warning on line 91 in api/src/test/java/org/apache/cloudstack/api/command/admin/account/CreateAccountCmdTest.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Use a static import for "times".

See more on https://sonarcloud.io/project/issues?id=apache_cloudstack&issues=AaArhVEOrd49U0dPX_rS&open=AaArhVEOrd49U0dPX_rS&pullRequest=13955

Check warning on line 91 in api/src/test/java/org/apache/cloudstack/api/command/admin/account/CreateAccountCmdTest.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Use a static import for "verify".

See more on https://sonarcloud.io/project/issues?id=apache_cloudstack&issues=AaArhVEOrd49U0dPX_rR&open=AaArhVEOrd49U0dPX_rR&pullRequest=13955
}

@Test
public void testExecuteWithEmptyPassword() {
public void testExecuteWithEmptyPasswordGeneratesOne() {
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());
Assert.assertTrue("Received exception as the mock accountService createUserAccount returns null user", true);
}
Mockito.verify(accountService, Mockito.never()).createUserAccount(createAccountCmd);
Assert.assertNotNull("a password should be generated for accounts that authenticate externally", createAccountCmd.getPassword());
Mockito.verify(accountService, Mockito.times(1)).createUserAccount(createAccountCmd);

Check warning on line 103 in api/src/test/java/org/apache/cloudstack/api/command/admin/account/CreateAccountCmdTest.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Use a static import for "times".

See more on https://sonarcloud.io/project/issues?id=apache_cloudstack&issues=AaArhVEOrd49U0dPX_rU&open=AaArhVEOrd49U0dPX_rU&pullRequest=13955

Check warning on line 103 in api/src/test/java/org/apache/cloudstack/api/command/admin/account/CreateAccountCmdTest.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Use a static import for "verify".

See more on https://sonarcloud.io/project/issues?id=apache_cloudstack&issues=AaArhVEOrd49U0dPX_rT&open=AaArhVEOrd49U0dPX_rT&pullRequest=13955
}
}
1 change: 1 addition & 0 deletions ui/public/locales/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -3982,6 +3982,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 <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>",
"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",
Expand Down
22 changes: 20 additions & 2 deletions ui/src/views/iam/AddAccount.vue
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@
v-model:value="form.username"
:placeholder="apiParams.username.description" />
</a-form-item>
<a-row :gutter="12">
<a-row :gutter="12" v-if="!form.samlenable">
<a-col :md="24" :lg="12">
<a-form-item ref="password" name="password">
<template #label>
Expand All @@ -75,6 +75,12 @@
</a-form-item>
</a-col>
</a-row>
<a-alert
v-else
type="info"
show-icon
:message="$t('message.saml.account.no.password')"
style="margin-bottom: 12px;" />
<a-form-item ref="email" name="email">
<template #label>
<tooltip-label :title="$t('label.email')" :tooltip="apiParams.email.description"/>
Expand Down Expand Up @@ -250,6 +256,14 @@ export default {
}
},
immediate: false
},
'form.samlenable' (samlEnabled) {
// a SAML-authenticated account never logs in with a native password
this.rules.password = samlEnabled ? [] : [{ required: true, message: this.$t('message.error.required.input') }]
this.rules.confirmpassword = samlEnabled ? [] : [
{ required: true, message: this.$t('message.error.required.input') },
{ validator: this.validateConfirmPassword }
]
}
},
methods: {
Expand Down Expand Up @@ -408,12 +422,16 @@ export default {
const params = {
roleid: values.roleid,
username: values.username,
password: values.password,
email: values.email,
firstname: values.firstname,
lastname: values.lastname,
domainid: values.domainid
}
if (!values.samlenable) {
// SAML-authenticated accounts never log in with a native password; let the API
// generate one rather than asking the admin to set one that will never be used
params.password = values.password
}
if (this.isValidValueForKey(values, 'account') && values.account.length > 0) {
params.account = values.account
}
Expand Down
Loading