[PM-18236] - Use Single Org Requirement

[jrmccannon]

🎟️ Tracking

PM-18236

📔 Objective

In order to help encapsulate all business logic around Policies, we are moving the logic into requirements that can take the raw data and provide meaningful methods to discern the state of the organization and what actions can be performed by a user.

For single organization, this captures that a user cannot be another member of an organization in order to join an exisitng org that has Single Org enabled. If a user is a member of anotehr orgh with Single Organization Policy enabled, they cannot join or create another org.

For more information see here

⏰ Reminders before review

• Contributor guidelines followed
• All formatters and local linters executed and passed
• Written new unit and / or integration tests where applicable
• Protected functional changes with optionality (feature flags)
• Used internationalization (i18n) for all UI strings
• CI builds passed
• Communicated to DevOps any deployment requirements
• Updated any necessary documentation (Confluence, contributing docs) or informed the documentation team

🦮 Reviewer guidelines

• 👍 (:+1:) or similar for great changes
• 📝 (:memo:) or ℹ️ (:information_source:) for notes or general info
• ❓ (:question:) for questions
• 🤔 (:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion
• 🎨 (:art:) for suggestions / improvements
• ❌ (:x:) or ⚠️ (:warning:) for more significant problems or concerns needing attention
• 🌱 (:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt
• ⛏ (:pick:) for minor or nitpick changes

[claude]

Claude finished @jrmccannon's task in 4m 9s —— View job

---

Re-Review: PM-18236 - Use Single Org Requirement

Scope

Re-review triggered by commit 7e6cb45e3 since the last review at 02f73452a.

Changes Since Last Review

Commit 7e6cb45e3 ("Fixed tests by adding no op for req.") adds SingleOrganizationPolicyRequirement mock setups to two existing auto-confirm tests in ConfirmOrganizationUserCommandTests.cs:

• ConfirmUserAsync_WithAutoConfirmPolicyEnabled_DeletesEmergencyAccess (line 759-761)
• ConfirmUserAsync_WithAutoConfirmPolicyNotEnabled_DoesNotDeleteEmergencyAccess (line 803-805)

Both use SingleOrganizationPolicyRequirementTestFactory.NoSinglePolicyOrganizationsForUser() to return an empty policy requirement (no single org policy enforced).

Why this is needed: ConfirmOrganizationUserCommand.CheckPoliciesAsync() unconditionally queries SingleOrganizationPolicyRequirement at line 226, regardless of feature flags. These auto-confirm tests exercise the confirm flow through CheckPoliciesAsync, so they require this mock to avoid test failures from NSubstitute's default null return.

Verification

1. Fix is correct and minimal: Only the two tests that were failing are updated. The mock appropriately uses the no-policy factory method since these tests focus on auto-confirm behavior, not single org policy enforcement.
2. Core requirement logic unchanged: SingleOrganizationPolicyRequirement, SingleOrganizationPolicyErrors, and all command files are unmodified since last review.
3. Test factory usage is consistent: The same SingleOrganizationPolicyRequirementTestFactory.NoSinglePolicyOrganizationsForUser() pattern is used in the earlier ConfirmUser_SingleOrgPolicyEnabled_ThrowsOnConfirm test at line 720.

Previous Findings Status

Previous Finding	Status
⚠️ IMPORTANT - Missing "has other organizations" check	Resolved
🎨 SUGGESTED - Remove development notes	Resolved
🎨 SUGGESTED - Fix test method name typo	Partially resolved (line 396 fixed; line 439 RestoreUse_WithSingleOrgPolicyEnabled_Fails still has the typo)
All eliykat feedback threads	Resolved

New Findings

No new issues found. The change is a straightforward test fix.

[github-actions]

Checkmarx One – Scan Summary & Details – bd5793d0-89d2-4b42-b621-71a683fe41f6

---

New Issues (2)

Checkmarx found the following issues in this Pull Request

#	Severity	Issue	Source File / Package	Checkmarx Insight
1		Path_Traversal	/src/Api/Controllers/SelfHosted/SelfHostedOrganizationLicensesController.cs: 56	details Method at line 56 of /src/Api/Controllers/SelfHosted/SelfHostedOrganizationLicensesController.cs gets dynamic data from the model element. This ... Attack Vector
2		CSRF	/src/Api/Auth/Controllers/AccountsController.cs: 291	details Method at line 291 of /src/Api/Auth/Controllers/AccountsController.cs gets a parameter from a user request from model. This parameter value flow... Attack Vector

---

Fixed Issues (1)

Great job! The following issues were fixed in this Pull Request

Severity	Issue	Source File / Package
	CSRF	/src/Api/KeyManagement/Controllers/AccountsKeyManagementController.cs: 146

[codecov]

Codecov Report

❌ Patch coverage is 87.83784% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 56.87%. Comparing base (fa5fde5) to head (7e6cb45).

Files with missing lines	Patch %	Lines
...uirements/Errors/SingleOrganizationPolicyErrors.cs	57.14%	3 Missing ⚠️
...Features/OrganizationUsers/AcceptOrgUserCommand.cs	85.71%	1 Missing and 1 partial ⚠️
...rganizationUsers/ConfirmOrganizationUserCommand.cs	81.81%	1 Missing and 1 partial ⚠️
...s/RestoreUser/v1/RestoreOrganizationUserCommand.cs	85.71%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #6999   +/-   ##
=======================================
  Coverage   56.87%   56.87%           
=======================================
  Files        2028     2029    +1     
  Lines       88827    88831    +4     
  Branches     7917     7921    +4     
=======================================
+ Hits        50523    50527    +4     
+ Misses      36472    36469    -3     
- Partials     1832     1835    +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[eliykat]

This looks good! I have started a thread in the dev channel about how we handle this feature flag, please let me know what you think there. I will have a closer look at call sites once we agree on an approach.

[eliykat]

Nobody objected, so I suggest you can remove the feature flag from these changes, and add the needs-qa label.

[eliykat]

Great work @jrmccannon , thanks for helping to close out this implementation. The call sites look a lot better.

[eliykat]

Nailed it
EDIT: except that tests are failing

[sonarqubecloud]

Quality Gate passed

Issues
2 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud