Repository navigation
Refactor password reset link domain flow #13209
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -216,6 +216,7 @@ void processRequestInContext(final HttpServletRequest req, final HttpServletResp | |
| HttpUtils.RESPONSE_TYPE_XML, ApiServer.JSONcontentType.value()); | ||
| return; | ||
| } | ||
| CallContext.current().setRequestRemoteAddress(req.getServerName()); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @erikbocks , can you consider this comment and see if there is any mitigation to be done? |
||
|
|
||
| final StringBuilder auditTrailSb = new StringBuilder(128); | ||
| auditTrailSb.append(" ").append(remoteAddress.getHostAddress()); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -29,13 +29,16 @@ | |
| import com.github.mustachejava.MustacheFactory; | ||
| import org.apache.cloudstack.api.ApiErrorCode; | ||
| import org.apache.cloudstack.api.ServerApiException; | ||
| import org.apache.cloudstack.context.CallContext; | ||
| import org.apache.cloudstack.framework.config.ConfigKey; | ||
| import org.apache.cloudstack.framework.config.Configurable; | ||
| import org.apache.cloudstack.gui.theme.dao.GuiThemeDetailsDao; | ||
| 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 org.apache.http.conn.util.InetAddressUtils; | ||
|
|
||
| import javax.inject.Inject; | ||
| import javax.naming.ConfigurationException; | ||
|
|
@@ -45,6 +48,7 @@ | |
| import java.util.Date; | ||
| import java.util.HashMap; | ||
| import java.util.HashSet; | ||
| import java.util.List; | ||
| import java.util.Map; | ||
| import java.util.Set; | ||
| import java.util.UUID; | ||
|
|
@@ -65,6 +69,9 @@ public class UserPasswordResetManagerImpl extends ManagerBase implements UserPas | |
| @Inject | ||
| private UserDao userDao; | ||
|
|
||
| @Inject | ||
| private GuiThemeDetailsDao guiThemeDetailsDao; | ||
|
|
||
| private SMTPMailSender mailSender; | ||
|
|
||
| public static ConfigKey<String> PasswordResetMailTemplate = | ||
|
|
@@ -182,26 +189,12 @@ public void setResetTokenAndSend(UserAccount userAccount) { | |
| final String email = userAccount.getEmail(); | ||
| final String username = userAccount.getUsername(); | ||
| final String subject = "Password Reset Request"; | ||
| String domainUrl = UserPasswordResetDomainURL.value(); | ||
| if (StringUtils.isBlank(domainUrl)) { | ||
| String mgmtServerAddr = ManagementServerAddresses.value().split(",")[0]; | ||
| if (ServerProperties.isHttpsEnabled()) { | ||
| domainUrl = "https://" + mgmtServerAddr + ":" + ServerProperties.getHttpsPort(); | ||
| } else { | ||
| domainUrl = "http://" + mgmtServerAddr + ":" + ServerProperties.getHttpPort(); | ||
| } | ||
| } else if (!domainUrl.startsWith("http://") && !domainUrl.startsWith("https://")) { | ||
| if (ServerProperties.isHttpsEnabled()) { | ||
| domainUrl = "https://" + domainUrl; | ||
| } else { | ||
| domainUrl = "http://" + domainUrl; | ||
| } | ||
| } | ||
|
|
||
| domainUrl = domainUrl.replaceAll("/+$", ""); | ||
|
|
||
| String requestDomain = CallContext.current().getRequestRemoteAddress(); | ||
| String resetLinkDomain = getResetLinkDomain(requestDomain); | ||
| String formattedResetLinkDomain = formatResetLinkDomain(resetLinkDomain); | ||
| String resetLink = String.format("%s/client/#/user/resetPassword?username=%s&token=%s", | ||
| domainUrl, username, resetToken); | ||
| formattedResetLinkDomain, username, resetToken); | ||
|
Comment on lines
+193
to
+197
|
||
| String content = getMessageBody(userAccount, resetToken, resetLink); | ||
|
|
||
| SMTPMailProperties mailProperties = new SMTPMailProperties(); | ||
|
|
@@ -222,6 +215,44 @@ public void setResetTokenAndSend(UserAccount userAccount) { | |
| userAccount, userAccount.getAccountId(), userAccount.getDomainId(), email, resetTokenExpiryTime); | ||
| } | ||
|
|
||
| private String getResetLinkDomain(String requestDomain) { | ||
| if (StringUtils.isNotBlank(requestDomain)) { | ||
| logger.debug("Searching for GUI theme with common name that matches the request's domain: [{}]", requestDomain); | ||
| List<Long> commonNameDetails = guiThemeDetailsDao.listGuiThemeIdsByCommonName(requestDomain); | ||
|
|
||
| if (!commonNameDetails.isEmpty()) { | ||
| logger.debug("GUI theme with ID {} was found; using request's domain for password reset link.", commonNameDetails.get(0)); | ||
| return requestDomain; | ||
| } else { | ||
| logger.debug("No GUI theme was found with a common name that matches the request's domain."); | ||
| } | ||
| } | ||
|
|
||
| String configurationDomain = UserPasswordResetDomainURL.value(); | ||
| if (StringUtils.isNotBlank(configurationDomain)) { | ||
| logger.debug("Defaulting reset link's domain to the [{}] configuration value: [{}].", UserPasswordResetDomainURL.key(), UserPasswordResetDomainURL.value()); | ||
| return configurationDomain; | ||
| } | ||
|
|
||
| logger.debug("Using the first IP address in the [{}] configuration for the reset password email domain because the [{}] configuration is not defined.", ManagementServerAddresses.key(), UserPasswordResetDomainURL.key()); | ||
| return ManagementServerAddresses.value().split(",")[0]; | ||
| } | ||
|
|
||
| private String formatResetLinkDomain(String resetLinkDomain) { | ||
| String protocol = ServerProperties.isHttpsEnabled() ? "https" : "http"; | ||
|
|
||
| if (InetAddressUtils.isIPv4Address(resetLinkDomain)) { | ||
| int port = protocol.equals("https") ? ServerProperties.getHttpsPort() : ServerProperties.getHttpPort(); | ||
| resetLinkDomain = resetLinkDomain + ":" + port; | ||
| } | ||
|
|
||
| if (!resetLinkDomain.startsWith("http")) { | ||
| resetLinkDomain = protocol + "://" + resetLinkDomain; | ||
| } | ||
|
|
||
| return resetLinkDomain.replaceAll("/+$", ""); | ||
| } | ||
|
Comment on lines
+218
to
+254
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. maybe use ai to generate these trivial tests (they excel at this) |
||
|
|
||
| @Override | ||
| public boolean validateAndResetPassword(UserAccount user, String token, String password) { | ||
| UserDetailVO resetTokenDetail = userDetailsDao.findDetail(user.getId(), PasswordResetToken); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this valid @erikbocks ?