[PM-2682] Fix v0 attachments migration on share cipher with org

[fedemkr]

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

Objective

Fix v0 attachments migration when sharing cipher with an organization

Code changes

• CiphersController: Passed the filename and key to the _cipherService.CreateAttachmentShareAsync method to take care of those values.
• CipherAttachment: Added TempMetadata property to hold the new metadata values while we do the migration. This allows us to maintain the data safe while the migration is ongoing or any connectivity issues/error happens in between.
• Cipher: Just add the AttachmentId given that is JSON ignored and when deserializing we lose the id.
• CIpherService:
  • Now in CreateShareAttachmentAsync(...) we pass the filename and key and instead of replacing those values into the current metadata of the cipher's attachment, we clone the attachment, set the new metadata there and save it into the TempMetadata of the original one. So the original gets maintained until the end of the migration flow in case something bad happens in between. Furthermore, this approach was chosen so clients don't have to change anything and it's backwards compatible.
  • Then in ShareAsync(...), fixed a couple of issues regarding Key being referenced in the attachments list as the KeyValuePair instead of the actual attachment metadata. Also, now it handles switching the old metadata for the new metadata correctly by cloning the attachments and updating it so no weird issues happens there or in the possible rollback.
  • Also fixed the rollback logic when an exception is raised inside the ShareAsync(...) process replacing the metadata and removing the collection id <-> cipher id relationship that happens inside _cipherRepository.ReplaceAsync(cipher, collectionIds). There is currently no stored procedure to delete many collection ids for a given cipher so given that this would be an edge case (rollback) of an edge case (v0 migration) I decided to use the _collectionCipherRepository.UpdateCollectionsAsync(...) which I just give the current collections minus the ones we want to remove and that performs the update. LMK if I should change this and create the new Stored Procedure or some other way (btw there is an SP to delete a collection id of a cipher but I didn't want to use it because it was not done in bulk and I had to iterate for each collection id which IMO is worse from the current approach)
• CipherServiceTests: Added main unit tests regarding this change and v0 attachments' 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

[bitwarden-bot]

Checkmarx One – Scan Summary & Details – 105682ce-04d9-42de-9a59-8fe27de029a4

No New Or Fixed Issues Found

[MGibson1]

This looks good from my perspective! nice work

[gbubemismith]

Thanks for this, @fedemkr. LGTM!

[LRNcardozoWDF]

Nice job @fedemkr, LGTM