Commit ca69c725 authored by Rémy Coutable's avatar Rémy Coutable Committed by Timothy Andrew

API: Memoize the current_user so that the sudo can work properly

The issue was arising when `#current_user` was called a second time
after a user was impersonated: the `User#is_admin?` check would be
performed on it and it would fail.
Signed-off-by: default avatarRémy Coutable <remy@rymai.me>
parent ff307843
---
title: 'API: Memoize the current_user so that the sudo can work properly'
merge_request: 8017
author:
......@@ -7,67 +7,23 @@ module API
SUDO_HEADER = "HTTP_SUDO"
SUDO_PARAM = :sudo
def private_token
params[PRIVATE_TOKEN_PARAM] || env[PRIVATE_TOKEN_HEADER]
end
def warden
env['warden']
end
# Check the Rails session for valid authentication details
#
# Until CSRF protection is added to the API, disallow this method for
# state-changing endpoints
def find_user_from_warden
warden.try(:authenticate) if %w[GET HEAD].include?(env['REQUEST_METHOD'])
end
def declared_params(options = {})
options = { include_parent_namespaces: false }.merge(options)
declared(params, options).to_h.symbolize_keys
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 current_user
@current_user ||= find_user_by_private_token
@current_user ||= doorkeeper_guard
@current_user ||= find_user_from_warden
unless @current_user && Gitlab::UserAccess.new(@current_user).allowed?
return nil
end
identifier = sudo_identifier
return @current_user if defined?(@current_user)
if identifier
# We check for private_token because we cannot allow PAT to be used
forbidden!('Must be admin to use sudo') unless @current_user.is_admin?
forbidden!('Private token must be specified in order to use sudo') unless private_token_used?
@current_user = initial_current_user
@impersonator = @current_user
@current_user = User.by_username_or_id(identifier)
not_found!("No user id or username for: #{identifier}") if @current_user.nil?
end
sudo!
@current_user
end
def sudo_identifier
identifier ||= params[SUDO_PARAM] || env[SUDO_HEADER]
# Regex for integers
if !!(identifier =~ /\A[0-9]+\z/)
identifier.to_i
else
identifier
end
def sudo?
initial_current_user != current_user
end
def user_project
......@@ -365,6 +321,79 @@ module API
private
def private_token
params[PRIVATE_TOKEN_PARAM] || env[PRIVATE_TOKEN_HEADER]
end
def warden
env['warden']
end
# Check the Rails session for valid authentication details
#
# Until CSRF protection is added to the API, disallow this method for
# state-changing endpoints
def find_user_from_warden
warden.try(:authenticate) if %w[GET HEAD].include?(env['REQUEST_METHOD'])
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
return @initial_current_user if defined?(@initial_current_user)
@initial_current_user ||= find_user_by_private_token
@initial_current_user ||= doorkeeper_guard
@initial_current_user ||= find_user_from_warden
unless @initial_current_user && Gitlab::UserAccess.new(@initial_current_user).allowed?
@initial_current_user = nil
end
@initial_current_user
end
def sudo!
return unless sudo_identifier
return unless initial_current_user.is_a?(User)
unless initial_current_user.is_admin?
forbidden!('Must be admin to use sudo')
end
# Only private tokens should be used for the SUDO feature
unless private_token == initial_current_user.private_token
forbidden!('Private token must be specified in order to use sudo')
end
sudoed_user = User.by_username_or_id(sudo_identifier)
if sudoed_user
@current_user = sudoed_user
else
not_found!("No user id or username for: #{sudo_identifier}")
end
end
def sudo_identifier
return @sudo_identifier if defined?(@sudo_identifier)
identifier ||= params[SUDO_PARAM] || env[SUDO_HEADER]
# Regex for integers
@sudo_identifier =
if !!(identifier =~ /\A[0-9]+\z/)
identifier.to_i
else
identifier
end
end
def add_pagination_headers(paginated_data)
header 'X-Total', paginated_data.total_count.to_s
header 'X-Total-Pages', paginated_data.total_pages.to_s
......@@ -397,10 +426,6 @@ module API
links.join(', ')
end
def private_token_used?
private_token == @current_user.private_token
end
def secret_token
Gitlab::Shell.secret_token
end
......
......@@ -355,7 +355,7 @@ module API
success Entities::UserPublic
end
get do
present current_user, with: @impersonator ? Entities::UserWithPrivateToken : Entities::UserPublic
present current_user, with: sudo? ? Entities::UserWithPrivateToken : Entities::UserPublic
end
desc "Get the currently authenticated user's SSH keys" do
......
......@@ -2,7 +2,6 @@ require 'spec_helper'
describe API::Helpers, api: true do
include API::Helpers
include ApiHelpers
include SentryHelper
let(:user) { create(:user) }
......@@ -13,17 +12,17 @@ describe API::Helpers, api: true do
let(:env) { { 'REQUEST_METHOD' => 'GET' } }
let(:request) { Rack::Request.new(env) }
def set_env(token_usr, identifier)
def set_env(user_or_token, identifier)
clear_env
clear_param
env[API::Helpers::PRIVATE_TOKEN_HEADER] = token_usr.private_token
env[API::Helpers::PRIVATE_TOKEN_HEADER] = user_or_token.respond_to?(:private_token) ? user_or_token.private_token : user_or_token
env[API::Helpers::SUDO_HEADER] = identifier
end
def set_param(token_usr, identifier)
def set_param(user_or_token, identifier)
clear_env
clear_param
params[API::Helpers::PRIVATE_TOKEN_PARAM] = token_usr.private_token
params[API::Helpers::PRIVATE_TOKEN_PARAM] = user_or_token.respond_to?(:private_token) ? user_or_token.private_token : user_or_token
params[API::Helpers::SUDO_PARAM] = identifier
end
......@@ -163,6 +162,13 @@ describe API::Helpers, api: true do
expect(current_user).to eq(user)
end
it 'memoize the current_user: sudo permissions are not run against the sudoed user' do
set_env(admin, user.id)
expect(current_user).to eq(user)
expect(current_user).to eq(user)
end
it 'handles sudo to oneself' do
set_env(admin, admin.id)
......@@ -294,33 +300,48 @@ describe API::Helpers, api: true do
end
end
describe '.sudo_identifier' do
it "returns integers when input is an int" do
set_env(admin, '123')
expect(sudo_identifier).to eq(123)
set_env(admin, '0001234567890')
expect(sudo_identifier).to eq(1234567890)
set_param(admin, '123')
expect(sudo_identifier).to eq(123)
set_param(admin, '0001234567890')
expect(sudo_identifier).to eq(1234567890)
describe '.sudo?' do
context 'when no sudo env or param is passed' do
before do
doorkeeper_guard_returns(nil)
end
it 'returns false' do
expect(sudo?).to be_falsy
end
end
it "returns string when input is an is not an int" do
set_env(admin, '12.30')
expect(sudo_identifier).to eq("12.30")
set_env(admin, 'hello')
expect(sudo_identifier).to eq('hello')
set_env(admin, ' 123')
expect(sudo_identifier).to eq(' 123')
set_param(admin, '12.30')
expect(sudo_identifier).to eq("12.30")
set_param(admin, 'hello')
expect(sudo_identifier).to eq('hello')
set_param(admin, ' 123')
expect(sudo_identifier).to eq(' 123')
context 'when sudo env or param is passed', 'user is not an admin' do
before do
set_env(user, '123')
end
it 'returns an 403 Forbidden' do
expect { sudo? }.to raise_error '403 - {"message"=>"403 Forbidden - Must be admin to use sudo"}'
end
end
context 'when sudo env or param is passed', 'user is admin' do
context 'personal access token is used' do
before do
personal_access_token = create(:personal_access_token, user: admin)
set_env(personal_access_token.token, user.id)
end
it 'returns an 403 Forbidden' do
expect { sudo? }.to raise_error '403 - {"message"=>"403 Forbidden - Private token must be specified in order to use sudo"}'
end
end
context 'private access token is used' do
before do
set_env(admin.private_token, user.id)
end
it 'returns true' do
expect(sudo?).to be_truthy
end
end
end
end
......
......@@ -663,13 +663,12 @@ describe API::Users, api: true do
end
describe "GET /user" do
let(:personal_access_token) { create(:personal_access_token, user: user) }
let(:private_token) { user.private_token }
let(:personal_access_token) { create(:personal_access_token, user: user).token }
context 'with regular user' do
context 'with personal access token' do
it 'returns 403 without private token when sudo is defined' do
get api("/user?private_token=#{personal_access_token.token}&sudo=#{user.id}")
get api("/user?private_token=#{personal_access_token}&sudo=123")
expect(response).to have_http_status(403)
end
......@@ -677,7 +676,7 @@ describe API::Users, api: true do
context 'with private token' do
it 'returns 403 without private token when sudo defined' do
get api("/user?private_token=#{private_token}&sudo=#{user.id}")
get api("/user?private_token=#{user.private_token}&sudo=123")
expect(response).to have_http_status(403)
end
......@@ -688,40 +687,44 @@ describe API::Users, api: true do
expect(response).to have_http_status(200)
expect(response).to match_response_schema('user/public')
expect(json_response['id']).to eq(user.id)
end
end
context 'with admin' do
let(:user) { create(:admin) }
let(:admin_personal_access_token) { create(:personal_access_token, user: admin).token }
context 'with personal access token' do
it 'returns 403 without private token when sudo defined' do
get api("/user?private_token=#{personal_access_token.token}&sudo=#{user.id}")
get api("/user?private_token=#{admin_personal_access_token}&sudo=#{user.id}")
expect(response).to have_http_status(403)
end
it 'returns current user without private token when sudo not defined' do
get api("/user?private_token=#{personal_access_token.token}")
it 'returns initial current user without private token when sudo not defined' do
get api("/user?private_token=#{admin_personal_access_token}")
expect(response).to have_http_status(200)
expect(response).to match_response_schema('user/public')
expect(json_response['id']).to eq(admin.id)
end
end
context 'with private token' do
it 'returns current user with private token when sudo defined' do
get api("/user?private_token=#{private_token}&sudo=#{user.id}")
it 'returns sudoed user with private token when sudo defined' do
get api("/user?private_token=#{admin.private_token}&sudo=#{user.id}")
expect(response).to have_http_status(200)
expect(response).to match_response_schema('user/login')
expect(json_response['id']).to eq(user.id)
end
it 'returns current user without private token when sudo not defined' do
get api("/user?private_token=#{private_token}")
it 'returns initial current user without private token when sudo not defined' do
get api("/user?private_token=#{admin.private_token}")
expect(response).to have_http_status(200)
expect(response).to match_response_schema('user/public')
expect(json_response['id']).to eq(admin.id)
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