From b6c5a73c0bc0bc68ca8c66a5cefa50d314cc164a Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Wed, 27 Sep 2017 15:31:52 +0200 Subject: Make sure API responds with 401 when invalid authentication info is provided --- lib/api/helpers.rb | 30 ++++++++++++++++++++---------- 1 file changed, 20 insertions(+), 10 deletions(-) (limited to 'lib/api/helpers.rb') diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 00dbc2aee7a..1e8475ba3ec 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -3,6 +3,8 @@ module API include Gitlab::Utils include Helpers::Pagination + UnauthorizedError = Class.new(StandardError) + SUDO_HEADER = "HTTP_SUDO".freeze SUDO_PARAM = :sudo @@ -139,7 +141,7 @@ module API end def authenticate! - unauthorized! unless current_user && can?(initial_current_user, :access_api) + unauthorized! unless current_user end def authenticate_non_get! @@ -397,19 +399,27 @@ module API def initial_current_user return @initial_current_user if defined?(@initial_current_user) - Gitlab::Auth::UniqueIpsLimiter.limit_user! do - @initial_current_user ||= find_user_by_private_token(scopes: scopes_registered_for_endpoint) - @initial_current_user ||= doorkeeper_guard(scopes: scopes_registered_for_endpoint) - @initial_current_user ||= find_user_from_warden - - unless @initial_current_user && Gitlab::UserAccess.new(@initial_current_user).allowed? - @initial_current_user = nil - end - @initial_current_user + begin + @initial_current_user = Gitlab::Auth::UniqueIpsLimiter.limit_user! { find_current_user } + rescue APIGuard::UnauthorizedError, UnauthorizedError + unauthorized! end end + def find_current_user + user = + find_user_by_private_token(scopes: scopes_registered_for_endpoint) || + doorkeeper_guard(scopes: scopes_registered_for_endpoint) || + find_user_from_warden + + return nil unless user + + raise UnauthorizedError unless Gitlab::UserAccess.new(user).allowed? && user.can?(:access_api) + + user + end + def sudo! return unless sudo_identifier return unless initial_current_user -- cgit v1.2.1 From 3040b994df0de7b6a6c4159a887fe7c8733bbb4d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Fri, 29 Sep 2017 13:14:08 +0200 Subject: Ensure no exception is raised when Raven tries to get the current user in API context MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rémy Coutable --- lib/api/helpers.rb | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'lib/api/helpers.rb') diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 1e8475ba3ec..4964a76bef6 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -464,10 +464,12 @@ module API header(*Gitlab::Workhorse.send_artifacts_entry(build, entry)) end - # The Grape Error Middleware only has access to env but no params. We workaround this by - # defining a method that returns the right value. + # The Grape Error Middleware only has access to `env` but not `params` nor + # `request`. We workaround this by defining methods that returns the right + # values. def define_params_for_grape_middleware - self.define_singleton_method(:params) { Rack::Request.new(env).params.symbolize_keys } + self.define_singleton_method(:request) { Rack::Request.new(env) } + self.define_singleton_method(:params) { request.params.symbolize_keys } end # We could get a Grape or a standard Ruby exception. We should only report anything that -- cgit v1.2.1