diff --git a/app/services/access_token_validation_service.rb b/app/services/access_token_validation_service.rb index b2a543daa00ccae857a0aa41b29470b938955d44..f171f8194bd1868592f424899efc7decd6470034 100644 --- a/app/services/access_token_validation_service.rb +++ b/app/services/access_token_validation_service.rb @@ -31,8 +31,11 @@ class AccessTokenValidationService if scopes.blank? true else + #scopes = scopes.reject { |scope| scope[:if].presence && !scope[:if].call(request) } # Check whether the token is allowed access to any of the required scopes. - Set.new(scopes).intersection(Set.new(token.scopes)).present? + + scope_names = scopes.map { |scope| scope[:name].to_s } + Set.new(scope_names).intersection(Set.new(token.scopes)).present? end end end diff --git a/lib/api/api.rb b/lib/api/api.rb index d767af36e8e84707ad7c1f9103bc267d88c207cf..efcf0976a819d097589ab2a1f48fc7293f0cda01 100644 --- a/lib/api/api.rb +++ b/lib/api/api.rb @@ -2,6 +2,8 @@ module API class API < Grape::API include APIGuard + allow_access_with_scope :api + version %w(v3 v4), using: :path version 'v3', using: :path do @@ -44,7 +46,6 @@ module API mount ::API::V3::Variables end - before { allow_access_with_scope :api } before { header['X-Frame-Options'] = 'SAMEORIGIN' } before { Gitlab::I18n.locale = current_user&.preferred_language } diff --git a/lib/api/api_guard.rb b/lib/api/api_guard.rb index 9fcf04efa38aa69cab202141d20af935de943f5f..9a9e32a02423b40db3bf200c28b8bdbc6f092cdc 100644 --- a/lib/api/api_guard.rb +++ b/lib/api/api_guard.rb @@ -23,6 +23,27 @@ module API install_error_responders(base) end + class_methods do + # Set the authorization scope(s) allowed for the current request. + # + # 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. + def allow_access_with_scope(scopes, options = {}) + @scopes ||= [] + + params = Array.wrap(scopes).map { |scope| { name: scope, if: options[:if] } } + + @scopes.concat(params) + end + + def scopes + @scopes + end + end + # Helper Methods for Grape Endpoint module HelperMethods # Invokes the doorkeeper guard. @@ -74,18 +95,6 @@ module API @current_user end - # Set the authorization scope(s) allowed for the current request. - # - # Note: A call to this method adds to any previous scopes in place. This is done because - # `Grape` callbacks run from the outside-in: the top-level callback (API::API) runs first, then - # the next-level callback (API::API::Users, for example) runs. All these scopes are valid for the - # given endpoint (GET `/api/users` is accessible by the `api` and `read_user` scopes), and so they - # need to be stored. - def allow_access_with_scope(*scopes) - @scopes ||= [] - @scopes.concat(scopes.map(&:to_s)) - end - private def find_user_by_authentication_token(token_string) diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 2c73a6fdc4ecf1813be6d53f3fac1595a526b61a..3cf04e6df3cafaaff41865cde3ca3a18a0d89053 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -340,10 +340,12 @@ module API end def initial_current_user + endpoint_class = options[:for] + 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) - @initial_current_user ||= doorkeeper_guard(scopes: @scopes) + @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_from_warden unless @initial_current_user && Gitlab::UserAccess.new(@initial_current_user).allowed? diff --git a/lib/api/users.rb b/lib/api/users.rb index f9555842daf9e317ec1ccccccb071e7b2ed8ff67..2cac8c089f215d775eee4ce826ff01fb8b747ab5 100644 --- a/lib/api/users.rb +++ b/lib/api/users.rb @@ -1,9 +1,11 @@ module API class Users < Grape::API include PaginationParams + include APIGuard + + allow_access_with_scope :read_user, if: -> (request) { request.get? } before do - allow_access_with_scope :read_user if request.get? authenticate! end diff --git a/lib/api/v3/users.rb b/lib/api/v3/users.rb index 37020019e07b4d185bcf66f32da5b37eccaed751..cf106f2552d8edc123bab908aa310cdf5c9826ee 100644 --- a/lib/api/v3/users.rb +++ b/lib/api/v3/users.rb @@ -2,9 +2,11 @@ module API module V3 class Users < Grape::API include PaginationParams + include APIGuard + + allow_access_with_scope :read_user, if: -> (request) { request.get? } before do - allow_access_with_scope :read_user if request.get? authenticate! end diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb index c0174b304c84f0b9ce72fc2d1dd02bb313693a7d..c8e22799ba4c8a460a938ba3eb71540ed4dc6215 100644 --- a/spec/requests/api/users_spec.rb +++ b/spec/requests/api/users_spec.rb @@ -50,6 +50,28 @@ describe API::Users do end['username']).to eq(username) end + context "scopes" do + context 'when the requesting token has the "read_user" scope' do + let(:token) { create(:personal_access_token, scopes: ['read_user']) } + + it 'returns a "200" response' do + get api("/users", user, personal_access_token: token) + + expect(response).to have_http_status(200) + end + end + + context 'when the requesting token does not have any required scope' do + let(:token) { create(:personal_access_token, scopes: ['read_registry']) } + + it 'returns a "401" response' do + get api("/users", user, personal_access_token: token) + + expect(response).to have_http_status(401) + end + end + end + it "returns an array of blocked users" do ldap_blocked_user create(:user, state: 'blocked') diff --git a/spec/support/api_helpers.rb b/spec/support/api_helpers.rb index 35d1e1cfc7dbb95c4b687b7d07feb9ee288716bb..163979a2a2858c1a4bc16d0f848400b1569125a0 100644 --- a/spec/support/api_helpers.rb +++ b/spec/support/api_helpers.rb @@ -17,14 +17,16 @@ module ApiHelpers # => "/api/v2/issues?foo=bar&private_token=..." # # Returns the relative path to the requested API resource - def api(path, user = nil, version: API::API.version) + def api(path, user = nil, version: API::API.version, personal_access_token: nil) "/api/#{version}#{path}" + # Normalize query string (path.index('?') ? '' : '?') + + if personal_access_token.present? + "&private_token=#{personal_access_token.token}" # Append private_token if given a User object - if user.respond_to?(:private_token) + elsif user.respond_to?(:private_token) "&private_token=#{user.private_token}" else ''