[AC-1981] Fix CollectionsController.Get auth check by just checking collections for the requested orgId

[r-tome]

Type of change

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

Objective

CollectionsController.Get would throw a NotFoundException if the user was assigned to collections in multiple organizations.

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]

I still think this is problematic. orgCollections will be all collections the user is assigned to for that org, but not necessarily all they have manage permissions for. So the authorizationService call could still fail.

The idea of this endpoint is "give me all the collections the user is allowed to read". If they can't read all, that basically means just the ones they have manage permissions for. If they don't have any, then it returns an empty ListResponseModel. It should basically never throw.

Therefore I think you just want to do orgCollections = assignedCollections.Where(c => c.OrganizationId == orgId && c.Manage).ToList(); without any further call to AuthorizationService.

Yes it's unfortunate that this is effectively repeating auth logic that we're trying to centralize in the handler, but handlers don't do filtering. It's breaking the abstraction a little, but I think we gotta.

Tagging @shane-melton as he was in these discussions as well.

If (if!) my suggestions here are correct, I think it would also apply to the other "filtering" endpoint here.

[r-tome]

That sounds about right. We did have this discussion for this other endpoint here. I went ahead and did those modifications, let me know what you think.

[eliykat]

lgtm! Thanks for following up on this