Delete duplicate BulkCollectionOperation.ReadWithAccess

[eliykat]

Type of change

- [ ] Bug fix
- [ ] New feature development
- [x] Tech debt (refactoring, code cleanup, dependency upgrades, etc)
- [ ] Build/deploy pipeline (DevOps)
- [ ] Other

Objective

A number of us were working on the BulkCollectionAuthorizationHandler at the same time, and we ended up with some duplicate BulkCollectionOperations:

• Read
• ReadAccess
• ReadWithAccess

Read and ReadAccess both shared the same private method in the handler; ReadWithAccess had its own private method which was identical, except that it also authorized the Manage Users custom permission.

We should favour more granular operations, so that "read with access" should be Read && ReadAccess, without the need for the third operation. This also reduces duplicate code.

Code changes

1. delete BulkCollectionOperation.ReadWithAccess, its private method in the handler, and its tests
2. update the usage in the controller to check Read && ReadAccess instead
3. add the Manage Users custom permission to the existing BulkCollectionAuthorizationHandler.CanReadAsync private method, which is used by Read and ReadAccess operations. This ensures no change to the actual authorization logic.

Before you submit

• Please check for formatting errors (dotnet format --verify-no-changes) (required)
• If making database changes - make sure you also update Entity Framework queries and/or migrations
• Please add unit tests where it makes sense to do so (encouraged but not required)
• If this change requires a documentation update - notify the documentation team
• If this change has particular deployment requirements - notify the DevOps team

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 0% with 5 lines in your changes are missing coverage. Please review.

Project coverage is 38.11%. Comparing base (ba36b2d) to head (a80962b).

Files	Patch %	Lines
src/Api/Controllers/CollectionsController.cs	0.00%	5 Missing :warning:

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4029      +/-   ##
==========================================
- Coverage   38.13%   38.11%   -0.03%     
==========================================
  Files        1195     1195              
  Lines       58194    58172      -22     
  Branches     5576     5571       -5     
==========================================
- Hits        22192    22171      -21     
  Misses      34956    34956              
+ Partials     1046     1045       -1     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[github-actions[bot]]

Checkmarx One – Scan Summary & Details – 91fcd390-72e9-408b-b91f-3707ee8ff672

New Issues

Severity	Issue	Source File / Package	Checkmarx Insight
	CSRF	/src/Api/Billing/Public/Controllers/OrganizationController.cs: 44	Attack Vector
	CSRF	/src/Api/Controllers/SelfHosted/SelfHostedOrganizationLicensesController.cs: 71	Attack Vector
	CSRF	/src/Api/AdminConsole/Controllers/OrganizationsController.cs: 376	Attack Vector
	CSRF	/src/Api/Controllers/SelfHosted/SelfHostedOrganizationLicensesController.cs: 97	Attack Vector
	CSRF	/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs: 526	Attack Vector
	CSRF	/src/Api/AdminConsole/Controllers/OrganizationsController.cs: 303	Attack Vector
	CSRF	/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs: 512	Attack Vector
	CSRF	/src/Api/AdminConsole/Controllers/OrganizationsController.cs: 794	Attack Vector
	CSRF	/src/Api/AdminConsole/Controllers/OrganizationsController.cs: 852	Attack Vector
	CSRF	/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs: 244	Attack Vector
	CSRF	/src/Api/AdminConsole/Controllers/OrganizationsController.cs: 827	Attack Vector
	CSRF	/src/Api/AdminConsole/Controllers/OrganizationsController.cs: 407	Attack Vector

[eliykat]

Converting to draft until #4032 is merged as that will cause conflicts.

[eliykat]

This is out of date and I'm not particularly satisfied with it. I'm going to revisit this as part of authorization generally.