diff --git a/app/models/user.rb b/app/models/user.rb index b9bb4a9e3f7..1bd28203523 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -304,10 +304,6 @@ class User < ActiveRecord::Base personal_access_token.user if personal_access_token end - def by_username_or_id(name_or_id) - find_by('users.username = ? OR users.id = ?', name_or_id.to_s, name_or_id.to_i) - end - # Returns a user for the given SSH key. def find_by_ssh_key_id(key_id) find_by(id: Key.unscoped.select(:user_id).where(id: key_id)) diff --git a/changelogs/unreleased/25482-fix-api-sudo.yml b/changelogs/unreleased/25482-fix-api-sudo.yml index 3b23bfd3a21..4c11fe1622e 100644 --- a/changelogs/unreleased/25482-fix-api-sudo.yml +++ b/changelogs/unreleased/25482-fix-api-sudo.yml @@ -1,4 +1,4 @@ --- -title: 'API: Memoize the current_user so that the sudo can work properly' +title: 'API: Memoize the current_user so that sudo can work properly' merge_request: 8017 -author: +author: diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 2041f0dac6b..8260fcab80a 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -34,6 +34,14 @@ module API @available_labels ||= LabelsFinder.new(current_user, project_id: user_project.id).execute end + def find_user(id) + if id =~ /^\d+$/ + User.find_by(id: id) + else + User.find_by(username: id) + end + end + def find_project(id) if id =~ /^\d+$/ Project.find_by(id: id) @@ -349,7 +357,7 @@ module API def sudo! return unless sudo_identifier - return unless initial_current_user.is_a?(User) + return unless initial_current_user unless initial_current_user.is_admin? forbidden!('Must be admin to use sudo') @@ -360,7 +368,7 @@ module API forbidden!('Private token must be specified in order to use sudo') end - sudoed_user = User.by_username_or_id(sudo_identifier) + sudoed_user = find_user(sudo_identifier) if sudoed_user @current_user = sudoed_user @@ -370,17 +378,7 @@ module API 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 + @sudo_identifier ||= params[SUDO_PARAM] || env[SUDO_HEADER] end def add_pagination_headers(paginated_data) diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index bad6ed9e146..8b20ee81614 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -727,17 +727,6 @@ describe User, models: true do end end - describe 'by_username_or_id' do - let(:user1) { create(:user, username: 'foo') } - - it "gets the correct user" do - expect(User.by_username_or_id(user1.id)).to eq(user1) - expect(User.by_username_or_id('foo')).to eq(user1) - expect(User.by_username_or_id(-1)).to be_nil - expect(User.by_username_or_id('bar')).to be_nil - end - end - describe '.find_by_ssh_key_id' do context 'using an existing SSH key ID' do let(:user) { create(:user) } diff --git a/spec/requests/api/helpers_spec.rb b/spec/requests/api/helpers_spec.rb index 573154f69b4..4035fd97af5 100644 --- a/spec/requests/api/helpers_spec.rb +++ b/spec/requests/api/helpers_spec.rb @@ -16,14 +16,14 @@ describe API::Helpers, api: true do clear_env 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::Helpers::SUDO_HEADER] = identifier + env[API::Helpers::SUDO_HEADER] = identifier.to_s end def set_param(user_or_token, identifier) clear_env 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::Helpers::SUDO_PARAM] = identifier + params[API::Helpers::SUDO_PARAM] = identifier.to_s end def clear_env