Comparing another known ruby 3/rails 7 fork #2
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Prefixes our encrypted_attributes with library specific attr_encrypted so we are not clashing with the Rails 7.0 definition of encrypted_attributes
Prefix encrypt and decrypt methods with attr_encrypted so we don't clash with rails 7
Address ruby 3.0 support
merrington
changed the base branch from
attr-encrypted-encrypted-attributes
to
master
merrington
commented
Mar 7, 2023
There was a problem hiding this comment.
Highlighted differences between this branch and similar fork - master...wealthsimple:attr_encrypted:attr-encrypted-encrypted-attributes
Comment on lines
+163
to
+167
| instance_variable_get("@#{attribute}") || instance_variable_set("@#{attribute}", attr_encrypted_decrypt(attribute, send(encrypted_attribute_name))) | ||
| end | ||
|
|
||
| define_method("#{attribute}=") do |value| | ||
| send("#{encrypted_attribute_name}=", encrypt(attribute, value)) | ||
| send("#{encrypted_attribute_name}=", attr_encrypted_encrypt(attribute, value)) |
There was a problem hiding this comment.
Calling attr_encrypted(:secure_field) is supposed to create two methods, #secure_field and #secure_field=. This ensures that those methods use the attr_encrypted implementations for decrypt/encrypt.
Otherwise, in rails 7 this would call the rails-versions of #encrypt/#decrypt?
| def decrypt(attribute, encrypted_value, options = {}) | ||
| options = encrypted_attributes[attribute.to_sym].merge(options) | ||
| # email = User.attr_encrypted_decrypt(:email, 'SOME_ENCRYPTED_EMAIL_STRING') | ||
| def attr_encrypted_decrypt(attribute, encrypted_value, options = {}) |
There was a problem hiding this comment.
Renames the .decrypt method (I don't think this or the next one impact Rails 7)
| def encrypt(attribute, value, options = {}) | ||
| options = encrypted_attributes[attribute.to_sym].merge(options) | ||
| # encrypted_email = User.attr_encrypted_encrypt(:email, '[email protected]') | ||
| def attr_encrypted_encrypt(attribute, value, options = {}) |
| encrypted_attributes[attribute.to_sym][:operation] = :decrypting | ||
| encrypted_attributes[attribute.to_sym][:value_present] = self.class.not_empty?(encrypted_value) | ||
| self.class.decrypt(attribute, encrypted_value, evaluated_attr_encrypted_options_for(attribute)) | ||
| def attr_encrypted_decrypt(attribute, encrypted_value) |
There was a problem hiding this comment.
Rename the instance method to not collide with AR 7 #decrypt
| encrypted_attributes[attribute.to_sym][:value_present] = self.class.not_empty?(value) | ||
| self.class.encrypt(attribute, value, evaluated_attr_encrypted_options_for(attribute)) | ||
| # @user.attr_encrypted_encrypt(:email, '[email protected]') | ||
| def attr_encrypted_encrypt(attribute, value) |
There was a problem hiding this comment.
Rename the instance method to not collide with AR 7 #encrypt
|
|
||
| define_method("#{attr}_was") do | ||
| attribute_was(attr) | ||
| end | ||
|
|
||
| if ::ActiveRecord::VERSION::STRING >= "4.1" | ||
| define_method("#{attr}_changed?") do |options = {}| | ||
| attribute_changed?(attr, options) | ||
| attribute_changed?(attr, **options) |
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Why
Until we move to rails AR 7 encryption, we need this gem. Another team has previously used a similar branch, however this one has a couple minor additional changes - notably a ruby 3 fix, and a few other methods were renamed. TBH from what I've seen I think we don't strictly need the other renames, however there are some breaking changes:
SomeModel.encrypt/decryptis renamed to.attr_encrypted_encrypt/.attr_encrypted_decryptsome_record.encrypt/decryptis renamed to#attr_encrypted_encrypt/#attr_encrypted_decryptWhat is changing
See above - most methods get prefixed with
attr_encrypted_to clarify they are not using AR 7 encryptionHow I tested
Security 🛡 (required)
Checklist
How does this PR handle security?
Screenshot
Next steps