[PM-6760] Add support for URI checksums #662
Conversation
dani-garcia
changed the title
|
No New Or Fixed Issues Found |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #662 +/- ##
==========================================
+ Coverage 60.39% 60.62% +0.23%
==========================================
Files 172 172
Lines 10553 10638 +85
==========================================
+ Hits 6373 6449 +76
- Misses 4180 4189 +9 ☔ View full report in Codecov by Sentry. |
| if cipher_view.key.is_some() { | ||
| cipher_view.generate_checksums(); | ||
| } |
There was a problem hiding this comment.
I think this should move into the cipher logic. This is not temporary behaviour, and I believe we could easily do this as part of the encyrpt operation?
There was a problem hiding this comment.
Yeah, my thought was to group it with the cipher key behaviour and eventually when the flag was removed move both into the encrypt operation, but we can do it now with the checksums. I've updated the PR
| @@ -139,10 +139,15 @@ pub struct CipherListView { | |||
| } | |||
|
|
|||
| impl KeyEncryptable<SymmetricCryptoKey, Cipher> for CipherView { | |||
| fn encrypt_with_key(self, key: &SymmetricCryptoKey) -> Result<Cipher, CryptoError> { | |||
| fn encrypt_with_key(mut self, key: &SymmetricCryptoKey) -> Result<Cipher, CryptoError> { | |||
There was a problem hiding this comment.
Do we need to actually mutate self? We should be able to mutate just the URIs in login.
There was a problem hiding this comment.
We could change the mutability to only affect the login like this, but to me it seems harder to read:
This code will be nicer when moved to LoginUriView, but we can't do that until we decide to unconditionally generate the checksums, as the LoginUriView doesn't have knowledge of whether a cipher key is used or not.
fn encrypt_with_key(self, key: &SymmetricCryptoKey) -> Result<Cipher, CryptoError> {
let ciphers_key = Cipher::get_cipher_key(key, &self.key)?;
let key = ciphers_key.as_ref().unwrap_or(key);
let mut login = self.login;
// For compatibility reasons, we only create checksums for ciphers that have a key
if ciphers_key.is_some() {
login.generate_checksums();
}
Ok(Cipher {
id: self.id,
organization_id: self.organization_id,
...
login: login.encrypt_with_key(key)?,
Type of change
Objective
Validate the URI checksums of ciphers. This is done in a similar way to the
enable_cipher_key_encryptionflag check