Add latest changes from gitlab-org/gitlab@master

app/controllers/concerns/issuable_actions.rb

@@ -137,7 +137,7 @@ module IssuableActions
     end
 
     notes = prepare_notes_for_rendering(notes)
-    notes = notes.select { |n| n.visible_for?(current_user) }
+    notes = notes.select { |n| n.readable_by?(current_user) }
 
     discussions = Discussion.build_collection(notes, issuable)
 

app/controllers/concerns/notes_actions.rb

@@ -29,7 +29,7 @@ module NotesActions
     end
 
     notes = prepare_notes_for_rendering(notes)
-    notes = notes.select { |n| n.visible_for?(current_user) }
+    notes = notes.select { |n| n.readable_by?(current_user) }
 
     notes_json[:notes] =
       if use_note_serializer?

app/models/ci/job_artifact.rb

@@ -74,7 +74,7 @@ module Ci
 
     scope :with_files_stored_locally, -> { where(file_store: [nil, ::JobArtifactUploader::Store::LOCAL]) }
     scope :with_files_stored_remotely, -> { where(file_store: ::JobArtifactUploader::Store::REMOTE) }
-    scope :for_sha, ->(sha) { joins(job: :pipeline).where(ci_pipelines: { sha: sha }) }
+    scope :for_sha, ->(sha, project_id) { joins(job: :pipeline).where(ci_pipelines: { sha: sha, project_id: project_id }) }
 
     scope :with_file_types, -> (file_types) do
       types = self.file_types.select { |file_type| file_types.include?(file_type) }.values

app/models/discussion.rb

@@ -19,7 +19,8 @@ class Discussion
             :noteable_ability_name,
             :to_ability_name,
             :editable?,
-            :visible_for?,
+            :system_note_with_references_visible_for?,
+            :resource_parent,
 
             to: :first_note
 

app/models/note.rb

@@ -223,7 +223,7 @@ class Note < ApplicationRecord
   end
 
   # rubocop: disable CodeReuse/ServiceClass
-  def cross_reference?
+  def system_note_with_references?
     return unless system?
 
     if force_cross_reference_regex_check?
@@ -339,12 +339,10 @@ class Note < ApplicationRecord
     super
   end
 
-  def cross_reference_not_visible_for?(user)
-    cross_reference? && !all_referenced_mentionables_allowed?(user)
-  end
-
-  def visible_for?(user)
-    !cross_reference_not_visible_for?(user) && system_note_viewable_by?(user)
+  # This method is to be used for checking read permissions on a note instead of `system_note_with_references_visible_for?`
+  def readable_by?(user)
+    # note_policy accounts for #system_note_with_references_visible_for?(user) check when granting read access
+    Ability.allowed?(user, :read_note, self)
   end
 
   def award_emoji?
@@ -504,6 +502,10 @@ class Note < ApplicationRecord
     noteable.user_mentions.where(note: self)
   end
 
+  def system_note_with_references_visible_for?(user)
+    (!system_note_with_references? || all_referenced_mentionables_allowed?(user)) && system_note_viewable_by?(user)
+  end
+
   private
 
   # Using this method followed by a call to `save` may result in ActiveRecord::RecordNotUnique exception

app/models/project.rb

@@ -1374,7 +1374,7 @@ class Project < ApplicationRecord
     @lfs_storage_project ||= begin
       result = self
 
-      # TODO: Make this go to the fork_network root immeadiatly
+      # TODO: Make this go to the fork_network root immediately
       # dependant on the discussion in: https://gitlab.com/gitlab-org/gitlab-foss/issues/39769
       result = result.fork_source while result&.forked?
 

app/policies/note_policy.rb

 # frozen_string_literal: true
 
 class NotePolicy < BasePolicy
-  delegate { @subject.project }
+  delegate { @subject.resource_parent }
   delegate { @subject.noteable if DeclarativePolicy.has_policy?(@subject.noteable) }
 
   condition(:is_author) { @user && @subject.author == @user }
@@ -11,7 +11,7 @@ class NotePolicy < BasePolicy
 
   condition(:can_read_noteable) { can?(:"read_#{@subject.noteable_ability_name}") }
 
-  condition(:is_visible) { @subject.visible_for?(@user) }
+  condition(:is_visible) { @subject.system_note_with_references_visible_for?(@user) }
 
   rule { ~editable }.prevent :admin_note
 

app/policies/personal_snippet_policy.rb

@@ -7,6 +7,7 @@ class PersonalSnippetPolicy < BasePolicy
 
   rule { public_snippet }.policy do
     enable :read_snippet
+    enable :read_note
     enable :create_note
   end
 
@@ -14,11 +15,13 @@ class PersonalSnippetPolicy < BasePolicy
     enable :read_snippet
     enable :update_snippet
     enable :admin_snippet
+    enable :read_note
     enable :create_note
   end
 
   rule { internal_snippet & ~external_user }.policy do
     enable :read_snippet
+    enable :read_note
     enable :create_note
   end
 

app/services/notification_service.rb

@@ -283,7 +283,7 @@ class NotificationService
     return true unless note.noteable_type.present?
 
     # ignore gitlab service messages
-    return true if note.cross_reference? && note.system?
+    return true if note.system_note_with_references?
 
     send_new_note_notifications(note)
   end

app/views/shared/notes/_note.html.haml

 - return unless note.author
-- return if note.cross_reference_not_visible_for?(current_user)
+- return unless note.readable_by?(current_user)
 
 - show_image_comment_badge = local_assigns.fetch(:show_image_comment_badge, false)
 - note_editable = can?(current_user, :admin_note, note)

doc/topics/autodevops/index.md

@@ -742,15 +742,15 @@ workers:
 > [Introduced](https://gitlab.com/gitlab-org/charts/auto-deploy-app/-/merge_requests/30) in GitLab 12.7.
 
 By default, all Kubernetes pods are
-[non-isolated](https://kubernetes.io/docs/concepts/services-networking/network-policies/#isolated-and-non-isolated-pods)
+[non-isolated](https://kubernetes.io/docs/concepts/services-networking/network-policies/#isolated-and-non-isolated-pods),
 and accept traffic from any source. You can use
 [NetworkPolicy](https://kubernetes.io/docs/concepts/services-networking/network-policies/)
 to restrict connections to selected pods or namespaces.
 
 NOTE: **Note:**
 You must use a Kubernetes network plugin that implements support for
-`NetworkPolicy`, the default network plugin for Kubernetes (`kubenet`)
-[doesn't implement](https://kubernetes.io/docs/concepts/extend-kubernetes/compute-storage-net/network-plugins/#kubenet)
+`NetworkPolicy`. The default network plugin for Kubernetes (`kubenet`)
+[does not implement](https://kubernetes.io/docs/concepts/extend-kubernetes/compute-storage-net/network-plugins/#kubenet)
 support for it. The [Cilium](https://cilium.io/) network plugin can be
 installed as a [cluster application](../../user/clusters/applications.md#install-cilium-using-gitlab-ci)
 to enable support for network policies.
@@ -758,20 +758,20 @@ to enable support for network policies.
 You can enable deployment of a network policy by setting the following
 in the `.gitlab/auto-deploy-values.yaml` file:
 
-```yml
+```yaml
 networkPolicy:
   enabled: true
 ```
 
 The default policy deployed by the auto deploy pipeline will allow
 traffic within a local namespace and from the `gitlab-managed-apps`
-namespace, all other inbound connection will be blocked. Outbound
+namespace. All other inbound connection will be blocked. Outbound
 traffic is not affected by the default policy.
 
 You can also provide a custom [policy specification](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.16/#networkpolicyspec-v1-networking-k8s-io)
 via the `.gitlab/auto-deploy-values.yaml` file, for example:
 
-```yml
+```yaml
 networkPolicy:
   enabled: true
   spec:

doc/topics/git/troubleshooting_git.md

@@ -101,19 +101,38 @@ ssh_exchange_identification: read: Connection reset by peer
 fatal: Could not read from remote repository.
 ```
 
+or
+
+```text
+ssh_exchange_identification: Connection closed by remote host
+fatal: The remote end hung up unexpectedly
+```
+
 This error usually indicates that SSH daemon's `MaxStartups` value is throttling
-SSH connections. This setting specifies the maximum number of unauthenticated
+SSH connections. This setting specifies the maximum number of concurrent, unauthenticated
 connections to the SSH daemon. This affects users with proper authentication
 credentials (SSH keys) because every connection is 'unauthenticated' in the
 beginning. The default value is `10`.
 
-Increase `MaxStartups` by adding or modifying the value in `/etc/ssh/sshd_config`:
+Increase `MaxStartups` on the GitLab server
+by adding or modifying the value in `/etc/ssh/sshd_config`:
 
 ```text
-MaxStartups 100
+MaxStartups 100:30:200
 ```
 
-Restart SSHD for the change to take effect.
+`100:30:200` means up to 100 SSH sessions are allowed without restriction,
+after which 30% of connections will be dropped until reaching an absolute maximum of 200.
+
+Once configured, restart the SSH daemon for the change to take effect.
+
+```shell
+# Debian/Ubuntu
+sudo systemctl restart ssh
+
+# CentOS/RHEL
+sudo service sshd restart
+```
 
 ## Timeout during `git push` / `git pull`
 

doc/update/README.md

@@ -115,17 +115,35 @@ following command:
 
 **For Omnibus installations**
 
+If using GitLab 12.9 and newer, run:
+
 ```shell
 sudo gitlab-rails runner -e production 'puts Gitlab::BackgroundMigration.remaining'
 ```
 
-**For installations from source**
+If using GitLab 12.8 and older, run the following using a Rails console:
 
+```ruby
+puts Sidekiq::Queue.new("background_migration").size
+Sidekiq::ScheduledSet.new.select { |r| r.klass == 'BackgroundMigrationWorker' }.size
 ```
+
+**For installations from source**
+
+If using GitLab 12.9 and newer, run:
+
+```shell
 cd /home/git/gitlab
 sudo -u git -H bundle exec rails runner -e production 'puts Gitlab::BackgroundMigration.remaining'
 ```
 
+If using GitLab 12.8 and older, run the following using a Rails console:
+
+```ruby
+puts Sidekiq::Queue.new("background_migration").size
+Sidekiq::ScheduledSet.new.select { |r| r.klass == 'BackgroundMigrationWorker' }.size
+```
+
 ## Upgrading to a new major version
 
 Major versions are reserved for backwards incompatible changes. We recommend that

doc/user/admin_area/settings/usage_statistics.md

@@ -10,6 +10,9 @@ to perform various actions.
 All statistics are opt-out. You can enable/disable them in the
 **Admin Area > Settings > Metrics and profiling** section **Usage statistics**.
 
+NOTE: **Note:**
+Allow network traffic from your GitLab instance to IP address 104.196.17.203 to send usage statistics to GitLab Inc.
+
 ## Version Check **(CORE ONLY)**
 
 If enabled, version check will inform you if a new version is available and the

doc/user/packages/nuget_repository/index.md

@@ -152,7 +152,7 @@ Where:
 
 ### Upload packages with .NET CLI
 
-This section assumes that your project is properly built and you already [created a NuGet package with .NET CLI](https://docs.microsoft.com/en-us/nuget/create-packages/creating-a-package-dotnet-cli.).
+This section assumes that your project is properly built and you already [created a NuGet package with .NET CLI](https://docs.microsoft.com/en-us/nuget/create-packages/creating-a-package-dotnet-cli).
 Upload your package using the following command:
 
 ```shell

lib/api/discussions.rb

@@ -230,7 +230,7 @@ module API
           .fresh
 
         # Without RendersActions#prepare_notes_for_rendering,
-        # Note#cross_reference_not_visible_for? will attempt to render
+        # Note#system_note_with_references_visible_for? will attempt to render
         # Markdown references mentioned in the note to see whether they
         # should be redacted. For notes that reference a commit, this
         # would also incur a Gitaly call to verify the commit exists.
@@ -239,7 +239,7 @@ module API
         # because notes are redacted if they point to projects that
         # cannot be accessed by the user.
         notes = prepare_notes_for_rendering(notes)
-        notes.select { |n| n.visible_for?(current_user) }
+        notes.select { |n| n.readable_by?(current_user) }
       end
       # rubocop: enable CodeReuse/ActiveRecord
     end

lib/api/helpers/notes_helpers.rb

@@ -62,7 +62,7 @@ module API
 
       def get_note(noteable, note_id)
         note = noteable.notes.with_metadata.find(note_id)
-        can_read_note = note.visible_for?(current_user)
+        can_read_note = note.readable_by?(current_user)
 
         if can_read_note
           present note, with: Entities::Note

lib/api/lsif_data.rb

@@ -21,9 +21,9 @@ module API
           authorize! :download_code, user_project
 
           artifact =
-            @project.job_artifacts
+            Ci::JobArtifact
               .with_file_types(['lsif'])
-              .for_sha(params[:commit_id])
+              .for_sha(params[:commit_id], @project.id)
               .last
 
           not_found! unless artifact

lib/api/notes.rb

@@ -45,7 +45,7 @@ module API
           # array returned, but this is really a edge-case.
           notes = paginate(raw_notes)
           notes = prepare_notes_for_rendering(notes)
-          notes = notes.select { |note| note.visible_for?(current_user) }
+          notes = notes.select { |note| note.readable_by?(current_user) }
           present notes, with: Entities::Note
         end
         # rubocop: enable CodeReuse/ActiveRecord

lib/banzai/filter/issuable_state_filter.rb

@@ -18,7 +18,7 @@ module Banzai
         issuables = extractor.extract([doc])
 
         issuables.each do |node, issuable|
-          next if !can_read_cross_project? && cross_reference?(issuable)
+          next if !can_read_cross_project? && cross_referenced?(issuable)
 
           if VISIBLE_STATES.include?(issuable.state) && issuable_reference?(node.inner_html, issuable)
             state = moved_issue?(issuable) ? s_("IssuableStatus|moved") : issuable.state
@@ -39,7 +39,7 @@ module Banzai
         CGI.unescapeHTML(text) == issuable.reference_link_text(project || group)
       end
 
-      def cross_reference?(issuable)
+      def cross_referenced?(issuable)
         return true if issuable.project != project
         return true if issuable.respond_to?(:group) && issuable.group != group
 

spec/controllers/snippets/notes_controller_spec.rb

@@ -108,7 +108,7 @@ describe Snippets::NotesController do
 
         sign_in(user)
 
-        expect_any_instance_of(Note).to receive(:cross_reference_not_visible_for?).and_return(true)
+        expect_any_instance_of(Note).to receive(:readable_by?).and_return(false)
       end
 
       it "does not return any note" do

spec/models/ci/job_artifact_spec.rb

@@ -113,13 +113,14 @@ describe Ci::JobArtifact do
 
   describe '.for_sha' do
     it 'returns job artifacts for a given pipeline sha' do
-      first_pipeline = create(:ci_pipeline)
-      second_pipeline = create(:ci_pipeline, sha: Digest::SHA1.hexdigest(SecureRandom.hex))
+      project = create(:project)
+      first_pipeline = create(:ci_pipeline, project: project)
+      second_pipeline = create(:ci_pipeline, project: project, sha: Digest::SHA1.hexdigest(SecureRandom.hex))
       first_artifact = create(:ci_job_artifact, job: create(:ci_build, pipeline: first_pipeline))
       second_artifact = create(:ci_job_artifact, job: create(:ci_build, pipeline: second_pipeline))
 
-      expect(described_class.for_sha(first_pipeline.sha)).to eq([first_artifact])
-      expect(described_class.for_sha(second_pipeline.sha)).to eq([second_artifact])
+      expect(described_class.for_sha(first_pipeline.sha, project.id)).to eq([first_artifact])
+      expect(described_class.for_sha(second_pipeline.sha, project.id)).to eq([second_artifact])
     end
   end
 

spec/models/note_spec.rb

@@ -285,28 +285,37 @@ describe Note do
     end
   end
 
-  describe "#visible_for?" do
-    using RSpec::Parameterized::TableSyntax
+  describe "#system_note_with_references_visible_for?" do
+    let(:project) { create(:project, :public) }
+    let(:user) { create(:user) }
+    let(:guest) { create(:project_member, :guest, project: project, user: create(:user)).user }
+    let(:reporter) { create(:project_member, :reporter, project: project, user: create(:user)).user }
+    let(:maintainer) { create(:project_member, :maintainer, project: project, user: create(:user)).user }
+    let(:non_member) { create(:user) }
 
-    let_it_be(:note) { create(:note) }
-    let_it_be(:user) { create(:user) }
+    let(:note) { create(:note, project: project) }
 
-    where(:cross_reference_visible, :system_note_viewable, :result) do
-      true  | true  | false
-      false | true  | true
-      false | false | false
+    context 'when project is public' do
+      it_behaves_like 'users with note access' do
+        let(:users) { [reporter, maintainer, guest, non_member, nil] }
+      end
     end
 
-    with_them do
-      it "returns expected result" do
-        expect(note).to receive(:cross_reference_not_visible_for?).and_return(cross_reference_visible)
+    context 'when group is private' do
+      let(:project) { create(:project, :private) }
+
+      it_behaves_like 'users with note access' do
+        let(:users) { [reporter, maintainer, guest] }
+      end
 
-        unless cross_reference_visible
-          expect(note).to receive(:system_note_viewable_by?)
-            .with(user).and_return(system_note_viewable)
+      it 'returns visible but not readable for non-member user' do
+        expect(note.system_note_with_references_visible_for?(non_member)).to be_truthy
+        expect(note.readable_by?(non_member)).to be_falsy
       end
 
-        expect(note.visible_for?(user)).to eq result
+      it 'returns visible but not readable for a nil user' do
+        expect(note.system_note_with_references_visible_for?(nil)).to be_truthy
+        expect(note.readable_by?(nil)).to be_falsy
       end
     end
   end
@@ -349,7 +358,7 @@ describe Note do
     end
   end
 
-  describe "cross_reference_not_visible_for?" do
+  describe "system_note_with_references_visible_for?" do
     let_it_be(:private_user)    { create(:user) }
     let_it_be(:private_project) { create(:project, namespace: private_user.namespace) { |p| p.add_maintainer(private_user) } }
     let_it_be(:private_issue)   { create(:issue, project: private_project) }
@@ -359,11 +368,11 @@ describe Note do
 
     shared_examples "checks references" do
       it "returns true" do
-        expect(note.cross_reference_not_visible_for?(ext_issue.author)).to be_truthy
+        expect(note.system_note_with_references_visible_for?(ext_issue.author)).to be_falsy
       end
 
       it "returns false" do
-        expect(note.cross_reference_not_visible_for?(private_user)).to be_falsy
+        expect(note.system_note_with_references_visible_for?(private_user)).to be_truthy
       end
 
       it "returns false if user visible reference count set" do
@@ -371,14 +380,14 @@ describe Note do
         note.total_reference_count = 1
 
         expect(note).not_to receive(:reference_mentionables)
-        expect(note.cross_reference_not_visible_for?(ext_issue.author)).to be_falsy
+        expect(note.system_note_with_references_visible_for?(ext_issue.author)).to be_truthy
       end
 
       it "returns true if ref count is 0" do
         note.user_visible_reference_count = 0
 
         expect(note).not_to receive(:reference_mentionables)
-        expect(note.cross_reference_not_visible_for?(ext_issue.author)).to be_truthy
+        expect(note.system_note_with_references_visible_for?(ext_issue.author)).to be_falsy
       end
     end
 
@@ -423,16 +432,16 @@ describe Note do
         note.total_reference_count = 2
 
         expect(note).not_to receive(:reference_mentionables)
-        expect(note.cross_reference_not_visible_for?(ext_issue.author)).to be_truthy
+        expect(note.system_note_with_references_visible_for?(ext_issue.author)).to be_falsy
       end
     end
   end
 
-  describe '#cross_reference?' do
+  describe '#system_note_with_references?' do
     it 'falsey for user-generated notes' do
       note = create(:note, system: false)
 
-      expect(note.cross_reference?).to be_falsy
+      expect(note.system_note_with_references?).to be_falsy
     end
 
     context 'when the note might contain cross references' do
@@ -443,7 +452,7 @@ describe Note do
         it 'delegates to the cross-reference regex' do
           expect(note).to receive(:matches_cross_reference_regex?).and_return(false)
 
-          note.cross_reference?
+          note.system_note_with_references?
         end
       end
     end
@@ -453,8 +462,8 @@ describe Note do
       let(:label_note) { build(:note, note: 'added ~2323232323', system: true) }
 
       it 'scan for a `mentioned in` prefix' do
-        expect(commit_note.cross_reference?).to be_truthy
-        expect(label_note.cross_reference?).to be_falsy
+        expect(commit_note.system_note_with_references?).to be_truthy
+        expect(label_note.system_note_with_references?).to be_falsy
       end
     end
 
@@ -468,7 +477,7 @@ describe Note do
       it 'delegates to the system note service' do
         expect(SystemNotes::IssuablesService).to receive(:cross_reference?).with(note.note)
 
-        note.cross_reference?
+        note.system_note_with_references?
       end
     end
 
@@ -480,7 +489,7 @@ describe Note do
         it 'delegates to the cross-reference regex' do
           expect(note).to receive(:matches_cross_reference_regex?)
 
-          note.cross_reference?
+          note.system_note_with_references?
         end
       end
 
@@ -489,13 +498,13 @@ describe Note do
 
         it_behaves_like 'system_note_metadata includes note action'
 
-        it { expect(note.cross_reference?).to be_falsy }
+        it { expect(note.system_note_with_references?).to be_falsy }
 
         context 'with cross reference label note' do
           let(:label) { create(:label, project: issue.project)}
           let(:note) { create(:system_note, note: "added #{label.to_reference} label", noteable: issue, project: issue.project) }
 
-          it { expect(note.cross_reference?).to be_truthy }
+          it { expect(note.system_note_with_references?).to be_truthy }
         end
       end
 
@@ -504,13 +513,13 @@ describe Note do
 
         it_behaves_like 'system_note_metadata includes note action'
 
-        it { expect(note.cross_reference?).to be_falsy }
+        it { expect(note.system_note_with_references?).to be_falsy }
 
         context 'with cross reference milestone note' do
           let(:milestone) { create(:milestone, project: issue.project)}
           let(:note) { create(:system_note, note: "added #{milestone.to_reference} milestone", noteable: issue, project: issue.project) }
 
-          it { expect(note.cross_reference?).to be_truthy }
+          it { expect(note.system_note_with_references?).to be_truthy }
         end
       end
     end

spec/support/shared_examples/models/note_access_check_shared_examples.rb

+# frozen_string_literal: true
+
+shared_examples 'users with note access' do
+  it 'returns true' do
+    users.each do |user|
+      expect(note.system_note_with_references_visible_for?(user)).to be_truthy
+      expect(note.readable_by?(user)).to be_truthy
+    end
+  end
+end
+
+shared_examples 'users without note access' do
+  it 'returns false' do
+    users.each do |user|
+      expect(note.system_note_with_references_visible_for?(user)).to be_falsy
+      expect(note.readable_by?(user)).to be_falsy
+    end
+  end
+end