[AC-1344] Provider users unable to bulk restore vault items for client organizations

[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

Bulk restoring items in the web vault is not working for Provider users because the existing API endpoint only allows that action for Organization members.

Code changes

This PR is related to this clients PR.

• src/Api/Vault/Controllers/CiphersController.cs: Changed output model to ListResponseModel<CipherMiniResponseModel> and moved logic to CipherService. Added new endpoint PutRestoreManyAdmin
• src/Api/Vault/Models/Request/CipherRequestModel.cs: Added OrganizationId field to CipherBulkRestoreRequestModel
• src/Core/Vault/Repositories/ICipherRepository.cs: Added method RestoreByIdsOrganizationIdAsync to restore certain ciphers for an organization
• src/Core/Vault/Services/ICipherService.cs: Added parameters organizationId and orgAdmin to RestoreManyAsync and updated output to ICollection<CipherOrganizationDetails>
• src/Core/Vault/Services/Implementations/CipherService.cs: If an organizationId value is passed and orgAdmin == true then CipherRepository.RestoreByIdsOrganizationIdAsync is used otherwise its CipherRepository.RestoreAsync
• src/Infrastructure.Dapper/Vault/Repositories/CipherRepository.cs: Added method to restore certain ciphers for an Organization
• src/Infrastructure.EntityFramework/Vault/Repositories/CipherRepository.cs: Added method to restore certain ciphers for an Organization
• src/Sql/Sql.sqlproj: Added file Cipher_RestoreByIdsOrganizationId.sql
• src/Sql/Vault/dbo/Stored Procedures/Cipher/Cipher_RestoreByIdsOrganizationId.sql: Stored procedure to bulk restore ciphers that belong to an organization
• test/Core.Test/Vault/Services/CipherServiceTests.cs: Fixed existing unit test
• util/Migrator/DbScripts/2023-04-21_00_CipherRestoreByIdsOrganizationId.sql: SQL migration

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 think having -admin variants of basic operations is an antipattern. It duplicates a lot of the logic - the request, the controller endpoint, conditional logic in the service, and the db sproc, all to do the same thing (restore some ciphers). The only difference is the user's role, which is just authorization logic.

Can you please consider whether we can

1. create a CiphersAuthorizationHandler to encapsulate authorization logic (refer back to [AC-1387] Resource-based authentication for Groups #2845 for this pattern)
2. have 1 repository method + sproc which just restores the provided cipherIds, without needing to distinguish between users and admins

So the flow could more or less be

1. get ciphers (or CipherDetails, whatever the auth logic requires)
2. pass them to bitAuthorizationService to check permissions
3. restore ciphers
4. log the events

(That's very simplified - it may not be that tidy - but that's the ideal.)

This also has the advantage of creating up CiphersAuthorizationHandler for future use. If we can't do the full refactor above then I'd at least like to do that.

[bitwarden-bot]

Checkmarx One – Scan Summary & Details – 5d8bae5a-a9e5-4c55-bdf3-fd853b553c7b

No New Or Fixed Issues Found

[r-tome]

@eliykat as we've agreed to refactor this later to use resource-based authentication, please re-review this.

[eliykat]

Looks good! I'll also request a review from the Vault Team because ciphers are their domain.

[eliykat]

Would be good to add a test for the new parameters/behaviour as well.

[r-tome]

Test added!

[gbubemismith]

Everything looks good. I just have asked one question.

[gbubemismith]

This looks good. Nice job!