Discource-C
/
discourse
mirror of https://github.com/discourse/discourse.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

FIX: Don't leak unhashed user API keys to redis (#14682) 

User API keys (not the same thing as admin API keys) are currently
leaked to redis when rate limits are applied to them since redis is the
backend for rate limits in Discourse and the API keys are included in
the redis keys that are used to track usage of user API keys in the last
24 hours.

This commit stops the leak by using a SHA-256 representation of the user
API key instead of the key itself to form the redis key.

We don't need to manually delete the existing redis keys that contain
unhashed user API keys because they're not long-lived and will be
automatically deleted within 48 hours after this commit is deployed to
your Discourse instance.
This commit is contained in:
Osama Sayegh 2021-10-21 19:43:26 +03:00 committed by GitHub
parent 3b90d7de66
commit 70fa67a9e1
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 5 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
lib/auth/default_current_user_provider.rb
View File

@ -137,8 +137,9 @@ class Auth::DefaultCurrentUserProvider
# user api key handling
if user_api_key
limiter_min = RateLimiter.new(nil, "user_api_min_#{user_api_key}", GlobalSetting.max_user_api_reqs_per_minute, 60)
limiter_day = RateLimiter.new(nil, "user_api_day_#{user_api_key}", GlobalSetting.max_user_api_reqs_per_day, 86400)
hashed_user_api_key = ApiKey.hash_key(user_api_key)
limiter_min = RateLimiter.new(nil, "user_api_min_#{hashed_user_api_key}", GlobalSetting.max_user_api_reqs_per_minute, 60)
limiter_day = RateLimiter.new(nil, "user_api_day_#{hashed_user_api_key}", GlobalSetting.max_user_api_reqs_per_day, 86400)
unless limiter_day.can_perform?
limiter_day.performed!

4
spec/components/auth/default_current_user_provider_spec.rb
View File

@ -621,8 +621,8 @@ describe Auth::DefaultCurrentUserProvider do
end
it "rate limits api usage" do
limiter1 = RateLimiter.new(nil, "user_api_day_#{api_key.key}", 10, 60)
limiter2 = RateLimiter.new(nil, "user_api_min_#{api_key.key}", 10, 60)
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!