diff options
| -rw-r--r-- | app/services/access_token_validation_service.rb | 5 | ||||
| -rw-r--r-- | lib/api/api.rb | 3 | ||||
| -rw-r--r-- | lib/api/api_guard.rb | 33 | ||||
| -rw-r--r-- | lib/api/helpers.rb | 6 | ||||
| -rw-r--r-- | lib/api/users.rb | 4 | ||||
| -rw-r--r-- | lib/api/v3/users.rb | 4 | ||||
| -rw-r--r-- | spec/requests/api/users_spec.rb | 22 | ||||
| -rw-r--r-- | spec/support/api_helpers.rb | 6 |
8 files changed, 63 insertions, 20 deletions
diff --git a/app/services/access_token_validation_service.rb b/app/services/access_token_validation_service.rb index b2a543daa00..f171f8194bd 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 d767af36e8e..efcf0976a81 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 9fcf04efa38..9a9e32a0242 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 2c73a6fdc4e..3cf04e6df3c 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 f9555842daf..2cac8c089f2 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 37020019e07..cf106f2552d 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 c0174b304c8..c8e22799ba4 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 35d1e1cfc7d..163979a2a28 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 '' |