FINERACT-2006: Forgot password on login page #5369
Conversation
| public void sendForgotPasswordEmail(String organisationName, String contactName, String address, String username, | ||
| String temporaryPassword) { | ||
| final String subject = "Password Reset Request - " + organisationName; | ||
| final String body = "Dear " + contactName + ",\n\n" + "You have requested to reset your password for your account on " |
There was a problem hiding this comment.
You can use textblock instead as it was introduced in 15+ version
There was a problem hiding this comment.
I tried using text blocks initially, but SpotBugs throws a VA_FORMAT_STRING_USES_NEWLINE error when using .formatted() with text blocks containing literal newlines. I've kept the string concatenation approach to match the existing
sendToUserAccount method pattern in the same file, which avoids the SpotBugs issue while maintaining consistency.
| @Path("/forgot") | ||
| @Consumes({ MediaType.APPLICATION_JSON }) | ||
| @Produces({ MediaType.APPLICATION_JSON }) | ||
| @Operation(summary = "Request password reset", description = "Requests a password reset for the user with the given email. " |
There was a problem hiding this comment.
Done! Updated the @operation description to use a text block for better readability.
| + "If the email exists and the user is active, a temporary password will be sent to the email address. " | ||
| + "The temporary password expires in 24 hours.") | ||
| @RequestBody(required = true, content = @Content(schema = @Schema(implementation = ForgotPasswordRequest.class))) | ||
| @ApiResponses({ @ApiResponse(responseCode = "200", description = "OK") }) |
There was a problem hiding this comment.
no need of @ApiResponses as we can use multiple @ ApiResponse(
There was a problem hiding this comment.
Yes I've removed @ApiResponses wrapper and using @apiresponse directly
| props.put("mail.smtp.socketFactory.class", "javax.net.ssl.SSLSocketFactory");// NOSONAR | ||
| props.put("mail.smtp.socketFactory.fallback", "true"); | ||
| // Only apply strict Gmail settings if we are actually connecting to Gmail | ||
| if (smtpCredentialsData.getHost() != null && smtpCredentialsData.getHost().endsWith("gmail.com")) { |
There was a problem hiding this comment.
What if user has email of other network provider what will happen? eg proton mail, yahoo , outlook?
There was a problem hiding this comment.
The code already handles this - it only applies strict SSL/TLS settings when the SMTP host ends with gmail.com. For other providers (Yahoo, Outlook, ProtonMail, or even local testing with Mailhog), it uses relaxed settings without forcing the SSL SocketFactory. This way, the email service works with any SMTP provider configured in the external services settings.
| + "The temporary password expires in 24 hours.") | ||
| @RequestBody(required = true, content = @Content(schema = @Schema(implementation = ForgotPasswordRequest.class))) | ||
| @ApiResponses({ @ApiResponse(responseCode = "200", description = "OK") }) | ||
| public Response forgotPassword(final ForgotPasswordRequest request) { |
There was a problem hiding this comment.
Need to crosscheck if there is any rate-limit or any safeguards against abuse
There was a problem hiding this comment.
The endpoint has several safeguards in many places but rate limiting is not implemnted.
airajena
force-pushed
the
FINERACT-2006/forgot-password
branch
from
d548441 to
aa3b4a0
Compare
|
Brother, by your logic if it's for strict gmail. Then what about cases
which uses their custom domain for example organization email or education
email from Google workspace. What would happen to that? Will that work?
…On Sat, 24 Jan, 2026, 5:05 pm Aira Jena, ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In
fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/service/GmailBackedPlatformEmailService.java
<#5369 (comment)>:
> @@ -74,12 +74,16 @@ public void sendDefinedEmail(EmailDetail emailDetails) {
props.put("mail.debug", "true");
// these are the added lines
- props.put("mail.smtp.starttls.enable", "true");
- // props.put("mail.smtp.ssl.enable", "true");
-
- props.put("mail.smtp.socketFactory.port", Integer.parseInt(smtpCredentialsData.getPort()));
- props.put("mail.smtp.socketFactory.class", "javax.net.ssl.SSLSocketFactory");// NOSONAR
- props.put("mail.smtp.socketFactory.fallback", "true");
+ // Only apply strict Gmail settings if we are actually connecting to Gmail
+ if (smtpCredentialsData.getHost() != null && smtpCredentialsData.getHost().endsWith("gmail.com")) {
The code already handles this - it only applies strict SSL/TLS settings
when the SMTP host ends with gmail.com. For other providers (Yahoo,
Outlook, ProtonMail, or even local testing with Mailhog), it uses relaxed
settings without forcing the SSL SocketFactory. This way, the email service
works with any SMTP provider configured in the external services settings.
—
Reply to this email directly, view it on GitHub
<#5369 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AHV6TA63OV2P7O3AEJXFMS34INKIHAVCNFSM6AAAAACSUFDBTOVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZTOMBRGQ3TKMRRG4>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
The check is based on the SMTP host (smtpCredentialsData.getHost()), not the user's email domain. Examples: |
|
I'm not sure about this, simply hard coding this logic is not the correct
way to do it. What if google adds new SMTP hostname, de we have to maintain
this setting everytime A new provider gets add. I think it should handled
based on configuration based. This logic seems too fragile for me. Maybe I
am wrong but way it is implemented in not right, these type of settings
should be configuration driven.
…On Sat, 24 Jan, 2026, 6:58 pm Aira Jena, ***@***.***> wrote:
*airajena* left a comment (apache/fineract#5369)
<#5369 (comment)>
Brother, by your logic if it's for strict gmail. Then what about cases
which uses their custom domain for example organization email or education
email from Google workspace. What would happen to that? Will that work?
… <#m_800436846468510565_>
On Sat, 24 Jan, 2026, 5:05 pm Aira Jena, *@*.*> wrote: @.** commented on
this pull request. ------------------------------ In
fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/service/GmailBackedPlatformEmailService.java
<#5369 (comment)
<#5369 (comment)>>: >
@@ -74,12 +74,16 @@ public void sendDefinedEmail(EmailDetail emailDetails)
{ props.put("mail.debug", "true"); // these are the added lines -
props.put("mail.smtp.starttls.enable", "true"); - //
props.put("mail.smtp.ssl.enable", "true"); - -
props.put("mail.smtp.socketFactory.port",
Integer.parseInt(smtpCredentialsData.getPort())); -
props.put("mail.smtp.socketFactory.class",
"javax.net.ssl.SSLSocketFactory");// NOSONAR -
props.put("mail.smtp.socketFactory.fallback", "true"); + // Only apply
strict Gmail settings if we are actually connecting to Gmail + if
(smtpCredentialsData.getHost() != null &&
smtpCredentialsData.getHost().endsWith("gmail.com")) { The code already
handles this - it only applies strict SSL/TLS settings when the SMTP host
ends with gmail.com. For other providers (Yahoo, Outlook, ProtonMail, or
even local testing with Mailhog), it uses relaxed settings without forcing
the SSL SocketFactory. This way, the email service works with any SMTP
provider configured in the external services settings. — Reply to this
email directly, view it on GitHub <#5369 (comment)
<#5369 (comment)>>,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AHV6TA63OV2P7O3AEJXFMS34INKIHAVCNFSM6AAAAACSUFDBTOVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZTOMBRGQ3TKMRRG4
. You are receiving this because you commented.Message ID: *@*.***>
The check is based on the SMTP host (smtpCredentialsData.getHost()), not
the user's email domain.
For Google Workspace with custom domains (e.g., ***@***.*** or
***@***.***), the SMTP server is still smtp.gmail.com. So the
condition getHost().endsWith("gmail.com") would correctly apply the
Gmail-specific SSL settings.
Examples:
Gmail ***@***.***) → SMTP host: smtp.gmail.com → Gmail settings
applied
Google Workspace ***@***.***) → SMTP host: smtp.gmail.com → Gmail
settings applied
Outlook 365 ***@***.***) → SMTP host: smtp.office365.com → elaxed
settings
Yahoo → SMTP host: smtp.mail.yahoo.com → Relaxed settings
The SMTP host is configured in Fineract's external services settings by
the administrator, so it correctly identifies the mail provider regardless
of the sender's email domain.
—
Reply to this email directly, view it on GitHub
<#5369 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AHV6TA4DQKCTCOXHYJ4GUE34INXQXAVCNFSM6AAAAACSUFDBTOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTOOJUGYZTEMJZGQ>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
You make a very valid point. Ideally, these SSL/TLS settings (socketFactory, starttls, etc.) should be stored in the database configuration rather than inferred from the hostname. However, the current SMTPCredentialsData only supports basic fields (host, port, username, password). Making this fully configuration-driven would require schema changes, API updates, and UI changes to the External Services configuration, which is a significant refactor outside the scope of this "Forgot Password" feature. This logic was added as a pragmatic "bridge" to support standard Gmail configurations (which many users employ) while preventing breakage for non-standard providers (like Mailtrap/Mailhog/Outlook) that fail if SSLSocketFactory is forced. Would you be okay if we keep this safeguard for now to unblock this feature? I can create a follow-up issue/PR to refactor the Email Service to be fully generic and configuration-driven. |
| props.put("mail.smtp.socketFactory.class", "javax.net.ssl.SSLSocketFactory");// NOSONAR | ||
| props.put("mail.smtp.socketFactory.fallback", "true"); | ||
| // Only apply strict Gmail settings if we are actually connecting to Gmail | ||
| if (smtpCredentialsData.getHost() != null && smtpCredentialsData.getHost().endsWith("gmail.com")) { |
There was a problem hiding this comment.
I think this change in the source code is not part of the Jira ticket and must be removed. A new Jira ticket should be raised for improving or refactoring the connection for SMTP servers.
There was a problem hiding this comment.
I agree, the SMTP settings refactoring is outside the scope of this ticket. I have reverted those changes and updated the PR. I'll raise a separate ticket to handle the SMTP connection improvements.
airajena
force-pushed
the
FINERACT-2006/forgot-password
branch
from
abf2cff to
b29e23b
Compare
Aman-Mittal
left a comment
Aman-Mittal
left a comment
There was a problem hiding this comment.
Seems ok now, but really curious what type of safeguards are implemented in this, Additionally we need to tests for the code coverage
Below things are on the safeguards side
|
|
I think it would be a good practice if we can implement a limiter on this.
Like you can use forgot password at certain times. Within a span of
interval. Like in every 5-10 minutes at a time for single user?
…On Sun, 25 Jan, 2026, 10:31 pm Aira Jena, ***@***.***> wrote:
*airajena* left a comment (apache/fineract#5369)
<#5369 (comment)>
Seems ok now, but really curious what type of safeguards are implemented
in this, Additionally we need to tests for the code coverage
Below things are on the safeguards side
- The generated password is time-bound (24 hours). After expiry, it’s
automatically invalid and cannot be used.
- Once the user logs in using the temporary password, they are
required to change it immediately, and the temporary credentials are
cleared.
- Any successful password update (either through this flow or via an
admin action) invalidates the temporary password.
- If email delivery fails, the transaction is rolled back so no
orphaned temporary passwords are stored.
- Password reset requests go through the service layer and are logged,
so the activity is traceable
—
Reply to this email directly, view it on GitHub
<#5369 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AHV6TAZBOYWXIQOMB2PGVW34ITZHRAVCNFSM6AAAAACSUFDBTOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTOOJWHE2TOMZRHA>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
I didn’t include it in this PR to keep the scope limited to the core “forgot password” functionality and avoid introducing additional persistence or infrastructure concerns. That said, I think this is definitely worth addressing. I’m happy to raise a follow-up JIRA ticket and work on the rate-limiting implementation separately so we can design it cleanly without blocking this feature. Let me know if that approach works for you. Also I wanted to ask one thing, is there any slack channel that I can get access for discussions? |
|
It's of fineract website you have to subscribe on your email
***@***.***
…On Sun, 25 Jan, 2026, 11:56 pm Aira Jena, ***@***.***> wrote:
*airajena* left a comment (apache/fineract#5369)
<#5369 (comment)>
I think it would be a good practice if we can implement a limiter on this.
Like you can use forgot password at certain times. Within a span of
interval. Like in every 5-10 minutes at a time for single user?
… <#m_-176778364799698856_m_-3566639483503118824_>
On Sun, 25 Jan, 2026, 10:31 pm Aira Jena, *@*.*> wrote: airajena left a
comment (apache/fineract#5369
<#5369>) <#5369 (comment)
<#5369 (comment)>>
Seems ok now, but really curious what type of safeguards are implemented in
this, Additionally we need to tests for the code coverage Below things are
on the safeguards side - The generated password is time-bound (24 hours).
After expiry, it’s automatically invalid and cannot be used. - Once the
user logs in using the temporary password, they are required to change it
immediately, and the temporary credentials are cleared. - Any successful
password update (either through this flow or via an admin action)
invalidates the temporary password. - If email delivery fails, the
transaction is rolled back so no orphaned temporary passwords are stored. -
Password reset requests go through the service layer and are logged, so the
activity is traceable — Reply to this email directly, view it on GitHub
<#5369 (comment)
<#5369 (comment)>>, or
unsubscribe
https://github.com/notifications/unsubscribe-auth/AHV6TAZBOYWXIQOMB2PGVW34ITZHRAVCNFSM6AAAAACSUFDBTOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTOOJWHE2TOMZRHA
<https://github.com/notifications/unsubscribe-auth/AHV6TAZBOYWXIQOMB2PGVW34ITZHRAVCNFSM6AAAAACSUFDBTOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTOOJWHE2TOMZRHA>
. You are receiving this because you commented.Message ID: @.*>
I didn’t include it in this PR to keep the scope limited to the core
“forgot password” functionality and avoid introducing additional
persistence or infrastructure concerns. That said, I think this is
definitely worth addressing. I’m happy to raise a follow-up JIRA ticket and
work on the rate-limiting implementation separately so we can design it
cleanly without blocking this feature. Let me know if that approach works
for you. Also I wanted to ask one thing, is there any slack channel that I
can get access for discussions?
—
Reply to this email directly, view it on GitHub
<#5369 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AHV6TA4CORFA5SHHOOVEC3D4IUDE7AVCNFSM6AAAAACSUFDBTOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTOOJXGA2TCNBWGQ>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
|
dev (at) fineract (dot) apache (dot) org
On Mon, 26 Jan, 2026, 12:01 am Aman Mittal, ***@***.***>
wrote:
… It's of fineract website you have to subscribe on your email
***@***.***
On Sun, 25 Jan, 2026, 11:56 pm Aira Jena, ***@***.***>
wrote:
> *airajena* left a comment (apache/fineract#5369)
> <#5369 (comment)>
>
> I think it would be a good practice if we can implement a limiter on
> this. Like you can use forgot password at certain times. Within a span of
> interval. Like in every 5-10 minutes at a time for single user?
> … <#m_-3083136956501644212_m_-176778364799698856_m_-3566639483503118824_>
> On Sun, 25 Jan, 2026, 10:31 pm Aira Jena, *@*.*> wrote: airajena left a
> comment (apache/fineract#5369
> <#5369>) <#5369 (comment)
> <#5369 (comment)>>
> Seems ok now, but really curious what type of safeguards are implemented in
> this, Additionally we need to tests for the code coverage Below things are
> on the safeguards side - The generated password is time-bound (24 hours).
> After expiry, it’s automatically invalid and cannot be used. - Once the
> user logs in using the temporary password, they are required to change it
> immediately, and the temporary credentials are cleared. - Any successful
> password update (either through this flow or via an admin action)
> invalidates the temporary password. - If email delivery fails, the
> transaction is rolled back so no orphaned temporary passwords are stored. -
> Password reset requests go through the service layer and are logged, so the
> activity is traceable — Reply to this email directly, view it on GitHub
> <#5369 (comment)
> <#5369 (comment)>>, or
> unsubscribe
> https://github.com/notifications/unsubscribe-auth/AHV6TAZBOYWXIQOMB2PGVW34ITZHRAVCNFSM6AAAAACSUFDBTOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTOOJWHE2TOMZRHA
> <https://github.com/notifications/unsubscribe-auth/AHV6TAZBOYWXIQOMB2PGVW34ITZHRAVCNFSM6AAAAACSUFDBTOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTOOJWHE2TOMZRHA>
> . You are receiving this because you commented.Message ID: @.*>
>
> I didn’t include it in this PR to keep the scope limited to the core
> “forgot password” functionality and avoid introducing additional
> persistence or infrastructure concerns. That said, I think this is
> definitely worth addressing. I’m happy to raise a follow-up JIRA ticket and
> work on the rate-limiting implementation separately so we can design it
> cleanly without blocking this feature. Let me know if that approach works
> for you. Also I wanted to ask one thing, is there any slack channel that I
> can get access for discussions?
>
> —
> Reply to this email directly, view it on GitHub
> <#5369 (comment)>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AHV6TA4CORFA5SHHOOVEC3D4IUDE7AVCNFSM6AAAAACSUFDBTOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTOOJXGA2TCNBWGQ>
> .
> You are receiving this because you commented.Message ID:
> ***@***.***>
>
|
3 participants
Description
Implemented the "Forgot Password" functionality to allow users to reset their forgotten passwords via email. This feature introduces a new public API endpoint that verifies the user's email, generates a temporary password, and emails it to them.
Changes
POST /api/v1/password/forgotwhich accepts an email address in the request body.SecurityConfigto permit unauthenticated access to this endpoint.temporary_password_expiry_timecolumn to them_appusertable (via Liquibase migration0209_add_forgot_password.xml).AppUserentity to handle temporary password expiry.AppUserRepository.findActiveUserByEmailto lookup users.ForgotPasswordServiceand its implementationForgotPasswordServiceImpl.GmailBackedPlatformEmailServiceto make strict SSL/TLS settings conditional. This allows the service to support standard SMTP servers (like Mailhog) for easier local testing and development, while still enforcing strict security when connecting to Gmail.Checklist
Please confirm these details:
developbranch./gradlew spotlessApply)Testing
200 OKon success.