Gotcha with guard clause in overridden methods

The proper way of handling anonymous rules should be by overriding #anonymous_rules in BasePolicy subclasses. The default for #anonymous_rules is #rules.

As mentioned in https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/822#note_19898741, an early return does not return from the "overrider" method (be it via inheritance or via prepend).

For instance:

# app/policies/group_policy.rb
class GroupMemberPolicy < BasePolicy
  def rules
    return unless @user

    target_user = @subject.user
    group = @subject.group

    return if group.last_owner?(target_user)

    can_manage = Ability.allowed?(@user, :admin_group_member, group)

    if can_manage
      can! :update_group_member
      can! :destroy_group_member
    elsif @user == target_user
      can! :destroy_group_member
    end
  end
end

# app/policies/ee/group_policy.rb
module EE
  module GroupPolicy
    def rules
      raise NotImplementedError unless defined?(super)

      super

      owner = @user.admin? || @subject.has_owner?(@user)
      master = owner || @subject.has_master?(@user)

      if @subject.ldap_synced?
        cannot! :admin_group_member
        can! :override_group_member if master
      end
    end
  end
end

=> NoMethodError - undefined method `admin?' for nil:NilClass:
   app/policies/ee/group_policy.rb:8:in `rules

So there's two thing here:

  1. Audit our current usage of prepend and make sure we don't have code that don't behave as we'd expect

  2. When guard clauses are used in an overridden method, another strategy (documented by https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/963/diffs) should be used:

    create a "hook" method that is empty in CE, and with the EE-specific implementation in EE

Edited Jun 23, 2025 by 🤖 GitLab Bot 🤖
Assignee Loading
Time tracking Loading