From d774825f981a73263c9a6c276c672b0c3e9bf104 Mon Sep 17 00:00:00 2001 From: Timothy Andrew Date: Tue, 20 Jun 2017 12:00:57 +0000 Subject: [PATCH] When verifying scopes, manually include scopes from `API::API`. - They are not included automatically since `API::Users` does not inherit from `API::API`, as I initially assumed. - Scopes declared in `API::API` are considered global (to the API), and need to be included in all cases. --- lib/api/api_guard.rb | 10 ++++------ lib/api/helpers.rb | 23 +++++++++++++++++++---- 2 files changed, 23 insertions(+), 10 deletions(-) diff --git a/lib/api/api_guard.rb b/lib/api/api_guard.rb index ceeecbbc00b..29ca760ec25 100644 --- a/lib/api/api_guard.rb +++ b/lib/api/api_guard.rb @@ -24,13 +24,11 @@ module API end class_methods do - # Set the authorization scope(s) allowed for the current request. + # Set the authorization scope(s) allowed for an API endpoint. # - # A call to this method adds to any previous scopes in place, either from the same class, or - # higher up in the inheritance chain. For example, if we call `allow_access_with_scope :api` from - # `API::API`, and `allow_access_with_scope :read_user` from `API::Users` (which inherits from `API::API`), - # `API::Users` will allow access with either the `api` or `read_user` scope. `API::API` will allow - # access only with the `api` scope. + # A call to this method maps the given scope(s) to the current API + # endpoint class. If this method is called multiple times on the same class, + # the scopes are all aggregated. def allow_access_with_scope(scopes, options = {}) @scopes ||= [] diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index c69e7afea8c..5c0b82587ab 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -340,12 +340,10 @@ module API end def initial_current_user - endpoint_class = options[:for].presence || ::API::API - return @initial_current_user if defined?(@initial_current_user) Gitlab::Auth::UniqueIpsLimiter.limit_user! do - @initial_current_user ||= find_user_by_private_token(scopes: endpoint_class.scopes) - @initial_current_user ||= doorkeeper_guard(scopes: endpoint_class.scopes) + @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? @@ -409,5 +407,22 @@ module API exception.status == 500 end + + # An array of scopes that were registered (using `allow_access_with_scope`) + # for the current endpoint class. It also returns scopes registered on + # `API::API`, since these are meant to apply to all API routes. + def scopes_registered_for_endpoint + @scopes_registered_for_endpoint ||= + begin + endpoint_classes = [options[:for].presence, ::API::API].compact + endpoint_classes.reduce([]) do |memo, endpoint| + if endpoint.respond_to?(:scopes) + memo.concat(endpoint.scopes) + else + memo + end + end + end + end end end