Add shared user SSO integration tests.#28090
Open
Yasasr1 wants to merge 1 commit into
Open
Conversation
Contributor
📝 WalkthroughSummaryThis PR introduces comprehensive integration test coverage for shared user cross-organization Single Sign-On (SSO) functionality in WSO2 Identity Server, addressing test requirements for issue ChangesNew Test ClassesSharedUserSharedAppCrossOrgSsoTestCase.java (545 lines)
SharedUserSubOrgAppCrossOrgSsoTestCase.java (526 lines)
Test Configuration Updatetestng.xml
Test CoverageBoth test classes validate:
WalkthroughTwo new TestNG integration test classes are added for cross-organization SSO scenarios. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/oauth2/SharedUserSharedAppCrossOrgSsoTestCase.java`:
- Around line 160-171: The testDisableMultiAttributeLogin method disables the
multi-attribute login connector but never restores it, breaking test isolation
and potentially affecting other tests in the suite. Either capture the original
state of the MULTI_ATTRIBUTE_ENABLE_PROPERTY before disabling it in the
testDisableMultiAttributeLogin method and restore it afterward, or add cleanup
logic in an `@AfterClass` or `@After` method that explicitly re-enables the
MULTI_ATTRIBUTE_ENABLE_PROPERTY by setting its value back to "true" using the
identityGovernanceRestClient.updateConnectors call. This ensures the governance
setting is restored to its original state after the test completes.
- Around line 260-264: The filter expression in the userSearchReq object is
missing quotes around the userName value. In the line where the filter is set
with "userName eq " + ROOT_USER_USERNAME, the ROOT_USER_USERNAME value needs to
be wrapped in double quotes to comply with SCIM filter syntax. Modify the filter
string construction to include escaped double quotes around the
ROOT_USER_USERNAME variable so the filter becomes a properly quoted string
value.
In
`@modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/oauth2/SharedUserSubOrgAppCrossOrgSsoTestCase.java`:
- Around line 163-172: The testDisableMultiAttributeLogin() method disables
multi-attribute login by setting MULTI_ATTRIBUTE_ENABLE_PROPERTY to false but
does not restore this configuration afterward, which breaks test isolation by
persisting the change across tests. Add cleanup code in the cleanupTest() method
to revert the MULTI_ATTRIBUTE_ENABLE_PROPERTY back to its original value (true)
using the same ConnectorsPatchReq and
identityGovernanceRestClient.updateConnectors approach, ensuring the
configuration state is properly restored after each test execution.
- Around line 243-247: The SCIM filter construction in the userSearchReq object
is missing quotes around the username value in the filter string. In the filter
parameter where "userName eq " is concatenated with ROOT_USER_USERNAME, wrap the
ROOT_USER_USERNAME variable with double quotes to ensure the SCIM filter syntax
is correct and the username is treated as a proper string literal value.
🪄 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: d067fcb5-9413-4eff-8be1-64fc332de371
📒 Files selected for processing (3)
modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/oauth2/SharedUserSharedAppCrossOrgSsoTestCase.javamodules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/oauth2/SharedUserSubOrgAppCrossOrgSsoTestCase.javamodules/integration/tests-integration/tests-backend/src/test/resources/testng.xml
Comment on lines
+160
to
+171
| @Test(priority = 2, dependsOnMethods = "testInit") | ||
| public void testDisableMultiAttributeLogin() throws Exception { | ||
|
|
||
| ConnectorsPatchReq connectorPatchReq = new ConnectorsPatchReq(); | ||
| connectorPatchReq.setOperation(ConnectorsPatchReq.OperationEnum.UPDATE); | ||
| PropertyReq enableProperty = new PropertyReq(); | ||
| enableProperty.setName(MULTI_ATTRIBUTE_ENABLE_PROPERTY); | ||
| enableProperty.setValue("false"); | ||
| connectorPatchReq.addProperties(enableProperty); | ||
| identityGovernanceRestClient.updateConnectors(ACCOUNT_MGT_CATEGORY_ID, MULTI_ATTRIBUTE_CONNECTOR_ID, | ||
| connectorPatchReq); | ||
| } |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Restore the multi-attribute-login connector state during teardown.
This test disables a shared governance setting but never restores it. That can affect later tests in the same suite and reduce isolation. Capture the original value (or explicitly re-enable in @AfterClass) and restore it.
Suggested fix
+ private String originalMultiAttributeLoginEnabled;
...
public void testDisableMultiAttributeLogin() throws Exception {
+ // Read and cache current value before patch (via existing governance client helper if available).
+ // originalMultiAttributeLoginEnabled = ...;
ConnectorsPatchReq connectorPatchReq = new ConnectorsPatchReq();
connectorPatchReq.setOperation(ConnectorsPatchReq.OperationEnum.UPDATE);
PropertyReq enableProperty = new PropertyReq();
enableProperty.setName(MULTI_ATTRIBUTE_ENABLE_PROPERTY);
enableProperty.setValue("false");
connectorPatchReq.addProperties(enableProperty);
identityGovernanceRestClient.updateConnectors(ACCOUNT_MGT_CATEGORY_ID, MULTI_ATTRIBUTE_CONNECTOR_ID,
connectorPatchReq);
}
...
public void cleanupTest() {
+ if (identityGovernanceRestClient != null) {
+ try {
+ ConnectorsPatchReq restorePatch = new ConnectorsPatchReq();
+ restorePatch.setOperation(ConnectorsPatchReq.OperationEnum.UPDATE);
+ PropertyReq enableProperty = new PropertyReq();
+ enableProperty.setName(MULTI_ATTRIBUTE_ENABLE_PROPERTY);
+ enableProperty.setValue(originalMultiAttributeLoginEnabled != null ?
+ originalMultiAttributeLoginEnabled : "true");
+ restorePatch.addProperties(enableProperty);
+ identityGovernanceRestClient.updateConnectors(
+ ACCOUNT_MGT_CATEGORY_ID, MULTI_ATTRIBUTE_CONNECTOR_ID, restorePatch);
+ } catch (Exception e) {
+ log.error("Failed to restore multi attribute login connector setting.", e);
+ }
+ }
if (identityGovernanceRestClient != null) {
try {
identityGovernanceRestClient.closeHttpClient();Also applies to: 373-446
🤖 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/oauth2/SharedUserSharedAppCrossOrgSsoTestCase.java`
around lines 160 - 171, The testDisableMultiAttributeLogin method disables the
multi-attribute login connector but never restores it, breaking test isolation
and potentially affecting other tests in the suite. Either capture the original
state of the MULTI_ATTRIBUTE_ENABLE_PROPERTY before disabling it in the
testDisableMultiAttributeLogin method and restore it afterward, or add cleanup
logic in an `@AfterClass` or `@After` method that explicitly re-enables the
MULTI_ATTRIBUTE_ENABLE_PROPERTY by setting its value back to "true" using the
identityGovernanceRestClient.updateConnectors call. This ensures the governance
setting is restored to its original state after the test completes.
Comment on lines
+260
to
+264
| String userSearchReq = new JSONObject() | ||
| .put("schemas", new JSONArray().put("urn:ietf:params:scim:api:messages:2.0:SearchRequest")) | ||
| .put("attributes", new JSONArray().put("id")) | ||
| .put("filter", "userName eq " + ROOT_USER_USERNAME) | ||
| .toString(); |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Quote the SCIM userName value in the filter expression.
The filter payload currently builds userName eq <value> without string quoting, which can cause shared-user polling to fail or become parser-dependent. Build it as a quoted value.
Suggested fix
- String userSearchReq = new JSONObject()
+ String userSearchReq = new JSONObject()
.put("schemas", new JSONArray().put("urn:ietf:params:scim:api:messages:2.0:SearchRequest"))
.put("attributes", new JSONArray().put("id"))
- .put("filter", "userName eq " + ROOT_USER_USERNAME)
+ .put("filter", "userName eq \"" + ROOT_USER_USERNAME + "\"")
.toString();📝 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.
Suggested change
| String userSearchReq = new JSONObject() | |
| .put("schemas", new JSONArray().put("urn:ietf:params:scim:api:messages:2.0:SearchRequest")) | |
| .put("attributes", new JSONArray().put("id")) | |
| .put("filter", "userName eq " + ROOT_USER_USERNAME) | |
| .toString(); | |
| String userSearchReq = new JSONObject() | |
| .put("schemas", new JSONArray().put("urn:ietf:params:scim:api:messages:2.0:SearchRequest")) | |
| .put("attributes", new JSONArray().put("id")) | |
| .put("filter", "userName eq \"" + ROOT_USER_USERNAME + "\"") | |
| .toString(); |
🤖 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/oauth2/SharedUserSharedAppCrossOrgSsoTestCase.java`
around lines 260 - 264, The filter expression in the userSearchReq object is
missing quotes around the userName value. In the line where the filter is set
with "userName eq " + ROOT_USER_USERNAME, the ROOT_USER_USERNAME value needs to
be wrapped in double quotes to comply with SCIM filter syntax. Modify the filter
string construction to include escaped double quotes around the
ROOT_USER_USERNAME variable so the filter becomes a properly quoted string
value.
Comment on lines
+163
to
+172
| public void testDisableMultiAttributeLogin() throws Exception { | ||
|
|
||
| ConnectorsPatchReq connectorPatchReq = new ConnectorsPatchReq(); | ||
| connectorPatchReq.setOperation(ConnectorsPatchReq.OperationEnum.UPDATE); | ||
| PropertyReq enableProperty = new PropertyReq(); | ||
| enableProperty.setName(MULTI_ATTRIBUTE_ENABLE_PROPERTY); | ||
| enableProperty.setValue("false"); | ||
| connectorPatchReq.addProperties(enableProperty); | ||
| identityGovernanceRestClient.updateConnectors(ACCOUNT_MGT_CATEGORY_ID, MULTI_ATTRIBUTE_CONNECTOR_ID, | ||
| connectorPatchReq); |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Revert the connector configuration change in teardown to preserve test isolation.
This class disables multi-attribute login but does not restore it. Persisting this change across tests can create order-dependent failures. Restore the previous value (or explicitly re-enable) in cleanupTest().
Also applies to: 360-426
🤖 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/oauth2/SharedUserSubOrgAppCrossOrgSsoTestCase.java`
around lines 163 - 172, The testDisableMultiAttributeLogin() method disables
multi-attribute login by setting MULTI_ATTRIBUTE_ENABLE_PROPERTY to false but
does not restore this configuration afterward, which breaks test isolation by
persisting the change across tests. Add cleanup code in the cleanupTest() method
to revert the MULTI_ATTRIBUTE_ENABLE_PROPERTY back to its original value (true)
using the same ConnectorsPatchReq and
identityGovernanceRestClient.updateConnectors approach, ensuring the
configuration state is properly restored after each test execution.
Comment on lines
+243
to
+247
| String userSearchReq = new JSONObject() | ||
| .put("schemas", new JSONArray().put("urn:ietf:params:scim:api:messages:2.0:SearchRequest")) | ||
| .put("attributes", new JSONArray().put("id")) | ||
| .put("filter", "userName eq " + ROOT_USER_USERNAME) | ||
| .toString(); |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Fix SCIM filter construction by quoting the username value.
The current filter omits quotes around the userName string value, which can make shared-user polling brittle. Build the filter with a quoted literal.
Suggested fix
String userSearchReq = new JSONObject()
.put("schemas", new JSONArray().put("urn:ietf:params:scim:api:messages:2.0:SearchRequest"))
.put("attributes", new JSONArray().put("id"))
- .put("filter", "userName eq " + ROOT_USER_USERNAME)
+ .put("filter", "userName eq \"" + ROOT_USER_USERNAME + "\"")
.toString();🤖 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/oauth2/SharedUserSubOrgAppCrossOrgSsoTestCase.java`
around lines 243 - 247, The SCIM filter construction in the userSearchReq object
is missing quotes around the username value in the filter string. In the filter
parameter where "userName eq " is concatenated with ROOT_USER_USERNAME, wrap the
ROOT_USER_USERNAME variable with double quotes to ensure the SCIM filter syntax
is correct and the username is treated as a proper string literal value.
Contributor
|
PR builder started |
Contributor
|
PR builder completed |
jenkins-is-staging
approved these changes
Jul 7, 2026
jenkins-is-staging
left a comment
jenkins-is-staging
left a comment
Contributor
There was a problem hiding this comment.
Approving the pull request based on the successful pr build https://github.com/wso2/product-is/actions/runs/28842618423
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
Adding the integration tests
Related issue