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

9 files changed, 133 insertions, 35 deletions

diff --git a/spec/controllers/dashboard/todos_controller_spec.rb b/spec/controllers/dashboard/todos_controller_spec.rb
index cf528b414c0..abada97fb10 100644
--- a/spec/controllers/dashboard/todos_controller_spec.rb
+++ b/spec/controllers/dashboard/todos_controller_spec.rb
@@ -65,7 +65,7 @@ RSpec.describe Dashboard::TodosController do
         project_2 = create(:project, namespace: user.namespace)
         project_2.add_developer(user)
         merge_request_2 = create(:merge_request, source_project: project_2)
-        create(:todo, project: project, author: author, user: user, target: merge_request_2)
+        create(:todo, project: project_2, author: author, user: user, target: merge_request_2)
 
         expect { get :index }.not_to exceed_query_limit(control)
         expect(response).to have_gitlab_http_status(:ok)
diff --git a/spec/lib/gitlab/import_export/group/relation_tree_restorer_spec.rb b/spec/lib/gitlab/import_export/group/relation_tree_restorer_spec.rb
index 473dbf5ecc5..ce6607f6a26 100644
--- a/spec/lib/gitlab/import_export/group/relation_tree_restorer_spec.rb
+++ b/spec/lib/gitlab/import_export/group/relation_tree_restorer_spec.rb
@@ -10,8 +10,8 @@
 require 'spec_helper'
 
 RSpec.describe Gitlab::ImportExport::Group::RelationTreeRestorer do
-  let_it_be(:group) { create(:group) }
-  let_it_be(:importable) { create(:group, parent: group) }
+  let(:group) { create(:group).tap { |g| g.add_owner(user) } }
+  let(:importable) { create(:group, parent: group) }
 
   include_context 'relation tree restorer shared context' do
     let(:importable_name) { nil }
diff --git a/spec/lib/gitlab/import_export/members_mapper_spec.rb b/spec/lib/gitlab/import_export/members_mapper_spec.rb
index 847d6b5d1ed..8b9ca90a280 100644
--- a/spec/lib/gitlab/import_export/members_mapper_spec.rb
+++ b/spec/lib/gitlab/import_export/members_mapper_spec.rb
@@ -267,6 +267,66 @@ RSpec.describe Gitlab::ImportExport::MembersMapper do
       end
     end
 
+    context 'when importer is not an admin' do
+      let(:user) { create(:user) }
+      let(:group) { create(:group) }
+      let(:members_mapper) do
+        described_class.new(
+          exported_members: [], user: user, importable: importable)
+      end
+
+      shared_examples_for 'it fetches the access level from parent group' do
+        before do
+          group.add_users([user], group_access_level)
+        end
+
+        it "and resolves it correctly" do
+          members_mapper.map
+          expect(member_class.find_by_user_id(user.id).access_level).to eq(resolved_access_level)
+        end
+      end
+
+      context 'and the imported project is part of a group' do
+        let(:importable) { create(:project, namespace: group) }
+        let(:member_class) { ProjectMember }
+
+        it_behaves_like 'it fetches the access level from parent group' do
+          let(:group_access_level) { GroupMember::DEVELOPER }
+          let(:resolved_access_level) { ProjectMember::DEVELOPER }
+        end
+
+        it_behaves_like 'it fetches the access level from parent group' do
+          let(:group_access_level) { GroupMember::MAINTAINER }
+          let(:resolved_access_level) { ProjectMember::MAINTAINER }
+        end
+
+        it_behaves_like 'it fetches the access level from parent group' do
+          let(:group_access_level) { GroupMember::OWNER }
+          let(:resolved_access_level) { ProjectMember::MAINTAINER }
+        end
+      end
+
+      context 'and the imported group is part of another group' do
+        let(:importable) { create(:group, parent: group) }
+        let(:member_class) { GroupMember }
+
+        it_behaves_like 'it fetches the access level from parent group' do
+          let(:group_access_level) { GroupMember::DEVELOPER }
+          let(:resolved_access_level) { GroupMember::DEVELOPER }
+        end
+
+        it_behaves_like 'it fetches the access level from parent group' do
+          let(:group_access_level) { GroupMember::MAINTAINER }
+          let(:resolved_access_level) { GroupMember::MAINTAINER }
+        end
+
+        it_behaves_like 'it fetches the access level from parent group' do
+          let(:group_access_level) { GroupMember::OWNER }
+          let(:resolved_access_level) { GroupMember::OWNER }
+        end
+      end
+    end
+
     context 'when importable is Group' do
       include_examples 'imports exported members' do
         let(:source_type) { 'Namespace' }
diff --git a/spec/lib/gitlab/import_export/project/relation_factory_spec.rb b/spec/lib/gitlab/import_export/project/relation_factory_spec.rb
index 49df2313924..80ba50976af 100644
--- a/spec/lib/gitlab/import_export/project/relation_factory_spec.rb
+++ b/spec/lib/gitlab/import_export/project/relation_factory_spec.rb
@@ -3,7 +3,7 @@
 require 'spec_helper'
 
 RSpec.describe Gitlab::ImportExport::Project::RelationFactory, :use_clean_rails_memory_store_caching do
-  let(:group) { create(:group) }
+  let(:group) { create(:group).tap { |g| g.add_maintainer(importer_user) } }
   let(:project) { create(:project, :repository, group: group) }
   let(:members_mapper) { double('members_mapper').as_null_object }
   let(:admin) { create(:admin) }
diff --git a/spec/lib/gitlab/import_export/project/relation_tree_restorer_spec.rb b/spec/lib/gitlab/import_export/project/relation_tree_restorer_spec.rb
index 5ebace263ba..577f1e46db6 100644
--- a/spec/lib/gitlab/import_export/project/relation_tree_restorer_spec.rb
+++ b/spec/lib/gitlab/import_export/project/relation_tree_restorer_spec.rb
@@ -101,7 +101,7 @@ RSpec.describe Gitlab::ImportExport::Project::RelationTreeRestorer do
     it_behaves_like 'import project successfully'
 
     context 'with logging of relations creation' do
-      let_it_be(:group) { create(:group) }
+      let_it_be(:group) { create(:group).tap { |g| g.add_maintainer(user) } }
       let_it_be(:importable) do
         create(:project, :builds_enabled, :issues_disabled, name: 'project', path: 'project', group: group)
       end
@@ -118,7 +118,7 @@ RSpec.describe Gitlab::ImportExport::Project::RelationTreeRestorer do
 
     context 'when inside a group' do
       let_it_be(:group) do
-        create(:group, :disabled_and_unoverridable)
+        create(:group, :disabled_and_unoverridable).tap { |g| g.add_maintainer(user) }
       end
 
       before do
diff --git a/spec/lib/gitlab/import_export/project/tree_restorer_spec.rb b/spec/lib/gitlab/import_export/project/tree_restorer_spec.rb
index cd3d29f1a51..6bb6be07749 100644
--- a/spec/lib/gitlab/import_export/project/tree_restorer_spec.rb
+++ b/spec/lib/gitlab/import_export/project/tree_restorer_spec.rb
@@ -674,6 +674,7 @@ RSpec.describe Gitlab::ImportExport::Project::TreeRestorer do
           # Project needs to be in a group for visibility level comparison
           # to happen
           group = create(:group)
+          group.add_maintainer(user)
           project.group = group
 
           project.create_import_data(data: { override_params: { visibility_level: Gitlab::VisibilityLevel::INTERNAL.to_s } })
@@ -715,13 +716,19 @@ RSpec.describe Gitlab::ImportExport::Project::TreeRestorer do
       end
 
       context 'with a project that has a group' do
+        let(:group) do
+          create(:group, visibility_level: Gitlab::VisibilityLevel::PRIVATE).tap do |g|
+            g.add_maintainer(user)
+          end
+        end
+
         let!(:project) do
           create(:project,
                  :builds_disabled,
                  :issues_disabled,
                  name: 'project',
                  path: 'project',
-                 group: create(:group, visibility_level: Gitlab::VisibilityLevel::PRIVATE))
+                 group: group)
         end
 
         before do
@@ -750,13 +757,14 @@ RSpec.describe Gitlab::ImportExport::Project::TreeRestorer do
       end
 
       context 'with existing group models' do
+        let(:group) { create(:group).tap { |g| g.add_maintainer(user) } }
         let!(:project) do
           create(:project,
                  :builds_disabled,
                  :issues_disabled,
                  name: 'project',
                  path: 'project',
-                 group: create(:group))
+                 group: group)
         end
 
         before do
@@ -785,13 +793,14 @@ RSpec.describe Gitlab::ImportExport::Project::TreeRestorer do
       end
 
       context 'with clashing milestones on IID' do
+        let(:group) { create(:group).tap { |g| g.add_maintainer(user) } }
         let!(:project) do
           create(:project,
                  :builds_disabled,
                  :issues_disabled,
                  name: 'project',
                  path: 'project',
-                 group: create(:group))
+                 group: group)
         end
 
         before do
@@ -870,7 +879,7 @@ RSpec.describe Gitlab::ImportExport::Project::TreeRestorer do
       context 'with group visibility' do
         before do
           group = create(:group, visibility_level: group_visibility)
-
+          group.add_users([user], GroupMember::MAINTAINER)
           project.update(group: group)
         end
 
diff --git a/spec/policies/merge_request_policy_spec.rb b/spec/policies/merge_request_policy_spec.rb
index b94df4d4374..e05de25f182 100644
--- a/spec/policies/merge_request_policy_spec.rb
+++ b/spec/policies/merge_request_policy_spec.rb
@@ -5,10 +5,11 @@ require 'spec_helper'
 RSpec.describe MergeRequestPolicy do
   include ExternalAuthorizationServiceHelpers
 
-  let(:guest) { create(:user) }
-  let(:author) { create(:user) }
-  let(:developer) { create(:user) }
-  let(:non_team_member) { create(:user) }
+  let_it_be(:guest) { create(:user) }
+  let_it_be(:author) { create(:user) }
+  let_it_be(:developer) { create(:user) }
+  let_it_be(:non_team_member) { create(:user) }
+
   let(:project) { create(:project, :public) }
 
   def permissions(user, merge_request)
@@ -50,15 +51,31 @@ RSpec.describe MergeRequestPolicy do
   end
 
   context 'when merge request is public' do
-    context 'and user is anonymous' do
-      let(:merge_request) { create(:merge_request, source_project: project, target_project: project, author: author) }
+    let(:merge_request) { create(:merge_request, source_project: project, target_project: project, author: author) }
 
+    context 'and user is anonymous' do
       subject { permissions(nil, merge_request) }
 
       it do
         is_expected.to be_disallowed(:create_todo, :update_subscription)
       end
     end
+
+    describe 'the author, who became a guest' do
+      subject { permissions(author, merge_request) }
+
+      it do
+        is_expected.to be_allowed(:update_merge_request)
+      end
+
+      it do
+        is_expected.to be_allowed(:reopen_merge_request)
+      end
+
+      it do
+        is_expected.to be_allowed(:approve_merge_request)
+      end
+    end
   end
 
   context 'when merge requests have been disabled' do
@@ -107,6 +124,12 @@ RSpec.describe MergeRequestPolicy do
       it_behaves_like 'a denied user'
     end
 
+    describe 'the author' do
+      subject { author }
+
+      it_behaves_like 'a denied user'
+    end
+
     describe 'a developer' do
       subject { developer }
 
diff --git a/spec/requests/api/todos_spec.rb b/spec/requests/api/todos_spec.rb
index c9deb84ff98..c6b4f50afae 100644
--- a/spec/requests/api/todos_spec.rb
+++ b/spec/requests/api/todos_spec.rb
@@ -378,30 +378,36 @@ RSpec.describe API::Todos do
         expect(response).to have_gitlab_http_status(:not_found)
       end
     end
-
-    it 'returns an error if the issuable author does not have access' do
-      project_1.add_guest(issuable.author)
-
-      post api("/projects/#{project_1.id}/#{issuable_type}/#{issuable.iid}/todo", issuable.author)
-
-      expect(response).to have_gitlab_http_status(:not_found)
-    end
   end
 
   describe 'POST :id/issuable_type/:issueable_id/todo' do
     context 'for an issue' do
-      it_behaves_like 'an issuable', 'issues' do
-        let_it_be(:issuable) do
-          create(:issue, :confidential, author: author_1, project: project_1)
-        end
+      let_it_be(:issuable) do
+        create(:issue, :confidential, project: project_1)
+      end
+
+      it_behaves_like 'an issuable', 'issues'
+
+      it 'returns an error if the issue author does not have access' do
+        post api("/projects/#{project_1.id}/issues/#{issuable.iid}/todo", issuable.author)
+
+        expect(response).to have_gitlab_http_status(:not_found)
       end
     end
 
     context 'for a merge request' do
-      it_behaves_like 'an issuable', 'merge_requests' do
-        let_it_be(:issuable) do
-          create(:merge_request, :simple, source_project: project_1)
-        end
+      let_it_be(:issuable) do
+        create(:merge_request, :simple, source_project: project_1)
+      end
+
+      it_behaves_like 'an issuable', 'merge_requests'
+
+      it 'returns an error if the merge request author does not have access' do
+        project_1.add_guest(issuable.author)
+
+        post api("/projects/#{project_1.id}/merge_requests/#{issuable.iid}/todo", issuable.author)
+
+        expect(response).to have_gitlab_http_status(:forbidden)
       end
     end
   end
diff --git a/spec/support/shared_examples/requests/api/merge_requests_shared_examples.rb b/spec/support/shared_examples/requests/api/merge_requests_shared_examples.rb
index e6f9e5a434c..28813a23fed 100644
--- a/spec/support/shared_examples/requests/api/merge_requests_shared_examples.rb
+++ b/spec/support/shared_examples/requests/api/merge_requests_shared_examples.rb
@@ -14,10 +14,10 @@ RSpec.shared_examples 'rejects user from accessing merge request info' do
     project.add_guest(user)
   end
 
-  it 'returns a 404 error' do
+  it 'returns a 403 error' do
     get api(url, user)
 
-    expect(response).to have_gitlab_http_status(:not_found)
-    expect(json_response['message']).to eq('404 Merge Request Not Found')
+    expect(response).to have_gitlab_http_status(:forbidden)
+    expect(json_response['message']).to eq('403 Forbidden')
   end
 end