diff --git a/app/services/access_token_validation_service.rb b/app/services/access_token_validation_service.rb index 6d39ad245d2..450e90d947d 100644 --- a/app/services/access_token_validation_service.rb +++ b/app/services/access_token_validation_service.rb @@ -7,7 +7,7 @@ class AccessTokenValidationService attr_reader :token, :request - def initialize(token, request) + def initialize(token, request: nil) @token = token @request = request end diff --git a/lib/api/api_guard.rb b/lib/api/api_guard.rb index 29ca760ec25..d4599aaeed0 100644 --- a/lib/api/api_guard.rb +++ b/lib/api/api_guard.rb @@ -66,7 +66,7 @@ module API access_token = find_access_token return nil unless access_token - case AccessTokenValidationService.new(access_token, request).validate(scopes: scopes) + case AccessTokenValidationService.new(access_token, request: request).validate(scopes: scopes) when AccessTokenValidationService::INSUFFICIENT_SCOPE raise InsufficientScopeError.new(scopes) @@ -103,7 +103,7 @@ module API access_token = PersonalAccessToken.active.find_by_token(token_string) return unless access_token - if AccessTokenValidationService.new(access_token, request).include_any_scope?(scopes) + if AccessTokenValidationService.new(access_token, request: request).include_any_scope?(scopes) User.find(access_token.user_id) end end diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb index 3933c3b04dd..37ac8ecc2f0 100644 --- a/lib/gitlab/auth.rb +++ b/lib/gitlab/auth.rb @@ -130,13 +130,13 @@ module Gitlab token = PersonalAccessTokensFinder.new(state: 'active').find_by(token: password) - if token && valid_scoped_token?(token, AVAILABLE_SCOPES.map(&:to_s)) + if token && valid_scoped_token?(token, AVAILABLE_SCOPES.map { |scope| { name: scope.to_s }}) Gitlab::Auth::Result.new(token.user, nil, :personal_token, abilities_for_scope(token.scopes)) end end def valid_oauth_token?(token) - token && token.accessible? && valid_scoped_token?(token, ["api"]) + token && token.accessible? && valid_scoped_token?(token, [{ name: "api" }]) end def valid_scoped_token?(token, scopes) diff --git a/spec/requests/api/v3/users_spec.rb b/spec/requests/api/v3/users_spec.rb index b2c5003c97a..de7499a4e43 100644 --- a/spec/requests/api/v3/users_spec.rb +++ b/spec/requests/api/v3/users_spec.rb @@ -300,7 +300,7 @@ describe API::V3::Users do end it 'returns a 404 error if not found' do - get v3_api('/users/42/events', user) + get v3_api('/users/420/events', user) expect(response).to have_http_status(404) expect(json_response['message']).to eq('404 User Not Found') diff --git a/spec/services/access_token_validation_service_spec.rb b/spec/services/access_token_validation_service_spec.rb index 0023678dc3b..eff4269a4d5 100644 --- a/spec/services/access_token_validation_service_spec.rb +++ b/spec/services/access_token_validation_service_spec.rb @@ -7,37 +7,37 @@ describe AccessTokenValidationService, services: true do it "returns true if the required scope is present in the token's scopes" do token = double("token", scopes: [:api, :read_user]) - expect(described_class.new(token, request).include_any_scope?([{ name: :api }])).to be(true) + expect(described_class.new(token, request: request).include_any_scope?([{ name: :api }])).to be(true) end it "returns true if more than one of the required scopes is present in the token's scopes" do token = double("token", scopes: [:api, :read_user, :other_scope]) - expect(described_class.new(token, request).include_any_scope?([{ name: :api }, { name: :other_scope }])).to be(true) + expect(described_class.new(token, request: request).include_any_scope?([{ name: :api }, { name: :other_scope }])).to be(true) end it "returns true if the list of required scopes is an exact match for the token's scopes" do token = double("token", scopes: [:api, :read_user, :other_scope]) - expect(described_class.new(token, request).include_any_scope?([{ name: :api }, { name: :read_user }, { name: :other_scope }])).to be(true) + expect(described_class.new(token, request: request).include_any_scope?([{ name: :api }, { name: :read_user }, { name: :other_scope }])).to be(true) end it "returns true if the list of required scopes contains all of the token's scopes, in addition to others" do token = double("token", scopes: [:api, :read_user]) - expect(described_class.new(token, request).include_any_scope?([{ name: :api }, { name: :read_user }, { name: :other_scope }])).to be(true) + expect(described_class.new(token, request: request).include_any_scope?([{ name: :api }, { name: :read_user }, { name: :other_scope }])).to be(true) end it 'returns true if the list of required scopes is blank' do token = double("token", scopes: []) - expect(described_class.new(token, request).include_any_scope?([])).to be(true) + expect(described_class.new(token, request: request).include_any_scope?([])).to be(true) end it "returns false if there are no scopes in common between the required scopes and the token scopes" do token = double("token", scopes: [:api, :read_user]) - expect(described_class.new(token, request).include_any_scope?([{ name: :other_scope }])).to be(false) + expect(described_class.new(token, request: request).include_any_scope?([{ name: :other_scope }])).to be(false) end context "conditions" do @@ -45,19 +45,19 @@ describe AccessTokenValidationService, services: true do it "ignores any scopes whose `if` condition returns false" do token = double("token", scopes: [:api, :read_user]) - expect(described_class.new(token, request).include_any_scope?([{ name: :api, if: ->(_) { false } }])).to be(false) + expect(described_class.new(token, request: request).include_any_scope?([{ name: :api, if: ->(_) { false } }])).to be(false) end it "does not ignore scopes whose `if` condition is not set" do token = double("token", scopes: [:api, :read_user]) - expect(described_class.new(token, request).include_any_scope?([{ name: :api, if: ->(_) { false } }, { name: :read_user }])).to be(true) + expect(described_class.new(token, request: request).include_any_scope?([{ name: :api, if: ->(_) { false } }, { name: :read_user }])).to be(true) end it "does not ignore scopes whose `if` condition returns true" do token = double("token", scopes: [:api, :read_user]) - expect(described_class.new(token, request).include_any_scope?([{ name: :api, if: ->(_) { true } }, { name: :read_user, if: ->(_) { false } }])).to be(true) + expect(described_class.new(token, request: request).include_any_scope?([{ name: :api, if: ->(_) { true } }, { name: :read_user, if: ->(_) { false } }])).to be(true) end end end diff --git a/spec/support/api/scopes/read_user_shared_examples.rb b/spec/support/api/scopes/read_user_shared_examples.rb index cae6099a0c2..3bd589d64b9 100644 --- a/spec/support/api/scopes/read_user_shared_examples.rb +++ b/spec/support/api/scopes/read_user_shared_examples.rb @@ -32,7 +32,6 @@ shared_examples_for 'allows the "read_user" scope' do end context 'for doorkeeper (OAuth) tokens' do - let!(:user) {create(:user)} let!(:application) { Doorkeeper::Application.create!(name: "MyApp", redirect_uri: "https://app.com", owner: user) } context 'when the requesting token has the "api" scope' do