Merge pull request #3037 from wvanbergen/master

MessageVerifier: set serializer and crypto algorithm using an option hash

4 files changed, 40 insertions, 22 deletions

diff --git a/activesupport/lib/active_support/message_encryptor.rb b/activesupport/lib/active_support/message_encryptor.rb
index 66e121be26..7637cfe2b0 100644
--- a/activesupport/lib/active_support/message_encryptor.rb
+++ b/activesupport/lib/active_support/message_encryptor.rb
@@ -13,12 +13,15 @@ module ActiveSupport
     class InvalidMessage < StandardError; end
     OpenSSLCipherError = OpenSSL::Cipher.const_defined?(:CipherError) ? OpenSSL::Cipher::CipherError : OpenSSL::CipherError
 
-    attr_accessor :serializer
-
-    def initialize(secret, cipher = 'aes-256-cbc', serializer = Marshal)
+    def initialize(secret, options = {})
+      unless options.is_a?(Hash)
+        ActiveSupport::Deprecation.warn "The second parameter should be an options hash. Use :cipher => 'algorithm' to sepcify the cipher algorithm."
+        options = { :cipher => options }
+      end
+      
       @secret = secret
-      @cipher = cipher
-      @serializer = serializer
+      @cipher = options[:cipher] || 'aes-256-cbc'
+      @serializer = options[:serializer] || Marshal
     end
 
     def encrypt(value)
@@ -30,7 +33,7 @@ module ActiveSupport
       cipher.key = @secret
       cipher.iv  = iv
 
-      encrypted_data = cipher.update(serializer.dump(value))
+      encrypted_data = cipher.update(@serializer.dump(value))
       encrypted_data << cipher.final
 
       [encrypted_data, iv].map {|v| ActiveSupport::Base64.encode64s(v)}.join("--")
@@ -47,7 +50,7 @@ module ActiveSupport
       decrypted_data = cipher.update(encrypted_data)
       decrypted_data << cipher.final
 
-      serializer.load(decrypted_data)
+      @serializer.load(decrypted_data)
     rescue OpenSSLCipherError, TypeError
       raise InvalidMessage
     end
diff --git a/activesupport/lib/active_support/message_verifier.rb b/activesupport/lib/active_support/message_verifier.rb
index 0181070479..57317028fc 100644
--- a/activesupport/lib/active_support/message_verifier.rb
+++ b/activesupport/lib/active_support/message_verifier.rb
@@ -26,12 +26,15 @@ module ActiveSupport
   class MessageVerifier
     class InvalidSignature < StandardError; end
 
-    attr_accessor :serializer
-    
-    def initialize(secret, digest = 'SHA1', serializer = Marshal)
+    def initialize(secret, options = {})
+      unless options.is_a?(Hash)
+        ActiveSupport::Deprecation.warn "The second parameter should be an options hash. Use :digest => 'algorithm' to sepcify the digest algorithm."
+        options = { :digest => options }
+      end
+
       @secret = secret
-      @digest = digest
-      @serializer = serializer
+      @digest = options[:digest] || 'SHA1'
+      @serializer = options[:serializer] || Marshal
     end
 
     def verify(signed_message)
@@ -39,14 +42,14 @@ module ActiveSupport
 
       data, digest = signed_message.split("--")
       if data.present? && digest.present? && secure_compare(digest, generate_digest(data))
-        serializer.load(ActiveSupport::Base64.decode64(data))
+        @serializer.load(ActiveSupport::Base64.decode64(data))
       else
         raise InvalidSignature
       end
     end
 
     def generate(value)
-      data = ActiveSupport::Base64.encode64s(serializer.dump(value))
+      data = ActiveSupport::Base64.encode64s(@serializer.dump(value))
       "#{data}--#{generate_digest(data)}"
     end
 
diff --git a/activesupport/test/message_encryptor_test.rb b/activesupport/test/message_encryptor_test.rb
index 9c89512b42..83a19f8106 100644
--- a/activesupport/test/message_encryptor_test.rb
+++ b/activesupport/test/message_encryptor_test.rb
@@ -10,7 +10,7 @@ else
 require 'active_support/time'
 require 'active_support/json'
 
-class MessageEncryptorTest < Test::Unit::TestCase
+class MessageEncryptorTest < ActiveSupport::TestCase
   
   class JSONSerializer
     def dump(value)
@@ -52,11 +52,17 @@ class MessageEncryptorTest < Test::Unit::TestCase
   end
   
   def test_alternative_serialization_method
-    @encryptor.serializer = JSONSerializer.new
-    message = @encryptor.encrypt_and_sign({ :foo => 123, 'bar' => Time.utc(2010) })
-    assert_equal @encryptor.decrypt_and_verify(message), { "foo" => 123, "bar" => "2010-01-01T00:00:00Z" }
+    encryptor = ActiveSupport::MessageEncryptor.new(SecureRandom.hex(64), :serializer => JSONSerializer.new)
+    message = encryptor.encrypt_and_sign({ :foo => 123, 'bar' => Time.utc(2010) })
+    assert_equal encryptor.decrypt_and_verify(message), { "foo" => 123, "bar" => "2010-01-01T00:00:00Z" }
   end
 
+  def test_digest_algorithm_as_second_parameter_deprecation
+    assert_deprecated(/options hash/) do
+      ActiveSupport::MessageEncryptor.new(SecureRandom.hex(64), 'aes-256-cbc')
+    end
+  end
+  
   private
     def assert_not_decrypted(value)
       assert_raise(ActiveSupport::MessageEncryptor::InvalidMessage) do
diff --git a/activesupport/test/message_verifier_test.rb b/activesupport/test/message_verifier_test.rb
index 10a1738786..35747abe5b 100644
--- a/activesupport/test/message_verifier_test.rb
+++ b/activesupport/test/message_verifier_test.rb
@@ -10,7 +10,7 @@ else
 require 'active_support/time'
 require 'active_support/json'
 
-class MessageVerifierTest < Test::Unit::TestCase
+class MessageVerifierTest < ActiveSupport::TestCase
   
   class JSONSerializer
     def dump(value)
@@ -45,9 +45,15 @@ class MessageVerifierTest < Test::Unit::TestCase
   end
   
   def test_alternative_serialization_method
-    @verifier.serializer = JSONSerializer.new
-    message = @verifier.generate({ :foo => 123, 'bar' => Time.utc(2010) })
-    assert_equal @verifier.verify(message), { "foo" => 123, "bar" => "2010-01-01T00:00:00Z" }
+    verifier = ActiveSupport::MessageVerifier.new("Hey, I'm a secret!", :serializer => JSONSerializer.new)
+    message = verifier.generate({ :foo => 123, 'bar' => Time.utc(2010) })
+    assert_equal verifier.verify(message), { "foo" => 123, "bar" => "2010-01-01T00:00:00Z" }
+  end
+  
+  def test_digest_algorithm_as_second_parameter_deprecation
+    assert_deprecated(/options hash/) do
+      ActiveSupport::MessageVerifier.new("secret", "SHA1")
+    end
   end
 
   def assert_not_verified(message)