Remove advanced key generator rotations from verifier/encryptor.

Noticed that verifiers and encryptors never once mentioned key generators
and salts but only concerned themselves with generated secrets.

Clears up the confusing naming around raw_key and secret as well. And
makes the rotation API follow the constructor signature to the letter.

3 files changed, 66 insertions, 198 deletions

diff --git a/activesupport/lib/active_support/messages/rotator.rb b/activesupport/lib/active_support/messages/rotator.rb
index e18549d735..823a399d67 100644
--- a/activesupport/lib/active_support/messages/rotator.rb
+++ b/activesupport/lib/active_support/messages/rotator.rb
@@ -10,8 +10,8 @@ module ActiveSupport
         @rotations = []
       end
 
-      def rotate(*args)
-        @rotations << create_rotation(*args)
+      def rotate(*secrets, **options)
+        @rotations << build_rotation(*secrets, @options.merge(options))
       end
 
       module Encryptor
@@ -24,27 +24,8 @@ module ActiveSupport
         end
 
         private
-          def create_rotation(raw_key: nil, raw_signed_key: nil, **options)
-            options[:cipher] ||= @cipher
-
-            self.class.new \
-              raw_key || extract_key(options),
-              raw_signed_key || extract_signing_key(options),
-              @options.merge(options.slice(:cipher, :digest, :serializer))
-          end
-
-          def extract_key(cipher:, salt:, key_generator: nil, secret: nil, **)
-            key_generator ||= key_generator_for(secret)
-            key_generator.generate_key(salt, self.class.key_len(cipher))
-          end
-
-          def extract_signing_key(cipher:, signed_salt: nil, key_generator: nil, secret: nil, **)
-            if cipher.downcase.end_with?("cbc")
-              raise ArgumentError, "missing signed_salt for signing key generation" unless signed_salt
-
-              key_generator ||= key_generator_for(secret)
-              key_generator.generate_key(signed_salt)
-            end
+          def build_rotation(secret = @secret, sign_secret = @sign_secret, options)
+            self.class.new(secret, sign_secret, options)
           end
       end
 
@@ -56,21 +37,12 @@ module ActiveSupport
         end
 
         private
-          def create_rotation(raw_key: nil, **options)
-            self.class.new(raw_key || extract_key(options), @options.merge(options.slice(:digest, :serializer)))
-          end
-
-          def extract_key(key_generator: nil, secret: nil, salt:)
-            key_generator ||= key_generator_for(secret)
-            key_generator.generate_key(salt)
+          def build_rotation(secret = @secret, options)
+            self.class.new(secret, options)
           end
       end
 
       private
-        def key_generator_for(secret)
-          ActiveSupport::KeyGenerator.new(secret, iterations: 1000)
-        end
-
         def run_rotations(on_rotation)
           @rotations.find do |rotation|
             if message = yield(rotation) rescue next
diff --git a/activesupport/test/message_encryptor_test.rb b/activesupport/test/message_encryptor_test.rb
index 915038c854..8fde3928dc 100644
--- a/activesupport/test/message_encryptor_test.rb
+++ b/activesupport/test/message_encryptor_test.rb
@@ -115,134 +115,70 @@ class MessageEncryptorTest < ActiveSupport::TestCase
     assert_equal "Ruby on Rails", encryptor.decrypt_and_verify(encrypted_message)
   end
 
-  def test_with_rotated_raw_key
-    old_raw_key = SecureRandom.random_bytes(32)
-    old_encryptor = ActiveSupport::MessageEncryptor.new(old_raw_key, cipher: "aes-256-gcm")
-    old_message = old_encryptor.encrypt_and_sign("message encrypted with old raw key")
+  def test_rotating_secret
+    old_message = ActiveSupport::MessageEncryptor.new(secrets[:old], cipher: "aes-256-gcm").encrypt_and_sign("old")
 
     encryptor = ActiveSupport::MessageEncryptor.new(@secret, cipher: "aes-256-gcm")
-    encryptor.rotate raw_key: old_raw_key
+    encryptor.rotate secrets[:old]
 
-    assert_equal "message encrypted with old raw key", encryptor.decrypt_and_verify(old_message)
+    assert_equal "old", encryptor.decrypt_and_verify(old_message)
   end
 
   def test_rotating_serializer
-    old_raw_key = SecureRandom.random_bytes(32)
-
-    old_message = ActiveSupport::MessageEncryptor.new(old_raw_key, cipher: "aes-256-gcm", serializer: JSON).
+    old_message = ActiveSupport::MessageEncryptor.new(secrets[:old], cipher: "aes-256-gcm", serializer: JSON).
       encrypt_and_sign(ahoy: :hoy)
 
     encryptor = ActiveSupport::MessageEncryptor.new(@secret, cipher: "aes-256-gcm", serializer: JSON)
-    encryptor.rotate raw_key: old_raw_key
+    encryptor.rotate secrets[:old]
 
     assert_equal({ "ahoy" => "hoy" }, encryptor.decrypt_and_verify(old_message))
   end
 
-  def test_with_rotated_secret_and_salt
-    old_secret, old_salt = SecureRandom.random_bytes(32), "old salt"
-    old_raw_key = ActiveSupport::KeyGenerator.new(old_secret, iterations: 1000).generate_key(old_salt, 32)
-
-    old_encryptor = ActiveSupport::MessageEncryptor.new(old_raw_key, cipher: "aes-256-gcm")
-    old_message = old_encryptor.encrypt_and_sign("message encrypted with old secret and salt")
-
-    encryptor = ActiveSupport::MessageEncryptor.new(@secret, cipher: "aes-256-gcm")
-    encryptor.rotate secret: old_secret, salt: old_salt
-
-    assert_equal "message encrypted with old secret and salt", encryptor.decrypt_and_verify(old_message)
-  end
-
-  def test_with_rotated_key_generator
-    old_key_gen, old_salt = ActiveSupport::KeyGenerator.new(SecureRandom.random_bytes(32), iterations: 256), "old salt"
-
-    old_raw_key = old_key_gen.generate_key(old_salt, 32)
-    old_encryptor = ActiveSupport::MessageEncryptor.new(old_raw_key, cipher: "aes-256-gcm")
-    old_message = old_encryptor.encrypt_and_sign("message encrypted with old key generator and salt")
-
-    encryptor = ActiveSupport::MessageEncryptor.new(@secret, cipher: "aes-256-gcm")
-    encryptor.rotate key_generator: old_key_gen, salt: old_salt
-
-    assert_equal "message encrypted with old key generator and salt", encryptor.decrypt_and_verify(old_message)
-  end
-
-  def test_with_rotated_aes_cbc_encryptor_with_raw_keys
-    old_raw_key, old_raw_signed_key = SecureRandom.random_bytes(32), SecureRandom.random_bytes(16)
-
-    old_encryptor = ActiveSupport::MessageEncryptor.new(old_raw_key, old_raw_signed_key, cipher: "aes-256-cbc", digest: "SHA1")
-    old_message = old_encryptor.encrypt_and_sign("message encrypted with old raw keys")
-
-    encryptor = ActiveSupport::MessageEncryptor.new(@secret)
-    encryptor.rotate raw_key: old_raw_key, raw_signed_key: old_raw_signed_key, cipher: "aes-256-cbc", digest: "SHA1"
-
-    assert_equal "message encrypted with old raw keys", encryptor.decrypt_and_verify(old_message)
-  end
-
-  def test_with_rotated_aes_cbc_encryptor_with_secret_and_salts
-    old_secret, old_salt, old_signed_salt = SecureRandom.random_bytes(32), "old salt", "old signed salt"
-
-    old_key_gen = ActiveSupport::KeyGenerator.new(old_secret, iterations: 1000)
-    old_raw_key = old_key_gen.generate_key(old_salt, 32)
-    old_raw_signed_key = old_key_gen.generate_key(old_signed_salt)
-
-    old_encryptor = ActiveSupport::MessageEncryptor.new(old_raw_key, old_raw_signed_key, cipher: "aes-256-cbc", digest: "SHA1")
-    old_message = old_encryptor.encrypt_and_sign("message encrypted with old secret and salts")
+  def test_rotating_aes_cbc_secrets
+    old_encryptor = ActiveSupport::MessageEncryptor.new(secrets[:old], "old sign", cipher: "aes-256-cbc")
+    old_message = old_encryptor.encrypt_and_sign("old")
 
     encryptor = ActiveSupport::MessageEncryptor.new(@secret)
-    encryptor.rotate secret: old_secret, salt: old_salt, signed_salt: old_signed_salt, cipher: "aes-256-cbc", digest: "SHA1"
+    encryptor.rotate secrets[:old], "old sign", cipher: "aes-256-cbc"
 
-    assert_equal "message encrypted with old secret and salts", encryptor.decrypt_and_verify(old_message)
+    assert_equal "old", encryptor.decrypt_and_verify(old_message)
   end
 
-  def test_with_rotating_multiple_encryptors
-    older_raw_key, older_raw_signed_key = SecureRandom.random_bytes(32), SecureRandom.random_bytes(16)
-    old_raw_key, old_raw_signed_key = SecureRandom.random_bytes(32), SecureRandom.random_bytes(16)
-
-    older_encryptor = ActiveSupport::MessageEncryptor.new(older_raw_key, older_raw_signed_key, cipher: "aes-256-cbc", digest: "SHA1")
-    older_message = older_encryptor.encrypt_and_sign("message encrypted with older raw key")
-
-    old_encryptor = ActiveSupport::MessageEncryptor.new(old_raw_key, old_raw_signed_key, cipher: "aes-256-cbc", digest: "SHA1")
-    old_message = old_encryptor.encrypt_and_sign("message encrypted with old raw key")
+  def test_multiple_rotations
+    older_message = ActiveSupport::MessageEncryptor.new(secrets[:older], "older sign").encrypt_and_sign("older")
+    old_message = ActiveSupport::MessageEncryptor.new(secrets[:old], "old sign").encrypt_and_sign("old")
 
     encryptor = ActiveSupport::MessageEncryptor.new(@secret)
-    encryptor.rotate raw_key: old_raw_key, raw_signed_key: old_raw_signed_key, cipher: "aes-256-cbc", digest: "SHA1"
-    encryptor.rotate raw_key: older_raw_key, raw_signed_key: older_raw_signed_key, cipher: "aes-256-cbc", digest: "SHA1"
+    encryptor.rotate secrets[:old], "old sign"
+    encryptor.rotate secrets[:older], "older sign"
 
-    assert_equal "encrypted message", encryptor.decrypt_and_verify(encryptor.encrypt_and_sign("encrypted message"))
-    assert_equal "message encrypted with old raw key", encryptor.decrypt_and_verify(old_message)
-    assert_equal "message encrypted with older raw key", encryptor.decrypt_and_verify(older_message)
+    assert_equal "new",   encryptor.decrypt_and_verify(encryptor.encrypt_and_sign("new"))
+    assert_equal "old",   encryptor.decrypt_and_verify(old_message)
+    assert_equal "older", encryptor.decrypt_and_verify(older_message)
   end
 
-  def test_on_rotation_instance_callback_is_called_and_returns_modified_messages
-    callback_ran, message = nil, nil
-
-    older_raw_key, older_raw_signed_key = SecureRandom.random_bytes(32), SecureRandom.random_bytes(16)
-    old_raw_key, old_raw_signed_key = SecureRandom.random_bytes(32), SecureRandom.random_bytes(16)
-
-    older_encryptor = ActiveSupport::MessageEncryptor.new(older_raw_key, older_raw_signed_key, cipher: "aes-256-cbc", digest: "SHA1")
-    older_message = older_encryptor.encrypt_and_sign(encoded: "message")
+  def test_on_rotation_is_called_and_returns_modified_messages
+    older_message = ActiveSupport::MessageEncryptor.new(secrets[:older], "older sign").encrypt_and_sign(encoded: "message")
 
     encryptor = ActiveSupport::MessageEncryptor.new(@secret)
-    encryptor.rotate raw_key: old_raw_key, raw_signed_key: old_raw_signed_key, cipher: "aes-256-cbc", digest: "SHA1"
-    encryptor.rotate raw_key: older_raw_key, raw_signed_key: older_raw_signed_key, cipher: "aes-256-cbc", digest: "SHA1"
+    encryptor.rotate secrets[:old]
+    encryptor.rotate secrets[:older], "older sign"
 
-    message = encryptor.decrypt_and_verify(older_message, on_rotation: proc { callback_ran = true })
+    rotated = false
+    message = encryptor.decrypt_and_verify(older_message, on_rotation: proc { rotated = true })
 
-    assert callback_ran, "callback was ran"
     assert_equal({ encoded: "message" }, message)
+    assert rotated
   end
 
   def test_with_rotated_metadata
-    old_secret, old_salt = SecureRandom.random_bytes(32), "old salt"
-    old_raw_key = ActiveSupport::KeyGenerator.new(old_secret, iterations: 1000).generate_key(old_salt, 32)
-
-    old_encryptor = ActiveSupport::MessageEncryptor.new(old_raw_key, cipher: "aes-256-gcm")
-    old_message = old_encryptor.encrypt_and_sign(
-      "message encrypted with old secret, salt, and metadata", purpose: "rotation")
+    old_message = ActiveSupport::MessageEncryptor.new(secrets[:old], cipher: "aes-256-gcm").
+      encrypt_and_sign("metadata", purpose: :rotation)
 
     encryptor = ActiveSupport::MessageEncryptor.new(@secret, cipher: "aes-256-gcm")
-    encryptor.rotate secret: old_secret, salt: old_salt
+    encryptor.rotate secrets[:old]
 
-    assert_equal "message encrypted with old secret, salt, and metadata",
-      encryptor.decrypt_and_verify(old_message, purpose: "rotation")
+    assert_equal "metadata", encryptor.decrypt_and_verify(old_message, purpose: :rotation)
   end
 
   private
@@ -264,6 +200,10 @@ class MessageEncryptorTest < ActiveSupport::TestCase
       end
     end
 
+    def secrets
+      @secrets ||= Hash.new { |h,k| h[k] = SecureRandom.random_bytes(32) }
+    end
+
     def munge(base64_string)
       bits = ::Base64.strict_decode64(base64_string)
       bits.reverse!
diff --git a/activesupport/test/message_verifier_test.rb b/activesupport/test/message_verifier_test.rb
index b43fc4b269..05d5c1cbc3 100644
--- a/activesupport/test/message_verifier_test.rb
+++ b/activesupport/test/message_verifier_test.rb
@@ -92,93 +92,49 @@ class MessageVerifierTest < ActiveSupport::TestCase
     assert_equal @data, @verifier.verify(signed_message)
   end
 
-  def test_with_rotated_raw_key
-    old_raw_key = SecureRandom.random_bytes(32)
-
-    old_verifier = ActiveSupport::MessageVerifier.new(old_raw_key, digest: "SHA1")
-    old_message = old_verifier.generate("message verified with old raw key")
-
-    verifier = ActiveSupport::MessageVerifier.new(@secret, digest: "SHA1")
-    verifier.rotate raw_key: old_raw_key
-
-    assert_equal "message verified with old raw key", verifier.verified(old_message)
-  end
-
-  def test_with_rotated_secret_and_salt
-    old_secret, old_salt = SecureRandom.random_bytes(32), "old salt"
-
-    old_raw_key = ActiveSupport::KeyGenerator.new(old_secret, iterations: 1000).generate_key(old_salt)
-    old_verifier = ActiveSupport::MessageVerifier.new(old_raw_key, digest: "SHA1")
-    old_message = old_verifier.generate("message verified with old secret and salt")
+  def test_rotating_secret
+    old_message = ActiveSupport::MessageVerifier.new("old", digest: "SHA1").generate("old")
 
     verifier = ActiveSupport::MessageVerifier.new(@secret, digest: "SHA1")
-    verifier.rotate secret: old_secret, salt: old_salt
+    verifier.rotate "old"
 
-    assert_equal "message verified with old secret and salt", verifier.verified(old_message)
+    assert_equal "old", verifier.verified(old_message)
   end
 
-  def test_with_rotated_key_generator
-    old_key_gen, old_salt = ActiveSupport::KeyGenerator.new(SecureRandom.random_bytes(32), iterations: 256), "old salt"
+  def test_multiple_rotations
+    old_message   = ActiveSupport::MessageVerifier.new("old", digest: "SHA256").generate("old")
+    older_message = ActiveSupport::MessageVerifier.new("older", digest: "SHA1").generate("older")
 
-    old_raw_key = old_key_gen.generate_key(old_salt)
-    old_verifier = ActiveSupport::MessageVerifier.new(old_raw_key, digest: "SHA1")
-    old_message = old_verifier.generate("message verified with old key generator and salt")
+    verifier = ActiveSupport::MessageVerifier.new(@secret, digest: "SHA512")
+    verifier.rotate "old",   digest: "SHA256"
+    verifier.rotate "older", digest: "SHA1"
 
-    verifier = ActiveSupport::MessageVerifier.new(@secret, digest: "SHA1")
-    verifier.rotate key_generator: old_key_gen, salt: old_salt
-
-    assert_equal "message verified with old key generator and salt", verifier.verified(old_message)
+    assert_equal "new",   verifier.verified(verifier.generate("new"))
+    assert_equal "old",   verifier.verified(old_message)
+    assert_equal "older", verifier.verified(older_message)
   end
 
-  def test_with_rotating_multiple_verifiers
-    old_raw_key, older_raw_key = SecureRandom.random_bytes(32), SecureRandom.random_bytes(32)
-
-    old_verifier = ActiveSupport::MessageVerifier.new(old_raw_key, digest: "SHA256")
-    old_message = old_verifier.generate("message verified with old raw key")
+  def test_on_rotation_is_called_and_verified_returns_message
+    older_message = ActiveSupport::MessageVerifier.new("older", digest: "SHA1").generate(encoded: "message")
 
-    older_verifier = ActiveSupport::MessageVerifier.new(older_raw_key, digest: "SHA1")
-    older_message = older_verifier.generate("message verified with older raw key")
+    verifier = ActiveSupport::MessageVerifier.new(@secret, digest: "SHA512")
+    verifier.rotate "old",   digest: "SHA256"
+    verifier.rotate "older", digest: "SHA1"
 
-    verifier = ActiveSupport::MessageVerifier.new("new secret", digest: "SHA512")
-    verifier.rotate raw_key: old_raw_key, digest: "SHA256"
-    verifier.rotate raw_key: older_raw_key, digest: "SHA1"
+    rotated = false
+    message = verifier.verified(older_message, on_rotation: proc { rotated = true })
 
-    assert_equal "verified message", verifier.verified(verifier.generate("verified message"))
-    assert_equal "message verified with old raw key", verifier.verified(old_message)
-    assert_equal "message verified with older raw key", verifier.verified(older_message)
-  end
-
-  def test_on_rotation_keyword_block_is_called_and_verified_returns_message
-    callback_ran, message = nil, nil
-
-    old_raw_key, older_raw_key = SecureRandom.random_bytes(32), SecureRandom.random_bytes(32)
-
-    older_verifier = ActiveSupport::MessageVerifier.new(older_raw_key, digest: "SHA1")
-    older_message = older_verifier.generate(encoded: "message")
-
-    verifier = ActiveSupport::MessageVerifier.new("new secret", digest: "SHA512")
-    verifier.rotate raw_key: old_raw_key, digest: "SHA256"
-    verifier.rotate raw_key: older_raw_key, digest: "SHA1"
-
-    message = verifier.verified(older_message, on_rotation: proc { callback_ran = true })
-
-    assert callback_ran, "callback was ran"
     assert_equal({ encoded: "message" }, message)
+    assert rotated
   end
 
-  def test_with_rotated_metadata
-    old_secret, old_salt = SecureRandom.random_bytes(32), "old salt"
+  def test_rotations_with_metadata
+    old_message = ActiveSupport::MessageVerifier.new("old").generate("old", purpose: :rotation)
 
-    old_raw_key = ActiveSupport::KeyGenerator.new(old_secret, iterations: 1000).generate_key(old_salt)
-    old_verifier = ActiveSupport::MessageVerifier.new(old_raw_key, digest: "SHA1")
-    old_message = old_verifier.generate(
-      "message verified with old secret, salt, and metadata", purpose: "rotation")
-
-    verifier = ActiveSupport::MessageVerifier.new(@secret, digest: "SHA1")
-    verifier.rotate secret: old_secret, salt: old_salt
+    verifier = ActiveSupport::MessageVerifier.new(@secret)
+    verifier.rotate "old"
 
-    assert_equal "message verified with old secret, salt, and metadata",
-      verifier.verified(old_message, purpose: "rotation")
+    assert_equal "old", verifier.verified(old_message, purpose: :rotation)
   end
 end