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

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