Merge branch 'master' of gitlab.com:gitlab-org/gitlab-ce into 18608-lock-issues-v2

# Conflicts:
#	db/schema.rb

6 files changed, 82 insertions, 38 deletions

diff --git a/spec/requests/api/commits_spec.rb b/spec/requests/api/commits_spec.rb
index f663719d28c..94462b4572d 100644
--- a/spec/requests/api/commits_spec.rb
+++ b/spec/requests/api/commits_spec.rb
@@ -491,6 +491,7 @@ describe API::Commits do
         expect(json_response['stats']['deletions']).to eq(commit.stats.deletions)
         expect(json_response['stats']['total']).to eq(commit.stats.total)
         expect(json_response['status']).to be_nil
+        expect(json_response['last_pipeline']).to be_nil
       end
 
       context 'when ref does not exist' do
@@ -573,6 +574,10 @@ describe API::Commits do
           expect(response).to have_http_status(200)
           expect(response).to match_response_schema('public_api/v4/commit/detail')
           expect(json_response['status']).to eq('created')
+          expect(json_response['last_pipeline']['id']).to eq(pipeline.id)
+          expect(json_response['last_pipeline']['ref']).to eq(pipeline.ref)
+          expect(json_response['last_pipeline']['sha']).to eq(pipeline.sha)
+          expect(json_response['last_pipeline']['status']).to eq(pipeline.status)
         end
 
         context 'when pipeline succeeds' do
diff --git a/spec/requests/api/environments_spec.rb b/spec/requests/api/environments_spec.rb
index 2361809e0e1..f8cd529a06c 100644
--- a/spec/requests/api/environments_spec.rb
+++ b/spec/requests/api/environments_spec.rb
@@ -20,6 +20,7 @@ describe API::Environments do
           path path_with_namespace
           star_count forks_count
           created_at last_activity_at
+          avatar_url
         )
 
         get api("/projects/#{project.id}/environments", user)
diff --git a/spec/requests/api/helpers_spec.rb b/spec/requests/api/helpers_spec.rb
index d4006fe71a2..060c8902471 100644
--- a/spec/requests/api/helpers_spec.rb
+++ b/spec/requests/api/helpers_spec.rb
@@ -159,18 +159,25 @@ describe API::Helpers do
     end
 
     describe "when authenticating using a user's private token" do
-      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'
         allow_any_instance_of(self.class).to receive(:doorkeeper_guard) { false }
 
-        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 401 response 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
+        expect { current_user }.to raise_error /401/
+      end
+
+      it 'returns a 401 response for a user who is blocked' do
+        user.block!
+        env[API::APIGuard::PRIVATE_TOKEN_HEADER] = user.private_token
+
+        expect { current_user }.to raise_error /401/
       end
 
       it "leaves user as is when sudo not specified" do
@@ -193,24 +200,31 @@ describe API::Helpers 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 401 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 /401/
       end
 
-      it "returns nil for a token without the appropriate scope" do
+      it 'returns a 401 response for a user who is blocked' do
+        user.block!
+        env[API::APIGuard::PRIVATE_TOKEN_HEADER] = personal_access_token.token
+
+        expect { current_user }.to raise_error /401/
+      end
+
+      it "returns a 401 response for a token 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 be_nil
+        expect { current_user }.to raise_error /401/
       end
 
       it "leaves user as is when sudo not specified" do
@@ -226,14 +240,14 @@ describe API::Helpers 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 /401/
       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
+        expect { current_user }.to raise_error /401/
       end
     end
 
@@ -351,6 +365,18 @@ describe API::Helpers do
             end
           end
         end
+
+        context 'when user is blocked' do
+          before do
+            user.block!
+          end
+
+          it 'changes current_user to sudo' do
+            set_env(admin, user.id)
+
+            expect(current_user).to eq(user)
+          end
+        end
       end
 
       context 'with regular user' do
@@ -454,6 +480,27 @@ describe API::Helpers do
 
       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
   end
 
   describe '.authenticate_non_get!' do
@@ -490,11 +537,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,35 +548,12 @@ 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
-
-    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
diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb
index 508df990952..18f6f7df1fa 100644
--- a/spec/requests/api/projects_spec.rb
+++ b/spec/requests/api/projects_spec.rb
@@ -193,6 +193,7 @@ describe API::Projects do
             path path_with_namespace
             star_count forks_count
             created_at last_activity_at
+            avatar_url
           )
 
           get api('/projects?simple=true', user)
diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb
index 5b306ec6cbf..69c8aa4482a 100644
--- a/spec/requests/api/users_spec.rb
+++ b/spec/requests/api/users_spec.rb
@@ -125,6 +125,15 @@ describe API::Users do
     end
 
     context "when admin" do
+      context 'when sudo is defined' do
+        it 'does not return 500' do
+          admin_personal_access_token = create(:personal_access_token, user: admin).token
+          get api("/users?private_token=#{admin_personal_access_token}&sudo=#{user.id}", admin)
+
+          expect(response).to have_http_status(:success)
+        end
+      end
+
       it "returns an array of users" do
         get api("/users", admin)
 
@@ -1896,4 +1905,8 @@ describe API::Users do
       expect(impersonation_token.reload.revoked).to be_truthy
     end
   end
+
+  include_examples 'custom attributes endpoints', 'users' do
+    let(:attributable) { user }
+  end
 end
diff --git a/spec/requests/api/v3/projects_spec.rb b/spec/requests/api/v3/projects_spec.rb
index cae2c3118da..e5282c3311f 100644
--- a/spec/requests/api/v3/projects_spec.rb
+++ b/spec/requests/api/v3/projects_spec.rb
@@ -89,6 +89,7 @@ describe API::V3::Projects do
             path path_with_namespace
             star_count forks_count
             created_at last_activity_at
+            avatar_url
           )
 
           get v3_api('/projects?simple=true', user)