FEATURE: Header based auth for API requests (#7129)

Now you can also make authenticated API requests by passing the
`api_key` and `api_username` in the HTTP header instead of query params.

The new header values are: `Api-key` and `Api-Username`.

Here is an example in cURL:

``` text
curl -i -sS -X POST "http://127.0.0.1:3000/categories" \
  -H "Content-Type: multipart/form-data;" \
  -H "Api-Key: 7aa202bec1ff70563bc0a3d102feac0a7dd2af96b5b772a9feaf27485f9d31a2" \
  -H "Api-Username: system" \
  -F "name=7c1c0ed93583cba7124b745d1bd56b32" \
  -F "color=49d9e9" \
  -F "text_color=f0fcfd"
```

There is also support for `Api-User-Id` and `Api-User-External-Id`
instead of specifying the username along with the key.
This commit is contained in:
Blake Erickson 2019-03-08 09:13:31 -07:00 committed by GitHub
parent 5e58cedfbd
commit f03b293e6a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 165 additions and 4 deletions

View File

@ -7,6 +7,11 @@ class Auth::DefaultCurrentUserProvider
CURRENT_USER_KEY ||= "_DISCOURSE_CURRENT_USER"
API_KEY ||= "api_key"
API_USERNAME ||= "api_username"
HEADER_API_KEY ||= "HTTP_API_KEY"
HEADER_API_USERNAME ||= "HTTP_API_USERNAME"
HEADER_API_USER_EXTERNAL_ID ||= "HTTP_API_USER_EXTERNAL_ID"
HEADER_API_USER_ID ||= "HTTP_API_USER_ID"
USER_API_KEY ||= "HTTP_USER_API_KEY"
USER_API_CLIENT_ID ||= "HTTP_USER_API_CLIENT_ID"
API_KEY_ENV ||= "_DISCOURSE_API"
@ -40,7 +45,7 @@ class Auth::DefaultCurrentUserProvider
request = @request
user_api_key = @env[USER_API_KEY]
api_key = @env.blank? ? nil : request[API_KEY]
api_key = @env.blank? ? nil : request[API_KEY] || @env[HEADER_API_KEY]
auth_token = request.cookies[TOKEN_COOKIE] unless user_api_key || api_key
@ -279,7 +284,7 @@ class Auth::DefaultCurrentUserProvider
def lookup_api_user(api_key_value, request)
if api_key = ApiKey.where(key: api_key_value).includes(:user).first
api_username = request["api_username"]
api_username = request[API_USERNAME] || @env[HEADER_API_USERNAME]
if api_key.allowed_ips.present? && !api_key.allowed_ips.any? { |ip| ip.include?(request.ip) }
Rails.logger.warn("[Unauthorized API Access] username: #{api_username}, IP address: #{request.ip}")
@ -290,9 +295,9 @@ class Auth::DefaultCurrentUserProvider
api_key.user if !api_username || (api_key.user.username_lower == api_username.downcase)
elsif api_username
User.find_by(username_lower: api_username.downcase)
elsif user_id = request["api_user_id"]
elsif user_id = request["api_user_id"] || @env[HEADER_API_USER_ID]
User.find_by(id: user_id.to_i)
elsif external_id = request["api_user_external_id"]
elsif external_id = request["api_user_external_id"] || @env[HEADER_API_USER_EXTERNAL_ID]
SingleSignOnRecord.find_by(external_id: external_id.to_s).try(:user)
end
end

View File

@ -153,6 +153,162 @@ describe Auth::DefaultCurrentUserProvider do
end
context "server header api" do
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
user = Fabricate(:user)
ApiKey.create!(key: "hello", user_id: user.id, created_by_id: -1)
params = { "HTTP_API_KEY" => "hello" }
good_provider = provider("/", params)
expect(good_provider.current_user.id).to eq(user.id)
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
user = Fabricate(:user)
user2 = Fabricate(:user)
ApiKey.create!(key: "hello", user_id: user.id, created_by_id: -1)
params = { "HTTP_API_KEY" => "hello", "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
user = Fabricate(:user)
ApiKey.create!(key: "hello", user_id: user.id, created_by_id: -1, allowed_ips: ['10.0.0.0/24'])
params = {
"HTTP_API_KEY" => "hello",
"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
user = Fabricate(:user)
ApiKey.create!(key: "hello", user_id: user.id, created_by_id: -1, allowed_ips: ['100.0.0.0/24'])
params = {
"HTTP_API_KEY" => "hello",
"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" => "hello",
"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
user = Fabricate(:user)
ApiKey.create!(key: "hello", created_by_id: -1)
params = { "HTTP_API_KEY" => "hello", "HTTP_API_USERNAME" => user.username.downcase }
expect(provider("/", params).current_user.id).to eq(user.id)
end
it "finds a user for a correct system api key with external id" do
user = Fabricate(:user)
ApiKey.create!(key: "hello", created_by_id: -1)
SingleSignOnRecord.create(user_id: user.id, external_id: "abc", last_payload: '')
params = { "HTTP_API_KEY" => "hello", "HTTP_API_USER_EXTERNAL_ID" => "abc" }
expect(provider("/", params).current_user.id).to eq(user.id)
end
it "finds a user for a correct system api key with id" do
user = Fabricate(:user)
ApiKey.create!(key: "hello", created_by_id: -1)
params = { "HTTP_API_KEY" => "hello", "HTTP_API_USER_ID" => user.id }
expect(provider("/", params).current_user.id).to eq(user.id)
end
context "rate limiting" do
before do
RateLimiter.enable
end
after do
RateLimiter.disable
end
it "rate limits api requests per api key" do
global_setting :max_admin_api_reqs_per_key_per_minute, 3
freeze_time
user = Fabricate(:user)
key = SecureRandom.hex
api_key = ApiKey.create!(key: key, created_by_id: -1)
params = { "HTTP_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
key = SecureRandom.hex
ApiKey.create!(key: key, created_by_id: -1)
params = { "HTTP_API_KEY" => key, "HTTP_API_USERNAME" => user.username.downcase }
provider("/", params).current_user
end
end
end
it "should not update last seen for ajax calls without Discourse-Visible header" do
expect(provider("/topic/anything/goes",
:method => "POST",