From 4dfdef2ddfc3cdeb6f6231e397543d120083a4c2 Mon Sep 17 00:00:00 2001 From: Timothy Andrew Date: Fri, 21 Apr 2017 06:44:47 +0000 Subject: [PATCH] Allow admins to sudo to blocked users. - Currently, (for example) admins can't delete snippets for blocked users, which is an unexpected limitation. - We modify `authenticate!` to conduct the `access_api` policy check against the `initial_current_user`, instead of the user being impersonated. - Update CHANGELOG for !10842 --- ...505-allow-admins-sudo-to-blocked-users.yml | 4 +++ lib/api/helpers.rb | 2 +- spec/requests/api/helpers_spec.rb | 28 ++++++++++++++++++- 3 files changed, 32 insertions(+), 2 deletions(-) create mode 100644 changelogs/unreleased/29505-allow-admins-sudo-to-blocked-users.yml diff --git a/changelogs/unreleased/29505-allow-admins-sudo-to-blocked-users.yml b/changelogs/unreleased/29505-allow-admins-sudo-to-blocked-users.yml new file mode 100644 index 00000000000..42fd71ccd5f --- /dev/null +++ b/changelogs/unreleased/29505-allow-admins-sudo-to-blocked-users.yml @@ -0,0 +1,4 @@ +--- +title: Allow admins to sudo to blocked users via the API +merge_request: 10842 +author: diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index ddff3c8c1e8..86bf567fe69 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -102,7 +102,7 @@ module API end def authenticate! - unauthorized! unless current_user && can?(current_user, :access_api) + unauthorized! unless current_user && can?(initial_current_user, :access_api) end def authenticate_non_get! diff --git a/spec/requests/api/helpers_spec.rb b/spec/requests/api/helpers_spec.rb index 988a57a80ea..6cbe1ffa1a4 100644 --- a/spec/requests/api/helpers_spec.rb +++ b/spec/requests/api/helpers_spec.rb @@ -427,6 +427,7 @@ describe API::Helpers, api: true do context 'current_user is nil' do before do expect_any_instance_of(self.class).to receive(:current_user).and_return(nil) + allow_any_instance_of(self.class).to receive(:initial_current_user).and_return(nil) end it 'returns a 401 response' do @@ -435,13 +436,38 @@ describe API::Helpers, api: true do end context 'current_user is present' do + let(:user) { build(:user) } + before do - expect_any_instance_of(self.class).to receive(:current_user).at_least(:once).and_return(User.new) + expect_any_instance_of(self.class).to receive(:current_user).at_least(:once).and_return(user) + expect_any_instance_of(self.class).to receive(:initial_current_user).and_return(user) end it 'does not raise an error' do expect { authenticate! }.not_to raise_error end end + + context 'current_user is blocked' do + let(:user) { build(:user, :blocked) } + + before do + expect_any_instance_of(self.class).to receive(:current_user).at_least(:once).and_return(user) + end + + it 'raises an error' do + expect_any_instance_of(self.class).to receive(:initial_current_user).and_return(user) + + expect { authenticate! }.to raise_error '401 - {"message"=>"401 Unauthorized"}' + end + + it "doesn't raise an error if an admin user is impersonating a blocked user (via sudo)" do + admin_user = build(:user, :admin) + + expect_any_instance_of(self.class).to receive(:initial_current_user).and_return(admin_user) + + expect { authenticate! }.not_to raise_error + end + end end end