[AC-2604] Fix aggregation of CollectionGroup permissions

[eliykat]

Type of change

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

Objective

Fix the following bug: when a user is in multiple groups, and those groups have varying levels of access to the same collection (including at least 1 group with Can Manage access), the user does not always receive Can Manage access to that collection.

This is a bug that @shane-melton picked up earlier when implementing #3793. For the CollectionGroup permissions HidePasswords and ReadOnly, we resolve multiple groups by taking the MIN value - representing the most generous permissions for the user (i.e. 0, indicating that they can view passwords and edit). However, we also applied this to Manage, which gives them the least generous permissions (0, indicating that they cannot manage).

Change MIN([Manage]) to MAX([Manage]) so that if the user has Manage permissions from 1 group, that will override the others.

Code changes

• file.ext: Description of what was changed and why

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

[eliykat]

As this is a db change, let's merge after rc is cut on Monday. (But please review in the meantime!)

EDIT: rc cut has apparently been moved so that's OK

[codecov]

Codecov Report

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

Project coverage is 39.07%. Comparing base (98b7866) to head (ab33fb9).

Files	Patch %	Lines
...tityFramework/Repositories/CollectionRepository.cs	0.00%	2 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #4097   +/-   ##
=======================================
  Coverage   39.07%   39.07%           
=======================================
  Files        1201     1201           
  Lines       58021    58021           
  Branches     5339     5339           
=======================================
  Hits        22670    22670           
- Misses      34294    34295    +1     
+ Partials     1057     1056    -1     

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

[github-actions]

Checkmarx One – Scan Summary & Details – 24f9f193-ea7a-477e-87c2-389ad69193bf

New Issues

Severity	Issue	Source File / Package	Checkmarx Insight
	Privacy_Violation	/src/Api/AdminConsole/Controllers/OrganizationsController.cs: 396	Attack Vector
	Privacy_Violation	/src/Api/AdminConsole/Controllers/OrganizationsController.cs: 343	Attack Vector
	Log_Forging	/src/Api/AdminConsole/Controllers/OrganizationsController.cs: 371	Attack Vector
	Log_Forging	/src/Api/AdminConsole/Controllers/OrganizationsController.cs: 308	Attack Vector

[shane-melton]

Thanks for fixing these!

[rkac-bw]

lgtm