Skip to content

[PM-33951] feat(admin-console): Add InjectOrganizationAttribute and OrganizationModelBinder#7659

Open
JaredScar wants to merge 7 commits into
mainfrom
ac/pm-33919-inject-organization-attribute
Open

[PM-33951] feat(admin-console): Add InjectOrganizationAttribute and OrganizationModelBinder#7659
JaredScar wants to merge 7 commits into
mainfrom
ac/pm-33919-inject-organization-attribute

Conversation

@JaredScar
Copy link
Copy Markdown
Contributor

@JaredScar JaredScar commented May 18, 2026

🎟️ Tracking

https://bitwarden.atlassian.net/browse/PM-33951

📔 Objective

This PR contains the implementation of InjectOrganizationAttribute (renamed to Bind because of conflicting Billing one) needed for #7527, split to their own PR to manage PR size.

…ModelBinder for automatic organization parameter binding
@JaredScar JaredScar requested a review from a team as a code owner May 18, 2026 16:02
@JaredScar JaredScar added the ai-review Request a Claude code review label May 18, 2026
@JaredScar JaredScar requested a review from jrmccannon May 18, 2026 16:02
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 18, 2026

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

This PR introduces a [BindOrganization] parameter attribute and OrganizationModelBinder that loads an Organization from the route (orgId/organizationId) and binds it to action parameters, mirroring the established [InjectOrganizationUser] pattern in the same namespace. The new attribute is applied to GetResetPasswordDetails in OrganizationUsersController, replacing the inline _organizationRepository.GetByIdAsync(orgId) lookup and removing the now-redundant orgId route parameter. Authorization order is preserved: [Authorize<ManageAccountRecoveryRequirement>] runs before model binding, so the DB lookup never executes for unauthorized callers. Comprehensive unit tests cover the happy path, missing/invalid route values, both orgId and organizationId keys, and the not-found case.

Code Review Details

No findings.

@JaredScar JaredScar changed the title [PM-33919] feat(admin-console): Add InjectOrganizationAttribute and OrganizationModelBinder [PM-33951] feat(admin-console): Add InjectOrganizationAttribute and OrganizationModelBinder May 18, 2026
Comment thread src/Api/AdminConsole/Attributes/BindOrganizationAttribute.cs
Comment thread src/Api/AdminConsole/Attributes/InjectOrganizationAttribute.cs Outdated
@codecov
Copy link
Copy Markdown

codecov Bot commented May 18, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 60.44%. Comparing base (7180015) to head (0fdb651).
⚠️ Report is 6 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7659      +/-   ##
==========================================
+ Coverage   60.43%   60.44%   +0.01%     
==========================================
  Files        2140     2141       +1     
  Lines       94622    94641      +19     
  Branches     8443     8445       +2     
==========================================
+ Hits        57188    57209      +21     
+ Misses      35429    35428       -1     
+ Partials     2005     2004       -1     

☔ 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.

Copy link
Copy Markdown
Contributor

@jrmccannon jrmccannon left a comment

Choose a reason for hiding this comment

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

We should add unit tests to this and apply it to a simple endpoint.

…tionModelBinder for organization parameter binding with unit tests
@JaredScar JaredScar requested a review from jrmccannon May 19, 2026 20:04
Comment thread src/Api/AdminConsole/Controllers/OrganizationUsersController.cs Outdated
/// ]]></code>
/// </example>
[AttributeUsage(AttributeTargets.Parameter)]
public sealed class BindOrganizationAttribute() : ModelBinderAttribute(typeof(OrganizationModelBinder));
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.

Lets keep this consistent with the existing InjectOrganizationUser that already exists and changed the name to InjectOrganization

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Clarified in our DMs, but posting here for closing loop. InjectOrganizationAttribute exists via Billing, but is different with how we want to use it. Therefore this will stay named as BindOrganizationAttribute to avoid that ambiguous issue

@JaredScar JaredScar requested a review from jrmccannon May 20, 2026 19:09
Comment thread src/Api/AdminConsole/Controllers/OrganizationUsersController.cs
JaredScar and others added 3 commits May 21, 2026 12:04
…n GetResetPasswordDetails method

- Updated test cases to pass the organization directly instead of relying on repository calls.
- Ensured that the tests correctly assert NotFoundException when the organization user does not match the bound organization.
- Improved clarity in test setup by explicitly binding the organization to the method calls.
@sonarqubecloud
Copy link
Copy Markdown

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

Labels

ai-review Request a Claude code review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants