api: return error if login or api_key params are given but blank.
* Make it an error to supply empty API credentials, like this: `https://danbooru.donmai.us/posts.json?login=&api_key=`. Some clients did this for some reason. * Make it so that the `login` and `api_key` params are only allowed as URL params, not as POST or PUT body params. Allowing them as body params could interfere with the `PUT /api_keys/:id` endpoint, which takes an `api_key` param.
This commit is contained in:
@@ -16,7 +16,7 @@ class SessionLoader
|
||||
def initialize(request)
|
||||
@request = request
|
||||
@session = request.session
|
||||
@params = request.parameters
|
||||
@params = request.query_parameters
|
||||
end
|
||||
|
||||
# Attempt to log a user in with the given username and password. Records a
|
||||
@@ -90,7 +90,7 @@ class SessionLoader
|
||||
|
||||
# @return [Boolean] true if the current request has an API key
|
||||
def has_api_authentication?
|
||||
request.authorization.present? || params[:login].present? || (params[:api_key].present? && params[:api_key].is_a?(String))
|
||||
request.authorization.present? || params.has_key?(:login) || params.has_key?(:api_key)
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
@@ -165,19 +165,27 @@ class ApplicationControllerTest < ActionDispatch::IntegrationTest
|
||||
end
|
||||
|
||||
should "fail for api key mismatches" do
|
||||
get profile_path, as: :json, params: { login: @user.name }
|
||||
get profile_path(login: @user.name), as: :json
|
||||
assert_response 401
|
||||
|
||||
get profile_path, as: :json, params: { api_key: @api_key.key }
|
||||
get profile_path(api_key: @api_key.key), as: :json
|
||||
assert_response 401
|
||||
|
||||
get profile_path, as: :json, params: { login: @user.name, api_key: "bad" }
|
||||
get profile_path(login: @user.name, api_key: "bad"), as: :json
|
||||
assert_response 401
|
||||
end
|
||||
|
||||
should "fail for a blank API key" do
|
||||
get profile_path(login: ""), as: :json
|
||||
assert_response 401
|
||||
|
||||
get profile_path(api_key: ""), as: :json
|
||||
assert_response 401
|
||||
end
|
||||
|
||||
should "succeed for non-GET requests without a CSRF token" do
|
||||
assert_changes -> { @user.reload.enable_safe_mode }, from: false, to: true do
|
||||
put user_path(@user), params: { login: @user.name, api_key: @api_key.key, user: { enable_safe_mode: "true" } }, as: :json
|
||||
put user_path(@user, login: @user.name, api_key: @api_key.key), params: { user: { enable_safe_mode: "true" }}, as: :json
|
||||
assert_response :success
|
||||
end
|
||||
end
|
||||
@@ -220,16 +228,16 @@ class ApplicationControllerTest < ActionDispatch::IntegrationTest
|
||||
@post = create(:post)
|
||||
@api_key = create(:api_key, permissions: ["posts:index", "posts:show"])
|
||||
|
||||
get posts_path, params: { login: @api_key.user.name, api_key: @api_key.key }
|
||||
get posts_path(login: @api_key.user.name, api_key: @api_key.key)
|
||||
assert_response :success
|
||||
|
||||
get post_path(@post), params: { login: @api_key.user.name, api_key: @api_key.key }
|
||||
get post_path(@post, login: @api_key.user.name, api_key: @api_key.key)
|
||||
assert_response :success
|
||||
|
||||
get tags_path, params: { login: @api_key.user.name, api_key: @api_key.key }
|
||||
get tags_path(login: @api_key.user.name, api_key: @api_key.key)
|
||||
assert_response 403
|
||||
|
||||
put post_path(@post), params: { login: @api_key.user.name, api_key: @api_key.key, post: { rating: "s" }}
|
||||
put post_path(@post, login: @api_key.user.name, api_key: @api_key.key), params: { post: { rating: "s" }}
|
||||
assert_response 403
|
||||
|
||||
assert_equal(4, @api_key.reload.uses)
|
||||
|
||||
@@ -11,6 +11,7 @@ class SessionLoaderTest < ActiveSupport::TestCase
|
||||
@request.stubs(:cookie_jar).returns({})
|
||||
@request.stubs(:cookies).returns({})
|
||||
@request.stubs(:parameters).returns({})
|
||||
@request.stubs(:query_parameters).returns({})
|
||||
@request.stubs(:session).returns({})
|
||||
@request.stubs(:headers).returns({})
|
||||
SessionLoader.any_instance.stubs(:initialize_session_cookies)
|
||||
|
||||
Reference in New Issue
Block a user