Merge remote-tracking branch 'dev/14-10-stable-ee' into 14-10-stable-ee

CHANGELOG.md

@@ -2,6 +2,18 @@
 documentation](doc/development/changelog.md) for instructions on adding your own
 entry.
 
+## 14.10.4 (2022-06-01)
+
+### Security (7 changes)
+
+- [Fix IP restrictions not applying to deploy tokens](gitlab-org/security/gitlab@8866d00e50f1d2857d54130239851f21404d7432) ([merge request](gitlab-org/security/gitlab!2471))
+- [Trigger token should respect group IP restrictions](gitlab-org/security/gitlab@8534ca1be10f115dad2e0c1a4e167673049e401a) ([merge request](gitlab-org/security/gitlab!2478))
+- [Fix content injection in Jira issue title](gitlab-org/security/gitlab@b8f82ec8d7ddf30c656642bff12de8fc8b5930a2) ([merge request](gitlab-org/security/gitlab!2464))
+- [Subgroup member can list members of parent group](gitlab-org/security/gitlab@b59c49fa7b681a93bbe4bc69b20e72930a8b9d8d) ([merge request](gitlab-org/security/gitlab!2480))
+- [Do not allow project member import when membership is locked](gitlab-org/security/gitlab@baed30570206b5ed9973ad8bfac5462721745a5d) ([merge request](gitlab-org/security/gitlab!2447))
+- [Disable changing user attributes when updating SCIM provisioned user](gitlab-org/security/gitlab@ae4eb58668513f38c0daf1dc3b977c6b22a9a476) ([merge request](gitlab-org/security/gitlab!2454))
+- [Allow only job owner to run interactive terminal](gitlab-org/security/gitlab@b0819e77b5a65d4412b42f27a513c02cc056a2b8) ([merge request](gitlab-org/security/gitlab!2433))
+
 ## 14.10.3 (2022-05-20)
 
 ### Added (1 change)

GITALY_SERVER_VERSION

-14.10.3
\ No newline at end of file
+14.10.4
\ No newline at end of file

VERSION

-14.10.3-ee
\ No newline at end of file
+14.10.4-ee
\ No newline at end of file

app/controllers/groups/application_controller.rb

@@ -67,6 +67,12 @@ class Groups::ApplicationController < ApplicationController
     end
   end
 
+  def authorize_read_group_member!
+    unless can?(current_user, :read_group_member, group)
+      render_403
+    end
+  end
+
   def build_canonical_path(group)
     params[:group_id] = group.to_param
 

app/controllers/groups/group_members_controller.rb

@@ -14,6 +14,7 @@ class Groups::GroupMembersController < Groups::ApplicationController
 
   # Authorize
   before_action :authorize_admin_group_member!, except: admin_not_required_endpoints
+  before_action :authorize_read_group_member!, only: :index
 
   skip_before_action :check_two_factor_requirement, only: :leave
   skip_cross_project_access_check :index, :update, :destroy, :request_access,

app/policies/ci/build_policy.rb

@@ -84,7 +84,7 @@ module Ci
       enable :update_commit_status
     end
 
-    rule { can?(:update_build) & terminal }.enable :create_build_terminal
+    rule { can?(:update_build) & terminal & owner_of_job }.enable :create_build_terminal
 
     rule { can?(:update_build) }.enable :play_job
 

app/policies/group_policy.rb

@@ -22,6 +22,7 @@ class GroupPolicy < Namespaces::GroupProjectNamespaceSharedPolicy
   condition(:share_with_group_locked, scope: :subject) { @subject.share_with_group_lock? }
   condition(:parent_share_with_group_locked, scope: :subject) { @subject.parent&.share_with_group_lock? }
   condition(:can_change_parent_share_with_group_lock) { can?(:change_share_with_group_lock, @subject.parent) }
+  condition(:can_read_group_member) { can_read_group_member? }
 
   desc "User is a project bot"
   condition(:project_bot) { user.project_bot? && access_level >= GroupMember::GUEST }
@@ -127,6 +128,10 @@ class GroupPolicy < Namespaces::GroupProjectNamespaceSharedPolicy
 
   rule { ~public_group & ~has_access }.prevent :read_counts
 
+  rule { ~can_read_group_member }.policy do
+    prevent :read_group_member
+  end
+
   rule { ~can?(:read_group) }.policy do
     prevent :read_design_activity
   end
@@ -308,6 +313,10 @@ class GroupPolicy < Namespaces::GroupProjectNamespaceSharedPolicy
     true
   end
 
+  def can_read_group_member?
+    !(@subject.private? && access_level == GroupMember::NO_ACCESS)
+  end
+
   def resource_access_token_creation_allowed?
     resource_access_token_feature_available? && group.root_ancestor.namespace_settings.resource_access_token_creation_allowed?
   end

app/policies/project_policy.rb

@@ -748,6 +748,10 @@ class ProjectPolicy < BasePolicy
     prevent :register_project_runners
   end
 
+  rule { can?(:admin_project_member) }.policy do
+    enable :import_project_members_from_another_project
+  end
+
   private
 
   def user_is_user?

app/services/ci/pipeline_trigger_service.rb

@@ -26,6 +26,7 @@ module Ci
     def create_pipeline_from_trigger(trigger)
       # this check is to not leak the presence of the project if user cannot read it
       return unless trigger.project == project
+      return unless can?(trigger.owner, :read_project, project)
 
       response = Ci::CreatePipelineService
         .new(project, trigger.owner, ref: params[:ref], variables_attributes: variables)

app/services/members/import_project_team_service.rb

@@ -29,7 +29,7 @@ module Members
     def import_project_team
       return false unless target_project.present? && source_project.present? && current_user.present?
       return false unless can?(current_user, :read_project_member, source_project)
-      return false unless can?(current_user, :admin_project_member, target_project)
+      return false unless can?(current_user, :import_project_members_from_another_project, target_project)
 
       target_project.team.import(source_project, current_user)
     end

doc/api/scim.md

@@ -170,13 +170,13 @@ Returns a `201` status code if successful.
 
 Fields that can be updated are:
 
-| SCIM/IdP field                   | GitLab field                           |
-|:---------------------------------|:---------------------------------------|
-| `id/externalId`                  | `extern_uid`                           |
-| `name.formatted`                 | `name`                                 |
-| `emails\[type eq "work"\].value` | `email`                                |
-| `active`                         | Identity removal if `active` = `false` |
-| `userName`                       | `username`                             |
+| SCIM/IdP field                   | GitLab field                                                                 |
+|:---------------------------------|:-----------------------------------------------------------------------------|
+| `id/externalId`                  | `extern_uid`                                                                 |
+| `name.formatted`                 | `name` ([Removed](https://gitlab.com/gitlab-org/gitlab/-/issues/363058))     |
+| `emails\[type eq "work"\].value` | `email` ([Removed](https://gitlab.com/gitlab-org/gitlab/-/issues/363058))    |
+| `active`                         | Identity removal if `active` = `false`                                       |
+| `userName`                       | `username` ([Removed](https://gitlab.com/gitlab-org/gitlab/-/issues/363058)) |
 
 ```plaintext
 PATCH /api/scim/v2/groups/:group_path/Users/:id

doc/user/group/subgroups/index.md

@@ -93,6 +93,9 @@ For more information, view the [permissions table](../../permissions.md#group-me
 
 ## Subgroup membership
 
+NOTE:
+There is a bug that causes some pages in the parent group to be accessible by subgroup members. For more details, see [this issue](https://gitlab.com/gitlab-org/gitlab/-/issues/340421).
+
 When you add a member to a group, that member is also added to all subgroups. The user's permissions are inherited from
 the group's parent.
 

ee/app/policies/ee/group_policy.rb

@@ -433,6 +433,13 @@ module EE
       super
     end
 
+    override :can_read_group_member?
+    def can_read_group_member?
+      return true if user&.can_read_all_resources?
+
+      super
+    end
+
     def ldap_lock_bypassable?
       return false unless ::Feature.enabled?(:ldap_settings_unlock_groups_by_owners)
       return false unless ::Gitlab::CurrentSettings.allow_group_owners_to_manage_ldap?

ee/app/policies/ee/project_policy.rb

@@ -145,6 +145,15 @@ module EE
         ::Gitlab::IncidentManagement.escalation_policies_available?(@subject)
       end
 
+      with_scope :subject
+      condition(:membership_locked_via_parent_group) do
+        @subject.group && (@subject.group.membership_lock? || ::Gitlab::CurrentSettings.lock_memberships_to_ldap?)
+      end
+
+      rule { membership_locked_via_parent_group }.policy do
+        prevent :import_project_members_from_another_project
+      end
+
       rule { visual_review_bot }.policy do
         prevent :read_note
         enable :create_note

ee/app/serializers/integrations/jira_serializers/issue_entity.rb

@@ -13,6 +13,10 @@ module Integrations
         jira_issue.summary
       end
 
+      expose :title_html do |jira_issue|
+        html_escape jira_issue.summary
+      end
+
       expose :created_at do |jira_issue|
         jira_issue.created.to_datetime.utc
       end

ee/lib/api/scim.rb

@@ -75,13 +75,10 @@ module API
           elsif parsed_hash[:extern_uid]
             identity.update(parsed_hash.slice(:extern_uid))
           else
-            scim_conflict!(message: 'Email has already been taken') if email_taken?(parsed_hash[:email], identity)
-
-            result = ::Users::UpdateService.new(identity.user,
-                                                parsed_hash.except(:extern_uid, :active)
-                                                  .merge(user: identity.user)).execute
-
-            result[:status] == :success
+            # With 15.0, we no longer allow modifying user attributes.
+            # However, we mark the operation as successful to avoid breaking
+            # existing automations
+            true
           end
         end
 
@@ -91,12 +88,6 @@ module API
           false
         end
 
-        def email_taken?(email, identity)
-          return unless email
-
-          User.by_any_email(email.downcase).where.not(id: identity.user.id).exists?
-        end
-
         def find_user_identity(group, extern_uid)
           return unless group.saml_provider
 

ee/lib/ee/gitlab/git_access.rb

@@ -25,6 +25,23 @@ module EE
         super
       end
 
+      override :check_project_accessibility!
+      def check_project_accessibility!
+        super
+
+        # Deploy keys and tokens are unique in that we don't check
+        # against the project policy, where IP restrictions normally are
+        # checked. The existence of a project's associated key or
+        # token is enough to authenticate read access. To ensure deploy keys
+        # and tokens honor the IP allow list, we need to force a check here. We
+        # don't want to do this for all Git access because GitLab admin users
+        # aren't subject to this IP restriction, but deploy keys and tokens don't
+        # necessarily have an associated user.
+        return unless deploy_key? || deploy_token?
+
+        raise ::Gitlab::GitAccess::NotFoundError, not_found_message if ip_restricted?
+      end
+
       def group?
         # Strict nil check, to avoid any surprises with Object#present?
         # which can delegate to #empty?
@@ -46,6 +63,10 @@ module EE
 
       private
 
+      def ip_restricted?
+        !::Gitlab::IpRestriction::Enforcer.new(project.group).allows_current_ip? if project.group
+      end
+
       override :check_custom_action
       def check_custom_action
         geo_custom_action || super

ee/spec/features/projects/integrations/jira_issues_list_spec.rb

@@ -79,6 +79,43 @@ RSpec.describe 'Jira issues list', :js do
     end
   end
 
+  context 'when title or description contains HTML characters' do
+    let(:html) { '<script>foobar</script>' }
+    let(:escaped_html) { ERB::Util.html_escape(html) }
+    let(:issue) { build_issue(1).deep_merge(fields: { summary: html }) }
+
+    before do
+      stub_licensed_features(jira_issues_integration: true)
+    end
+
+    it 'escapes the HTML on issues#index' do
+      stub_issues([issue])
+
+      visit project_integrations_jira_issues_path(project)
+
+      expect(page).to have_text(html)
+      expect(page).not_to have_css('script', text: 'foobar')
+      expect(page.source).to include(escaped_html)
+    end
+
+    it 'escapes the HTML on issues#show' do
+      issue.deep_merge!(
+        fields: { comment: { comments: [] } },
+        renderedFields: { description: html },
+        duedate: Time.zone.now.to_s
+      )
+
+      stub_request(:get, /\A#{public_url}/)
+        .to_return(headers: { 'Content-Type' => 'application/json' }, body: issue.to_json)
+
+      visit project_integrations_jira_issue_path(project, 1)
+
+      expect(page).to have_text(html)
+      expect(page).not_to have_css('script', text: 'foobar')
+      expect(page.source).to include(escaped_html)
+    end
+  end
+
   private
 
   def all_pages

ee/spec/lib/gitlab/git_access_spec.rb

@@ -30,6 +30,180 @@ RSpec.describe Gitlab::GitAccess do
     end
   end
 
+  describe '#check_project_accessibility!' do
+    let_it_be_with_reload(:group) { create(:group, :public) }
+    let_it_be(:project) { create(:project, :repository, group: group) }
+    let_it_be(:deploy_key) { create(:deploy_key, user: user) }
+    let_it_be(:admin) { create(:admin) }
+
+    let(:deploy_token) { create(:deploy_token, projects: [project]) }
+
+    let(:start_sha) { '6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9' }
+    let(:end_sha)   { '570e7b2abdd848b95f2f578043fc23bd6f6fd24d' }
+    let(:changes)   { "#{start_sha} #{end_sha} refs/heads/master" }
+    let(:push_error_message) { Gitlab::GitAccess::ERROR_MESSAGES[:upload] }
+
+    before(:all) do
+      project.add_developer(user)
+      deploy_key.deploy_keys_projects.create!(project: project, can_push: true)
+    end
+
+    context 'with ip restriction' do
+      before do
+        allow(Gitlab::IpAddressState).to receive(:current).and_return('192.168.0.2')
+        stub_licensed_features(group_ip_restriction: true)
+      end
+
+      context 'group with restriction' do
+        before do
+          create(:ip_restriction, group: group, range: range)
+        end
+
+        context 'address is within the range' do
+          let(:range) { '192.168.0.0/24' }
+
+          context 'when actor is a DeployKey with access to project' do
+            let(:actor) { deploy_key }
+
+            it 'allows pull, push access' do
+              aggregate_failures do
+                expect { pull_changes }.not_to raise_error
+                expect { push_changes }.not_to raise_error
+              end
+            end
+          end
+
+          context 'when actor is DeployToken with access to project' do
+            let(:actor) { deploy_token }
+
+            it 'allows pull access' do
+              aggregate_failures do
+                expect { pull_changes }.not_to raise_error
+                expect { push_changes }.to raise_forbidden(push_error_message)
+              end
+            end
+          end
+
+          context 'when actor is user with access to project' do
+            let(:actor) { user }
+
+            it 'allows push, pull access' do
+              aggregate_failures do
+                expect { pull_changes }.not_to raise_error
+                expect { push_changes }.not_to raise_error
+              end
+            end
+          end
+
+          context 'when actor is instance admin', :enable_admin_mode do
+            let(:actor) { admin }
+
+            it 'allows push, pull access' do
+              aggregate_failures do
+                expect { pull_changes }.not_to raise_error
+                expect { push_changes }.not_to raise_error
+              end
+            end
+          end
+        end
+
+        context 'address is outside the range' do
+          let(:range) { '10.0.0.0/8' }
+
+          context 'when actor is a DeployKey with access to project' do
+            let(:actor) { deploy_key }
+
+            it 'blocks pull, push with "not found"' do
+              aggregate_failures do
+                expect { pull_changes }.to raise_not_found
+                expect { push_changes }.to raise_not_found
+              end
+            end
+          end
+
+          context 'when actor is DeployToken with access to project' do
+            let(:actor) { deploy_token }
+
+            it 'blocks pull, push with "not found"' do
+              aggregate_failures do
+                expect { pull_changes }.to raise_not_found
+                expect { push_changes }.to raise_not_found
+              end
+            end
+          end
+
+          context 'when actor is user with access to project' do
+            let(:actor) { user }
+
+            it 'blocks pull, push with "not found"' do
+              aggregate_failures do
+                expect { pull_changes }.to raise_not_found
+                expect { push_changes }.to raise_not_found
+              end
+            end
+          end
+
+          context 'when actor is instance admin', :enable_admin_mode do
+            let(:actor) { admin }
+
+            it 'allows push, pull access' do
+              aggregate_failures do
+                expect { pull_changes }.not_to raise_error
+                expect { push_changes }.not_to raise_error
+              end
+            end
+          end
+        end
+      end
+
+      context 'group without restriction' do
+        context 'when actor is a DeployKey with access to project' do
+          let(:actor) { deploy_key }
+
+          it 'allows pull, push access' do
+            aggregate_failures do
+              expect { pull_changes }.not_to raise_error
+              expect { push_changes }.not_to raise_error
+            end
+          end
+        end
+
+        context 'when actor is DeployToken with access to project' do
+          let(:actor) { deploy_token }
+
+          it 'allows pull access' do
+            aggregate_failures do
+              expect { pull_changes }.not_to raise_error
+              expect { push_changes }.to raise_forbidden(push_error_message)
+            end
+          end
+        end
+
+        context 'when actor is user with access to project' do
+          let(:actor) { user }
+
+          it 'allows push, pull access' do
+            aggregate_failures do
+              expect { pull_changes }.not_to raise_error
+              expect { push_changes }.not_to raise_error
+            end
+          end
+        end
+
+        context 'when actor is instance admin', :enable_admin_mode do
+          let(:actor) { admin }
+
+          it 'allows push, pull access' do
+            aggregate_failures do
+              expect { pull_changes }.not_to raise_error
+              expect { push_changes }.not_to raise_error
+            end
+          end
+        end
+      end
+    end
+  end
+
   context "when in a read-only GitLab instance" do
     before do
       create(:protected_branch, name: 'feature', project: project)
@@ -1094,4 +1268,8 @@ RSpec.describe Gitlab::GitAccess do
   def raise_forbidden(message)
     raise_error(Gitlab::GitAccess::ForbiddenError, message)
   end
+
+  def raise_not_found
+    raise_error(Gitlab::GitAccess::NotFoundError, Gitlab::GitAccess::ERROR_MESSAGES[:project_not_found])
+  end
 end

ee/spec/policies/project_policy_spec.rb

@@ -2052,4 +2052,34 @@ RSpec.describe ProjectPolicy do
       expect_disallowed(*owner_permissions)
     end
   end
+
+  context 'importing members from another project' do
+    let(:current_user) { owner }
+
+    context 'for a personal project' do
+      it { is_expected.to be_allowed(:import_project_members_from_another_project) }
+    end
+
+    context 'for a project in a group' do
+      let(:project) { create(:project, group: create(:group)) }
+
+      context 'when the project has locked their membership' do
+        context 'via the parent group' do
+          before do
+            project.group.update!(membership_lock: true)
+          end
+
+          it { is_expected.to be_disallowed(:import_project_members_from_another_project) }
+        end
+
+        context 'via LDAP' do
+          before do
+            stub_application_setting(lock_memberships_to_ldap: true)
+          end
+
+          it { is_expected.to be_disallowed(:import_project_members_from_another_project) }
+        end
+      end
+    end
+  end
 end

ee/spec/requests/api/projects_spec.rb

@@ -1433,4 +1433,46 @@ RSpec.describe API::Projects do
       end
     end
   end
+
+  describe 'POST /projects/:id/import_project_members/:project_id' do
+    let_it_be(:project) { create(:project) }
+    let_it_be(:target_project) { create(:project, group: create(:group)) }
+
+    before_all do
+      project.add_maintainer(another_user)
+      target_project.add_maintainer(another_user)
+    end
+
+    context 'when the target project has locked their membership' do
+      context 'via the parent group' do
+        before do
+          target_project.group.update!(membership_lock: true)
+        end
+
+        it 'returns 403' do
+          expect do
+            post api("/projects/#{target_project.id}/import_project_members/#{project.id}", another_user)
+          end.not_to change { target_project.members.count }
+
+          expect(response).to have_gitlab_http_status(:unprocessable_entity)
+          expect(json_response['message']).to eq('Import failed')
+        end
+      end
+
+      context 'via LDAP' do
+        before do
+          stub_application_setting(lock_memberships_to_ldap: true)
+        end
+
+        it 'returns 403' do
+          expect do
+            post api("/projects/#{target_project.id}/import_project_members/#{project.id}", another_user)
+          end.not_to change { target_project.members.count }
+
+          expect(response).to have_gitlab_http_status(:unprocessable_entity)
+          expect(json_response['message']).to eq('Import failed')
+        end
+      end
+    end
+  end
 end

ee/spec/requests/api/scim_spec.rb

@@ -371,7 +371,6 @@ RSpec.describe API::Scim do
 
         it 'does not call reprovision service when identity is already active' do
           expect(::EE::Gitlab::Scim::ReprovisionService).not_to receive(:new)
-          expect(::Users::UpdateService).to receive(:new).and_call_original
 
           call_patch_api(params)
         end
@@ -394,36 +393,36 @@ RSpec.describe API::Scim do
           end
         end
 
-        context 'name' do
-          before do
-            params = { Operations: [{ 'op': 'Replace', 'path': 'name.formatted', 'value': 'new_name' }] }.to_query
+        context 'user attributes' do
+          context 'name' do
+            before do
+              params = { Operations: [{ 'op': 'Replace', 'path': 'name.formatted', 'value': 'new_name' }] }.to_query
 
-            call_patch_api(params)
-          end
+              call_patch_api(params)
+            end
 
-          it 'responds with 204' do
-            expect(response).to have_gitlab_http_status(:no_content)
-          end
+            it 'responds with 204' do
+              expect(response).to have_gitlab_http_status(:no_content)
+            end
 
-          it 'updates the name' do
-            expect(user.reload.name).to eq('new_name')
-          end
+            it 'does not update the name' do
+              expect(user.reload.name).not_to eq('new_name')
+            end
 
-          it 'responds with an empty response' do
-            expect(response.body).to eq('')
+            it 'responds with an empty response' do
+              expect(response.body).to eq('')
+            end
           end
-        end
 
-        context 'email' do
-          context 'non existent email' do
+          context 'email' do
             before do
               params = { Operations: [{ 'op': 'Replace', 'path': 'emails[type eq "work"].value', 'value': 'new@mail.com' }] }.to_query
 
               call_patch_api(params)
             end
 
-            it 'updates the email' do
-              expect(user.reload.unconfirmed_email).to eq('new@mail.com')
+            it 'does not update the email' do
+              expect(user.reload.unconfirmed_email).not_to eq('new@mail.com')
             end
 
             it 'responds with 204' do
@@ -431,21 +430,23 @@ RSpec.describe API::Scim do
             end
           end
 
-          context 'existent email' do
+          context 'userName' do
             before do
-              create(:user, email: 'new@mail.com')
-
-              params = { Operations: [{ 'op': 'Replace', 'path': 'emails[type eq "work"].value', 'value': 'new@mail.com' }] }.to_query
+              params = { Operations: [{ 'op': 'Replace', 'path': 'userName', 'value': 'new_username' }] }.to_query
 
               call_patch_api(params)
             end
 
-            it 'does not update a duplicated email' do
-              expect(user.reload.unconfirmed_email).not_to eq('new@mail.com')
+            it 'responds with 204' do
+              expect(response).to have_gitlab_http_status(:no_content)
+            end
+
+            it 'does not update the username' do
+              expect(user.reload.username).not_to eq('new_username')
             end
 
-            it 'responds with 209' do
-              expect(response).to have_gitlab_http_status(:conflict)
+            it 'responds with an empty response' do
+              expect(response.body).to eq('')
             end
           end
         end

ee/spec/serializers/integrations/jira_serializers/issue_entity_spec.rb

@@ -26,7 +26,7 @@ RSpec.describe Integrations::JiraSerializers::IssueEntity do
 
   let(:jira_issue) do
     double(
-      summary: 'Title',
+      summary: 'Title with <h1>HTML</h1>',
       created: '2020-06-25T15:39:30.000+0000',
       updated: '2020-06-26T15:38:32.000+0000',
       resolutiondate: '2020-06-27T13:23:51.000+0000',
@@ -46,7 +46,8 @@ RSpec.describe Integrations::JiraSerializers::IssueEntity do
   it 'returns the Jira issues attributes' do
     expect(subject).to include(
       project_id: project.id,
-      title: 'Title',
+      title: 'Title with <h1>HTML</h1>',
+      title_html: 'Title with &lt;h1&gt;HTML&lt;/h1&gt;',
       created_at: '2020-06-25T15:39:30.000+0000'.to_datetime.utc,
       updated_at: '2020-06-26T15:38:32.000+0000'.to_datetime.utc,
       closed_at: '2020-06-27T13:23:51.000+0000'.to_datetime.utc,

ee/spec/services/ci/pipeline_trigger_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Ci::PipelineTriggerService do
+  let_it_be(:project) { create(:project, :repository) }
+
+  before do
+    stub_ci_pipeline_to_return_yaml_file
+  end
+
+  describe '#execute' do
+    let_it_be(:user) { create(:user) }
+
+    let(:result) { described_class.new(project, user, params).execute }
+
+    before do
+      project.add_developer(user)
+    end
+
+    shared_examples 'with ip restriction' do
+      let_it_be_with_reload(:group) { create(:group, :public) }
+      let_it_be_with_reload(:project) { create(:project, :repository, group: group) }
+
+      before do
+        allow(Gitlab::IpAddressState).to receive(:current).and_return('192.168.0.2')
+        stub_licensed_features(group_ip_restriction: true)
+      end
+
+      context 'group with restriction' do
+        before do
+          create(:ip_restriction, group: group, range: range)
+        end
+
+        context 'address is within the range' do
+          let(:range) { '192.168.0.0/24' }
+
+          it 'triggers a pipeline' do
+            expect { result }.to change { Ci::Pipeline.count }.by(1)
+          end
+        end
+
+        context 'address is outside the range' do
+          let(:range) { '10.0.0.0/8' }
+
+          it 'does nothing' do
+            expect { result }.not_to change { Ci::Pipeline.count }
+          end
+        end
+      end
+
+      context 'group without restriction' do
+        it 'triggers a pipeline' do
+          expect { result }.to change { Ci::Pipeline.count }.by(1)
+        end
+      end
+    end
+
+    context 'with a trigger token' do
+      let(:params) { { token: trigger.token, ref: 'master', variables: nil } }
+
+      let(:trigger) { create(:ci_trigger, project: project, owner: user) }
+
+      include_examples 'with ip restriction'
+    end
+
+    context 'with a job token' do
+      let!(:pipeline) { create(:ci_empty_pipeline, project: project) }
+      let(:job) { create(:ci_build, :running, pipeline: pipeline, user: user) }
+      let(:params) { { token: job.token, ref: 'master', variables: nil } }
+
+      include_examples 'with ip restriction'
+    end
+  end
+end

ee/spec/services/ee/members/import_project_team_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Members::ImportProjectTeamService do
+  describe '#execute' do
+    let_it_be(:source_project) { create(:project) }
+    let_it_be(:target_project) { create(:project, group: create(:group)) }
+    let_it_be(:user) { create(:user) }
+
+    let(:source_project_id) { source_project.id }
+    let(:target_project_id) { target_project.id }
+
+    subject { described_class.new(user, { id: target_project_id, project_id: source_project_id }) }
+
+    before_all do
+      source_project.add_guest(user)
+      target_project.add_maintainer(user)
+    end
+
+    context 'when the project team import fails' do
+      context 'when the target project has locked their membership' do
+        context 'via the parent group' do
+          before do
+            target_project.group.update!(membership_lock: true)
+          end
+
+          it 'returns false' do
+            expect(subject.execute).to be(false)
+          end
+        end
+
+        context 'via LDAP' do
+          before do
+            stub_application_setting(lock_memberships_to_ldap: true)
+          end
+
+          it 'returns false' do
+            expect(subject.execute).to be(false)
+          end
+        end
+      end
+    end
+  end
+end

lib/api/helpers/members_helpers.rb

@@ -15,6 +15,10 @@ module API
         public_send("find_#{source_type}!", id) # rubocop:disable GitlabSecurity/PublicSend
       end
 
+      def authorize_read_source_member!(source_type, source)
+        authorize! :"read_#{source_type}_member", source
+      end
+
       def authorize_admin_source!(source_type, source)
         authorize! :"admin_#{source_type}", source
       end

lib/api/members.rb

@@ -30,6 +30,8 @@ module API
         get ":id/members" do
           source = find_source(source_type, params[:id])
 
+          authorize_read_source_member!(source_type, source)
+
           members = paginate(retrieve_members(source, params: params))
 
           present_members members
@@ -49,6 +51,8 @@ module API
         get ":id/members/all" do
           source = find_source(source_type, params[:id])
 
+          authorize_read_source_member!(source_type, source)
+
           members = paginate(retrieve_members(source, params: params, deep: true))
 
           present_members members
@@ -64,6 +68,8 @@ module API
         get ":id/members/:user_id" do
           source = find_source(source_type, params[:id])
 
+          authorize_read_source_member!(source_type, source)
+
           members = source_members(source)
           member = members.find_by!(user_id: params[:user_id])
 
@@ -81,6 +87,8 @@ module API
         get ":id/members/all/:user_id" do
           source = find_source(source_type, params[:id])
 
+          authorize_read_source_member!(source_type, source)
+
           members = find_all_members(source)
           member = members.find_by!(user_id: params[:user_id])
 

spec/controllers/projects/jobs_controller_spec.rb

@@ -183,7 +183,7 @@ RSpec.describe Projects::JobsController, :clean_gitlab_redis_shared_state do
         end
 
         context 'with web terminal' do
-          let(:job) { create(:ci_build, :running, :with_runner_session, pipeline: pipeline) }
+          let(:job) { create(:ci_build, :running, :with_runner_session, pipeline: pipeline, user: user) }
 
           it 'exposes the terminal path' do
             expect(response).to have_gitlab_http_status(:ok)
@@ -1284,7 +1284,7 @@ RSpec.describe Projects::JobsController, :clean_gitlab_redis_shared_state do
 
     context 'when job exists' do
       context 'and it has a terminal' do
-        let!(:job) { create(:ci_build, :running, :with_runner_session, pipeline: pipeline) }
+        let!(:job) { create(:ci_build, :running, :with_runner_session, pipeline: pipeline, user: user) }
 
         it 'has a job' do
           get_terminal(id: job.id)
@@ -1295,7 +1295,7 @@ RSpec.describe Projects::JobsController, :clean_gitlab_redis_shared_state do
       end
 
       context 'and does not have a terminal' do
-        let!(:job) { create(:ci_build, :running, pipeline: pipeline) }
+        let!(:job) { create(:ci_build, :running, pipeline: pipeline, user: user) }
 
         it 'returns not_found' do
           get_terminal(id: job.id)
@@ -1324,7 +1324,7 @@ RSpec.describe Projects::JobsController, :clean_gitlab_redis_shared_state do
   end
 
   describe 'GET #terminal_websocket_authorize' do
-    let!(:job) { create(:ci_build, :running, :with_runner_session, pipeline: pipeline) }
+    let!(:job) { create(:ci_build, :running, :with_runner_session, pipeline: pipeline, user: user) }
 
     before do
       project.add_developer(user)

spec/features/security/group/private_access_spec.rb

@@ -97,7 +97,7 @@ RSpec.describe 'Private Group access' do
     it { is_expected.to be_allowed_for(:developer).of(group) }
     it { is_expected.to be_allowed_for(:reporter).of(group) }
     it { is_expected.to be_allowed_for(:guest).of(group) }
-    it { is_expected.to be_allowed_for(project_guest) }
+    it { is_expected.to be_denied_for(project_guest) }
     it { is_expected.to be_denied_for(:user) }
     it { is_expected.to be_denied_for(:external) }
     it { is_expected.to be_denied_for(:visitor) }

spec/policies/ci/build_policy_spec.rb

@@ -405,4 +405,52 @@ RSpec.describe Ci::BuildPolicy do
       end
     end
   end
+
+  describe 'ability :create_build_terminal' do
+    let(:project) { create(:project, :private) }
+
+    subject { described_class.new(user, build) }
+
+    context 'when user can update_build' do
+      before do
+        project.add_maintainer(user)
+      end
+
+      context 'when job has terminal' do
+        before do
+          allow(build).to receive(:has_terminal?).and_return(true)
+        end
+
+        context 'when current user is the job owner' do
+          before do
+            build.update!(user: user)
+          end
+
+          it { expect_allowed(:create_build_terminal) }
+        end
+
+        context 'when current user is not the job owner' do
+          it { expect_disallowed(:create_build_terminal) }
+        end
+      end
+
+      context 'when job does not have terminal' do
+        before do
+          allow(build).to receive(:has_terminal?).and_return(false)
+          build.update!(user: user)
+        end
+
+        it { expect_disallowed(:create_build_terminal) }
+      end
+    end
+
+    context 'when user cannot update build' do
+      before do
+        project.add_guest(user)
+        allow(build).to receive(:has_terminal?).and_return(true)
+      end
+
+      it { expect_disallowed(:create_build_terminal) }
+    end
+  end
 end

spec/policies/project_policy_spec.rb

@@ -396,6 +396,36 @@ RSpec.describe ProjectPolicy do
     end
   end
 
+  context 'importing members from another project' do
+    %w(maintainer owner).each do |role|
+      context "with #{role}" do
+        let(:current_user) { send(role) }
+
+        it { is_expected.to be_allowed(:import_project_members_from_another_project) }
+      end
+    end
+
+    %w(guest reporter developer anonymous).each do |role|
+      context "with #{role}" do
+        let(:current_user) { send(role) }
+
+        it { is_expected.to be_disallowed(:import_project_members_from_another_project) }
+      end
+    end
+
+    context 'with an admin' do
+      let(:current_user) { admin }
+
+      context 'when admin mode is enabled', :enable_admin_mode do
+        it { expect_allowed(:import_project_members_from_another_project) }
+      end
+
+      context 'when admin mode is disabled' do
+        it { expect_disallowed(:import_project_members_from_another_project) }
+      end
+    end
+  end
+
   context 'reading usage quotas' do
     %w(maintainer owner).each do |role|
       context "with #{role}" do

spec/requests/api/members_spec.rb

@@ -184,6 +184,21 @@ RSpec.describe API::Members do
       expect(json_response).to be_an Array
       expect(json_response.map { |u| u['id'] }).to match_array [maintainer.id, developer.id, nested_user.id]
     end
+
+    context 'with a subgroup' do
+      let(:group) { create(:group, :private)}
+      let(:subgroup) { create(:group, :private, parent: group)}
+      let(:project) { create(:project, group: subgroup) }
+
+      before do
+        subgroup.add_developer(developer)
+      end
+
+      it 'subgroup member cannot get parent group members list' do
+        get api("/groups/#{group.id}/members/all", developer)
+        expect(response).to have_gitlab_http_status(:forbidden)
+      end
+    end
   end
 
   shared_examples 'GET /:source_type/:id/members/(all/):user_id' do |source_type, all|

spec/services/ci/pipeline_trigger_service_spec.rb

@@ -56,6 +56,15 @@ RSpec.describe Ci::PipelineTriggerService do
         end
       end
 
+      context 'when trigger owner does not have a permission to read a project' do
+        let(:params) { { token: trigger.token, ref: 'master', variables: nil } }
+        let(:trigger) { create(:ci_trigger, project: project, owner: create(:user)) }
+
+        it 'does nothing' do
+          expect { result }.not_to change { Ci::Pipeline.count }
+        end
+      end
+
       context 'when params have an existing trigger token' do
         context 'when params have an existing ref' do
           let(:params) { { token: trigger.token, ref: 'master', variables: nil } }