Add integration tests to password recovery flow enumeration controls#28052
Add integration tests to password recovery flow enumeration controls#28052VIHANGAGIT wants to merge 6 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR adds integration test infrastructure for the 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
modules/integration/tests-integration/tests-backend/src/test/resources/org/wso2/identity/integration/test/rest/api/server/flow/execution/v1/password-recovery-flow.json (1)
1-248: ⚡ Quick winDuplicate flow definition increases maintenance burden.
This file is identical to
modules/integration/tests-integration/tests-backend/src/test/resources/org/wso2/identity/integration/test/rest/api/server/flow/management/v1/password-recovery-flow.json. Maintaining two copies risks inconsistency when updates are needed. Consider extracting the common definition to a shared location or adding validation tests to ensure the files remain synchronized.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@modules/integration/tests-integration/tests-backend/src/test/resources/org/wso2/identity/integration/test/rest/api/server/flow/execution/v1/password-recovery-flow.json` around lines 1 - 248, This password-recovery-flow.json file is a duplicate of the file at modules/integration/tests-integration/tests-backend/src/test/resources/org/wso2/identity/integration/test/rest/api/server/flow/management/v1/password-recovery-flow.json. To fix this duplication issue, identify which location is the canonical source for the password recovery flow definition, delete the duplicate file, and update any test code or references that point to the deleted file to instead use the canonical location. Alternatively, if both locations need to be maintained for different test contexts, add a validation test that verifies both password-recovery-flow.json files remain synchronized to prevent future inconsistencies.modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/flow/management/v1/PasswordRecoveryFlowManagementTest.java (2)
118-121: ⚡ Quick winConsider reusing a static ObjectMapper instance.
Creating a new
ObjectMapperinstance on each call is inefficient.ObjectMapperis thread-safe and expensive to construct, so using a static final instance would improve performance.♻️ Proposed refactor
Add a static ObjectMapper field at the class level:
+ private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper(new JsonFactory()); + private FlowManagementClient flowManagementClient; private String passwordRecoveryFlowRequestJson;Then use it in the helper:
private FlowRequest getFlowRequest() throws IOException { - return new ObjectMapper(new JsonFactory()).readValue(passwordRecoveryFlowRequestJson, FlowRequest.class); + return OBJECT_MAPPER.readValue(passwordRecoveryFlowRequestJson, FlowRequest.class); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/flow/management/v1/PasswordRecoveryFlowManagementTest.java` around lines 118 - 121, The getFlowRequest() method is creating a new ObjectMapper instance with a JsonFactory on each invocation, which is inefficient since ObjectMapper is thread-safe and expensive to construct. Create a static final ObjectMapper field at the class level initialized once with the JsonFactory, and then modify the getFlowRequest() method to reuse this static instance instead of creating a new one each time the method is called.
55-55: ⚡ Quick winRemove unnecessary
staticmodifier.The
passwordRecoveryFlowRequestJsonvariable is declared static but is populated in@BeforeClass, which runs per test instance created by@Factory. The static modifier is unnecessary and could cause issues if the suite configuration changes to allow parallel execution.♻️ Proposed fix
- private static String passwordRecoveryFlowRequestJson; + private String passwordRecoveryFlowRequestJson;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/flow/management/v1/PasswordRecoveryFlowManagementTest.java` at line 55, Remove the static modifier from the passwordRecoveryFlowRequestJson field declaration. Since this variable is populated in the `@BeforeClass` method which runs per test instance created by `@Factory`, the static modifier is unnecessary and can cause issues with parallel test execution. Change the field from a static variable to an instance variable by removing the static keyword.modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/flow/execution/v1/PasswordRecoveryFlowExecutionTest.java (1)
257-267: ⚡ Quick winExtract hard-coded Base64 values to named constants.
Lines 265-266 pass hard-coded Base64-encoded strings
"QWNjb3VudCBNYW5hZ2VtZW50"(Account Management) and"YWNjb3VudC1yZWNvdmVyeQ"(account-recovery) as category and connector IDs. These magic strings reduce readability and maintainability.♻️ Proposed refactor
Add constants at the class level:
private static final String ACCOUNT_LOCKED_I18N_KEY_PREFIX = "account.locked"; private static final String ACCOUNT_DISABLED_I18N_KEY = "account.disabled"; + + // Identity Governance connector IDs (Base64-encoded) + private static final String ACCOUNT_MANAGEMENT_CATEGORY_ID = "QWNjb3VudCBNYW5hZ2VtZW50"; // "Account Management" + private static final String ACCOUNT_RECOVERY_CONNECTOR_ID = "YWNjb3VudC1yZWNvdmVyeQ"; // "account-recovery"Then use them:
- identityGovernanceRestClient.updateConnectors("QWNjb3VudCBNYW5hZ2VtZW50", - "YWNjb3VudC1yZWNvdmVyeQ", connectorsPatchReq); + identityGovernanceRestClient.updateConnectors(ACCOUNT_MANAGEMENT_CATEGORY_ID, + ACCOUNT_RECOVERY_CONNECTOR_ID, connectorsPatchReq);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/flow/execution/v1/PasswordRecoveryFlowExecutionTest.java` around lines 257 - 267, The updatePasswordRecoveryFeatureStatus method contains hard-coded Base64-encoded strings that should be extracted to named constants for better readability and maintainability. Define class-level constants for the Base64 values "QWNjb3VudCBNYW5hZ2VtZW50" (which represents Account Management) and "YWNjb3VudC1yZWNvdmVyeQ" (which represents account-recovery), then replace the hard-coded strings in the identityGovernanceRestClient.updateConnectors call with these constants.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/flow/execution/v1/model/Message.java`:
- Around line 167-173: In the toIndentedString method, the replace call on the
return statement is replacing newline characters with the same newline character
without adding any indentation. Fix this by replacing each newline with a
newline followed by 4 spaces to properly indent the output. This will ensure
that multi-line object string representations are properly formatted with
consistent indentation.
In
`@modules/integration/tests-integration/tests-backend/src/test/resources/org/wso2/identity/integration/test/rest/api/server/flow/execution/v1/password-recovery-flow.json`:
- Around line 171-184: The OTP input field in the password-recovery-flow.json
file has a label mismatch with its executor. In the INPUT field with variant OTP
and identifier otp, update the label property from "Enter the code sent to your
mobile" to "Enter the code sent to your email" to correctly match the
EmailOTPExecutor that delivers the code via email instead of mobile.
In
`@modules/integration/tests-integration/tests-backend/src/test/resources/org/wso2/identity/integration/test/rest/api/server/flow/management/v1/password-recovery-flow.json`:
- Around line 171-184: The label text in the OTP input field's config section is
inconsistent with the executor type being used. The label field currently states
"Enter the code sent to your mobile" but the flow uses EmailOTPExecutor which
sends codes via email. Update the label value in the config object to say "Enter
the code sent to your email" to match the actual delivery method and provide
accurate user guidance.
---
Nitpick comments:
In
`@modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/flow/execution/v1/PasswordRecoveryFlowExecutionTest.java`:
- Around line 257-267: The updatePasswordRecoveryFeatureStatus method contains
hard-coded Base64-encoded strings that should be extracted to named constants
for better readability and maintainability. Define class-level constants for the
Base64 values "QWNjb3VudCBNYW5hZ2VtZW50" (which represents Account Management)
and "YWNjb3VudC1yZWNvdmVyeQ" (which represents account-recovery), then replace
the hard-coded strings in the identityGovernanceRestClient.updateConnectors call
with these constants.
In
`@modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/flow/management/v1/PasswordRecoveryFlowManagementTest.java`:
- Around line 118-121: The getFlowRequest() method is creating a new
ObjectMapper instance with a JsonFactory on each invocation, which is
inefficient since ObjectMapper is thread-safe and expensive to construct. Create
a static final ObjectMapper field at the class level initialized once with the
JsonFactory, and then modify the getFlowRequest() method to reuse this static
instance instead of creating a new one each time the method is called.
- Line 55: Remove the static modifier from the passwordRecoveryFlowRequestJson
field declaration. Since this variable is populated in the `@BeforeClass` method
which runs per test instance created by `@Factory`, the static modifier is
unnecessary and can cause issues with parallel test execution. Change the field
from a static variable to an instance variable by removing the static keyword.
In
`@modules/integration/tests-integration/tests-backend/src/test/resources/org/wso2/identity/integration/test/rest/api/server/flow/execution/v1/password-recovery-flow.json`:
- Around line 1-248: This password-recovery-flow.json file is a duplicate of the
file at
modules/integration/tests-integration/tests-backend/src/test/resources/org/wso2/identity/integration/test/rest/api/server/flow/management/v1/password-recovery-flow.json.
To fix this duplication issue, identify which location is the canonical source
for the password recovery flow definition, delete the duplicate file, and update
any test code or references that point to the deleted file to instead use the
canonical location. Alternatively, if both locations need to be maintained for
different test contexts, add a validation test that verifies both
password-recovery-flow.json files remain synchronized to prevent future
inconsistencies.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: dff843af-23ad-4d01-9ece-776091422832
📒 Files selected for processing (9)
modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/flow/execution/v1/FlowExecutionTestBase.javamodules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/flow/execution/v1/PasswordRecoveryFlowExecutionTest.javamodules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/flow/execution/v1/model/Data.javamodules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/flow/execution/v1/model/Message.javamodules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/flow/management/v1/FlowManagementTestBase.javamodules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/rest/api/server/flow/management/v1/PasswordRecoveryFlowManagementTest.javamodules/integration/tests-integration/tests-backend/src/test/resources/org/wso2/identity/integration/test/rest/api/server/flow/execution/v1/password-recovery-flow.jsonmodules/integration/tests-integration/tests-backend/src/test/resources/org/wso2/identity/integration/test/rest/api/server/flow/management/v1/password-recovery-flow.jsonmodules/integration/tests-integration/tests-backend/src/test/resources/testng.xml
1ab79a2 to
204368b
Compare
2c55b2a to
d291565
Compare
d291565 to
21651a8
Compare
cef1f6d to
f1f0dce
Compare
…to integration-test-recovery-flow-enumeration-controls
|
PR builder started |
|
PR builder completed |
c203c91 to
9ab3715
Compare
|



Purpose
Adds integration coverage for the password-recovery flow's user-enumeration and account-status controls — the UserResolveExecutor's notifyUserExistence and notifyUserAccountStatus flags. When enabled, the executor surfaces a user-facing message (carrying an i18n key) instead of silently advancing the flow, for a non-existent identifier, a locked account, or a disabled account.
Password Recovery Flow Support:
FlowExecutionTestBaseandFlowManagementTestBase, including methods to add, enable, and disable password recovery flows, and the corresponding flow type constant. [1] [2] [3] [4] [5]Negative Test Coverage for Password Recovery:
RecoveryEnumerationControlsExecutionNegativeTestto cover negative scenarios for password recovery flow execution, such as attempting to initiate or execute the flow before it is enabled, using invalid flow IDs, and submitting empty inputs.User-Facing Message Model and Integration:
Messagemodel class to represent messages (error, warning, info) surfaced to the user during flow steps, including type, message, and i18n key fields.Datamodel to include a list ofMessageobjects, with appropriate getters, setters, and methods for adding messages, and updatedequals,hashCode, andtoStringmethods to include the new field. [1] [2] [3] [4] [5]Related Issues