Merge branch 'move-group-wiki-policies-into-ee' into 'master'

Move group policies for group wikis into EE

See merge request gitlab-org/gitlab!30867

app/models/group.rb

@@ -478,16 +478,6 @@ class Group < Namespace
     false
   end
 
-  def wiki_access_level
-    # TODO: Remove this method once we implement group-level features.
-    # https://gitlab.com/gitlab-org/gitlab/-/issues/208412
-    if Feature.enabled?(:group_wiki, self)
-      ProjectFeature::ENABLED
-    else
-      ProjectFeature::DISABLED
-    end
-  end
-
   private
 
   def update_two_factor_requirement

app/policies/group_policy.rb

 # frozen_string_literal: true
 
 class GroupPolicy < BasePolicy
-  include CrudPolicyHelpers
   include FindGroupProjects
 
   desc "Group is public"
@@ -43,23 +42,15 @@ class GroupPolicy < BasePolicy
     @subject.subgroup_creation_level == ::Gitlab::Access::MAINTAINER_SUBGROUP_ACCESS
   end
 
-  desc "Group has wiki disabled"
-  condition(:wiki_disabled, score: 32) { !feature_available?(:wiki) }
-
   rule { public_group }.policy do
     enable :read_group
     enable :read_package
-    enable :read_wiki
   end
 
-  rule { logged_in_viewable }.policy do
-    enable :read_group
-    enable :read_wiki
-  end
+  rule { logged_in_viewable }.enable :read_group
 
   rule { guest }.policy do
     enable :read_group
-    enable :read_wiki
     enable :upload_file
   end
 
@@ -87,13 +78,11 @@ class GroupPolicy < BasePolicy
     enable :create_metrics_dashboard_annotation
     enable :delete_metrics_dashboard_annotation
     enable :update_metrics_dashboard_annotation
-    enable :create_wiki
   end
 
   rule { reporter }.policy do
     enable :reporter_access
     enable :read_container_image
-    enable :download_wiki_code
     enable :admin_label
     enable :admin_list
     enable :admin_issue
@@ -112,7 +101,6 @@ class GroupPolicy < BasePolicy
     enable :destroy_deploy_token
     enable :read_deploy_token
     enable :create_deploy_token
-    enable :admin_wiki
   end
 
   rule { owner }.policy do
@@ -159,11 +147,6 @@ class GroupPolicy < BasePolicy
 
   rule { maintainer & can?(:create_projects) }.enable :transfer_projects
 
-  rule { wiki_disabled }.policy do
-    prevent(*create_read_update_admin_destroy(:wiki))
-    prevent(:download_wiki_code)
-  end
-
   def access_level
     return GroupMember::NO_ACCESS if @user.nil?
 
@@ -173,21 +156,6 @@ class GroupPolicy < BasePolicy
   def lookup_access_level!
     @subject.max_member_access_for_user(@user)
   end
-
-  # TODO: Extract this into a helper shared with ProjectPolicy, once we implement group-level features.
-  # https://gitlab.com/gitlab-org/gitlab/-/issues/208412
-  def feature_available?(feature)
-    return false unless feature == :wiki
-
-    case @subject.wiki_access_level
-    when ProjectFeature::DISABLED
-      false
-    when ProjectFeature::PRIVATE
-      admin? || access_level >= ProjectFeature.required_minimum_access_level(feature)
-    else
-      true
-    end
-  end
 end
 
 GroupPolicy.prepend_if_ee('EE::GroupPolicy')

ee/app/models/ee/group.rb

@@ -359,6 +359,16 @@ module EE
       end
     end
 
+    def wiki_access_level
+      # TODO: Remove this method once we implement group-level features.
+      # https://gitlab.com/gitlab-org/gitlab/-/issues/208412
+      if ::Feature.enabled?(:group_wiki, self)
+        ::ProjectFeature::ENABLED
+      else
+        ::ProjectFeature::DISABLED
+      end
+    end
+
     private
 
     def custom_project_templates_group_allowed

ee/app/policies/ee/group_policy.rb

@@ -6,6 +6,8 @@ module EE
     extend ::Gitlab::Utils::Override
 
     prepended do
+      include CrudPolicyHelpers
+
       with_scope :subject
       condition(:ldap_synced) { @subject.ldap_synced? }
       condition(:epics_available) { @subject.feature_available?(:epics) }
@@ -67,6 +69,13 @@ module EE
         License.feature_available?(:cluster_health)
       end
 
+      rule { public_group | logged_in_viewable }.policy do
+        enable :read_wiki
+        enable :download_wiki_code
+      end
+
+      rule { guest }.enable :read_wiki
+
       rule { reporter }.policy do
         enable :admin_list
         enable :admin_board
@@ -74,11 +83,13 @@ module EE
         enable :view_productivity_analytics
         enable :view_type_of_work_charts
         enable :read_group_timelogs
+        enable :download_wiki_code
       end
 
       rule { maintainer }.policy do
         enable :create_jira_connect_subscription
         enable :maintainer_access
+        enable :admin_wiki
       end
 
       rule { owner }.policy do
@@ -152,6 +163,7 @@ module EE
       end
 
       rule { developer }.policy do
+        enable :create_wiki
         enable :admin_merge_request
       end
 
@@ -181,6 +193,14 @@ module EE
       rule { ~(admin | allow_to_manage_default_branch_protection) }.policy do
         prevent :update_default_branch_protection
       end
+
+      desc "Group has wiki disabled"
+      condition(:wiki_disabled, score: 32) { !feature_available?(:wiki) }
+
+      rule { wiki_disabled }.policy do
+        prevent(*create_read_update_admin_destroy(:wiki))
+        prevent(:download_wiki_code)
+      end
     end
 
     override :lookup_access_level!
@@ -190,6 +210,21 @@ module EE
       super
     end
 
+    # TODO: Extract this into a helper shared with ProjectPolicy, once we implement group-level features.
+    # https://gitlab.com/gitlab-org/gitlab/-/issues/208412
+    def feature_available?(feature)
+      return false unless feature == :wiki
+
+      case subject.wiki_access_level
+      when ::ProjectFeature::DISABLED
+        false
+      when ::ProjectFeature::PRIVATE
+        admin? || access_level >= ::ProjectFeature.required_minimum_access_level(feature)
+      else
+        true
+      end
+    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/spec/policies/group_policy_spec.rb

@@ -837,4 +837,27 @@ describe GroupPolicy do
       end
     end
   end
+
+  it_behaves_like 'model with wiki policies' do
+    let_it_be(:container) { create(:group) }
+    let_it_be(:user) { owner }
+
+    def set_access_level(access_level)
+      allow(container).to receive(:wiki_access_level).and_return(access_level)
+    end
+
+    before do
+      stub_feature_flags(group_wiki: true)
+    end
+
+    context 'when the feature flag is disabled' do
+      before do
+        stub_feature_flags(group_wiki: false)
+      end
+
+      it 'does not include the wiki permissions' do
+        expect_disallowed(*wiki_permissions[:all])
+      end
+    end
+  end
 end

spec/policies/group_policy_spec.rb

@@ -655,26 +655,4 @@ describe GroupPolicy do
       end
     end
   end
-
-  it_behaves_like 'model with wiki policies' do
-    let(:container) { create(:group) }
-
-    def set_access_level(access_level)
-      allow(container).to receive(:wiki_access_level).and_return(access_level)
-    end
-
-    before do
-      stub_feature_flags(group_wiki: true)
-    end
-
-    context 'when the feature flag is disabled' do
-      before do
-        stub_feature_flags(group_wiki: false)
-      end
-
-      it 'does not include the wiki permissions' do
-        expect_disallowed(*permissions)
-      end
-    end
-  end
 end

spec/policies/project_policy_spec.rb

@@ -124,6 +124,7 @@ describe ProjectPolicy do
 
   it_behaves_like 'model with wiki policies' do
     let(:container) { project }
+    let_it_be(:user) { owner }
 
     def set_access_level(access_level)
       project.project_feature.update_attribute(:wiki_access_level, access_level)

spec/support/shared_contexts/policies/group_policy_shared_context.rb

@@ -14,17 +14,16 @@ RSpec.shared_context 'GroupPolicy context' do
     %i[
       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 read_wiki
+      read_group_merge_requests
    ]
   end
   let(:read_group_permissions) { %i[read_label read_list read_milestone read_board] }
-  let(:reporter_permissions) { %i[admin_label read_container_image read_metrics_dashboard_annotation download_wiki_code] }
-  let(:developer_permissions) { %i[admin_milestone create_metrics_dashboard_annotation delete_metrics_dashboard_annotation update_metrics_dashboard_annotation create_wiki] }
+  let(:reporter_permissions) { %i[admin_label read_container_image read_metrics_dashboard_annotation] }
+  let(:developer_permissions) { %i[admin_milestone create_metrics_dashboard_annotation delete_metrics_dashboard_annotation update_metrics_dashboard_annotation] }
   let(:maintainer_permissions) do
     %i[
       create_projects
       read_cluster create_cluster update_cluster admin_cluster add_cluster
-      admin_wiki
     ]
   end
   let(:owner_permissions) do

spec/support/shared_examples/policies/wiki_policies_shared_examples.rb

 # frozen_string_literal: true
 
 RSpec.shared_examples 'model with wiki policies' do
-  let(:container) { raise NotImplementedError }
-  let(:permissions) { %i(read_wiki create_wiki update_wiki admin_wiki download_wiki_code) }
-
-  # TODO: Remove this helper once we implement group features
-  # https://gitlab.com/gitlab-org/gitlab/-/issues/208412
-  def set_access_level(access_level)
-    raise NotImplementedError
-  end
-
-  subject { described_class.new(owner, container) }
-
-  context 'when the feature is disabled' do
-    before do
-      set_access_level(ProjectFeature::DISABLED)
-    end
+  include ProjectHelpers
 
-    it 'does not include the wiki permissions' do
-      expect_disallowed(*permissions)
-    end
+  let(:container) { raise NotImplementedError }
+  let(:user) { raise NotImplementedError }
 
-    context 'when there is an external wiki' do
-      it 'does not include the wiki permissions' do
-        allow(container).to receive(:has_external_wiki?).and_return(true)
+  subject { described_class.new(user, container) }
 
-        expect_disallowed(*permissions)
-      end
+  let_it_be(:wiki_permissions) do
+    {}.tap do |permissions|
+      permissions[:guest] = %i[read_wiki]
+      permissions[:reporter] = permissions[:guest] + %i[download_wiki_code]
+      permissions[:developer] = permissions[:reporter] + %i[create_wiki]
+      permissions[:maintainer] = permissions[:developer] + %i[admin_wiki]
+      permissions[:all] = permissions[:maintainer]
     end
   end
 
-  describe 'read_wiki' do
-    subject { described_class.new(user, container) }
-
-    member_roles = %i[guest developer]
-    stranger_roles = %i[anonymous non_member]
-
-    user_roles = stranger_roles + member_roles
+  using RSpec::Parameterized::TableSyntax
+
+  where(:container_level, :access_level, :membership, :access) do
+    :public   | :enabled  | :admin      | :all
+    :public   | :enabled  | :maintainer | :maintainer
+    :public   | :enabled  | :developer  | :developer
+    :public   | :enabled  | :reporter   | :reporter
+    :public   | :enabled  | :guest      | :guest
+    :public   | :enabled  | :non_member | :guest
+    :public   | :enabled  | :anonymous  | :guest
+
+    :public   | :private  | :admin      | :all
+    :public   | :private  | :maintainer | :maintainer
+    :public   | :private  | :developer  | :developer
+    :public   | :private  | :reporter   | :reporter
+    :public   | :private  | :guest      | :guest
+    :public   | :private  | :non_member | nil
+    :public   | :private  | :anonymous  | nil
+
+    :public   | :disabled | :admin      | nil
+    :public   | :disabled | :maintainer | nil
+    :public   | :disabled | :developer  | nil
+    :public   | :disabled | :reporter   | nil
+    :public   | :disabled | :guest      | nil
+    :public   | :disabled | :non_member | nil
+    :public   | :disabled | :anonymous  | nil
+
+    :internal | :enabled  | :admin      | :all
+    :internal | :enabled  | :maintainer | :maintainer
+    :internal | :enabled  | :developer  | :developer
+    :internal | :enabled  | :reporter   | :reporter
+    :internal | :enabled  | :guest      | :guest
+    :internal | :enabled  | :non_member | :guest
+    :internal | :enabled  | :anonymous  | nil
+
+    :internal | :private  | :admin      | :all
+    :internal | :private  | :maintainer | :maintainer
+    :internal | :private  | :developer  | :developer
+    :internal | :private  | :reporter   | :reporter
+    :internal | :private  | :guest      | :guest
+    :internal | :private  | :non_member | nil
+    :internal | :private  | :anonymous  | nil
+
+    :internal | :disabled | :admin      | nil
+    :internal | :disabled | :maintainer | nil
+    :internal | :disabled | :developer  | nil
+    :internal | :disabled | :reporter   | nil
+    :internal | :disabled | :guest      | nil
+    :internal | :disabled | :non_member | nil
+    :internal | :disabled | :anonymous  | nil
+
+    :private  | :private  | :admin      | :all
+    :private  | :private  | :maintainer | :maintainer
+    :private  | :private  | :developer  | :developer
+    :private  | :private  | :reporter   | :reporter
+    :private  | :private  | :guest      | :guest
+    :private  | :private  | :non_member | nil
+    :private  | :private  | :anonymous  | nil
+
+    :private  | :disabled | :admin      | nil
+    :private  | :disabled | :maintainer | nil
+    :private  | :disabled | :developer  | nil
+    :private  | :disabled | :reporter   | nil
+    :private  | :disabled | :guest      | nil
+    :private  | :disabled | :non_member | nil
+    :private  | :disabled | :anonymous  | nil
+  end
 
-    # When a user is anonymous, their `current_user == nil`
-    let(:user) { create(:user) unless user_role == :anonymous }
+  with_them do
+    let(:user) { create_user_from_membership(container, membership) }
+    let(:allowed_permissions) { wiki_permissions[access].dup || [] }
+    let(:disallowed_permissions) { wiki_permissions[:all] - allowed_permissions }
 
     before do
-      container.visibility = container_visibility
-      set_access_level(wiki_access_level)
-      container.add_user(user, user_role) if member_roles.include?(user_role)
-    end
-
-    title = ->(container_visibility, wiki_access_level, user_role) do
-      [
-        "container is #{Gitlab::VisibilityLevel.level_name container_visibility}",
-        "wiki is #{ProjectFeature.str_from_access_level wiki_access_level}",
-        "user is #{user_role}"
-      ].join(', ')
-    end
-
-    describe 'Situations where :read_wiki is always false' do
-      where(case_names: title,
-            container_visibility: Gitlab::VisibilityLevel.options.values,
-            wiki_access_level: [ProjectFeature::DISABLED],
-            user_role: user_roles)
-
-      with_them do
-        it { is_expected.to be_disallowed(:read_wiki) }
-      end
-    end
-
-    describe 'Situations where :read_wiki is always true' do
-      where(case_names: title,
-            container_visibility: [Gitlab::VisibilityLevel::PUBLIC],
-            wiki_access_level: [ProjectFeature::ENABLED],
-            user_role: user_roles)
+      container.visibility = container_level.to_s
+      set_access_level(ProjectFeature.access_level_from_str(access_level.to_s))
 
-      with_them do
-        it { is_expected.to be_allowed(:read_wiki) }
+      if allowed_permissions.any? && [container_level, access_level, membership] != [:private, :private, :guest]
+        allowed_permissions << :download_wiki_code
       end
     end
 
-    describe 'Situations where :read_wiki requires membership' do
-      context 'the wiki is private, and the user is a member' do
-        where(case_names: title,
-              container_visibility: [Gitlab::VisibilityLevel::PUBLIC,
-                                     Gitlab::VisibilityLevel::INTERNAL],
-              wiki_access_level: [ProjectFeature::PRIVATE],
-              user_role: member_roles)
-
-        with_them do
-          it { is_expected.to be_allowed(:read_wiki) }
-        end
-      end
-
-      context 'the wiki is private, and the user is not member' do
-        where(case_names: title,
-              container_visibility: [Gitlab::VisibilityLevel::PUBLIC,
-                                     Gitlab::VisibilityLevel::INTERNAL],
-              wiki_access_level: [ProjectFeature::PRIVATE],
-              user_role: stranger_roles)
-
-        with_them do
-          it { is_expected.to be_disallowed(:read_wiki) }
-        end
-      end
-
-      context 'the wiki is enabled, and the user is a member' do
-        where(case_names: title,
-              container_visibility: [Gitlab::VisibilityLevel::PRIVATE],
-              wiki_access_level: [ProjectFeature::ENABLED],
-              user_role: member_roles)
-
-        with_them do
-          it { is_expected.to be_allowed(:read_wiki) }
-        end
-      end
-
-      context 'the wiki is enabled, and the user is not a member' do
-        where(case_names: title,
-              container_visibility: [Gitlab::VisibilityLevel::PRIVATE],
-              wiki_access_level: [ProjectFeature::ENABLED],
-              user_role: stranger_roles)
-
-        with_them do
-          it { is_expected.to be_disallowed(:read_wiki) }
-        end
-      end
+    it 'allows actions based on membership' do
+      expect_allowed(*allowed_permissions)
+      expect_disallowed(*disallowed_permissions)
     end
+  end
 
-    describe 'Situations where :read_wiki prohibits anonymous access' do
-      context 'the user is not anonymous' do
-        where(case_names: title,
-              container_visibility: [Gitlab::VisibilityLevel::INTERNAL],
-              wiki_access_level: [ProjectFeature::ENABLED, ProjectFeature::PUBLIC],
-              user_role: user_roles.reject { |u| u == :anonymous })
-
-        with_them do
-          it { is_expected.to be_allowed(:read_wiki) }
-        end
-      end
-
-      context 'the user is anonymous' do
-        where(case_names: title,
-              container_visibility: [Gitlab::VisibilityLevel::INTERNAL],
-              wiki_access_level: [ProjectFeature::ENABLED, ProjectFeature::PUBLIC],
-              user_role: %i[anonymous])
-
-        with_them do
-          it { is_expected.to be_disallowed(:read_wiki) }
-        end
-      end
-    end
+  # TODO: Remove this helper once we implement group features
+  # https://gitlab.com/gitlab-org/gitlab/-/issues/208412
+  def set_access_level(access_level)
+    raise NotImplementedError
   end
 end