From a5df0afcf21c51ea75612791a71b6ebc332a9521 Mon Sep 17 00:00:00 2001 From: Vishesh Date: Wed, 24 Jul 2024 13:16:31 +0530 Subject: [PATCH 1/6] Feature: Forgot password --- .../cloudstack/api/ApiServerService.java | 5 + .../api/auth/APIAuthenticationType.java | 2 +- .../java/com/cloud/user/UserAccountVO.java | 4 + .../resourcedetail/UserDetailVO.java | 2 + .../management/MockAccountManager.java | 6 + server/pom.xml | 5 + .../main/java/com/cloud/api/ApiServer.java | 46 ++- .../auth/APIAuthenticationManagerImpl.java | 2 + ...aultForgotPasswordAPIAuthenticatorCmd.java | 163 +++++++++ ...faultResetPasswordAPIAuthenticatorCmd.java | 192 ++++++++++ .../java/com/cloud/user/AccountManager.java | 2 + .../com/cloud/user/AccountManagerImpl.java | 9 +- .../apache/cloudstack/user/PasswordReset.java | 55 +++ .../cloudstack/user/PasswordResetImpl.java | 248 +++++++++++++ .../spring-server-core-managers-context.xml | 1 + .../java/com/cloud/api/ApiServerTest.java | 59 +++ .../cloud/user/AccountManagerImplTest.java | 20 +- .../cloud/user/MockAccountManagerImpl.java | 4 + .../user/PasswordResetImplTest.java | 115 ++++++ tools/apidoc/gen_toc.py | 4 +- ui/public/locales/en.json | 5 + ui/src/config/router.js | 10 + ui/src/permission.js | 2 +- ui/src/views/auth/ForgotPassword.vue | 268 ++++++++++++++ ui/src/views/auth/Login.vue | 12 +- ui/src/views/auth/ResetPassword.vue | 337 ++++++++++++++++++ .../utils/mailing/SMTPMailSender.java | 20 +- 27 files changed, 1563 insertions(+), 35 deletions(-) create mode 100644 server/src/main/java/com/cloud/api/auth/DefaultForgotPasswordAPIAuthenticatorCmd.java create mode 100644 server/src/main/java/com/cloud/api/auth/DefaultResetPasswordAPIAuthenticatorCmd.java create mode 100644 server/src/main/java/org/apache/cloudstack/user/PasswordReset.java create mode 100644 server/src/main/java/org/apache/cloudstack/user/PasswordResetImpl.java create mode 100644 server/src/test/java/org/apache/cloudstack/user/PasswordResetImplTest.java create mode 100644 ui/src/views/auth/ForgotPassword.vue create mode 100644 ui/src/views/auth/ResetPassword.vue diff --git a/api/src/main/java/org/apache/cloudstack/api/ApiServerService.java b/api/src/main/java/org/apache/cloudstack/api/ApiServerService.java index 54fda7e36b82..a7d99f4546f6 100644 --- a/api/src/main/java/org/apache/cloudstack/api/ApiServerService.java +++ b/api/src/main/java/org/apache/cloudstack/api/ApiServerService.java @@ -22,6 +22,7 @@ import javax.servlet.http.HttpSession; import com.cloud.exception.CloudAuthenticationException; +import com.cloud.user.UserAccount; public interface ApiServerService { public boolean verifyRequest(Map requestParameters, Long userId, InetAddress remoteAddress) throws ServerApiException; @@ -42,4 +43,8 @@ public ResponseObject loginUser(HttpSession session, String username, String pas public String handleRequest(Map params, String responseType, StringBuilder auditTrailSb) throws ServerApiException; public Class getCmdClass(String cmdName); + + boolean forgotPassword(UserAccount userAccount); + + boolean resetPassword(UserAccount userAccount, String token, String password); } diff --git a/api/src/main/java/org/apache/cloudstack/api/auth/APIAuthenticationType.java b/api/src/main/java/org/apache/cloudstack/api/auth/APIAuthenticationType.java index 5ba9d182daa0..1f78708f7e58 100644 --- a/api/src/main/java/org/apache/cloudstack/api/auth/APIAuthenticationType.java +++ b/api/src/main/java/org/apache/cloudstack/api/auth/APIAuthenticationType.java @@ -17,5 +17,5 @@ package org.apache.cloudstack.api.auth; public enum APIAuthenticationType { - LOGIN_API, LOGOUT_API, READONLY_API, LOGIN_2FA_API + LOGIN_API, LOGOUT_API, READONLY_API, LOGIN_2FA_API, PASSWORD_RESET } diff --git a/engine/schema/src/main/java/com/cloud/user/UserAccountVO.java b/engine/schema/src/main/java/com/cloud/user/UserAccountVO.java index c18ca53f7abe..1da7d52a366a 100644 --- a/engine/schema/src/main/java/com/cloud/user/UserAccountVO.java +++ b/engine/schema/src/main/java/com/cloud/user/UserAccountVO.java @@ -17,6 +17,7 @@ package com.cloud.user; import java.util.Date; +import java.util.HashMap; import java.util.Map; import javax.persistence.Column; @@ -361,6 +362,9 @@ public void setUser2faProvider(String user2faProvider) { @Override public Map getDetails() { + if (details == null) { + details = new HashMap<>(); + } return details; } diff --git a/engine/schema/src/main/java/org/apache/cloudstack/resourcedetail/UserDetailVO.java b/engine/schema/src/main/java/org/apache/cloudstack/resourcedetail/UserDetailVO.java index 1b430e806e29..d0cfcc3d4396 100644 --- a/engine/schema/src/main/java/org/apache/cloudstack/resourcedetail/UserDetailVO.java +++ b/engine/schema/src/main/java/org/apache/cloudstack/resourcedetail/UserDetailVO.java @@ -46,6 +46,8 @@ public class UserDetailVO implements ResourceDetail { private boolean display = true; public static final String Setup2FADetail = "2FASetupStatus"; + public static final String PasswordResetToken = "PasswordResetToken"; + public static final String PasswordResetTokenExpiryDate = "PasswordResetTokenExpiryDate"; public UserDetailVO() { } diff --git a/plugins/network-elements/juniper-contrail/src/test/java/org/apache/cloudstack/network/contrail/management/MockAccountManager.java b/plugins/network-elements/juniper-contrail/src/test/java/org/apache/cloudstack/network/contrail/management/MockAccountManager.java index 610f4aa82aac..f3df71f95a9e 100644 --- a/plugins/network-elements/juniper-contrail/src/test/java/org/apache/cloudstack/network/contrail/management/MockAccountManager.java +++ b/plugins/network-elements/juniper-contrail/src/test/java/org/apache/cloudstack/network/contrail/management/MockAccountManager.java @@ -514,4 +514,10 @@ public String getConfigComponentName() { public ConfigKey[] getConfigKeys() { return null; } + + public void validateUserPasswordAndUpdateIfNeeded(String newPassword, UserVO user, + String currentPassword, + boolean skipCurrentPassValidation) { + + } } diff --git a/server/pom.xml b/server/pom.xml index e18dcb5fe280..1d822a2444c3 100644 --- a/server/pom.xml +++ b/server/pom.xml @@ -101,6 +101,11 @@ commons-math3 ${cs.commons-math3.version} + + com.github.spullara.mustache.java + compiler + 0.9.14 + org.apache.cloudstack cloud-utils diff --git a/server/src/main/java/com/cloud/api/ApiServer.java b/server/src/main/java/com/cloud/api/ApiServer.java index 0d4382097c24..bc31f76f4d3b 100644 --- a/server/src/main/java/com/cloud/api/ApiServer.java +++ b/server/src/main/java/com/cloud/api/ApiServer.java @@ -55,6 +55,13 @@ import javax.servlet.http.HttpServletResponse; import javax.servlet.http.HttpSession; +import com.cloud.user.Account; +import com.cloud.user.AccountManager; +import com.cloud.user.AccountManagerImpl; +import com.cloud.user.DomainManager; +import com.cloud.user.User; +import com.cloud.user.UserAccount; +import com.cloud.user.UserVO; import org.apache.cloudstack.acl.APIChecker; import org.apache.cloudstack.api.APICommand; import org.apache.cloudstack.api.ApiConstants; @@ -103,6 +110,7 @@ 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.user.PasswordReset; import org.apache.commons.codec.binary.Base64; import org.apache.http.ConnectionClosedException; import org.apache.http.HttpException; @@ -157,13 +165,6 @@ import com.cloud.exception.UnavailableCommandException; import com.cloud.projects.dao.ProjectDao; import com.cloud.storage.VolumeApiService; -import com.cloud.user.Account; -import com.cloud.user.AccountManager; -import com.cloud.user.AccountManagerImpl; -import com.cloud.user.DomainManager; -import com.cloud.user.User; -import com.cloud.user.UserAccount; -import com.cloud.user.UserVO; import com.cloud.utils.ConstantTimeComparator; import com.cloud.utils.DateUtil; import com.cloud.utils.HttpUtils; @@ -182,6 +183,9 @@ import com.cloud.utils.net.NetUtils; import com.google.gson.reflect.TypeToken; +import static org.apache.cloudstack.resourcedetail.UserDetailVO.PasswordResetToken; +import static org.apache.cloudstack.resourcedetail.UserDetailVO.PasswordResetTokenExpiryDate; + @Component public class ApiServer extends ManagerBase implements HttpRequestHandler, ApiServerService, Configurable { @@ -214,6 +218,8 @@ public class ApiServer extends ManagerBase implements HttpRequestHandler, ApiSer private ProjectDao projectDao; @Inject private UUIDManager uuidMgr; + @Inject + private PasswordReset passwordReset; private List pluggableServices; @@ -1223,6 +1229,32 @@ public boolean verifyUser(final Long userId) { return true; } + @Override + public boolean forgotPassword(UserAccount userAccount) { + String resetToken = userAccount.getDetails().get(PasswordResetToken); + String resetTokenExpiryTimeString = userAccount.getDetails().getOrDefault(PasswordResetTokenExpiryDate, "0"); + + if (StringUtils.isBlank(userAccount.getEmail())) { + throw new CloudRuntimeException("Email is not set for the user. Please contact your administrator."); + } + + if (StringUtils.isNotEmpty(resetToken) && StringUtils.isNotEmpty(resetTokenExpiryTimeString)) { + final Date resetTokenExpiryTime = new Date(Long.parseLong(resetTokenExpiryTimeString)); + final Date currentTime = new Date(); + if (currentTime.after(resetTokenExpiryTime)) { + passwordReset.setResetTokenAndSend(userAccount); + } + } else if (StringUtils.isEmpty(resetToken)) { + passwordReset.setResetTokenAndSend(userAccount); + } + return true; + } + + @Override + public boolean resetPassword(UserAccount userAccount, String token, String password) { + return passwordReset.validateAndResetPassword(userAccount, token, password); + } + private void checkCommandAvailable(final User user, final String commandName, final InetAddress remoteAddress) throws PermissionDeniedException { if (user == null) { throw new PermissionDeniedException("User is null for role based API access check for command" + commandName); diff --git a/server/src/main/java/com/cloud/api/auth/APIAuthenticationManagerImpl.java b/server/src/main/java/com/cloud/api/auth/APIAuthenticationManagerImpl.java index 907ef088ee8d..990486d1d65e 100644 --- a/server/src/main/java/com/cloud/api/auth/APIAuthenticationManagerImpl.java +++ b/server/src/main/java/com/cloud/api/auth/APIAuthenticationManagerImpl.java @@ -75,6 +75,8 @@ public List> getCommands() { List> cmdList = new ArrayList>(); cmdList.add(DefaultLoginAPIAuthenticatorCmd.class); cmdList.add(DefaultLogoutAPIAuthenticatorCmd.class); + cmdList.add(DefaultForgotPasswordAPIAuthenticatorCmd.class); + cmdList.add(DefaultResetPasswordAPIAuthenticatorCmd.class); cmdList.add(ListUserTwoFactorAuthenticatorProvidersCmd.class); cmdList.add(ValidateUserTwoFactorAuthenticationCodeCmd.class); diff --git a/server/src/main/java/com/cloud/api/auth/DefaultForgotPasswordAPIAuthenticatorCmd.java b/server/src/main/java/com/cloud/api/auth/DefaultForgotPasswordAPIAuthenticatorCmd.java new file mode 100644 index 000000000000..172bcd9fb1d2 --- /dev/null +++ b/server/src/main/java/com/cloud/api/auth/DefaultForgotPasswordAPIAuthenticatorCmd.java @@ -0,0 +1,163 @@ +// 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. +package com.cloud.api.auth; + +import com.cloud.api.ApiServlet; +import com.cloud.api.response.ApiResponseSerializer; +import com.cloud.domain.Domain; +import com.cloud.user.Account; +import com.cloud.user.User; +import com.cloud.user.UserAccount; +import com.cloud.utils.exception.CloudRuntimeException; +import org.apache.cloudstack.api.APICommand; +import org.apache.cloudstack.api.ApiConstants; +import org.apache.cloudstack.api.ApiErrorCode; +import org.apache.cloudstack.api.ApiServerService; +import org.apache.cloudstack.api.BaseCmd; +import org.apache.cloudstack.api.Parameter; +import org.apache.cloudstack.api.ServerApiException; +import org.apache.cloudstack.api.auth.APIAuthenticationType; +import org.apache.cloudstack.api.auth.APIAuthenticator; +import org.apache.cloudstack.api.auth.PluggableAPIAuthenticator; +import org.apache.cloudstack.api.response.SuccessResponse; +import org.jetbrains.annotations.Nullable; + +import javax.inject.Inject; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import javax.servlet.http.HttpSession; +import java.net.InetAddress; +import java.util.List; +import java.util.Map; + +@APICommand(name = "forgotPassword", + description = "Sends an email to the user with a token to reset the password using resetPassword command.", + requestHasSensitiveInfo = true, + responseObject = SuccessResponse.class) +public class DefaultForgotPasswordAPIAuthenticatorCmd extends BaseCmd implements APIAuthenticator { + + + ///////////////////////////////////////////////////// + //////////////// API parameters ///////////////////// + ///////////////////////////////////////////////////// + @Parameter(name = ApiConstants.USERNAME, type = CommandType.STRING, description = "Username", required = true) + private String username; + + @Parameter(name = ApiConstants.DOMAIN, type = CommandType.STRING, description = "Path of the domain that the user belongs to. Example: domain=/com/cloud/internal. If no domain is passed in, the ROOT (/) domain is assumed.") + private String domain; + + @Inject + ApiServerService _apiServer; + + ///////////////////////////////////////////////////// + /////////////////// Accessors /////////////////////// + ///////////////////////////////////////////////////// + + public String getUsername() { + return username; + } + + public String getDomainName() { + return domain; + } + + + ///////////////////////////////////////////////////// + /////////////// API Implementation/////////////////// + ///////////////////////////////////////////////////// + + @Override + public long getEntityOwnerId() { + return Account.Type.NORMAL.ordinal(); + } + + @Override + public void execute() throws ServerApiException { + // We should never reach here + throw new ServerApiException(ApiErrorCode.METHOD_NOT_ALLOWED, "This is an authentication api, cannot be used directly"); + } + + @Override + public String authenticate(String command, Map params, HttpSession session, InetAddress remoteAddress, String responseType, StringBuilder auditTrailSb, final HttpServletRequest req, final HttpServletResponse resp) throws ServerApiException { + final String[] username = (String[])params.get(ApiConstants.USERNAME); + final String[] domainName = (String[])params.get(ApiConstants.DOMAIN); + + Long domainId = null; + String domain = null; + domain = getDomainName(auditTrailSb, domainName, domain); + + String serializedResponse = null; + if (username != null) { + try { + final Domain userDomain = _domainService.findDomainByPath(domain); + if (userDomain != null) { + domainId = userDomain.getId(); + } else { + throw new ServerApiException(ApiErrorCode.PARAM_ERROR, String.format("Unable to find the domain from the path %s", domain)); + } + final UserAccount userAccount = _accountService.getActiveUserAccount(username[0], domainId); + if (userAccount != null && List.of(User.Source.SAML2, User.Source.OAUTH2, User.Source.LDAP).contains(userAccount.getSource())) { + throw new ServerApiException(ApiErrorCode.PARAM_ERROR, "Forgot Password is not allowed for this user"); + } + boolean success = _apiServer.forgotPassword(userAccount); + SuccessResponse successResponse = new SuccessResponse(); + successResponse.setSuccess(success); + successResponse.setResponseName(getCommandName()); + return ApiResponseSerializer.toSerializedString(successResponse, responseType); + } catch (final CloudRuntimeException ex) { + ApiServlet.invalidateHttpSession(session, "fall through to API key,"); + String msg = String.format("%s", ex.getMessage() != null ? + ex.getMessage() : + "forgot password request failed for user, check if username/domain are correct"); + auditTrailSb.append(" " + ApiErrorCode.ACCOUNT_ERROR + " " + msg); + serializedResponse = _apiServer.getSerializedApiError(ApiErrorCode.ACCOUNT_ERROR.getHttpCode(), msg, params, responseType); + if (logger.isTraceEnabled()) { + logger.trace(msg); + } + } + } + // We should not reach here and if we do we throw an exception + throw new ServerApiException(ApiErrorCode.ACCOUNT_ERROR, serializedResponse); + } + + @Nullable + private String getDomainName(StringBuilder auditTrailSb, String[] domainName, String domain) { + if (domainName != null) { + domain = domainName[0]; + auditTrailSb.append(" domain=" + domain); + if (domain != null) { + // ensure domain starts with '/' and ends with '/' + if (!domain.endsWith("/")) { + domain += '/'; + } + if (!domain.startsWith("/")) { + domain = "/" + domain; + } + } + } + return domain; + } + + @Override + public APIAuthenticationType getAPIType() { + return APIAuthenticationType.PASSWORD_RESET; + } + + @Override + public void setAuthenticators(List authenticators) { + } +} diff --git a/server/src/main/java/com/cloud/api/auth/DefaultResetPasswordAPIAuthenticatorCmd.java b/server/src/main/java/com/cloud/api/auth/DefaultResetPasswordAPIAuthenticatorCmd.java new file mode 100644 index 000000000000..57740c2b922a --- /dev/null +++ b/server/src/main/java/com/cloud/api/auth/DefaultResetPasswordAPIAuthenticatorCmd.java @@ -0,0 +1,192 @@ +// 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. +package com.cloud.api.auth; + +import com.cloud.api.ApiServlet; +import com.cloud.api.response.ApiResponseSerializer; +import com.cloud.domain.Domain; +import com.cloud.exception.CloudAuthenticationException; +import com.cloud.user.Account; +import com.cloud.user.User; +import com.cloud.user.UserAccount; +import com.cloud.utils.UuidUtils; +import com.cloud.utils.exception.CloudRuntimeException; +import org.apache.cloudstack.api.APICommand; +import org.apache.cloudstack.api.ApiConstants; +import org.apache.cloudstack.api.ApiErrorCode; +import org.apache.cloudstack.api.ApiServerService; +import org.apache.cloudstack.api.BaseCmd; +import org.apache.cloudstack.api.Parameter; +import org.apache.cloudstack.api.ServerApiException; +import org.apache.cloudstack.api.auth.APIAuthenticationType; +import org.apache.cloudstack.api.auth.APIAuthenticator; +import org.apache.cloudstack.api.auth.PluggableAPIAuthenticator; +import org.apache.cloudstack.api.response.SuccessResponse; +import org.jetbrains.annotations.Nullable; + +import javax.inject.Inject; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import javax.servlet.http.HttpSession; +import java.net.InetAddress; +import java.util.List; +import java.util.Map; + +@APICommand(name = "resetPassword", + description = "Resets the password for the user using the token generated via forgotPassword command.", + requestHasSensitiveInfo = true, + responseObject = SuccessResponse.class) +public class DefaultResetPasswordAPIAuthenticatorCmd extends BaseCmd implements APIAuthenticator { + + + ///////////////////////////////////////////////////// + //////////////// API parameters ///////////////////// + ///////////////////////////////////////////////////// + @Parameter(name = ApiConstants.USERNAME, + type = CommandType.STRING, + description = "Username", required = true) + private String username; + + @Parameter(name = ApiConstants.DOMAIN, + type = CommandType.STRING, + description = "Path of the domain that the user belongs to. Example: domain=/com/cloud/internal. If no domain is passed in, the ROOT (/) domain is assumed.") + private String domain; + + @Parameter(name = ApiConstants.TOKEN, + type = CommandType.STRING, + required = true, + description = "Token generated via forgotPassword command.") + private String token; + + @Parameter(name = ApiConstants.PASSWORD, + type = CommandType.STRING, + required = true, + description = "New password in clear text (Default hashed to SHA256SALT).") + private String password; + + @Inject + ApiServerService _apiServer; + + ///////////////////////////////////////////////////// + /////////////////// Accessors /////////////////////// + ///////////////////////////////////////////////////// + + public String getUsername() { + return username; + } + + public String getDomainName() { + return domain; + } + + public String getToken() { + return token; + } + + ///////////////////////////////////////////////////// + /////////////// API Implementation/////////////////// + ///////////////////////////////////////////////////// + + @Override + public long getEntityOwnerId() { + return Account.Type.NORMAL.ordinal(); + } + + @Override + public void execute() throws ServerApiException { + // We should never reach here + throw new ServerApiException(ApiErrorCode.METHOD_NOT_ALLOWED, "This is an authentication api, cannot be used directly"); + } + + @Override + public String authenticate(String command, Map params, HttpSession session, InetAddress remoteAddress, String responseType, StringBuilder auditTrailSb, final HttpServletRequest req, final HttpServletResponse resp) throws ServerApiException { + final String[] username = (String[])params.get(ApiConstants.USERNAME); + final String[] password = (String[])params.get(ApiConstants.PASSWORD); + final String[] domainName = (String[])params.get(ApiConstants.DOMAIN); + final String[] token = (String[])params.get(ApiConstants.TOKEN); + + Long domainId = null; + String domain = null; + domain = getDomainName(auditTrailSb, domainName, domain); + + String serializedResponse = null; + + if (!UuidUtils.isUuid(token[0])) { + throw new ServerApiException(ApiErrorCode.PARAM_ERROR, "Invalid token"); + } + + if (username != null) { + final String pwd = ((password == null) ? null : password[0]); + try { + final Domain userDomain = _domainService.findDomainByPath(domain); + if (userDomain != null) { + domainId = userDomain.getId(); + } else { + throw new ServerApiException(ApiErrorCode.PARAM_ERROR, String.format("Unable to find the domain from the path %s", domain)); + } + final UserAccount userAccount = _accountService.getActiveUserAccount(username[0], domainId); + if (userAccount != null && List.of(User.Source.SAML2, User.Source.OAUTH2, User.Source.LDAP).contains(userAccount.getSource())) { + throw new CloudAuthenticationException("Password reset is not allowed for CloudStack login"); + } + boolean success = _apiServer.resetPassword(userAccount, token[0], pwd); + SuccessResponse successResponse = new SuccessResponse(); + successResponse.setSuccess(success); + successResponse.setResponseName(getCommandName()); + return ApiResponseSerializer.toSerializedString(successResponse, responseType); + } catch (final CloudRuntimeException ex) { + ApiServlet.invalidateHttpSession(session, "fall through to API key,"); + String msg = String.format("%s", ex.getMessage() != null ? + ex.getMessage() : + "failed to reset password for user, check your inputs"); + auditTrailSb.append(" " + ApiErrorCode.ACCOUNT_ERROR + " " + msg); + serializedResponse = _apiServer.getSerializedApiError(ApiErrorCode.ACCOUNT_ERROR.getHttpCode(), msg, params, responseType); + if (logger.isTraceEnabled()) { + logger.trace(msg); + } + } + } + // We should not reach here and if we do we throw an exception + throw new ServerApiException(ApiErrorCode.ACCOUNT_ERROR, serializedResponse); + } + + @Nullable + private String getDomainName(StringBuilder auditTrailSb, String[] domainName, String domain) { + if (domainName != null) { + domain = domainName[0]; + auditTrailSb.append(" domain=" + domain); + if (domain != null) { + // ensure domain starts with '/' and ends with '/' + if (!domain.endsWith("/")) { + domain += '/'; + } + if (!domain.startsWith("/")) { + domain = "/" + domain; + } + } + } + return domain; + } + + @Override + public APIAuthenticationType getAPIType() { + return APIAuthenticationType.PASSWORD_RESET; + } + + @Override + public void setAuthenticators(List authenticators) { + } +} diff --git a/server/src/main/java/com/cloud/user/AccountManager.java b/server/src/main/java/com/cloud/user/AccountManager.java index 6d2d1db5668d..3b278adb8654 100644 --- a/server/src/main/java/com/cloud/user/AccountManager.java +++ b/server/src/main/java/com/cloud/user/AccountManager.java @@ -199,4 +199,6 @@ void buildACLViewSearchCriteria(SearchCriteria s UserTwoFactorAuthenticationSetupResponse setupUserTwoFactorAuthentication(SetupUserTwoFactorAuthenticationCmd cmd); List getApiNameList(); + + void validateUserPasswordAndUpdateIfNeeded(String newPassword, UserVO user, String currentPassword, boolean skipCurrentPassValidation); } diff --git a/server/src/main/java/com/cloud/user/AccountManagerImpl.java b/server/src/main/java/com/cloud/user/AccountManagerImpl.java index dcb12cbdb74e..804299d97ad4 100644 --- a/server/src/main/java/com/cloud/user/AccountManagerImpl.java +++ b/server/src/main/java/com/cloud/user/AccountManagerImpl.java @@ -1440,7 +1440,7 @@ public UserAccount updateUser(UpdateUserCmd updateUserCmd) { validateAndUpdateLastNameIfNeeded(updateUserCmd, user); validateAndUpdateUsernameIfNeeded(updateUserCmd, user, account); - validateUserPasswordAndUpdateIfNeeded(updateUserCmd.getPassword(), user, updateUserCmd.getCurrentPassword()); + validateUserPasswordAndUpdateIfNeeded(updateUserCmd.getPassword(), user, updateUserCmd.getCurrentPassword(), false); String email = updateUserCmd.getEmail(); if (StringUtils.isNotBlank(email)) { user.setEmail(email); @@ -1468,7 +1468,7 @@ public UserAccount updateUser(UpdateUserCmd updateUserCmd) { * * If all checks pass, we encode the given password with the most preferable password mechanism given in {@link #_userPasswordEncoders}. */ - protected void validateUserPasswordAndUpdateIfNeeded(String newPassword, UserVO user, String currentPassword) { + public void validateUserPasswordAndUpdateIfNeeded(String newPassword, UserVO user, String currentPassword, boolean skipCurrentPassValidation) { if (newPassword == null) { logger.trace("No new password to update for user: " + user.getUuid()); return; @@ -1483,16 +1483,17 @@ protected void validateUserPasswordAndUpdateIfNeeded(String newPassword, UserVO boolean isRootAdminExecutingPasswordUpdate = callingAccount.getId() == Account.ACCOUNT_ID_SYSTEM || isRootAdmin(callingAccount.getId()); boolean isDomainAdmin = isDomainAdmin(callingAccount.getId()); boolean isAdmin = isDomainAdmin || isRootAdminExecutingPasswordUpdate; + boolean skipValidation = isAdmin || skipCurrentPassValidation; if (isAdmin) { logger.trace(String.format("Admin account [uuid=%s] executing password update for user [%s] ", callingAccount.getUuid(), user.getUuid())); } - if (!isAdmin && StringUtils.isBlank(currentPassword)) { + if (!skipValidation && StringUtils.isBlank(currentPassword)) { throw new InvalidParameterValueException("To set a new password the current password must be provided."); } if (CollectionUtils.isEmpty(_userPasswordEncoders)) { throw new CloudRuntimeException("No user authenticators configured!"); } - if (!isAdmin) { + if (!skipValidation) { validateCurrentPassword(user, currentPassword); } UserAuthenticator userAuthenticator = _userPasswordEncoders.get(0); diff --git a/server/src/main/java/org/apache/cloudstack/user/PasswordReset.java b/server/src/main/java/org/apache/cloudstack/user/PasswordReset.java new file mode 100644 index 000000000000..feba06a6eb0a --- /dev/null +++ b/server/src/main/java/org/apache/cloudstack/user/PasswordReset.java @@ -0,0 +1,55 @@ +// 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. + +package org.apache.cloudstack.user; + +import com.cloud.user.UserAccount; +import org.apache.cloudstack.framework.config.ConfigKey; + +public interface PasswordReset { + ConfigKey PasswordResetTtl = new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED, Long.class, + "password.reset.ttl", "30", + "Password reset ttl in minutes", true, ConfigKey.Scope.Global); + + ConfigKey PasswordResetEmailSender = new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED, + String.class, "password.reset.email.sender", null, + "Password reset email sender", true, ConfigKey.Scope.Global); + + ConfigKey PasswordResetSMTPHost = new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED, + String.class, "password.reset.smtp.host", null, + "Password reset smtp host", false, ConfigKey.Scope.Global); + + ConfigKey PasswordResetSMTPPort = new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED, + Integer.class, "password.reset.smtp.port", "25", + "Password reset smtp port", false, ConfigKey.Scope.Global); + + ConfigKey PasswordResetSMTPUseAuth = new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED, + Boolean.class, "password.reset.smtp.useAuth", "false", + "Use auth for smtp in Password reset", false, ConfigKey.Scope.Global); + + ConfigKey PasswordResetSMTPUsername = new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED, + String.class, "password.reset.smtp.username", null, + "Password reset smtp username", false, ConfigKey.Scope.Global); + + ConfigKey PasswordResetSMTPPassword = new ConfigKey<>("Secure", String.class, + "password.reset.smtp.password", null, + "Password reset smtp password", false, ConfigKey.Scope.Global); + + void setResetTokenAndSend(UserAccount userAccount); + + boolean validateAndResetPassword(UserAccount user, String token, String password); +} diff --git a/server/src/main/java/org/apache/cloudstack/user/PasswordResetImpl.java b/server/src/main/java/org/apache/cloudstack/user/PasswordResetImpl.java new file mode 100644 index 000000000000..777479d281e6 --- /dev/null +++ b/server/src/main/java/org/apache/cloudstack/user/PasswordResetImpl.java @@ -0,0 +1,248 @@ +// 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. + +package org.apache.cloudstack.user; + +import com.cloud.user.AccountManager; +import com.cloud.user.UserAccount; +import com.cloud.user.UserVO; +import com.cloud.user.dao.UserDao; +import com.cloud.utils.StringUtils; +import com.cloud.utils.component.ManagerBase; +import com.github.mustachejava.DefaultMustacheFactory; +import com.github.mustachejava.Mustache; +import com.github.mustachejava.MustacheFactory; +import org.apache.cloudstack.api.ApiErrorCode; +import org.apache.cloudstack.api.ServerApiException; +import org.apache.cloudstack.framework.config.ConfigKey; +import org.apache.cloudstack.framework.config.Configurable; +import org.apache.cloudstack.resourcedetail.UserDetailVO; +import org.apache.cloudstack.resourcedetail.dao.UserDetailsDao; +import org.apache.cloudstack.utils.mailing.MailAddress; +import org.apache.cloudstack.utils.mailing.SMTPMailProperties; +import org.apache.cloudstack.utils.mailing.SMTPMailSender; + +import javax.inject.Inject; +import javax.naming.ConfigurationException; +import java.io.IOException; +import java.io.StringReader; +import java.io.StringWriter; +import java.util.Date; +import java.util.HashMap; +import java.util.HashSet; +import java.util.Map; +import java.util.Set; +import java.util.UUID; + +import static org.apache.cloudstack.config.ApiServiceConfiguration.ManagementServerAddresses; +import static org.apache.cloudstack.resourcedetail.UserDetailVO.PasswordResetToken; +import static org.apache.cloudstack.resourcedetail.UserDetailVO.PasswordResetTokenExpiryDate; + +public class PasswordResetImpl extends ManagerBase implements PasswordReset, Configurable { + + @Inject + private AccountManager accountManager; + + @Inject + private UserDetailsDao userDetailsDao; + + @Inject + private UserDao userDao; + + private SMTPMailSender mailSender; + + public static ConfigKey PasswordResetMailTemplate = + new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED, String.class, + "password.reset.mail.template", "Hello {{username}}!\n" + + "You have requested to reset your password. Please click the following link to reset your password:\n" + + "{{{reset_link}}}\n" + + "If you did not request a password reset, please ignore this email.\n" + + "\n" + + "Regards,\n" + + "The CloudStack Team", + "Password reset mail template. This uses mustache template engine. Available " + + "variables are: username, firstName, lastName, resetLink, token", + true, + ConfigKey.Scope.Global); + + @Override + public String getConfigComponentName() { + return PasswordResetImpl.class.getSimpleName(); + } + + @Override + public ConfigKey[] getConfigKeys() { + return new ConfigKey[]{PasswordResetTtl, + PasswordResetEmailSender, + PasswordResetSMTPHost, + PasswordResetSMTPPort, + PasswordResetSMTPUseAuth, + PasswordResetSMTPUsername, + PasswordResetSMTPPassword, + PasswordResetMailTemplate + }; + } + + @Override + public boolean configure(String name, Map params) throws ConfigurationException { + String smtpHost = PasswordResetSMTPHost.value(); + Integer smtpPort = PasswordResetSMTPPort.value(); + Boolean useAuth = PasswordResetSMTPUseAuth.value(); + String username = PasswordResetSMTPUsername.value(); + String password = PasswordResetSMTPPassword.value(); + + String namespace = "password.reset.smtp"; + + Map configs = new HashMap<>(); + + configs.put(getKey(namespace, SMTPMailSender.CONFIG_HOST), smtpHost); + configs.put(getKey(namespace, SMTPMailSender.CONFIG_PORT), smtpPort.toString()); + configs.put(getKey(namespace, SMTPMailSender.CONFIG_USE_AUTH), useAuth.toString()); + configs.put(getKey(namespace, SMTPMailSender.CONFIG_USERNAME), username); + configs.put(getKey(namespace, SMTPMailSender.CONFIG_PASSWORD), password); + + mailSender = new SMTPMailSender(configs, namespace); + return true; + } + + private String getKey(String namespace, String config) { + return String.format("%s.%s", namespace, config); + } + + + public void setResetTokenAndSend(UserAccount userAccount) { + final String resetToken = UUID.randomUUID().toString(); + final Date resetTokenExpiryTime = new Date(System.currentTimeMillis() + PasswordResetTtl.value() * 60 * 1000); + + userDetailsDao.addDetail(userAccount.getId(), PasswordResetToken, resetToken, false); + userDetailsDao.addDetail(userAccount.getId(), PasswordResetTokenExpiryDate, String.valueOf(resetTokenExpiryTime.getTime()), false); + + final String email = userAccount.getEmail(); + final String username = userAccount.getUsername(); + final String subject = "Password Reset Request"; + + String resetLink = String.format("%s/user/resetPassword?username=%s&token=%s", ManagementServerAddresses.value(), username, resetToken); + String content = getMessageBody(userAccount, resetToken, resetLink); + + SMTPMailProperties mailProperties = new SMTPMailProperties(); + + mailProperties.setSender(new MailAddress(PasswordResetEmailSender.value())); + mailProperties.setSubject(subject); + mailProperties.setContent(content); + mailProperties.setContentType("text/plain"); + + Set addresses = new HashSet<>(); + + addresses.add(new MailAddress(email)); + + mailProperties.setRecipients(addresses); + + mailSender.sendMail(mailProperties); + } + + @Override + public boolean validateAndResetPassword(UserAccount user, String token, String password) { + UserDetailVO resetTokenDetail = userDetailsDao.findDetail(user.getId(), PasswordResetToken); + UserDetailVO resetTokenExpiryDate = userDetailsDao.findDetail(user.getId(), PasswordResetTokenExpiryDate); + + if (resetTokenDetail == null || resetTokenExpiryDate == null) { + throw new ServerApiException(ApiErrorCode.PARAM_ERROR, String.format("No reset token found for user %s", user.getUsername())); + } + + Date resetTokenExpiryTime = new Date(Long.parseLong(resetTokenExpiryDate.getValue())); + + Date now = new Date(); + String resetToken = resetTokenDetail.getValue(); + if (StringUtils.isEmpty(resetToken)) { + throw new ServerApiException(ApiErrorCode.PARAM_ERROR, String.format("No reset token found for user %s", user.getUsername())); + } + if (!resetToken.equals(token)) { + throw new ServerApiException(ApiErrorCode.PARAM_ERROR, String.format("Invalid reset token for user %s", user.getUsername())); + } + if (now.after(resetTokenExpiryTime)) { + throw new ServerApiException(ApiErrorCode.PARAM_ERROR, String.format("Reset token has expired for user %s", user.getUsername())); + } + + resetPassword(user, password); + return true; + } + + + void resetPassword(UserAccount userAccount, String password) { + UserVO user = userDao.getUser(userAccount.getId()); + + accountManager.validateUserPasswordAndUpdateIfNeeded(password, user, "", true); + + userDetailsDao.removeDetail(userAccount.getId(), PasswordResetToken); + userDetailsDao.removeDetail(userAccount.getId(), PasswordResetTokenExpiryDate); + + userDao.persist(user); + } + + String getMessageBody(UserAccount userAccount, String token, String resetLink) { + MustacheFactory mf = new DefaultMustacheFactory(); + Mustache mustache = mf.compile(new StringReader(PasswordResetMailTemplate.value()), "password.reset.mail"); + StringWriter writer = new StringWriter(); + + PasswordResetMail values = new PasswordResetMail(userAccount.getUsername(), userAccount.getFirstname(), userAccount.getLastname(), resetLink, token); + + try { + mustache.execute(writer, values).flush(); + } catch (IOException e) { + throw new RuntimeException(e); + } + return writer.toString(); + + } + + static class PasswordResetMail { + private String username; + private String firstName; + private String lastName; + private String resetLink; + private String token; + + + public PasswordResetMail(String username, String firstName, String lastName, String resetLink, String token) { + this.username = username; + this.firstName = firstName; + this.lastName = lastName; + this.resetLink = resetLink; + this.token = token; + } + + public String getUsername() { + return username; + } + + public String getFirstName() { + return firstName; + } + + public String getLastName() { + return lastName; + } + + public String getResetLink() { + return resetLink; + } + + public String getToken() { + return token; + } + } +} diff --git a/server/src/main/resources/META-INF/cloudstack/core/spring-server-core-managers-context.xml b/server/src/main/resources/META-INF/cloudstack/core/spring-server-core-managers-context.xml index 1ca630cfa8a6..383ac3157c28 100644 --- a/server/src/main/resources/META-INF/cloudstack/core/spring-server-core-managers-context.xml +++ b/server/src/main/resources/META-INF/cloudstack/core/spring-server-core-managers-context.xml @@ -56,6 +56,7 @@ + diff --git a/server/src/test/java/com/cloud/api/ApiServerTest.java b/server/src/test/java/com/cloud/api/ApiServerTest.java index 7b0380f8e646..caf923332151 100644 --- a/server/src/test/java/com/cloud/api/ApiServerTest.java +++ b/server/src/test/java/com/cloud/api/ApiServerTest.java @@ -17,22 +17,34 @@ package com.cloud.api; import java.util.ArrayList; +import java.util.Collections; import java.util.List; +import java.util.Map; +import com.cloud.user.UserAccount; +import com.cloud.utils.exception.CloudRuntimeException; +import org.apache.cloudstack.user.PasswordReset; import org.junit.Assert; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.InjectMocks; +import org.mockito.Mock; import org.mockito.MockedConstruction; import org.mockito.Mockito; import org.mockito.junit.MockitoJUnitRunner; +import static org.apache.cloudstack.resourcedetail.UserDetailVO.PasswordResetToken; +import static org.apache.cloudstack.resourcedetail.UserDetailVO.PasswordResetTokenExpiryDate; + @RunWith(MockitoJUnitRunner.class) public class ApiServerTest { @InjectMocks ApiServer apiServer = new ApiServer(); + @Mock + PasswordReset passwordReset; + private void runTestSetupIntegrationPortListenerInvalidPorts(Integer port) { try (MockedConstruction mocked = Mockito.mockConstruction(ApiServer.ListenerThread.class)) { @@ -61,4 +73,51 @@ public void testSetupIntegrationPortListenerValidPort() { Mockito.verify(listenerThread).start(); } } + + @Test + public void testForgotPasswordSuccessFirstRequest() { + UserAccount userAccount = Mockito.mock(UserAccount.class); + + Mockito.when(userAccount.getDetails()).thenReturn(Collections.emptyMap()); + Mockito.when(userAccount.getEmail()).thenReturn("test@test.com"); + Mockito.doNothing().when(passwordReset).setResetTokenAndSend(userAccount); + Assert.assertTrue(apiServer.forgotPassword(userAccount)); + Mockito.verify(passwordReset).setResetTokenAndSend(userAccount); + } + + @Test(expected = CloudRuntimeException.class) + public void testForgotPasswordFailureNoEmail() { + UserAccount userAccount = Mockito.mock(UserAccount.class); + + Mockito.when(userAccount.getDetails()).thenReturn(Collections.emptyMap()); + Mockito.when(userAccount.getEmail()).thenReturn(""); + Assert.assertTrue(apiServer.forgotPassword(userAccount)); + } + + @Test + public void testForgotPasswordSuccessSecondRequestExpired() { + UserAccount userAccount = Mockito.mock(UserAccount.class); + + Mockito.when(userAccount.getDetails()).thenReturn(Map.of( + PasswordResetToken, "token", + PasswordResetTokenExpiryDate, String.valueOf(System.currentTimeMillis() - 5 * 60 * 1000) + )); + Mockito.when(userAccount.getEmail()).thenReturn("test@test.com"); + Mockito.doNothing().when(passwordReset).setResetTokenAndSend(userAccount); + Assert.assertTrue(apiServer.forgotPassword(userAccount)); + Mockito.verify(passwordReset).setResetTokenAndSend(userAccount); + } + + @Test + public void testForgotPasswordSuccessSecondRequestUnexpired() { + UserAccount userAccount = Mockito.mock(UserAccount.class); + + Mockito.when(userAccount.getDetails()).thenReturn(Map.of( + PasswordResetToken, "token", + PasswordResetTokenExpiryDate, String.valueOf(System.currentTimeMillis() + 5 * 60 * 1000) + )); + Mockito.when(userAccount.getEmail()).thenReturn("test@test.com"); + Assert.assertTrue(apiServer.forgotPassword(userAccount)); + Mockito.verify(passwordReset, Mockito.never()).setResetTokenAndSend(userAccount); + } } diff --git a/server/src/test/java/com/cloud/user/AccountManagerImplTest.java b/server/src/test/java/com/cloud/user/AccountManagerImplTest.java index 9d780096abfc..09b9d0f27721 100644 --- a/server/src/test/java/com/cloud/user/AccountManagerImplTest.java +++ b/server/src/test/java/com/cloud/user/AccountManagerImplTest.java @@ -372,7 +372,7 @@ private void prepareMockAndExecuteUpdateUserTest(int numberOfExpectedCallsForSet Mockito.doNothing().when(accountManagerImpl).validateAndUpdateFirstNameIfNeeded(UpdateUserCmdMock, userVoMock); Mockito.doNothing().when(accountManagerImpl).validateAndUpdateLastNameIfNeeded(UpdateUserCmdMock, userVoMock); Mockito.doNothing().when(accountManagerImpl).validateAndUpdateUsernameIfNeeded(UpdateUserCmdMock, userVoMock, accountMock); - Mockito.doNothing().when(accountManagerImpl).validateUserPasswordAndUpdateIfNeeded(Mockito.anyString(), Mockito.eq(userVoMock), Mockito.anyString()); + Mockito.doNothing().when(accountManagerImpl).validateUserPasswordAndUpdateIfNeeded(Mockito.anyString(), Mockito.eq(userVoMock), Mockito.anyString(), Mockito.eq(false)); Mockito.doReturn(true).when(userDaoMock).update(Mockito.anyLong(), Mockito.eq(userVoMock)); Mockito.doReturn(Mockito.mock(UserAccountVO.class)).when(userAccountDaoMock).findById(Mockito.anyLong()); @@ -388,7 +388,7 @@ private void prepareMockAndExecuteUpdateUserTest(int numberOfExpectedCallsForSet inOrder.verify(accountManagerImpl).validateAndUpdateFirstNameIfNeeded(UpdateUserCmdMock, userVoMock); inOrder.verify(accountManagerImpl).validateAndUpdateLastNameIfNeeded(UpdateUserCmdMock, userVoMock); inOrder.verify(accountManagerImpl).validateAndUpdateUsernameIfNeeded(UpdateUserCmdMock, userVoMock, accountMock); - inOrder.verify(accountManagerImpl).validateUserPasswordAndUpdateIfNeeded(UpdateUserCmdMock.getPassword(), userVoMock, UpdateUserCmdMock.getCurrentPassword()); + inOrder.verify(accountManagerImpl).validateUserPasswordAndUpdateIfNeeded(UpdateUserCmdMock.getPassword(), userVoMock, UpdateUserCmdMock.getCurrentPassword(), false); inOrder.verify(userVoMock, Mockito.times(numberOfExpectedCallsForSetEmailAndSetTimeZone)).setEmail(Mockito.anyString()); inOrder.verify(userVoMock, Mockito.times(numberOfExpectedCallsForSetEmailAndSetTimeZone)).setTimezone(Mockito.anyString()); @@ -674,14 +674,14 @@ public void validateAndUpdateUsernameIfNeededTestNoDuplicatedUserNames() { @Test public void valiateUserPasswordAndUpdateIfNeededTestPasswordNull() { - accountManagerImpl.validateUserPasswordAndUpdateIfNeeded(null, userVoMock, null); + accountManagerImpl.validateUserPasswordAndUpdateIfNeeded(null, userVoMock, null, false); Mockito.verify(userVoMock, Mockito.times(0)).setPassword(Mockito.anyString()); } @Test(expected = InvalidParameterValueException.class) public void valiateUserPasswordAndUpdateIfNeededTestBlankPassword() { - accountManagerImpl.validateUserPasswordAndUpdateIfNeeded(" ", userVoMock, null); + accountManagerImpl.validateUserPasswordAndUpdateIfNeeded(" ", userVoMock, null, false); } @Test(expected = InvalidParameterValueException.class) @@ -695,7 +695,7 @@ public void valiateUserPasswordAndUpdateIfNeededTestNoAdminAndNoCurrentPasswordP Mockito.lenient().doNothing().when(passwordPolicyMock).verifyIfPasswordCompliesWithPasswordPolicies(Mockito.anyString(), Mockito.anyString(), Mockito.anyLong()); - accountManagerImpl.validateUserPasswordAndUpdateIfNeeded("newPassword", userVoMock, " "); + accountManagerImpl.validateUserPasswordAndUpdateIfNeeded("newPassword", userVoMock, " ", false); } @Test(expected = CloudRuntimeException.class) @@ -710,7 +710,7 @@ public void valiateUserPasswordAndUpdateIfNeededTestNoUserAuthenticatorsConfigur Mockito.lenient().doNothing().when(passwordPolicyMock).verifyIfPasswordCompliesWithPasswordPolicies(Mockito.anyString(), Mockito.anyString(), Mockito.anyLong()); - accountManagerImpl.validateUserPasswordAndUpdateIfNeeded("newPassword", userVoMock, null); + accountManagerImpl.validateUserPasswordAndUpdateIfNeeded("newPassword", userVoMock, null, false); } @Test @@ -729,7 +729,7 @@ public void validateUserPasswordAndUpdateIfNeededTestRootAdminUpdatingUserPasswo Mockito.lenient().doNothing().when(passwordPolicyMock).verifyIfPasswordCompliesWithPasswordPolicies(Mockito.anyString(), Mockito.anyString(), Mockito.anyLong()); - accountManagerImpl.validateUserPasswordAndUpdateIfNeeded(newPassword, userVoMock, null); + accountManagerImpl.validateUserPasswordAndUpdateIfNeeded(newPassword, userVoMock, null, false); Mockito.verify(accountManagerImpl, Mockito.times(0)).validateCurrentPassword(Mockito.eq(userVoMock), Mockito.anyString()); Mockito.verify(userVoMock, Mockito.times(1)).setPassword(expectedUserPasswordAfterEncoded); @@ -751,7 +751,7 @@ public void validateUserPasswordAndUpdateIfNeededTestDomainAdminUpdatingUserPass Mockito.lenient().doNothing().when(passwordPolicyMock).verifyIfPasswordCompliesWithPasswordPolicies(Mockito.anyString(), Mockito.anyString(), Mockito.anyLong()); - accountManagerImpl.validateUserPasswordAndUpdateIfNeeded(newPassword, userVoMock, null); + accountManagerImpl.validateUserPasswordAndUpdateIfNeeded(newPassword, userVoMock, null, false); Mockito.verify(accountManagerImpl, Mockito.times(0)).validateCurrentPassword(Mockito.eq(userVoMock), Mockito.anyString()); Mockito.verify(userVoMock, Mockito.times(1)).setPassword(expectedUserPasswordAfterEncoded); @@ -774,7 +774,7 @@ public void validateUserPasswordAndUpdateIfNeededTestUserUpdatingHisPassword() { Mockito.lenient().doNothing().when(passwordPolicyMock).verifyIfPasswordCompliesWithPasswordPolicies(Mockito.anyString(), Mockito.anyString(), Mockito.anyLong()); - accountManagerImpl.validateUserPasswordAndUpdateIfNeeded(newPassword, userVoMock, currentPassword); + accountManagerImpl.validateUserPasswordAndUpdateIfNeeded(newPassword, userVoMock, currentPassword, false); Mockito.verify(accountManagerImpl, Mockito.times(1)).validateCurrentPassword(userVoMock, currentPassword); Mockito.verify(userVoMock, Mockito.times(1)).setPassword(expectedUserPasswordAfterEncoded); @@ -793,7 +793,7 @@ public void validateUserPasswordAndUpdateIfNeededTestIfVerifyIfPasswordCompliesW Mockito.doThrow(new InvalidParameterValueException("")).when(passwordPolicyMock).verifyIfPasswordCompliesWithPasswordPolicies(Mockito.anyString(), Mockito.anyString(), Mockito.anyLong()); - accountManagerImpl.validateUserPasswordAndUpdateIfNeeded(newPassword, userVoMock, currentPassword); + accountManagerImpl.validateUserPasswordAndUpdateIfNeeded(newPassword, userVoMock, currentPassword, false); } private String configureUserMockAuthenticators(String newPassword) { diff --git a/server/src/test/java/com/cloud/user/MockAccountManagerImpl.java b/server/src/test/java/com/cloud/user/MockAccountManagerImpl.java index 334e1f334818..739e1d522197 100644 --- a/server/src/test/java/com/cloud/user/MockAccountManagerImpl.java +++ b/server/src/test/java/com/cloud/user/MockAccountManagerImpl.java @@ -483,4 +483,8 @@ public ConfigKey[] getConfigKeys() { public List getApiNameList() { return null; } + + @Override + public void validateUserPasswordAndUpdateIfNeeded(String newPassword, UserVO user, String currentPassword, boolean skipCurrentPassValidation) { + } } diff --git a/server/src/test/java/org/apache/cloudstack/user/PasswordResetImplTest.java b/server/src/test/java/org/apache/cloudstack/user/PasswordResetImplTest.java new file mode 100644 index 000000000000..8d729f289216 --- /dev/null +++ b/server/src/test/java/org/apache/cloudstack/user/PasswordResetImplTest.java @@ -0,0 +1,115 @@ +// 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. +package org.apache.cloudstack.user; + +import com.cloud.user.UserAccount; +import org.apache.cloudstack.api.ServerApiException; +import org.apache.cloudstack.framework.config.ConfigKey; +import org.apache.cloudstack.resourcedetail.UserDetailVO; +import org.apache.cloudstack.resourcedetail.dao.UserDetailsDao; +import org.junit.Assert; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.Spy; +import org.mockito.junit.MockitoJUnitRunner; + +import static org.apache.cloudstack.resourcedetail.UserDetailVO.PasswordResetToken; +import static org.apache.cloudstack.resourcedetail.UserDetailVO.PasswordResetTokenExpiryDate; + +@RunWith(MockitoJUnitRunner.class) +public class PasswordResetImplTest { + @Spy + @InjectMocks + PasswordResetImpl passwordReset; + + @Mock + private UserDetailsDao userDetailsDao; + + @Test + public void testGetMessageBody() { + ConfigKey passwordResetMailTemplate = Mockito.mock(ConfigKey.class); + PasswordResetImpl.PasswordResetMailTemplate = passwordResetMailTemplate; + Mockito.when(passwordResetMailTemplate.value()).thenReturn("Hello {{username}}!\n" + + "You have requested to reset your password. Please click the following link to reset your password:\n" + + "{{{resetLink}}}\n" + + "If you did not request a password reset, please ignore this email.\n" + + "\n" + + "Regards,\n" + + "The CloudStack Team"); + + UserAccount userAccount = Mockito.mock(UserAccount.class); + Mockito.when(userAccount.getUsername()).thenReturn("test_user"); + + String messageBody = passwordReset.getMessageBody(userAccount, "reset_token", "reset_link"); + String expectedMessageBody = "Hello test_user!\n" + + "You have requested to reset your password. Please click the following link to reset your password:\n" + + "reset_link\n" + + "If you did not request a password reset, please ignore this email.\n" + + "\n" + + "Regards,\n" + + "The CloudStack Team"; + Assert.assertEquals("Message body doesn't match", expectedMessageBody, messageBody); + } + + @Test + public void testValidateAndResetPassword() { + UserAccount userAccount = Mockito.mock(UserAccount.class); + Mockito.when(userAccount.getId()).thenReturn(1L); + Mockito.when(userAccount.getUsername()).thenReturn("test_user"); + + Mockito.doNothing().when(passwordReset).resetPassword(userAccount, "new_password"); + + UserDetailVO resetTokenDetail = Mockito.mock(UserDetailVO.class); + UserDetailVO resetTokenExpiryDate = Mockito.mock(UserDetailVO.class); + Mockito.when(userDetailsDao.findDetail(1L, PasswordResetToken)).thenReturn(resetTokenDetail); + Mockito.when(userDetailsDao.findDetail(1L, PasswordResetTokenExpiryDate)).thenReturn(resetTokenExpiryDate); + Mockito.when(resetTokenExpiryDate.getValue()).thenReturn(String.valueOf(System.currentTimeMillis() - 5 * 60 * 1000)); + + try { + passwordReset.validateAndResetPassword(userAccount, "reset_token", "new_password"); + Assert.fail("Should have thrown exception"); + } catch (ServerApiException e) { + Assert.assertEquals("No reset token found for user test_user", e.getMessage()); + } + + Mockito.when(resetTokenDetail.getValue()).thenReturn("reset_token_XXX"); + + try { + passwordReset.validateAndResetPassword(userAccount, "reset_token", "new_password"); + Assert.fail("Should have thrown exception"); + } catch (ServerApiException e) { + Assert.assertEquals("Invalid reset token for user test_user", e.getMessage()); + } + + Mockito.when(resetTokenDetail.getValue()).thenReturn("reset_token"); + + try { + passwordReset.validateAndResetPassword(userAccount, "reset_token", "new_password"); + Assert.fail("Should have thrown exception"); + } catch (ServerApiException e) { + Assert.assertEquals("Reset token has expired for user test_user", e.getMessage()); + } + + Mockito.when(resetTokenExpiryDate.getValue()).thenReturn(String.valueOf(System.currentTimeMillis() + 5 * 60 * 1000)); + + Assert.assertTrue(passwordReset.validateAndResetPassword(userAccount, "reset_token", "new_password")); + Mockito.verify(passwordReset, Mockito.times(1)).resetPassword(userAccount, "new_password"); + } +} diff --git a/tools/apidoc/gen_toc.py b/tools/apidoc/gen_toc.py index 872acc21a081..1763f73eff6d 100644 --- a/tools/apidoc/gen_toc.py +++ b/tools/apidoc/gen_toc.py @@ -278,7 +278,9 @@ 'importVm': 'Virtual Machine', 'Webhook': 'Webhook', 'Webhooks': 'Webhook', - 'purgeExpungedResources': 'Resource' + 'purgeExpungedResources': 'Resource', + 'forgotPassword': 'Authentication', + 'resetPassword': 'Authentication', } diff --git a/ui/public/locales/en.json b/ui/public/locales/en.json index e922819fc668..ec5b8bb0b2e6 100644 --- a/ui/public/locales/en.json +++ b/ui/public/locales/en.json @@ -392,6 +392,7 @@ "label.availableprocessors": "Available processor cores", "label.availablevirtualmachinecount": "Available Instances", "label.back": "Back", +"label.back.login": "Back to login", "label.backup": "Backups", "label.backup.attach.restore": "Restore and attach backup volume", "label.backup.configure.schedule": "Configure Backup Schedule", @@ -951,6 +952,7 @@ "label.force.reboot": "Force reboot", "label.forceencap": "Force UDP encapsulation of ESP packets", "label.forgedtransmits": "Forged transmits", +"label.forgot.password": "Forgot password?", "label.format": "Format", "label.fornsx": "NSX", "label.forvpc": "VPC", @@ -3056,6 +3058,7 @@ "message.failed.to.add": "Failed to add", "message.failed.to.assign.vms": "Failed to assign Instances", "message.failed.to.remove": "Failed to remove", +"message.forgot.password.success": "An email has been sent to your email address with instructions on how to reset your password.", "message.generate.keys": "Please confirm that you would like to generate new API/Secret keys for this User.", "message.chart.statistic.info": "The shown charts are self-adjustable, that means, if the value gets close to the limit or overpass it, it will grow to adjust the shown value", "message.chart.statistic.info.hypervisor.additionals": "The metrics data depend on the hypervisor plugin used for each hypervisor. The behavior can vary across different hypervisors. For instance, with KVM, metrics are real-time statistics provided by libvirt. In contrast, with VMware, the metrics are averaged data for a given time interval controlled by configuration.", @@ -3179,6 +3182,8 @@ "message.offering.internet.protocol.warning": "WARNING: IPv6 supported Networks use static routing and will require upstream routes to be configured manually.", "message.offering.ipv6.warning": "Please refer documentation for creating IPv6 enabled Network/VPC offering IPv6 support in CloudStack - Isolated Networks and VPC Network Tiers", "message.ovf.configurations": "OVF configurations available for the selected appliance. Please select the desired value. Incompatible compute offerings will get disabled.", +"message.password.reset.failed": "Failed to reset password.", +"message.password.reset.success": "Password has been reset successfully. Please login using your new credentials.", "message.path.description": "NFS: exported path from the server. VMFS: /datacenter name/datastore name. SharedMountPoint: path where primary storage is mounted, such as /mnt/primary.", "message.please.confirm.remove.ssh.key.pair": "Please confirm that you want to remove this SSH key pair.", "message.please.confirm.remove.user.data": "Please confirm that you want to remove this Userdata", diff --git a/ui/src/config/router.js b/ui/src/config/router.js index 90fae577ce42..ccb6d10d1378 100644 --- a/ui/src/config/router.js +++ b/ui/src/config/router.js @@ -299,6 +299,16 @@ export const constantRouterMap = [ path: 'login', name: 'login', component: () => import(/* webpackChunkName: "auth" */ '@/views/auth/Login') + }, + { + path: 'forgotPassword', + name: 'forgotPassword', + component: () => import(/* webpackChunkName: "auth" */ '@/views/auth/ForgotPassword') + }, + { + path: 'resetPassword', + name: 'resetPassword', + component: () => import(/* webpackChunkName: "auth" */ '@/views/auth/ResetPassword') } ] }, diff --git a/ui/src/permission.js b/ui/src/permission.js index 4380c7660d86..266dc992c8db 100644 --- a/ui/src/permission.js +++ b/ui/src/permission.js @@ -30,7 +30,7 @@ import { ACCESS_TOKEN, APIS, SERVER_MANAGER, CURRENT_PROJECT } from '@/store/mut NProgress.configure({ showSpinner: false }) // NProgress Configuration -const allowList = ['login', 'VerifyOauth'] // no redirect allowlist +const allowList = ['login', 'VerifyOauth', 'forgotPassword', 'resetPassword'] // no redirect allowlist router.beforeEach((to, from, next) => { // start progress bar diff --git a/ui/src/views/auth/ForgotPassword.vue b/ui/src/views/auth/ForgotPassword.vue new file mode 100644 index 000000000000..cb0e5f68626b --- /dev/null +++ b/ui/src/views/auth/ForgotPassword.vue @@ -0,0 +1,268 @@ +// 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. + + + + + + diff --git a/ui/src/views/auth/Login.vue b/ui/src/views/auth/Login.vue index 8503f71082b5..e97dddcfc3bf 100644 --- a/ui/src/views/auth/Login.vue +++ b/ui/src/views/auth/Login.vue @@ -152,7 +152,17 @@ @click="handleSubmit" >{{ $t('label.login') }} - + + + + + + + {{ $t('label.forgot.password') }} + + + +

or

diff --git a/ui/src/views/auth/ResetPassword.vue b/ui/src/views/auth/ResetPassword.vue new file mode 100644 index 000000000000..c48e9118e5b3 --- /dev/null +++ b/ui/src/views/auth/ResetPassword.vue @@ -0,0 +1,337 @@ +// 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. + + + + + + diff --git a/utils/src/main/java/org/apache/cloudstack/utils/mailing/SMTPMailSender.java b/utils/src/main/java/org/apache/cloudstack/utils/mailing/SMTPMailSender.java index 4afa3c9100bc..b354772fde08 100644 --- a/utils/src/main/java/org/apache/cloudstack/utils/mailing/SMTPMailSender.java +++ b/utils/src/main/java/org/apache/cloudstack/utils/mailing/SMTPMailSender.java @@ -48,16 +48,16 @@ public class SMTPMailSender { protected Session session = null; protected SMTPSessionProperties sessionProps; - protected static final String CONFIG_HOST = "host"; - protected static final String CONFIG_PORT = "port"; - protected static final String CONFIG_USE_AUTH = "useAuth"; - protected static final String CONFIG_USERNAME = "username"; - protected static final String CONFIG_PASSWORD = "password"; - protected static final String CONFIG_DEBUG_MODE = "debug"; - protected static final String CONFIG_USE_STARTTLS = "useStartTLS"; - protected static final String CONFIG_ENABLED_SECURITY_PROTOCOLS = "enabledSecurityProtocols"; - protected static final String CONFIG_TIMEOUT = "timeout"; - protected static final String CONFIG_CONNECTION_TIMEOUT = "connectiontimeout"; + public static final String CONFIG_HOST = "host"; + public static final String CONFIG_PORT = "port"; + public static final String CONFIG_USE_AUTH = "useAuth"; + public static final String CONFIG_USERNAME = "username"; + public static final String CONFIG_PASSWORD = "password"; + public static final String CONFIG_DEBUG_MODE = "debug"; + public static final String CONFIG_USE_STARTTLS = "useStartTLS"; + public static final String CONFIG_ENABLED_SECURITY_PROTOCOLS = "enabledSecurityProtocols"; + public static final String CONFIG_TIMEOUT = "timeout"; + public static final String CONFIG_CONNECTION_TIMEOUT = "connectiontimeout"; protected Map configs; protected String namespace; From 77ac320bcdb4e8205a0977d7c5cd381b07463203 Mon Sep 17 00:00:00 2001 From: Vishesh Date: Tue, 27 Aug 2024 17:12:54 +0530 Subject: [PATCH 2/6] Address comments --- .../cloudstack/api/ApiServerService.java | 3 +- pom.xml | 1 + server/pom.xml | 2 +- .../main/java/com/cloud/api/ApiServer.java | 45 ++++++-- ...aultForgotPasswordAPIAuthenticatorCmd.java | 3 +- ...faultResetPasswordAPIAuthenticatorCmd.java | 1 + ...rdReset.java => PasswordResetManager.java} | 16 +-- ...mpl.java => PasswordResetManagerImpl.java} | 55 ++++++++-- .../spring-server-core-managers-context.xml | 2 +- .../java/com/cloud/api/ApiServerTest.java | 101 ++++++++++++++---- ...java => PasswordResetManagerImplTest.java} | 6 +- 11 files changed, 181 insertions(+), 54 deletions(-) rename server/src/main/java/org/apache/cloudstack/user/{PasswordReset.java => PasswordResetManager.java} (82%) rename server/src/main/java/org/apache/cloudstack/user/{PasswordResetImpl.java => PasswordResetManagerImpl.java} (74%) rename server/src/test/java/org/apache/cloudstack/user/{PasswordResetImplTest.java => PasswordResetManagerImplTest.java} (96%) diff --git a/api/src/main/java/org/apache/cloudstack/api/ApiServerService.java b/api/src/main/java/org/apache/cloudstack/api/ApiServerService.java index a7d99f4546f6..cbbcdc3bda42 100644 --- a/api/src/main/java/org/apache/cloudstack/api/ApiServerService.java +++ b/api/src/main/java/org/apache/cloudstack/api/ApiServerService.java @@ -21,6 +21,7 @@ import javax.servlet.http.HttpSession; +import com.cloud.domain.Domain; import com.cloud.exception.CloudAuthenticationException; import com.cloud.user.UserAccount; @@ -44,7 +45,7 @@ public ResponseObject loginUser(HttpSession session, String username, String pas public Class getCmdClass(String cmdName); - boolean forgotPassword(UserAccount userAccount); + boolean forgotPassword(UserAccount userAccount, Domain domain); boolean resetPassword(UserAccount userAccount, String token, String password); } diff --git a/pom.xml b/pom.xml index 727ac2f7c5be..3b86f0bef2a7 100644 --- a/pom.xml +++ b/pom.xml @@ -166,6 +166,7 @@ 2.7.0 0.5.3 1.5.0-b01 + 0.9.14 8.0.33 2.0.4 10.1 diff --git a/server/pom.xml b/server/pom.xml index 1d822a2444c3..38054679a0fd 100644 --- a/server/pom.xml +++ b/server/pom.xml @@ -104,7 +104,7 @@ com.github.spullara.mustache.java compiler - 0.9.14 + ${cs.mustache.version} org.apache.cloudstack diff --git a/server/src/main/java/com/cloud/api/ApiServer.java b/server/src/main/java/com/cloud/api/ApiServer.java index bc31f76f4d3b..468de4d7d1b9 100644 --- a/server/src/main/java/com/cloud/api/ApiServer.java +++ b/server/src/main/java/com/cloud/api/ApiServer.java @@ -110,8 +110,9 @@ 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.user.PasswordReset; +import org.apache.cloudstack.user.PasswordResetManager; import org.apache.commons.codec.binary.Base64; +import org.apache.commons.lang3.EnumUtils; import org.apache.http.ConnectionClosedException; import org.apache.http.HttpException; import org.apache.http.HttpRequest; @@ -219,7 +220,7 @@ public class ApiServer extends ManagerBase implements HttpRequestHandler, ApiSer @Inject private UUIDManager uuidMgr; @Inject - private PasswordReset passwordReset; + private PasswordResetManager passwordResetManager; private List pluggableServices; @@ -1230,29 +1231,59 @@ public boolean verifyUser(final Long userId) { } @Override - public boolean forgotPassword(UserAccount userAccount) { + public boolean forgotPassword(UserAccount userAccount, Domain domain) { String resetToken = userAccount.getDetails().get(PasswordResetToken); String resetTokenExpiryTimeString = userAccount.getDetails().getOrDefault(PasswordResetTokenExpiryDate, "0"); if (StringUtils.isBlank(userAccount.getEmail())) { - throw new CloudRuntimeException("Email is not set for the user. Please contact your administrator."); + logger.error(String.format( + "Email is not set. username: %s account id: %d domain id: %d", + userAccount.getUsername(), userAccount.getAccountId(), userAccount.getDomainId())); + throw new CloudRuntimeException("Email is not set for the user."); + } + + if (!EnumUtils.getEnumIgnoreCase(Account.State.class, userAccount.getState()).equals(Account.State.ENABLED)) { + logger.error(String.format( + "User is not enabled. username: %s account id: %d domain id: %s", + userAccount.getUsername(), userAccount.getAccountId(), domain.getUuid())); + throw new CloudRuntimeException("User is not enabled."); + } + + if (!EnumUtils.getEnumIgnoreCase(Account.State.class, userAccount.getAccountState()).equals(Account.State.ENABLED)) { + logger.error(String.format( + "Account is not enabled. username: %s account id: %d domain id: %s", + userAccount.getUsername(), userAccount.getAccountId(), domain.getUuid())); + throw new CloudRuntimeException("Account is not enabled."); + } + + if (!domain.getState().equals(Domain.State.Active)) { + logger.error(String.format( + "Domain is not active. username: %s account id: %d domain id: %s", + userAccount.getUsername(), userAccount.getAccountId(), domain.getUuid())); + throw new CloudRuntimeException("Domain is not active."); } if (StringUtils.isNotEmpty(resetToken) && StringUtils.isNotEmpty(resetTokenExpiryTimeString)) { final Date resetTokenExpiryTime = new Date(Long.parseLong(resetTokenExpiryTimeString)); final Date currentTime = new Date(); if (currentTime.after(resetTokenExpiryTime)) { - passwordReset.setResetTokenAndSend(userAccount); + passwordResetManager.setResetTokenAndSend(userAccount); } } else if (StringUtils.isEmpty(resetToken)) { - passwordReset.setResetTokenAndSend(userAccount); + passwordResetManager.setResetTokenAndSend(userAccount); + } else { + logger.debug(String.format( + "Password reset token is already set for user %s in " + + "domain id: %s with account %s and email %s", + userAccount.getUsername(), domain.getUuid(), + userAccount.getAccountName(), userAccount.getEmail())); } return true; } @Override public boolean resetPassword(UserAccount userAccount, String token, String password) { - return passwordReset.validateAndResetPassword(userAccount, token, password); + return passwordResetManager.validateAndResetPassword(userAccount, token, password); } private void checkCommandAvailable(final User user, final String commandName, final InetAddress remoteAddress) throws PermissionDeniedException { diff --git a/server/src/main/java/com/cloud/api/auth/DefaultForgotPasswordAPIAuthenticatorCmd.java b/server/src/main/java/com/cloud/api/auth/DefaultForgotPasswordAPIAuthenticatorCmd.java index 172bcd9fb1d2..d8d7e361c6ac 100644 --- a/server/src/main/java/com/cloud/api/auth/DefaultForgotPasswordAPIAuthenticatorCmd.java +++ b/server/src/main/java/com/cloud/api/auth/DefaultForgotPasswordAPIAuthenticatorCmd.java @@ -46,6 +46,7 @@ @APICommand(name = "forgotPassword", description = "Sends an email to the user with a token to reset the password using resetPassword command.", + since = "4.20.0.0", requestHasSensitiveInfo = true, responseObject = SuccessResponse.class) public class DefaultForgotPasswordAPIAuthenticatorCmd extends BaseCmd implements APIAuthenticator { @@ -113,7 +114,7 @@ public String authenticate(String command, Map params, HttpSes if (userAccount != null && List.of(User.Source.SAML2, User.Source.OAUTH2, User.Source.LDAP).contains(userAccount.getSource())) { throw new ServerApiException(ApiErrorCode.PARAM_ERROR, "Forgot Password is not allowed for this user"); } - boolean success = _apiServer.forgotPassword(userAccount); + boolean success = _apiServer.forgotPassword(userAccount, userDomain); SuccessResponse successResponse = new SuccessResponse(); successResponse.setSuccess(success); successResponse.setResponseName(getCommandName()); diff --git a/server/src/main/java/com/cloud/api/auth/DefaultResetPasswordAPIAuthenticatorCmd.java b/server/src/main/java/com/cloud/api/auth/DefaultResetPasswordAPIAuthenticatorCmd.java index 57740c2b922a..077efdee0879 100644 --- a/server/src/main/java/com/cloud/api/auth/DefaultResetPasswordAPIAuthenticatorCmd.java +++ b/server/src/main/java/com/cloud/api/auth/DefaultResetPasswordAPIAuthenticatorCmd.java @@ -48,6 +48,7 @@ @APICommand(name = "resetPassword", description = "Resets the password for the user using the token generated via forgotPassword command.", + since = "4.20.0.0", requestHasSensitiveInfo = true, responseObject = SuccessResponse.class) public class DefaultResetPasswordAPIAuthenticatorCmd extends BaseCmd implements APIAuthenticator { diff --git a/server/src/main/java/org/apache/cloudstack/user/PasswordReset.java b/server/src/main/java/org/apache/cloudstack/user/PasswordResetManager.java similarity index 82% rename from server/src/main/java/org/apache/cloudstack/user/PasswordReset.java rename to server/src/main/java/org/apache/cloudstack/user/PasswordResetManager.java index feba06a6eb0a..f4eef55504c4 100644 --- a/server/src/main/java/org/apache/cloudstack/user/PasswordReset.java +++ b/server/src/main/java/org/apache/cloudstack/user/PasswordResetManager.java @@ -20,33 +20,33 @@ import com.cloud.user.UserAccount; import org.apache.cloudstack.framework.config.ConfigKey; -public interface PasswordReset { +public interface PasswordResetManager { ConfigKey PasswordResetTtl = new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED, Long.class, - "password.reset.ttl", "30", + "user.password.reset.ttl", "30", "Password reset ttl in minutes", true, ConfigKey.Scope.Global); ConfigKey PasswordResetEmailSender = new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED, - String.class, "password.reset.email.sender", null, + String.class, "user.password.reset.email.sender", null, "Password reset email sender", true, ConfigKey.Scope.Global); ConfigKey PasswordResetSMTPHost = new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED, - String.class, "password.reset.smtp.host", null, + String.class, "user.password.reset.smtp.host", null, "Password reset smtp host", false, ConfigKey.Scope.Global); ConfigKey PasswordResetSMTPPort = new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED, - Integer.class, "password.reset.smtp.port", "25", + Integer.class, "user.password.reset.smtp.port", "25", "Password reset smtp port", false, ConfigKey.Scope.Global); ConfigKey PasswordResetSMTPUseAuth = new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED, - Boolean.class, "password.reset.smtp.useAuth", "false", + Boolean.class, "user.password.reset.smtp.useAuth", "false", "Use auth for smtp in Password reset", false, ConfigKey.Scope.Global); ConfigKey PasswordResetSMTPUsername = new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED, - String.class, "password.reset.smtp.username", null, + String.class, "user.password.reset.smtp.username", null, "Password reset smtp username", false, ConfigKey.Scope.Global); ConfigKey PasswordResetSMTPPassword = new ConfigKey<>("Secure", String.class, - "password.reset.smtp.password", null, + "user.password.reset.smtp.password", null, "Password reset smtp password", false, ConfigKey.Scope.Global); void setResetTokenAndSend(UserAccount userAccount); diff --git a/server/src/main/java/org/apache/cloudstack/user/PasswordResetImpl.java b/server/src/main/java/org/apache/cloudstack/user/PasswordResetManagerImpl.java similarity index 74% rename from server/src/main/java/org/apache/cloudstack/user/PasswordResetImpl.java rename to server/src/main/java/org/apache/cloudstack/user/PasswordResetManagerImpl.java index 777479d281e6..962e27678bab 100644 --- a/server/src/main/java/org/apache/cloudstack/user/PasswordResetImpl.java +++ b/server/src/main/java/org/apache/cloudstack/user/PasswordResetManagerImpl.java @@ -52,7 +52,7 @@ import static org.apache.cloudstack.resourcedetail.UserDetailVO.PasswordResetToken; import static org.apache.cloudstack.resourcedetail.UserDetailVO.PasswordResetTokenExpiryDate; -public class PasswordResetImpl extends ManagerBase implements PasswordReset, Configurable { +public class PasswordResetManagerImpl extends ManagerBase implements PasswordResetManager, Configurable { @Inject private AccountManager accountManager; @@ -81,7 +81,7 @@ public class PasswordResetImpl extends ManagerBase implements PasswordReset, Con @Override public String getConfigComponentName() { - return PasswordResetImpl.class.getSimpleName(); + return PasswordResetManagerImpl.class.getSimpleName(); } @Override @@ -105,17 +105,19 @@ public boolean configure(String name, Map params) throws Configu String username = PasswordResetSMTPUsername.value(); String password = PasswordResetSMTPPassword.value(); - String namespace = "password.reset.smtp"; + if (!StringUtils.isEmpty(smtpHost) && smtpPort != null && smtpPort > 0) { + String namespace = "password.reset.smtp"; - Map configs = new HashMap<>(); + Map configs = new HashMap<>(); - configs.put(getKey(namespace, SMTPMailSender.CONFIG_HOST), smtpHost); - configs.put(getKey(namespace, SMTPMailSender.CONFIG_PORT), smtpPort.toString()); - configs.put(getKey(namespace, SMTPMailSender.CONFIG_USE_AUTH), useAuth.toString()); - configs.put(getKey(namespace, SMTPMailSender.CONFIG_USERNAME), username); - configs.put(getKey(namespace, SMTPMailSender.CONFIG_PASSWORD), password); + configs.put(getKey(namespace, SMTPMailSender.CONFIG_HOST), smtpHost); + configs.put(getKey(namespace, SMTPMailSender.CONFIG_PORT), smtpPort.toString()); + configs.put(getKey(namespace, SMTPMailSender.CONFIG_USE_AUTH), useAuth.toString()); + configs.put(getKey(namespace, SMTPMailSender.CONFIG_USERNAME), username); + configs.put(getKey(namespace, SMTPMailSender.CONFIG_PASSWORD), password); - mailSender = new SMTPMailSender(configs, namespace); + mailSender = new SMTPMailSender(configs, namespace); + } return true; } @@ -125,6 +127,12 @@ private String getKey(String namespace, String config) { public void setResetTokenAndSend(UserAccount userAccount) { + if (mailSender == null) { + logger.debug("Failed to reset token and send email. SMTP mail sender is not configured."); + throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, + "Failed to reset token and send email. SMTP mail sender is not configured"); + } + final String resetToken = UUID.randomUUID().toString(); final Date resetTokenExpiryTime = new Date(System.currentTimeMillis() + PasswordResetTtl.value() * 60 * 1000); @@ -135,7 +143,8 @@ public void setResetTokenAndSend(UserAccount userAccount) { final String username = userAccount.getUsername(); final String subject = "Password Reset Request"; - String resetLink = String.format("%s/user/resetPassword?username=%s&token=%s", ManagementServerAddresses.value(), username, resetToken); + String resetLink = String.format("%s/user/resetPassword?username=%s&token=%s", + ManagementServerAddresses.value().split(",")[0], username, resetToken); String content = getMessageBody(userAccount, resetToken, resetLink); SMTPMailProperties mailProperties = new SMTPMailProperties(); @@ -152,6 +161,11 @@ public void setResetTokenAndSend(UserAccount userAccount) { mailProperties.setRecipients(addresses); mailSender.sendMail(mailProperties); + logger.debug(String.format( + "User password reset email for user id: %d username: %s account id: %d" + + " domain id:%d sent to %s with token expiry at %s", + userAccount.getId(), username, userAccount.getAccountId(), + userAccount.getDomainId(), email, resetTokenExpiryTime)); } @Override @@ -160,6 +174,10 @@ public boolean validateAndResetPassword(UserAccount user, String token, String p UserDetailVO resetTokenExpiryDate = userDetailsDao.findDetail(user.getId(), PasswordResetTokenExpiryDate); if (resetTokenDetail == null || resetTokenExpiryDate == null) { + logger.debug(String.format( + "Failed to reset password. No reset token found for user id: %d username: %s account" + + " id: %d domain id: %d", + user.getId(), user.getUsername(), user.getAccountId(), user.getDomainId())); throw new ServerApiException(ApiErrorCode.PARAM_ERROR, String.format("No reset token found for user %s", user.getUsername())); } @@ -168,16 +186,31 @@ public boolean validateAndResetPassword(UserAccount user, String token, String p Date now = new Date(); String resetToken = resetTokenDetail.getValue(); if (StringUtils.isEmpty(resetToken)) { + logger.debug(String.format( + "Failed to reset password. No reset token found for user id: %d username: %s account" + + " id: %d domain id: %d", + user.getId(), user.getUsername(), user.getAccountId(), user.getDomainId())); throw new ServerApiException(ApiErrorCode.PARAM_ERROR, String.format("No reset token found for user %s", user.getUsername())); } if (!resetToken.equals(token)) { + logger.debug(String.format( + "Failed to reset password. Invalid reset token for user id: %d username: %s " + + "account id: %d domain id: %d", + user.getId(), user.getUsername(), user.getAccountId(), user.getDomainId())); throw new ServerApiException(ApiErrorCode.PARAM_ERROR, String.format("Invalid reset token for user %s", user.getUsername())); } if (now.after(resetTokenExpiryTime)) { + logger.debug(String.format( + "Failed to reset password. Reset token has expired for user id: %d username: %s " + + "account id: %d domain id: %d", + user.getId(), user.getUsername(), user.getAccountId(), user.getDomainId())); throw new ServerApiException(ApiErrorCode.PARAM_ERROR, String.format("Reset token has expired for user %s", user.getUsername())); } resetPassword(user, password); + logger.debug(String.format( + "Password reset successful for user id: %d username: %s account id: %d domain id: %d", + user.getId(), user.getUsername(), user.getAccountId(), user.getDomainId())); return true; } diff --git a/server/src/main/resources/META-INF/cloudstack/core/spring-server-core-managers-context.xml b/server/src/main/resources/META-INF/cloudstack/core/spring-server-core-managers-context.xml index 383ac3157c28..cbee68aea07e 100644 --- a/server/src/main/resources/META-INF/cloudstack/core/spring-server-core-managers-context.xml +++ b/server/src/main/resources/META-INF/cloudstack/core/spring-server-core-managers-context.xml @@ -56,7 +56,7 @@ - + diff --git a/server/src/test/java/com/cloud/api/ApiServerTest.java b/server/src/test/java/com/cloud/api/ApiServerTest.java index caf923332151..3584f9f9ed00 100644 --- a/server/src/test/java/com/cloud/api/ApiServerTest.java +++ b/server/src/test/java/com/cloud/api/ApiServerTest.java @@ -16,14 +16,10 @@ // under the License. package com.cloud.api; -import java.util.ArrayList; -import java.util.Collections; -import java.util.List; -import java.util.Map; - +import com.cloud.domain.Domain; import com.cloud.user.UserAccount; import com.cloud.utils.exception.CloudRuntimeException; -import org.apache.cloudstack.user.PasswordReset; +import org.apache.cloudstack.user.PasswordResetManager; import org.junit.Assert; import org.junit.Test; import org.junit.runner.RunWith; @@ -33,6 +29,11 @@ import org.mockito.Mockito; import org.mockito.junit.MockitoJUnitRunner; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.Map; + import static org.apache.cloudstack.resourcedetail.UserDetailVO.PasswordResetToken; import static org.apache.cloudstack.resourcedetail.UserDetailVO.PasswordResetTokenExpiryDate; @@ -43,7 +44,7 @@ public class ApiServerTest { ApiServer apiServer = new ApiServer(); @Mock - PasswordReset passwordReset; + PasswordResetManager passwordResetManager; private void runTestSetupIntegrationPortListenerInvalidPorts(Integer port) { try (MockedConstruction mocked = @@ -77,47 +78,105 @@ public void testSetupIntegrationPortListenerValidPort() { @Test public void testForgotPasswordSuccessFirstRequest() { UserAccount userAccount = Mockito.mock(UserAccount.class); + Domain domain = Mockito.mock(Domain.class); Mockito.when(userAccount.getDetails()).thenReturn(Collections.emptyMap()); Mockito.when(userAccount.getEmail()).thenReturn("test@test.com"); - Mockito.doNothing().when(passwordReset).setResetTokenAndSend(userAccount); - Assert.assertTrue(apiServer.forgotPassword(userAccount)); - Mockito.verify(passwordReset).setResetTokenAndSend(userAccount); + Mockito.when(userAccount.getState()).thenReturn("ENABLED"); + Mockito.when(userAccount.getAccountState()).thenReturn("ENABLED"); + Mockito.when(domain.getState()).thenReturn(Domain.State.Active); + Mockito.doNothing().when(passwordResetManager).setResetTokenAndSend(userAccount); + Assert.assertTrue(apiServer.forgotPassword(userAccount, domain)); + Mockito.verify(passwordResetManager).setResetTokenAndSend(userAccount); + } + + @Test + public void testForgotPasswordSuccessSecondRequestExpired() { + UserAccount userAccount = Mockito.mock(UserAccount.class); + Domain domain = Mockito.mock(Domain.class); + + Mockito.when(userAccount.getDetails()).thenReturn(Map.of( + PasswordResetToken, "token", + PasswordResetTokenExpiryDate, String.valueOf(System.currentTimeMillis() - 5 * 60 * 1000) + )); + Mockito.when(userAccount.getEmail()).thenReturn("test@test.com"); + Mockito.doNothing().when(passwordResetManager).setResetTokenAndSend(userAccount); + Mockito.when(userAccount.getState()).thenReturn("ENABLED"); + Mockito.when(userAccount.getAccountState()).thenReturn("ENABLED"); + Mockito.when(domain.getState()).thenReturn(Domain.State.Active); + Assert.assertTrue(apiServer.forgotPassword(userAccount, domain)); + Mockito.verify(passwordResetManager).setResetTokenAndSend(userAccount); + } + + @Test + public void testForgotPasswordSuccessSecondRequestUnexpired() { + UserAccount userAccount = Mockito.mock(UserAccount.class); + Domain domain = Mockito.mock(Domain.class); + + Mockito.when(userAccount.getDetails()).thenReturn(Map.of( + PasswordResetToken, "token", + PasswordResetTokenExpiryDate, String.valueOf(System.currentTimeMillis() + 5 * 60 * 1000) + )); + Mockito.when(userAccount.getEmail()).thenReturn("test@test.com"); + Mockito.when(userAccount.getState()).thenReturn("ENABLED"); + Mockito.when(userAccount.getAccountState()).thenReturn("ENABLED"); + Mockito.when(domain.getState()).thenReturn(Domain.State.Active); + Assert.assertTrue(apiServer.forgotPassword(userAccount, domain)); + Mockito.verify(passwordResetManager, Mockito.never()).setResetTokenAndSend(userAccount); } @Test(expected = CloudRuntimeException.class) public void testForgotPasswordFailureNoEmail() { UserAccount userAccount = Mockito.mock(UserAccount.class); + Domain domain = Mockito.mock(Domain.class); Mockito.when(userAccount.getDetails()).thenReturn(Collections.emptyMap()); Mockito.when(userAccount.getEmail()).thenReturn(""); - Assert.assertTrue(apiServer.forgotPassword(userAccount)); + apiServer.forgotPassword(userAccount, domain); } - @Test - public void testForgotPasswordSuccessSecondRequestExpired() { + @Test(expected = CloudRuntimeException.class) + public void testForgotPasswordFailureDisabledUser() { UserAccount userAccount = Mockito.mock(UserAccount.class); + Domain domain = Mockito.mock(Domain.class); Mockito.when(userAccount.getDetails()).thenReturn(Map.of( PasswordResetToken, "token", - PasswordResetTokenExpiryDate, String.valueOf(System.currentTimeMillis() - 5 * 60 * 1000) + PasswordResetTokenExpiryDate, String.valueOf(System.currentTimeMillis() + 5 * 60 * 1000) )); Mockito.when(userAccount.getEmail()).thenReturn("test@test.com"); - Mockito.doNothing().when(passwordReset).setResetTokenAndSend(userAccount); - Assert.assertTrue(apiServer.forgotPassword(userAccount)); - Mockito.verify(passwordReset).setResetTokenAndSend(userAccount); + Mockito.when(userAccount.getState()).thenReturn("DISABLED"); + apiServer.forgotPassword(userAccount, domain); } - @Test - public void testForgotPasswordSuccessSecondRequestUnexpired() { + @Test(expected = CloudRuntimeException.class) + public void testForgotPasswordFailureDisabledAccount() { + UserAccount userAccount = Mockito.mock(UserAccount.class); + Domain domain = Mockito.mock(Domain.class); + + Mockito.when(userAccount.getDetails()).thenReturn(Map.of( + PasswordResetToken, "token", + PasswordResetTokenExpiryDate, String.valueOf(System.currentTimeMillis() + 5 * 60 * 1000) + )); + Mockito.when(userAccount.getEmail()).thenReturn("test@test.com"); + Mockito.when(userAccount.getState()).thenReturn("ENABLED"); + Mockito.when(userAccount.getAccountState()).thenReturn("DISABLED"); + apiServer.forgotPassword(userAccount, domain); + } + + @Test(expected = CloudRuntimeException.class) + public void testForgotPasswordFailureInactiveDomain() { UserAccount userAccount = Mockito.mock(UserAccount.class); + Domain domain = Mockito.mock(Domain.class); Mockito.when(userAccount.getDetails()).thenReturn(Map.of( PasswordResetToken, "token", PasswordResetTokenExpiryDate, String.valueOf(System.currentTimeMillis() + 5 * 60 * 1000) )); Mockito.when(userAccount.getEmail()).thenReturn("test@test.com"); - Assert.assertTrue(apiServer.forgotPassword(userAccount)); - Mockito.verify(passwordReset, Mockito.never()).setResetTokenAndSend(userAccount); + Mockito.when(userAccount.getState()).thenReturn("ENABLED"); + Mockito.when(userAccount.getAccountState()).thenReturn("ENABLED"); + Mockito.when(domain.getState()).thenReturn(Domain.State.Inactive); + apiServer.forgotPassword(userAccount, domain); } } diff --git a/server/src/test/java/org/apache/cloudstack/user/PasswordResetImplTest.java b/server/src/test/java/org/apache/cloudstack/user/PasswordResetManagerImplTest.java similarity index 96% rename from server/src/test/java/org/apache/cloudstack/user/PasswordResetImplTest.java rename to server/src/test/java/org/apache/cloudstack/user/PasswordResetManagerImplTest.java index 8d729f289216..e08e4212b3a4 100644 --- a/server/src/test/java/org/apache/cloudstack/user/PasswordResetImplTest.java +++ b/server/src/test/java/org/apache/cloudstack/user/PasswordResetManagerImplTest.java @@ -34,10 +34,10 @@ import static org.apache.cloudstack.resourcedetail.UserDetailVO.PasswordResetTokenExpiryDate; @RunWith(MockitoJUnitRunner.class) -public class PasswordResetImplTest { +public class PasswordResetManagerImplTest { @Spy @InjectMocks - PasswordResetImpl passwordReset; + PasswordResetManagerImpl passwordReset; @Mock private UserDetailsDao userDetailsDao; @@ -45,7 +45,7 @@ public class PasswordResetImplTest { @Test public void testGetMessageBody() { ConfigKey passwordResetMailTemplate = Mockito.mock(ConfigKey.class); - PasswordResetImpl.PasswordResetMailTemplate = passwordResetMailTemplate; + PasswordResetManagerImpl.PasswordResetMailTemplate = passwordResetMailTemplate; Mockito.when(passwordResetMailTemplate.value()).thenReturn("Hello {{username}}!\n" + "You have requested to reset your password. Please click the following link to reset your password:\n" + "{{{resetLink}}}\n" + From dc8cf2c9cec2065d8f2a576bc25afcb8a8a747f5 Mon Sep 17 00:00:00 2001 From: Vishesh Date: Fri, 30 Aug 2024 13:43:24 +0530 Subject: [PATCH 3/6] fixups --- .../main/java/com/cloud/api/ApiServer.java | 28 ++------ .../cloudstack/user/PasswordResetManager.java | 55 ---------------- .../user/UserPasswordResetManager.java | 64 ++++++++++++++++++ ...java => UserPasswordResetManagerImpl.java} | 65 ++++++++++++++----- .../spring-server-core-managers-context.xml | 2 +- .../java/com/cloud/api/ApiServerTest.java | 64 ++---------------- ... => UserPasswordResetManagerImplTest.java} | 41 +++++++++++- 7 files changed, 159 insertions(+), 160 deletions(-) delete mode 100644 server/src/main/java/org/apache/cloudstack/user/PasswordResetManager.java create mode 100644 server/src/main/java/org/apache/cloudstack/user/UserPasswordResetManager.java rename server/src/main/java/org/apache/cloudstack/user/{PasswordResetManagerImpl.java => UserPasswordResetManagerImpl.java} (82%) rename server/src/test/java/org/apache/cloudstack/user/{PasswordResetManagerImplTest.java => UserPasswordResetManagerImplTest.java} (75%) diff --git a/server/src/main/java/com/cloud/api/ApiServer.java b/server/src/main/java/com/cloud/api/ApiServer.java index 468de4d7d1b9..904fa41bec2c 100644 --- a/server/src/main/java/com/cloud/api/ApiServer.java +++ b/server/src/main/java/com/cloud/api/ApiServer.java @@ -110,7 +110,7 @@ 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.user.PasswordResetManager; +import org.apache.cloudstack.user.UserPasswordResetManager; import org.apache.commons.codec.binary.Base64; import org.apache.commons.lang3.EnumUtils; import org.apache.http.ConnectionClosedException; @@ -184,9 +184,6 @@ import com.cloud.utils.net.NetUtils; import com.google.gson.reflect.TypeToken; -import static org.apache.cloudstack.resourcedetail.UserDetailVO.PasswordResetToken; -import static org.apache.cloudstack.resourcedetail.UserDetailVO.PasswordResetTokenExpiryDate; - @Component public class ApiServer extends ManagerBase implements HttpRequestHandler, ApiServerService, Configurable { @@ -220,7 +217,7 @@ public class ApiServer extends ManagerBase implements HttpRequestHandler, ApiSer @Inject private UUIDManager uuidMgr; @Inject - private PasswordResetManager passwordResetManager; + private UserPasswordResetManager userPasswordResetManager; private List pluggableServices; @@ -1232,9 +1229,6 @@ public boolean verifyUser(final Long userId) { @Override public boolean forgotPassword(UserAccount userAccount, Domain domain) { - String resetToken = userAccount.getDetails().get(PasswordResetToken); - String resetTokenExpiryTimeString = userAccount.getDetails().getOrDefault(PasswordResetTokenExpiryDate, "0"); - if (StringUtils.isBlank(userAccount.getEmail())) { logger.error(String.format( "Email is not set. username: %s account id: %d domain id: %d", @@ -1263,27 +1257,13 @@ public boolean forgotPassword(UserAccount userAccount, Domain domain) { throw new CloudRuntimeException("Domain is not active."); } - if (StringUtils.isNotEmpty(resetToken) && StringUtils.isNotEmpty(resetTokenExpiryTimeString)) { - final Date resetTokenExpiryTime = new Date(Long.parseLong(resetTokenExpiryTimeString)); - final Date currentTime = new Date(); - if (currentTime.after(resetTokenExpiryTime)) { - passwordResetManager.setResetTokenAndSend(userAccount); - } - } else if (StringUtils.isEmpty(resetToken)) { - passwordResetManager.setResetTokenAndSend(userAccount); - } else { - logger.debug(String.format( - "Password reset token is already set for user %s in " + - "domain id: %s with account %s and email %s", - userAccount.getUsername(), domain.getUuid(), - userAccount.getAccountName(), userAccount.getEmail())); - } + userPasswordResetManager.setResetTokenAndSend(userAccount); return true; } @Override public boolean resetPassword(UserAccount userAccount, String token, String password) { - return passwordResetManager.validateAndResetPassword(userAccount, token, password); + return userPasswordResetManager.validateAndResetPassword(userAccount, token, password); } private void checkCommandAvailable(final User user, final String commandName, final InetAddress remoteAddress) throws PermissionDeniedException { diff --git a/server/src/main/java/org/apache/cloudstack/user/PasswordResetManager.java b/server/src/main/java/org/apache/cloudstack/user/PasswordResetManager.java deleted file mode 100644 index f4eef55504c4..000000000000 --- a/server/src/main/java/org/apache/cloudstack/user/PasswordResetManager.java +++ /dev/null @@ -1,55 +0,0 @@ -// 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. - -package org.apache.cloudstack.user; - -import com.cloud.user.UserAccount; -import org.apache.cloudstack.framework.config.ConfigKey; - -public interface PasswordResetManager { - ConfigKey PasswordResetTtl = new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED, Long.class, - "user.password.reset.ttl", "30", - "Password reset ttl in minutes", true, ConfigKey.Scope.Global); - - ConfigKey PasswordResetEmailSender = new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED, - String.class, "user.password.reset.email.sender", null, - "Password reset email sender", true, ConfigKey.Scope.Global); - - ConfigKey PasswordResetSMTPHost = new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED, - String.class, "user.password.reset.smtp.host", null, - "Password reset smtp host", false, ConfigKey.Scope.Global); - - ConfigKey PasswordResetSMTPPort = new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED, - Integer.class, "user.password.reset.smtp.port", "25", - "Password reset smtp port", false, ConfigKey.Scope.Global); - - ConfigKey PasswordResetSMTPUseAuth = new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED, - Boolean.class, "user.password.reset.smtp.useAuth", "false", - "Use auth for smtp in Password reset", false, ConfigKey.Scope.Global); - - ConfigKey PasswordResetSMTPUsername = new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED, - String.class, "user.password.reset.smtp.username", null, - "Password reset smtp username", false, ConfigKey.Scope.Global); - - ConfigKey PasswordResetSMTPPassword = new ConfigKey<>("Secure", String.class, - "user.password.reset.smtp.password", null, - "Password reset smtp password", false, ConfigKey.Scope.Global); - - void setResetTokenAndSend(UserAccount userAccount); - - boolean validateAndResetPassword(UserAccount user, String token, String password); -} diff --git a/server/src/main/java/org/apache/cloudstack/user/UserPasswordResetManager.java b/server/src/main/java/org/apache/cloudstack/user/UserPasswordResetManager.java new file mode 100644 index 000000000000..cb91c3314d8e --- /dev/null +++ b/server/src/main/java/org/apache/cloudstack/user/UserPasswordResetManager.java @@ -0,0 +1,64 @@ +// 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. + +package org.apache.cloudstack.user; + +import com.cloud.user.UserAccount; +import org.apache.cloudstack.framework.config.ConfigKey; + +public interface UserPasswordResetManager { + ConfigKey UserPasswordResetTtl = new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED, Long.class, + "user.password.reset.ttl", "30", + "TTL in minutes for the token generated to reset the ACS user's password", true, + ConfigKey.Scope.Global); + + ConfigKey UserPasswordResetEmailSender = new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED, + String.class, "user.password.reset.email.sender", null, + "Sender for emails sent to the user to reset ACS user's password ", true, + ConfigKey.Scope.Global); + + ConfigKey UserPasswordResetSMTPHost = new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED, + String.class, "user.password.reset.smtp.host", null, + "Host for SMTP server for sending emails for resetting password for ACS users", + false, + ConfigKey.Scope.Global); + + ConfigKey UserPasswordResetSMTPPort = new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED, + Integer.class, "user.password.reset.smtp.port", "25", + "Port for SMTP server for sending emails for resetting password for ACS users", + false, + ConfigKey.Scope.Global); + + ConfigKey UserPasswordResetSMTPUseAuth = new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED, + Boolean.class, "user.password.reset.smtp.useAuth", "false", + "Use auth in the SMTP server for sending emails for resetting password for ACS users", + false, ConfigKey.Scope.Global); + + ConfigKey UserPasswordResetSMTPUsername = new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED, + String.class, "user.password.reset.smtp.username", null, + "Username for SMTP server for sending emails for resetting password for ACS users", + false, ConfigKey.Scope.Global); + + ConfigKey UserPasswordResetSMTPPassword = new ConfigKey<>("Secure", String.class, + "user.password.reset.smtp.password", null, + "Password for SMTP server for sending emails for resetting password for ACS users", + false, ConfigKey.Scope.Global); + + void setResetTokenAndSend(UserAccount userAccount); + + boolean validateAndResetPassword(UserAccount user, String token, String password); +} diff --git a/server/src/main/java/org/apache/cloudstack/user/PasswordResetManagerImpl.java b/server/src/main/java/org/apache/cloudstack/user/UserPasswordResetManagerImpl.java similarity index 82% rename from server/src/main/java/org/apache/cloudstack/user/PasswordResetManagerImpl.java rename to server/src/main/java/org/apache/cloudstack/user/UserPasswordResetManagerImpl.java index 962e27678bab..e233e8fb02d9 100644 --- a/server/src/main/java/org/apache/cloudstack/user/PasswordResetManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/user/UserPasswordResetManagerImpl.java @@ -52,7 +52,7 @@ import static org.apache.cloudstack.resourcedetail.UserDetailVO.PasswordResetToken; import static org.apache.cloudstack.resourcedetail.UserDetailVO.PasswordResetTokenExpiryDate; -public class PasswordResetManagerImpl extends ManagerBase implements PasswordResetManager, Configurable { +public class UserPasswordResetManagerImpl extends ManagerBase implements UserPasswordResetManager, Configurable { @Inject private AccountManager accountManager; @@ -67,9 +67,9 @@ public class PasswordResetManagerImpl extends ManagerBase implements PasswordRes public static ConfigKey PasswordResetMailTemplate = new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED, String.class, - "password.reset.mail.template", "Hello {{username}}!\n" + + "user.password.reset.mail.template", "Hello {{username}}!\n" + "You have requested to reset your password. Please click the following link to reset your password:\n" + - "{{{reset_link}}}\n" + + "{{{resetLink}}}\n" + "If you did not request a password reset, please ignore this email.\n" + "\n" + "Regards,\n" + @@ -81,29 +81,29 @@ public class PasswordResetManagerImpl extends ManagerBase implements PasswordRes @Override public String getConfigComponentName() { - return PasswordResetManagerImpl.class.getSimpleName(); + return UserPasswordResetManagerImpl.class.getSimpleName(); } @Override public ConfigKey[] getConfigKeys() { - return new ConfigKey[]{PasswordResetTtl, - PasswordResetEmailSender, - PasswordResetSMTPHost, - PasswordResetSMTPPort, - PasswordResetSMTPUseAuth, - PasswordResetSMTPUsername, - PasswordResetSMTPPassword, + return new ConfigKey[]{UserPasswordResetTtl, + UserPasswordResetEmailSender, + UserPasswordResetSMTPHost, + UserPasswordResetSMTPPort, + UserPasswordResetSMTPUseAuth, + UserPasswordResetSMTPUsername, + UserPasswordResetSMTPPassword, PasswordResetMailTemplate }; } @Override public boolean configure(String name, Map params) throws ConfigurationException { - String smtpHost = PasswordResetSMTPHost.value(); - Integer smtpPort = PasswordResetSMTPPort.value(); - Boolean useAuth = PasswordResetSMTPUseAuth.value(); - String username = PasswordResetSMTPUsername.value(); - String password = PasswordResetSMTPPassword.value(); + String smtpHost = UserPasswordResetSMTPHost.value(); + Integer smtpPort = UserPasswordResetSMTPPort.value(); + Boolean useAuth = UserPasswordResetSMTPUseAuth.value(); + String username = UserPasswordResetSMTPUsername.value(); + String password = UserPasswordResetSMTPPassword.value(); if (!StringUtils.isEmpty(smtpHost) && smtpPort != null && smtpPort > 0) { String namespace = "password.reset.smtp"; @@ -126,6 +126,26 @@ private String getKey(String namespace, String config) { } + protected boolean validateExistingToken(UserAccount userAccount) { + + Map details = userDetailsDao.listDetailsKeyPairs(userAccount.getId()); + + String resetToken = details.get(PasswordResetToken); + String resetTokenExpiryTimeString = details.getOrDefault(PasswordResetTokenExpiryDate, "0"); + + + if (StringUtils.isNotEmpty(resetToken) && StringUtils.isNotEmpty(resetTokenExpiryTimeString)) { + final Date resetTokenExpiryTime = new Date(Long.parseLong(resetTokenExpiryTimeString)); + final Date currentTime = new Date(); + if (currentTime.after(resetTokenExpiryTime)) { + return true; + } + } else if (StringUtils.isEmpty(resetToken)) { + return true; + } + return false; + } + public void setResetTokenAndSend(UserAccount userAccount) { if (mailSender == null) { logger.debug("Failed to reset token and send email. SMTP mail sender is not configured."); @@ -133,8 +153,17 @@ public void setResetTokenAndSend(UserAccount userAccount) { "Failed to reset token and send email. SMTP mail sender is not configured"); } + if (!validateExistingToken(userAccount)) { + logger.debug(String.format( + "Failed to reset token and send email. Password reset token is already set for user %s in " + + "domain id: %s with account %s and email %s", + userAccount.getUsername(), userAccount.getDomainId(), + userAccount.getAccountName(), userAccount.getEmail())); + return; + } + final String resetToken = UUID.randomUUID().toString(); - final Date resetTokenExpiryTime = new Date(System.currentTimeMillis() + PasswordResetTtl.value() * 60 * 1000); + final Date resetTokenExpiryTime = new Date(System.currentTimeMillis() + UserPasswordResetTtl.value() * 60 * 1000); userDetailsDao.addDetail(userAccount.getId(), PasswordResetToken, resetToken, false); userDetailsDao.addDetail(userAccount.getId(), PasswordResetTokenExpiryDate, String.valueOf(resetTokenExpiryTime.getTime()), false); @@ -149,7 +178,7 @@ public void setResetTokenAndSend(UserAccount userAccount) { SMTPMailProperties mailProperties = new SMTPMailProperties(); - mailProperties.setSender(new MailAddress(PasswordResetEmailSender.value())); + mailProperties.setSender(new MailAddress(UserPasswordResetEmailSender.value())); mailProperties.setSubject(subject); mailProperties.setContent(content); mailProperties.setContentType("text/plain"); diff --git a/server/src/main/resources/META-INF/cloudstack/core/spring-server-core-managers-context.xml b/server/src/main/resources/META-INF/cloudstack/core/spring-server-core-managers-context.xml index cbee68aea07e..8edaf60fb357 100644 --- a/server/src/main/resources/META-INF/cloudstack/core/spring-server-core-managers-context.xml +++ b/server/src/main/resources/META-INF/cloudstack/core/spring-server-core-managers-context.xml @@ -56,7 +56,7 @@ - + diff --git a/server/src/test/java/com/cloud/api/ApiServerTest.java b/server/src/test/java/com/cloud/api/ApiServerTest.java index 3584f9f9ed00..5be4ee1c754e 100644 --- a/server/src/test/java/com/cloud/api/ApiServerTest.java +++ b/server/src/test/java/com/cloud/api/ApiServerTest.java @@ -19,7 +19,7 @@ import com.cloud.domain.Domain; import com.cloud.user.UserAccount; import com.cloud.utils.exception.CloudRuntimeException; -import org.apache.cloudstack.user.PasswordResetManager; +import org.apache.cloudstack.user.UserPasswordResetManager; import org.junit.Assert; import org.junit.Test; import org.junit.runner.RunWith; @@ -30,12 +30,7 @@ import org.mockito.junit.MockitoJUnitRunner; import java.util.ArrayList; -import java.util.Collections; import java.util.List; -import java.util.Map; - -import static org.apache.cloudstack.resourcedetail.UserDetailVO.PasswordResetToken; -import static org.apache.cloudstack.resourcedetail.UserDetailVO.PasswordResetTokenExpiryDate; @RunWith(MockitoJUnitRunner.class) public class ApiServerTest { @@ -44,7 +39,7 @@ public class ApiServerTest { ApiServer apiServer = new ApiServer(); @Mock - PasswordResetManager passwordResetManager; + UserPasswordResetManager userPasswordResetManager; private void runTestSetupIntegrationPortListenerInvalidPorts(Integer port) { try (MockedConstruction mocked = @@ -76,53 +71,17 @@ public void testSetupIntegrationPortListenerValidPort() { } @Test - public void testForgotPasswordSuccessFirstRequest() { - UserAccount userAccount = Mockito.mock(UserAccount.class); - Domain domain = Mockito.mock(Domain.class); - - Mockito.when(userAccount.getDetails()).thenReturn(Collections.emptyMap()); - Mockito.when(userAccount.getEmail()).thenReturn("test@test.com"); - Mockito.when(userAccount.getState()).thenReturn("ENABLED"); - Mockito.when(userAccount.getAccountState()).thenReturn("ENABLED"); - Mockito.when(domain.getState()).thenReturn(Domain.State.Active); - Mockito.doNothing().when(passwordResetManager).setResetTokenAndSend(userAccount); - Assert.assertTrue(apiServer.forgotPassword(userAccount, domain)); - Mockito.verify(passwordResetManager).setResetTokenAndSend(userAccount); - } - - @Test - public void testForgotPasswordSuccessSecondRequestExpired() { - UserAccount userAccount = Mockito.mock(UserAccount.class); - Domain domain = Mockito.mock(Domain.class); - - Mockito.when(userAccount.getDetails()).thenReturn(Map.of( - PasswordResetToken, "token", - PasswordResetTokenExpiryDate, String.valueOf(System.currentTimeMillis() - 5 * 60 * 1000) - )); - Mockito.when(userAccount.getEmail()).thenReturn("test@test.com"); - Mockito.doNothing().when(passwordResetManager).setResetTokenAndSend(userAccount); - Mockito.when(userAccount.getState()).thenReturn("ENABLED"); - Mockito.when(userAccount.getAccountState()).thenReturn("ENABLED"); - Mockito.when(domain.getState()).thenReturn(Domain.State.Active); - Assert.assertTrue(apiServer.forgotPassword(userAccount, domain)); - Mockito.verify(passwordResetManager).setResetTokenAndSend(userAccount); - } - - @Test - public void testForgotPasswordSuccessSecondRequestUnexpired() { + public void testForgotPasswordSuccess() { UserAccount userAccount = Mockito.mock(UserAccount.class); Domain domain = Mockito.mock(Domain.class); - Mockito.when(userAccount.getDetails()).thenReturn(Map.of( - PasswordResetToken, "token", - PasswordResetTokenExpiryDate, String.valueOf(System.currentTimeMillis() + 5 * 60 * 1000) - )); Mockito.when(userAccount.getEmail()).thenReturn("test@test.com"); Mockito.when(userAccount.getState()).thenReturn("ENABLED"); Mockito.when(userAccount.getAccountState()).thenReturn("ENABLED"); Mockito.when(domain.getState()).thenReturn(Domain.State.Active); + Mockito.doNothing().when(userPasswordResetManager).setResetTokenAndSend(userAccount); Assert.assertTrue(apiServer.forgotPassword(userAccount, domain)); - Mockito.verify(passwordResetManager, Mockito.never()).setResetTokenAndSend(userAccount); + Mockito.verify(userPasswordResetManager).setResetTokenAndSend(userAccount); } @Test(expected = CloudRuntimeException.class) @@ -130,7 +89,6 @@ public void testForgotPasswordFailureNoEmail() { UserAccount userAccount = Mockito.mock(UserAccount.class); Domain domain = Mockito.mock(Domain.class); - Mockito.when(userAccount.getDetails()).thenReturn(Collections.emptyMap()); Mockito.when(userAccount.getEmail()).thenReturn(""); apiServer.forgotPassword(userAccount, domain); } @@ -140,10 +98,6 @@ public void testForgotPasswordFailureDisabledUser() { UserAccount userAccount = Mockito.mock(UserAccount.class); Domain domain = Mockito.mock(Domain.class); - Mockito.when(userAccount.getDetails()).thenReturn(Map.of( - PasswordResetToken, "token", - PasswordResetTokenExpiryDate, String.valueOf(System.currentTimeMillis() + 5 * 60 * 1000) - )); Mockito.when(userAccount.getEmail()).thenReturn("test@test.com"); Mockito.when(userAccount.getState()).thenReturn("DISABLED"); apiServer.forgotPassword(userAccount, domain); @@ -154,10 +108,6 @@ public void testForgotPasswordFailureDisabledAccount() { UserAccount userAccount = Mockito.mock(UserAccount.class); Domain domain = Mockito.mock(Domain.class); - Mockito.when(userAccount.getDetails()).thenReturn(Map.of( - PasswordResetToken, "token", - PasswordResetTokenExpiryDate, String.valueOf(System.currentTimeMillis() + 5 * 60 * 1000) - )); Mockito.when(userAccount.getEmail()).thenReturn("test@test.com"); Mockito.when(userAccount.getState()).thenReturn("ENABLED"); Mockito.when(userAccount.getAccountState()).thenReturn("DISABLED"); @@ -169,10 +119,6 @@ public void testForgotPasswordFailureInactiveDomain() { UserAccount userAccount = Mockito.mock(UserAccount.class); Domain domain = Mockito.mock(Domain.class); - Mockito.when(userAccount.getDetails()).thenReturn(Map.of( - PasswordResetToken, "token", - PasswordResetTokenExpiryDate, String.valueOf(System.currentTimeMillis() + 5 * 60 * 1000) - )); Mockito.when(userAccount.getEmail()).thenReturn("test@test.com"); Mockito.when(userAccount.getState()).thenReturn("ENABLED"); Mockito.when(userAccount.getAccountState()).thenReturn("ENABLED"); diff --git a/server/src/test/java/org/apache/cloudstack/user/PasswordResetManagerImplTest.java b/server/src/test/java/org/apache/cloudstack/user/UserPasswordResetManagerImplTest.java similarity index 75% rename from server/src/test/java/org/apache/cloudstack/user/PasswordResetManagerImplTest.java rename to server/src/test/java/org/apache/cloudstack/user/UserPasswordResetManagerImplTest.java index e08e4212b3a4..17092e6311dd 100644 --- a/server/src/test/java/org/apache/cloudstack/user/PasswordResetManagerImplTest.java +++ b/server/src/test/java/org/apache/cloudstack/user/UserPasswordResetManagerImplTest.java @@ -30,14 +30,17 @@ import org.mockito.Spy; import org.mockito.junit.MockitoJUnitRunner; +import java.util.Collections; +import java.util.Map; + import static org.apache.cloudstack.resourcedetail.UserDetailVO.PasswordResetToken; import static org.apache.cloudstack.resourcedetail.UserDetailVO.PasswordResetTokenExpiryDate; @RunWith(MockitoJUnitRunner.class) -public class PasswordResetManagerImplTest { +public class UserPasswordResetManagerImplTest { @Spy @InjectMocks - PasswordResetManagerImpl passwordReset; + UserPasswordResetManagerImpl passwordReset; @Mock private UserDetailsDao userDetailsDao; @@ -45,7 +48,7 @@ public class PasswordResetManagerImplTest { @Test public void testGetMessageBody() { ConfigKey passwordResetMailTemplate = Mockito.mock(ConfigKey.class); - PasswordResetManagerImpl.PasswordResetMailTemplate = passwordResetMailTemplate; + UserPasswordResetManagerImpl.PasswordResetMailTemplate = passwordResetMailTemplate; Mockito.when(passwordResetMailTemplate.value()).thenReturn("Hello {{username}}!\n" + "You have requested to reset your password. Please click the following link to reset your password:\n" + "{{{resetLink}}}\n" + @@ -112,4 +115,36 @@ public void testValidateAndResetPassword() { Assert.assertTrue(passwordReset.validateAndResetPassword(userAccount, "reset_token", "new_password")); Mockito.verify(passwordReset, Mockito.times(1)).resetPassword(userAccount, "new_password"); } + + @Test + public void testValidateExistingTokenFirstRequest() { + UserAccount userAccount = Mockito.mock(UserAccount.class); + Mockito.when(userAccount.getId()).thenReturn(1L); + Mockito.when(userDetailsDao.listDetailsKeyPairs(1L)).thenReturn(Collections.emptyMap()); + + Assert.assertTrue(passwordReset.validateExistingToken(userAccount)); + } + + @Test + public void testValidateExistingTokenSecondRequestExpired() { + UserAccount userAccount = Mockito.mock(UserAccount.class); + Mockito.when(userAccount.getId()).thenReturn(1L); + Mockito.when(userDetailsDao.listDetailsKeyPairs(1L)).thenReturn(Map.of( + PasswordResetToken, "reset_token", + PasswordResetTokenExpiryDate, String.valueOf(System.currentTimeMillis() - 5 * 60 * 1000))); + + Assert.assertTrue(passwordReset.validateExistingToken(userAccount)); + } + + + @Test + public void testValidateExistingTokenSecondRequestUnexpired() { + UserAccount userAccount = Mockito.mock(UserAccount.class); + Mockito.when(userAccount.getId()).thenReturn(1L); + Mockito.when(userDetailsDao.listDetailsKeyPairs(1L)).thenReturn(Map.of( + PasswordResetToken, "reset_token", + PasswordResetTokenExpiryDate, String.valueOf(System.currentTimeMillis() + 5 * 60 * 1000))); + + Assert.assertFalse(passwordReset.validateExistingToken(userAccount)); + } } From 6fdc1703b84a1f84cf90803272c1f55a1462edfe Mon Sep 17 00:00:00 2001 From: Vishesh Date: Wed, 4 Sep 2024 17:37:34 +0530 Subject: [PATCH 4/6] Make forgot password disabled by default --- .../main/java/com/cloud/api/ApiServer.java | 14 ++++++++++++ .../auth/APIAuthenticationManagerImpl.java | 8 +++++-- .../user/UserPasswordResetManager.java | 7 ++++++ .../java/com/cloud/api/ApiServerTest.java | 22 +++++++++++++++++++ ui/public/config.json | 1 + ui/src/views/auth/Login.vue | 3 +-- 6 files changed, 51 insertions(+), 4 deletions(-) diff --git a/server/src/main/java/com/cloud/api/ApiServer.java b/server/src/main/java/com/cloud/api/ApiServer.java index 904fa41bec2c..739ad765afa0 100644 --- a/server/src/main/java/com/cloud/api/ApiServer.java +++ b/server/src/main/java/com/cloud/api/ApiServer.java @@ -184,6 +184,8 @@ import com.cloud.utils.net.NetUtils; import com.google.gson.reflect.TypeToken; +import static org.apache.cloudstack.user.UserPasswordResetManager.UserPasswordResetEnabled; + @Component public class ApiServer extends ManagerBase implements HttpRequestHandler, ApiServerService, Configurable { @@ -1229,6 +1231,12 @@ public boolean verifyUser(final Long userId) { @Override public boolean forgotPassword(UserAccount userAccount, Domain domain) { + if (!UserPasswordResetEnabled.value()) { + String errorMessage = String.format("%s is false. Password reset for the user is not allowed.", + UserPasswordResetEnabled.key()); + logger.error(errorMessage); + throw new CloudRuntimeException(errorMessage); + } if (StringUtils.isBlank(userAccount.getEmail())) { logger.error(String.format( "Email is not set. username: %s account id: %d domain id: %d", @@ -1263,6 +1271,12 @@ public boolean forgotPassword(UserAccount userAccount, Domain domain) { @Override public boolean resetPassword(UserAccount userAccount, String token, String password) { + if (!UserPasswordResetEnabled.value()) { + String errorMessage = String.format("%s is false. Password reset for the user is not allowed.", + UserPasswordResetEnabled.key()); + logger.error(errorMessage); + throw new CloudRuntimeException(errorMessage); + } return userPasswordResetManager.validateAndResetPassword(userAccount, token, password); } diff --git a/server/src/main/java/com/cloud/api/auth/APIAuthenticationManagerImpl.java b/server/src/main/java/com/cloud/api/auth/APIAuthenticationManagerImpl.java index 990486d1d65e..3c8282d02803 100644 --- a/server/src/main/java/com/cloud/api/auth/APIAuthenticationManagerImpl.java +++ b/server/src/main/java/com/cloud/api/auth/APIAuthenticationManagerImpl.java @@ -31,6 +31,8 @@ import com.cloud.utils.component.ComponentContext; import com.cloud.utils.component.ManagerBase; +import static org.apache.cloudstack.user.UserPasswordResetManager.UserPasswordResetEnabled; + @SuppressWarnings("unchecked") public class APIAuthenticationManagerImpl extends ManagerBase implements APIAuthenticationManager { @@ -75,8 +77,10 @@ public List> getCommands() { List> cmdList = new ArrayList>(); cmdList.add(DefaultLoginAPIAuthenticatorCmd.class); cmdList.add(DefaultLogoutAPIAuthenticatorCmd.class); - cmdList.add(DefaultForgotPasswordAPIAuthenticatorCmd.class); - cmdList.add(DefaultResetPasswordAPIAuthenticatorCmd.class); + if (UserPasswordResetEnabled.value()) { + cmdList.add(DefaultForgotPasswordAPIAuthenticatorCmd.class); + cmdList.add(DefaultResetPasswordAPIAuthenticatorCmd.class); + } cmdList.add(ListUserTwoFactorAuthenticatorProvidersCmd.class); cmdList.add(ValidateUserTwoFactorAuthenticationCodeCmd.class); diff --git a/server/src/main/java/org/apache/cloudstack/user/UserPasswordResetManager.java b/server/src/main/java/org/apache/cloudstack/user/UserPasswordResetManager.java index cb91c3314d8e..a42faf2835a5 100644 --- a/server/src/main/java/org/apache/cloudstack/user/UserPasswordResetManager.java +++ b/server/src/main/java/org/apache/cloudstack/user/UserPasswordResetManager.java @@ -21,6 +21,13 @@ import org.apache.cloudstack.framework.config.ConfigKey; public interface UserPasswordResetManager { + ConfigKey UserPasswordResetEnabled = new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED, + Boolean.class, + "user.password.reset.enabled", "false", + "Setting this to true allows the ACS user to request an email to reset their password", + false, + ConfigKey.Scope.Global); + ConfigKey UserPasswordResetTtl = new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED, Long.class, "user.password.reset.ttl", "30", "TTL in minutes for the token generated to reset the ACS user's password", true, diff --git a/server/src/test/java/com/cloud/api/ApiServerTest.java b/server/src/test/java/com/cloud/api/ApiServerTest.java index 5be4ee1c754e..fed1d95a625e 100644 --- a/server/src/test/java/com/cloud/api/ApiServerTest.java +++ b/server/src/test/java/com/cloud/api/ApiServerTest.java @@ -19,8 +19,11 @@ import com.cloud.domain.Domain; import com.cloud.user.UserAccount; import com.cloud.utils.exception.CloudRuntimeException; +import org.apache.cloudstack.framework.config.ConfigKey; import org.apache.cloudstack.user.UserPasswordResetManager; +import org.junit.AfterClass; import org.junit.Assert; +import org.junit.BeforeClass; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.InjectMocks; @@ -29,9 +32,12 @@ import org.mockito.Mockito; import org.mockito.junit.MockitoJUnitRunner; +import java.lang.reflect.Field; import java.util.ArrayList; import java.util.List; +import static org.apache.cloudstack.user.UserPasswordResetManager.UserPasswordResetEnabled; + @RunWith(MockitoJUnitRunner.class) public class ApiServerTest { @@ -41,6 +47,22 @@ public class ApiServerTest { @Mock UserPasswordResetManager userPasswordResetManager; + @BeforeClass + public static void beforeClass() throws Exception { + overrideDefaultConfigValue(UserPasswordResetEnabled, "_value", true); + } + + @AfterClass + public static void afterClass() throws Exception { + overrideDefaultConfigValue(UserPasswordResetEnabled, "_value", false); + } + + private static void overrideDefaultConfigValue(final ConfigKey configKey, final String name, final Object o) throws IllegalAccessException, NoSuchFieldException { + Field f = ConfigKey.class.getDeclaredField(name); + f.setAccessible(true); + f.set(configKey, o); + } + private void runTestSetupIntegrationPortListenerInvalidPorts(Integer port) { try (MockedConstruction mocked = Mockito.mockConstruction(ApiServer.ListenerThread.class)) { diff --git a/ui/public/config.json b/ui/public/config.json index 774e414af0a2..360fcf1328ec 100644 --- a/ui/public/config.json +++ b/ui/public/config.json @@ -96,5 +96,6 @@ "basicZoneEnabled": true, "multipleServer": false, "allowSettingTheme": true, + "forgotPasswordEnabled": false, "docHelpMappings": {} } diff --git a/ui/src/views/auth/Login.vue b/ui/src/views/auth/Login.vue index e97dddcfc3bf..5a5c8c6c9641 100644 --- a/ui/src/views/auth/Login.vue +++ b/ui/src/views/auth/Login.vue @@ -156,11 +156,10 @@ - + {{ $t('label.forgot.password') }} -
From e34b5a41ee83e3796d6cfc517f148f4883c46690 Mon Sep 17 00:00:00 2001 From: Vishesh Date: Wed, 4 Sep 2024 21:24:42 +0530 Subject: [PATCH 5/6] Apply suggestions from code review --- .../cloudstack/user/UserPasswordResetManagerImpl.java | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/server/src/main/java/org/apache/cloudstack/user/UserPasswordResetManagerImpl.java b/server/src/main/java/org/apache/cloudstack/user/UserPasswordResetManagerImpl.java index e233e8fb02d9..87bf3327771d 100644 --- a/server/src/main/java/org/apache/cloudstack/user/UserPasswordResetManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/user/UserPasswordResetManagerImpl.java @@ -69,7 +69,7 @@ public class UserPasswordResetManagerImpl extends ManagerBase implements UserPas new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED, String.class, "user.password.reset.mail.template", "Hello {{username}}!\n" + "You have requested to reset your password. Please click the following link to reset your password:\n" + - "{{{resetLink}}}\n" + + "http://{{{resetLink}}}\n" + "If you did not request a password reset, please ignore this email.\n" + "\n" + "Regards,\n" + @@ -86,7 +86,9 @@ public String getConfigComponentName() { @Override public ConfigKey[] getConfigKeys() { - return new ConfigKey[]{UserPasswordResetTtl, + return new ConfigKey[]{ + UserPasswordResetEnabled, + UserPasswordResetTtl, UserPasswordResetEmailSender, UserPasswordResetSMTPHost, UserPasswordResetSMTPPort, @@ -172,7 +174,7 @@ public void setResetTokenAndSend(UserAccount userAccount) { final String username = userAccount.getUsername(); final String subject = "Password Reset Request"; - String resetLink = String.format("%s/user/resetPassword?username=%s&token=%s", + String resetLink = String.format("%s/client/#/user/resetPassword?username=%s&token=%s", ManagementServerAddresses.value().split(",")[0], username, resetToken); String content = getMessageBody(userAccount, resetToken, resetLink); From 5fee64a2e721d8d54b91ea56f81dbe59460707ea Mon Sep 17 00:00:00 2001 From: Vishesh Date: Thu, 5 Sep 2024 16:20:11 +0530 Subject: [PATCH 6/6] Address comments --- ...aultForgotPasswordAPIAuthenticatorCmd.java | 9 +++++---- .../user/UserPasswordResetManagerImpl.java | 2 +- ui/public/config.json | 1 - ui/src/utils/request.js | 2 +- ui/src/views/auth/ForgotPassword.vue | 14 +++----------- ui/src/views/auth/Login.vue | 14 ++++++++++++-- ui/src/views/auth/ResetPassword.vue | 19 ------------------- 7 files changed, 22 insertions(+), 39 deletions(-) diff --git a/server/src/main/java/com/cloud/api/auth/DefaultForgotPasswordAPIAuthenticatorCmd.java b/server/src/main/java/com/cloud/api/auth/DefaultForgotPasswordAPIAuthenticatorCmd.java index d8d7e361c6ac..1e90b43c5e8b 100644 --- a/server/src/main/java/com/cloud/api/auth/DefaultForgotPasswordAPIAuthenticatorCmd.java +++ b/server/src/main/java/com/cloud/api/auth/DefaultForgotPasswordAPIAuthenticatorCmd.java @@ -115,10 +115,7 @@ public String authenticate(String command, Map params, HttpSes throw new ServerApiException(ApiErrorCode.PARAM_ERROR, "Forgot Password is not allowed for this user"); } boolean success = _apiServer.forgotPassword(userAccount, userDomain); - SuccessResponse successResponse = new SuccessResponse(); - successResponse.setSuccess(success); - successResponse.setResponseName(getCommandName()); - return ApiResponseSerializer.toSerializedString(successResponse, responseType); + logger.debug("Forgot password request for user " + username[0] + " in domain " + domain + " is successful: " + success); } catch (final CloudRuntimeException ex) { ApiServlet.invalidateHttpSession(session, "fall through to API key,"); String msg = String.format("%s", ex.getMessage() != null ? @@ -130,6 +127,10 @@ public String authenticate(String command, Map params, HttpSes logger.trace(msg); } } + SuccessResponse successResponse = new SuccessResponse(); + successResponse.setSuccess(true); + successResponse.setResponseName(getCommandName()); + return ApiResponseSerializer.toSerializedString(successResponse, responseType); } // We should not reach here and if we do we throw an exception throw new ServerApiException(ApiErrorCode.ACCOUNT_ERROR, serializedResponse); diff --git a/server/src/main/java/org/apache/cloudstack/user/UserPasswordResetManagerImpl.java b/server/src/main/java/org/apache/cloudstack/user/UserPasswordResetManagerImpl.java index 87bf3327771d..f35f69fb8bf2 100644 --- a/server/src/main/java/org/apache/cloudstack/user/UserPasswordResetManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/user/UserPasswordResetManagerImpl.java @@ -183,7 +183,7 @@ public void setResetTokenAndSend(UserAccount userAccount) { mailProperties.setSender(new MailAddress(UserPasswordResetEmailSender.value())); mailProperties.setSubject(subject); mailProperties.setContent(content); - mailProperties.setContentType("text/plain"); + mailProperties.setContentType("text/html; charset=utf-8"); Set addresses = new HashSet<>(); diff --git a/ui/public/config.json b/ui/public/config.json index 360fcf1328ec..774e414af0a2 100644 --- a/ui/public/config.json +++ b/ui/public/config.json @@ -96,6 +96,5 @@ "basicZoneEnabled": true, "multipleServer": false, "allowSettingTheme": true, - "forgotPasswordEnabled": false, "docHelpMappings": {} } diff --git a/ui/src/utils/request.js b/ui/src/utils/request.js index c2fe04ab9d1a..7c757691f2b3 100644 --- a/ui/src/utils/request.js +++ b/ui/src/utils/request.js @@ -51,7 +51,7 @@ const err = (error) => { }) } if (response.status === 401) { - if (response.config && response.config.params && ['listIdps', 'cloudianIsEnabled'].includes(response.config.params.command)) { + if (response.config && response.config.params && ['forgotPassword', 'listIdps', 'cloudianIsEnabled'].includes(response.config.params.command)) { return } const originalPath = router.currentRoute.value.fullPath diff --git a/ui/src/views/auth/ForgotPassword.vue b/ui/src/views/auth/ForgotPassword.vue index cb0e5f68626b..2d45938417fb 100644 --- a/ui/src/views/auth/ForgotPassword.vue +++ b/ui/src/views/auth/ForgotPassword.vue @@ -158,22 +158,14 @@ export default { loginParams.domain = '/' } api('forgotPassword', {}, 'POST', loginParams) - .then((res) => this.forgotPasswordSuccess(res)) - .catch(err => { - this.requestFailed(err) - }).finally(() => { - this.submitBtn = false + .finally(() => { + this.$message.success(this.$t('message.forgot.password.success')) + this.$router.push({ path: '/login' }).catch(() => {}) }) }).catch(error => { this.formRef.value.scrollToField(error.errorFields[0].name) }) }, - forgotPasswordSuccess (res) { - if (res.forgotpasswordresponse.success) { - this.$message.success(this.$t('message.forgot.password.success')) - this.$router.push({ path: '/login' }).catch(() => {}) - } - }, requestFailed (err) { if (err && err.response && err.response.data && err.response.data.forgotpasswordresponse) { const error = err.response.data.forgotpasswordresponse.errorcode + ': ' + err.response.data.forgotpasswordresponse.errortext diff --git a/ui/src/views/auth/Login.vue b/ui/src/views/auth/Login.vue index 5a5c8c6c9641..136455655577 100644 --- a/ui/src/views/auth/Login.vue +++ b/ui/src/views/auth/Login.vue @@ -156,7 +156,7 @@ - + {{ $t('label.forgot.password') }} @@ -229,7 +229,8 @@ export default { loginBtn: false, loginType: 0 }, - server: '' + server: '', + forgotPasswordEnabled: false } }, created () { @@ -312,6 +313,15 @@ export default { }) } }) + api('forgotPassword', {}).then(response => { + this.forgotPasswordEnabled = response.forgotpasswordresponse.enabled + }).catch((err) => { + if (err?.response?.data === null) { + this.forgotPasswordEnabled = true + } else { + this.forgotPasswordEnabled = false + } + }) }, // handler async handleUsernameOrEmail (rule, value) { diff --git a/ui/src/views/auth/ResetPassword.vue b/ui/src/views/auth/ResetPassword.vue index c48e9118e5b3..8a9047c5d3eb 100644 --- a/ui/src/views/auth/ResetPassword.vue +++ b/ui/src/views/auth/ResetPassword.vue @@ -69,19 +69,6 @@ - - - - -