Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't change iv field when value is the same #284

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

arthurnn
Copy link
Contributor

@arthurnn arthurnn commented Dec 6, 2017

user = User.new(name: 'hi')
user.name = 'hi'

That code would generate two different ivs.
At first look that is not an issue, but when using attr_encrypted combined
with ActiveRecord this could impose some issues. Like extra writes to the db.
Also, marking some attributes as dirty, when they were actually not changed.

[fixes #216]
also fixes https://stackoverflow.com/questions/47422328/attr-encrypted-updating-all-encrypted-fields-although-i-only-changed-one

```
user = User.new(name: 'hi')
user.name = 'hi'
```

That code would generate two different ivs.
At first look that is not an issue, but when using attr_encrypted combined
with ActiveRecord this could impose some issues. Like extra writes to the db.
Also, marking some attributes as dirty, when they were actually not changed.

[fixes attr-encrypted#216]
@saghaulor
Copy link
Contributor

This duplicates #216. Please see my response as to why this behavior exists here: #216 (comment)

@arthurnn
Copy link
Contributor Author

I understand the problem and I am investigating a solution. One problem with not changing the ciphertext when you update with the same value is that you prevent the ability to re-encrypt. Re-encryption is useful when you want to do a key rotation, or when changing your current crypto implementation.
I'm exploring how we might still conditionally allow an update to the record when the unencrypted value hasn't changed, so as to allow for re-encryption. However, at this time I do not have a solution that is complete or satisfactory.

(Copying your answer from the other issue)

I think it is sensible the use cases where you want to force an update, for the use cases you described. However, I believe when someone is doing so, they will want to force that behaviour. So the default, in my opinion, should be to not update if the value is the same.

@twanmaus
Copy link

twanmaus commented Mar 6, 2018

When updating a single attribute all encrypted attributes of that class are changed also. This is unwanted default behaviour when using audited recording all changes made by some user. This default behavior could be a configurable option.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

iv is updated when the attribute's value hasn't changed
3 participants