Merge remote-tracking branch 'upstream/master' into no-ivar-in-modules

* upstream/master: (1723 commits)
  Resolve "Editor icons"
  Refactor issuable destroy action
  Ignore routes matching legacy_*_redirect in route specs
  Gitlab::Git::RevList and LfsChanges use lazy popen
  Gitlab::Git::Popen can lazily hand output to a block
  Merge branch 'master-i18n' into 'master'
  Remove unique validation from external_url in Environment
  Expose `duration` in Job API entity
  Add TimeCop freeze for DST and Regular time
  Harcode project visibility
  update a changelog
  Put a condition to old migration that adds fast_forward column to MRs
  Expose project visibility as CI variable
  fix flaky tests by removing unneeded clicks and focus actions
  fix flaky test in gfm_autocomplete_spec.rb
  Use Gitlab::Git operations for repository mirroring
  Encapsulate git operations for mirroring in Gitlab::Git
  Create a Wiki Repository's raw_repository properly
  Add `Gitlab::Git::Repository#fetch` command
  Fix Gitlab::Metrics::System#real_time and #monotonic_time doc
  ...

1 files changed, 197 insertions, 284 deletions

diff --git a/spec/requests/api/helpers_spec.rb b/spec/requests/api/helpers_spec.rb
index d4006fe71a2..6c0996c543d 100644
--- a/spec/requests/api/helpers_spec.rb
+++ b/spec/requests/api/helpers_spec.rb
@@ -1,4 +1,6 @@
 require 'spec_helper'
+require 'raven/transports/dummy'
+require_relative '../../../config/initializers/sentry'
 
 describe API::Helpers do
   include API::APIGuard::HelperMethods
@@ -26,39 +28,11 @@ describe API::Helpers do
     allow_any_instance_of(self.class).to receive(:options).and_return({})
   end
 
-  def set_env(user_or_token, identifier)
-    clear_env
-    clear_param
-    env[API::APIGuard::PRIVATE_TOKEN_HEADER] = user_or_token.respond_to?(:private_token) ? user_or_token.private_token : user_or_token
-    env[API::Helpers::SUDO_HEADER] = identifier.to_s
-  end
-
-  def set_param(user_or_token, identifier)
-    clear_env
-    clear_param
-    params[API::APIGuard::PRIVATE_TOKEN_PARAM] = user_or_token.respond_to?(:private_token) ? user_or_token.private_token : user_or_token
-    params[API::Helpers::SUDO_PARAM] = identifier.to_s
-  end
-
-  def clear_env
-    env.delete(API::APIGuard::PRIVATE_TOKEN_HEADER)
-    env.delete(API::Helpers::SUDO_HEADER)
-  end
-
-  def clear_param
-    params.delete(API::APIGuard::PRIVATE_TOKEN_PARAM)
-    params.delete(API::Helpers::SUDO_PARAM)
-  end
-
   def warden_authenticate_returns(value)
     warden = double("warden", authenticate: value)
     env['warden'] = warden
   end
 
-  def doorkeeper_guard_returns(value)
-    allow_any_instance_of(self.class).to receive(:doorkeeper_guard) { value }
-  end
-
   def error!(message, status, header)
     raise Exception.new("#{status} - #{message}")
   end
@@ -67,10 +41,6 @@ describe API::Helpers do
     subject { current_user }
 
     describe "Warden authentication", :allow_forgery_protection do
-      before do
-        doorkeeper_guard_returns false
-      end
-
       context "with invalid credentials" do
         context "GET request" do
           before do
@@ -158,274 +128,53 @@ describe API::Helpers do
       end
     end
 
-    describe "when authenticating using a user's private token" do
-      it "returns nil for an invalid token" do
-        env[API::APIGuard::PRIVATE_TOKEN_HEADER] = 'invalid token'
-        allow_any_instance_of(self.class).to receive(:doorkeeper_guard) { false }
-
-        expect(current_user).to be_nil
-      end
-
-      it "returns nil for a user without access" do
-        env[API::APIGuard::PRIVATE_TOKEN_HEADER] = user.private_token
-        allow_any_instance_of(Gitlab::UserAccess).to receive(:allowed?).and_return(false)
-
-        expect(current_user).to be_nil
-      end
-
-      it "leaves user as is when sudo not specified" do
-        env[API::APIGuard::PRIVATE_TOKEN_HEADER] = user.private_token
-
-        expect(current_user).to eq(user)
-
-        clear_env
-
-        params[API::APIGuard::PRIVATE_TOKEN_PARAM] = user.private_token
-
-        expect(current_user).to eq(user)
-      end
-    end
-
     describe "when authenticating using a user's personal access tokens" do
       let(:personal_access_token) { create(:personal_access_token, user: user) }
 
-      before do
-        allow_any_instance_of(self.class).to receive(:doorkeeper_guard) { false }
-      end
-
-      it "returns nil for an invalid token" do
+      it "returns a 401 response for an invalid token" do
         env[API::APIGuard::PRIVATE_TOKEN_HEADER] = 'invalid token'
 
-        expect(current_user).to be_nil
+        expect { current_user }.to raise_error /401/
       end
 
-      it "returns nil for a user without access" do
+      it "returns a 403 response for a user without access" do
         env[API::APIGuard::PRIVATE_TOKEN_HEADER] = personal_access_token.token
         allow_any_instance_of(Gitlab::UserAccess).to receive(:allowed?).and_return(false)
 
-        expect(current_user).to be_nil
+        expect { current_user }.to raise_error /403/
       end
 
-      it "returns nil for a token without the appropriate scope" do
-        personal_access_token = create(:personal_access_token, user: user, scopes: ['read_user'])
+      it 'returns a 403 response for a user who is blocked' do
+        user.block!
         env[API::APIGuard::PRIVATE_TOKEN_HEADER] = personal_access_token.token
 
-        expect(current_user).to be_nil
+        expect { current_user }.to raise_error /403/
       end
 
-      it "leaves user as is when sudo not specified" do
+      it "sets current_user" do
         env[API::APIGuard::PRIVATE_TOKEN_HEADER] = personal_access_token.token
         expect(current_user).to eq(user)
-        clear_env
-        params[API::APIGuard::PRIVATE_TOKEN_PARAM] = personal_access_token.token
+      end
 
-        expect(current_user).to eq(user)
+      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 be_nil
+        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 be_nil
-      end
-    end
-
-    context 'sudo usage' do
-      context 'with admin' do
-        context 'with header' do
-          context 'with id' do
-            it 'changes current_user to sudo' do
-              set_env(admin, user.id)
-
-              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)
-
-              expect(current_user).to eq(admin)
-            end
-
-            it 'throws an error when user cannot be found' do
-              id = user.id + admin.id
-              expect(user.id).not_to eq(id)
-              expect(admin.id).not_to eq(id)
-
-              set_env(admin, id)
-
-              expect { current_user }.to raise_error(Exception)
-            end
-          end
-
-          context 'with username' do
-            it 'changes current_user to sudo' do
-              set_env(admin, user.username)
-
-              expect(current_user).to eq(user)
-            end
-
-            it 'handles sudo to oneself' do
-              set_env(admin, admin.username)
-
-              expect(current_user).to eq(admin)
-            end
-
-            it "throws an error when the user cannot be found for a given username" do
-              username = "#{user.username}#{admin.username}"
-              expect(user.username).not_to eq(username)
-              expect(admin.username).not_to eq(username)
-
-              set_env(admin, username)
-
-              expect { current_user }.to raise_error(Exception)
-            end
-          end
-        end
-
-        context 'with param' do
-          context 'with id' do
-            it 'changes current_user to sudo' do
-              set_param(admin, user.id)
-
-              expect(current_user).to eq(user)
-            end
-
-            it 'handles sudo to oneself' do
-              set_param(admin, admin.id)
-
-              expect(current_user).to eq(admin)
-            end
-
-            it 'handles sudo to oneself using string' do
-              set_env(admin, user.id.to_s)
-
-              expect(current_user).to eq(user)
-            end
-
-            it 'throws an error when user cannot be found' do
-              id = user.id + admin.id
-              expect(user.id).not_to eq(id)
-              expect(admin.id).not_to eq(id)
-
-              set_param(admin, id)
-
-              expect { current_user }.to raise_error(Exception)
-            end
-          end
-
-          context 'with username' do
-            it 'changes current_user to sudo' do
-              set_param(admin, user.username)
-
-              expect(current_user).to eq(user)
-            end
-
-            it 'handles sudo to oneself' do
-              set_param(admin, admin.username)
-
-              expect(current_user).to eq(admin)
-            end
-
-            it "throws an error when the user cannot be found for a given username" do
-              username = "#{user.username}#{admin.username}"
-              expect(user.username).not_to eq(username)
-              expect(admin.username).not_to eq(username)
-
-              set_param(admin, username)
-
-              expect { current_user }.to raise_error(Exception)
-            end
-          end
-        end
-      end
-
-      context 'with regular user' do
-        context 'with env' do
-          it 'changes current_user to sudo when admin and user id' do
-            set_env(user, admin.id)
-
-            expect { current_user }.to raise_error(Exception)
-          end
-
-          it 'changes current_user to sudo when admin and user username' do
-            set_env(user, admin.username)
-
-            expect { current_user }.to raise_error(Exception)
-          end
-        end
-
-        context 'with params' do
-          it 'changes current_user to sudo when admin and user id' do
-            set_param(user, admin.id)
-
-            expect { current_user }.to raise_error(Exception)
-          end
-
-          it 'changes current_user to sudo when admin and user username' do
-            set_param(user, admin.username)
-
-            expect { current_user }.to raise_error(Exception)
-          end
-        end
-      end
-    end
-  end
-
-  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
-
-    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
+        expect { current_user }.to raise_error API::APIGuard::ExpiredError
       end
     end
   end
@@ -450,10 +199,55 @@ describe API::Helpers do
       allow(exception).to receive(:backtrace).and_return(caller)
 
       expect_any_instance_of(self.class).to receive(:sentry_context)
-      expect(Raven).to receive(:capture_exception).with(exception)
+      expect(Raven).to receive(:capture_exception).with(exception, extra: {})
 
       handle_api_exception(exception)
     end
+
+    context 'with a personal access token given' do
+      let(:token) { create(:personal_access_token, scopes: ['api'], user: user) }
+
+      # Regression test for https://gitlab.com/gitlab-org/gitlab-ce/issues/38571
+      it 'does not raise an additional exception because of missing `request`' do
+        # We need to stub at a lower level than #sentry_enabled? otherwise
+        # Sentry is not enabled when the request below is made, and the test
+        # would pass even without the fix
+        expect(Gitlab::Sentry).to receive(:enabled?).twice.and_return(true)
+        expect(ProjectsFinder).to receive(:new).and_raise('Runtime Error!')
+
+        get api('/projects', personal_access_token: token)
+
+        # The 500 status is expected as we're testing a case where an exception
+        # is raised, but Grape shouldn't raise an additional exception
+        expect(response).to have_gitlab_http_status(500)
+        expect(json_response['message']).not_to include("undefined local variable or method `request'")
+        expect(json_response['message']).to start_with("\nRuntimeError (Runtime Error!):")
+      end
+    end
+
+    context 'extra information' do
+      # Sentry events are an array of the form [auth_header, data, options]
+      let(:event_data) { Raven.client.transport.events.first[1] }
+
+      before do
+        stub_application_setting(
+          sentry_enabled: true,
+          sentry_dsn: "dummy://12345:67890@sentry.localdomain/sentry/42"
+        )
+        configure_sentry
+        Raven.client.configuration.encoding = 'json'
+      end
+
+      it 'sends the params, excluding confidential values' do
+        expect(Gitlab::Sentry).to receive(:enabled?).twice.and_return(true)
+        expect(ProjectsFinder).to receive(:new).and_raise('Runtime Error!')
+
+        get api('/projects', user), password: 'dont_send_this', other_param: 'send_this'
+
+        expect(event_data).to include('other_param=send_this')
+        expect(event_data).to include('password=********')
+      end
+    end
   end
 
   describe '.authenticate_non_get!' do
@@ -490,11 +284,10 @@ describe API::Helpers 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
-        expect { authenticate! }.to raise_error '401 - {"message"=>"401 Unauthorized"}'
+        expect { authenticate! }.to raise_error /401/
       end
     end
 
@@ -502,34 +295,154 @@ describe API::Helpers do
       let(:user) { build(:user) }
 
       before do
-        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)
+        expect_any_instance_of(self.class).to receive(:current_user).and_return(user)
       end
 
       it 'does not raise an error' do
         expect { authenticate! }.not_to raise_error
       end
     end
+  end
 
-    context 'current_user is blocked' do
-      let(:user) { build(:user, :blocked) }
+  context 'sudo' do
+    shared_examples 'successful sudo' do
+      it 'sets current_user' do
+        expect(current_user).to eq(user)
+      end
+
+      it 'sets sudo?' do
+        expect(sudo?).to be_truthy
+      end
+    end
+
+    shared_examples 'sudo' do
+      context 'when admin' do
+        before do
+          token.user = admin
+          token.save!
+        end
+
+        context 'when token has sudo scope' do
+          before do
+            token.scopes = %w[sudo]
+            token.save!
+          end
+
+          context 'when user exists' do
+            context 'when using header' do
+              context 'when providing username' do
+                before do
+                  env[API::Helpers::SUDO_HEADER] = user.username
+                end
+
+                it_behaves_like 'successful sudo'
+              end
+
+              context 'when providing user ID' do
+                before do
+                  env[API::Helpers::SUDO_HEADER] = user.id.to_s
+                end
+
+                it_behaves_like 'successful sudo'
+              end
+            end
+
+            context 'when using param' do
+              context 'when providing username' do
+                before do
+                  params[API::Helpers::SUDO_PARAM] = user.username
+                end
+
+                it_behaves_like 'successful sudo'
+              end
+
+              context 'when providing user ID' do
+                before do
+                  params[API::Helpers::SUDO_PARAM] = user.id.to_s
+                end
+
+                it_behaves_like 'successful sudo'
+              end
+            end
+          end
+
+          context 'when user does not exist' do
+            before do
+              params[API::Helpers::SUDO_PARAM] = 'nonexistent'
+            end
+
+            it 'raises an error' do
+              expect { current_user }.to raise_error /User with ID or username 'nonexistent' Not Found/
+            end
+          end
+        end
+
+        context 'when token does not have sudo scope' do
+          before do
+            token.scopes = %w[api]
+            token.save!
+
+            params[API::Helpers::SUDO_PARAM] = user.id.to_s
+          end
+
+          it 'raises an error' do
+            expect { current_user }.to raise_error API::APIGuard::InsufficientScopeError
+          end
+        end
+      end
+
+      context 'when not admin' do
+        before do
+          token.user = user
+          token.save!
+
+          params[API::Helpers::SUDO_PARAM] = user.id.to_s
+        end
+
+        it 'raises an error' do
+          expect { current_user }.to raise_error /Must be admin to use sudo/
+        end
+      end
+    end
+
+    context 'using an OAuth token' do
+      let(:token) { create(:oauth_access_token) }
 
       before do
-        expect_any_instance_of(self.class).to receive(:current_user).at_least(:once).and_return(user)
+        env['HTTP_AUTHORIZATION'] = "Bearer #{token.token}"
       end
 
-      it 'raises an error' do
-        expect_any_instance_of(self.class).to receive(:initial_current_user).and_return(user)
+      it_behaves_like 'sudo'
+    end
 
-        expect { authenticate! }.to raise_error '401 - {"message"=>"401 Unauthorized"}'
+    context 'using a personal access token' do
+      let(:token) { create(:personal_access_token) }
+
+      context 'passed as param' do
+        before do
+          params[API::APIGuard::PRIVATE_TOKEN_PARAM] = token.token
+        end
+
+        it_behaves_like 'sudo'
       end
 
-      it "doesn't raise an error if an admin user is impersonating a blocked user (via sudo)" do
-        admin_user = build(:user, :admin)
+      context 'passed as header' do
+        before do
+          env[API::APIGuard::PRIVATE_TOKEN_HEADER] = token.token
+        end
 
-        expect_any_instance_of(self.class).to receive(:initial_current_user).and_return(admin_user)
+        it_behaves_like 'sudo'
+      end
+    end
 
-        expect { authenticate! }.not_to raise_error
+    context 'using warden authentication' do
+      before do
+        warden_authenticate_returns admin
+        env[API::Helpers::SUDO_HEADER] = user.username
+      end
+
+      it 'raises an error' do
+        expect { current_user }.to raise_error /Must be authenticated using an OAuth or Personal Access Token to use sudo/
       end
     end
   end