Skip to content
Merged
Show file tree
Hide file tree
Changes from 11 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 @@ -1257,6 +1257,7 @@ public class ApiConstants {
public static final String PROVIDER_FOR_2FA = "providerfor2fa";
public static final String ISSUER_FOR_2FA = "issuerfor2fa";
public static final String MANDATE_2FA = "mandate2fa";
public static final String PASSWORD_CHANGE_REQUIRED = "passwordchangerequired";
public static final String SECRET_CODE = "secretcode";
public static final String LOGIN = "login";
public static final String LOGOUT = "logout";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import org.apache.cloudstack.api.response.UserResponse;
import org.apache.cloudstack.context.CallContext;
import org.apache.cloudstack.region.RegionService;
import org.apache.commons.lang.BooleanUtils;

import com.cloud.user.Account;
import com.cloud.user.User;
Expand All @@ -38,6 +39,8 @@
requestHasSensitiveInfo = true, responseHasSensitiveInfo = true)
public class UpdateUserCmd extends BaseCmd {

@Inject
private RegionService _regionService;

/////////////////////////////////////////////////////
//////////////// API parameters /////////////////////
Expand Down Expand Up @@ -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.",
Comment thread
sudo87 marked this conversation as resolved.
Comment thread
sudo87 marked this conversation as resolved.
since = "4.23.0")
private Boolean passwordChangeRequired;

/////////////////////////////////////////////////////
/////////////////// Accessors ///////////////////////
Expand Down Expand Up @@ -193,4 +199,8 @@ public Long getApiResourceId() {
public ApiCommandResourceType getApiResourceType() {
return ApiCommandResourceType.User;
}

public Boolean isPasswordChangeRequired() {
return BooleanUtils.isTrue(passwordChangeRequired);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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 = "Indicates whether the User is required to change password on next login.", since = "4.23.0")
private Boolean passwordChangeRequired;

public String getUsername() {
return username;
}
Expand Down Expand Up @@ -223,4 +227,12 @@ public String getManagementServerId() {
public void setManagementServerId(String managementServerId) {
this.managementServerId = managementServerId;
}

public Boolean getPasswordChangeRequired() {
return passwordChangeRequired;
}

public void setPasswordChangeRequired(String passwordChangeRequired) {
this.passwordChangeRequired = Boolean.parseBoolean(passwordChangeRequired);
}
Comment thread
sudo87 marked this conversation as resolved.
Outdated
}
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ public class UserDetailVO implements ResourceDetail {
public static final String Setup2FADetail = "2FASetupStatus";
public static final String PasswordResetToken = "PasswordResetToken";
public static final String PasswordResetTokenExpiryDate = "PasswordResetTokenExpiryDate";
public static final String PasswordChangeRequired = "PasswordChangeRequired";

public UserDetailVO() {
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,10 @@
import org.apache.cloudstack.api.response.ApiParameterResponse;
import org.apache.cloudstack.api.response.ApiResponseResponse;
import org.apache.cloudstack.api.response.ListResponse;
import org.apache.cloudstack.resourcedetail.UserDetailVO;
import org.apache.cloudstack.utils.reflectiontostringbuilderutils.ReflectionToStringBuilderUtils;
import org.apache.commons.collections.CollectionUtils;
import org.apache.commons.collections.MapUtils;
import org.apache.commons.lang3.StringUtils;
import org.reflections.ReflectionUtils;
import org.springframework.stereotype.Component;
Expand All @@ -55,6 +57,7 @@
import com.cloud.user.Account;
import com.cloud.user.AccountService;
import com.cloud.user.User;
import com.cloud.user.UserAccount;
import com.cloud.utils.ReflectUtil;
import com.cloud.utils.component.ComponentLifecycleBase;
import com.cloud.utils.component.PluggableService;
Expand All @@ -66,6 +69,7 @@ public class ApiDiscoveryServiceImpl extends ComponentLifecycleBase implements A
List<APIChecker> _apiAccessCheckers = null;
List<PluggableService> _services = null;
protected static Map<String, ApiDiscoveryResponse> s_apiNameDiscoveryResponseMap = null;
public static final List<String> APIS_ALLOWED_FOR_PASSWORD_CHANGE = Arrays.asList("login", "logout", "updateUser", "listApis");
Comment thread
sudo87 marked this conversation as resolved.

@Inject
AccountService accountService;
Expand Down Expand Up @@ -280,12 +284,19 @@ 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());
Map<String, String> userAccDetails = userAccount.getDetails();
if (MapUtils.isNotEmpty(userAccDetails) && "true".equalsIgnoreCase(userAccDetails.get(UserDetailVO.PasswordChangeRequired))) {
apisAllowed = APIS_ALLOWED_FOR_PASSWORD_CHANGE;
} else {
for (APIChecker apiChecker : _apiAccessCheckers) {
apisAllowed = apiChecker.getApisAllowedToUser(role, user, apisAllowed);
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")));
} else {
for (APIChecker apiChecker : _apiAccessCheckers) {
apisAllowed = apiChecker.getApisAllowedToUser(role, user, apisAllowed);
}
}
}
Comment thread
sudo87 marked this conversation as resolved.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@
import com.cloud.user.AccountService;
import com.cloud.user.AccountVO;
import com.cloud.user.User;
import com.cloud.user.UserAccount;
import com.cloud.user.UserAccountVO;
import com.cloud.user.UserVO;

import org.apache.cloudstack.acl.APIChecker;
Expand All @@ -29,6 +31,8 @@
import org.apache.cloudstack.acl.RoleType;
import org.apache.cloudstack.acl.RoleVO;
import org.apache.cloudstack.api.response.ApiDiscoveryResponse;
import org.apache.cloudstack.api.response.ListResponse;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
Expand All @@ -39,11 +43,15 @@
import org.mockito.junit.MockitoJUnitRunner;

import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
import java.util.Map;

import static org.apache.cloudstack.resourcedetail.UserDetailVO.PasswordChangeRequired;
import static org.apache.cloudstack.resourcedetail.UserDetailVO.Setup2FADetail;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyList;
import static org.mockito.ArgumentMatchers.anyLong;

@RunWith(MockitoJUnitRunner.class)
public class ApiDiscoveryTest {
Expand All @@ -66,12 +74,17 @@ public class ApiDiscoveryTest {
@InjectMocks
ApiDiscoveryServiceImpl discoveryServiceSpy;

@Mock
UserAccount mockUserAccount;

@Before
public void setup() {
discoveryServiceSpy.s_apiNameDiscoveryResponseMap = apiNameDiscoveryResponseMapMock;
discoveryServiceSpy._apiAccessCheckers = apiAccessCheckersMock;

Mockito.when(discoveryServiceSpy._apiAccessCheckers.iterator()).thenReturn(Arrays.asList(apiCheckerMock).iterator());
Mockito.when(mockUserAccount.getDetails()).thenReturn(null);
Mockito.when(accountServiceMock.getUserAccountById(anyLong())).thenReturn(mockUserAccount);
}
Comment thread
sudo87 marked this conversation as resolved.

private User getTestUser() {
Expand Down Expand Up @@ -131,4 +144,29 @@ public void listApisTestGetsApisAllowedToUserOnUserRole() throws PermissionDenie

Mockito.verify(apiCheckerMock, Mockito.times(1)).getApisAllowedToUser(any(Role.class), any(User.class), anyList());
}

@Test
public void listApisForUserWithoutEnforcedPwdChange() throws PermissionDeniedException {
RoleVO userRoleVO = new RoleVO(4L, "name", RoleType.User, "description");
Map<String, String> userDetails = new HashMap<>();
userDetails.put(Setup2FADetail, UserAccountVO.Setup2FAstatus.ENABLED.name());
Mockito.when(mockUserAccount.getDetails()).thenReturn(userDetails);
Mockito.when(accountServiceMock.getAccount(Mockito.anyLong())).thenReturn(getNormalAccount());
Mockito.when(roleServiceMock.findRole(Mockito.anyLong())).thenReturn(userRoleVO);
discoveryServiceSpy.listApis(getTestUser(), null);
Mockito.verify(apiCheckerMock, Mockito.times(1)).getApisAllowedToUser(any(Role.class), any(User.class), anyList());
}

@Test
public void listApisForUserEnforcedPwdChange() throws PermissionDeniedException {
RoleVO userRoleVO = new RoleVO(4L, "name", RoleType.User, "description");
Map<String, String> userDetails = new HashMap<>();
userDetails.put(PasswordChangeRequired, "true");
Mockito.when(mockUserAccount.getDetails()).thenReturn(userDetails);
Mockito.when(accountServiceMock.getAccount(Mockito.anyLong())).thenReturn(getNormalAccount());
Mockito.when(roleServiceMock.findRole(Mockito.anyLong())).thenReturn(userRoleVO);
Mockito.when(apiNameDiscoveryResponseMapMock.get(Mockito.anyString())).thenReturn(Mockito.mock(ApiDiscoveryResponse.class));
ListResponse<ApiDiscoveryResponse> response = (ListResponse<ApiDiscoveryResponse>) discoveryServiceSpy.listApis(getTestUser(), null);
Assert.assertEquals(4, response.getResponses().size());
}
}
13 changes: 13 additions & 0 deletions server/src/main/java/com/cloud/api/ApiServer.java
Original file line number Diff line number Diff line change
Expand Up @@ -116,9 +116,11 @@
import org.apache.cloudstack.framework.messagebus.MessageDispatcher;
import org.apache.cloudstack.framework.messagebus.MessageHandler;
import org.apache.cloudstack.managed.context.ManagedContextRunnable;
import org.apache.cloudstack.resourcedetail.UserDetailVO;
import org.apache.cloudstack.user.UserPasswordResetManager;
import org.apache.cloudstack.utils.identity.ManagementServerNode;
import org.apache.commons.codec.binary.Base64;
import org.apache.commons.collections.MapUtils;
import org.apache.commons.lang3.EnumUtils;
import org.apache.http.ConnectionClosedException;
import org.apache.http.HttpException;
Expand Down Expand Up @@ -194,6 +196,7 @@
import com.google.gson.reflect.TypeToken;

import static com.cloud.user.AccountManagerImpl.apiKeyAccess;
import static org.apache.cloudstack.api.ApiConstants.PASSWORD_CHANGE_REQUIRED;
import static org.apache.cloudstack.user.UserPasswordResetManager.UserPasswordResetEnabled;

@Component
Expand Down Expand Up @@ -1227,6 +1230,9 @@ private ResponseObject createLoginResponse(HttpSession session) {
if (ApiConstants.MANAGEMENT_SERVER_ID.equalsIgnoreCase(attrName)) {
response.setManagementServerId(attrObj.toString());
}
if (PASSWORD_CHANGE_REQUIRED.equalsIgnoreCase(attrName)) {
response.setPasswordChangeRequired(attrObj.toString());
}
}
}
response.setResponseName("loginresponse");
Expand Down Expand Up @@ -1327,6 +1333,13 @@ public ResponseObject loginUser(final HttpSession session, final String username
final String sessionKey = Base64.encodeBase64URLSafeString(sessionKeyBytes);
session.setAttribute(ApiConstants.SESSIONKEY, sessionKey);

Map<String, String> userAccDetails = userAcct.getDetails();
if (MapUtils.isNotEmpty(userAccDetails)) {
String needPwdChangeStr = userAccDetails.get(UserDetailVO.PasswordChangeRequired);
if ("true".equalsIgnoreCase(needPwdChangeStr)) {
session.setAttribute(PASSWORD_CHANGE_REQUIRED, true);
}
}
return createLoginResponse(session);
}
throw new CloudAuthenticationException("Failed to authenticate user " + username + " in domain " + domainId + "; please provide valid credentials");
Expand Down
23 changes: 23 additions & 0 deletions server/src/main/java/com/cloud/user/AccountManagerImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
// under the License.
package com.cloud.user;

import static org.apache.cloudstack.resourcedetail.UserDetailVO.PasswordChangeRequired;

import java.net.InetAddress;
import java.net.URLEncoder;
import java.security.NoSuchAlgorithmException;
Expand Down Expand Up @@ -1586,9 +1588,28 @@ 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())) {
boolean isCallerSameAsUser = user.getId() == caller.getId();
boolean isPasswordResetRequired = updateUserCmd.isPasswordChangeRequired() && !isCallerSameAsUser;
// Admins only can enforce passwordChangeRequired for user
if (isRootAdmin(caller.getAccountId()) || isDomainAdmin(caller.getAccountId())) {
if (isPasswordResetRequired) {
_userDetailsDao.addDetail(user.getId(), PasswordChangeRequired, "true", false);
}
}

// Remove passwordChangeRequired if user updating own pwd or admin has not enforced it
if (isCallerSameAsUser || !isPasswordResetRequired) {
_userDetailsDao.removeDetail(user.getId(), PasswordChangeRequired);
}
}
}

@Override
public void verifyCallerPrivilegeForUserOrAccountOperations(Account userAccount) {
logger.debug(String.format("Verifying whether the caller has the correct privileges based on the user's role type and API permissions: %s", userAccount));
Expand Down Expand Up @@ -2847,6 +2868,8 @@ public UserAccount authenticateUser(final String username, final String password
logger.debug(String.format("User: %s in domain %d has successfully logged in, auth time duration - %d ms", username, domainId, validUserLastAuthTimeDurationInMs));
}

user.setDetails(_userDetailsDao.listDetailsKeyPairs(user.getId()));
Comment thread
sudo87 marked this conversation as resolved.

return user;
} else {
if (logger.isDebugEnabled()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
import java.util.Set;
import java.util.UUID;

import static org.apache.cloudstack.resourcedetail.UserDetailVO.PasswordChangeRequired;
import static org.apache.cloudstack.resourcedetail.UserDetailVO.PasswordResetToken;
import static org.apache.cloudstack.resourcedetail.UserDetailVO.PasswordResetTokenExpiryDate;

Expand Down Expand Up @@ -247,6 +248,8 @@ void resetPassword(UserAccount userAccount, String password) {

userDetailsDao.removeDetail(userAccount.getId(), PasswordResetToken);
userDetailsDao.removeDetail(userAccount.getId(), PasswordResetTokenExpiryDate);
// remove password change required if user reset password
userDetailsDao.removeDetail(userAccount.getId(), PasswordChangeRequired);

userDao.persist(user);
}
Expand Down
13 changes: 13 additions & 0 deletions server/src/test/java/com/cloud/user/AccountManagerImplTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -432,6 +432,19 @@ public void updateUserTestTimeZoneAndEmailNotNull() {
prepareMockAndExecuteUpdateUserTest(1);
}

@Test
public void updateUserTestPwdChange() {
Mockito.doReturn(true).when(UpdateUserCmdMock).isPasswordChangeRequired();
Mockito.when(userVoMock.getAccountId()).thenReturn(10L);
Mockito.doReturn(accountMock).when(accountManagerImpl).getAccount(10L);
Mockito.when(accountMock.getAccountId()).thenReturn(10L);
Mockito.doReturn(false).when(accountManagerImpl).isRootAdmin(10L);
Mockito.lenient().when(accountManagerImpl.getRoleType(Mockito.eq(accountMock))).thenReturn(RoleType.User);
Mockito.when(callingUser.getAccountId()).thenReturn(1L);
Mockito.doReturn(true).when(accountManagerImpl).isRootAdmin(1L);
prepareMockAndExecuteUpdateUserTest(0);
}

private void prepareMockAndExecuteUpdateUserTest(int numberOfExpectedCallsForSetEmailAndSetTimeZone) {
Mockito.doReturn("password").when(UpdateUserCmdMock).getPassword();
Mockito.doReturn("newpassword").when(UpdateUserCmdMock).getCurrentPassword();
Expand Down
4 changes: 4 additions & 0 deletions ui/public/locales/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -530,6 +530,7 @@
"label.change.ipaddress": "Change IP address for NIC",
"label.change.disk.offering": "Change disk offering",
"label.change.offering.for.volume": "Change disk offering for the volume",
"label.change.password.onlogin": "User must change password at next login",
"label.change.service.offering": "Change service offering",
"label.character": "Character",
"label.checksum": "Checksum",
Expand Down Expand Up @@ -3138,6 +3139,7 @@
"message.change.offering.for.volume.failed": "Change offering for the volume failed",
"message.change.offering.for.volume.processing": "Changing offering for the volume...",
"message.change.password": "Please change your password.",
"message.change.password.required": "You are required to change your password.",
"message.change.scope.failed": "Scope change failed",
"message.change.scope.processing": "Scope change in progress",
"message.change.service.offering.sharedfs.failed": "Failed to change service offering for the Shared FileSystem.",
Expand Down Expand Up @@ -3385,6 +3387,7 @@
"message.error.apply.tungsten.tag": "Applying Tag failed",
"message.error.binaries.iso.url": "Please enter binaries ISO URL.",
"message.error.bucket": "Please enter bucket",
"message.error.change.password": "Failed to change password.",
"message.error.cidr": "CIDR is required",
"message.error.cidr.or.cidrsize": "CIDR or cidr size is required",
"message.error.cloudian.console": "Single-Sign-On failed for Cloudian management console. Please ask your administrator to fix integration issues.",
Expand Down Expand Up @@ -3688,6 +3691,7 @@
"message.please.confirm.remove.user.data": "Please confirm that you want to remove this User Data",
"message.please.enter.valid.value": "Please enter a valid value.",
"message.please.enter.value": "Please enter values.",
"message.please.login.new.password": "Please log in again with your new password",
"message.please.wait.while.autoscale.vmgroup.is.being.created": "Please wait while your AutoScaling Group is being created; this may take a while...",
"message.please.wait.while.zone.is.being.created": "Please wait while your Zone is being created; this may take a while...",
"message.pod.dedicated": "Pod dedicated.",
Expand Down
5 changes: 5 additions & 0 deletions ui/src/config/router.js
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,11 @@ export const constantRouterMap = [
path: 'resetPassword',
name: 'resetPassword',
component: () => import(/* webpackChunkName: "auth" */ '@/views/auth/ResetPassword')
},
{
path: 'forceChangePassword',
name: 'forceChangePassword',
component: () => import(/* webpackChunkName: "auth" */ '@/views/iam/ForceChangePassword')
}
]
},
Expand Down
Loading
Loading