Skip to content

Commit c79b33c

Browse files
sudo87sudo87
authored
Allow enforcing password change for a user after reset by admin (root/domain) (#12294)
* API modifications for passwordchangerequired * ui login flow for passwordchangerequired * add passwordchangerequired in listUsers API response, it will be used in UI to render reset password form * cleanup redundant LOGIN_SOURCE and limiting apis for first time login * address copilot comments * allow enforcing password change for all role types and update reset pwd flow for passwordchangerequired * address review comments * add unit tests * cleanup ispasswordchangerequired from user_view * address review comments * 1. Allow enforcing password change while creating user 2. Admin can enforce password change on next login with out resetting password * address review comment, add unit test * improve code coverage * fix pre-commit license issue * 1. allow enter key to submit change password form 2. hide force password reset for disabled/locked user in ui * 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 * allow oauth login to skip force password change
1 parent b1edfb8 commit c79b33c

File tree

30 files changed

+1023
-38
lines changed

30 files changed

+1023
-38
lines changed

‎api/src/main/java/com/cloud/user/AccountService.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,8 @@ UserAccount createUserAccount(String userName, String password, String firstName
5959

6060
User getSystemUser();
6161

62-
User createUser(String userName, String password, String firstName, String lastName, String email, String timeZone, String accountName, Long domainId, String userUUID);
62+
User createUser(String userName, String password, String firstName, String lastName, String email, String timeZone,
63+
String accountName, Long domainId, String userUUID, boolean isPasswordChangeRequired);
6364

6465
User createUser(String userName, String password, String firstName, String lastName, String email, String timeZone, String accountName, Long domainId, String userUUID,
6566
User.Source source);

‎api/src/main/java/org/apache/cloudstack/api/ApiConstants.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1261,6 +1261,7 @@ public class ApiConstants {
12611261
public static final String PROVIDER_FOR_2FA = "providerfor2fa";
12621262
public static final String ISSUER_FOR_2FA = "issuerfor2fa";
12631263
public static final String MANDATE_2FA = "mandate2fa";
1264+
public static final String PASSWORD_CHANGE_REQUIRED = "passwordchangerequired";
12641265
public static final String SECRET_CODE = "secretcode";
12651266
public static final String LOGIN = "login";
12661267
public static final String LOGOUT = "logout";

‎api/src/main/java/org/apache/cloudstack/api/command/admin/user/CreateUserCmd.java‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import org.apache.cloudstack.api.response.DomainResponse;
2727
import org.apache.cloudstack.api.response.UserResponse;
2828
import org.apache.cloudstack.context.CallContext;
29+
import org.apache.commons.lang.BooleanUtils;
2930
import org.apache.commons.lang3.StringUtils;
3031

3132
import com.cloud.user.Account;
@@ -78,6 +79,12 @@ public class CreateUserCmd extends BaseCmd {
7879
@Parameter(name = ApiConstants.USER_ID, type = CommandType.STRING, description = "User UUID, required for adding account from external provisioning system")
7980
private String userUUID;
8081

82+
@Parameter(name = ApiConstants.PASSWORD_CHANGE_REQUIRED,
83+
type = CommandType.BOOLEAN,
84+
description = "Provide true to mandate the User to reset password on next login.",
85+
since = "4.23.0")
86+
private Boolean passwordChangeRequired;
87+
8188
/////////////////////////////////////////////////////
8289
/////////////////// Accessors ///////////////////////
8390
/////////////////////////////////////////////////////
@@ -118,6 +125,10 @@ public String getUserUUID() {
118125
return userUUID;
119126
}
120127

128+
public Boolean isPasswordChangeRequired() {
129+
return BooleanUtils.isTrue(passwordChangeRequired);
130+
}
131+
121132
/////////////////////////////////////////////////////
122133
/////////////// API Implementation///////////////////
123134
/////////////////////////////////////////////////////
@@ -147,7 +158,7 @@ public void execute() {
147158
CallContext.current().setEventDetails("UserName: " + getUserName() + ", FirstName :" + getFirstName() + ", LastName: " + getLastName());
148159
User user =
149160
_accountService.createUser(getUserName(), getPassword(), getFirstName(), getLastName(), getEmail(), getTimezone(), getAccountName(), getDomainId(),
150-
getUserUUID());
161+
getUserUUID(), isPasswordChangeRequired());
151162
if (user != null) {
152163
UserResponse response = _responseGenerator.createUserResponse(user);
153164
response.setResponseName(getCommandName());

‎api/src/main/java/org/apache/cloudstack/api/command/admin/user/UpdateUserCmd.java‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import org.apache.cloudstack.api.response.UserResponse;
3030
import org.apache.cloudstack.context.CallContext;
3131
import org.apache.cloudstack.region.RegionService;
32+
import org.apache.commons.lang.BooleanUtils;
3233

3334
import com.cloud.user.Account;
3435
import com.cloud.user.User;
@@ -38,6 +39,8 @@
3839
requestHasSensitiveInfo = true, responseHasSensitiveInfo = true)
3940
public class UpdateUserCmd extends BaseCmd {
4041

42+
@Inject
43+
private RegionService _regionService;
4144

4245
/////////////////////////////////////////////////////
4346
//////////////// API parameters /////////////////////
@@ -85,8 +88,11 @@ public class UpdateUserCmd extends BaseCmd {
8588
"This parameter is only used to mandate 2FA, not to disable 2FA", since = "4.18.0.0")
8689
private Boolean mandate2FA;
8790

88-
@Inject
89-
private RegionService _regionService;
91+
@Parameter(name = ApiConstants.PASSWORD_CHANGE_REQUIRED,
92+
type = CommandType.BOOLEAN,
93+
description = "Provide true to mandate the User to reset password on next login.",
94+
since = "4.23.0")
95+
private Boolean passwordChangeRequired;
9096

9197
/////////////////////////////////////////////////////
9298
/////////////////// Accessors ///////////////////////
@@ -193,4 +199,8 @@ public Long getApiResourceId() {
193199
public ApiCommandResourceType getApiResourceType() {
194200
return ApiCommandResourceType.User;
195201
}
202+
203+
public Boolean isPasswordChangeRequired() {
204+
return BooleanUtils.isTrue(passwordChangeRequired);
205+
}
196206
}

‎api/src/main/java/org/apache/cloudstack/api/response/LoginCmdResponse.java‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,10 @@ public class LoginCmdResponse extends AuthenticationCmdResponse {
9090
@Param(description = "Management Server ID that the user logged to", since = "4.21.0.0")
9191
private String managementServerId;
9292

93+
@SerializedName(value = ApiConstants.PASSWORD_CHANGE_REQUIRED)
94+
@Param(description = "Indicates whether the User is required to change password on next login.", since = "4.23.0")
95+
private Boolean passwordChangeRequired;
96+
9397
public String getUsername() {
9498
return username;
9599
}
@@ -223,4 +227,12 @@ public String getManagementServerId() {
223227
public void setManagementServerId(String managementServerId) {
224228
this.managementServerId = managementServerId;
225229
}
230+
231+
public Boolean getPasswordChangeRequired() {
232+
return passwordChangeRequired;
233+
}
234+
235+
public void setPasswordChangeRequired(Boolean passwordChangeRequired) {
236+
this.passwordChangeRequired = passwordChangeRequired;
237+
}
226238
}

‎api/src/test/java/org/apache/cloudstack/api/command/admin/user/CreateUserCmdTest.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ public void testExecuteWithNotBlankPassword() {
6969
} catch (ServerApiException e) {
7070
Assert.assertTrue("Received exception as the mock accountService createUser returns null user", true);
7171
}
72-
Mockito.verify(accountService, Mockito.times(1)).createUser(null, "Test", null, null, null, null, null, null, null);
72+
Mockito.verify(accountService, Mockito.times(1)).createUser(null, "Test", null, null, null, null, null, null, null, false);
7373
}
7474

7575
@Test
@@ -82,7 +82,7 @@ public void testExecuteWithNullPassword() {
8282
Assert.assertEquals(ApiErrorCode.PARAM_ERROR,e.getErrorCode());
8383
Assert.assertEquals("Empty passwords are not allowed", e.getMessage());
8484
}
85-
Mockito.verify(accountService, Mockito.never()).createUser(null, null, null, null, null, null, null, null, null);
85+
Mockito.verify(accountService, Mockito.never()).createUser(null, null, null, null, null, null, null, null, null, false);
8686
}
8787

8888
@Test
@@ -95,6 +95,6 @@ public void testExecuteWithEmptyPassword() {
9595
Assert.assertEquals(ApiErrorCode.PARAM_ERROR,e.getErrorCode());
9696
Assert.assertEquals("Empty passwords are not allowed", e.getMessage());
9797
}
98-
Mockito.verify(accountService, Mockito.never()).createUser(null, null, null, null, null, null, null, null, null);
98+
Mockito.verify(accountService, Mockito.never()).createUser(null, null, null, null, null, null, null, null, null, true);
9999
}
100100
}

‎api/src/test/java/org/apache/cloudstack/api/command/admin/user/UpdateUserCmdTest.java‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
package org.apache.cloudstack.api.command.admin.user;
21+
22+
import org.apache.cloudstack.api.ApiCommandResourceType;
23+
import org.junit.Assert;
24+
import org.junit.Test;
25+
import org.junit.runner.RunWith;
26+
import org.mockito.InjectMocks;
27+
import org.mockito.junit.MockitoJUnitRunner;
28+
import org.springframework.test.util.ReflectionTestUtils;
29+
30+
@RunWith(MockitoJUnitRunner.class)
31+
public class UpdateUserCmdTest {
32+
@InjectMocks
33+
private UpdateUserCmd cmd;
34+
35+
@Test
36+
public void testGetApiResourceId() {
37+
Long userId = 99L;
38+
cmd.setId(userId);
39+
Assert.assertEquals(userId, cmd.getApiResourceId());
40+
}
41+
42+
@Test
43+
public void testGetApiResourceType() {
44+
Assert.assertEquals(ApiCommandResourceType.User, cmd.getApiResourceType());
45+
}
46+
47+
@Test
48+
public void testIsPasswordChangeRequired_True() {
49+
ReflectionTestUtils.setField(cmd, "passwordChangeRequired", Boolean.TRUE);
50+
Assert.assertTrue(cmd.isPasswordChangeRequired());
51+
}
52+
53+
@Test
54+
public void testIsPasswordChangeRequired_False() {
55+
ReflectionTestUtils.setField(cmd, "passwordChangeRequired", Boolean.FALSE);
56+
Assert.assertFalse(cmd.isPasswordChangeRequired());
57+
}
58+
59+
@Test
60+
public void testIsPasswordChangeRequired_Null() {
61+
ReflectionTestUtils.setField(cmd, "passwordChangeRequired", null);
62+
Assert.assertFalse(cmd.isPasswordChangeRequired());
63+
}
64+
}

‎api/src/test/java/org/apache/cloudstack/api/response/LoginCmdResponseTest.java‎

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
// Licensed to the Apache Software Foundation (ASF) under one
2+
// or more contributor license agreements. See the NOTICE file
3+
// distributed with this work for additional information
4+
// regarding copyright ownership. The ASF licenses this file
5+
// to you under the Apache License, Version 2.0 (the
6+
// "License"); you may not use this file except in compliance
7+
// with the License. You may obtain a copy of the License at
8+
//
9+
// http://www.apache.org/licenses/LICENSE-2.0
10+
//
11+
// Unless required by applicable law or agreed to in writing,
12+
// software distributed under the License is distributed on an
13+
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
// KIND, either express or implied. See the License for the
15+
// specific language governing permissions and limitations
16+
// under the License.
17+
18+
package org.apache.cloudstack.api.response;
19+
20+
21+
import org.junit.Assert;
22+
import org.junit.Test;
23+
24+
public class LoginCmdResponseTest {
25+
26+
@Test
27+
public void testAllGettersAndSetters() {
28+
LoginCmdResponse response = new LoginCmdResponse();
29+
30+
response.setUsername("user1");
31+
response.setUserId("100");
32+
response.setDomainId("200");
33+
response.setTimeout(3600);
34+
response.setAccount("account1");
35+
response.setFirstName("John");
36+
response.setLastName("Doe");
37+
response.setType("admin");
38+
response.setTimeZone("UTC");
39+
response.setTimeZoneOffset("+00:00");
40+
response.setRegistered("true");
41+
response.setSessionKey("session-key");
42+
response.set2FAenabled("true");
43+
response.set2FAverfied("false");
44+
response.setProviderFor2FA("totp");
45+
response.setIssuerFor2FA("cloudstack");
46+
response.setManagementServerId("ms-1");
47+
48+
Assert.assertEquals("user1", response.getUsername());
49+
Assert.assertEquals("100", response.getUserId());
50+
Assert.assertEquals("200", response.getDomainId());
51+
Assert.assertEquals(Integer.valueOf(3600), response.getTimeout());
52+
Assert.assertEquals("account1", response.getAccount());
53+
Assert.assertEquals("John", response.getFirstName());
54+
Assert.assertEquals("Doe", response.getLastName());
55+
Assert.assertEquals("admin", response.getType());
56+
Assert.assertEquals("UTC", response.getTimeZone());
57+
Assert.assertEquals("+00:00", response.getTimeZoneOffset());
58+
Assert.assertEquals("true", response.getRegistered());
59+
Assert.assertEquals("session-key", response.getSessionKey());
60+
Assert.assertEquals("true", response.is2FAenabled());
61+
Assert.assertEquals("false", response.is2FAverfied());
62+
Assert.assertEquals("totp", response.getProviderFor2FA());
63+
Assert.assertEquals("cloudstack", response.getIssuerFor2FA());
64+
Assert.assertEquals("ms-1", response.getManagementServerId());
65+
}
66+
67+
@Test
68+
public void testPasswordChangeRequired_True() {
69+
LoginCmdResponse response = new LoginCmdResponse();
70+
response.setPasswordChangeRequired(true);
71+
Assert.assertTrue(response.getPasswordChangeRequired());
72+
}
73+
74+
@Test
75+
public void testPasswordChangeRequired_False() {
76+
LoginCmdResponse response = new LoginCmdResponse();
77+
response.setPasswordChangeRequired(false);
78+
Assert.assertFalse(response.getPasswordChangeRequired());
79+
}
80+
81+
@Test
82+
public void testPasswordChangeRequired_Null() {
83+
LoginCmdResponse response = new LoginCmdResponse();
84+
response.setPasswordChangeRequired(null);
85+
Assert.assertNull("Boolean.parseBoolean(null) should return null", response.getPasswordChangeRequired());
86+
}
87+
}

‎engine/schema/src/main/java/org/apache/cloudstack/resourcedetail/UserDetailVO.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,8 @@ public class UserDetailVO implements ResourceDetail {
4848
public static final String Setup2FADetail = "2FASetupStatus";
4949
public static final String PasswordResetToken = "PasswordResetToken";
5050
public static final String PasswordResetTokenExpiryDate = "PasswordResetTokenExpiryDate";
51+
public static final String PasswordChangeRequired = "PasswordChangeRequired";
52+
public static final String OauthLogin = "OauthLogin";
5153

5254
public UserDetailVO() {
5355
}

‎plugins/api/discovery/src/main/java/org/apache/cloudstack/discovery/ApiDiscoveryServiceImpl.java‎

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,10 @@
4444
import org.apache.cloudstack.api.response.ApiParameterResponse;
4545
import org.apache.cloudstack.api.response.ApiResponseResponse;
4646
import org.apache.cloudstack.api.response.ListResponse;
47+
import org.apache.cloudstack.resourcedetail.UserDetailVO;
4748
import org.apache.cloudstack.utils.reflectiontostringbuilderutils.ReflectionToStringBuilderUtils;
4849
import org.apache.commons.collections.CollectionUtils;
50+
import org.apache.commons.collections.MapUtils;
4951
import org.apache.commons.lang3.StringUtils;
5052
import org.reflections.ReflectionUtils;
5153
import org.springframework.stereotype.Component;
@@ -56,6 +58,7 @@
5658
import com.cloud.user.Account;
5759
import com.cloud.user.AccountService;
5860
import com.cloud.user.User;
61+
import com.cloud.user.UserAccount;
5962
import com.cloud.utils.ReflectUtil;
6063
import com.cloud.utils.component.ComponentLifecycleBase;
6164
import com.cloud.utils.component.PluggableService;
@@ -67,6 +70,7 @@ public class ApiDiscoveryServiceImpl extends ComponentLifecycleBase implements A
6770
List<APIChecker> _apiAccessCheckers = null;
6871
List<PluggableService> _services = null;
6972
protected static Map<String, ApiDiscoveryResponse> s_apiNameDiscoveryResponseMap = null;
73+
public static final List<String> APIS_ALLOWED_FOR_PASSWORD_CHANGE = Arrays.asList("login", "logout", "updateUser", "listApis");
7074

7175
@Inject
7276
AccountService accountService;
@@ -287,12 +291,20 @@ public ListResponse<? extends BaseResponse> listApis(User user, String name) {
287291
ReflectionToStringBuilderUtils.reflectOnlySelectedFields(account, "accountName", "uuid")));
288292
}
289293

290-
if (role.getRoleType() == RoleType.Admin && role.getId() == RoleType.Admin.getId()) {
291-
logger.info(String.format("Account [%s] is Root Admin, all APIs are allowed.",
292-
ReflectionToStringBuilderUtils.reflectOnlySelectedFields(account, "accountName", "uuid")));
294+
// Limit APIs on first login requiring password change
295+
UserAccount userAccount = accountService.getUserAccountById(user.getId());
296+
Map<String, String> userAccDetails = userAccount.getDetails();
297+
if (MapUtils.isNotEmpty(userAccDetails) && !userAccDetails.containsKey(UserDetailVO.OauthLogin) &&
298+
"true".equalsIgnoreCase(userAccDetails.get(UserDetailVO.PasswordChangeRequired))) {
299+
apisAllowed = APIS_ALLOWED_FOR_PASSWORD_CHANGE;
293300
} else {
294-
for (APIChecker apiChecker : _apiAccessCheckers) {
295-
apisAllowed = apiChecker.getApisAllowedToUser(role, user, apisAllowed);
301+
if (role.getRoleType() == RoleType.Admin && role.getId() == RoleType.Admin.getId()) {
302+
logger.info(String.format("Account [%s] is Root Admin, all APIs are allowed.",
303+
ReflectionToStringBuilderUtils.reflectOnlySelectedFields(account, "accountName", "uuid")));
304+
} else {
305+
for (APIChecker apiChecker : _apiAccessCheckers) {
306+
apisAllowed = apiChecker.getApisAllowedToUser(role, user, apisAllowed);
307+
}
296308
}
297309
}
298310

0 commit comments

Comments
 (0)