Skip to content

Fix ProvisioningTestCase user collision on shared external database - #28254

Open
Skynettheone wants to merge 2 commits into
wso2:masterfrom
Skynettheone:fix/provisioning-testcase-external-db-username-collision
Open

Fix ProvisioningTestCase user collision on shared external database#28254
Skynettheone wants to merge 2 commits into
wso2:masterfrom
Skynettheone:fix/provisioning-testcase-external-db-username-collision

Conversation

@Skynettheone

@Skynettheone Skynettheone commented Aug 2, 2026

Copy link
Copy Markdown

Summary

Fixes #22140.

createUser() and createUserForSecondServer() in ProvisioningTestCase create SCIM users both directly and indirectly through outbound SCIM provisioning, but atEnd() only cleaned up the IDP and SP outbound provisioning config, never the users themselves.

With the default isolated H2 database, each test run starts from a fresh DB, so this went unnoticed. Against an external database shared across the test server instances, the users created by a prior run persist, so createUserForSecondServer() fails on the next run with UserAlreadyExistingUsername when it tries to create testProvisioningUser2, since it already exists in the shared database.

Change

  • Added a USER_MANAGEMENT_CLIENT for the port-offset-0 server so its created user can be managed for cleanup.
  • Generalized isUserExists(String) into isUserExists(int portOffset, String userName) so existence can be checked against any of the three test server instances.
  • Added deleteUserIfExists(int portOffset, String userName) and called it from atEnd() to remove both the directly created users and the users landed via outbound provisioning, so the test is safe to rerun against a shared/persistent external database.

Test plan

  • mvn compile on modules/integration/tests-integration/tests-backend succeeds with this change (BUILD SUCCESS).
  • CI / maintainer to confirm ProvisioningTestCase passes end-to-end against both H2 and an external database, since running the full multi-server integration suite wasn't possible in this environment.

createUser() and createUserForSecondServer() never removed the SCIM
users they created (directly and via outbound provisioning). With
isolated per-run H2 databases this went unnoticed, but against a
persistent external database the users from a prior run remain,
causing createUserForSecondServer() to fail with
UserAlreadyExistingUsername on the next run.

Delete the created users in atEnd() so the test is safe to rerun
against a shared external database.

Fixes wso2#22140
@CLAassistant

CLAassistant commented Aug 2, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@coderabbitai

coderabbitai Bot commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Summary

  • Add user management support for the port-offset-0 server.
  • Check user existence across all test server instances.
  • Remove created users during atEnd() cleanup.
  • Run user cleanup even when other cleanup steps fail.
  • Aggregate cleanup errors and report them after all cleanup steps.
  • Preserve cleanup for IDP and SP outbound provisioning configurations.

Testing

  • mvn compile succeeds.
  • End-to-end validation with H2 and an external database remains pending.

Walkthrough

The provisioning test initializes USER_MANAGEMENT_CLIENT for server offset 0. Teardown attempts application, identity-provider, and user cleanup across server offsets 0, 1, and 2. The cleanup helper checks client availability and user existence before deletion. Cleanup exceptions are logged, aggregated, and thrown after all cleanup attempts. isUserExists(String) delegates to an offset-aware overload that uses the matching user-management client.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The changes address issue #22140 by cleaning up users across server instances and preventing username collisions in shared databases.
Out of Scope Changes check ✅ Passed The changes are limited to ProvisioningTestCase user cleanup and resilient teardown required by issue #22140.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Title check ✅ Passed The title clearly identifies the provisioning test user-collision fix caused by a shared external database.
Description check ✅ Passed The description directly explains the user-cleanup changes, shared-database failure, and validation status.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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/provisioning/ProvisioningTestCase.java`:
- Around line 158-178: Update the teardown flow in ProvisioningTestCase so the
four deleteUserIfExists calls execute from a finally path after application and
identity-provider cleanup, even when preceding cleanup fails. Make
deleteUserIfExists record or propagate exceptions from isUserExists and
deleteUser instead of only logging them, while ensuring all four deletions are
attempted and any cleanup failures are aggregated and reported after the
attempts.
🪄 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 Plus

Run ID: 0ef0574f-e0a5-48a8-8ba0-2aefdcc104ec

📥 Commits

Reviewing files that changed from the base of the PR and between 84e96a7 and d587d00.

📒 Files selected for processing (1)
  • modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/provisioning/ProvisioningTestCase.java

Previously the SP/IdP cleanup loop could throw and abort atEnd()
before the user cleanup calls ran, silently defeating the fix.
Move user cleanup to a finally block so it always runs, and
aggregate cleanup errors instead of only logging them so failures
are visible rather than swallowed.
@sonarqubecloud

sonarqubecloud Bot commented Aug 2, 2026

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ProvisioningTestCase fails when connecting to an external database

2 participants