Permalink
Browse files

Re-write legacy (marshal) cookies on read

  • Loading branch information...
1 parent a668bef commit ead947a3b2bc672b6064a6d0d33905d45299d22e @chancancode chancancode committed Feb 9, 2014
Showing with 57 additions and 35 deletions.
  1. +40 −20 actionpack/lib/action_dispatch/middleware/cookies.rb
  2. +17 −15 actionpack/test/dispatch/cookies_test.rb
@@ -384,29 +384,48 @@ def self.dump(value)
end
end
- class HybridSerializer < JsonSerializer
- MARSHAL_SIGNATURE = "\x04\x08".freeze
-
+ # Passing the NullSerializer downstream to the Message{Encryptor,Verifier}
+ # allows us to handle the (de)serialization step within the cookie jar,
+ # which gives us the opportunity to detect and migrate legacy cookies.
+ class NullSerializer
def self.load(value)
- if value.start_with?(MARSHAL_SIGNATURE)
- Marshal.load(value)
- else
- super
- end
+ value
+ end
+
+ def self.dump(value)
+ value
end
end
module SerializedCookieJars
+ MARSHAL_SIGNATURE = "\x04\x08".freeze
+
protected
+ def needs_migration?(value)
+ @options[:serializer] == :hybrid && value.start_with?(MARSHAL_SIGNATURE)
+ end
+
+ def serialize(name, value)
+ serializer.dump(value)
+ end
+
+ def deserialize(name, value)
+ if value
+ if needs_migration?(value)
+ self[name] = Marshal.load(value)
+ else
+ serializer.load(value)
+ end
+ end
+ end
+
def serializer
serializer = @options[:serializer] || :marshal
case serializer
when :marshal
Marshal
- when :json
+ when :json, :hybrid
JsonSerializer
- when :hybrid
- HybridSerializer
else
serializer
end
@@ -421,21 +440,21 @@ def initialize(parent_jar, key_generator, options = {})
@parent_jar = parent_jar
@options = options
secret = key_generator.generate_key(@options[:signed_cookie_salt])
- @verifier = ActiveSupport::MessageVerifier.new(secret, serializer: serializer)
+ @verifier = ActiveSupport::MessageVerifier.new(secret, serializer: NullSerializer)
end
def [](name)
if signed_message = @parent_jar[name]
- verify(signed_message)
+ deserialize name, verify(signed_message)
end
end
def []=(name, options)
if options.is_a?(Hash)
options.symbolize_keys!
- options[:value] = @verifier.generate(options[:value])
+ options[:value] = @verifier.generate(serialize(name, options[:value]))
else
- options = { :value => @verifier.generate(options) }
+ options = { :value => @verifier.generate(serialize(name, options)) }
end
raise CookieOverflow if options[:value].size > MAX_COOKIE_SIZE
@@ -459,7 +478,7 @@ class UpgradeLegacySignedCookieJar < SignedCookieJar #:nodoc:
def [](name)
if signed_message = @parent_jar[name]
- verify(signed_message) || verify_and_upgrade_legacy_signed_message(name, signed_message)
+ deserialize(name, verify(signed_message)) || verify_and_upgrade_legacy_signed_message(name, signed_message)
end
end
end
@@ -478,12 +497,12 @@ def initialize(parent_jar, key_generator, options = {})
@options = options
secret = key_generator.generate_key(@options[:encrypted_cookie_salt])
sign_secret = key_generator.generate_key(@options[:encrypted_signed_cookie_salt])
- @encryptor = ActiveSupport::MessageEncryptor.new(secret, sign_secret, serializer: serializer)
+ @encryptor = ActiveSupport::MessageEncryptor.new(secret, sign_secret, serializer: NullSerializer)
end
def [](name)
if encrypted_message = @parent_jar[name]
- decrypt_and_verify(encrypted_message)
+ deserialize name, decrypt_and_verify(encrypted_message)
end
end
@@ -493,7 +512,8 @@ def []=(name, options)
else
options = { :value => options }
end
- options[:value] = @encryptor.encrypt_and_sign(options[:value])
+
+ options[:value] = @encryptor.encrypt_and_sign(serialize(name, options[:value]))
raise CookieOverflow if options[:value].size > MAX_COOKIE_SIZE
@parent_jar[name] = options
@@ -516,7 +536,7 @@ class UpgradeLegacyEncryptedCookieJar < EncryptedCookieJar #:nodoc:
def [](name)
if encrypted_or_signed_message = @parent_jar[name]
- decrypt_and_verify(encrypted_or_signed_message) || verify_and_upgrade_legacy_signed_message(name, encrypted_or_signed_message)
+ deserialize(name, decrypt_and_verify(encrypted_or_signed_message)) || verify_and_upgrade_legacy_signed_message(name, encrypted_or_signed_message)
end
end
end
@@ -415,8 +415,8 @@ def test_signed_cookie_using_hybrid_serializer_can_migrate_marshal_dumped_value_
assert_not_equal 45, cookies[:user_id]
assert_equal 45, cookies.signed[:user_id]
- json_value = ActiveSupport::MessageVerifier.new(secret, serializer: JSON).generate(45)
- assert_equal @response.cookies['user_id'], json_value
+ verifier = ActiveSupport::MessageVerifier.new(secret, serializer: JSON)
+ assert_equal 45, verifier.verify(@response.cookies['user_id'])
end
def test_signed_cookie_using_hybrid_serializer_can_read_from_json_dumped_value
@@ -433,6 +433,8 @@ def test_signed_cookie_using_hybrid_serializer_can_read_from_json_dumped_value
cookies = @controller.send :cookies
assert_not_equal 45, cookies[:user_id]
assert_equal 45, cookies.signed[:user_id]
+
+ assert_nil @response.cookies["user_id"]
end
def test_accessing_nonexistant_signed_cookie_should_not_raise_an_invalid_signature
@@ -475,7 +477,7 @@ def test_encrypted_cookie_using_json_serializer
def test_encrypted_cookie_using_custom_serializer
@request.env["action_dispatch.cookies_serializer"] = CustomSerializer
get :set_encrypted_cookie
- assert_not_equal 45, cookies.encrypted[:foo]
+ assert_not_equal 'bar', cookies.encrypted[:foo]
assert_equal 'bar was dumped and loaded', cookies.encrypted[:foo]
end
@@ -488,17 +490,17 @@ def test_encrypted_cookie_using_hybrid_serializer_can_migrate_marshal_dumped_val
secret = key_generator.generate_key(encrypted_cookie_salt)
sign_secret = key_generator.generate_key(encrypted_signed_cookie_salt)
- marshal_value = ActiveSupport::MessageEncryptor.new(secret, sign_secret, serializer: Marshal).encrypt_and_sign(45)
- @request.headers["Cookie"] = "user_id=#{marshal_value}"
+ marshal_value = ActiveSupport::MessageEncryptor.new(secret, sign_secret, serializer: Marshal).encrypt_and_sign("bar")
+ @request.headers["Cookie"] = "foo=#{marshal_value}"
get :get_encrypted_cookie
cookies = @controller.send :cookies
- assert_not_equal 45, cookies[:user_id]
- assert_equal 45, cookies.encrypted[:user_id]
+ assert_not_equal "bar", cookies[:foo]
+ assert_equal "bar", cookies.encrypted[:foo]
- json_value = ActiveSupport::MessageEncryptor.new(secret, sign_secret, serializer: JSON).encrypt_and_sign(45)
- assert_equal @response.cookies["user_id"], json_value
+ encryptor = ActiveSupport::MessageEncryptor.new(secret, sign_secret, serializer: JSON)
+ assert_equal "bar", encryptor.decrypt_and_verify(@response.cookies["foo"])
end
def test_encrypted_cookie_using_hybrid_serializer_can_read_from_json_dumped_value
@@ -509,14 +511,16 @@ def test_encrypted_cookie_using_hybrid_serializer_can_read_from_json_dumped_valu
encrypted_signed_cookie_salt = @request.env["action_dispatch.encrypted_signed_cookie_salt"]
secret = key_generator.generate_key(encrypted_cookie_salt)
sign_secret = key_generator.generate_key(encrypted_signed_cookie_salt)
- json_value = ActiveSupport::MessageEncryptor.new(secret, sign_secret, serializer: JSON).encrypt_and_sign(45)
- @request.headers["Cookie"] = "user_id=#{json_value}"
+ json_value = ActiveSupport::MessageEncryptor.new(secret, sign_secret, serializer: JSON).encrypt_and_sign("bar")
+ @request.headers["Cookie"] = "foo=#{json_value}"
get :get_encrypted_cookie
cookies = @controller.send :cookies
- assert_not_equal 45, cookies[:user_id]
- assert_equal 45, cookies.encrypted[:user_id]
+ assert_not_equal "bar", cookies[:foo]
+ assert_equal "bar", cookies.encrypted[:foo]
+
+ assert_nil @response.cookies["foo"]
end
def test_accessing_nonexistant_encrypted_cookie_should_not_raise_invalid_message
@@ -834,8 +838,6 @@ def test_cookies_hash_is_indifferent_access
assert_equal "dhh", cookies['user_name']
end
-
-
def test_setting_request_cookies_is_indifferent_access
cookies.clear
cookies[:user_name] = "andrew"

0 comments on commit ead947a

Please sign in to comment.