discourse/spec/components/auth/default_current_user_provider_spec.rb
Osama Sayegh b86127ad12
FEATURE: Apply rate limits per user instead of IP for trusted users (#14706)
Currently, Discourse rate limits all incoming requests by the IP address they
originate from regardless of the user making the request. This can be
frustrating if there are multiple users using Discourse simultaneously while
sharing the same IP address (e.g. employees in an office).

This commit implements a new feature to make Discourse apply rate limits by
user id rather than IP address for users at or higher than the configured trust
level (1 is the default).

For example, let's say a Discourse instance is configured to allow 200 requests
per minute per IP address, and we have 10 users at trust level 4 using
Discourse simultaneously from the same IP address. Before this feature, the 10
users could only make a total of 200 requests per minute before they got rate
limited. But with the new feature, each user is allowed to make 200 requests
per minute because the rate limits are applied on user id rather than the IP
address.

The minimum trust level for applying user-id-based rate limits can be
configured by the `skip_per_ip_rate_limit_trust_level` global setting. The
default is 1, but it can be changed by either adding the
`DISCOURSE_SKIP_PER_IP_RATE_LIMIT_TRUST_LEVEL` environment variable with the
desired value to your `app.yml`, or changing the setting's value in the
`discourse.conf` file.

Requests made with API keys are still rate limited by IP address and the
relevant global settings that control API keys rate limits.

Before this commit, Discourse's auth cookie (`_t`) was simply a 32 characters
string that Discourse used to lookup the current user from the database and the
cookie contained no additional information about the user. However, we had to
change the cookie content in this commit so we could identify the user from the
cookie without making a database query before the rate limits logic and avoid
introducing a bottleneck on busy sites.

Besides the 32 characters auth token, the cookie now includes the user id,
trust level and the cookie's generation date, and we encrypt/sign the cookie to
prevent tampering.

Internal ticket number: t54739.
2021-11-17 23:27:30 +03:00

744 lines
23 KiB
Ruby

# frozen_string_literal: true
require 'rails_helper'
describe Auth::DefaultCurrentUserProvider do
# careful using fab! here is can lead to an erratic test
# we want a distinct user object per test so last_seen_at is
# handled correctly
let(:user) { Fabricate(:user) }
class TestProvider < Auth::DefaultCurrentUserProvider
attr_reader :env
def initialize(env)
super(env)
end
def cookie_jar
@cookie_jar ||= ActionDispatch::Request.new(env).cookie_jar
end
end
def provider(url, opts = nil)
opts ||= { method: "GET" }
env = create_request_env(path: url).merge(opts)
TestProvider.new(env)
end
def get_cookie_info(cookie_jar, name)
headers = {}
cookie_jar.always_write_cookie = true
cookie_jar.write(headers)
header = headers["Set-Cookie"]
return if header.nil?
info = {}
line = header.split("\n").find { |l| l.start_with?("#{name}=") }
parts = line.split(";").map(&:strip)
info[:value] = parts.shift.split("=")[1]
parts.each do |p|
key, value = p.split("=")
info[key.downcase.to_sym] = value || true
end
info
end
it "can be used to pretend that a user doesn't exist" do
provider = TestProvider.new(create_request_env(path: "/"))
expect(provider.current_user).to eq(nil)
end
context "server header api" do
it "raises for a revoked key" do
api_key = ApiKey.create!
params = { "HTTP_API_USERNAME" => user.username.downcase, "HTTP_API_KEY" => api_key.key }
expect(
provider("/", params).current_user.id
).to eq(user.id)
api_key.reload.update(revoked_at: Time.zone.now, last_used_at: nil)
expect(api_key.reload.last_used_at).to eq(nil)
params = { "HTTP_API_USERNAME" => user.username.downcase, "HTTP_API_KEY" => api_key.key }
expect {
provider("/", params).current_user
}.to raise_error(Discourse::InvalidAccess)
api_key.reload
expect(api_key.last_used_at).to eq(nil)
end
it "raises errors for incorrect api_key" do
params = { "HTTP_API_KEY" => "INCORRECT" }
expect {
provider("/", params).current_user
}.to raise_error(Discourse::InvalidAccess, /API username or key is invalid/)
end
it "finds a user for a correct per-user api key" do
api_key = ApiKey.create!(user_id: user.id, created_by_id: -1)
params = { "HTTP_API_KEY" => api_key.key }
good_provider = provider("/", params)
expect do
expect(good_provider.current_user.id).to eq(user.id)
end.to change { api_key.reload.last_used_at }
expect(good_provider.is_api?).to eq(true)
expect(good_provider.is_user_api?).to eq(false)
expect(good_provider.should_update_last_seen?).to eq(false)
user.update_columns(active: false)
expect {
provider("/", params).current_user
}.to raise_error(Discourse::InvalidAccess)
user.update_columns(active: true, suspended_till: 1.day.from_now)
expect {
provider("/", params).current_user
}.to raise_error(Discourse::InvalidAccess)
end
it "raises for a user pretending" do
user2 = Fabricate(:user)
api_key = ApiKey.create!(user_id: user.id, created_by_id: -1)
params = { "HTTP_API_KEY" => api_key.key, "HTTP_API_USERNAME" => user2.username.downcase }
expect {
provider("/", params).current_user
}.to raise_error(Discourse::InvalidAccess)
end
it "raises for a user with a mismatching ip" do
api_key = ApiKey.create!(user_id: user.id, created_by_id: -1, allowed_ips: ['10.0.0.0/24'])
params = {
"HTTP_API_KEY" => api_key.key,
"HTTP_API_USERNAME" => user.username.downcase,
"REMOTE_ADDR" => "10.1.0.1"
}
expect {
provider("/", params).current_user
}.to raise_error(Discourse::InvalidAccess)
end
it "allows a user with a matching ip" do
api_key = ApiKey.create!(user_id: user.id, created_by_id: -1, allowed_ips: ['100.0.0.0/24'])
params = {
"HTTP_API_KEY" => api_key.key,
"HTTP_API_USERNAME" => user.username.downcase,
"REMOTE_ADDR" => "100.0.0.22",
}
found_user = provider("/", params).current_user
expect(found_user.id).to eq(user.id)
params = {
"HTTP_API_KEY" => api_key.key,
"HTTP_API_USERNAME" => user.username.downcase,
"HTTP_X_FORWARDED_FOR" => "10.1.1.1, 100.0.0.22"
}
found_user = provider("/", params).current_user
expect(found_user.id).to eq(user.id)
end
it "finds a user for a correct system api key" do
api_key = ApiKey.create!(created_by_id: -1)
params = { "HTTP_API_KEY" => api_key.key, "HTTP_API_USERNAME" => user.username.downcase }
expect(provider("/", params).current_user.id).to eq(user.id)
end
it "raises for a mismatched api_key header and param username" do
api_key = ApiKey.create!(created_by_id: -1)
params = { "HTTP_API_KEY" => api_key.key }
expect {
provider("/?api_username=#{user.username.downcase}", params).current_user
}.to raise_error(Discourse::InvalidAccess)
end
it "finds a user for a correct system api key with external id" do
api_key = ApiKey.create!(created_by_id: -1)
SingleSignOnRecord.create(user_id: user.id, external_id: "abc", last_payload: '')
params = { "HTTP_API_KEY" => api_key.key, "HTTP_API_USER_EXTERNAL_ID" => "abc" }
expect(provider("/", params).current_user.id).to eq(user.id)
end
it "raises for a mismatched api_key header and param external id" do
api_key = ApiKey.create!(created_by_id: -1)
SingleSignOnRecord.create(user_id: user.id, external_id: "abc", last_payload: '')
params = { "HTTP_API_KEY" => api_key.key }
expect {
provider("/?api_user_external_id=abc", params).current_user
}.to raise_error(Discourse::InvalidAccess)
end
it "finds a user for a correct system api key with id" do
api_key = ApiKey.create!(created_by_id: -1)
params = { "HTTP_API_KEY" => api_key.key, "HTTP_API_USER_ID" => user.id }
expect(provider("/", params).current_user.id).to eq(user.id)
end
it "raises for a mismatched api_key header and param user id" do
api_key = ApiKey.create!(created_by_id: -1)
params = { "HTTP_API_KEY" => api_key.key }
expect {
provider("/?api_user_id=#{user.id}", params).current_user
}.to raise_error(Discourse::InvalidAccess)
end
describe "when readonly mode is enabled due to postgres" do
before do
Discourse.enable_readonly_mode(Discourse::PG_READONLY_MODE_KEY)
end
after do
Discourse.disable_readonly_mode(Discourse::PG_READONLY_MODE_KEY)
end
it "should not update ApiKey#last_used_at" do
api_key = ApiKey.create!(user_id: user.id, created_by_id: -1)
params = { "HTTP_API_KEY" => api_key.key }
good_provider = provider("/", params)
expect do
expect(good_provider.current_user.id).to eq(user.id)
end.to_not change { api_key.reload.last_used_at }
end
end
context "rate limiting" do
before do
RateLimiter.enable
end
it "rate limits admin api requests" do
global_setting :max_admin_api_reqs_per_minute, 3
freeze_time
RateLimiter.new(nil, "admin_api_min", 3, 60).clear!
api_key = ApiKey.create!(created_by_id: -1)
params = { "HTTP_API_KEY" => api_key.key, "HTTP_API_USERNAME" => user.username.downcase }
system_params = params.merge("HTTP_API_USERNAME" => "system")
provider("/", params).current_user
provider("/", system_params).current_user
provider("/", params).current_user
expect do
provider("/", system_params).current_user
end.to raise_error(RateLimiter::LimitExceeded)
freeze_time 59.seconds.from_now
expect do
provider("/", system_params).current_user
end.to raise_error(RateLimiter::LimitExceeded)
freeze_time 2.seconds.from_now
# 1 minute elapsed
provider("/", system_params).current_user
# should not rate limit a random key
api_key.destroy
api_key = ApiKey.create!(created_by_id: -1)
params = { "HTTP_API_KEY" => api_key.key, "HTTP_API_USERNAME" => user.username.downcase }
provider("/", params).current_user
end
end
end
describe "#current_user" do
let(:cookie) do
new_provider = provider('/')
new_provider.log_on_user(user, {}, new_provider.cookie_jar)
new_provider.cookie_jar["_t"]
end
before do
@orig = freeze_time
user.clear_last_seen_cache!(@orig)
end
after do
user.clear_last_seen_cache!(@orig)
end
it "should not update last seen for suspended users" do
provider2 = provider("/", "HTTP_COOKIE" => "_t=#{cookie}")
u = provider2.current_user
u.reload
expect(u.last_seen_at).to eq_time(Time.zone.now)
freeze_time 20.minutes.from_now
u.last_seen_at = nil
u.suspended_till = 1.year.from_now
u.save!
u.clear_last_seen_cache!
provider2 = provider("/", "HTTP_COOKIE" => "_t=#{cookie}")
expect(provider2.current_user).to eq(nil)
u.reload
expect(u.last_seen_at).to eq(nil)
end
describe "when readonly mode is enabled due to postgres" do
before do
Discourse.enable_readonly_mode(Discourse::PG_READONLY_MODE_KEY)
end
after do
Discourse.disable_readonly_mode(Discourse::PG_READONLY_MODE_KEY)
end
it "should not update User#last_seen_at" do
provider2 = provider("/", "HTTP_COOKIE" => "_t=#{cookie}")
u = provider2.current_user
u.reload
expect(u.last_seen_at).to eq(nil)
end
end
end
it "should update last seen for non ajax" do
expect(provider("/topic/anything/goes", method: "POST").should_update_last_seen?).to eq(true)
expect(provider("/topic/anything/goes", method: "GET").should_update_last_seen?).to eq(true)
end
it "should update ajax reqs with discourse visible" do
expect(provider("/topic/anything/goes",
:method => "POST",
"HTTP_X_REQUESTED_WITH" => "XMLHttpRequest",
"HTTP_DISCOURSE_PRESENT" => "true"
).should_update_last_seen?).to eq(true)
end
it "should not update last seen for ajax calls without Discourse-Present header" do
expect(provider("/topic/anything/goes",
:method => "POST",
"HTTP_X_REQUESTED_WITH" => "XMLHttpRequest"
).should_update_last_seen?).to eq(false)
end
it "should update last seen for API calls with Discourse-Present header" do
api_key = ApiKey.create!(user_id: user.id, created_by_id: -1)
params = { :method => "POST",
"HTTP_X_REQUESTED_WITH" => "XMLHttpRequest",
"HTTP_API_KEY" => api_key.key
}
expect(provider("/topic/anything/goes", params).should_update_last_seen?).to eq(false)
expect(provider("/topic/anything/goes", params.merge("HTTP_DISCOURSE_PRESENT" => "true")).should_update_last_seen?).to eq(true)
end
it "supports non persistent sessions" do
SiteSetting.persistent_sessions = false
@provider = provider('/')
@provider.log_on_user(user, {}, @provider.cookie_jar)
cookie_info = get_cookie_info(@provider.cookie_jar, "_t")
expect(cookie_info[:expires]).to eq(nil)
end
it "v0 of auth cookie is still acceptable" do
token = UserAuthToken.generate!(user_id: user.id).unhashed_auth_token
ip = "10.0.0.1"
env = { "HTTP_COOKIE" => "_t=#{token}", "REMOTE_ADDR" => ip }
expect(provider('/', env).current_user.id).to eq(user.id)
end
it "correctly rotates tokens" do
SiteSetting.maximum_session_age = 3
@provider = provider('/')
@provider.log_on_user(user, {}, @provider.cookie_jar)
cookie = @provider.cookie_jar["_t"]
unhashed_token = decrypt_auth_cookie(cookie)[:token]
token = UserAuthToken.find_by(user_id: user.id)
expect(token.auth_token_seen).to eq(false)
expect(token.auth_token).not_to eq(unhashed_token)
expect(token.auth_token).to eq(UserAuthToken.hash_token(unhashed_token))
# at this point we are going to try to rotate token
freeze_time 20.minutes.from_now
provider2 = provider("/", "HTTP_COOKIE" => "_t=#{cookie}")
provider2.current_user
token.reload
expect(token.auth_token_seen).to eq(true)
provider2.refresh_session(user, {}, provider2.cookie_jar)
expect(
decrypt_auth_cookie(provider2.cookie_jar["_t"])[:token]
).not_to eq(unhashed_token)
expect(
decrypt_auth_cookie(provider2.cookie_jar["_t"])[:token].size
).to eq(32)
token.reload
expect(token.auth_token_seen).to eq(false)
freeze_time 21.minutes.from_now
old_token = token.prev_auth_token
unverified_token = token.auth_token
# old token should still work
provider2 = provider("/", "HTTP_COOKIE" => "_t=#{cookie}")
expect(provider2.current_user.id).to eq(user.id)
provider2.refresh_session(user, {}, provider2.cookie_jar)
token.reload
# because this should cause a rotation since we can safely
# assume it never reached the client
expect(token.prev_auth_token).to eq(old_token)
expect(token.auth_token).not_to eq(unverified_token)
end
context "events" do
before do
@refreshes = 0
@increase_refreshes = -> (user) { @refreshes += 1 }
DiscourseEvent.on(:user_session_refreshed, &@increase_refreshes)
end
after do
DiscourseEvent.off(:user_session_refreshed, &@increase_refreshes)
end
it "fires event when updating last seen" do
@provider = provider('/')
@provider.log_on_user(user, {}, @provider.cookie_jar)
cookie = @provider.cookie_jar["_t"]
unhashed_token = decrypt_auth_cookie(cookie)[:token]
freeze_time 20.minutes.from_now
provider2 = provider("/", "HTTP_COOKIE" => "_t=#{cookie}")
provider2.refresh_session(user, {}, provider2.cookie_jar)
expect(@refreshes).to eq(1)
end
it "does not fire an event when last seen does not update" do
@provider = provider('/')
@provider.log_on_user(user, {}, @provider.cookie_jar)
cookie = @provider.cookie_jar["_t"]
unhashed_token = decrypt_auth_cookie(cookie)[:token]
freeze_time 2.minutes.from_now
provider2 = provider("/", "HTTP_COOKIE" => "_t=#{cookie}")
provider2.refresh_session(user, {}, provider2.cookie_jar)
expect(@refreshes).to eq(0)
end
end
context "rate limiting" do
before do
RateLimiter.enable
end
it "can only try 10 bad cookies a minute" do
token = UserAuthToken.generate!(user_id: user.id)
cookie = create_auth_cookie(
token: token.unhashed_auth_token,
user_id: user.id,
trust_level: user.trust_level,
issued_at: 5.minutes.ago
)
@provider = provider('/')
@provider.log_on_user(user, {}, @provider.cookie_jar)
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"
bad_cookie = create_auth_cookie(
token: SecureRandom.hex,
user_id: user.id,
trust_level: user.trust_level,
issued_at: 5.minutes.ago,
)
env = { "HTTP_COOKIE" => "_t=#{bad_cookie}", "REMOTE_ADDR" => ip }
10.times do
provider('/', env).current_user
end
expect {
provider('/', env).current_user
}.to raise_error(Discourse::InvalidAccess)
expect {
env["HTTP_COOKIE"] = "_t=#{cookie}"
provider("/", env).current_user
}.to raise_error(Discourse::InvalidAccess)
env["REMOTE_ADDR"] = "10.0.0.2"
expect {
provider('/', env).current_user
}.not_to raise_error
end
end
it "correctly removes invalid cookies" do
bad_cookie = create_auth_cookie(
token: SecureRandom.hex,
user_id: 1,
trust_level: 4,
issued_at: 5.minutes.ago,
)
@provider = provider('/')
@provider.cookie_jar["_t"] = bad_cookie
@provider.refresh_session(nil, {}, @provider.cookie_jar)
expect(@provider.cookie_jar.key?("_t")).to eq(false)
end
it "logging on user always creates a new token" do
@provider = provider('/')
@provider.log_on_user(user, {}, @provider.cookie_jar)
@provider2 = provider('/')
@provider2.log_on_user(user, {}, @provider2.cookie_jar)
expect(UserAuthToken.where(user_id: user.id).count).to eq(2)
end
it "cleans up old sessions when a user logs in" do
yesterday = 1.day.ago
UserAuthToken.insert_all((1..(UserAuthToken::MAX_SESSION_COUNT + 2)).to_a.map do |i|
{
user_id: user.id,
created_at: yesterday + i.seconds,
updated_at: yesterday + i.seconds,
rotated_at: yesterday + i.seconds,
prev_auth_token: "abc#{i}",
auth_token: "abc#{i}"
}
end)
# Check the oldest 3 still exist
expect(UserAuthToken.where(auth_token: (1..3).map { |i| "abc#{i}" }).count).to eq(3)
# On next login, gets fixed
@provider = provider('/')
@provider.log_on_user(user, {}, @provider.cookie_jar)
expect(UserAuthToken.where(user_id: user.id).count).to eq(UserAuthToken::MAX_SESSION_COUNT)
# Oldest sessions are 1, 2, 3. They should now be deleted
expect(UserAuthToken.where(auth_token: (1..3).map { |i| "abc#{i}" }).count).to eq(0)
end
it "sets secure, same site lax cookies" do
SiteSetting.force_https = false
SiteSetting.same_site_cookies = "Lax"
@provider = provider('/')
@provider.log_on_user(user, {}, @provider.cookie_jar)
cookie_info = get_cookie_info(@provider.cookie_jar, "_t")
expect(cookie_info[:samesite]).to eq("Lax")
expect(cookie_info[:httponly]).to eq(true)
expect(cookie_info.key?(:secure)).to eq(false)
SiteSetting.force_https = true
SiteSetting.same_site_cookies = "Disabled"
@provider = provider('/')
@provider.log_on_user(user, {}, @provider.cookie_jar)
cookie_info = get_cookie_info(@provider.cookie_jar, "_t")
expect(cookie_info[:secure]).to eq(true)
expect(cookie_info.key?(:same_site)).to eq(false)
end
it "correctly expires session" do
SiteSetting.maximum_session_age = 2
token = UserAuthToken.generate!(user_id: user.id)
cookie = create_auth_cookie(
token: token.unhashed_auth_token,
user_id: user.id,
trust_level: user.trust_level,
issued_at: 5.minutes.ago
)
@provider = provider('/')
@provider.log_on_user(user, {}, @provider.cookie_jar)
expect(provider("/", "HTTP_COOKIE" => "_t=#{cookie}").current_user.id).to eq(user.id)
freeze_time 3.hours.from_now
expect(provider("/", "HTTP_COOKIE" => "_t=#{cookie}").current_user).to eq(nil)
end
it "always unstage users" do
user.update!(staged: true)
@provider = provider("/")
@provider.log_on_user(user, {}, @provider.cookie_jar)
user.reload
expect(user.staged).to eq(false)
end
context "user api" do
fab! :user do
Fabricate(:user)
end
let :api_key do
UserApiKey.create!(
application_name: 'my app',
client_id: '1234',
scopes: ['read'].map { |name| UserApiKeyScope.new(name: name) },
user_id: user.id
)
end
it "can clear old duplicate keys correctly" do
dupe = UserApiKey.create!(
application_name: 'my app',
client_id: '12345',
scopes: ['read'].map { |name| UserApiKeyScope.new(name: name) },
user_id: user.id
)
params = {
"REQUEST_METHOD" => "GET",
"HTTP_USER_API_KEY" => api_key.key,
"HTTP_USER_API_CLIENT_ID" => dupe.client_id,
}
good_provider = provider("/", params)
expect(good_provider.current_user.id).to eq(user.id)
expect(UserApiKey.find_by(id: dupe.id)).to eq(nil)
end
it "allows user API access correctly" do
params = {
"REQUEST_METHOD" => "GET",
"HTTP_USER_API_KEY" => api_key.key,
}
good_provider = provider("/", params)
expect do
expect(good_provider.current_user.id).to eq(user.id)
end.to change { api_key.reload.last_used_at }
expect(good_provider.is_api?).to eq(false)
expect(good_provider.is_user_api?).to eq(true)
expect(good_provider.should_update_last_seen?).to eq(false)
expect {
provider("/", params.merge("REQUEST_METHOD" => "POST")).current_user
}.to raise_error(Discourse::InvalidAccess)
user.update_columns(suspended_till: 1.year.from_now)
expect {
provider("/", params).current_user
}.to raise_error(Discourse::InvalidAccess)
end
describe "when readonly mode is enabled due to postgres" do
before do
Discourse.enable_readonly_mode(Discourse::PG_READONLY_MODE_KEY)
end
after do
Discourse.disable_readonly_mode(Discourse::PG_READONLY_MODE_KEY)
end
it 'should not update ApiKey#last_used_at' do
params = {
"REQUEST_METHOD" => "GET",
"HTTP_USER_API_KEY" => api_key.key,
}
good_provider = provider("/", params)
expect do
expect(good_provider.current_user.id).to eq(user.id)
end.to_not change { api_key.reload.last_used_at }
end
end
context "rate limiting" do
before do
RateLimiter.enable
end
it "rate limits api usage" do
limiter1 = RateLimiter.new(nil, "user_api_day_#{ApiKey.hash_key(api_key.key)}", 10, 60)
limiter2 = RateLimiter.new(nil, "user_api_min_#{ApiKey.hash_key(api_key.key)}", 10, 60)
limiter1.clear!
limiter2.clear!
global_setting :max_user_api_reqs_per_day, 3
global_setting :max_user_api_reqs_per_minute, 4
params = {
"REQUEST_METHOD" => "GET",
"HTTP_USER_API_KEY" => api_key.key,
}
3.times do
provider("/", params).current_user
end
expect {
provider("/", params).current_user
}.to raise_error(RateLimiter::LimitExceeded)
global_setting :max_user_api_reqs_per_day, 4
global_setting :max_user_api_reqs_per_minute, 3
limiter1.clear!
limiter2.clear!
3.times do
provider("/", params).current_user
end
expect {
provider("/", params).current_user
}.to raise_error(RateLimiter::LimitExceeded)
end
end
end
it "ignores a valid auth cookie that has been tampered with" do
@provider = provider('/')
@provider.log_on_user(user, {}, @provider.cookie_jar)
cookie = @provider.cookie_jar["_t"]
cookie = swap_2_different_characters(cookie)
ip = "10.0.0.1"
env = { "HTTP_COOKIE" => "_t=#{cookie}", "REMOTE_ADDR" => ip }
expect(provider('/', env).current_user).to eq(nil)
end
end