mirror of
https://github.com/discourse/discourse.git
synced 2025-06-05 14:07:30 +08:00
SECURITY: don't grant same privileges to user_api and api access
User API is no longer gets bypasses that standard API gets. Only bypasses are CSRF and XHR requirements.
This commit is contained in:
@ -25,7 +25,7 @@ class ApplicationController < ActionController::Base
|
|||||||
# and then raising a CSRF exception
|
# and then raising a CSRF exception
|
||||||
def handle_unverified_request
|
def handle_unverified_request
|
||||||
# NOTE: API key is secret, having it invalidates the need for a CSRF token
|
# NOTE: API key is secret, having it invalidates the need for a CSRF token
|
||||||
unless is_api?
|
unless is_api? || is_user_api?
|
||||||
super
|
super
|
||||||
clear_current_user
|
clear_current_user
|
||||||
render text: "['BAD CSRF']", status: 403
|
render text: "['BAD CSRF']", status: 403
|
||||||
@ -501,7 +501,7 @@ class ApplicationController < ActionController::Base
|
|||||||
|
|
||||||
def check_xhr
|
def check_xhr
|
||||||
# bypass xhr check on PUT / POST / DELETE provided api key is there, otherwise calling api is annoying
|
# bypass xhr check on PUT / POST / DELETE provided api key is there, otherwise calling api is annoying
|
||||||
return if !request.get? && is_api?
|
return if !request.get? && (is_api? || is_user_api?)
|
||||||
raise RenderEmpty.new unless ((request.format && request.format.json?) || request.xhr?)
|
raise RenderEmpty.new unless ((request.format && request.format.json?) || request.xhr?)
|
||||||
end
|
end
|
||||||
|
|
||||||
|
@ -467,7 +467,7 @@ class PostsController < ApplicationController
|
|||||||
json_obj = json_obj[:post]
|
json_obj = json_obj[:post]
|
||||||
end
|
end
|
||||||
|
|
||||||
if !success && GlobalSetting.try(:verbose_api_logging) && is_api?
|
if !success && GlobalSetting.try(:verbose_api_logging) && (is_api? || is_user_api?)
|
||||||
Rails.logger.error "Error creating post via API:\n\n#{json_obj.inspect}"
|
Rails.logger.error "Error creating post via API:\n\n#{json_obj.inspect}"
|
||||||
end
|
end
|
||||||
|
|
||||||
|
@ -25,6 +25,10 @@ class Auth::CurrentUserProvider
|
|||||||
raise NotImplementedError
|
raise NotImplementedError
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def is_user_api?
|
||||||
|
raise NotImplementedError
|
||||||
|
end
|
||||||
|
|
||||||
# we may need to know very early on in the middleware if an auth token
|
# we may need to know very early on in the middleware if an auth token
|
||||||
# exists, to optimise caching
|
# exists, to optimise caching
|
||||||
def has_auth_cookie?
|
def has_auth_cookie?
|
||||||
|
@ -8,6 +8,7 @@ class Auth::DefaultCurrentUserProvider
|
|||||||
USER_API_KEY ||= "HTTP_USER_API_KEY".freeze
|
USER_API_KEY ||= "HTTP_USER_API_KEY".freeze
|
||||||
USER_API_CLIENT_ID ||= "HTTP_USER_API_CLIENT_ID".freeze
|
USER_API_CLIENT_ID ||= "HTTP_USER_API_CLIENT_ID".freeze
|
||||||
API_KEY_ENV ||= "_DISCOURSE_API".freeze
|
API_KEY_ENV ||= "_DISCOURSE_API".freeze
|
||||||
|
USER_API_KEY_ENV ||= "_DISCOURSE_USER_API".freeze
|
||||||
TOKEN_COOKIE ||= "_t".freeze
|
TOKEN_COOKIE ||= "_t".freeze
|
||||||
PATH_INFO ||= "PATH_INFO".freeze
|
PATH_INFO ||= "PATH_INFO".freeze
|
||||||
COOKIE_ATTEMPTS_PER_MIN ||= 10
|
COOKIE_ATTEMPTS_PER_MIN ||= 10
|
||||||
@ -97,7 +98,7 @@ class Auth::DefaultCurrentUserProvider
|
|||||||
limiter_min.performed!
|
limiter_min.performed!
|
||||||
limiter_day.performed!
|
limiter_day.performed!
|
||||||
|
|
||||||
@env[API_KEY_ENV] = true
|
@env[USER_API_KEY_ENV] = true
|
||||||
end
|
end
|
||||||
|
|
||||||
@env[CURRENT_USER_KEY] = current_user
|
@env[CURRENT_USER_KEY] = current_user
|
||||||
@ -172,7 +173,12 @@ class Auth::DefaultCurrentUserProvider
|
|||||||
# api has special rights return true if api was detected
|
# api has special rights return true if api was detected
|
||||||
def is_api?
|
def is_api?
|
||||||
current_user
|
current_user
|
||||||
@env[API_KEY_ENV]
|
!!(@env[API_KEY_ENV])
|
||||||
|
end
|
||||||
|
|
||||||
|
def is_user_api?
|
||||||
|
current_user
|
||||||
|
!!(@env[USER_API_KEY_ENV])
|
||||||
end
|
end
|
||||||
|
|
||||||
def has_auth_cookie?
|
def has_auth_cookie?
|
||||||
|
@ -26,6 +26,10 @@ module CurrentUser
|
|||||||
current_user_provider.is_api?
|
current_user_provider.is_api?
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def is_user_api?
|
||||||
|
current_user_provider.is_user_api?
|
||||||
|
end
|
||||||
|
|
||||||
def current_user
|
def current_user
|
||||||
current_user_provider.current_user
|
current_user_provider.current_user
|
||||||
end
|
end
|
||||||
|
@ -175,7 +175,11 @@ describe Auth::DefaultCurrentUserProvider do
|
|||||||
"HTTP_USER_API_KEY" => api_key.key,
|
"HTTP_USER_API_KEY" => api_key.key,
|
||||||
}
|
}
|
||||||
|
|
||||||
expect(provider("/", params).current_user.id).to eq(user.id)
|
good_provider = provider("/", params)
|
||||||
|
|
||||||
|
expect(good_provider.current_user.id).to eq(user.id)
|
||||||
|
expect(good_provider.is_api?).to eq(false)
|
||||||
|
expect(good_provider.is_user_api?).to eq(true)
|
||||||
|
|
||||||
expect {
|
expect {
|
||||||
provider("/", params.merge({"REQUEST_METHOD" => "POST"})).current_user
|
provider("/", params.merge({"REQUEST_METHOD" => "POST"})).current_user
|
||||||
|
Reference in New Issue
Block a user