Copilot commented on code in PR #12294:
URL: https://github.com/apache/cloudstack/pull/12294#discussion_r2630737215
##########
server/src/main/java/com/cloud/api/query/vo/UserAccountJoinVO.java:
##########
@@ -288,4 +291,8 @@ public boolean isUser2faEnabled() {
public Boolean getApiKeyAccess() {
return apiKeyAccess;
}
+
+ public Boolean isPasswordChangeRequired() {
Review Comment:
The method name isPasswordChangeRequired() returns a Boolean (potentially
null), but the naming convention suggests it should return a boolean primitive.
Additionally, the getter naming convention for Boolean fields should typically
be getPasswordChangeRequired() rather than isPasswordChangeRequired() to avoid
confusion, especially since the field can be null.
```suggestion
public Boolean getPasswordChangeRequired() {
```
##########
api/src/main/java/org/apache/cloudstack/api/response/UserResponse.java:
##########
@@ -317,4 +321,12 @@ public void set2FAmandated(Boolean is2FAmandated) {
public void setApiKeyAccess(Boolean apiKeyAccess) {
this.apiKeyAccess =
ApiConstants.ApiKeyAccess.fromBoolean(apiKeyAccess);
}
+
+ public Boolean isPasswordChangeRequired() {
+ return passwordChangeRequired;
Review Comment:
The method isPasswordChangeRequired() returns a Boolean (potentially null)
but uses the "is" prefix which is typically reserved for primitive boolean
types. For consistency and to avoid null-related issues, consider renaming to
getPasswordChangeRequired() or ensure the method returns a primitive boolean
with a null-safe check.
```suggestion
public boolean isPasswordChangeRequired() {
return Boolean.TRUE.equals(passwordChangeRequired);
```
##########
api/src/main/java/org/apache/cloudstack/api/response/UserResponse.java:
##########
@@ -132,6 +132,10 @@ public class UserResponse extends BaseResponse implements
SetResourceIconRespons
@Param(description = "whether api key access is Enabled, Disabled or set
to Inherit (it inherits the value from the parent)", since = "4.20.1.0")
ApiConstants.ApiKeyAccess apiKeyAccess;
+ @SerializedName(value = ApiConstants.PASSWORD_CHANGE_REQUIRED)
+ @Param(description = "Is User required to change password on next login.",
since = "4.23.0")
Review Comment:
The description text has an inconsistency. It says "Is User required" which
should be "Is user required" (lowercase 'user') to match the standard
capitalization pattern used in other API parameter descriptions. Additionally,
the description should be more grammatically correct: "Indicates whether the
user is required to change password on next login".
```suggestion
@Param(description = "Indicates whether the user is required to change
password on next login", since = "4.23.0")
```
##########
ui/src/views/iam/ForceChangePassword.vue:
##########
@@ -0,0 +1,284 @@
+// 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.
+
+<template>
+ <div class="user-layout-wrapper">
+ <div class="container">
+ <div class="user-layout-content">
+ <a-card :bordered="false" class="force-password-card">
+ <template #title>
+ <div style="text-align: center; font-size: 18px; font-weight:
bold;">
+ {{ $t('label.action.change.password') }}
+ </div>
+ <div v-if="!isSubmitted" style="text-align: center; font-size:
14px; color: #666; margin-top: 5px;">
+ {{ $t('message.change.password.required') }}
+ </div>
+ </template>
+ <a-spin :spinning="loading">
+ <div v-if="isSubmitted" class="success-state">
+ <div class="success-icon">✓</div>
+ <div class="success-text">
+ {{ $t('message.success.change.password') }}
+ </div>
+ <div class="success-subtext">
+ {{ $t('message.please.login.new.password') }}
+ </div>
+ <a-button
+ type="primary"
+ size="large"
+ block
+ @click="handleLogout"
+ style="margin-top: 20px;"
+ >
+ {{ $t('label.login') }}
+ </a-button>
+ </div>
+
+ <a-form
+ v-else
+ :ref="formRef"
+ :model="form"
+ :rules="rules"
+ layout="vertical"
+ @finish="handleSubmit"
+ v-ctrl-enter="handleSubmit"
+ >
+ <a-form-item name="currentpassword"
:label="$t('label.currentpassword')">
+ <a-input-password
+ v-model:value="form.currentpassword"
+ :placeholder="$t('label.currentpassword')"
+ size="large"
+ v-focus="true"
+ />
+ </a-form-item>
+
+ <a-form-item name="password" :label="$t('label.new.password')">
+ <a-input-password
+ v-model:value="form.password"
+ :placeholder="$t('label.new.password')"
+ size="large"
+ />
+ </a-form-item>
+
+ <a-form-item name="confirmpassword"
:label="$t('label.confirmpassword')">
+ <a-input-password
+ v-model:value="form.confirmpassword"
+ :placeholder="$t('label.confirmpassword')"
+ size="large"
+ />
+ </a-form-item>
+
+ <a-form-item>
+ <a-button
+ type="primary"
+ size="large"
+ block
+ :loading="loading"
+ @click="handleSubmit"
+ >
+ {{ $t('label.ok') }}
+ </a-button>
+ </a-form-item>
+
+ <div class="actions">
+ <a @click="handleLogout">{{ $t('label.logout') }}</a>
+ </div>
+ </a-form>
+ </a-spin>
+ </a-card>
+ </div>
+ </div>
+ </div>
+</template>
+
+<script>
+import { ref, reactive, toRaw } from 'vue'
+import { postAPI } from '@/api'
+import Cookies from 'js-cookie'
+import { PASSWORD_CHANGE_REQUIRED } from '@/store/mutation-types'
+
+export default {
+ name: 'ForceChangePassword',
+ data () {
+ return {
+ loading: false,
+ isSubmitted: false
+ }
+ },
+ created () {
+ this.formRef = ref()
+ this.form = reactive({})
+ this.isPasswordChangeRequired()
+ },
+ computed: {
+ rules () {
+ return {
+ currentpassword: [{ required: true, message:
this.$t('message.error.current.password') || 'Please enter current password' }],
+ password: [{ required: true, message:
this.$t('message.error.new.password') || 'Please enter new password' }],
+ confirmpassword: [
+ { required: true, message: this.$t('message.error.confirm.password')
|| 'Please confirm new password' },
+ { validator: this.validateTwoPassword, trigger: 'change' }
+ ]
+ }
+ }
+ },
+ methods: {
+ async validateTwoPassword (rule, value) {
+ if (!value || value.length === 0) {
+ return Promise.resolve()
+ } else if (rule.field === 'confirmpassword') {
+ const form = this.form
+ const messageConfirm = this.$t('message.validate.equalto')
+ const passwordVal = form.password
+ if (passwordVal && passwordVal !== value) {
+ return Promise.reject(messageConfirm)
+ } else {
+ return Promise.resolve()
+ }
+ } else {
+ return Promise.resolve()
+ }
+ },
+ handleSubmit (e) {
+ e.preventDefault()
+ if (this.loading) return
+ this.formRef.value.validate().then(() => {
+ this.loading = true
+ const values = toRaw(this.form)
+ const userId = Cookies.get('userid')
+
+ const params = {
+ id: userId,
+ password: values.password,
+ currentpassword: values.currentpassword
+ }
+ postAPI('updateUser', params).then(() => {
+ this.$message.success(this.$t('message.success.change.password'))
+ this.isSubmitted = true
+ }).catch(error => {
+ console.error(error)
+ this.$notification.error({
+ message: 'Error',
+ description: error.response?.data?.updateuserresponse?.errortext
|| 'Failed to update password'
Review Comment:
The error message fallback string 'Failed to update password' should be
extracted to a localization file for proper internationalization support.
Hard-coded error messages in fallbacks reduce the maintainability and
localization coverage of the application.
```suggestion
description: error.response?.data?.updateuserresponse?.errortext
|| this.$t('message.error.change.password')
```
##########
ui/src/views/iam/ChangeUserPassword.vue:
##########
@@ -134,6 +142,10 @@ export default {
if (this.isValidValueForKey(values, 'currentpassword') &&
values.currentpassword.length > 0) {
params.currentpassword = values.currentpassword
}
+
+ if (this.isAdminOrDomainAdmin && values.passwordChangeRequired) {
Review Comment:
The condition logic has a subtle issue. When isAdminOrDomainAdmin is called
on line 146, it's called as a method reference (missing parentheses). This
should be `this.isAdminOrDomainAdmin()` to actually invoke the method. Without
parentheses, this will cause a compilation error or unexpected behavior.
```suggestion
if (this.isAdminOrDomainAdmin() && values.passwordChangeRequired) {
```
##########
server/src/main/java/com/cloud/user/AccountManagerImpl.java:
##########
@@ -1580,9 +1582,30 @@ public UserAccount updateUser(UpdateUserCmd
updateUserCmd) {
user.setUser2faEnabled(true);
}
_userDao.update(user.getId(), user);
+ updatePasswordChangeRequired(caller, updateUserCmd, user);
return _userAccountDao.findById(user.getId());
}
+ private void updatePasswordChangeRequired(User caller, UpdateUserCmd
updateUserCmd, UserVO user) {
+ if (StringUtils.isNotBlank(updateUserCmd.getPassword()) &&
isNormalUser(user.getAccountId())) {
+ boolean isPasswordResetRequired =
updateUserCmd.isPasswordChangeRequired();
+ // Admins only can enforce passwordChangeRequired for user
+ if ((isRootAdmin(caller.getId()) ||
isDomainAdmin(caller.getAccountId()))) {
Review Comment:
In the updatePasswordChangeRequired method, both isRootAdmin and
isDomainAdmin are being called with different parameters (user.getId() vs
caller.getAccountId()). The isRootAdmin call uses user.getId(), but this should
likely be caller.getId() to check if the caller (not the user being updated) is
a root admin, since you're already checking `caller.getId() == user.getId()` in
the subsequent condition. The inconsistency could lead to incorrect
authorization checks.
```suggestion
if (isRootAdmin(caller.getAccountId()) ||
isDomainAdmin(caller.getAccountId())) {
```
##########
api/src/main/java/org/apache/cloudstack/api/command/admin/user/UpdateUserCmd.java:
##########
@@ -85,8 +88,11 @@ public class UpdateUserCmd extends BaseCmd {
"This parameter is only used to mandate 2FA, not to disable 2FA",
since = "4.18.0.0")
private Boolean mandate2FA;
- @Inject
- private RegionService _regionService;
+ @Parameter(name = ApiConstants.PASSWORD_CHANGE_REQUIRED,
+ type = CommandType.BOOLEAN,
+ description = "Provide true to mandate the User to reset password
on next login.",
Review Comment:
The description text ends without punctuation. For consistency with other
parameter descriptions in the API, consider ending with a period: "Provide true
to mandate the User to reset password on next login."
##########
plugins/api/discovery/src/main/java/org/apache/cloudstack/discovery/ApiDiscoveryServiceImpl.java:
##########
@@ -280,12 +283,23 @@ public ListResponse<? extends BaseResponse> listApis(User
user, String name) {
ReflectionToStringBuilderUtils.reflectOnlySelectedFields(account,
"accountName", "uuid")));
}
- if (role.getRoleType() == RoleType.Admin && role.getId() ==
RoleType.Admin.getId()) {
- logger.info(String.format("Account [%s] is Root Admin, all
APIs are allowed.",
-
ReflectionToStringBuilderUtils.reflectOnlySelectedFields(account,
"accountName", "uuid")));
+ // Limit APIs on first login requiring password change
+ UserAccount userAccount =
accountService.getUserAccountById(user.getId());
+ if (MapUtils.isNotEmpty(userAccount.getDetails()) &&
+
userAccount.getDetails().containsKey(UserDetailVO.PasswordChangeRequired)) {
+
+ String needPasswordChange =
userAccount.getDetails().get(UserDetailVO.PasswordChangeRequired);
+ if ("true".equalsIgnoreCase(needPasswordChange)) {
+ apisAllowed = Arrays.asList("login", "logout",
"updateUser", "listUsers", "listApis");
Review Comment:
The hard-coded list of allowed APIs when password change is required should
be extracted as a constant to improve maintainability and avoid duplication if
this list needs to be referenced elsewhere. Consider defining a constant like
`APIS_ALLOWED_FOR_PASSWORD_CHANGE` at the class level.
##########
ui/src/views/iam/ForceChangePassword.vue:
##########
@@ -0,0 +1,284 @@
+// 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.
+
+<template>
+ <div class="user-layout-wrapper">
+ <div class="container">
+ <div class="user-layout-content">
+ <a-card :bordered="false" class="force-password-card">
+ <template #title>
+ <div style="text-align: center; font-size: 18px; font-weight:
bold;">
+ {{ $t('label.action.change.password') }}
+ </div>
+ <div v-if="!isSubmitted" style="text-align: center; font-size:
14px; color: #666; margin-top: 5px;">
+ {{ $t('message.change.password.required') }}
+ </div>
+ </template>
+ <a-spin :spinning="loading">
+ <div v-if="isSubmitted" class="success-state">
+ <div class="success-icon">✓</div>
+ <div class="success-text">
+ {{ $t('message.success.change.password') }}
+ </div>
+ <div class="success-subtext">
+ {{ $t('message.please.login.new.password') }}
+ </div>
+ <a-button
+ type="primary"
+ size="large"
+ block
+ @click="handleLogout"
+ style="margin-top: 20px;"
+ >
+ {{ $t('label.login') }}
+ </a-button>
+ </div>
+
+ <a-form
+ v-else
+ :ref="formRef"
+ :model="form"
+ :rules="rules"
+ layout="vertical"
+ @finish="handleSubmit"
+ v-ctrl-enter="handleSubmit"
+ >
+ <a-form-item name="currentpassword"
:label="$t('label.currentpassword')">
+ <a-input-password
+ v-model:value="form.currentpassword"
+ :placeholder="$t('label.currentpassword')"
+ size="large"
+ v-focus="true"
+ />
+ </a-form-item>
+
+ <a-form-item name="password" :label="$t('label.new.password')">
+ <a-input-password
+ v-model:value="form.password"
+ :placeholder="$t('label.new.password')"
+ size="large"
+ />
+ </a-form-item>
+
+ <a-form-item name="confirmpassword"
:label="$t('label.confirmpassword')">
+ <a-input-password
+ v-model:value="form.confirmpassword"
+ :placeholder="$t('label.confirmpassword')"
+ size="large"
+ />
+ </a-form-item>
+
+ <a-form-item>
+ <a-button
+ type="primary"
+ size="large"
+ block
+ :loading="loading"
+ @click="handleSubmit"
+ >
+ {{ $t('label.ok') }}
+ </a-button>
+ </a-form-item>
+
+ <div class="actions">
+ <a @click="handleLogout">{{ $t('label.logout') }}</a>
+ </div>
+ </a-form>
+ </a-spin>
+ </a-card>
+ </div>
+ </div>
+ </div>
+</template>
+
+<script>
+import { ref, reactive, toRaw } from 'vue'
+import { postAPI } from '@/api'
+import Cookies from 'js-cookie'
+import { PASSWORD_CHANGE_REQUIRED } from '@/store/mutation-types'
+
+export default {
+ name: 'ForceChangePassword',
+ data () {
+ return {
+ loading: false,
+ isSubmitted: false
+ }
+ },
+ created () {
+ this.formRef = ref()
+ this.form = reactive({})
+ this.isPasswordChangeRequired()
+ },
+ computed: {
+ rules () {
+ return {
+ currentpassword: [{ required: true, message:
this.$t('message.error.current.password') || 'Please enter current password' }],
+ password: [{ required: true, message:
this.$t('message.error.new.password') || 'Please enter new password' }],
+ confirmpassword: [
+ { required: true, message: this.$t('message.error.confirm.password')
|| 'Please confirm new password' },
+ { validator: this.validateTwoPassword, trigger: 'change' }
+ ]
+ }
+ }
+ },
+ methods: {
+ async validateTwoPassword (rule, value) {
+ if (!value || value.length === 0) {
+ return Promise.resolve()
+ } else if (rule.field === 'confirmpassword') {
+ const form = this.form
+ const messageConfirm = this.$t('message.validate.equalto')
+ const passwordVal = form.password
+ if (passwordVal && passwordVal !== value) {
+ return Promise.reject(messageConfirm)
+ } else {
+ return Promise.resolve()
+ }
+ } else {
+ return Promise.resolve()
+ }
+ },
+ handleSubmit (e) {
+ e.preventDefault()
+ if (this.loading) return
+ this.formRef.value.validate().then(() => {
+ this.loading = true
+ const values = toRaw(this.form)
+ const userId = Cookies.get('userid')
+
+ const params = {
+ id: userId,
+ password: values.password,
+ currentpassword: values.currentpassword
+ }
+ postAPI('updateUser', params).then(() => {
+ this.$message.success(this.$t('message.success.change.password'))
+ this.isSubmitted = true
+ }).catch(error => {
+ console.error(error)
+ this.$notification.error({
+ message: 'Error',
+ description: error.response?.data?.updateuserresponse?.errortext
|| 'Failed to update password'
+ })
+ this.$message.error(this.$t('message.error.change.password'))
+ }).finally(() => {
+ this.loading = false
+ })
+ }).catch(error => {
+ console.log('Validation failed:', error)
+ })
+ },
+ async handleLogout () {
+ try {
+ await this.$store.dispatch('Logout')
+ } catch (e) {
+ console.error('Logout failed:', e)
+ } finally {
+ Cookies.remove('userid')
+ Cookies.remove('token')
+ this.$localStorage.remove(PASSWORD_CHANGE_REQUIRED)
+ this.$router.replace({ path: '/user/login' })
+ }
+ },
+ async isPasswordChangeRequired () {
+ try {
+ this.loading = true
+ const user = await this.getUserInfo()
+ if (user && !user.passwordchangerequired) {
+ this.isSubmitted = true
+ this.$router.replace({ path: '/user/login' })
Review Comment:
The login method isPasswordChangeRequired checks
`user.passwordchangerequired`, but the backend sets `passwordChangeRequired` in
the session, and the UI checks for this in localStorage. There's potential for
inconsistency in casing (camelCase vs lowercase). Ensure that the property name
used across the UI, API responses, and session storage is consistent to avoid
potential bugs where the flag isn't properly detected.
```suggestion
if (user) {
const passwordChangeRequiredFlag = typeof
user.passwordChangeRequired !== 'undefined'
? user.passwordChangeRequired
: user.passwordchangerequired
if (passwordChangeRequiredFlag === false) {
this.isSubmitted = true
this.$router.replace({ path: '/user/login' })
}
```
##########
engine/schema/src/main/resources/META-INF/db/views/cloud.user_view.sql:
##########
@@ -63,4 +64,7 @@ from
left join
`cloud`.`async_job` ON async_job.instance_id = user.id
and async_job.instance_type = 'User'
- and async_job.job_status = 0;
+ and async_job.job_status = 0
+ left join
+ `cloud`.`user_details` AS `user_details` ON `user_details`.`user_id` =
`user`.`id`
+ and `user_details`.`name` = 'PasswordChangeRequired';
Review Comment:
The SQL view includes a LEFT JOIN on user_details without any uniqueness
constraint or aggregation. If multiple user_details records exist for the same
user with the same name, this could produce duplicate rows in the view.
Consider adding DISTINCT or ensuring only one PasswordChangeRequired entry can
exist per user, or use a subquery to guarantee a single value.
##########
api/src/main/java/org/apache/cloudstack/api/response/LoginCmdResponse.java:
##########
@@ -90,6 +90,10 @@ public class LoginCmdResponse extends
AuthenticationCmdResponse {
@Param(description = "Management Server ID that the user logged to", since
= "4.21.0.0")
private String managementServerId;
+ @SerializedName(value = ApiConstants.PASSWORD_CHANGE_REQUIRED)
+ @Param(description = "Is User required to change password on next login.",
since = "4.23.0")
Review Comment:
The description text has an inconsistency. It says "Is User required" which
should be "Is user required" (lowercase 'user') to match standard
capitalization. Additionally, for clarity and consistency, consider rephrasing
to: "Indicates whether the user is required to change password on next login".
```suggestion
@Param(description = "Indicates whether the user is required to change
password on next login", since = "4.23.0")
```
##########
ui/src/views/iam/ForceChangePassword.vue:
##########
@@ -0,0 +1,284 @@
+// 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.
+
+<template>
+ <div class="user-layout-wrapper">
+ <div class="container">
+ <div class="user-layout-content">
+ <a-card :bordered="false" class="force-password-card">
+ <template #title>
+ <div style="text-align: center; font-size: 18px; font-weight:
bold;">
+ {{ $t('label.action.change.password') }}
+ </div>
+ <div v-if="!isSubmitted" style="text-align: center; font-size:
14px; color: #666; margin-top: 5px;">
+ {{ $t('message.change.password.required') }}
+ </div>
+ </template>
+ <a-spin :spinning="loading">
+ <div v-if="isSubmitted" class="success-state">
+ <div class="success-icon">✓</div>
+ <div class="success-text">
+ {{ $t('message.success.change.password') }}
+ </div>
+ <div class="success-subtext">
+ {{ $t('message.please.login.new.password') }}
+ </div>
+ <a-button
+ type="primary"
+ size="large"
+ block
+ @click="handleLogout"
+ style="margin-top: 20px;"
+ >
+ {{ $t('label.login') }}
+ </a-button>
+ </div>
+
+ <a-form
+ v-else
+ :ref="formRef"
+ :model="form"
+ :rules="rules"
+ layout="vertical"
+ @finish="handleSubmit"
+ v-ctrl-enter="handleSubmit"
+ >
+ <a-form-item name="currentpassword"
:label="$t('label.currentpassword')">
+ <a-input-password
+ v-model:value="form.currentpassword"
+ :placeholder="$t('label.currentpassword')"
+ size="large"
+ v-focus="true"
+ />
+ </a-form-item>
+
+ <a-form-item name="password" :label="$t('label.new.password')">
+ <a-input-password
+ v-model:value="form.password"
+ :placeholder="$t('label.new.password')"
+ size="large"
+ />
+ </a-form-item>
+
+ <a-form-item name="confirmpassword"
:label="$t('label.confirmpassword')">
+ <a-input-password
+ v-model:value="form.confirmpassword"
+ :placeholder="$t('label.confirmpassword')"
+ size="large"
+ />
+ </a-form-item>
+
+ <a-form-item>
+ <a-button
+ type="primary"
+ size="large"
+ block
+ :loading="loading"
+ @click="handleSubmit"
+ >
+ {{ $t('label.ok') }}
+ </a-button>
+ </a-form-item>
+
+ <div class="actions">
+ <a @click="handleLogout">{{ $t('label.logout') }}</a>
+ </div>
+ </a-form>
+ </a-spin>
+ </a-card>
+ </div>
+ </div>
+ </div>
+</template>
+
+<script>
+import { ref, reactive, toRaw } from 'vue'
+import { postAPI } from '@/api'
+import Cookies from 'js-cookie'
+import { PASSWORD_CHANGE_REQUIRED } from '@/store/mutation-types'
+
+export default {
+ name: 'ForceChangePassword',
+ data () {
+ return {
+ loading: false,
+ isSubmitted: false
+ }
+ },
+ created () {
+ this.formRef = ref()
+ this.form = reactive({})
+ this.isPasswordChangeRequired()
+ },
+ computed: {
+ rules () {
+ return {
+ currentpassword: [{ required: true, message:
this.$t('message.error.current.password') || 'Please enter current password' }],
+ password: [{ required: true, message:
this.$t('message.error.new.password') || 'Please enter new password' }],
+ confirmpassword: [
+ { required: true, message: this.$t('message.error.confirm.password')
|| 'Please confirm new password' },
Review Comment:
The validation rules have hard-coded fallback messages (e.g., 'Please enter
current password'). These should be removed since the template already uses
$t() for localization, and the fallback strings won't be translated. The
validation should rely on the localization system entirely.
```suggestion
currentpassword: [{ required: true, message:
this.$t('message.error.current.password') }],
password: [{ required: true, message:
this.$t('message.error.new.password') }],
confirmpassword: [
{ required: true, message:
this.$t('message.error.confirm.password') },
```
##########
ui/src/views/iam/ForceChangePassword.vue:
##########
@@ -0,0 +1,284 @@
+// 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.
+
+<template>
+ <div class="user-layout-wrapper">
+ <div class="container">
+ <div class="user-layout-content">
+ <a-card :bordered="false" class="force-password-card">
+ <template #title>
+ <div style="text-align: center; font-size: 18px; font-weight:
bold;">
+ {{ $t('label.action.change.password') }}
+ </div>
+ <div v-if="!isSubmitted" style="text-align: center; font-size:
14px; color: #666; margin-top: 5px;">
+ {{ $t('message.change.password.required') }}
+ </div>
+ </template>
+ <a-spin :spinning="loading">
+ <div v-if="isSubmitted" class="success-state">
+ <div class="success-icon">✓</div>
+ <div class="success-text">
+ {{ $t('message.success.change.password') }}
+ </div>
+ <div class="success-subtext">
+ {{ $t('message.please.login.new.password') }}
+ </div>
+ <a-button
+ type="primary"
+ size="large"
+ block
+ @click="handleLogout"
+ style="margin-top: 20px;"
+ >
+ {{ $t('label.login') }}
+ </a-button>
+ </div>
+
+ <a-form
+ v-else
+ :ref="formRef"
+ :model="form"
+ :rules="rules"
+ layout="vertical"
+ @finish="handleSubmit"
+ v-ctrl-enter="handleSubmit"
+ >
+ <a-form-item name="currentpassword"
:label="$t('label.currentpassword')">
+ <a-input-password
+ v-model:value="form.currentpassword"
+ :placeholder="$t('label.currentpassword')"
+ size="large"
+ v-focus="true"
+ />
+ </a-form-item>
+
+ <a-form-item name="password" :label="$t('label.new.password')">
+ <a-input-password
+ v-model:value="form.password"
+ :placeholder="$t('label.new.password')"
+ size="large"
+ />
+ </a-form-item>
+
+ <a-form-item name="confirmpassword"
:label="$t('label.confirmpassword')">
+ <a-input-password
+ v-model:value="form.confirmpassword"
+ :placeholder="$t('label.confirmpassword')"
+ size="large"
+ />
+ </a-form-item>
+
+ <a-form-item>
+ <a-button
+ type="primary"
+ size="large"
+ block
+ :loading="loading"
+ @click="handleSubmit"
+ >
+ {{ $t('label.ok') }}
+ </a-button>
+ </a-form-item>
+
+ <div class="actions">
+ <a @click="handleLogout">{{ $t('label.logout') }}</a>
+ </div>
+ </a-form>
+ </a-spin>
+ </a-card>
+ </div>
+ </div>
+ </div>
+</template>
+
+<script>
+import { ref, reactive, toRaw } from 'vue'
+import { postAPI } from '@/api'
+import Cookies from 'js-cookie'
+import { PASSWORD_CHANGE_REQUIRED } from '@/store/mutation-types'
+
+export default {
+ name: 'ForceChangePassword',
+ data () {
+ return {
+ loading: false,
+ isSubmitted: false
+ }
+ },
+ created () {
+ this.formRef = ref()
+ this.form = reactive({})
+ this.isPasswordChangeRequired()
+ },
+ computed: {
+ rules () {
+ return {
+ currentpassword: [{ required: true, message:
this.$t('message.error.current.password') || 'Please enter current password' }],
+ password: [{ required: true, message:
this.$t('message.error.new.password') || 'Please enter new password' }],
+ confirmpassword: [
+ { required: true, message: this.$t('message.error.confirm.password')
|| 'Please confirm new password' },
+ { validator: this.validateTwoPassword, trigger: 'change' }
+ ]
+ }
+ }
+ },
+ methods: {
+ async validateTwoPassword (rule, value) {
+ if (!value || value.length === 0) {
+ return Promise.resolve()
+ } else if (rule.field === 'confirmpassword') {
+ const form = this.form
+ const messageConfirm = this.$t('message.validate.equalto')
+ const passwordVal = form.password
+ if (passwordVal && passwordVal !== value) {
+ return Promise.reject(messageConfirm)
+ } else {
+ return Promise.resolve()
+ }
+ } else {
+ return Promise.resolve()
+ }
+ },
+ handleSubmit (e) {
+ e.preventDefault()
+ if (this.loading) return
+ this.formRef.value.validate().then(() => {
+ this.loading = true
+ const values = toRaw(this.form)
+ const userId = Cookies.get('userid')
+
+ const params = {
+ id: userId,
+ password: values.password,
+ currentpassword: values.currentpassword
+ }
+ postAPI('updateUser', params).then(() => {
+ this.$message.success(this.$t('message.success.change.password'))
+ this.isSubmitted = true
+ }).catch(error => {
+ console.error(error)
+ this.$notification.error({
+ message: 'Error',
+ description: error.response?.data?.updateuserresponse?.errortext
|| 'Failed to update password'
+ })
+ this.$message.error(this.$t('message.error.change.password'))
+ }).finally(() => {
+ this.loading = false
+ })
+ }).catch(error => {
+ console.log('Validation failed:', error)
+ })
+ },
+ async handleLogout () {
+ try {
+ await this.$store.dispatch('Logout')
+ } catch (e) {
+ console.error('Logout failed:', e)
+ } finally {
+ Cookies.remove('userid')
+ Cookies.remove('token')
+ this.$localStorage.remove(PASSWORD_CHANGE_REQUIRED)
+ this.$router.replace({ path: '/user/login' })
+ }
+ },
+ async isPasswordChangeRequired () {
+ try {
+ this.loading = true
+ const user = await this.getUserInfo()
+ if (user && !user.passwordchangerequired) {
+ this.isSubmitted = true
+ this.$router.replace({ path: '/user/login' })
+ }
+ } catch (e) {
+ console.error('Failed to resolve user info:', e)
+ } finally {
+ this.loading = false
+ }
+ },
+ async getUserInfo () {
+ const userInfo = this.$store.getters.userInfo
+ if (userInfo && userInfo.id) {
+ return userInfo
+ }
+ await this.$store.dispatch('GetInfo')
+ return this.$store.getters.userInfo
+ }
+ }
+}
+</script>
+
+<style scoped lang="less">
+.user-layout-wrapper {
+ display: flex;
+ justify-content: center;
+ align-items: center;
+
+ .container {
+ width: 100%;
+ padding: 16px;
+
+ .user-layout-content {
+ display: flex;
+ justify-content: center;
+
+ .force-password-card {
+ width: 100%;
+ max-width: 420px;
+ border-radius: 8px;
+ box-shadow: 0 6px 16px rgba(0, 0, 0, 0.08);
+ }
+ }
+ }
+}
+
+.actions {
+ text-align: center;
+ margin-top: 16px;
+
+ a {
+ color: #1890ff; /* Ant Design Link Color */
+ transition: color 0.3s;
+
+ &:hover {
+ color: #40a9ff;
Review Comment:
There's a hardcoded color value '#1890ff' and '#40a9ff' (Ant Design Link
Color comment). Consider using CSS variables or theme tokens instead of
hard-coded hex values to maintain consistency with the design system and
support theming.
##########
server/src/main/java/com/cloud/api/ApiServer.java:
##########
@@ -1227,6 +1230,9 @@ private ResponseObject createLoginResponse(HttpSession
session) {
if
(ApiConstants.MANAGEMENT_SERVER_ID.equalsIgnoreCase(attrName)) {
response.setManagementServerId(attrObj.toString());
}
+ if (PASSWORD_CHANGE_REQUIRED.endsWith(attrName)) {
Review Comment:
The condition `PASSWORD_CHANGE_REQUIRED.endsWith(attrName)` is incorrect.
This checks if the constant ends with the attribute name, but you likely meant
to check if `attrName.equals(PASSWORD_CHANGE_REQUIRED)`. The current logic
would match any attribute name that the constant ends with, which is not the
intended behavior.
```suggestion
if (PASSWORD_CHANGE_REQUIRED.equalsIgnoreCase(attrName)) {
```
##########
plugins/api/discovery/src/main/java/org/apache/cloudstack/discovery/ApiDiscoveryServiceImpl.java:
##########
@@ -280,12 +283,23 @@ public ListResponse<? extends BaseResponse> listApis(User
user, String name) {
ReflectionToStringBuilderUtils.reflectOnlySelectedFields(account,
"accountName", "uuid")));
}
- if (role.getRoleType() == RoleType.Admin && role.getId() ==
RoleType.Admin.getId()) {
- logger.info(String.format("Account [%s] is Root Admin, all
APIs are allowed.",
-
ReflectionToStringBuilderUtils.reflectOnlySelectedFields(account,
"accountName", "uuid")));
+ // Limit APIs on first login requiring password change
+ UserAccount userAccount =
accountService.getUserAccountById(user.getId());
+ if (MapUtils.isNotEmpty(userAccount.getDetails()) &&
+
userAccount.getDetails().containsKey(UserDetailVO.PasswordChangeRequired)) {
+
+ String needPasswordChange =
userAccount.getDetails().get(UserDetailVO.PasswordChangeRequired);
Review Comment:
The null check pattern could be improved. Instead of checking
`MapUtils.isNotEmpty(userAccount.getDetails()) &&
userAccount.getDetails().containsKey(...)`, you could simplify this by using
MapUtils.isNotEmpty() once and then safely accessing the value, or use a single
getOrDefault call with null checking. The current approach calls getDetails()
twice which is slightly inefficient.
```suggestion
Map<String, String> userDetails = userAccount.getDetails();
if (MapUtils.isNotEmpty(userDetails)) {
String needPasswordChange =
userDetails.get(UserDetailVO.PasswordChangeRequired);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
