Add latest changes from gitlab-org/security/gitlab@14-4-stable-ee

11 files changed, 127 insertions, 13 deletions

diff --git a/app/models/concerns/resolvable_discussion.rb b/app/models/concerns/resolvable_discussion.rb
index 3e1e5faee54..60e1dde17b9 100644
--- a/app/models/concerns/resolvable_discussion.rb
+++ b/app/models/concerns/resolvable_discussion.rb
@@ -81,8 +81,7 @@ module ResolvableDiscussion
     return false unless current_user
     return false unless resolvable?
 
-    current_user == self.noteable.try(:author) ||
-      current_user.can?(:resolve_note, self.project)
+    current_user.can?(:resolve_note, self.noteable)
   end
 
   def resolve!(current_user)
diff --git a/app/models/project_group_link.rb b/app/models/project_group_link.rb
index d704f4c2c87..8394ebe1df4 100644
--- a/app/models/project_group_link.rb
+++ b/app/models/project_group_link.rb
@@ -15,6 +15,7 @@ class ProjectGroupLink < ApplicationRecord
   validate :different_group
 
   scope :non_guests, -> { where('group_access > ?', Gitlab::Access::GUEST) }
+  scope :in_group, -> (group_ids) { where(group_id: group_ids) }
 
   alias_method :shared_with_group, :group
 
diff --git a/app/policies/issuable_policy.rb b/app/policies/issuable_policy.rb
index 61263e47d7c..39ce26526e6 100644
--- a/app/policies/issuable_policy.rb
+++ b/app/policies/issuable_policy.rb
@@ -11,6 +11,8 @@ class IssuablePolicy < BasePolicy
     @user && @subject.assignee_or_author?(@user)
   end
 
+  condition(:is_author) { @subject&.author == @user }
+
   rule { can?(:guest_access) & assignee_or_author }.policy do
     enable :read_issue
     enable :update_issue
@@ -20,6 +22,10 @@ class IssuablePolicy < BasePolicy
     enable :reopen_merge_request
   end
 
+  rule { is_author }.policy do
+    enable :resolve_note
+  end
+
   rule { locked & ~is_project_member }.policy do
     prevent :create_note
     prevent :admin_note
diff --git a/app/services/groups/transfer_service.rb b/app/services/groups/transfer_service.rb
index 774f81b0a5e..334083a859f 100644
--- a/app/services/groups/transfer_service.rb
+++ b/app/services/groups/transfer_service.rb
@@ -182,6 +182,14 @@ module Groups
 
       # schedule refreshing projects for all the members of the group
       @group.refresh_members_authorized_projects
+
+      # When a group is transferred, it also affects who gets access to the projects shared to
+      # the subgroups within its hierarchy, so we also schedule jobs that refresh authorizations for all such shared projects.
+      project_group_shares_within_the_hierarchy = ProjectGroupLink.in_group(group.self_and_descendants.select(:id))
+
+      project_group_shares_within_the_hierarchy.find_each do |project_group_link|
+        AuthorizedProjectUpdate::ProjectRecalculateWorker.perform_async(project_group_link.project_id)
+      end
     end
 
     def raise_transfer_error(message)
diff --git a/config/gitlab.yml.example b/config/gitlab.yml.example
index 3d2acce9a69..bb69c215f8d 100644
--- a/config/gitlab.yml.example
+++ b/config/gitlab.yml.example
@@ -176,8 +176,10 @@ production: &base
     ## Application settings cache expiry in seconds (default: 60)
     # application_settings_cache_seconds: 60
 
-    ## Print initial root password to stdout during initialization (default: true)
-    # display_initial_root_password: true
+    ## Print initial root password to stdout during initialization (default: false)
+    # WARNING: setting this to true means that the root password will be printed in
+    # plaintext. This can be a security risk.
+    # display_initial_root_password: false
 
   ## Reply by email
   # Allow users to comment on issues and merge requests by replying to notification emails.
diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb
index 0e4e6f5cc84..d6957491b16 100644
--- a/config/initializers/1_settings.rb
+++ b/config/initializers/1_settings.rb
@@ -218,8 +218,7 @@ Settings.gitlab['no_todos_messages'] ||= YAML.load_file(Rails.root.join('config'
 Settings.gitlab['impersonation_enabled'] ||= true if Settings.gitlab['impersonation_enabled'].nil?
 Settings.gitlab['usage_ping_enabled'] = true if Settings.gitlab['usage_ping_enabled'].nil?
 Settings.gitlab['max_request_duration_seconds'] ||= 57
-
-Settings.gitlab['display_initial_root_password'] = true if Settings.gitlab['display_initial_root_password'].nil?
+Settings.gitlab['display_initial_root_password'] = false if Settings.gitlab['display_initial_root_password'].nil?
 
 Gitlab.ee do
   Settings.gitlab['mirror_max_delay'] ||= 300
diff --git a/db/fixtures/production/002_admin.rb b/db/fixtures/production/002_admin.rb
index b6a6da3a188..b4710bc3e97 100644
--- a/db/fixtures/production/002_admin.rb
+++ b/db/fixtures/production/002_admin.rb
@@ -26,7 +26,7 @@ if user.persisted?
     if ::Settings.gitlab['display_initial_root_password']
       puts "password: #{user_args[:password]}".color(:green)
     else
-      puts "password: *** - You opted not to display initial root password to STDOUT."
+      puts "password: ******".color(:green)
     end
   else
     puts "password: You'll be prompted to create one on your first visit.".color(:green)
diff --git a/spec/graphql/mutations/discussions/toggle_resolve_spec.rb b/spec/graphql/mutations/discussions/toggle_resolve_spec.rb
index b03c6cb094f..8c11279a80a 100644
--- a/spec/graphql/mutations/discussions/toggle_resolve_spec.rb
+++ b/spec/graphql/mutations/discussions/toggle_resolve_spec.rb
@@ -7,6 +7,7 @@ RSpec.describe Mutations::Discussions::ToggleResolve do
     described_class.new(object: nil, context: { current_user: user }, field: nil)
   end
 
+  let_it_be(:author) { create(:user) }
   let_it_be(:project) { create(:project, :repository) }
 
   describe '#resolve' do
@@ -136,20 +137,37 @@ RSpec.describe Mutations::Discussions::ToggleResolve do
           end
         end
       end
+
+      context 'when user is the author and discussion is locked' do
+        let(:user) { author }
+
+        before do
+          issuable.update!(discussion_locked: true)
+        end
+
+        it 'raises an error' do
+          expect { mutation.resolve(id: id_arg, resolve: resolve_arg) }.to raise_error(
+            Gitlab::Graphql::Errors::ResourceNotAvailable,
+            "The resource that you are attempting to access does not exist or you don't have permission to perform this action"
+          )
+        end
+      end
     end
 
     context 'when discussion is on a merge request' do
-      let_it_be(:noteable) { create(:merge_request, source_project: project) }
+      let_it_be(:noteable) { create(:merge_request, source_project: project, author: author) }
 
       let(:discussion) { create(:diff_note_on_merge_request, noteable: noteable, project: project).to_discussion }
+      let(:issuable) { noteable }
 
       it_behaves_like 'a working resolve method'
     end
 
     context 'when discussion is on a design' do
-      let_it_be(:noteable) { create(:design, :with_file, issue: create(:issue, project: project)) }
+      let_it_be(:noteable) { create(:design, :with_file, issue: create(:issue, project: project, author: author)) }
 
       let(:discussion) { create(:diff_note_on_design, noteable: noteable, project: project).to_discussion }
+      let(:issuable) { noteable.issue }
 
       it_behaves_like 'a working resolve method'
     end
diff --git a/spec/models/concerns/resolvable_discussion_spec.rb b/spec/models/concerns/resolvable_discussion_spec.rb
index c0e5ddc23b1..fc154738f11 100644
--- a/spec/models/concerns/resolvable_discussion_spec.rb
+++ b/spec/models/concerns/resolvable_discussion_spec.rb
@@ -188,6 +188,16 @@ RSpec.describe Discussion, ResolvableDiscussion do
           it "returns true" do
             expect(subject.can_resolve?(current_user)).to be true
           end
+
+          context 'when noteable is locked' do
+            before do
+              allow(subject.noteable).to receive(:discussion_locked?).and_return(true)
+            end
+
+            it 'returns false' do
+              expect(subject.can_resolve?(current_user)).to be_falsey
+            end
+          end
         end
 
         context "when the signed in user can push to the project" do
@@ -200,8 +210,11 @@ RSpec.describe Discussion, ResolvableDiscussion do
           end
 
           context "when the noteable has no author" do
+            before do
+              subject.noteable.author = nil
+            end
+
             it "returns true" do
-              expect(noteable).to receive(:author).and_return(nil)
               expect(subject.can_resolve?(current_user)).to be true
             end
           end
@@ -213,8 +226,11 @@ RSpec.describe Discussion, ResolvableDiscussion do
           end
 
           context "when the noteable has no author" do
+            before do
+              subject.noteable.author = nil
+            end
+
             it "returns false" do
-              expect(noteable).to receive(:author).and_return(nil)
               expect(subject.can_resolve?(current_user)).to be false
             end
           end
diff --git a/spec/policies/issuable_policy_spec.rb b/spec/policies/issuable_policy_spec.rb
index 86b04ccda57..eeb298e853e 100644
--- a/spec/policies/issuable_policy_spec.rb
+++ b/spec/policies/issuable_policy_spec.rb
@@ -13,6 +13,10 @@ RSpec.describe IssuablePolicy, models: true do
       let(:merge_request) { create(:merge_request, source_project: project, author: user) }
       let(:policies) { described_class.new(user, merge_request) }
 
+      it 'allows resolving notes' do
+        expect(policies).to be_allowed(:resolve_note)
+      end
+
       context 'when user is able to read project' do
         it 'enables user to read and update issuables' do
           expect(policies).to be_allowed(:read_issue, :update_issue, :reopen_issue, :read_merge_request, :update_merge_request, :reopen_merge_request)
@@ -43,16 +47,24 @@ RSpec.describe IssuablePolicy, models: true do
         it 'can not create a note nor award emojis' do
           expect(policies).to be_disallowed(:create_note, :award_emoji)
         end
+
+        it 'does not allow resolving note' do
+          expect(policies).to be_disallowed(:resolve_note)
+        end
       end
 
       context 'when the user is a project member' do
         before do
-          project.add_guest(user)
+          project.add_developer(user)
         end
 
         it 'can create a note and award emojis' do
           expect(policies).to be_allowed(:create_note, :award_emoji)
         end
+
+        it 'allows resolving notes' do
+          expect(policies).to be_allowed(:resolve_note)
+        end
       end
     end
   end
diff --git a/spec/services/groups/transfer_service_spec.rb b/spec/services/groups/transfer_service_spec.rb
index ee38c0fbb44..8b506d2bc2c 100644
--- a/spec/services/groups/transfer_service_spec.rb
+++ b/spec/services/groups/transfer_service_spec.rb
@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe Groups::TransferService do
+RSpec.describe Groups::TransferService, :sidekiq_inline do
   shared_examples 'project namespace path is in sync with project path' do
     it 'keeps project and project namespace attributes in sync' do
       projects_with_project_namespace.each do |project|
@@ -653,6 +653,59 @@ RSpec.describe Groups::TransferService do
               transfer_service.execute(new_parent_group)
             end
           end
+
+          context 'transferring groups with shared_projects' do
+            let_it_be_with_reload(:shared_project) { create(:project, :public) }
+
+            shared_examples_for 'drops the authorizations of ancestor members from the old hierarchy' do
+              it 'drops the authorizations of ancestor members from the old hierarchy' do
+                expect { transfer_service.execute(new_parent_group) }.to change {
+                  ProjectAuthorization.where(project: shared_project, user: old_group_member).size
+                }.from(1).to(0)
+              end
+            end
+
+            context 'when the group that has existing project share is transferred' do
+              before do
+                create(:project_group_link, :maintainer, project: shared_project, group: group)
+              end
+
+              it_behaves_like 'drops the authorizations of ancestor members from the old hierarchy'
+            end
+
+            context 'when the group whose subgroup has an existing project share is transferred' do
+              let_it_be_with_reload(:subgroup) { create(:group, :private, parent: group) }
+
+              before do
+                create(:project_group_link, :maintainer, project: shared_project, group: subgroup)
+              end
+
+              it_behaves_like 'drops the authorizations of ancestor members from the old hierarchy'
+            end
+          end
+
+          context 'when a group that has existing group share is transferred' do
+            let(:shared_with_group) { group }
+
+            let_it_be(:member_of_shared_with_group) { create(:user) }
+            let_it_be(:shared_group) { create(:group, :private) }
+            let_it_be(:project_in_shared_group) { create(:project, namespace: shared_group) }
+
+            before do
+              shared_with_group.add_developer(member_of_shared_with_group)
+              create(:group_group_link, :maintainer, shared_group: shared_group, shared_with_group: shared_with_group)
+              shared_with_group.refresh_members_authorized_projects
+            end
+
+            it 'retains the authorizations of direct members' do
+              expect { transfer_service.execute(new_parent_group) }.not_to change {
+                ProjectAuthorization.where(
+                  project: project_in_shared_group,
+                  user: member_of_shared_with_group,
+                  access_level: Gitlab::Access::DEVELOPER).size
+              }.from(1)
+            end
+          end
         end
       end