aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorEdouard CHIN <edouard.chin@shopify.com>2019-05-13 16:02:38 +0200
committerEdouard CHIN <edouard.chin@shopify.com>2019-06-06 15:21:03 +0200
commita5502f4a795d6d34d4f05eeefc9f9a653eff0eb0 (patch)
treec9eeacd4f58546e7cfbceb6d164bc812b0655ab1
parent648144649a71310fc5950a2ffd6de7c284058108 (diff)
downloadrails-a5502f4a795d6d34d4f05eeefc9f9a653eff0eb0.tar.gz
rails-a5502f4a795d6d34d4f05eeefc9f9a653eff0eb0.tar.bz2
rails-a5502f4a795d6d34d4f05eeefc9f9a653eff0eb0.zip
Allow `on_rotation` in MessageEncryptor to be passed in constructor:
- Use case: I'm writing a wrapper around MessageEncryptor to make things easier to rotate a secret in our app. It works something like ```ruby crypt = RotatableSecret.new(['old_secret', 'new_secret']) crypt.decrypt_and_verify(message) ``` I'd like the caller to not have to care about passing the `on_rotation` option and have the wrapper deal with it when instantiating the MessageEncryptor object. Also, almost all of the time the on_rotation should be the same when rotating a secret (logging something or StatsD event) so I think it's not worth having to repeat ourselves each time we decrypt a message.
-rw-r--r--activesupport/CHANGELOG.md17
-rw-r--r--activesupport/lib/active_support/messages/rotator.rb9
-rw-r--r--activesupport/test/message_encryptor_test.rb28
3 files changed, 50 insertions, 4 deletions
diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md
index 955eb7eef9..29b22bb3f9 100644
--- a/activesupport/CHANGELOG.md
+++ b/activesupport/CHANGELOG.md
@@ -1,3 +1,20 @@
+* Allow the on_rotation proc used when decrypting/verifying a message to be
+ be passed at the constructor level.
+
+ Before:
+
+ crypt = ActiveSupport::MessageEncryptor.new('long_secret')
+ crypt.decrypt_and_verify(encrypted_message, on_rotation: proc { ... })
+ crypt.decrypt_and_verify(another_encrypted_message, on_rotation: proc { ... })
+
+ After:
+
+ crypt = ActiveSupport::MessageEncryptor.new('long_secret', on_rotation: proc { ... })
+ crypt.decrypt_and_verify(encrypted_message)
+ crypt.decrypt_and_verify(another_encrypted_message)
+
+ *Edouard Chin*
+
* `delegate_missing_to` would raise a `DelegationError` if the object
delegated to was `nil`. Now the `allow_nil` option has been added to enable
the user to specify they want `nil` returned in this case.
diff --git a/activesupport/lib/active_support/messages/rotator.rb b/activesupport/lib/active_support/messages/rotator.rb
index 823a399d67..50ea7dcd8d 100644
--- a/activesupport/lib/active_support/messages/rotator.rb
+++ b/activesupport/lib/active_support/messages/rotator.rb
@@ -3,11 +3,12 @@
module ActiveSupport
module Messages
module Rotator # :nodoc:
- def initialize(*, **options)
+ def initialize(*, on_rotation: nil, **options)
super
@options = options
@rotations = []
+ @on_rotation = on_rotation
end
def rotate(*secrets, **options)
@@ -17,7 +18,7 @@ module ActiveSupport
module Encryptor
include Rotator
- def decrypt_and_verify(*args, on_rotation: nil, **options)
+ def decrypt_and_verify(*args, on_rotation: @on_rotation, **options)
super
rescue MessageEncryptor::InvalidMessage, MessageVerifier::InvalidSignature
run_rotations(on_rotation) { |encryptor| encryptor.decrypt_and_verify(*args, options) } || raise
@@ -32,7 +33,7 @@ module ActiveSupport
module Verifier
include Rotator
- def verified(*args, on_rotation: nil, **options)
+ def verified(*args, on_rotation: @on_rotation, **options)
super || run_rotations(on_rotation) { |verifier| verifier.verified(*args, options) }
end
@@ -46,7 +47,7 @@ module ActiveSupport
def run_rotations(on_rotation)
@rotations.find do |rotation|
if message = yield(rotation) rescue next
- on_rotation.call if on_rotation
+ on_rotation&.call
return message
end
end
diff --git a/activesupport/test/message_encryptor_test.rb b/activesupport/test/message_encryptor_test.rb
index 9edf07f762..097aa8b5f8 100644
--- a/activesupport/test/message_encryptor_test.rb
+++ b/activesupport/test/message_encryptor_test.rb
@@ -171,6 +171,34 @@ class MessageEncryptorTest < ActiveSupport::TestCase
assert rotated
end
+ def test_on_rotation_can_be_passed_at_the_constructor_level
+ older_message = ActiveSupport::MessageEncryptor.new(secrets[:older], "older sign").encrypt_and_sign(encoded: "message")
+
+ rotated = false
+ encryptor = ActiveSupport::MessageEncryptor.new(@secret, on_rotation: proc { rotated = true })
+ encryptor.rotate secrets[:older], "older sign"
+
+ assert_changes(:rotated, from: false, to: true) do
+ message = encryptor.decrypt_and_verify(older_message)
+
+ assert_equal({ encoded: "message" }, message)
+ end
+ end
+
+ def test_on_rotation_option_takes_precedence_over_the_one_given_in_constructor
+ older_message = ActiveSupport::MessageEncryptor.new(secrets[:older], "older sign").encrypt_and_sign(encoded: "message")
+
+ rotated = false
+ encryptor = ActiveSupport::MessageEncryptor.new(@secret, on_rotation: proc { rotated = true })
+ encryptor.rotate secrets[:older], "older sign"
+
+ assert_changes(:rotated, from: false, to: "Yes") do
+ message = encryptor.decrypt_and_verify(older_message, on_rotation: proc { rotated = "Yes" })
+
+ assert_equal({ encoded: "message" }, message)
+ end
+ end
+
def test_with_rotated_metadata
old_message = ActiveSupport::MessageEncryptor.new(secrets[:old], cipher: "aes-256-gcm").
encrypt_and_sign("metadata", purpose: :rotation)