Introduce password reset completion property in authentication context#7731
Introduce password reset completion property in authentication context#7731NipuniBhagya wants to merge 1 commit intowso2:masterfrom
Conversation
![wso2-engineering[bot]](https://avatars.githubusercontent.com/in/1856828?s=60&v=4){kind=small}
| case FrameworkConstants.JSAttributes.JS_PASSWORD_RESET_COMPLETE: | ||
| return getWrapped().getProperty(FrameworkConstants.JSAttributes.JS_PASSWORD_RESET_COMPLETE) != null; |
There was a problem hiding this comment.
Log Improvement Suggestion No: 1
| case FrameworkConstants.JSAttributes.JS_PASSWORD_RESET_COMPLETE: | |
| return getWrapped().getProperty(FrameworkConstants.JSAttributes.JS_PASSWORD_RESET_COMPLETE) != null; | |
| case FrameworkConstants.JSAttributes.JS_PASSWORD_RESET_COMPLETE: | |
| return getWrapped().getProperty(FrameworkConstants.JSAttributes.JS_PASSWORD_RESET_COMPLETE) != null; | |
| boolean isPasswordResetComplete = getWrapped().getProperty(FrameworkConstants.JSAttributes.JS_PASSWORD_RESET_COMPLETE) != null; | |
| if (log.isDebugEnabled()) { | |
| log.debug("Checking if password reset is complete: " + isPasswordResetComplete); | |
| } | |
| return isPasswordResetComplete; |
| case FrameworkConstants.JSAttributes.JS_PASSWORD_RESET_COMPLETE: | ||
| return getWrapped().getProperty(FrameworkConstants.JSAttributes.JS_PASSWORD_RESET_COMPLETE); |
There was a problem hiding this comment.
Log Improvement Suggestion No: 2
| case FrameworkConstants.JSAttributes.JS_PASSWORD_RESET_COMPLETE: | |
| return getWrapped().getProperty(FrameworkConstants.JSAttributes.JS_PASSWORD_RESET_COMPLETE); | |
| case FrameworkConstants.JSAttributes.JS_PASSWORD_RESET_COMPLETE: | |
| return getWrapped().getProperty(FrameworkConstants.JSAttributes.JS_PASSWORD_RESET_COMPLETE); | |
| Object passwordResetStatus = getWrapped().getProperty(FrameworkConstants.JSAttributes.JS_PASSWORD_RESET_COMPLETE); | |
| if (log.isDebugEnabled()) { | |
| log.debug("Retrieved password reset complete status from authentication context"); | |
| } | |
| return passwordResetStatus; |
There was a problem hiding this comment.
AI Agent Log Improvement Checklist
- The log-related comments and suggestions in this review were generated by an AI tool to assist with identifying potential improvements. Purpose of reviewing the code for log improvements is to improve the troubleshooting capabilities of our products.
- Please make sure to manually review and validate all suggestions before applying any changes. Not every code suggestion would make sense or add value to our purpose. Therefore, you have the freedom to decide which of the suggestions are helpful.
✅ Before merging this pull request:
- Review all AI-generated comments for accuracy and relevance.
- Complete and verify the table below. We need your feedback to measure the accuracy of these suggestions and the value they add. If you are rejecting a certain code suggestion, please mention the reason briefly in the suggestion for us to capture it.
| Comment | Accepted (Y/N) | Reason |
|---|---|---|
| #### Log Improvement Suggestion No: 1 | ||
| #### Log Improvement Suggestion No: 2 |
WalkthroughThese changes introduce password reset completion tracking to the authentication framework by adding a private field to Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
components/authentication-framework/org.wso2.carbon.identity.application.authentication.framework/src/main/java/org/wso2/carbon/identity/application/authentication/framework/context/AuthenticationContext.java (1)
1-2:⚠️ Potential issue | 🟡 MinorUpdate the copyright year range to include 2026.
The copyright header shows
2013-2023but since this file is being modified, it should be updated to2013-2026. As per coding guidelines, all Java files should contain the appropriate license header with copyright year as the current year or a range ending in the current year.- * Copyright (c) 2013-2023, WSO2 LLC. (http://www.wso2.com). + * Copyright (c) 2013-2026, WSO2 LLC. (http://www.wso2.com).components/authentication-framework/org.wso2.carbon.identity.application.authentication.framework/src/main/java/org/wso2/carbon/identity/application/authentication/framework/config/model/graph/js/JsAuthenticationContext.java (1)
1-2:⚠️ Potential issue | 🟡 MinorUpdate the copyright year range to include 2026.
- * Copyright (c) 2022, WSO2 LLC. (http://www.wso2.com). + * Copyright (c) 2022-2026, WSO2 LLC. (http://www.wso2.com).
🤖 Fix all issues with AI agents
In
`@components/authentication-framework/org.wso2.carbon.identity.application.authentication.framework/src/main/java/org/wso2/carbon/identity/application/authentication/framework/config/model/graph/js/JsAuthenticationContext.java`:
- Around line 81-82: The code reads JS_PASSWORD_RESET_COMPLETE from the generic
properties map (getWrapped().getProperty(...)) which is disconnected from the
dedicated boolean field passwordResetComplete on AuthenticationContext; after
adding a typed getter/setter on AuthenticationContext (e.g.,
isPasswordResetComplete()/setPasswordResetComplete(boolean)), update
JsAuthenticationContext to call the typed accessor instead of
getWrapped().getProperty(...) — replace usages in hasMember/getMember for
FrameworkConstants.JSAttributes.JS_PASSWORD_RESET_COMPLETE (and the other
occurrences noted) to use getWrapped().isPasswordResetComplete() (or the exact
getter name you added) and adjust any null checks accordingly.
In
`@components/authentication-framework/org.wso2.carbon.identity.application.authentication.framework/src/main/java/org/wso2/carbon/identity/application/authentication/framework/context/AuthenticationContext.java`:
- Around line 128-132: The private field passwordResetComplete in
AuthenticationContext is unused (JsAuthenticationContext reads
JS_PASSWORD_RESET_COMPLETE from the properties map) — either remove the dead
field or add proper accessors and switch JsAuthenticationContext to use them:
add getPasswordResetComplete() and setPasswordResetComplete(boolean) to
AuthenticationContext and update JsAuthenticationContext.getMember() and
hasMember() to call
getWrapped().getPasswordResetComplete()/setPasswordResetComplete(...) instead of
getWrapped().getProperty(JS_PASSWORD_RESET_COMPLETE); if the properties map
approach is intended, delete the passwordResetComplete field to avoid confusion.
| case FrameworkConstants.JSAttributes.JS_PASSWORD_RESET_COMPLETE: | ||
| return getWrapped().getProperty(FrameworkConstants.JSAttributes.JS_PASSWORD_RESET_COMPLETE) != null; |
There was a problem hiding this comment.
Property map access is disconnected from the passwordResetComplete field in AuthenticationContext.
Both hasMember and getMember use getWrapped().getProperty(...) to read from the generic properties map. However, AuthenticationContext declares a dedicated private boolean passwordResetComplete field (with no getter/setter). These two mechanisms are disconnected — setting the boolean field won't affect what getProperty() returns, and vice versa.
If getter/setter methods are added to AuthenticationContext (as suggested in the related comment), update these to use the typed accessor:
Proposed fix using the typed accessor
// In hasMember:
case FrameworkConstants.JSAttributes.JS_PASSWORD_RESET_COMPLETE:
- return getWrapped().getProperty(FrameworkConstants.JSAttributes.JS_PASSWORD_RESET_COMPLETE) != null;
+ return true;
// In getMember:
case FrameworkConstants.JSAttributes.JS_PASSWORD_RESET_COMPLETE:
- return getWrapped().getProperty(FrameworkConstants.JSAttributes.JS_PASSWORD_RESET_COMPLETE);
+ return getWrapped().isPasswordResetComplete();Also applies to: 126-127
🤖 Prompt for AI Agents
In
`@components/authentication-framework/org.wso2.carbon.identity.application.authentication.framework/src/main/java/org/wso2/carbon/identity/application/authentication/framework/config/model/graph/js/JsAuthenticationContext.java`
around lines 81 - 82, The code reads JS_PASSWORD_RESET_COMPLETE from the generic
properties map (getWrapped().getProperty(...)) which is disconnected from the
dedicated boolean field passwordResetComplete on AuthenticationContext; after
adding a typed getter/setter on AuthenticationContext (e.g.,
isPasswordResetComplete()/setPasswordResetComplete(boolean)), update
JsAuthenticationContext to call the typed accessor instead of
getWrapped().getProperty(...) — replace usages in hasMember/getMember for
FrameworkConstants.JSAttributes.JS_PASSWORD_RESET_COMPLETE (and the other
occurrences noted) to use getWrapped().isPasswordResetComplete() (or the exact
getter name you added) and adjust any null checks accordingly.
| /** | ||
| * This attribute is used to mark whether the password reset flow is completed. | ||
| */ | ||
| private boolean passwordResetComplete; | ||
|
|
There was a problem hiding this comment.
The passwordResetComplete field is unused — JsAuthenticationContext reads from the properties map instead.
The new private boolean passwordResetComplete field has no getter or setter methods. Meanwhile, JsAuthenticationContext.getMember() and hasMember() use getWrapped().getProperty(JS_PASSWORD_RESET_COMPLETE), which reads from the generic parameters map — not this field. The field is effectively dead code.
Either add proper accessor methods and use them in JsAuthenticationContext:
Option A: Add getter/setter and use the field
In AuthenticationContext.java, add accessors:
private boolean passwordResetComplete;
+
+ /**
+ * Check whether the password reset flow is completed.
+ *
+ * `@return` true if the password reset flow is completed.
+ */
+ public boolean isPasswordResetComplete() {
+ return passwordResetComplete;
+ }
+
+ /**
+ * Set whether the password reset flow is completed.
+ *
+ * `@param` passwordResetComplete true if the password reset flow is completed.
+ */
+ public void setPasswordResetComplete(boolean passwordResetComplete) {
+ this.passwordResetComplete = passwordResetComplete;
+ }Then update JsAuthenticationContext to use the field directly (see related comment on that file).
Or, if using the generic properties map is the intended approach, remove the field entirely to avoid confusion.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** | |
| * This attribute is used to mark whether the password reset flow is completed. | |
| */ | |
| private boolean passwordResetComplete; | |
| /** | |
| * This attribute is used to mark whether the password reset flow is completed. | |
| */ | |
| private boolean passwordResetComplete; | |
| /** | |
| * Check whether the password reset flow is completed. | |
| * | |
| * `@return` true if the password reset flow is completed. | |
| */ | |
| public boolean isPasswordResetComplete() { | |
| return passwordResetComplete; | |
| } | |
| /** | |
| * Set whether the password reset flow is completed. | |
| * | |
| * `@param` passwordResetComplete true if the password reset flow is completed. | |
| */ | |
| public void setPasswordResetComplete(boolean passwordResetComplete) { | |
| this.passwordResetComplete = passwordResetComplete; | |
| } |
🤖 Prompt for AI Agents
In
`@components/authentication-framework/org.wso2.carbon.identity.application.authentication.framework/src/main/java/org/wso2/carbon/identity/application/authentication/framework/context/AuthenticationContext.java`
around lines 128 - 132, The private field passwordResetComplete in
AuthenticationContext is unused (JsAuthenticationContext reads
JS_PASSWORD_RESET_COMPLETE from the properties map) — either remove the dead
field or add proper accessors and switch JsAuthenticationContext to use them:
add getPasswordResetComplete() and setPasswordResetComplete(boolean) to
AuthenticationContext and update JsAuthenticationContext.getMember() and
hasMember() to call
getWrapped().getPasswordResetComplete()/setPasswordResetComplete(...) instead of
getWrapped().getProperty(JS_PASSWORD_RESET_COMPLETE); if the properties map
approach is intended, delete the passwordResetComplete field to avoid confusion.
|
There was a problem hiding this comment.
Pull request overview
This PR aims to track password-reset flow completion in the authentication framework and expose that state to adaptive authentication JavaScript via the authentication context.
Changes:
- Added
JS_PASSWORD_RESET_COMPLETEconstant inFrameworkConstants.JSAttributes. - Introduced a
passwordResetCompleteboolean field inAuthenticationContext. - Exposed
passwordResetCompleteas a JS context member inJsAuthenticationContext(member presence, value access, and key listing).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
.../util/FrameworkConstants.java |
Adds a JS attribute constant for passwordResetComplete. |
.../context/AuthenticationContext.java |
Adds a new boolean field intended to represent password reset completion state. |
.../js/JsAuthenticationContext.java |
Exposes the new flag to JS via hasMember, getMember, and getMemberKeys. |
| /** | ||
| * This attribute is used to mark whether the password reset flow is completed. | ||
| */ | ||
| private boolean passwordResetComplete; | ||
|
|
There was a problem hiding this comment.
AuthenticationContext introduces the passwordResetComplete field but does not provide any public getter/setter, and there are no internal references to it. As-is, the completion state cannot be set or read by framework code or exposed reliably to JS; add isPasswordResetComplete()/setPasswordResetComplete(boolean) (and, if JS reads via getProperty, keep the property map in sync in the setter).
| /** | ||
| * This attribute is used to mark whether the password reset flow is completed. | ||
| */ | ||
| private boolean passwordResetComplete; | ||
|
|
There was a problem hiding this comment.
License header in this modified Java file ends at 2023, which doesn’t meet the repository requirement for updated files (year/range must end in 2026 and match the standard header format). Update the file header accordingly.
| case FrameworkConstants.JSAttributes.JS_PASSWORD_RESET_COMPLETE: | ||
| return getWrapped().getProperty(FrameworkConstants.JSAttributes.JS_PASSWORD_RESET_COMPLETE) != null; |
There was a problem hiding this comment.
hasMember checks getProperty(JS_PASSWORD_RESET_COMPLETE) != null, which makes context.passwordResetComplete disappear from the JS context unless some code explicitly sets the property. For a boolean state flag, scripts typically need a stable false default; consider always returning true here and have getMember return the boolean value (from a dedicated getter) rather than null/undefined.
| case FrameworkConstants.JSAttributes.JS_PASSWORD_RESET_COMPLETE: | ||
| return getWrapped().getProperty(FrameworkConstants.JSAttributes.JS_PASSWORD_RESET_COMPLETE); |
There was a problem hiding this comment.
getMember(JS_PASSWORD_RESET_COMPLETE) currently returns getProperty(...), but nothing in this PR sets that property, and it may return null or a non-boolean type. To make this reliably usable from adaptive scripts, return a boolean derived from AuthenticationContext state (e.g., isPasswordResetComplete()), or ensure the property is always set to a Boolean before JS execution begins.
| FrameworkConstants.JSAttributes.JS_STEPS, | ||
| FrameworkConstants.JSAttributes.JS_CURRENT_STEP, | ||
| FrameworkConstants.JSAttributes.JS_CURRENT_KNOWN_SUBJECT, | ||
| FrameworkConstants.JSAttributes.JS_RETRY_STEP}; | ||
| FrameworkConstants.JSAttributes.JS_RETRY_STEP, | ||
| FrameworkConstants.JSAttributes.JS_PASSWORD_RESET_COMPLETE | ||
| }; |
There was a problem hiding this comment.
New JS member passwordResetComplete is added to hasMember/getMember/getMemberKeys, but there are existing unit tests for JS context members (Nashorn/Graal). Add tests to verify the new member is exposed and returns the expected boolean value for both true and default/false cases.
| case FrameworkConstants.JSAttributes.JS_PASSWORD_RESET_COMPLETE: | ||
| return getWrapped().getProperty(FrameworkConstants.JSAttributes.JS_PASSWORD_RESET_COMPLETE) != null; |
There was a problem hiding this comment.
License header in this modified Java file uses an older year and format (2022, and doesn’t match the current standard header required for updated files). Update the header to the repository-standard 2026 Apache 2.0 header.
Codecov Report❌ Patch coverage is
❌ Your patch check has failed because the patch coverage (0.00%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## master #7731 +/- ##
============================================
- Coverage 51.19% 49.66% -1.54%
- Complexity 19663 20371 +708
============================================
Files 2151 2155 +4
Lines 126598 135238 +8640
Branches 26053 28316 +2263
============================================
+ Hits 64808 67161 +2353
- Misses 53519 59435 +5916
- Partials 8271 8642 +371
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This pull request introduces support for tracking the completion status of the password reset flow within the authentication framework. The main changes add a new attribute and expose it to the JavaScript authentication context, enabling scripts to check and access the password reset completion state.
Enhancements for password reset flow tracking:
passwordResetCompleteto theAuthenticationContextclass to mark whether the password reset flow is completed.JS_PASSWORD_RESET_COMPLETEinFrameworkConstants.JSAttributesfor consistent referencing in JavaScript contexts.JavaScript context integration:
JsAuthenticationContextto exposeJS_PASSWORD_RESET_COMPLETEas a member, allowing scripts to check its presence and retrieve its value. [1] [2]JS_PASSWORD_RESET_COMPLETEin the list of available JavaScript member keys inJsAuthenticationContext, making it accessible for scripting and introspection.### Proposed changes in this pull request[List all changes you want to add here. If you fixed an issue, please
add a reference to that issue as well.]
When should this PR be merged
[Please describe any preconditions that need to be addressed before we
can merge this pull request.]
Follow up actions
[List any possible follow-up actions here; for instance, testing data
migrations, software that we need to install on staging and production
environments.]
Developer Checklist (Mandatory)
product-isissue to track any behavioral change or migration impact.Checklist (for reviewing)
General
Functionality
Code
Tests
Security
Documentation
Summary by CodeRabbit
Release Notes