Merge branch 'merge-dev-to-master' into 'master'

Merge EE dev.gitlab.org master into GitLab.com EE master

See merge request gitlab-org/gitlab-ee!9836

CHANGELOG-EE.md

@@ -227,6 +227,17 @@ Please view this file on the master branch, on stable branches it's out of date.
 - Gather deepest epic relationship data.
 
 
+## 11.6.10 (2019-02-28)
+
+### Security (5 changes)
+
+- Remove the possibility to share a project with a group that a user is not a member of.
+- Prevent Group SAML authorizing sign in without prior user approval.
+- Prevent SAML access when disabled by group admin on GitLab.com.
+- Respect group membership lock when importing a member from another group.
+- Ignore out of range epic IDs.
+
+
 ## 11.6.9 (2019-02-04)
 
 - No changes.

CHANGELOG.md

@@ -486,6 +486,33 @@ entry.
 - Update url placeholder for the sentry configuration page. !24338
 
 
+## 11.6.10 (2019-02-28)
+
+### Security (21 changes)
+
+- Stop linking to unrecognized package sources. !55518
+- Check snippet attached file to be moved is within designated directory.
+- Fix potential Addressable::URI::InvalidURIError.
+- Do not display impersonated sessions under active sessions and remove ability to revoke session.
+- Display only information visible to current user on the Milestone page.
+- Show only merge requests visible to user on milestone detail page.
+- Disable issue boards API when issues are disabled.
+- Don't show new issue link after move when a user does not have permissions.
+- Fix git clone revealing private repo's presence.
+- Fix blind SSRF in Prometheus integration by checking URL before querying.
+- Check if desired milestone for an issue is available.
+- Don't allow non-members to see private related MRs.
+- Fix arbitrary file read via diffs during import.
+- Display the correct number of MRs a user has access to.
+- Forbid creating discussions for users with restricted access.
+- Do not disclose milestone titles for unauthorized users.
+- Validate session key when authorizing with GCP to create a cluster.
+- Block local URLs for Kubernetes integration.
+- Limit mermaid rendering to 5K characters.
+- Remove the possibility to share a project with a group that a user is not a member of.
+- Fix leaking private repository information in API.
+
+
 ## 11.6.9 (2019-02-04)
 
 ### Security (1 change)

app/controllers/projects/project_members_controller.rb

@@ -7,6 +7,7 @@ class Projects::ProjectMembersController < Projects::ApplicationController
 
   # Authorize
   before_action :authorize_admin_project_member!, except: [:index, :leave, :request_access]
+  before_action :check_membership_lock!, only: [:create, :import, :apply_import]
 
   # rubocop: disable CodeReuse/ActiveRecord
   def index
@@ -50,4 +51,8 @@ class Projects::ProjectMembersController < Projects::ApplicationController
 
   # MembershipActions concern
   alias_method :membershipable, :project
+
+  def check_membership_lock!
+    access_denied!('Membership is locked by group settings') if membership_locked?
+  end
 end

app/models/concerns/milestoneish.rb

@@ -53,6 +53,18 @@ module Milestoneish
     end
   end
 
+  def issue_participants_visible_by_user(user)
+    User.joins(:issue_assignees)
+      .where('issue_assignees.issue_id' => issues_visible_to_user(user).select(:id))
+      .distinct
+  end
+
+  def issue_labels_visible_by_user(user)
+    Label.joins(:label_links)
+      .where('label_links.target_id' => issues_visible_to_user(user).select(:id), 'label_links.target_type' => 'Issue')
+      .distinct
+  end
+
   def sorted_issues(user)
     issues_visible_to_user(user).preload_associations.sort_by_attribute('label_priority')
   end

app/views/shared/milestones/_tabs.html.haml

@@ -21,11 +21,11 @@
     %li.nav-item
       = link_to '#tab-participants', class: 'nav-link', 'data-toggle' => 'tab', 'data-endpoint': milestone_participants_tab_path(milestone) do
         Participants
-        %span.badge.badge-pill= milestone.participants.count
+        %span.badge.badge-pill= milestone.issue_participants_visible_by_user(current_user).count
     %li.nav-item
       = link_to '#tab-labels', class: 'nav-link', 'data-toggle' => 'tab', 'data-endpoint': milestone_labels_tab_path(milestone) do
         Labels
-        %span.badge.badge-pill= milestone.labels.count
+        %span.badge.badge-pill= milestone.issue_labels_visible_by_user(current_user).count
 
 - issues = milestone.sorted_issues(current_user)
 - show_project_name = local_assigns.fetch(:show_project_name, false)

changelogs/unreleased/security-2774-milestones-detail.yml

+---
+title: Display only information visible to current user on the Milestone page
+merge_request:
+author:
+type: security

ee/app/controllers/groups/omniauth_callbacks_controller.rb

@@ -9,9 +9,11 @@ class Groups::OmniauthCallbacksController < OmniauthCallbacksController
     @unauthenticated_group = Group.find_by_full_path(params[:group_id])
     @saml_provider = @unauthenticated_group.saml_provider
 
-    identity_linker = Gitlab::Auth::GroupSaml::IdentityLinker.new(current_user, oauth, @saml_provider)
+    identity_linker = Gitlab::Auth::GroupSaml::IdentityLinker.new(current_user, oauth, @saml_provider, session)
 
     omniauth_flow(Gitlab::Auth::GroupSaml, identity_linker: identity_linker)
+  rescue Gitlab::Auth::GroupSaml::IdentityLinker::UnverifiedRequest
+    redirect_unverified_saml_initiation
   end
 
   private
@@ -44,6 +46,12 @@ class Groups::OmniauthCallbacksController < OmniauthCallbacksController
     super
   end
 
+  def redirect_unverified_saml_initiation
+    flash[:notice] = "Request to link SAML account must be authorized"
+
+    redirect_to sso_group_saml_providers_path(@unauthenticated_group)
+  end
+
   override :after_sign_in_path_for
   def after_sign_in_path_for(resource)
     saml_redirect_path || super

ee/app/helpers/ee/milestones_helper.rb

@@ -3,7 +3,7 @@
 module EE
   module MilestonesHelper
     def burndown_chart(milestone)
-      Burndown.new(milestone) if milestone.supports_burndown_charts?
+      Burndown.new(milestone, current_user) if milestone.supports_burndown_charts?
     end
 
     def can_generate_chart?(milestone, burndown)

ee/app/models/burndown.rb

@@ -17,12 +17,13 @@ class Burndown
     end
   end
 
-  attr_reader :start_date, :due_date, :end_date, :accurate, :legacy_data, :milestone
+  attr_reader :start_date, :due_date, :end_date, :accurate, :legacy_data, :milestone, :current_user
   alias_method :accurate?, :accurate
   alias_method :empty?, :legacy_data
 
-  def initialize(milestone)
+  def initialize(milestone, current_user)
     @milestone = milestone
+    @current_user = current_user
     @start_date = @milestone.start_date
     @due_date = @milestone.due_date
     @end_date = @milestone.due_date
@@ -89,10 +90,10 @@ class Burndown
     strong_memoize(:opened_issues_grouped_by_date) do
       issues =
         @milestone
-          .issues
-          .where('created_at <= ?', end_date)
+          .issues_visible_to_user(current_user)
+          .where('issues.created_at <= ?', end_date)
           .reorder(nil)
-          .order(:created_at).to_a
+          .order('issues.created_at').to_a
 
       issues.group_by do |issue|
         issue.created_at.to_date
@@ -124,9 +125,8 @@ class Burndown
         # `issues.closed_at` can't be used once it's nullified if the issue is
         # reopened.
         internal_clause =
-          ::Issue
+          @milestone.issues_visible_to_user(current_user)
             .joins("LEFT OUTER JOIN events e ON issues.id = e.target_id AND e.target_type = 'Issue' AND e.action = #{Event::CLOSED}")
-            .where(milestone: @milestone)
             .where("state = 'closed' OR (state = 'opened' AND e.action = #{Event::CLOSED})") # rubocop:disable GitlabSecurity/SqlInjection
 
         rel =

ee/app/services/ee/projects/group_links/create_service.rb

@@ -8,7 +8,10 @@ module EE
 
         override :execute
         def execute(group)
-          super.tap { |link| log_audit_event(link) if link && link&.persisted? }
+          result = super
+
+          log_audit_event(result[:link]) if result[:status] == :success
+          result
         end
 
         private

ee/changelogs/unreleased/2802-security-add-public-internal-groups-as-members-to-your-project-idor.yml

+---
+title: Remove the possibility to share a project with a group that a user is not a member
+  of
+merge_request:
+author:
+type: security

ee/changelogs/unreleased/jej-group-saml-link-origin-verification.yml

+---
+title: Prevent Group SAML authorizing sign in without prior user approval
+merge_request:
+author:
+type: security

ee/changelogs/unreleased/security-maintainer-can-ignore-membership-lock.yml

+---
+title: Respect group membership lock when importing a member from another group
+merge_request:
+author:
+type: security

ee/lib/gitlab/auth/group_saml/identity_linker.rb

@@ -4,15 +4,20 @@ module Gitlab
   module Auth
     module GroupSaml
       class IdentityLinker < Gitlab::Auth::Saml::IdentityLinker
-        attr_reader :saml_provider
+        attr_reader :saml_provider, :session
 
-        def initialize(current_user, oauth, saml_provider)
+        UnverifiedRequest = Class.new(StandardError)
+
+        def initialize(current_user, oauth, saml_provider, session)
           super(current_user, oauth)
 
           @saml_provider = saml_provider
+          @session = session
         end
 
         def link
+          raise_unless_request_is_gitlab_initiated! if unlinked?
+
           super
 
           update_group_membership unless failed?
@@ -32,6 +37,18 @@ module Gitlab
         def update_group_membership
           MembershipUpdater.new(current_user, saml_provider).execute
         end
+
+        def raise_unless_request_is_gitlab_initiated!
+          raise UnverifiedRequest unless valid_gitlab_initated_request?
+        end
+
+        def valid_gitlab_initated_request?
+          SamlOriginValidator.new(session).gitlab_initiated?(saml_response)
+        end
+
+        def saml_response
+          oauth.extra.response_object
+        end
       end
     end
   end

ee/lib/gitlab/auth/saml_origin_validator.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Auth
+    class SamlOriginValidator
+      attr_reader :session
+
+      AUTH_REQUEST_SESSION_KEY = "last_authn_request_id".freeze
+
+      def initialize(session)
+        @session = session
+      end
+
+      def store_origin(authn_request)
+        session[AUTH_REQUEST_SESSION_KEY] = authn_request.uuid
+      end
+
+      def gitlab_initiated?(saml_response)
+        return false if identity_provider_initiated?(saml_response)
+
+        matches?(saml_response)
+      end
+
+      private
+
+      def matches?(saml_response)
+        saml_response.in_response_to == expected_request_id
+      end
+
+      def identity_provider_initiated?(saml_response)
+        saml_response.in_response_to.blank?
+      end
+
+      def expected_request_id
+        session[AUTH_REQUEST_SESSION_KEY]
+      end
+    end
+  end
+end

ee/lib/omni_auth/strategies/group_saml.rb

@@ -36,6 +36,20 @@ module OmniAuth
         end
       end
 
+      # NOTE: This method duplicates code from omniauth-saml
+      #       so that we can access authn_request to store it
+      #       See: https://github.com/omniauth/omniauth-saml/issues/172
+      override :request_phase
+      def request_phase
+        authn_request = OneLogin::RubySaml::Authrequest.new
+
+        store_authn_request_id(authn_request)
+
+        with_settings do |settings|
+          redirect(authn_request.create(settings, additional_params_for_authn_request))
+        end
+      end
+
       def self.invalid_group!(path)
         raise ActionController::RoutingError, path
       end
@@ -54,6 +68,10 @@ module OmniAuth
         Feature.enabled?(:group_saml_metadata_available, group_lookup.group)
       end
 
+      def store_authn_request_id(authn_request)
+        Gitlab::Auth::SamlOriginValidator.new(session).store_origin(authn_request)
+      end
+
       def group_lookup
         @group_lookup ||= Gitlab::Auth::GroupSaml::GroupLookup.new(env)
       end

ee/spec/controllers/groups/omniauth_callbacks_controller_spec.rb

@@ -9,6 +9,9 @@ describe Groups::OmniauthCallbacksController do
   let(:provider) { :group_saml }
   let(:group) { create(:group, :private) }
   let!(:saml_provider) { create(:saml_provider, group: group) }
+  let(:in_response_to) { '12345' }
+  let(:last_request_id) { in_response_to }
+  let(:saml_response) { instance_double(OneLogin::RubySaml::Response, in_response_to: in_response_to) }
 
   before do
     stub_licensed_features(group_saml: true)
@@ -22,6 +25,10 @@ describe Groups::OmniauthCallbacksController do
     create(:omniauth_user, extern_uid: uid, provider: provider, saml_provider: saml_provider)
   end
 
+  def stub_last_request_id(id)
+    session["last_authn_request_id"] = id
+  end
+
   context "when request hasn't been validated by omniauth middleware" do
     it "prevents authentication" do
       sign_in(user)
@@ -34,8 +41,9 @@ describe Groups::OmniauthCallbacksController do
 
   context "valid credentials" do
     before do
-      mock_auth_hash(provider, uid, user.email)
+      mock_auth_hash(provider, uid, user.email, response_object: saml_response)
       stub_omniauth_provider(provider, context: request)
+      stub_last_request_id(last_request_id)
     end
 
     shared_examples "and identity already linked" do
@@ -104,6 +112,22 @@ describe Groups::OmniauthCallbacksController do
 
           expect(flash[:notice]).to match(/SAML for .* was added/)
         end
+
+        context 'with IdP initiated request' do
+          let(:last_request_id) { '99999' }
+
+          it 'redirects to account link page' do
+            post provider, params: { group_id: group }
+
+            expect(response).to redirect_to(sso_group_saml_providers_path(group))
+          end
+
+          it "lets the user know their account isn't linked yet" do
+            post provider, params: { group_id: group }
+
+            expect(flash[:notice]).to eq 'Request to link SAML account must be authorized'
+          end
+        end
       end
     end
 

ee/spec/features/profiles/account_spec.rb

@@ -20,7 +20,9 @@ describe 'Profile > Account' do
 
     def create_linked_identity
       oauth = { 'provider' => 'group_saml', 'uid' => '1' }
-      Gitlab::Auth::GroupSaml::IdentityLinker.new(user, oauth, saml_provider).link
+      identity_linker = Gitlab::Auth::GroupSaml::IdentityLinker.new(user, oauth, saml_provider, double(:session))
+      allow(identity_linker).to receive(:valid_gitlab_initated_request?).and_return(true)
+      identity_linker.link
     end
 
     before do

ee/spec/features/projects/members/invite_group_and_members_spec.rb

@@ -57,6 +57,7 @@ describe 'Project > Members > Invite group and members', :js do
 
       before do
         project.add_maintainer(maintainer)
+        group_to_share_with.add_developer(maintainer)
         sign_in(maintainer)
       end
 

ee/spec/lib/gitlab/auth/group_saml/identity_linker_spec.rb

@@ -4,10 +4,13 @@ describe Gitlab::Auth::GroupSaml::IdentityLinker do
   let(:user) { create(:user) }
   let(:provider) { 'group_saml' }
   let(:uid) { user.email }
-  let(:oauth) { { 'provider' => provider, 'uid' => uid } }
+  let(:in_response_to) { '12345' }
+  let(:saml_response) { instance_double(OneLogin::RubySaml::Response, in_response_to: in_response_to) }
+  let(:oauth) { OmniAuth::AuthHash.new(provider: provider, uid: uid, extra: { response_object: saml_response }) }
   let(:saml_provider) { create(:saml_provider) }
+  let(:session) { {} }
 
-  subject { described_class.new(user, oauth, saml_provider) }
+  subject { described_class.new(user, oauth, saml_provider, session) }
 
   context 'linked identity exists' do
     let!(:identity) { user.identities.create!(provider: provider, extern_uid: uid, saml_provider: saml_provider) }
@@ -30,6 +33,15 @@ describe Gitlab::Auth::GroupSaml::IdentityLinker do
   end
 
   context 'identity needs to be created' do
+    context 'with identity provider initiated request' do
+      it 'attempting to link accounts raises an exception' do
+        expect { subject.link }.to raise_error(Gitlab::Auth::GroupSaml::IdentityLinker::UnverifiedRequest)
+      end
+    end
+
+    context 'with valid gitlab initiated request' do
+      let(:session) { { 'last_authn_request_id' => in_response_to } }
+
       it 'creates linked identity' do
         expect { subject.link }.to change { user.identities.count }
       end
@@ -64,4 +76,5 @@ describe Gitlab::Auth::GroupSaml::IdentityLinker do
         expect(saml_provider.group.member?(user)).to eq(true)
       end
     end
+  end
 end

ee/spec/lib/gitlab/auth/saml_origin_validator_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::Auth::SamlOriginValidator do
+  let(:session) { instance_double(ActionDispatch::Request::Session) }
+
+  subject { described_class.new(session) }
+
+  describe '#store_origin' do
+    it 'stores the SAML request ID' do
+      request_id = double
+      authn_request = instance_double(OneLogin::RubySaml::Authrequest, uuid: request_id)
+
+      expect(session).to receive(:[]=).with('last_authn_request_id', request_id)
+
+      subject.store_origin(authn_request)
+    end
+  end
+
+  describe '#gitlab_initiated?' do
+    it 'returns false if InResponseTo is not present' do
+      saml_response = instance_double(OneLogin::RubySaml::Response, in_response_to: nil)
+
+      expect(subject.gitlab_initiated?(saml_response)).to eq(false)
+    end
+
+    it 'returns false if InResponseTo does not match stored value' do
+      saml_response = instance_double(OneLogin::RubySaml::Response, in_response_to: "abc")
+      allow(session).to receive(:[]).with('last_authn_request_id').and_return('123')
+
+      expect(subject.gitlab_initiated?(saml_response)).to eq(false)
+    end
+
+    it 'returns true if InResponseTo matches stored value' do
+      saml_response = instance_double(OneLogin::RubySaml::Response, in_response_to: "123")
+      allow(session).to receive(:[]).with('last_authn_request_id').and_return('123')
+
+      expect(subject.gitlab_initiated?(saml_response)).to eq(true)
+    end
+  end
+end

ee/spec/lib/omni_auth/strategies/group_saml_spec.rb

@@ -110,6 +110,15 @@ describe OmniAuth::Strategies::GroupSaml, type: :strategy do
         post '/users/auth/group_saml'
       end.to raise_error(ActionController::RoutingError)
     end
+
+    it "stores request ID during request phase" do
+      request_id = double
+      allow_any_instance_of(OneLogin::RubySaml::Authrequest).to receive(:uuid).and_return(request_id)
+
+      post '/users/auth/group_saml', group_path: 'my-group'
+
+      expect(session['last_authn_request_id']).to eq(request_id)
+    end
   end
 
   describe 'POST /users/auth/group_saml/metadata' do

ee/spec/models/burndown_spec.rb

@@ -2,6 +2,7 @@ require 'spec_helper'
 
 describe Burndown do
   set(:user) { create(:user) }
+  set(:non_member) { create(:user) }
   let(:start_date) { "2017-03-01" }
   let(:due_date) { "2017-03-03" }
 
@@ -16,13 +17,13 @@ describe Burndown do
       end
     end
 
-    subject { described_class.new(milestone).to_json }
+    subject { described_class.new(milestone, user).to_json }
 
     it "generates an array with date, issue count and weight" do
       expect(subject).to eq([
-        ["2017-03-01", 3, 6],
-        ["2017-03-02", 4, 8],
-        ["2017-03-03", 2, 4]
+        ["2017-03-01", 4, 8],
+        ["2017-03-02", 5, 10],
+        ["2017-03-03", 3, 6]
       ].to_json)
     end
 
@@ -45,7 +46,7 @@ describe Burndown do
     end
 
     it "sets attribute accurate to true" do
-      burndown = described_class.new(milestone)
+      burndown = described_class.new(milestone, user)
 
       expect(burndown).to be_accurate
     end
@@ -57,14 +58,14 @@ describe Burndown do
 
       it "considers closed_at as milestone start date" do
         expect(subject).to eq([
-          ["2017-03-01", 3, 6],
-          ["2017-03-02", 3, 6],
-          ["2017-03-03", 3, 6]
+          ["2017-03-01", 4, 8],
+          ["2017-03-02", 4, 8],
+          ["2017-03-03", 4, 8]
         ].to_json)
       end
 
       it "sets attribute empty to true" do
-        burndown = described_class.new(milestone)
+        burndown = described_class.new(milestone, user)
 
         expect(burndown).to be_empty
       end
@@ -76,7 +77,7 @@ describe Burndown do
       end
 
       it "sets attribute accurate to false" do
-        burndown = described_class.new(milestone)
+        burndown = described_class.new(milestone, user)
 
         expect(burndown).not_to be_accurate
       end
@@ -92,12 +93,26 @@ describe Burndown do
         create(:issue, milestone: milestone, project: project, created_at: creation_date, weight: 3)
 
         expect(subject).to eq([
-          ['2017-03-01', 3, 6],
-          ['2017-03-02', 6, 13],
-          ['2017-03-03', 4, 9]
+          ['2017-03-01', 4, 8],
+          ['2017-03-02', 7, 15],
+          ['2017-03-03', 5, 11]
         ].to_json)
       end
     end
+
+    context 'when issues belong to a public project' do
+      it 'does not include confidential issues for users who are not project members' do
+        project.update(visibility_level: Gitlab::VisibilityLevel::PUBLIC)
+
+        expected_result = [
+            ["2017-03-01", 3, 6],
+            ["2017-03-02", 4, 8],
+            ["2017-03-03", 2, 4]
+        ].to_json
+
+        expect(described_class.new(milestone, non_member).to_json).to eq(expected_result)
+      end
+    end
   end
 
   describe 'project milestone burndown' do
@@ -126,6 +141,10 @@ describe Burndown do
     let(:nested_group_milestone) { create(:milestone, group: nested_group, start_date: start_date, due_date: due_date) }
 
     context 'when nested group milestone', :nested_groups do
+      before do
+        group.add_developer(user)
+      end
+
       it_behaves_like 'burndown for milestone' do
         let(:milestone) { nested_group_milestone }
         let(:project) { nested_group_project }
@@ -194,6 +213,9 @@ describe Burndown do
         issue_closed_twice = reopened_issues.last
         close_issue(issue_closed_twice)
         reopen_issue(issue_closed_twice)
+
+        # create one confidential issue
+        create(:issue, :confidential, issue_params) if Date.today == milestone.start_date
       end
     end
   end

ee/spec/services/projects/group_links/create_service_spec.rb

@@ -36,6 +36,7 @@ describe Projects::GroupLinks::CreateService, '#execute' do
   end
 
   def create_group_link(user, project, group, opts)
+    group.add_developer(user)
     described_class.new(project, user, opts).execute(group)
   end
 end

spec/controllers/projects/project_members_controller_spec.rb

@@ -2,7 +2,8 @@ require('spec_helper')
 
 describe Projects::ProjectMembersController do
   let(:user) { create(:user) }
-  let(:project) { create(:project, :public, :access_requestable) }
+  let(:project) { create(:project, :public, :access_requestable, namespace: namespace) }
+  let(:namespace) { create :group }
 
   describe 'GET index' do
     it 'should have the project_members address with a 200 status code' do
@@ -313,6 +314,13 @@ describe Projects::ProjectMembersController do
   end
 
   describe 'POST apply_import' do
+    subject(:apply_import) do
+      post(:apply_import, params: {
+        namespace_id: project.namespace,
+        project_id: project,
+        source_project_id: another_project.id
+      })
+    end
     let(:another_project) { create(:project, :private) }
     let(:member) { create(:user) }
 
@@ -322,40 +330,44 @@ describe Projects::ProjectMembersController do
       sign_in(user)
     end
 
-    shared_context 'import applied' do
-      before do
-        post(:apply_import, params: {
-                              namespace_id: project.namespace,
-                              project_id: project,
-                              source_project_id: another_project.id
-                            })
-      end
-    end
-
     context 'when user can access source project members' do
       before do
         another_project.add_guest(user)
       end
 
-      include_context 'import applied'
-
       it 'imports source project members' do
+        apply_import
+
         expect(project.team_members).to include member
         expect(response).to set_flash.to 'Successfully imported'
         expect(response).to redirect_to(
           project_project_members_path(project)
         )
       end
+
+      context 'and the project group has membership lock enabled' do
+        before do
+          project.namespace.update(membership_lock: true)
         end
 
-    context 'when user is not member of a source project' do
-      include_context 'import applied'
+        it 'responds with 403' do
+          apply_import
 
+          expect(response.status).to eq 403
+        end
+      end
+    end
+
+    context 'when user is not member of a source project' do
       it 'does not import team members' do
+        apply_import
+
         expect(project.team_members).not_to include member
       end
 
       it 'responds with not found' do
+        apply_import
+
         expect(response.status).to eq 404
       end
     end
@@ -363,40 +375,78 @@ describe Projects::ProjectMembersController do
 
   describe 'POST create' do
     let(:stranger) { create(:user) }
+    subject(:create_member) do
+      post :create, params: {
+        user_ids: stranger.id,
+        namespace_id: project.namespace,
+        access_level: access_level,
+        project_id: project
+      }
+    end
+    let(:access_level) { nil }
 
-    context 'when creating owner' do
     before do
       project.add_maintainer(user)
       sign_in(user)
     end
 
+    context 'when creating owner' do
+      let(:access_level) { Member::OWNER }
+
       it 'does not create a member' do
-        expect do
-          post :create, params: {
-                          user_ids: stranger.id,
-                          namespace_id: project.namespace,
-                          access_level: Member::OWNER,
-                          project_id: project
-                        }
-        end.to change { project.members.count }.by(0)
+        expect { create_member }.not_to change { project.members.count }
       end
     end
 
     context 'when create maintainer' do
+      let(:access_level) { Member::MAINTAINER }
+
+      it 'creates a member' do
+        expect { create_member }.to change { project.members.count }.by(1)
+      end
+    end
+
+    context 'when project group has membership lock enabled' do
       before do
-        project.add_maintainer(user)
-        sign_in(user)
+        project.namespace.update(membership_lock: true)
       end
 
-      it 'creates a member' do
-        expect do
-          post :create, params: {
-                          user_ids: stranger.id,
+      it 'responds with 403' do
+        create_member
+
+        expect(response.status).to eq 403
+      end
+    end
+  end
+
+  describe 'GET import' do
+    subject(:import) do
+      get :import, params: {
         namespace_id: project.namespace,
-                          access_level: Member::MAINTAINER,
         project_id: project
       }
-        end.to change { project.members.count }.by(1)
+    end
+
+    before do
+      project.add_maintainer(user)
+      sign_in(user)
+    end
+
+    it 'responds with 200' do
+      import
+
+      expect(response.status).to eq 200
+    end
+
+    context 'when project group has membership lock enabled' do
+      before do
+        project.namespace.update(membership_lock: true)
+      end
+
+      it 'responds with 403' do
+        import
+
+        expect(response.status).to eq 403
       end
     end
   end

spec/models/concerns/milestoneish_spec.rb

@@ -9,8 +9,10 @@ describe Milestone, 'Milestoneish' do
   let(:admin) { create(:admin) }
   let(:project) { create(:project, :public) }
   let(:milestone) { create(:milestone, project: project) }
-  let!(:issue) { create(:issue, project: project, milestone: milestone) }
-  let!(:security_issue_1) { create(:issue, :confidential, project: project, author: author, milestone: milestone) }
+  let(:label1) { create(:label, project: project) }
+  let(:label2) { create(:label, project: project) }
+  let!(:issue) { create(:issue, project: project, milestone: milestone, assignees: [member], labels: [label1]) }
+  let!(:security_issue_1) { create(:issue, :confidential, project: project, author: author, milestone: milestone, labels: [label2]) }
   let!(:security_issue_2) { create(:issue, :confidential, project: project, assignees: [assignee], milestone: milestone) }
   let!(:closed_issue_1) { create(:issue, :closed, project: project, milestone: milestone) }
   let!(:closed_issue_2) { create(:issue, :closed, project: project, milestone: milestone) }
@@ -42,6 +44,95 @@ describe Milestone, 'Milestoneish' do
     end
   end
 
+  context 'attributes visibility' do
+    using RSpec::Parameterized::TableSyntax
+
+    let(:users) do
+      {
+        anonymous: nil,
+        non_member: non_member,
+        guest: guest,
+        member: member,
+        assignee: assignee
+      }
+    end
+
+    let(:project_visibility_levels) do
+      {
+        public: Gitlab::VisibilityLevel::PUBLIC,
+        internal: Gitlab::VisibilityLevel::INTERNAL,
+        private: Gitlab::VisibilityLevel::PRIVATE
+      }
+    end
+
+    describe '#issue_participants_visible_by_user' do
+      where(:visibility, :user_role, :result) do
+        :public   | nil         | [:member]
+        :public   | :non_member | [:member]
+        :public   | :guest      | [:member]
+        :public   | :member     | [:member, :assignee]
+        :internal | nil         | []
+        :internal | :non_member | [:member]
+        :internal | :guest      | [:member]
+        :internal | :member     | [:member, :assignee]
+        :private  | nil         | []
+        :private  | :non_member | []
+        :private  | :guest      | [:member]
+        :private  | :member     | [:member, :assignee]
+      end
+
+      with_them do
+        before do
+          project.update(visibility_level: project_visibility_levels[visibility])
+        end
+
+        it 'returns the proper participants' do
+          user = users[user_role]
+          participants = result.map { |role| users[role] }
+
+          expect(milestone.issue_participants_visible_by_user(user)).to match_array(participants)
+        end
+      end
+    end
+
+    describe '#issue_labels_visible_by_user' do
+      let(:labels) do
+        {
+          label1: label1,
+          label2: label2
+        }
+      end
+
+      where(:visibility, :user_role, :result) do
+        :public   | nil         | [:label1]
+        :public   | :non_member | [:label1]
+        :public   | :guest      | [:label1]
+        :public   | :member     | [:label1, :label2]
+        :internal | nil         | []
+        :internal | :non_member | [:label1]
+        :internal | :guest      | [:label1]
+        :internal | :member     | [:label1, :label2]
+        :private  | nil         | []
+        :private  | :non_member | []
+        :private  | :guest      | [:label1]
+        :private  | :member     | [:label1, :label2]
+      end
+
+      with_them do
+        before do
+          project.update(visibility_level: project_visibility_levels[visibility])
+        end
+
+        it 'returns the proper participants' do
+          user = users[user_role]
+          expected_labels = result.map { |label| labels[label] }
+
+          expect(milestone.issue_labels_visible_by_user(user)).to match_array(expected_labels)
+        end
+      end
+    end
+  end
+
   describe '#sorted_merge_requests' do
     it 'sorts merge requests by label priority' do
       merge_request_1 = create(:labeled_merge_request, labels: [label_2], source_project: project, source_branch: 'branch_1', milestone: milestone)