diff options
-rw-r--r-- | app/services/access_token_validation_service.rb | 9 | ||||
-rw-r--r-- | lib/api/scope.rb | 2 | ||||
-rw-r--r-- | lib/gitlab/auth.rb | 1 | ||||
-rw-r--r-- | spec/services/access_token_validation_service_spec.rb | 12 |
4 files changed, 15 insertions, 9 deletions
diff --git a/app/services/access_token_validation_service.rb b/app/services/access_token_validation_service.rb index bf5aef0055e..9c00ea789ec 100644 --- a/app/services/access_token_validation_service.rb +++ b/app/services/access_token_validation_service.rb @@ -37,7 +37,14 @@ class AccessTokenValidationService # small number of records involved. # https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/12300/#note_33689006 token_scopes = token.scopes.map(&:to_sym) - required_scopes.any? { |scope| scope.sufficient?(token_scopes, request) } + + required_scopes.any? do |scope| + if scope.respond_to?(:sufficient?) + scope.sufficient?(token_scopes, request) + else + API::Scope.new(scope).sufficient?(token_scopes, request) + end + end end end end diff --git a/lib/api/scope.rb b/lib/api/scope.rb index c23846d1e7d..d5165b2e482 100644 --- a/lib/api/scope.rb +++ b/lib/api/scope.rb @@ -11,7 +11,7 @@ module API # Are the `scopes` passed in sufficient to adequately authorize the passed # request for the scope represented by the current instance of this class? def sufficient?(scopes, request) - verify_if_condition(request) && scopes.include?(self.name) + scopes.include?(self.name) && verify_if_condition(request) end private diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb index 6d0d638ba14..ccb5d886bab 100644 --- a/lib/gitlab/auth.rb +++ b/lib/gitlab/auth.rb @@ -140,7 +140,6 @@ module Gitlab end def valid_scoped_token?(token, scopes) - scopes = scopes.map { |scope| API::Scope.new(scope) } AccessTokenValidationService.new(token).include_any_scope?(scopes) end diff --git a/spec/services/access_token_validation_service_spec.rb b/spec/services/access_token_validation_service_spec.rb index 660a05e0b6d..11225fad18a 100644 --- a/spec/services/access_token_validation_service_spec.rb +++ b/spec/services/access_token_validation_service_spec.rb @@ -6,28 +6,28 @@ 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]) - scopes = [API::Scope.new(:api)] + scopes = [:api] expect(described_class.new(token, request: request).include_any_scope?(scopes)).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]) - scopes = [API::Scope.new(:api), API::Scope.new(:other_scope)] + scopes = [:api, :other_scope] expect(described_class.new(token, request: request).include_any_scope?(scopes)).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]) - scopes = [API::Scope.new(:api), API::Scope.new(:read_user), API::Scope.new(:other_scope)] + scopes = [:api, :read_user, :other_scope] expect(described_class.new(token, request: request).include_any_scope?(scopes)).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]) - scopes = [API::Scope.new(:api), API::Scope.new(:read_user), API::Scope.new(:other_scope)] + scopes = [:api, :read_user, :other_scope] expect(described_class.new(token, request: request).include_any_scope?(scopes)).to be(true) end @@ -41,7 +41,7 @@ describe AccessTokenValidationService, services: true do 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]) - scopes = [API::Scope.new(:other_scope)] + scopes = [:other_scope] expect(described_class.new(token, request: request).include_any_scope?(scopes)).to be(false) end @@ -56,7 +56,7 @@ describe AccessTokenValidationService, services: true do it "does not ignore scopes whose `if` condition is not set" do token = double("token", scopes: [:api, :read_user]) - scopes = [API::Scope.new(:api, if: ->(_) { false }), API::Scope.new(:read_user)] + scopes = [API::Scope.new(:api, if: ->(_) { false }), :read_user] expect(described_class.new(token, request: request).include_any_scope?(scopes)).to be(true) end |