kotovalexarian-likes-github
/
rails--rails
mirror of https://github.com/rails/rails.git
1
0
Fork 0
Code Releases Activity

Simplify cookie_store by simply relying on cookies.signed.

This commit is contained in:
José Valim 2010-05-18 01:43:06 +02:00
parent 941b653627
commit 25f7c030e4
9 changed files with 138 additions and 151 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

32
actionpack/lib/action_dispatch/middleware/cookies.rb
View File

@ -52,6 +52,9 @@ module ActionDispatch
# * <tt>:httponly</tt> - Whether this cookie is accessible via scripting or
# only HTTP. Defaults to +false+.
class Cookies
# Raised when storing more than 4K of session data.
class CookieOverflow < StandardError; end
class CookieJar < Hash #:nodoc:
def self.build(request)
secret = request.env["action_dispatch.secret_token"]
@ -166,8 +169,11 @@ module ActionDispatch
end
class SignedCookieJar < CookieJar #:nodoc:
MAX_COOKIE_SIZE = 4096 # Cookies can typically store 4096 bytes.
SECRET_MIN_LENGTH = 30 # Characters
def initialize(parent_jar, secret)
raise "You must set config.secret_token in your app's config" if secret.blank?
ensure_secret_secure(secret)
@parent_jar = parent_jar
@verifier = ActiveSupport::MessageVerifier.new(secret)
end
@ -176,6 +182,8 @@ module ActionDispatch
if signed_message = @parent_jar[name]
@verifier.verify(signed_message)
end
rescue ActiveSupport::MessageVerifier::InvalidSignature
nil
end
def []=(key, options)
@ -186,12 +194,34 @@ module ActionDispatch
options = { :value => @verifier.generate(options) }
end
raise CookieOverflow if options[:value].size > MAX_COOKIE_SIZE
@parent_jar[key] = options
end
def method_missing(method, *arguments, &block)
@parent_jar.send(method, *arguments, &block)
end
protected
# To prevent users from using something insecure like "Password" we make sure that the
# secret they've provided is at least 30 characters in length.
def ensure_secret_secure(secret)
if secret.blank?
raise ArgumentError, "A secret is required to generate an " +
"integrity hash for cookie session data. Use " +
"config.secret_token = \"some secret phrase of at " +
"least #{SECRET_MIN_LENGTH} characters\"" +
"in config/application.rb"
end
if secret.length < SECRET_MIN_LENGTH
raise ArgumentError, "Secret should be something secure, " +
"like \"#{ActiveSupport::SecureRandom.hex(16)}\". The value you " +
"provided, \"#{secret}\", is shorter than the minimum length " +
"of #{SECRET_MIN_LENGTH} characters"
end
end
end
def initialize(app)

107
actionpack/lib/action_dispatch/middleware/session/cookie_store.rb
View File

@ -1,3 +1,4 @@
require 'action_dispatch/middleware/cookies'
require 'active_support/core_ext/hash/keys'
require 'active_support/core_ext/object/blank'
@ -39,10 +40,6 @@ module ActionDispatch
#
# Note that changing digest or secret invalidates all existing sessions!
class CookieStore
# Cookies can typically store 4096 bytes.
MAX = 4096
SECRET_MIN_LENGTH = 30 # characters
DEFAULT_OPTIONS = {
:key => '_session_id',
:domain => nil,
@ -54,7 +51,7 @@ module ActionDispatch
class OptionsHash < Hash
def initialize(by, env, default_options)
@session_data = env[CookieStore::ENV_SESSION_KEY]
default_options.each { |key, value| self[key] = value }
merge!(default_options)
end
def [](key)
@ -64,14 +61,12 @@ module ActionDispatch
ENV_SESSION_KEY = "rack.session".freeze
ENV_SESSION_OPTIONS_KEY = "rack.session.options".freeze
HTTP_SET_COOKIE = "Set-Cookie".freeze
# Raised when storing more than 4K of session data.
class CookieOverflow < StandardError; end
def initialize(app, options = {})
# Process legacy CGI options
# TODO Refactor and deprecate me
options = options.symbolize_keys
if options.has_key?(:session_path)
options[:path] = options.delete(:session_path)
end
@ -88,15 +83,7 @@ module ActionDispatch
ensure_session_key(options[:key])
@key = options.delete(:key).freeze
# The secret option is required.
ensure_secret_secure(options[:secret])
@secret = options.delete(:secret).freeze
@digest = options.delete(:digest) || 'SHA1'
@verifier = verifier_for(@secret, @digest)
@default_options = DEFAULT_OPTIONS.merge(options).freeze
freeze
end
@ -111,66 +98,32 @@ module ActionDispatch
if !session_data.is_a?(AbstractStore::SessionHash) || session_data.send(:loaded?) || options[:expire_after]
session_data.send(:load!) if session_data.is_a?(AbstractStore::SessionHash) && !session_data.send(:loaded?)
session_data = marshal(session_data.to_hash)
session_data = persistent_session_id!(session_data.to_hash)
raise CookieOverflow if session_data.size > MAX
cookie = Hash.new
cookie[:value] = session_data
cookie = { :value => session_data }
unless options[:expire_after].nil?
cookie[:expires] = Time.now + options[:expire_after]
cookie[:expires] = Time.now + options.delete(:expire_after)
end
cookie = build_cookie(@key, cookie.merge(options))
unless headers[HTTP_SET_COOKIE].blank?
headers[HTTP_SET_COOKIE] << "\n#{cookie}"
else
headers[HTTP_SET_COOKIE] = cookie
end
request = ActionDispatch::Request.new(env)
request.cookie_jar.signed[@key] = cookie.merge!(options)
end
[status, headers, body]
end
private
# Should be in Rack::Utils soon
def build_cookie(key, value)
case value
when Hash
domain = "; domain=" + value[:domain] if value[:domain]
path = "; path=" + value[:path] if value[:path]
# According to RFC 2109, we need dashes here.
# N.B.: cgi.rb uses spaces...
expires = "; expires=" + value[:expires].clone.gmtime.
strftime("%a, %d-%b-%Y %H:%M:%S GMT") if value[:expires]
secure = "; secure" if value[:secure]
httponly = "; HttpOnly" if value[:httponly]
value = value[:value]
end
value = [value] unless Array === value
cookie = Rack::Utils.escape(key) + "=" +
value.map { |v| Rack::Utils.escape(v) }.join("&") +
"#{domain}#{path}#{expires}#{secure}#{httponly}"
end
def load_session(env)
request = Rack::Request.new(env)
session_data = request.cookies[@key]
data = unmarshal(session_data) || persistent_session_id!({})
request = ActionDispatch::Request.new(env)
data = request.cookie_jar.signed[@key]
data = persistent_session_id!(data || {})
data.stringify_keys!
[data["session_id"], data]
end
# Marshal a session hash into safe cookie data. Include an integrity hash.
def marshal(session)
@verifier.generate(persistent_session_id!(session))
end
# Unmarshal cookie data to a hash and verify its integrity.
def unmarshal(cookie)
persistent_session_id!(@verifier.verify(cookie)) if cookie
rescue ActiveSupport::MessageVerifier::InvalidSignature
nil
def generate_sid
ActiveSupport::SecureRandom.hex(16)
end
def ensure_session_key(key)
@ -182,38 +135,6 @@ module ActionDispatch
end
end
# To prevent users from using something insecure like "Password" we make sure that the
# secret they've provided is at least 30 characters in length.
def ensure_secret_secure(secret)
# There's no way we can do this check if they've provided a proc for the
# secret.
return true if secret.is_a?(Proc)
if secret.blank?
raise ArgumentError, "A secret is required to generate an " +
"integrity hash for cookie session data. Use " +
"config.secret_token = \"some secret phrase of at " +
"least #{SECRET_MIN_LENGTH} characters\"" +
"in config/application.rb"
end
if secret.length < SECRET_MIN_LENGTH
raise ArgumentError, "Secret should be something secure, " +
"like \"#{ActiveSupport::SecureRandom.hex(16)}\". The value you " +
"provided, \"#{secret}\", is shorter than the minimum length " +
"of #{SECRET_MIN_LENGTH} characters"
end
end
def verifier_for(secret, digest)
key = secret.respond_to?(:call) ? secret.call : secret
ActiveSupport::MessageVerifier.new(key, digest)
end
def generate_sid
ActiveSupport::SecureRandom.hex(16)
end
def persistent_session_id!(data)
(data ||= {}).merge!(inject_persistent_session_id(data))
end

1
actionpack/test/abstract_unit.rb
View File

@ -162,6 +162,7 @@ class ActionController::IntegrationTest < ActiveSupport::TestCase
middleware.use "ActionDispatch::Cookies"
middleware.use "ActionDispatch::Flash"
middleware.use "ActionDispatch::Head"
yield(middleware) if block_given?
end
end

55
actionpack/test/controller/cookie_test.rb
View File

@ -58,6 +58,17 @@ class CookieTest < ActionController::TestCase
head :ok
end
def raise_data_overflow
cookies.signed[:foo] = 'bye!' * 1024
head :ok
end
def tampered_cookies
cookies[:tampered] = "BAh7BjoIZm9vIghiYXI%3D--123456780"
cookies.signed[:tampered]
head :ok
end
def set_permanent_signed_cookie
cookies.permanent.signed[:remember_me] = 100
head :ok
@ -74,7 +85,7 @@ class CookieTest < ActionController::TestCase
def setup
super
@request.env["action_dispatch.secret_token"] = "thisISverySECRET123"
@request.env["action_dispatch.secret_token"] = "b3c631c314c0bbca50c1b2843150fe33"
@request.host = "www.nextangle.com"
end
@ -163,6 +174,48 @@ class CookieTest < ActionController::TestCase
assert_equal({"user_name" => "david"}, @response.cookies)
end
def test_raise_data_overflow
assert_raise(ActionDispatch::Cookies::CookieOverflow) do
get :raise_data_overflow
end
end
def test_tampered_cookies
assert_nothing_raised do
get :tampered_cookies
assert_response :success
end
end
def test_raises_argument_error_if_missing_secret
assert_raise(ArgumentError, nil.inspect) {
@request.env["action_dispatch.secret_token"] = nil
get :set_signed_cookie
}
assert_raise(ArgumentError, ''.inspect) {
@request.env["action_dispatch.secret_token"] = ""
get :set_signed_cookie
}
end
def test_raises_argument_error_if_secret_is_probably_insecure
assert_raise(ArgumentError, "password".inspect) {
@request.env["action_dispatch.secret_token"] = "password"
get :set_signed_cookie
}
assert_raise(ArgumentError, "secret".inspect) {
@request.env["action_dispatch.secret_token"] = "secret"
get :set_signed_cookie
}
assert_raise(ArgumentError, "12345678901234567890123456789".inspect) {
@request.env["action_dispatch.secret_token"] = "12345678901234567890123456789"
get :set_signed_cookie
}
end
private
def assert_cookie_header(expected)
header = @response.headers["Set-Cookie"]

11
actionpack/test/controller/flash_test.rb
View File

@ -237,10 +237,19 @@ class FlashIntegrationTest < ActionController::IntegrationTest
end
private
# Overwrite get to send SessionSecret in env hash
def get(path, parameters = nil, env = {})
env["action_dispatch.secret_token"] ||= SessionSecret
super
end
def with_test_route_set
with_routing do |set|
set.draw do |map|
match ':action', :to => ActionDispatch::Session::CookieStore.new(FlashIntegrationTest::TestController, :key => FlashIntegrationTest::SessionKey, :secret => FlashIntegrationTest::SessionSecret)
match ':action', :to => ActionDispatch::Session::CookieStore.new(
FlashIntegrationTest::TestController, :key => SessionKey, :secret => SessionSecret
)
end
yield
end

66
actionpack/test/dispatch/session/cookie_store_test.rb
View File

@ -55,42 +55,13 @@ class CookieStoreTest < ActionController::IntegrationTest
}
end
def test_raises_argument_error_if_missing_secret
assert_raise(ArgumentError, nil.inspect) {
ActionDispatch::Session::CookieStore.new(nil,
:key => SessionKey, :secret => nil)
}
assert_raise(ArgumentError, ''.inspect) {
ActionDispatch::Session::CookieStore.new(nil,
:key => SessionKey, :secret => '')
}
end
def test_raises_argument_error_if_secret_is_probably_insecure
assert_raise(ArgumentError, "password".inspect) {
ActionDispatch::Session::CookieStore.new(nil,
:key => SessionKey, :secret => "password")
}
assert_raise(ArgumentError, "secret".inspect) {
ActionDispatch::Session::CookieStore.new(nil,
:key => SessionKey, :secret => "secret")
}
assert_raise(ArgumentError, "12345678901234567890123456789".inspect) {
ActionDispatch::Session::CookieStore.new(nil,
:key => SessionKey, :secret => "12345678901234567890123456789")
}
end
def test_setting_session_value
with_test_route_set do
get '/set_session_value'
assert_response :success
assert_equal "_myapp_session=#{response.body}; path=/; HttpOnly",
headers['Set-Cookie']
end
end
end
def test_getting_session_value
@ -99,7 +70,7 @@ class CookieStoreTest < ActionController::IntegrationTest
get '/get_session_value'
assert_response :success
assert_equal 'foo: "bar"', response.body
end
end
end
def test_getting_session_id
@ -127,7 +98,7 @@ class CookieStoreTest < ActionController::IntegrationTest
def test_close_raises_when_data_overflows
with_test_route_set do
assert_raise(ActionDispatch::Session::CookieStore::CookieOverflow) {
assert_raise(ActionDispatch::Cookies::CookieOverflow) {
get '/raise_data_overflow'
}
end
@ -209,30 +180,33 @@ class CookieStoreTest < ActionController::IntegrationTest
get '/no_session_access'
assert_response :success
# Mystery bug that came up in 2.3 as well. What is this trying to test?!
# assert_equal "_myapp_session=#{cookie_body}; path=/; expires=#{expected_expiry}; HttpOnly",
# headers['Set-Cookie']
assert_equal "_myapp_session=#{cookie_body}; path=/; expires=#{expected_expiry}; HttpOnly",
headers['Set-Cookie']
end
end
private
# Overwrite get to send SessionSecret in env hash
def get(path, parameters = nil, env = {})
env["action_dispatch.secret_token"] ||= SessionSecret
super
end
def with_test_route_set(options = {})
with_routing do |set|
set.draw do |map|
match ':action', :to => ::CookieStoreTest::TestController
end
options = {:key => SessionKey, :secret => SessionSecret}.merge(options)
@app = ActionDispatch::Session::CookieStore.new(set, options)
options = { :key => SessionKey, :secret => SessionSecret }.merge!(options)
@app = self.class.build_app(set) do |middleware|
middleware.use ActionDispatch::Session::CookieStore, options
middleware.delete "ActionDispatch::ShowExceptions"
end
yield
end
end
def unmarshal_session(cookie_string)
session = Rack::Utils.parse_query(cookie_string, ';,').inject({}) {|h,(k,v)|
h[k] = Array === v ? v.first : v
h
}[SessionKey]
verifier = ActiveSupport::MessageVerifier.new(SessionSecret, 'SHA1')
verifier.verify(session)
end
end

8
railties/lib/rails/application/configuration.rb
View File

@ -11,7 +11,8 @@ module Rails
:encoding, :consider_all_requests_local, :dependency_loading,
:filter_parameters, :log_level, :logger, :metals,
:plugins, :preload_frameworks, :reload_engines, :reload_plugins,
:secret_token, :serve_static_assets, :time_zone, :whiny_nils
:secret_token, :serve_static_assets, :session_options,
:time_zone, :whiny_nils
def initialize(*)
super
@ -138,11 +139,6 @@ module Rails
end
end
def session_options
return @session_options unless @session_store == :cookie_store
@session_options.merge(:secret => @secret_token)
end
protected
def default_middleware_stack

4
railties/test/application/configuration_test.rb
View File

@ -196,7 +196,7 @@ module ApplicationTests
test "config.secret_token is sent in env" do
make_basic_app do |app|
app.config.secret_token = 'ThisIsASECRET123'
app.config.secret_token = 'b3c631c314c0bbca50c1b2843150fe33'
app.config.session_store :disabled
end
@ -208,7 +208,7 @@ module ApplicationTests
end
get "/"
assert_equal 'ThisIsASECRET123', last_response.body
assert_equal 'b3c631c314c0bbca50c1b2843150fe33', last_response.body
end
test "protect from forgery is the default in a new app" do

5
railties/test/application/middleware_test.rb
View File

@ -175,7 +175,10 @@ module ApplicationTests
def remote_ip(env = {})
remote_ip = nil
env = Rack::MockRequest.env_for("/").merge(env).merge('action_dispatch.show_exceptions' => false)
env = Rack::MockRequest.env_for("/").merge(env).merge!(
'action_dispatch.show_exceptions' => false,
'action_dispatch.secret_token' => 'b3c631c314c0bbca50c1b2843150fe33'
)
endpoint = Proc.new do |e|
remote_ip = ActionDispatch::Request.new(e).remote_ip