Commit aecc3eb0 authored by Francisco Lopez's avatar Francisco Lopez

Applied some code review comments

parent 374179a9
...@@ -99,8 +99,7 @@ class ApplicationController < ActionController::Base ...@@ -99,8 +99,7 @@ class ApplicationController < ActionController::Base
return try(:authenticated_user) return try(:authenticated_user)
end end
# This filter handles private tokens, personal access tokens, and atom # This filter handles personal access tokens, and atom requests with rss tokens
# requests with rss tokens
def authenticate_sessionless_user! def authenticate_sessionless_user!
user = Gitlab::Auth::RequestAuthenticator.new(request).find_sessionless_user user = Gitlab::Auth::RequestAuthenticator.new(request).find_sessionless_user
......
...@@ -45,7 +45,6 @@ module API ...@@ -45,7 +45,6 @@ module API
include Gitlab::Utils::StrongMemoize include Gitlab::Utils::StrongMemoize
def find_current_user! def find_current_user!
set_raise_unauthorized_error
user = find_user_from_access_token || find_user_from_warden user = find_user_from_access_token || find_user_from_warden
return unless user return unless user
...@@ -75,10 +74,6 @@ module API ...@@ -75,10 +74,6 @@ module API
private private
def private_token
params[PRIVATE_TOKEN_PARAM].presence || env[PRIVATE_TOKEN_HEADER].presence
end
# An array of scopes that were registered (using `allow_access_with_scope`) # An array of scopes that were registered (using `allow_access_with_scope`)
# for the current endpoint class. It also returns scopes registered on # for the current endpoint class. It also returns scopes registered on
# `API::API`, since these are meant to apply to all API routes. # `API::API`, since these are meant to apply to all API routes.
......
...@@ -7,8 +7,10 @@ module Gitlab ...@@ -7,8 +7,10 @@ module Gitlab
attr_reader :request attr_reader :request
delegate :params, :env, to: :request
def initialize(request) def initialize(request)
@request = ensure_action_dispatch_request(request) @request = request
end end
def user def user
...@@ -16,7 +18,9 @@ module Gitlab ...@@ -16,7 +18,9 @@ module Gitlab
end end
def find_sessionless_user def find_sessionless_user
find_user_from_access_token || find_user_by_rss_token find_user_from_access_token || find_user_from_rss_token
rescue StandardError
nil
end end
end end
end end
......
module Gitlab module Gitlab
module Auth module Auth
module UserAuthFinders module UserAuthFinders
PRIVATE_TOKEN_HEADER = 'HTTP_PRIVATE_TOKEN'.freeze
PRIVATE_TOKEN_PARAM = :private_token
# Check the Rails session for valid authentication details # Check the Rails session for valid authentication details
def find_user_from_warden def find_user_from_warden
request.env['warden']&.authenticate if verified_request? env['warden']&.authenticate if verified_request?
end end
def find_user_by_rss_token def find_user_from_rss_token
return unless request.format.atom? return unless request.format.atom?
token = request.params[:rss_token].presence token = params[:rss_token].presence
return unless token.present? return unless token
handle_return_value!(User.find_by_rss_token(token)) handle_return_value!(User.find_by_rss_token(token))
end end
...@@ -24,14 +27,22 @@ module Gitlab ...@@ -24,14 +27,22 @@ module Gitlab
end end
def validate_access_token!(scopes: []) def validate_access_token!(scopes: [])
return unless access_token
case AccessTokenValidationService.new(access_token, request: request).validate(scopes: scopes)
when AccessTokenValidationService::INSUFFICIENT_SCOPE
raise API::APIGuard::InsufficientScopeError.new(scopes)
when AccessTokenValidationService::EXPIRED
raise API::APIGuard::ExpiredError
when AccessTokenValidationService::REVOKED
raise API::APIGuard::RevokedError
end
end end
private private
def handle_return_value!(value, &block) def handle_return_value!(value, &block)
unless value raise API::APIGuard::UnauthorizedError unless value
raise_unauthorized_error? ? raise_unauthorized_error! : return
end
block_given? ? yield(value) : value block_given? ? yield(value) : value
end end
...@@ -43,13 +54,13 @@ module Gitlab ...@@ -43,13 +54,13 @@ module Gitlab
end end
def private_token def private_token
request.params[:private_token].presence || params[PRIVATE_TOKEN_PARAM].presence ||
request.headers['PRIVATE-TOKEN'].presence env[PRIVATE_TOKEN_HEADER].presence
end end
def find_personal_access_token def find_personal_access_token
token = private_token.to_s token = private_token
return unless token.present? return unless token
# Expiration, revocation and scopes are verified in `validate_access_token!` # Expiration, revocation and scopes are verified in `validate_access_token!`
handle_return_value!(PersonalAccessToken.find_by(token: token)) handle_return_value!(PersonalAccessToken.find_by(token: token))
...@@ -77,18 +88,6 @@ module Gitlab ...@@ -77,18 +88,6 @@ module Gitlab
ActionDispatch::Request.new(request.env) ActionDispatch::Request.new(request.env)
end end
def raise_unauthorized_error?
defined?(@raise_unauthorized_error) ? @raise_unauthorized_error : false
end
def set_raise_unauthorized_error
@raise_unauthorized_error = true
end
def raise_unauthorized_error!
raise API::APIGuard::UnauthorizedError
end
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