summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDouwe Maan <douwe@selenight.nl>2017-09-27 15:56:48 +0200
committerDouwe Maan <douwe@selenight.nl>2017-10-12 11:13:37 +0200
commit025c6eeaa1e02dce31cb836c39ee4a5f312f202f (patch)
tree14c6de5fb8ab55694413a22cc0ba668ae3773f9d
parentad5b96952e9eb90dc72d640f01aca01b5d0a2a12 (diff)
downloadgitlab-ce-025c6eeaa1e02dce31cb836c39ee4a5f312f202f.tar.gz
Move all API authentication code to APIGuard
-rw-r--r--app/models/oauth_access_token.rb2
-rw-r--r--lib/api/api_guard.rb133
-rw-r--r--lib/api/helpers.rb52
-rw-r--r--spec/requests/api/helpers_spec.rb18
-rw-r--r--spec/support/api/scopes/read_user_shared_examples.rb8
5 files changed, 108 insertions, 105 deletions
diff --git a/app/models/oauth_access_token.rb b/app/models/oauth_access_token.rb
index b85f5dbaf2e..f89e60ad9f4 100644
--- a/app/models/oauth_access_token.rb
+++ b/app/models/oauth_access_token.rb
@@ -1,4 +1,6 @@
class OauthAccessToken < Doorkeeper::AccessToken
belongs_to :resource_owner, class_name: 'User'
belongs_to :application, class_name: 'Doorkeeper::Application'
+
+ alias_method :user, :resource_owner
end
diff --git a/lib/api/api_guard.rb b/lib/api/api_guard.rb
index e79f988f549..87b9db66efd 100644
--- a/lib/api/api_guard.rb
+++ b/lib/api/api_guard.rb
@@ -42,6 +42,38 @@ module API
# Helper Methods for Grape Endpoint
module HelperMethods
+ def find_current_user
+ user =
+ find_user_from_private_token ||
+ find_user_from_oauth_token ||
+ find_user_from_warden
+
+ return nil unless user
+
+ raise UnauthorizedError unless Gitlab::UserAccess.new(user).allowed? && user.can?(:access_api)
+
+ user
+ end
+
+ def private_token
+ params[PRIVATE_TOKEN_PARAM] || env[PRIVATE_TOKEN_HEADER]
+ end
+
+ private
+
+ def find_user_from_private_token
+ token_string = private_token.to_s
+ return nil unless token_string.present?
+
+ user =
+ find_user_by_authentication_token(token_string) ||
+ find_user_by_personal_access_token(token_string)
+
+ raise UnauthorizedError unless user
+
+ user
+ end
+
# Invokes the doorkeeper guard.
#
# If token is presented and valid, then it sets @current_user.
@@ -60,70 +92,89 @@ module API
# scopes: (optional) scopes required for this guard.
# Defaults to empty array.
#
- def doorkeeper_guard(scopes: [])
- access_token = find_access_token
- return nil unless access_token
-
- case AccessTokenValidationService.new(access_token, request: request).validate(scopes: scopes)
- when AccessTokenValidationService::INSUFFICIENT_SCOPE
- raise InsufficientScopeError.new(scopes)
-
- when AccessTokenValidationService::EXPIRED
- raise ExpiredError
+ def find_user_from_oauth_token
+ access_token = find_oauth_access_token
+ return unless access_token
- when AccessTokenValidationService::REVOKED
- raise RevokedError
+ find_user_by_access_token(access_token)
+ end
- when AccessTokenValidationService::VALID
- User.find(access_token.resource_owner_id)
- end
+ def find_user_by_authentication_token(token_string)
+ User.find_by_authentication_token(token_string)
end
- def find_user_by_private_token(scopes: [])
- token_string = (params[PRIVATE_TOKEN_PARAM] || env[PRIVATE_TOKEN_HEADER]).to_s
+ def find_user_by_personal_access_token(token_string)
+ access_token = PersonalAccessToken.find_by_token(token_string)
+ return unless access_token
- return nil unless token_string.present?
+ find_user_by_access_token(access_token)
+ end
- user =
- find_user_by_authentication_token(token_string) ||
- find_user_by_personal_access_token(token_string, scopes)
+ # Check the Rails session for valid authentication details
+ def find_user_from_warden
+ warden.try(:authenticate) if verified_request?
+ end
- raise UnauthorizedError unless user
+ def warden
+ env['warden']
+ end
- user
+ # Check if the request is GET/HEAD, or if CSRF token is valid.
+ def verified_request?
+ Gitlab::RequestForgeryProtection.verified?(env)
end
- private
+ def find_oauth_access_token
+ return @oauth_access_token if defined?(@oauth_access_token)
- def find_user_by_authentication_token(token_string)
- User.find_by_authentication_token(token_string)
- end
+ token = Doorkeeper::OAuth::Token.from_request(doorkeeper_request, *Doorkeeper.configuration.access_token_methods)
+ return @oauth_access_token = nil unless token
- def find_user_by_personal_access_token(token_string, scopes)
- access_token = PersonalAccessToken.active.find_by_token(token_string)
- return unless access_token
+ @oauth_access_token = OauthAccessToken.by_token(token)
+ raise UnauthorizedError unless @oauth_access_token
- if AccessTokenValidationService.new(access_token, request: request).include_any_scope?(scopes)
- User.find(access_token.user_id)
- end
+ @oauth_access_token.revoke_previous_refresh_token!
+ @oauth_access_token
end
- def find_access_token
- return @access_token if defined?(@access_token)
+ def find_user_by_access_token(access_token)
+ scopes = scopes_registered_for_endpoint
- token = Doorkeeper::OAuth::Token.from_request(doorkeeper_request, *Doorkeeper.configuration.access_token_methods)
- return @access_token = nil unless token
+ case AccessTokenValidationService.new(access_token, request: request).validate(scopes: scopes)
+ when AccessTokenValidationService::INSUFFICIENT_SCOPE
+ raise InsufficientScopeError.new(scopes)
+
+ when AccessTokenValidationService::EXPIRED
+ raise ExpiredError
- @access_token = Doorkeeper::AccessToken.by_token(token)
- raise UnauthorizedError unless @access_token
+ when AccessTokenValidationService::REVOKED
+ raise RevokedError
- @access_token.revoke_previous_refresh_token!
- @access_token
+ when AccessTokenValidationService::VALID
+ access_token.user
+ end
end
def doorkeeper_request
@doorkeeper_request ||= ActionDispatch::Request.new(env)
end
+
+ # An array of scopes that were registered (using `allow_access_with_scope`)
+ # for the current endpoint class. It also returns scopes registered on
+ # `API::API`, since these are meant to apply to all API routes.
+ def scopes_registered_for_endpoint
+ @scopes_registered_for_endpoint ||=
+ begin
+ endpoint_classes = [options[:for].presence, ::API::API].compact
+ endpoint_classes.reduce([]) do |memo, endpoint|
+ if endpoint.respond_to?(:allowed_scopes)
+ memo.concat(endpoint.allowed_scopes)
+ else
+ memo
+ end
+ end
+ end
+ end
end
module ClassMethods
diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb
index a87297a604c..2b316b58ed9 100644
--- a/lib/api/helpers.rb
+++ b/lib/api/helpers.rb
@@ -3,8 +3,6 @@ module API
include Gitlab::Utils
include Helpers::Pagination
- UnauthorizedError = Class.new(StandardError)
-
SUDO_HEADER = "HTTP_SUDO".freeze
SUDO_PARAM = :sudo
@@ -379,47 +377,16 @@ module API
private
- def private_token
- params[APIGuard::PRIVATE_TOKEN_PARAM] || env[APIGuard::PRIVATE_TOKEN_HEADER]
- end
-
- def warden
- env['warden']
- end
-
- # Check if the request is GET/HEAD, or if CSRF token is valid.
- def verified_request?
- Gitlab::RequestForgeryProtection.verified?(env)
- end
-
- # Check the Rails session for valid authentication details
- def find_user_from_warden
- warden.try(:authenticate) if verified_request?
- end
-
def initial_current_user
return @initial_current_user if defined?(@initial_current_user)
begin
@initial_current_user = Gitlab::Auth::UniqueIpsLimiter.limit_user! { find_current_user }
- rescue APIGuard::UnauthorizedError, UnauthorizedError
+ rescue APIGuard::UnauthorizedError
unauthorized!
end
end
- def find_current_user
- user =
- find_user_by_private_token(scopes: scopes_registered_for_endpoint) ||
- doorkeeper_guard(scopes: scopes_registered_for_endpoint) ||
- find_user_from_warden
-
- return nil unless user
-
- raise UnauthorizedError unless Gitlab::UserAccess.new(user).allowed? && user.can?(:access_api)
-
- user
- end
-
def sudo!
return unless sudo_identifier
return unless initial_current_user
@@ -479,22 +446,5 @@ module API
exception.status == 500
end
-
- # An array of scopes that were registered (using `allow_access_with_scope`)
- # for the current endpoint class. It also returns scopes registered on
- # `API::API`, since these are meant to apply to all API routes.
- def scopes_registered_for_endpoint
- @scopes_registered_for_endpoint ||=
- begin
- endpoint_classes = [options[:for].presence, ::API::API].compact
- endpoint_classes.reduce([]) do |memo, endpoint|
- if endpoint.respond_to?(:allowed_scopes)
- memo.concat(endpoint.allowed_scopes)
- else
- memo
- end
- end
- end
- end
end
end
diff --git a/spec/requests/api/helpers_spec.rb b/spec/requests/api/helpers_spec.rb
index 862920ad7c3..9f3b5a809d7 100644
--- a/spec/requests/api/helpers_spec.rb
+++ b/spec/requests/api/helpers_spec.rb
@@ -222,13 +222,6 @@ describe API::Helpers do
expect { current_user }.to raise_error /401/
end
- it "returns a 401 response 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
-
- expect { current_user }.to raise_error /401/
- end
-
it "leaves user as is when sudo not specified" do
env[API::APIGuard::PRIVATE_TOKEN_HEADER] = personal_access_token.token
expect(current_user).to eq(user)
@@ -238,18 +231,25 @@ describe API::Helpers do
expect(current_user).to eq(user)
end
+ it "does not allow tokens 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
+
+ expect { current_user }.to raise_error API::APIGuard::InsufficientScopeError
+ end
+
it 'does not allow revoked tokens' do
personal_access_token.revoke!
env[API::APIGuard::PRIVATE_TOKEN_HEADER] = personal_access_token.token
- expect { current_user }.to raise_error /401/
+ expect { current_user }.to raise_error API::APIGuard::RevokedError
end
it 'does not allow expired tokens' do
personal_access_token.update_attributes!(expires_at: 1.day.ago)
env[API::APIGuard::PRIVATE_TOKEN_HEADER] = personal_access_token.token
- expect { current_user }.to raise_error /401/
+ expect { current_user }.to raise_error API::APIGuard::ExpiredError
end
end
diff --git a/spec/support/api/scopes/read_user_shared_examples.rb b/spec/support/api/scopes/read_user_shared_examples.rb
index 57e28e040d7..111534f2f26 100644
--- a/spec/support/api/scopes/read_user_shared_examples.rb
+++ b/spec/support/api/scopes/read_user_shared_examples.rb
@@ -27,10 +27,10 @@ shared_examples_for 'allows the "read_user" scope' do
stub_container_registry_config(enabled: true)
end
- it 'returns a "401" response' do
+ it 'returns a "403" response' do
get api_call.call(path, user, personal_access_token: token)
- expect(response).to have_http_status(401)
+ expect(response).to have_http_status(403)
end
end
end
@@ -74,10 +74,10 @@ shared_examples_for 'does not allow the "read_user" scope' do
context 'when the requesting token has the "read_user" scope' do
let(:token) { create(:personal_access_token, scopes: ['read_user'], user: user) }
- it 'returns a "401" response' do
+ it 'returns a "403" response' do
post api_call.call(path, user, personal_access_token: token), attributes_for(:user, projects_limit: 3)
- expect(response).to have_http_status(401)
+ expect(response).to have_http_status(403)
end
end
end