diff options
author | Timothy Andrew <mail@timothyandrew.net> | 2016-12-05 20:25:53 +0300 |
---|---|---|
committer | Timothy Andrew <mail@timothyandrew.net> | 2016-12-16 13:59:32 +0300 |
commit | b303948ff549ce57d3b6985c2c366dfcdc5a2ca3 (patch) | |
tree | 3d286b8704e63cf8c26b10a1f0c538d77f24ab6b | |
parent | f706a973c26f9de9a1f1599d532b33e9e66a80bb (diff) |
Convert AccessTokenValidationService into a class.
- Previously, AccessTokenValidationService was a module, and all its public
methods accepted a token. It makes sense to convert it to a class which accepts
a token during initialization.
- Also rename the `sufficient_scope?` method to `include_any_scope?`
- Based on feedback from @rymai
-rw-r--r-- | app/services/access_token_validation_service.rb | 38 | ||||
-rw-r--r-- | lib/api/api_guard.rb | 4 | ||||
-rw-r--r-- | lib/gitlab/auth.rb | 2 | ||||
-rw-r--r-- | spec/services/access_token_validation_service_spec.rb | 14 |
4 files changed, 28 insertions, 30 deletions
diff --git a/app/services/access_token_validation_service.rb b/app/services/access_token_validation_service.rb index 69449f3a445..ddaaed90e5b 100644 --- a/app/services/access_token_validation_service.rb +++ b/app/services/access_token_validation_service.rb @@ -1,34 +1,32 @@ -module AccessTokenValidationService +AccessTokenValidationService = Struct.new(:token) do # Results: VALID = :valid EXPIRED = :expired REVOKED = :revoked INSUFFICIENT_SCOPE = :insufficient_scope - class << self - def validate(token, scopes: []) - if token.expired? - return EXPIRED + def validate(scopes: []) + if token.expired? + return EXPIRED - elsif token.revoked? - return REVOKED + elsif token.revoked? + return REVOKED - elsif !self.sufficient_scope?(token, scopes) - return INSUFFICIENT_SCOPE + elsif !self.include_any_scope?(scopes) + return INSUFFICIENT_SCOPE - else - return VALID - end + else + return VALID end + end - # True if the token's scope contains any of the required scopes. - def sufficient_scope?(token, required_scopes) - if required_scopes.blank? - true - else - # Check whether the token is allowed access to any of the required scopes. - Set.new(required_scopes).intersection(Set.new(token.scopes)).present? - end + # True if the token's scope contains any of the passed scopes. + def include_any_scope?(scopes) + if scopes.blank? + true + else + # Check whether the token is allowed access to any of the required scopes. + Set.new(scopes).intersection(Set.new(token.scopes)).present? end end end diff --git a/lib/api/api_guard.rb b/lib/api/api_guard.rb index 563224a580f..df6db140d0e 100644 --- a/lib/api/api_guard.rb +++ b/lib/api/api_guard.rb @@ -47,7 +47,7 @@ module API access_token = find_access_token return nil unless access_token - case AccessTokenValidationService.validate(access_token, scopes: scopes) + case AccessTokenValidationService.new(access_token).validate(scopes: scopes) when AccessTokenValidationService::INSUFFICIENT_SCOPE raise InsufficientScopeError.new(scopes) @@ -96,7 +96,7 @@ module API access_token = PersonalAccessToken.active.find_by_token(token_string) return unless access_token - if AccessTokenValidationService.sufficient_scope?(access_token, scopes) + if AccessTokenValidationService.new(access_token).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 c21afaa1551..2879a4d2f5d 100644 --- a/lib/gitlab/auth.rb +++ b/lib/gitlab/auth.rb @@ -119,7 +119,7 @@ module Gitlab end def token_has_scope?(token) - AccessTokenValidationService.sufficient_scope?(token, ['api']) + AccessTokenValidationService.new(token).include_any_scope?(['api']) end def lfs_token_check(login, password) diff --git a/spec/services/access_token_validation_service_spec.rb b/spec/services/access_token_validation_service_spec.rb index 332e745aa36..87f093ee8ce 100644 --- a/spec/services/access_token_validation_service_spec.rb +++ b/spec/services/access_token_validation_service_spec.rb @@ -1,41 +1,41 @@ require 'spec_helper' describe AccessTokenValidationService, services: true do - describe ".sufficient_scope?" do + describe ".include_any_scope?" 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.sufficient_scope?(token, [:api])).to be(true) + expect(described_class.new(token).include_any_scope?([: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.sufficient_scope?(token, [:api, :other_scope])).to be(true) + expect(described_class.new(token).include_any_scope?([:api, :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.sufficient_scope?(token, [:api, :read_user, :other_scope])).to be(true) + expect(described_class.new(token).include_any_scope?([:api, :read_user, :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.sufficient_scope?(token, [:api, :read_user, :other_scope])).to be(true) + expect(described_class.new(token).include_any_scope?([:api, :read_user, :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.sufficient_scope?(token, [])).to be(true) + expect(described_class.new(token).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.sufficient_scope?(token, [:other_scope])).to be(false) + expect(described_class.new(token).include_any_scope?([:other_scope])).to be(false) end end end |