Commit 184b923f authored by Timothy Andrew's avatar Timothy Andrew

Calls to the API are checked for scope.

- Move the `Oauth2::AccessTokenValidationService` class to
  `AccessTokenValidationService`, since it is now being used for
  personal access token validation as well.

- Each API endpoint declares the scopes it accepts (if any). Currently,
  the top level API module declares the `api` scope, and the `Users` API
  module declares the `read_user` scope (for GET requests).

- Move the `find_user_by_private_token` from the API `Helpers` module to
  the `APIGuard` module, to avoid littering `Helpers` with more
  auth-related methods to support `find_user_by_private_token`
parent 4e01306d
module Oauth2::AccessTokenValidationService module AccessTokenValidationService
# Results: # Results:
VALID = :valid VALID = :valid
EXPIRED = :expired EXPIRED = :expired
...@@ -21,21 +21,13 @@ module Oauth2::AccessTokenValidationService ...@@ -21,21 +21,13 @@ module Oauth2::AccessTokenValidationService
end end
end end
protected # True if the token's scope contains any of the required scopes.
def sufficient_scope?(token, required_scopes)
# True if the token's scope is a superset of required scopes, if required_scopes.blank?
# or the required scopes is empty. true
def sufficient_scope?(token, scopes)
if scopes.blank?
# if no any scopes required, the scopes of token is sufficient.
return true
else else
# If there are scopes required, then check whether # Check whether the token is allowed access to any of the required scopes.
# the set of authorized scopes is a superset of the set of required scopes Set.new(required_scopes).intersection(Set.new(token.scopes)).present?
required_scopes = Set.new(scopes)
authorized_scopes = Set.new(token.scopes)
return authorized_scopes >= required_scopes
end end
end end
end end
......
...@@ -52,8 +52,8 @@ Doorkeeper.configure do ...@@ -52,8 +52,8 @@ Doorkeeper.configure do
# Define access token scopes for your provider # Define access token scopes for your provider
# For more information go to # For more information go to
# https://github.com/doorkeeper-gem/doorkeeper/wiki/Using-Scopes # https://github.com/doorkeeper-gem/doorkeeper/wiki/Using-Scopes
default_scopes :api default_scopes(*Gitlab::Auth::DEFAULT_SCOPES)
# optional_scopes :write, :update optional_scopes(*Gitlab::Auth::OPTIONAL_SCOPES)
# Change the way client credentials are retrieved from the request object. # Change the way client credentials are retrieved from the request object.
# By default it retrieves first from the `HTTP_AUTHORIZATION` header, then # By default it retrieves first from the `HTTP_AUTHORIZATION` header, then
......
...@@ -59,6 +59,7 @@ en: ...@@ -59,6 +59,7 @@ en:
unknown: "The access token is invalid" unknown: "The access token is invalid"
scopes: scopes:
api: Access your API api: Access your API
read_user: Read user information
flash: flash:
applications: applications:
......
...@@ -3,6 +3,8 @@ module API ...@@ -3,6 +3,8 @@ module API
include APIGuard include APIGuard
version 'v3', using: :path version 'v3', using: :path
before { allow_access_with_scope :api }
rescue_from Gitlab::Access::AccessDeniedError do rescue_from Gitlab::Access::AccessDeniedError do
rack_response({ 'message' => '403 Forbidden' }.to_json, 403) rack_response({ 'message' => '403 Forbidden' }.to_json, 403)
end end
......
...@@ -6,6 +6,9 @@ module API ...@@ -6,6 +6,9 @@ module API
module APIGuard module APIGuard
extend ActiveSupport::Concern extend ActiveSupport::Concern
PRIVATE_TOKEN_HEADER = "HTTP_PRIVATE_TOKEN"
PRIVATE_TOKEN_PARAM = :private_token
included do |base| included do |base|
# OAuth2 Resource Server Authentication # OAuth2 Resource Server Authentication
use Rack::OAuth2::Server::Resource::Bearer, 'The API' do |request| use Rack::OAuth2::Server::Resource::Bearer, 'The API' do |request|
...@@ -41,30 +44,59 @@ module API ...@@ -41,30 +44,59 @@ module API
# Defaults to empty array. # Defaults to empty array.
# #
def doorkeeper_guard(scopes: []) def doorkeeper_guard(scopes: [])
access_token = find_access_token if access_token = find_access_token
return nil unless access_token case AccessTokenValidationService.validate(access_token, scopes: scopes)
when AccessTokenValidationService::INSUFFICIENT_SCOPE
case validate_access_token(access_token, scopes) raise InsufficientScopeError.new(scopes)
when Oauth2::AccessTokenValidationService::INSUFFICIENT_SCOPE when AccessTokenValidationService::EXPIRED
raise InsufficientScopeError.new(scopes) raise ExpiredError
when AccessTokenValidationService::REVOKED
raise RevokedError
when AccessTokenValidationService::VALID
@current_user = User.find(access_token.resource_owner_id)
end
end
end
when Oauth2::AccessTokenValidationService::EXPIRED def find_user_by_private_token(scopes: [])
raise ExpiredError token_string = (params[PRIVATE_TOKEN_PARAM] || env[PRIVATE_TOKEN_HEADER]).to_s
when Oauth2::AccessTokenValidationService::REVOKED return nil unless token_string.present?
raise RevokedError
when Oauth2::AccessTokenValidationService::VALID find_user_by_authentication_token(token_string) || find_user_by_personal_access_token(token_string, scopes)
@current_user = User.find(access_token.resource_owner_id)
end
end end
def current_user def current_user
@current_user @current_user
end end
# Set the authorization scope(s) allowed for the current request.
#
# Note: A call to this method adds to any previous scopes in place. This is done because
# `Grape` callbacks run from the outside-in: the top-level callback (API::API) runs first, then
# the next-level callback (API::API::Users, for example) runs. All these scopes are valid for the
# given endpoint (GET `/api/users` is accessible by the `api` and `read_user` scopes), and so they
# need to be stored.
def allow_access_with_scope(*scopes)
@scopes ||= []
@scopes.concat(scopes.map(&:to_s))
end
private private
def find_user_by_authentication_token(token_string)
User.find_by_authentication_token(token_string)
end
def find_user_by_personal_access_token(token_string, scopes)
access_token = PersonalAccessToken.active.find_by_token(token_string)
return unless access_token
if AccessTokenValidationService.sufficient_scope?(access_token, scopes)
User.find(access_token.user_id)
end
end
def find_access_token def find_access_token
@access_token ||= Doorkeeper.authenticate(doorkeeper_request, Doorkeeper.configuration.access_token_methods) @access_token ||= Doorkeeper.authenticate(doorkeeper_request, Doorkeeper.configuration.access_token_methods)
end end
...@@ -72,10 +104,6 @@ module API ...@@ -72,10 +104,6 @@ module API
def doorkeeper_request def doorkeeper_request
@doorkeeper_request ||= ActionDispatch::Request.new(env) @doorkeeper_request ||= ActionDispatch::Request.new(env)
end end
def validate_access_token(access_token, scopes)
Oauth2::AccessTokenValidationService.validate(access_token, scopes: scopes)
end
end end
module ClassMethods module ClassMethods
......
...@@ -2,8 +2,6 @@ module API ...@@ -2,8 +2,6 @@ module API
module Helpers module Helpers
include Gitlab::Utils include Gitlab::Utils
PRIVATE_TOKEN_HEADER = "HTTP_PRIVATE_TOKEN"
PRIVATE_TOKEN_PARAM = :private_token
SUDO_HEADER = "HTTP_SUDO" SUDO_HEADER = "HTTP_SUDO"
SUDO_PARAM = :sudo SUDO_PARAM = :sudo
...@@ -322,7 +320,7 @@ module API ...@@ -322,7 +320,7 @@ module API
private private
def private_token def private_token
params[PRIVATE_TOKEN_PARAM] || env[PRIVATE_TOKEN_HEADER] params[APIGuard::PRIVATE_TOKEN_PARAM] || env[APIGuard::PRIVATE_TOKEN_HEADER]
end end
def warden def warden
...@@ -337,18 +335,11 @@ module API ...@@ -337,18 +335,11 @@ module API
warden.try(:authenticate) if %w[GET HEAD].include?(env['REQUEST_METHOD']) warden.try(:authenticate) if %w[GET HEAD].include?(env['REQUEST_METHOD'])
end end
def find_user_by_private_token
token = private_token
return nil unless token.present?
User.find_by_authentication_token(token) || User.find_by_personal_access_token(token)
end
def initial_current_user def initial_current_user
return @initial_current_user if defined?(@initial_current_user) return @initial_current_user if defined?(@initial_current_user)
@initial_current_user ||= find_user_by_private_token @initial_current_user ||= find_user_by_private_token(scopes: @scopes)
@initial_current_user ||= doorkeeper_guard @initial_current_user ||= doorkeeper_guard(scopes: @scopes)
@initial_current_user ||= find_user_from_warden @initial_current_user ||= find_user_from_warden
unless @initial_current_user && Gitlab::UserAccess.new(@initial_current_user).allowed? unless @initial_current_user && Gitlab::UserAccess.new(@initial_current_user).allowed?
......
...@@ -2,7 +2,10 @@ module API ...@@ -2,7 +2,10 @@ module API
class Users < Grape::API class Users < Grape::API
include PaginationParams include PaginationParams
before { authenticate! } before do
allow_access_with_scope :read_user if request.get?
authenticate!
end
resource :users, requirements: { uid: /[0-9]*/, id: /[0-9]*/ } do resource :users, requirements: { uid: /[0-9]*/, id: /[0-9]*/ } do
helpers do helpers do
......
...@@ -2,6 +2,10 @@ module Gitlab ...@@ -2,6 +2,10 @@ module Gitlab
module Auth module Auth
class MissingPersonalTokenError < StandardError; end class MissingPersonalTokenError < StandardError; end
SCOPES = [:api, :read_user]
DEFAULT_SCOPES = [:api]
OPTIONAL_SCOPES = SCOPES - DEFAULT_SCOPES
class << self class << self
def find_for_git_client(login, password, project:, ip:) def find_for_git_client(login, password, project:, ip:)
raise "Must provide an IP for rate limiting" if ip.nil? raise "Must provide an IP for rate limiting" if ip.nil?
......
...@@ -5,7 +5,7 @@ describe API::API, api: true do ...@@ -5,7 +5,7 @@ describe API::API, api: true do
let!(:user) { create(:user) } let!(:user) { create(:user) }
let!(:application) { Doorkeeper::Application.create!(name: "MyApp", redirect_uri: "https://app.com", owner: user) } let!(:application) { Doorkeeper::Application.create!(name: "MyApp", redirect_uri: "https://app.com", owner: user) }
let!(:token) { Doorkeeper::AccessToken.create! application_id: application.id, resource_owner_id: user.id } let!(:token) { Doorkeeper::AccessToken.create! application_id: application.id, resource_owner_id: user.id, scopes: "api" }
describe "when unauthenticated" do describe "when unauthenticated" do
it "returns authentication success" do it "returns authentication success" do
......
require 'spec_helper' require 'spec_helper'
describe API::Helpers, api: true do describe API::Helpers, api: true do
include API::APIGuard::HelperMethods
include API::Helpers include API::Helpers
include SentryHelper include SentryHelper
...@@ -15,24 +16,24 @@ describe API::Helpers, api: true do ...@@ -15,24 +16,24 @@ describe API::Helpers, api: true do
def set_env(user_or_token, identifier) def set_env(user_or_token, identifier)
clear_env clear_env
clear_param clear_param
env[API::Helpers::PRIVATE_TOKEN_HEADER] = user_or_token.respond_to?(:private_token) ? user_or_token.private_token : user_or_token env[API::APIGuard::PRIVATE_TOKEN_HEADER] = user_or_token.respond_to?(:private_token) ? user_or_token.private_token : user_or_token
env[API::Helpers::SUDO_HEADER] = identifier env[API::Helpers::SUDO_HEADER] = identifier
end end
def set_param(user_or_token, identifier) def set_param(user_or_token, identifier)
clear_env clear_env
clear_param clear_param
params[API::Helpers::PRIVATE_TOKEN_PARAM] = user_or_token.respond_to?(:private_token) ? user_or_token.private_token : user_or_token params[API::APIGuard::PRIVATE_TOKEN_PARAM] = user_or_token.respond_to?(:private_token) ? user_or_token.private_token : user_or_token
params[API::Helpers::SUDO_PARAM] = identifier params[API::Helpers::SUDO_PARAM] = identifier
end end
def clear_env def clear_env
env.delete(API::Helpers::PRIVATE_TOKEN_HEADER) env.delete(API::APIGuard::PRIVATE_TOKEN_HEADER)
env.delete(API::Helpers::SUDO_HEADER) env.delete(API::Helpers::SUDO_HEADER)
end end
def clear_param def clear_param
params.delete(API::Helpers::PRIVATE_TOKEN_PARAM) params.delete(API::APIGuard::PRIVATE_TOKEN_PARAM)
params.delete(API::Helpers::SUDO_PARAM) params.delete(API::Helpers::SUDO_PARAM)
end end
...@@ -94,22 +95,22 @@ describe API::Helpers, api: true do ...@@ -94,22 +95,22 @@ describe API::Helpers, api: true do
describe "when authenticating using a user's private token" do describe "when authenticating using a user's private token" do
it "returns nil for an invalid token" do it "returns nil for an invalid token" do
env[API::Helpers::PRIVATE_TOKEN_HEADER] = 'invalid token' env[API::APIGuard::PRIVATE_TOKEN_HEADER] = 'invalid token'
allow_any_instance_of(self.class).to receive(:doorkeeper_guard){ false } allow_any_instance_of(self.class).to receive(:doorkeeper_guard){ false }
expect(current_user).to be_nil expect(current_user).to be_nil
end end
it "returns nil for a user without access" do it "returns nil for a user without access" do
env[API::Helpers::PRIVATE_TOKEN_HEADER] = user.private_token env[API::APIGuard::PRIVATE_TOKEN_HEADER] = user.private_token
allow_any_instance_of(Gitlab::UserAccess).to receive(:allowed?).and_return(false) allow_any_instance_of(Gitlab::UserAccess).to receive(:allowed?).and_return(false)
expect(current_user).to be_nil expect(current_user).to be_nil
end end
it "leaves user as is when sudo not specified" do it "leaves user as is when sudo not specified" do
env[API::Helpers::PRIVATE_TOKEN_HEADER] = user.private_token env[API::APIGuard::PRIVATE_TOKEN_HEADER] = user.private_token
expect(current_user).to eq(user) expect(current_user).to eq(user)
clear_env clear_env
params[API::Helpers::PRIVATE_TOKEN_PARAM] = user.private_token params[API::APIGuard::PRIVATE_TOKEN_PARAM] = user.private_token
expect(current_user).to eq(user) expect(current_user).to eq(user)
end end
end end
...@@ -117,37 +118,45 @@ describe API::Helpers, api: true do ...@@ -117,37 +118,45 @@ describe API::Helpers, api: true do
describe "when authenticating using a user's personal access tokens" do describe "when authenticating using a user's personal access tokens" do
let(:personal_access_token) { create(:personal_access_token, user: user) } let(:personal_access_token) { create(:personal_access_token, user: user) }
before do
allow_any_instance_of(self.class).to receive(:doorkeeper_guard) { false }
end
it "returns nil for an invalid token" do it "returns nil for an invalid token" do
env[API::Helpers::PRIVATE_TOKEN_HEADER] = 'invalid token' env[API::APIGuard::PRIVATE_TOKEN_HEADER] = 'invalid token'
allow_any_instance_of(self.class).to receive(:doorkeeper_guard){ false }
expect(current_user).to be_nil expect(current_user).to be_nil
end end
it "returns nil for a user without access" do it "returns nil for a user without access" do
env[API::Helpers::PRIVATE_TOKEN_HEADER] = personal_access_token.token env[API::APIGuard::PRIVATE_TOKEN_HEADER] = personal_access_token.token
allow_any_instance_of(Gitlab::UserAccess).to receive(:allowed?).and_return(false) allow_any_instance_of(Gitlab::UserAccess).to receive(:allowed?).and_return(false)
expect(current_user).to be_nil expect(current_user).to be_nil
end end
it "returns nil for a token without the appropriate scope" do
personal_access_token = create(:personal_access_token, user: user, scopes: ['read_user'])
env[API::APIGuard::PRIVATE_TOKEN_HEADER] = personal_access_token.token
allow_access_with_scope('write_user')
expect(current_user).to be_nil
end
it "leaves user as is when sudo not specified" do it "leaves user as is when sudo not specified" do
env[API::Helpers::PRIVATE_TOKEN_HEADER] = personal_access_token.token env[API::APIGuard::PRIVATE_TOKEN_HEADER] = personal_access_token.token
expect(current_user).to eq(user) expect(current_user).to eq(user)
clear_env clear_env
params[API::Helpers::PRIVATE_TOKEN_PARAM] = personal_access_token.token params[API::APIGuard::PRIVATE_TOKEN_PARAM] = personal_access_token.token
expect(current_user).to eq(user) expect(current_user).to eq(user)
end end
it 'does not allow revoked tokens' do it 'does not allow revoked tokens' do
personal_access_token.revoke! personal_access_token.revoke!
env[API::Helpers::PRIVATE_TOKEN_HEADER] = personal_access_token.token env[API::APIGuard::PRIVATE_TOKEN_HEADER] = personal_access_token.token
allow_any_instance_of(self.class).to receive(:doorkeeper_guard){ false }
expect(current_user).to be_nil expect(current_user).to be_nil
end end
it 'does not allow expired tokens' do it 'does not allow expired tokens' do
personal_access_token.update_attributes!(expires_at: 1.day.ago) personal_access_token.update_attributes!(expires_at: 1.day.ago)
env[API::Helpers::PRIVATE_TOKEN_HEADER] = personal_access_token.token env[API::APIGuard::PRIVATE_TOKEN_HEADER] = personal_access_token.token
allow_any_instance_of(self.class).to receive(:doorkeeper_guard){ false }
expect(current_user).to be_nil expect(current_user).to be_nil
end end
end end
......
require 'spec_helper'
describe AccessTokenValidationService, services: true do
describe ".sufficient_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)
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)
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)
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)
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)
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)
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