Merge branch 'bvl-wrap-cross-project-group-permissions-ee' into 'master'

[EE-Port] Create cross project group features

Closes #4829

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

app/helpers/groups_helper.rb

@@ -130,8 +130,10 @@ module GroupsHelper
   def get_group_sidebar_links
     links = [:overview, :group_members]
 
-    if can?(current_user, :read_cross_project)
-      links += [:activity, :issues, :boards, :labels, :milestones, :merge_requests]
+    resources = [:activity, :issues, :boards, :labels, :milestones,
+                 :merge_requests]
+    links += resources.select do |resource|
+      can?(current_user, "read_group_#{resource}".to_sym, @group)
     end
 
     if can?(current_user, :admin_group, @group)

app/policies/group_policy.rb

@@ -75,6 +75,19 @@ class GroupPolicy < BasePolicy
     enable :change_visibility_level
   end
 
+  rule { can?(:read_nested_project_resources) }.policy do
+    enable :read_group_activity
+    enable :read_group_issues
+    enable :read_group_boards
+    enable :read_group_labels
+    enable :read_group_milestones
+    enable :read_group_merge_requests
+  end
+
+  rule { can?(:read_cross_project) & can?(:read_group) }.policy do
+    enable :read_nested_project_resources
+  end
+
   rule { owner & nested_groups_supported }.enable :create_subgroup
 
   rule { public_group | logged_in_viewable }.enable :view_globally

ee/app/helpers/ee/groups_helper.rb

@@ -22,18 +22,12 @@ module EE
     def get_group_sidebar_links
       links = super
 
-      if can?(current_user, :read_cross_project)
-        if @group.feature_available?(:contribution_analytics) || show_promotions?
-          links << :contribution_analytics
-        end
-
-        if @group.feature_available?(:group_issue_boards)
-          links << :boards
-        end
-
-        if @group.feature_available?(:epics)
-          links << :epics
-        end
+      if can?(current_user, :read_group_contribution_analytics, @group) || show_promotions?
+        links << :contribution_analytics
+      end
+
+      if can?(current_user, :read_epic, @group)
+        links << :epics
       end
 
       links

ee/app/policies/ee/group_policy.rb

@@ -5,7 +5,10 @@ module EE
     prepended do
       with_scope :subject
       condition(:ldap_synced) { @subject.ldap_synced? }
-      condition(:epics_disabled) { !@subject.feature_available?(:epics) }
+      condition(:epics_available) { @subject.feature_available?(:epics) }
+      condition(:contribution_analytics_available) do
+        @subject.feature_available?(:contribution_analytics)
+      end
 
       condition(:project_creation_level_enabled) { @subject.feature_available?(:project_creation_level) }
 
@@ -17,37 +20,38 @@ module EE
         @subject.project_creation_level == ::EE::Gitlab::Access::DEVELOPER_MASTER_PROJECT_ACCESS
       end
 
-      rule { reporter }.policy do
-        enable :admin_list
-        enable :admin_board
-      end
-
       condition(:can_owners_manage_ldap, scope: :global) do
         ::Gitlab::CurrentSettings.current_application_settings
           .allow_group_owners_to_manage_ldap
       end
 
-      rule { public_group }.enable :read_epic
+      rule { reporter }.policy do
+        enable :admin_list
+        enable :admin_board
+      end
 
-      rule { logged_in_viewable }.enable :read_epic
+      rule { can?(:read_group) & contribution_analytics_available }
+        .enable :read_group_contribution_analytics
 
-      rule { guest }.enable :read_epic
+      rule { can?(:read_group) & epics_available }.enable :read_epic
 
-      rule { reporter }.policy do
+      rule { reporter & epics_available }.policy do
         enable :create_epic
         enable :admin_epic
         enable :update_epic
       end
 
-      rule { owner }.enable :destroy_epic
+      rule { owner & epics_available }.enable :destroy_epic
 
-      rule { auditor }.policy do
-        enable  :read_group
-        enable  :read_epic
+      rule { ~can?(:read_cross_project) }.policy do
+        prevent :read_group_contribution_analytics
+        prevent :read_epic
+        prevent :create_epic
+        prevent :admin_epic
+        prevent :update_epic
       end
 
-      rule { admin }.enable :read_epic
-      rule { has_projects }.enable :read_epic
+      rule { auditor }.enable :read_group
 
       rule { admin | owner }.enable :admin_group_saml
 
@@ -59,14 +63,6 @@ module EE
 
       rule { ldap_synced & (admin | (can_owners_manage_ldap & owner)) }.enable :override_group_member
 
-      rule { epics_disabled }.policy do
-        prevent :read_epic
-        prevent :create_epic
-        prevent :admin_epic
-        prevent :update_epic
-        prevent :destroy_epic
-      end
-
       rule { project_creation_level_enabled & developer & developer_master_access }.enable :create_projects
       rule { project_creation_level_enabled & create_projects_disabled }.prevent :create_projects
     end

ee/spec/finders/epics_finder_spec.rb

@@ -27,7 +27,9 @@ describe EpicsFinder do
       end
     end
 
-    context 'when epics feature is enabled' do
+    # Enabeling the `request_store` for this to avoid counting queries that check
+    # the license.
+    context 'when epics feature is enabled', :request_store do
       before do
         stub_licensed_features(epics: true)
       end

ee/spec/helpers/ee/groups_helper_spec.rb

@@ -3,21 +3,28 @@ require 'spec_helper'
 describe GroupsHelper do
   describe '#group_sidebar_links' do
     let(:user) { create(:user) }
-    let(:group) { create(:group) }
+    let(:group) { create(:group, :private) }
 
     before do
       allow(helper).to receive(:current_user) { user }
+      group.add_owner(user)
       helper.instance_variable_set(:@group, group)
-      allow(helper).to receive(:can?).with(user, :admin_group, group) { false }
+      allow(helper).to receive(:can?) { |*args| Ability.allowed?(*args) }
+      allow(helper).to receive(:show_promotions?) { false }
     end
 
-    it 'shows the licenced cross project features when the user can read cross project' do
-      expect(helper).to receive(:can?).with(user, :read_cross_project).at_least(1) { true }
+    it 'shows the licensed features when they are available' do
       stub_licensed_features(contribution_analytics: true,
-                             group_issue_boards: true,
                              epics: true)
 
-      expect(helper.group_sidebar_links).to include(:contribution_analytics, :boards, :epics)
+      expect(helper.group_sidebar_links).to include(:contribution_analytics, :epics)
+    end
+
+    it 'hides the licensed features when they are not available' do
+      stub_licensed_features(contribution_analytics: false,
+                             epics: false)
+
+      expect(helper.group_sidebar_links).not_to include(:contribution_analytics, :epics)
     end
   end
 end

ee/spec/policies/group_policy_spec.rb

@@ -36,6 +36,26 @@ describe GroupPolicy do
     it { is_expected.to be_allowed(:read_epic, :create_epic, :admin_epic, :destroy_epic) }
   end
 
+  context 'when contribution analytics is available' do
+    let(:current_user) { developer }
+
+    before do
+      stub_licensed_features(contribution_analytics: true)
+    end
+
+    it { is_expected.to be_allowed(:read_group_contribution_analytics) }
+  end
+
+  context 'when contribution analytics is not available' do
+    let(:current_user) { developer }
+
+    before do
+      stub_licensed_features(contribution_analytics: false)
+    end
+
+    it { is_expected.not_to be_allowed(:read_group_contribution_analytics) }
+  end
+
   describe 'per group SAML' do
     let(:current_user) { master }
 

ee/spec/views/layouts/nav/sidebar/_group.html.haml_spec.rb

@@ -21,7 +21,7 @@ describe 'layouts/nav/sidebar/_group' do
         allow(License).to receive(:current).and_return(nil)
         stub_application_setting(check_namespace_plan: false)
 
-        allow(view).to receive(:can?).and_return(true)
+        allow(view).to receive(:can?) { |*args| Ability.allowed?(*args) }
         allow(view).to receive(:current_user).and_return(cuser)
       end
 

spec/helpers/groups_helper_spec.rb

@@ -206,8 +206,9 @@ describe GroupsHelper do
     let(:group) { create(:group, :public) }
     let(:user) { create(:user) }
     before do
+      group.add_owner(user)
       allow(helper).to receive(:current_user) { user }
-      allow(helper).to receive(:can?) { true }
+      allow(helper).to receive(:can?) { |*args| Ability.allowed?(*args) }
       helper.instance_variable_set(:@group, group)
     end
 
@@ -231,7 +232,10 @@ describe GroupsHelper do
       cross_project_features = [:activity, :issues, :labels, :milestones,
                                 :merge_requests]
 
-      expect(helper).to receive(:can?).with(user, :read_cross_project) { false }
+      allow(Ability).to receive(:allowed?).and_call_original
+      cross_project_features.each do |feature|
+        expect(Ability).to receive(:allowed?).with(user, "read_group_#{feature}".to_sym, group) { false }
+      end
 
       expect(helper.group_sidebar_links).not_to include(*cross_project_features)
     end

spec/policies/group_policy_spec.rb

@@ -10,7 +10,11 @@ describe GroupPolicy do
   let(:admin) { create(:admin) }
   let(:group) { create(:group, :private) }
 
-  let(:guest_permissions) { [:read_label, :read_group, :upload_file, :read_namespace] }
+  let(:guest_permissions) do
+    [:read_label, :read_group, :upload_file, :read_namespace, :read_group_activity,
+     :read_group_issues, :read_group_boards, :read_group_labels, :read_group_milestones,
+     :read_group_merge_requests]
+  end
 
   let(:reporter_permissions) { [:admin_label] }