Fix url in password reset email #12078

sureshanaparti · 2025-11-14T07:29:28Z

Description

This PR fixes the url in password reset email. (regression from #11379)

Fixes #12050

Doc PR: apache/cloudstack-documentation#621

Types of changes

Breaking change (fix or feature that would cause existing functionality to change)
New feature (non-breaking change which adds functionality)
Bug fix (non-breaking change which fixes an issue)
Enhancement (improves an existing feature and functionality)
Cleanup (Code refactoring and cleanup, that may add test cases)
Build/CI
Test (unit or integration test code)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Major
Minor

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

How did you try to break this feature and the system with this change?

sureshanaparti · 2025-11-14T07:30:51Z

@blueorangutan package

blueorangutan · 2025-11-14T07:32:04Z

@sureshanaparti a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

codecov · 2025-11-14T07:38:36Z

Codecov Report

❌ Patch coverage is 0% with 30 lines in your changes missing coverage. Please review.
✅ Project coverage is 17.60%. Comparing base (4379666) to head (538344d).
⚠️ Report is 127 commits behind head on 4.22.

Files with missing lines	Patch %	Lines
.../java/com/cloud/utils/server/ServerProperties.java	0.00%	18 Missing ⚠️
.../cloudstack/user/UserPasswordResetManagerImpl.java	0.00%	12 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##              4.22   #12078       +/-   ##
============================================
+ Coverage     3.58%   17.60%   +14.01%     
- Complexity       0    15613    +15613     
============================================
  Files          445     5911     +5466     
  Lines        37536   529987   +492451     
  Branches      6905    64751    +57846     
============================================
+ Hits          1346    93278    +91932     
- Misses       36024   426209   +390185     
- Partials       166    10500    +10334

Flag	Coverage Δ
uitests	`3.60% <ø> (+0.01%)`	⬆️
unittests	`18.66% <0.00%> (?)`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copilot

Pull Request Overview

This PR fixes a regression in the password reset email URL functionality introduced in PR #11379. The URL construction was broken because the domain URL was being included twice in the email template.

Key changes:

Consolidated URL construction logic to build the complete reset link in code rather than in the email template
Added fallback to use ManagementServerAddresses when UserPasswordResetDomainURL is not configured
Added trailing slash removal to ensure clean URL formatting

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

server/src/main/java/org/apache/cloudstack/user/UserPasswordResetManagerImpl.java

sureshanaparti · 2025-11-14T07:48:31Z

@blueorangutan package

sureshanaparti · 2025-11-14T07:50:57Z

@blueorangutan package

blueorangutan · 2025-11-14T07:52:04Z

@sureshanaparti a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

blueorangutan · 2025-11-14T09:16:25Z

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 15767

DaanHoogland

looks generally good, one question.

server/src/main/java/org/apache/cloudstack/user/UserPasswordResetManagerImpl.java

sureshanaparti · 2025-11-14T11:47:09Z

@blueorangutan test

blueorangutan · 2025-11-14T11:48:04Z

@sureshanaparti a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests

blueorangutan · 2025-11-14T12:35:58Z

[SF] Trillian Build Failed (tid-14834)

vladimirpetrov · 2025-11-24T15:49:16Z

@blueorangutan test

blueorangutan · 2025-11-24T15:50:04Z

@vladimirpetrov a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests

blueorangutan · 2025-11-25T06:15:47Z

[SF] Trillian test result (tid-14862)
Environment: kvm-ol8 (x2), zone: Advanced Networking with Mgmt server ol8
Total time taken: 48886 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr12078-t14862-kvm-ol8.zip
Smoke tests completed. 149 look OK, 0 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File

DaanHoogland

clgtm

kiranchavala

Hi @sureshanaparti

As discussed please add http before the domain or management IP to make sure the link is clickable

Please check the screenshot

with domainurl

Without domainurl

davift · 2025-12-06T14:23:47Z

Hi @sureshanaparti

As discussed please add http before the domain or management IP to make sure the link is clickable

Please check the screenshot

with domainurl
Without domainurl

dear @kiranchavala and @sureshanaparti ,

I appreciated the checks for the presence of http:// or https://, as well as the logic to apply a default when neither is provided. My recommendation would be to always default to https:// and require users to manually choose a less secure option if they really need it.

This mindset should be applied universally, as users tend to accept whatever the default is. Beyond the general risk of “rogue Wi-Fi” exposing password-reset links, browsers are increasingly moving toward HTTPS-first behavior, and email filters/inspection systems are becoming more suspicious of plain-text HTTP URLs.

Please understand this as purely constructive feedback.

…server.properties file

sureshanaparti · 2025-12-08T11:40:57Z

Hi @sureshanaparti
As discussed please add http before the domain or management IP to make sure the link is clickable
Please check the screenshot
with domainurl

dear @kiranchavala and @sureshanaparti ,

I appreciated the checks for the presence of http:// or https://, as well as the logic to apply a default when neither is provided. My recommendation would be to always default to https:// and require users to manually choose a less secure option if they really need it.

This mindset should be applied universally, as users tend to accept whatever the default is. Beyond the general risk of “rogue Wi-Fi” exposing password-reset links, browsers are increasingly moving toward HTTPS-first behavior, and email filters/inspection systems are becoming more suspicious of plain-text HTTP URLs.

Please understand this as purely constructive feedback.

@davift thanks for the feedback. earlier the password reset link was defaulted to http://, so I've considered the same. Now, I updated the scheme based on the https.enabled flag in server.properties file in the management server (as SSL is enabled when it is set - https://docs.cloudstack.apache.org/en/4.22.0.0/installguide/optional_installation.html#ssl-optional).

sureshanaparti · 2025-12-08T11:42:13Z

@blueorangutan package

blueorangutan · 2025-12-08T11:44:04Z

@sureshanaparti a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

blueorangutan · 2025-12-08T13:08:12Z

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 15945

DaanHoogland · 2025-12-08T13:10:46Z

@blueorangutan test

blueorangutan · 2025-12-08T13:12:03Z

@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests

blueorangutan · 2025-12-09T06:51:43Z

[SF] Trillian test result (tid-14921)
Environment: kvm-ol8 (x2), zone: Advanced Networking with Mgmt server ol8
Total time taken: 59983 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr12078-t14921-kvm-ol8.zip
Smoke tests completed. 143 look OK, 6 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File
test_08_arping_in_ssvm	`Failure`	5.21	test_diagnostics.py
test_uservm_host_control_state	`Failure`	17.13	test_host_control_state.py
ContextSuite context=TestHostControlState>:teardown	`Error`	32.77	test_host_control_state.py
test_02_list_cpvm_vm	`Failure`	0.05	test_ssvm.py
test_04_cpvm_internals	`Failure`	0.06	test_ssvm.py
test_01_vpn_usage	`Error`	1.11	test_usage.py
test_02_unsecure_vm_migration	`Error`	357.95	test_vm_life_cycle.py
test_02_unsecure_vm_migration	`Error`	357.96	test_vm_life_cycle.py
test_08_migrate_vm	`Error`	20.29	test_vm_life_cycle.py
test_01_migrate_vm_strict_tags_success	`Error`	25.42	test_vm_strict_host_tags.py

vishesh92 · 2026-01-12T09:54:20Z

utils/src/main/java/com/cloud/utils/server/ServerProperties.java

        return properties;
    }
+
+    public static boolean isHttpsEnabled() {


IMO, it will be better to set a variable during initialization and just return that. Updating https and other things, require restarting server anyway.

it's already set in ServerDaemon, but not directly accessible. so checking it through server properties where the config is defined.

@vishesh92 update, please check.

server/src/main/java/org/apache/cloudstack/user/UserPasswordResetManagerImpl.java

vishesh92

Left some comments.

kiranchavala · 2026-01-27T08:36:27Z

@blueorangutan package

blueorangutan · 2026-01-27T08:38:04Z

@kiranchavala a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

blueorangutan · 2026-01-27T09:39:17Z

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 16555

vishesh92

clgtm

vishesh92 · 2026-01-27T10:40:42Z

@blueorangutan package

blueorangutan · 2026-01-27T10:42:09Z

@vishesh92 a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

blueorangutan · 2026-01-27T11:45:21Z

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 16559

kiranchavala

LGTM

Tested with no value in user.password.reset.mail.domain.url

Tested with value user.password.reset.mail.domain.url set to kiranchavala.in

HTTP got appened

Tested with value user.password.reset.mail.domain.url to set to a port kiranchavala.in:908

Tested with value user.password.reset.mail.domain.url set to https

sureshanaparti linked an issue Nov 14, 2025 that may be closed by this pull request

[Password Reset Email Template Ignores {{domainUrl}} Variable in CloudStack 4.22] #12050

Open

sureshanaparti requested a review from Copilot November 14, 2025 07:32

Copilot started reviewing on behalf of sureshanaparti November 14, 2025 07:32 View session

Copilot finished reviewing on behalf of sureshanaparti November 14, 2025 07:42

Copilot AI reviewed Nov 14, 2025

View reviewed changes

server/src/main/java/org/apache/cloudstack/user/UserPasswordResetManagerImpl.java Outdated Show resolved Hide resolved

server/src/main/java/org/apache/cloudstack/user/UserPasswordResetManagerImpl.java Outdated Show resolved Hide resolved

sureshanaparti requested review from DaanHoogland and kiranchavala November 14, 2025 07:48

sureshanaparti added the status:needs-testing label Nov 14, 2025

sureshanaparti added this to the 4.22.1 milestone Nov 14, 2025

DaanHoogland reviewed Nov 14, 2025

View reviewed changes

server/src/main/java/org/apache/cloudstack/user/UserPasswordResetManagerImpl.java Outdated Show resolved Hide resolved

sureshanaparti marked this pull request as ready for review December 2, 2025 10:28

sureshanaparti requested review from DaanHoogland and daviftorres December 2, 2025 10:29

DaanHoogland approved these changes Dec 3, 2025

View reviewed changes

kiranchavala reviewed Dec 5, 2025

View reviewed changes

sureshanaparti added 2 commits December 5, 2025 18:19

Fix url in password reset email

067a1cc

Include http:// in the default password reset url (same as earlier)

859f765

Updated http scheme for reset password url based on https.enabled in …

911bb60

…server.properties file

boring-cyborg bot added the component:management-server label Dec 8, 2025

rajujith added this to Apache CloudStack 4.22.1 Jan 5, 2026

rajujith moved this to In Progress in Apache CloudStack 4.22.1 Jan 6, 2026

vishesh92 reviewed Jan 12, 2026

View reviewed changes

server/src/main/java/org/apache/cloudstack/user/UserPasswordResetManagerImpl.java Show resolved Hide resolved

vishesh92 reviewed Jan 12, 2026

View reviewed changes

RosiKyu self-assigned this Jan 26, 2026

sureshanaparti requested a review from vishesh92 January 27, 2026 07:29

kiranchavala self-assigned this Jan 27, 2026

RosiKyu removed their assignment Jan 27, 2026

review comments

538344d

vishesh92 approved these changes Jan 27, 2026

View reviewed changes

kiranchavala removed the status:needs-testing label Jan 28, 2026

kiranchavala approved these changes Jan 28, 2026

View reviewed changes

kiranchavala added the status:ready-for-merge label Jan 28, 2026

Fix url in password reset email #12078

Are you sure you want to change the base?

Fix url in password reset email #12078

Conversation

sureshanaparti commented Nov 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Types of changes

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

How did you try to break this feature and the system with this change?

Uh oh!

sureshanaparti commented Nov 14, 2025

Uh oh!

blueorangutan commented Nov 14, 2025

Uh oh!

codecov bot commented Nov 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

Uh oh!

sureshanaparti commented Nov 14, 2025

Uh oh!

sureshanaparti commented Nov 14, 2025

Uh oh!

blueorangutan commented Nov 14, 2025

Uh oh!

blueorangutan commented Nov 14, 2025

Uh oh!

DaanHoogland left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sureshanaparti commented Nov 14, 2025

Uh oh!

blueorangutan commented Nov 14, 2025

Uh oh!

blueorangutan commented Nov 14, 2025

Uh oh!

vladimirpetrov commented Nov 24, 2025

Uh oh!

blueorangutan commented Nov 24, 2025

Uh oh!

blueorangutan commented Nov 25, 2025

Uh oh!

DaanHoogland left a comment

Choose a reason for hiding this comment

Uh oh!

kiranchavala left a comment

Choose a reason for hiding this comment

Uh oh!

davift commented Dec 6, 2025

Uh oh!

sureshanaparti commented Dec 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sureshanaparti commented Dec 8, 2025

Uh oh!

blueorangutan commented Dec 8, 2025

Uh oh!

blueorangutan commented Dec 8, 2025

Uh oh!

DaanHoogland commented Dec 8, 2025

Uh oh!

blueorangutan commented Dec 8, 2025

Uh oh!

blueorangutan commented Dec 9, 2025

Uh oh!

vishesh92 Jan 12, 2026

Choose a reason for hiding this comment

Uh oh!

sureshanaparti Jan 27, 2026

sureshanaparti commented Nov 14, 2025 •

edited

Loading

codecov bot commented Nov 14, 2025 •

edited

Loading

sureshanaparti commented Dec 8, 2025 •

edited

Loading