Merge pull request #9978 from trevorturk/cookie-store-auto-upgrade

Cookie-base session store auto-upgrade

actionpack/CHANGELOG.md

@@ -1,5 +1,16 @@
 ## Rails 4.0.0 (unreleased) ##
 
+*   Automatically configure cookie-based sessions to be encrypted if
+    `secret_key_base` is set, falling back to signed if only `secret_token`
+    is set. Automatically upgrade existing signed cookie-based sessions from
+    Rails 3.x to be encrypted if both `secret_key_base` and `secret_token`
+    are set, or signed with the new key generator if only `secret_token` is
+    set. This leaves only the `config.session_store :cookie_store` option and
+    removes the two new options introduced in 4.0.0.beta1:
+    `encrypted_cookie_store` and `upgrade_signature_to_encryption_cookie_store`.
+
+    *Trevor Turk*
+
 *   Ensure consistent fallback to the default layout lookup for layouts set
     using symbols or procs that return `nil`.
 

actionpack/lib/action_dispatch.rb

@@ -84,8 +84,6 @@ module Http
   module Session
     autoload :AbstractStore,                           'action_dispatch/middleware/session/abstract_store'
     autoload :CookieStore,                             'action_dispatch/middleware/session/cookie_store'
-    autoload :EncryptedCookieStore,                    'action_dispatch/middleware/session/cookie_store'
-    autoload :UpgradeSignatureToEncryptionCookieStore, 'action_dispatch/middleware/session/cookie_store'
     autoload :MemCacheStore,                           'action_dispatch/middleware/session/mem_cache_store'
     autoload :CacheStore,                              'action_dispatch/middleware/session/cache_store'
   end

actionpack/lib/action_dispatch/middleware/cookies.rb

@@ -117,6 +117,9 @@ def permanent
       # the cookie again. This is useful for creating cookies with values that the user is not supposed to change. If a signed
       # cookie was tampered with by the user (or a 3rd party), nil will be returned.
       #
+      # If +config.secret_key_base+ and +config.secret_token+ (deprecated) are both set,
+      # legacy cookies signed with the old key generator will be transparently upgraded.
+      #
       # This jar requires that you set a suitable secret for the verification on your app's +config.secret_key_base+.
       #
       # Example:
@@ -126,23 +129,20 @@ def permanent
       #
       #   cookies.signed[:discount] # => 45
       def signed
-        @signed ||= begin
-          if @options[:upgrade_legacy_signed_cookie_jar]
+        @signed ||=
+          if @options[:upgrade_legacy_signed_cookies]
             UpgradeLegacySignedCookieJar.new(self, @key_generator, @options)
           else
             SignedCookieJar.new(self, @key_generator, @options)
           end
-        end
-      end
-
-      # Only needed for supporting the +UpgradeSignatureToEncryptionCookieStore+, users and plugin authors should not use this
-      def signed_using_old_secret #:nodoc:
-        @signed_using_old_secret ||= SignedCookieJar.new(self, ActiveSupport::DummyKeyGenerator.new(@options[:secret_token]), @options)
       end
 
       # Returns a jar that'll automatically encrypt cookie values before sending them to the client and will decrypt them for read.
       # If the cookie was tampered with by the user (or a 3rd party), nil will be returned.
       #
+      # If +config.secret_key_base+ and +config.secret_token+ (deprecated) are both set,
+      # legacy cookies signed with the old key generator will be transparently upgraded.
+      #
       # This jar requires that you set a suitable secret for the verification on your app's +config.secret_key_base+.
       #
       # Example:
@@ -152,7 +152,38 @@ def signed_using_old_secret #:nodoc:
       #
       #   cookies.encrypted[:discount] # => 45
       def encrypted
-        @encrypted ||= EncryptedCookieJar.new(self, @key_generator, @options)
+        @encrypted ||=
+          if @options[:upgrade_legacy_signed_cookies]
+            UpgradeLegacyEncryptedCookieJar.new(self, @key_generator, @options)
+          else
+            EncryptedCookieJar.new(self, @key_generator, @options)
+          end
+      end
+
+      # Returns the +signed+ or +encrypted jar, preferring +encrypted+ if +secret_key_base+ is set.
+      # Used by ActionDispatch::Session::CookieStore to avoid the need to introduce new cookie stores.
+      def signed_or_encrypted
+        @signed_or_encrypted ||=
+          if @options[:secret_key_base].present?
+            encrypted
+          else
+            signed
+          end
+      end
+    end
+
+    module VerifyAndUpgradeLegacySignedMessage
+      def initialize(*args)
+        super
+        @legacy_verifier = ActiveSupport::MessageVerifier.new(@options[:secret_token])
+      end
+
+      def verify_and_upgrade_legacy_signed_message(name, signed_message)
+        @legacy_verifier.verify(signed_message).tap do |value|
+          self[name] = value
+        end
+      rescue ActiveSupport::MessageVerifier::InvalidSignature
+        nil
       end
     end
 
@@ -179,7 +210,7 @@ def self.options_for_env(env) #:nodoc:
           encrypted_signed_cookie_salt: env[ENCRYPTED_SIGNED_COOKIE_SALT] || '',
           secret_token: env[SECRET_TOKEN],
           secret_key_base: env[SECRET_KEY_BASE],
-          upgrade_legacy_signed_cookie_jar: env[SECRET_TOKEN].present? && env[SECRET_KEY_BASE].present?
+          upgrade_legacy_signed_cookies: env[SECRET_TOKEN].present? && env[SECRET_KEY_BASE].present?
         }
       end
 
@@ -354,10 +385,8 @@ def initialize(parent_jar, key_generator, options = {})
 
       def [](name)
         if signed_message = @parent_jar[name]
-          @verifier.verify(signed_message)
+          verify(signed_message)
         end
-      rescue ActiveSupport::MessageVerifier::InvalidSignature
-        nil
       end
 
       def []=(key, options)
@@ -371,46 +400,37 @@ def []=(key, options)
         raise CookieOverflow if options[:value].size > MAX_COOKIE_SIZE
         @parent_jar[key] = options
       end
+
+      private
+
+        def verify(signed_message)
+          @verifier.verify(signed_message)
+        rescue ActiveSupport::MessageVerifier::InvalidSignature
+          nil
+        end
     end
 
     # UpgradeLegacySignedCookieJar is used instead of SignedCookieJar if
     # config.secret_token and config.secret_key_base are both set. It reads
     # legacy cookies signed with the old dummy key generator and re-saves
     # them using the new key generator to provide a smooth upgrade path.
     class UpgradeLegacySignedCookieJar < SignedCookieJar #:nodoc:
-      def initialize(*args)
-        super
-        @legacy_verifier = ActiveSupport::MessageVerifier.new(@options[:secret_token])
-      end
+      include VerifyAndUpgradeLegacySignedMessage
 
       def [](name)
         if signed_message = @parent_jar[name]
-          verify_signed_message(signed_message) || verify_and_upgrade_legacy_signed_message(name, signed_message)
+          verify(signed_message) || verify_and_upgrade_legacy_signed_message(name, signed_message)
         end
       end
-
-      def verify_signed_message(signed_message)
-        @verifier.verify(signed_message)
-      rescue ActiveSupport::MessageVerifier::InvalidSignature
-        nil
-      end
-
-      def verify_and_upgrade_legacy_signed_message(name, signed_message)
-        @legacy_verifier.verify(signed_message).tap do |value|
-          self[name] = value
-        end
-      rescue ActiveSupport::MessageVerifier::InvalidSignature
-        nil
-      end
     end
 
     class EncryptedCookieJar #:nodoc:
       include ChainedCookieJars
 
       def initialize(parent_jar, key_generator, options = {})
         if ActiveSupport::DummyKeyGenerator === key_generator
-          raise "Encrypted Cookies must be used in conjunction with config.secret_key_base." +
-                "Set config.secret_key_base in config/initializers/secret_token.rb"
+          raise "You didn't set config.secret_key_base, which is required for this cookie jar. " +
+            "Read the upgrade documentation to learn more about this new config option."
         end
 
         @parent_jar = parent_jar
@@ -422,11 +442,8 @@ def initialize(parent_jar, key_generator, options = {})
 
       def [](key)
         if encrypted_message = @parent_jar[key]
-          @encryptor.decrypt_and_verify(encrypted_message)
+          decrypt_and_verify(encrypted_message)
         end
-      rescue ActiveSupport::MessageVerifier::InvalidSignature,
-             ActiveSupport::MessageEncryptor::InvalidMessage
-        nil
       end
 
       def []=(key, options)
@@ -440,6 +457,28 @@ def []=(key, options)
         raise CookieOverflow if options[:value].size > MAX_COOKIE_SIZE
         @parent_jar[key] = options
       end
+
+      private
+
+        def decrypt_and_verify(encrypted_message)
+          @encryptor.decrypt_and_verify(encrypted_message)
+        rescue ActiveSupport::MessageVerifier::InvalidSignature, ActiveSupport::MessageEncryptor::InvalidMessage
+          nil
+        end
+    end
+
+    # UpgradeLegacyEncryptedCookieJar is used by ActionDispatch::Session::CookieStore
+    # instead of EncryptedCookieJar if config.secret_token and config.secret_key_base
+    # are both set. It reads legacy cookies signed with the old dummy key generator and
+    # encrypts and re-saves them using the new key generator to provide a smooth upgrade path.
+    class UpgradeLegacyEncryptedCookieJar < EncryptedCookieJar #:nodoc:
+      include VerifyAndUpgradeLegacySignedMessage
+
+      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)
+        end
+      end
     end
 
     def initialize(app)

actionpack/lib/action_dispatch/middleware/session/cookie_store.rb

@@ -100,42 +100,7 @@ def get_cookie(env)
 
       def cookie_jar(env)
         request = ActionDispatch::Request.new(env)
-        request.cookie_jar.signed
-      end
-    end
-
-    class EncryptedCookieStore < CookieStore
-
-      private
-
-      def cookie_jar(env)
-        request = ActionDispatch::Request.new(env)
-        request.cookie_jar.encrypted
-      end
-    end
-
-    # This cookie store helps you upgrading apps that use +CookieStore+ to the new default +EncryptedCookieStore+
-    # To use this CookieStore set
-    #
-    # Myapp::Application.config.session_store :upgrade_signature_to_encryption_cookie_store, key: '_myapp_session'
-    #
-    # in your config/initializers/session_store.rb
-    #
-    # You will also need to add
-    #
-    # Myapp::Application.config.secret_key_base = 'some secret'
-    #
-    # in your config/initializers/secret_token.rb, but do not remove +Myapp::Application.config.secret_token = 'some secret'+
-    class UpgradeSignatureToEncryptionCookieStore < EncryptedCookieStore
-      private
-
-      def get_cookie(env)
-        signed_using_old_secret_cookie_jar(env)[@key] || cookie_jar(env)[@key]
-      end
-
-      def signed_using_old_secret_cookie_jar(env)
-        request = ActionDispatch::Request.new(env)
-        request.cookie_jar.signed_using_old_secret
+        request.cookie_jar.signed_or_encrypted
       end
     end
   end

actionpack/test/dispatch/cookies_test.rb

@@ -86,6 +86,11 @@ def set_encrypted_cookie
       head :ok
     end
 
+    def get_encrypted_cookie
+      cookies.encrypted[:foo]
+      head :ok
+    end
+
     def set_invalid_encrypted_cookie
       cookies[:invalid_cookie] = 'invalid--9170e00a57cfc27083363b5c75b835e477bd90cf'
       head :ok
@@ -456,7 +461,42 @@ def test_signed_uses_upgrade_legacy_signed_cookie_jar_if_both_secret_token_and_s
     assert_kind_of ActionDispatch::Cookies::UpgradeLegacySignedCookieJar, cookies.signed
   end
 
-  def test_legacy_signed_cookie_is_read_and_transparently_upgraded_if_both_secret_token_and_secret_key_base_are_set
+  def test_signed_or_encrypted_uses_signed_cookie_jar_if_only_secret_token_is_set
+    @request.env["action_dispatch.secret_token"] = "b3c631c314c0bbca50c1b2843150fe33"
+    @request.env["action_dispatch.secret_key_base"] = nil
+    get :get_encrypted_cookie
+    assert_kind_of ActionDispatch::Cookies::SignedCookieJar, cookies.signed_or_encrypted
+  end
+
+  def test_signed_or_encrypted_uses_encrypted_cookie_jar_if_only_secret_key_base_is_set
+    @request.env["action_dispatch.secret_token"] = nil
+    @request.env["action_dispatch.secret_key_base"] = "c3b95688f35581fad38df788add315ff"
+    get :get_encrypted_cookie
+    assert_kind_of ActionDispatch::Cookies::EncryptedCookieJar, cookies.signed_or_encrypted
+  end
+
+  def test_signed_or_encrypted_uses_upgrade_legacy_encrypted_cookie_jar_if_both_secret_token_and_secret_key_base_are_set
+    @request.env["action_dispatch.secret_token"] = "b3c631c314c0bbca50c1b2843150fe33"
+    @request.env["action_dispatch.secret_key_base"] = "c3b95688f35581fad38df788add315ff"
+    get :get_encrypted_cookie
+    assert_kind_of ActionDispatch::Cookies::UpgradeLegacyEncryptedCookieJar, cookies.signed_or_encrypted
+  end
+
+  def test_encrypted_uses_encrypted_cookie_jar_if_only_secret_key_base_is_set
+    @request.env["action_dispatch.secret_token"] = nil
+    @request.env["action_dispatch.secret_key_base"] = "c3b95688f35581fad38df788add315ff"
+    get :get_encrypted_cookie
+    assert_kind_of ActionDispatch::Cookies::EncryptedCookieJar, cookies.encrypted
+  end
+
+  def test_encrypted_uses_upgrade_legacy_encrypted_cookie_jar_if_both_secret_token_and_secret_key_base_are_set
+    @request.env["action_dispatch.secret_token"] = "b3c631c314c0bbca50c1b2843150fe33"
+    @request.env["action_dispatch.secret_key_base"] = "c3b95688f35581fad38df788add315ff"
+    get :get_encrypted_cookie
+    assert_kind_of ActionDispatch::Cookies::UpgradeLegacyEncryptedCookieJar, cookies.encrypted
+  end
+
+  def test_legacy_signed_cookie_is_read_and_transparently_upgraded_by_signed_cookie_jar_if_both_secret_token_and_secret_key_base_are_set
     @request.env["action_dispatch.secret_token"] = "b3c631c314c0bbca50c1b2843150fe33"
     @request.env["action_dispatch.secret_key_base"] = "c3b95688f35581fad38df788add315ff"
 
@@ -473,7 +513,27 @@ def test_legacy_signed_cookie_is_read_and_transparently_upgraded_if_both_secret_
     assert_equal 45, verifier.verify(@response.cookies["user_id"])
   end
 
-  def test_legacy_signed_cookie_is_nil_if_tampered
+  def test_legacy_signed_cookie_is_read_and_transparently_encrypted_by_encrypted_cookie_jar_if_both_secret_token_and_secret_key_base_are_set
+    @request.env["action_dispatch.secret_token"] = "b3c631c314c0bbca50c1b2843150fe33"
+    @request.env["action_dispatch.secret_key_base"] = "c3b95688f35581fad38df788add315ff"
+    @request.env["action_dispatch.encrypted_cookie_salt"] = "4433796b79d99a7735553e316522acee"
+    @request.env["action_dispatch.encrypted_signed_cookie_salt"] = "00646eb40062e1b1deff205a27cd30f9"
+
+    legacy_value = ActiveSupport::MessageVerifier.new("b3c631c314c0bbca50c1b2843150fe33").generate('bar')
+
+    @request.headers["Cookie"] = "foo=#{legacy_value}"
+    get :get_encrypted_cookie
+
+    assert_equal 'bar', @controller.send(:cookies).encrypted[:foo]
+
+    key_generator = @request.env["action_dispatch.key_generator"]
+    secret = key_generator.generate_key(@request.env["action_dispatch.encrypted_cookie_salt"])
+    sign_secret = key_generator.generate_key(@request.env["action_dispatch.encrypted_signed_cookie_salt"])
+    encryptor = ActiveSupport::MessageEncryptor.new(secret, sign_secret)
+    assert_equal 'bar', encryptor.decrypt_and_verify(@response.cookies["foo"])
+  end
+
+  def test_legacy_signed_cookie_is_treated_as_nil_by_signed_cookie_jar_if_tampered
     @request.env["action_dispatch.secret_token"] = "b3c631c314c0bbca50c1b2843150fe33"
     @request.env["action_dispatch.secret_key_base"] = "c3b95688f35581fad38df788add315ff"
 
@@ -484,6 +544,17 @@ def test_legacy_signed_cookie_is_nil_if_tampered
     assert_equal nil, @response.cookies["user_id"]
   end
 
+  def test_legacy_signed_cookie_is_treated_as_nil_by_encrypted_cookie_jar_if_tampered
+    @request.env["action_dispatch.secret_token"] = "b3c631c314c0bbca50c1b2843150fe33"
+    @request.env["action_dispatch.secret_key_base"] = "c3b95688f35581fad38df788add315ff"
+
+    @request.headers["Cookie"] = "foo=baz"
+    get :get_encrypted_cookie
+
+    assert_equal nil, @controller.send(:cookies).encrypted[:foo]
+    assert_equal nil, @response.cookies["foo"]
+  end
+
   def test_cookie_with_all_domain_option
     get :set_cookie_with_domain
     assert_response :success

guides/code/getting_started/config/initializers/session_store.rb

@@ -1,3 +1,3 @@
 # Be sure to restart your server when you modify this file.
 
-Blog::Application.config.session_store :encrypted_cookie_store, key: '_blog_session'
+Blog::Application.config.session_store :cookie_store, key: '_blog_session'