diff --git a/lib/auth/default_current_user_provider.rb b/lib/auth/default_current_user_provider.rb index 84ef64dc9a5..066f55d2c1c 100644 --- a/lib/auth/default_current_user_provider.rb +++ b/lib/auth/default_current_user_provider.rb @@ -1,4 +1,5 @@ require_dependency "auth/current_user_provider" +require_dependency "rate_limiter" class Auth::DefaultCurrentUserProvider @@ -7,6 +8,7 @@ class Auth::DefaultCurrentUserProvider API_KEY_ENV ||= "_DISCOURSE_API".freeze TOKEN_COOKIE ||= "_t".freeze PATH_INFO ||= "PATH_INFO".freeze + COOKIE_ATTEMPTS_PER_MIN ||= 10 # do all current user initialization here def initialize(env) @@ -40,6 +42,14 @@ class Auth::DefaultCurrentUserProvider .where('auth_token_updated_at IS NULL OR auth_token_updated_at > ?', SiteSetting.maximum_session_age.hours.ago) .first + + unless current_user + begin + RateLimiter.new(nil, "cookie_auth_#{request.ip}", COOKIE_ATTEMPTS_PER_MIN ,60).performed! + rescue RateLimiter::LimitExceeded + raise Discourse::InvalidAccess + end + end end if current_user && (current_user.suspended? || !current_user.active) @@ -69,6 +79,9 @@ class Auth::DefaultCurrentUserProvider user.update_column(:auth_token_updated_at, Time.zone.now) cookies[TOKEN_COOKIE] = { value: user.auth_token, httponly: true, expires: SiteSetting.maximum_session_age.hours.from_now } end + if !user && cookies.key?(TOKEN_COOKIE) + cookies.delete(TOKEN_COOKIE) + end end def log_on_user(user, session, cookies) @@ -107,12 +120,12 @@ class Auth::DefaultCurrentUserProvider if user.admin && defined?(Rack::MiniProfiler) # clear the profiling cookie to keep stuff tidy - cookies["__profilin"] = nil + cookies.delete("__profilin") end user.logged_out end - cookies[TOKEN_COOKIE] = nil + cookies.delete(TOKEN_COOKIE) end diff --git a/spec/components/auth/default_current_user_provider_spec.rb b/spec/components/auth/default_current_user_provider_spec.rb index 14991eafa2a..3fecd09e565 100644 --- a/spec/components/auth/default_current_user_provider_spec.rb +++ b/spec/components/auth/default_current_user_provider_spec.rb @@ -85,6 +85,34 @@ describe Auth::DefaultCurrentUserProvider do expect(user.auth_token_updated_at - Time.now).to eq(0) end + it "can only try 10 bad cookies a minute" do + RateLimiter.stubs(:disabled?).returns(false) + + RateLimiter.new(nil, "cookie_auth_10.0.0.1", 10, 60).clear! + RateLimiter.new(nil, "cookie_auth_10.0.0.2", 10, 60).clear! + + ip = "10.0.0.1" + env = { "HTTP_COOKIE" => "_t=#{SecureRandom.hex}", "REMOTE_ADDR" => ip } + + 10.times do + provider('/', env).current_user + end + expect { + provider('/', env).current_user + }.to raise_error(Discourse::InvalidAccess) + + env["REMOTE_ADDR"] = "10.0.0.2" + provider('/', env).current_user + end + + it "correctly removes invalid cookies" do + + cookies = {"_t" => "BAAAD"} + provider('/').refresh_session(nil, {}, cookies) + + expect(cookies.key?("_t")).to eq(false) + end + it "recycles existing auth_token correctly" do SiteSetting.maximum_session_age = 3 user = Fabricate(:user)