Skip to content

Commit 993e211

Browse files
committed
1. throw exception when force reset password is done for locked/disabled user/account
2. ui validation on current and new password being same 3. allow enforce change password for add user until saml is not enabled
1 parent e47dee8 commit 993e211

5 files changed

Lines changed: 67 additions & 15 deletions

File tree

server/src/main/java/com/cloud/user/AccountManagerImpl.java

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1607,17 +1607,22 @@ public UserAccount updateUser(UpdateUserCmd updateUserCmd) {
16071607
if (mandate2FA != null && mandate2FA) {
16081608
user.setUser2faEnabled(true);
16091609
}
1610-
updatePasswordChangeRequired(caller, updateUserCmd, user);
1610+
validateAndUpdatePasswordChangeRequired(caller, updateUserCmd, user, account);
16111611
_userDao.update(user.getId(), user);
16121612
return _userAccountDao.findById(user.getId());
16131613
}
16141614

1615-
private void updatePasswordChangeRequired(User caller, UpdateUserCmd updateUserCmd, UserVO user) {
1616-
User.Source userSource = user.getSource();
1617-
if ((userSource == User.Source.SAML2 || userSource == User.Source.SAML2DISABLED || userSource == User.Source.LDAP)
1618-
&& updateUserCmd.isPasswordChangeRequired()) {
1619-
logger.warn("Enforcing password change is not permitted for source [{}].", user.getSource());
1620-
throw new InvalidParameterValueException("CloudStack does not support enforcing password change for SAML or LDAP users.");
1615+
private void validateAndUpdatePasswordChangeRequired(User caller, UpdateUserCmd updateUserCmd, UserVO user, Account account) {
1616+
if (updateUserCmd.isPasswordChangeRequired()) {
1617+
if (user.getState() != State.ENABLED || account.getState() != State.ENABLED) {
1618+
throw new CloudRuntimeException("CloudStack does not support enforcing password change for locked/disabled User or Account.");
1619+
}
1620+
1621+
User.Source userSource = user.getSource();
1622+
if (userSource == User.Source.SAML2 || userSource == User.Source.SAML2DISABLED || userSource == User.Source.LDAP) {
1623+
logger.warn("Enforcing password change is not permitted for source [{}].", user.getSource());
1624+
throw new InvalidParameterValueException("CloudStack does not support enforcing password change for SAML or LDAP users.");
1625+
}
16211626
}
16221627

16231628
boolean isCallerSameAsUser = user.getId() == caller.getId();

server/src/test/java/com/cloud/user/AccountManagerImplTest.java

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -433,8 +433,33 @@ public void updateUserTestTimeZoneAndEmailNotNull() {
433433
prepareMockAndExecuteUpdateUserTest(1);
434434
}
435435

436+
@Test(expected = CloudRuntimeException.class)
437+
public void updateUserTestPwdChangeDisabledUser() {
438+
Mockito.when(userVoMock.getState()).thenReturn(State.DISABLED);
439+
updateUserPwdChange();
440+
}
441+
442+
@Test(expected = CloudRuntimeException.class)
443+
public void updateUserTestPwdChangeLockedUser() {
444+
Mockito.when(userVoMock.getState()).thenReturn(State.LOCKED);
445+
updateUserPwdChange();
446+
}
447+
448+
@Test(expected = CloudRuntimeException.class)
449+
public void updateUserTestPwdChangeDisabledAccount() {
450+
Mockito.when(userVoMock.getState()).thenReturn(State.ENABLED);
451+
Mockito.when(accountMock.getState()).thenReturn(State.LOCKED);
452+
updateUserPwdChange();
453+
}
454+
436455
@Test
437-
public void updateUserTestPwdChange() {
456+
public void testUpdateUserTestPwdChange() {
457+
Mockito.when(userVoMock.getState()).thenReturn(State.ENABLED);
458+
Mockito.when(accountMock.getState()).thenReturn(State.ENABLED);
459+
updateUserPwdChange();
460+
}
461+
462+
private void updateUserPwdChange() {
438463
Mockito.doReturn(true).when(UpdateUserCmdMock).isPasswordChangeRequired();
439464
Mockito.when(userVoMock.getAccountId()).thenReturn(10L);
440465
Mockito.doReturn(accountMock).when(accountManagerImpl).getAccount(10L);

ui/public/locales/en.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3481,6 +3481,7 @@
34813481
"message.error.netmask": "Please enter Netmask.",
34823482
"message.error.network.offering": "Please select Network offering.",
34833483
"message.error.new.password": "Please enter new password.",
3484+
"message.error.newpassword.sameascurrent": "New password cannot be the same as the current password.",
34843485
"message.error.nexus1000v.ipaddress": "Please enter Nexus 1000v IP address.",
34853486
"message.error.nexus1000v.password": "Please enter Nexus 1000v password.",
34863487
"message.error.nexus1000v.username": "Please enter Nexus 1000v username.",

ui/src/views/iam/AddUser.vue

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -133,16 +133,16 @@
133133
</a-select-option>
134134
</a-select>
135135
</a-form-item>
136-
<a-form-item v-if="isAdminOrDomainAdmin() && !samlAllowed" name="passwordChangeRequired" ref="passwordChangeRequired">
136+
<a-form-item v-if="isAdminOrDomainAdmin() && !samlEnable" name="passwordChangeRequired" ref="passwordChangeRequired">
137137
<a-checkbox v-model:checked="form.passwordChangeRequired">
138138
{{ $t('label.change.password.onlogin') }}
139139
</a-checkbox>
140140
</a-form-item>
141141
<div v-if="samlAllowed">
142-
<a-form-item name="samlenable" ref="samlenable" :label="$t('label.samlenable')">
143-
<a-switch v-model:checked="form.samlenable" />
142+
<a-form-item name="samlEnable" ref="samlEnable" :label="$t('label.samlenable')">
143+
<a-switch v-model:checked="samlEnable" />
144144
</a-form-item>
145-
<a-form-item name="samlentity" ref="samlentity" v-if="form.samlenable">
145+
<a-form-item name="samlentity" ref="samlentity" v-if="samlEnable">
146146
<template #label>
147147
<tooltip-label :title="$t('label.samlentity')" :tooltip="apiParams.entityid.description"/>
148148
</template>
@@ -203,6 +203,13 @@ export default {
203203
this.initForm()
204204
this.fetchData()
205205
},
206+
watch: {
207+
samlEnable (newVal) {
208+
if (newVal) {
209+
this.form.passwordChangeRequired = false
210+
}
211+
}
212+
},
206213
computed: {
207214
samlAllowed () {
208215
return 'authorizeSamlSso' in this.$store.getters.apis
@@ -296,9 +303,9 @@ export default {
296303
})
297304
298305
const user = userCreationResponse?.createuserresponse?.user
299-
if (values.samlenable && user) {
306+
if (this.samlEnable && user) {
300307
await postAPI('authorizeSamlSso', {
301-
enable: values.samlenable,
308+
enable: this.samlEnable,
302309
entityid: values.samlentity,
303310
userid: user.id
304311
})

ui/src/views/iam/ForceChangePassword.vue

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,10 @@ export default {
130130
rules () {
131131
return {
132132
currentpassword: [{ required: true, message: this.$t('message.error.current.password') }],
133-
password: [{ required: true, message: this.$t('message.error.new.password') }],
133+
password: [
134+
{ required: true, message: this.$t('message.error.new.password') },
135+
{ validator: this.validateNewPassword, trigger: 'change' }
136+
],
134137
confirmpassword: [
135138
{ required: true, message: this.$t('message.error.confirm.password') },
136139
{ validator: this.validateTwoPassword, trigger: 'change' }
@@ -139,6 +142,17 @@ export default {
139142
}
140143
},
141144
methods: {
145+
async validateNewPassword (rule, value) {
146+
const currentPassword = this.form.currentpassword
147+
if (!value || value.length === 0) {
148+
return Promise.resolve()
149+
}
150+
// Ensure new password is different from current password
151+
if (currentPassword && value === currentPassword) {
152+
return Promise.reject(this.$t('message.error.newpassword.sameascurrent'))
153+
}
154+
return Promise.resolve()
155+
},
142156
async validateTwoPassword (rule, value) {
143157
if (!value || value.length === 0) {
144158
return Promise.resolve()

0 commit comments

Comments
 (0)