Commit b303948f authored by Timothy Andrew's avatar Timothy Andrew

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
parent f706a973
module AccessTokenValidationService AccessTokenValidationService = Struct.new(:token) do
# Results: # Results:
VALID = :valid VALID = :valid
EXPIRED = :expired EXPIRED = :expired
REVOKED = :revoked REVOKED = :revoked
INSUFFICIENT_SCOPE = :insufficient_scope INSUFFICIENT_SCOPE = :insufficient_scope
class << self def validate(scopes: [])
def validate(token, scopes: [])
if token.expired? if token.expired?
return EXPIRED return EXPIRED
elsif token.revoked? elsif token.revoked?
return REVOKED return REVOKED
elsif !self.sufficient_scope?(token, scopes) elsif !self.include_any_scope?(scopes)
return INSUFFICIENT_SCOPE return INSUFFICIENT_SCOPE
else else
...@@ -21,14 +20,13 @@ module AccessTokenValidationService ...@@ -21,14 +20,13 @@ module AccessTokenValidationService
end end
end end
# True if the token's scope contains any of the required scopes. # True if the token's scope contains any of the passed scopes.
def sufficient_scope?(token, required_scopes) def include_any_scope?(scopes)
if required_scopes.blank? if scopes.blank?
true true
else else
# Check whether the token is allowed access to any of the required scopes. # Check whether the token is allowed access to any of the required scopes.
Set.new(required_scopes).intersection(Set.new(token.scopes)).present? Set.new(scopes).intersection(Set.new(token.scopes)).present?
end
end end
end end
end end
...@@ -47,7 +47,7 @@ module API ...@@ -47,7 +47,7 @@ module API
access_token = find_access_token access_token = find_access_token
return nil unless 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 when AccessTokenValidationService::INSUFFICIENT_SCOPE
raise InsufficientScopeError.new(scopes) raise InsufficientScopeError.new(scopes)
...@@ -96,7 +96,7 @@ module API ...@@ -96,7 +96,7 @@ module API
access_token = PersonalAccessToken.active.find_by_token(token_string) access_token = PersonalAccessToken.active.find_by_token(token_string)
return unless access_token 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) User.find(access_token.user_id)
end end
end end
......
...@@ -119,7 +119,7 @@ module Gitlab ...@@ -119,7 +119,7 @@ module Gitlab
end end
def token_has_scope?(token) def token_has_scope?(token)
AccessTokenValidationService.sufficient_scope?(token, ['api']) AccessTokenValidationService.new(token).include_any_scope?(['api'])
end end
def lfs_token_check(login, password) def lfs_token_check(login, password)
......
require 'spec_helper' require 'spec_helper'
describe AccessTokenValidationService, services: true do 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 it "returns true if the required scope is present in the token's scopes" do
token = double("token", scopes: [:api, :read_user]) 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 end
it "returns true if more than one of the required scopes is present in the token's scopes" do 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]) 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 end
it "returns true if the list of required scopes is an exact match for the token's scopes" do 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]) 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 end
it "returns true if the list of required scopes contains all of the token's scopes, in addition to others" do 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]) 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 end
it 'returns true if the list of required scopes is blank' do it 'returns true if the list of required scopes is blank' do
token = double("token", scopes: []) 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 end
it "returns false if there are no scopes in common between the required scopes and the token scopes" 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]) 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 end
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment