Add latest changes from gitlab-org/security/gitlab@12-7-stable-ee

lib/api/commits.rb

@@ -154,7 +154,7 @@ module API
         not_found! 'Commit' unless commit
-        present commit, with: Entities::CommitDetail, stats: params[:stats]
+        present commit, with: Entities::CommitDetail, stats: params[:stats], current_user: current_user
       end
       desc 'Get the diff for a specific commit of a project' do

lib/api/entities.rb

@@ -491,8 +491,18 @@ module API
     class CommitDetail < Commit
       expose :stats, using: Entities::CommitStats, if: :stats
       expose :status
-      expose :last_pipeline, using: 'API::Entities::PipelineBasic'
       expose :project_id
+
+      expose :last_pipeline do |commit, options|
+        pipeline = commit.last_pipeline if can_read_pipeline?
+        ::API::Entities::PipelineBasic.represent(pipeline, options)
+      end
+
+      private
+
+      def can_read_pipeline?
+        Ability.allowed?(options[:current_user], :read_pipeline, object.last_pipeline)
+      end
     end
     class CommitSignature < Grape::Entity

lib/gitlab/git/cross_repo_comparer.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Git
+    class CrossRepoComparer
+      attr_reader :source_repo, :target_repo
+
+      def initialize(source_repo, target_repo)
+        @source_repo = source_repo
+        @target_repo = target_repo
+      end
+
+      def compare(source_ref, target_ref, straight:)
+        ensuring_ref_in_source(target_ref) do |target_commit_id|
+          Gitlab::Git::Compare.new(
+            source_repo,
+            target_commit_id,
+            source_ref,
+            straight: straight
+          )
+        end
+      end
+
+      private
+
+      def ensuring_ref_in_source(ref, &blk)
+        return yield ref if source_repo == target_repo
+
+        # If the commit doesn't exist in the target, there's nothing we can do
+        commit_id = target_repo.commit(ref)&.sha
+        return unless commit_id
+
+        # The commit pointed to by ref may exist in the source even when they
+        # are different repositories. This is particularly true of close forks,
+        # but may also be the case if a temporary ref for this comparison has
+        # already been created in the past, and the result hasn't been GC'd yet.
+        return yield commit_id if source_repo.commit(commit_id)
+
+        # Worst case: the commit is not in the source repo so we need to fetch
+        # it. Use a temporary ref and clean up afterwards
+        with_commit_in_source_tmp(commit_id, &blk)
+      end
+
+      # Fetch the ref into source_repo from target_repo, using a temporary ref
+      # name that will be deleted once the method completes. This is a no-op if
+      # fetching the source branch fails
+      def with_commit_in_source_tmp(commit_id, &blk)
+        tmp_ref = "refs/tmp/#{SecureRandom.hex}"
+
+        yield commit_id if source_repo.fetch_source_branch!(target_repo, commit_id, tmp_ref)
+      ensure
+        source_repo.delete_refs(tmp_ref) # best-effort
+      end
+    end
+  end
+end

lib/gitlab/git/repository.rb

@@ -746,29 +746,9 @@ module Gitlab
       end
       def compare_source_branch(target_branch_name, source_repository, source_branch_name, straight:)
-        reachable_ref =
-          if source_repository == self
-            source_branch_name
-          else
-            # If a tmp ref was created before for a separate repo comparison (forks),
-            # we're able to short-circuit the tmp ref re-creation:
-            # 1. Take the SHA from the source repo
-            # 2. Read that in the current "target" repo
-            # 3. If that SHA is still known (readable), it means GC hasn't
-            # cleaned it up yet, so we can use it instead re-writing the tmp ref.
-            source_commit_id = source_repository.commit(source_branch_name)&.sha
-            commit(source_commit_id)&.sha if source_commit_id
-          end
-
-        return compare(target_branch_name, reachable_ref, straight: straight) if reachable_ref
-
-        tmp_ref = "refs/tmp/#{SecureRandom.hex}"
-
-        return unless fetch_source_branch!(source_repository, source_branch_name, tmp_ref)
-
-        compare(target_branch_name, tmp_ref, straight: straight)
-      ensure
-        delete_refs(tmp_ref) if tmp_ref
+        CrossRepoComparer
+          .new(source_repository, self)
+          .compare(source_branch_name, target_branch_name, straight: straight)
       end
       def write_ref(ref_path, ref, old_ref: nil)
@@ -1046,13 +1026,6 @@ module Gitlab
       private
-      def compare(base_ref, head_ref, straight:)
-        Gitlab::Git::Compare.new(self,
-                                 base_ref,
-                                 head_ref,
-                                 straight: straight)
-      end
-
       def empty_diff_stats
         Gitlab::Git::DiffStatsCollection.new([])
       end

spec/controllers/dashboard_controller_spec.rb

@@ -23,6 +23,47 @@ describe DashboardController do
     end
   end
+  describe "GET activity as JSON" do
+    render_views
+
+    let(:user) { create(:user) }
+    let(:project) { create(:project, :public, issues_access_level: ProjectFeature::PRIVATE) }
+
+    before do
+      create(:event, :created, project: project, target: create(:issue))
+
+      sign_in(user)
+
+      request.cookies[:event_filter] = 'all'
+    end
+
+    context 'when user has permission to see the event' do
+      before do
+        project.add_developer(user)
+      end
+
+      it 'returns count' do
+        get :activity, params: { format: :json }
+
+        expect(json_response['count']).to eq(1)
+      end
+    end
+
+    context 'when user has no permission to see the event' do
+      it 'filters out invisible event' do
+        get :activity, params: { format: :json }
+
+        expect(json_response['html']).to include(_('No activities found'))
+      end
+
+      it 'filters out invisible event when calculating the count' do
+        get :activity, params: { format: :json }
+
+        expect(json_response['count']).to eq(0)
+      end
+    end
+  end
+
   it_behaves_like 'authenticates sessionless user', :issues, :atom, author_id: User.first
   it_behaves_like 'authenticates sessionless user', :issues_calendar, :ics

spec/controllers/groups_controller_spec.rb

@@ -47,7 +47,7 @@ describe GroupsController do
       it 'assigns events for all the projects in the group', :sidekiq_might_not_need_inline do
         subject
-        expect(assigns(:events)).to contain_exactly(event)
+        expect(assigns(:events).map(&:id)).to contain_exactly(event.id)
       end
     end
   end
@@ -119,12 +119,12 @@ describe GroupsController do
   describe 'GET #activity' do
     render_views
-    before do
-      sign_in(user)
-      project
-    end
-
     context 'as json' do
+      before do
+        sign_in(user)
+        project
+      end
+
       it 'includes events from all projects in group and subgroups', :sidekiq_might_not_need_inline do
         2.times do
           project = create(:project, group: group)
@@ -141,6 +141,31 @@ describe GroupsController do
         expect(assigns(:projects).limit_value).to be_nil
       end
     end
+
+    context 'when user has no permission to see the event' do
+      let(:user) { create(:user) }
+      let(:group) { create(:group) }
+      let(:project) { create(:project, group: group) }
+
+      let(:project_with_restricted_access) do
+        create(:project, :public, issues_access_level: ProjectFeature::PRIVATE, group: group)
+      end
+
+      before do
+        create(:event, project: project)
+        create(:event, :created, project: project_with_restricted_access, target: create(:issue))
+
+        group.add_guest(user)
+
+        sign_in(user)
+      end
+
+      it 'filters out invisible event' do
+        get :activity, params: { id: group.to_param }, format: :json
+
+        expect(json_response['count']).to eq(1)
+      end
+    end
   end
   describe 'POST #create' do

spec/controllers/projects_controller_spec.rb

@@ -64,6 +64,46 @@ describe ProjectsController do
     end
   end
+  describe "GET #activity as JSON" do
+    render_views
+
+    let(:project) { create(:project, :public, issues_access_level: ProjectFeature::PRIVATE) }
+
+    before do
+      create(:event, :created, project: project, target: create(:issue))
+
+      sign_in(user)
+
+      request.cookies[:event_filter] = 'all'
+    end
+
+    context 'when user has permission to see the event' do
+      before do
+        project.add_developer(user)
+      end
+
+      it 'returns count' do
+        get :activity, params: { namespace_id: project.namespace, id: project, format: :json }
+
+        expect(json_response['count']).to eq(1)
+      end
+    end
+
+    context 'when user has no permission to see the event' do
+      it 'filters out invisible event' do
+        get :activity, params: { namespace_id: project.namespace, id: project, format: :json }
+
+        expect(json_response['html']).to eq("\n")
+      end
+
+      it 'filters out invisible event when calculating the count' do
+        get :activity, params: { namespace_id: project.namespace, id: project, format: :json }
+
+        expect(json_response['count']).to eq(0)
+      end
+    end
+  end
+
   describe "GET show" do
     context "user not project member" do
       before do

spec/javascripts/frequent_items/utils_spec.js

 import { GlBreakpointInstance as bp } from '@gitlab/ui/dist/utils';
-import { isMobile, getTopFrequentItems, updateExistingFrequentItem } from '~/frequent_items/utils';
+import {
+  isMobile,
+  getTopFrequentItems,
+  updateExistingFrequentItem,
+  sanitizeItem,
+} from '~/frequent_items/utils';
 import { HOUR_IN_MS, FREQUENT_ITEMS } from '~/frequent_items/constants';
 import { mockProject, unsortedFrequentItems, sortedFrequentItems } from './mock_data';
@@ -92,4 +97,16 @@ describe('Frequent Items utils spec', () => {
       expect(result.frequency).toBe(mockedProject.frequency);
     });
   });
+
+  describe('sanitizeItem', () => {
+    it('strips HTML tags for name and namespace', () => {
+      const input = {
+        name: '<br><b>test</b>',
+        namespace: '<br>test',
+        id: 1,
+      };
+
+      expect(sanitizeItem(input)).toEqual({ name: 'test', namespace: 'test', id: 1 });
+    });
+  });
 });

spec/lib/gitlab/git/cross_repo_comparer_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::Git::CrossRepoComparer do
+  let(:source_project) { create(:project, :repository) }
+  let(:target_project) { create(:project, :repository) }
+
+  let(:source_repo) { source_project.repository.raw_repository }
+  let(:target_repo) { target_project.repository.raw_repository }
+
+  let(:source_branch) { 'feature' }
+  let(:target_branch) { 'master' }
+  let(:straight) { false }
+
+  let(:source_commit) { source_repo.commit(source_branch) }
+  let(:target_commit) { source_repo.commit(target_branch) }
+
+  subject(:result) { described_class.new(source_repo, target_repo).compare(source_branch, target_branch, straight: straight) }
+
+  describe '#compare' do
+    context 'within a single repository' do
+      let(:target_project) { source_project }
+
+      context 'a non-straight comparison' do
+        it 'compares without fetching from another repo' do
+          expect(source_repo).not_to receive(:fetch_source_branch!)
+
+          expect_compare(result, from: source_commit, to: target_commit)
+          expect(result.straight).to eq(false)
+        end
+      end
+
+      context 'a straight comparison' do
+        let(:straight) { true }
+
+        it 'compares without fetching from another repo' do
+          expect(source_repo).not_to receive(:fetch_source_branch!)
+
+          expect_compare(result, from: source_commit, to: target_commit)
+          expect(result.straight).to eq(true)
+        end
+      end
+    end
+
+    context 'across two repositories' do
+      context 'target ref exists in source repo' do
+        it 'compares without fetching from another repo' do
+          expect(source_repo).not_to receive(:fetch_source_branch!)
+          expect(source_repo).not_to receive(:delete_refs)
+
+          expect_compare(result, from: source_commit, to: target_commit)
+        end
+      end
+
+      context 'target ref does not exist in source repo' do
+        it 'compares in the source repo by fetching from the target to a temporary ref' do
+          new_commit_id = create_commit(target_project.owner, target_repo, target_branch)
+          new_commit = target_repo.commit(new_commit_id)
+
+          # This is how the temporary ref is generated
+          expect(SecureRandom).to receive(:hex).at_least(:once).and_return('foo')
+
+          expect(source_repo)
+            .to receive(:fetch_source_branch!)
+            .with(target_repo, new_commit_id, 'refs/tmp/foo')
+            .and_call_original
+
+          expect(source_repo).to receive(:delete_refs).with('refs/tmp/foo').and_call_original
+
+          expect_compare(result, from: source_commit, to: new_commit)
+        end
+      end
+
+      context 'source ref does not exist in source repo' do
+        let(:source_branch) { 'does-not-exist' }
+
+        it 'returns an empty comparison' do
+          expect(source_repo).not_to receive(:fetch_source_branch!)
+          expect(source_repo).not_to receive(:delete_refs)
+
+          expect(result).to be_a(::Gitlab::Git::Compare)
+          expect(result.commits.size).to eq(0)
+        end
+      end
+
+      context 'target ref does not exist in target repo' do
+        let(:target_branch) { 'does-not-exist' }
+
+        it 'returns nil' do
+          expect(source_repo).not_to receive(:fetch_source_branch!)
+          expect(source_repo).not_to receive(:delete_refs)
+
+          is_expected.to be_nil
+        end
+      end
+    end
+  end
+
+  def expect_compare(of, from:, to:)
+    expect(of).to be_a(::Gitlab::Git::Compare)
+    expect(from).to be_a(::Gitlab::Git::Commit)
+    expect(to).to be_a(::Gitlab::Git::Commit)
+
+    expect(of.commits).not_to be_empty
+    expect(of.head).to eq(from)
+    expect(of.base).to eq(to)
+  end
+
+  def create_commit(user, repo, branch)
+    action = { action: :create, file_path: '/FILE', content: 'content' }
+
+    result = repo.multi_action(user, branch_name: branch, message: 'Commit', actions: [action])
+
+    result.newrev
+  end
+end

spec/lib/gitlab/git/repository_spec.rb

@@ -1962,66 +1962,15 @@ describe Gitlab::Git::Repository, :seed_helper do
   end
   describe '#compare_source_branch' do
-    let(:repository) { Gitlab::Git::Repository.new('default', TEST_GITATTRIBUTES_REPO_PATH, '', 'group/project') }
-
-    context 'within same repository' do
-      it 'does not create a temp ref' do
-        expect(repository).not_to receive(:fetch_source_branch!)
-        expect(repository).not_to receive(:delete_refs)
-
-        compare = repository.compare_source_branch('master', repository, 'feature', straight: false)
-        expect(compare).to be_a(Gitlab::Git::Compare)
-        expect(compare.commits.count).to be > 0
-      end
-
-      it 'returns empty commits when source ref does not exist' do
-        compare = repository.compare_source_branch('master', repository, 'non-existent-branch', straight: false)
-        expect(compare.commits).to be_empty
+    it 'delegates to Gitlab::Git::CrossRepoComparer' do
+      expect_next_instance_of(::Gitlab::Git::CrossRepoComparer) do |instance|
+        expect(instance.source_repo).to eq(:source_repository)
+        expect(instance.target_repo).to eq(repository)
+        expect(instance).to receive(:compare).with('feature', 'master', straight: :straight)
       end
-    end
-    context 'with different repositories' do
-      context 'when ref is known by source repo, but not by target' do
-        before do
-          mutable_repository.write_ref('another-branch', 'feature')
-        end
-
-        it 'creates temp ref' do
-          expect(repository).not_to receive(:fetch_source_branch!)
-          expect(repository).not_to receive(:delete_refs)
-
-          compare = repository.compare_source_branch('master', mutable_repository, 'another-branch', straight: false)
-          expect(compare).to be_a(Gitlab::Git::Compare)
-          expect(compare.commits.count).to be > 0
-        end
-      end
-
-      context 'when ref is known by source and target repos' do
-        before do
-          mutable_repository.write_ref('another-branch', 'feature')
-          repository.write_ref('another-branch', 'feature')
-        end
-
-        it 'does not create a temp ref' do
-          expect(repository).not_to receive(:fetch_source_branch!)
-          expect(repository).not_to receive(:delete_refs)
-
-          compare = repository.compare_source_branch('master', mutable_repository, 'another-branch', straight: false)
-          expect(compare).to be_a(Gitlab::Git::Compare)
-          expect(compare.commits.count).to be > 0
-        end
-      end
-
-      context 'when ref is unknown by source repo' do
-        it 'returns nil when source ref does not exist' do
-          expect(repository).to receive(:fetch_source_branch!).and_call_original
-          expect(repository).to receive(:delete_refs).and_call_original
-
-          compare = repository.compare_source_branch('master', mutable_repository, 'non-existent-branch', straight: false)
-          expect(compare).to be_nil
-        end
-      end
+      repository.compare_source_branch('master', :source_repository, 'feature', straight: :straight)
     end
   end

spec/models/generic_commit_status_spec.rb

@@ -19,6 +19,74 @@ describe GenericCommitStatus do
     it { is_expected.not_to allow_value('javascript:alert(1)').for(:target_url) }
   end
+  describe '#name_uniqueness_across_types' do
+    let(:attributes) { {} }
+    let(:commit_status) { described_class.new(attributes) }
+    let(:status_name) { 'test-job' }
+
+    subject(:errors) { commit_status.errors[:name] }
+
+    shared_examples 'it does not have uniqueness errors' do
+      it 'does not return errors' do
+        commit_status.valid?
+
+        is_expected.to be_empty
+      end
+    end
+
+    context 'without attributes' do
+      it_behaves_like 'it does not have uniqueness errors'
+    end
+
+    context 'with only a pipeline' do
+      let(:attributes) { { pipeline: pipeline } }
+
+      context 'without name' do
+        it_behaves_like 'it does not have uniqueness errors'
+      end
+    end
+
+    context 'with only a name' do
+      let(:attributes) { { name: status_name } }
+
+      context 'without pipeline' do
+        it_behaves_like 'it does not have uniqueness errors'
+      end
+    end
+
+    context 'with pipeline and name' do
+      let(:attributes) do
+        {
+          pipeline: pipeline,
+          name: status_name
+        }
+      end
+
+      context 'without other statuses' do
+        it_behaves_like 'it does not have uniqueness errors'
+      end
+
+      context 'with generic statuses' do
+        before do
+          create(:generic_commit_status, pipeline: pipeline, name: status_name)
+        end
+
+        it_behaves_like 'it does not have uniqueness errors'
+      end
+
+      context 'with ci_build statuses' do
+        before do
+          create(:ci_build, pipeline: pipeline, name: status_name)
+        end
+
+        it 'returns name error' do
+          expect(commit_status).to be_invalid
+          is_expected.to include('has already been taken')
+        end
+      end
+    end
+  end
+
   describe '#context' do
     subject { generic_commit_status.context }
@@ -79,6 +147,12 @@ describe GenericCommitStatus do
       it { is_expected.not_to be_nil }
     end
+
+    describe '#stage_idx' do
+      subject { generic_commit_status.stage_idx }
+
+      it { is_expected.not_to be_nil }
+    end
   end
   describe '#present' do

spec/policies/global_policy_spec.rb

@@ -141,6 +141,34 @@ describe GlobalPolicy do
         it { is_expected.to be_allowed(:access_api) }
       end
     end
+
+    context 'inactive user' do
+      before do
+        current_user.update!(confirmed_at: nil, confirmation_sent_at: 5.days.ago)
+      end
+
+      context 'when within the confirmation grace period' do
+        before do
+          allow(User).to receive(:allow_unconfirmed_access_for).and_return(10.days)
+        end
+
+        it { is_expected.to be_allowed(:access_api) }
+      end
+
+      context 'when confirmation grace period is expired' do
+        before do
+          allow(User).to receive(:allow_unconfirmed_access_for).and_return(2.days)
+        end
+
+        it { is_expected.not_to be_allowed(:access_api) }
+      end
+
+      it 'when `inactive_policy_condition` feature flag is turned off' do
+        stub_feature_flags(inactive_policy_condition: false)
+
+        is_expected.to be_allowed(:access_api)
+      end
+    end
   end
   describe 'receive notifications' do
@@ -202,6 +230,20 @@ describe GlobalPolicy do
       it { is_expected.not_to be_allowed(:access_git) }
     end
+    describe 'inactive user' do
+      before do
+        current_user.update!(confirmed_at: nil)
+      end
+
+      it { is_expected.not_to be_allowed(:access_git) }
+
+      it 'when `inactive_policy_condition` feature flag is turned off' do
+        stub_feature_flags(inactive_policy_condition: false)
+
+        is_expected.to be_allowed(:access_git)
+      end
+    end
+
     context 'when terms are enforced' do
       before do
         enforce_terms
@@ -298,6 +340,20 @@ describe GlobalPolicy do
       it { is_expected.not_to be_allowed(:use_slash_commands) }
     end
+    describe 'inactive user' do
+      before do
+        current_user.update!(confirmed_at: nil)
+      end
+
+      it { is_expected.not_to be_allowed(:use_slash_commands) }
+
+      it 'when `inactive_policy_condition` feature flag is turned off' do
+        stub_feature_flags(inactive_policy_condition: false)
+
+        is_expected.to be_allowed(:use_slash_commands)
+      end
+    end
+
     context 'when access locked' do
       before do
         current_user.lock_access!

spec/requests/api/commit_statuses_spec.rb

@@ -164,6 +164,7 @@ describe API::CommitStatuses do
               expect(response).to have_gitlab_http_status(201)
               expect(job.status).to eq('pending')
+              expect(job.stage_idx).to eq(GenericCommitStatus::EXTERNAL_STAGE_IDX)
             end
           end
@@ -331,6 +332,29 @@ describe API::CommitStatuses do
         end
       end
+      context 'when updating a protected ref' do
+        before do
+          create(:protected_branch, project: project, name: 'master')
+          post api(post_url, user), params: { state: 'running', ref: 'master' }
+        end
+
+        context 'with user as developer' do
+          let(:user) { developer }
+
+          it 'does not create commit status' do
+            expect(response).to have_gitlab_http_status(403)
+          end
+        end
+
+        context 'with user as maintainer' do
+          let(:user) { create_user(:maintainer) }
+
+          it 'creates commit status' do
+            expect(response).to have_gitlab_http_status(201)
+          end
+        end
+      end
+
       context 'when commit SHA is invalid' do
         let(:sha) { 'invalid_sha' }
@@ -372,6 +396,22 @@ describe API::CommitStatuses do
               .to include 'is blocked: Only allowed schemes are http, https'
         end
       end
+
+      context 'when trying to update a status of a different type' do
+        let!(:pipeline) { create(:ci_pipeline, project: project, sha: sha, ref: 'ref') }
+        let!(:ci_build) { create(:ci_build, pipeline: pipeline, name: 'test-job') }
+        let(:params) { { state: 'pending', name: 'test-job' } }
+
+        before do
+          post api(post_url, developer), params: params
+        end
+
+        it 'responds with bad request status and validation errors' do
+          expect(response).to have_gitlab_http_status(400)
+          expect(json_response['message']['name'])
+              .to include 'has already been taken'
+        end
+      end
     end
     context 'reporter user' do

spec/requests/api/commits_spec.rb

@@ -8,6 +8,7 @@ describe API::Commits do
   let(:user) { create(:user) }
   let(:guest) { create(:user).tap { |u| project.add_guest(u) } }
+  let(:developer) { create(:user).tap { |u| project.add_developer(u) } }
   let(:project) { create(:project, :repository, creator: user, path: 'my.project') }
   let(:branch_with_dot) { project.repository.find_branch('ends-with.json') }
   let(:branch_with_slash) { project.repository.find_branch('improve/awesome') }
@@ -964,6 +965,56 @@ describe API::Commits do
       end
     end
+    shared_examples_for 'ref with pipeline' do
+      let!(:pipeline) do
+        project
+          .ci_pipelines
+          .create!(source: :push, ref: 'master', sha: commit.sha, protected: false)
+      end
+
+      it 'includes status as "created" and a last_pipeline object' do
+        get api(route, current_user)
+
+        expect(response).to have_gitlab_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
+        before do
+          pipeline.update!(status: 'success')
+        end
+
+        it 'includes a "success" status' do
+          get api(route, current_user)
+
+          expect(response).to have_gitlab_http_status(200)
+          expect(response).to match_response_schema('public_api/v4/commit/detail')
+          expect(json_response['status']).to eq('success')
+        end
+      end
+    end
+
+    shared_examples_for 'ref with unaccessible pipeline' do
+      let!(:pipeline) do
+        project
+          .ci_pipelines
+          .create!(source: :push, ref: 'master', sha: commit.sha, protected: false)
+      end
+
+      it 'does not include last_pipeline' do
+        get api(route, current_user)
+
+        expect(response).to match_response_schema('public_api/v4/commit/detail')
+        expect(response).to have_gitlab_http_status(200)
+        expect(json_response['last_pipeline']).to be_nil
+      end
+    end
+
     context 'when stat param' do
       let(:route) { "/projects/#{project_id}/repository/commits/#{commit_id}" }
@@ -993,6 +1044,15 @@ describe API::Commits do
       let(:project) { create(:project, :public, :repository) }
       it_behaves_like 'ref commit'
+      it_behaves_like 'ref with pipeline'
+
+      context 'with private builds' do
+        before do
+          project.project_feature.update!(builds_access_level: ProjectFeature::PRIVATE)
+        end
+
+        it_behaves_like 'ref with unaccessible pipeline'
+      end
     end
     context 'when unauthenticated', 'and project is private' do
@@ -1006,6 +1066,17 @@ describe API::Commits do
       let(:current_user) { user }
       it_behaves_like 'ref commit'
+      it_behaves_like 'ref with pipeline'
+
+      context 'when builds are disabled' do
+        before do
+          project
+            .project_feature
+            .update!(builds_access_level: ProjectFeature::DISABLED)
+        end
+
+        it_behaves_like 'ref with unaccessible pipeline'
+      end
       context 'when branch contains a dot' do
         let(:commit) { project.repository.commit(branch_with_dot.name) }
@@ -1041,35 +1112,53 @@ describe API::Commits do
           it_behaves_like 'ref commit'
         end
       end
-      context 'when the ref has a pipeline' do
-        let!(:pipeline) { project.ci_pipelines.create(source: :push, ref: 'master', sha: commit.sha, protected: false) }
-        it 'includes a "created" status' do
-          get api(route, current_user)
-          expect(response).to have_gitlab_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 authenticated', 'as a developer' do
+      let(:current_user) { developer }
+      it_behaves_like 'ref commit'
+      it_behaves_like 'ref with pipeline'
+      context 'with private builds' do
+        before do
+          project.project_feature.update!(builds_access_level: ProjectFeature::PRIVATE)
         end
-        context 'when pipeline succeeds' do
-          before do
-            pipeline.update(status: 'success')
-          end
-          it 'includes a "success" status' do
-            get api(route, current_user)
-            expect(response).to have_gitlab_http_status(200)
-            expect(response).to match_response_schema('public_api/v4/commit/detail')
-            expect(json_response['status']).to eq('success')
-          end
+        it_behaves_like 'ref with pipeline'
+      end
+    end
+    context 'when authenticated', 'as a guest' do
+      let(:current_user) { guest }
+      it_behaves_like '403 response' do
+        let(:request) { get api(route, guest) }
+        let(:message) { '403 Forbidden' }
+      end
+    end
+
+    context 'when authenticated', 'as a non member' do
+      let(:current_user) { create(:user) }
+
+      it_behaves_like '403 response' do
+        let(:request) { get api(route, guest) }
+        let(:message) { '403 Forbidden' }
+      end
+    end
+
+    context 'when authenticated', 'as non_member and project is public' do
+      let(:current_user) { create(:user) }
+      let(:project) { create(:project, :public, :repository) }
+
+      it_behaves_like 'ref with pipeline'
+
+      context 'with private builds' do
+        before do
+          project.project_feature.update!(builds_access_level: ProjectFeature::PRIVATE)
         end
+
+        it_behaves_like 'ref with unaccessible pipeline'
       end
     end
   end

spec/requests/api/oauth_tokens_spec.rb

@@ -30,26 +30,40 @@ describe 'OAuth tokens' do
       end
     end
-    context "when user is blocked" do
-      it "does not create an access token" do
-        user = create(:user)
+    shared_examples 'does not create an access token' do
+      let(:user) { create(:user) }
+
+      it { expect(response).to have_gitlab_http_status(401) }
+    end
+
+    context 'when user is blocked' do
+      before do
         user.block
         request_oauth_token(user)
-
-        expect(response).to have_gitlab_http_status(401)
       end
+
+      include_examples 'does not create an access token'
     end
-    context "when user is ldap_blocked" do
-      it "does not create an access token" do
-        user = create(:user)
+    context 'when user is ldap_blocked' do
+      before do
         user.ldap_block
         request_oauth_token(user)
-        expect(response).to have_gitlab_http_status(401)
+      end
+      include_examples 'does not create an access token'
+    end
+
+    context 'when user account is not confirmed' do
+      before do
+        user.update!(confirmed_at: nil)
+
+        request_oauth_token(user)
       end
+
+      include_examples 'does not create an access token'
     end
   end
 end

spec/services/projects/group_links/destroy_service_spec.rb

@@ -3,8 +3,8 @@
 require 'spec_helper'
 describe Projects::GroupLinks::DestroyService, '#execute' do
-  let(:group_link) { create :project_group_link }
-  let(:project) { group_link.project }
+  let(:project) { create(:project, :private) }
+  let!(:group_link) { create(:project_group_link, project: project) }
   let(:user) { create :user }
   let(:subject) { described_class.new(project, user) }
@@ -15,4 +15,39 @@ describe Projects::GroupLinks::DestroyService, '#execute' do
   it 'returns false if group_link is blank' do
     expect { subject.execute(nil) }.not_to change { project.project_group_links.count }
   end
+
+  describe 'todos cleanup' do
+    context 'when project is private' do
+      it 'triggers todos cleanup' do
+        expect(TodosDestroyer::ProjectPrivateWorker).to receive(:perform_in).with(Todo::WAIT_FOR_DELETE, project.id)
+        expect(project.private?).to be true
+
+        subject.execute(group_link)
+      end
+    end
+
+    context 'when project is public or internal' do
+      shared_examples_for 'removes confidential todos' do
+        it 'does not trigger todos cleanup' do
+          expect(TodosDestroyer::ProjectPrivateWorker).not_to receive(:perform_in).with(Todo::WAIT_FOR_DELETE, project.id)
+          expect(TodosDestroyer::ConfidentialIssueWorker).to receive(:perform_in).with(Todo::WAIT_FOR_DELETE, nil, project.id)
+          expect(project.private?).to be false
+
+          subject.execute(group_link)
+        end
+      end
+
+      context 'when project is public' do
+        let(:project) { create(:project, :public) }
+
+        it_behaves_like 'removes confidential todos'
+      end
+
+      context 'when project is internal' do
+        let(:project) { create(:project, :public) }
+
+        it_behaves_like 'removes confidential todos'
+      end
+    end
+  end
 end