From c2dd4239c939e003dfe569196ec2d39e2478606e Mon Sep 17 00:00:00 2001 From: "http://jneen.net/" Date: Tue, 1 Aug 2017 10:42:54 -0700 Subject: [PATCH] short-circuit if there is no policy, and add :read_project check --- app/models/notification_recipient.rb | 9 ++++++--- app/services/notification_recipient_service.rb | 6 +----- lib/declarative_policy.rb | 13 +++++++++---- 3 files changed, 16 insertions(+), 12 deletions(-) diff --git a/app/models/notification_recipient.rb b/app/models/notification_recipient.rb index 28ac22da6e2..837b62ec0cb 100644 --- a/app/models/notification_recipient.rb +++ b/app/models/notification_recipient.rb @@ -76,10 +76,13 @@ class NotificationRecipient end def has_access? - return false unless user.can?(:receive_notifications) - return true unless @read_ability - DeclarativePolicy.subject_scope do + return false unless user.can?(:receive_notifications) + return false if @project && !user.can?(:read_project, @project) + + return true unless @read_ability + return true unless DeclarativePolicy.has_policy?(@target) + user.can?(@read_ability, @target) end end diff --git a/app/services/notification_recipient_service.rb b/app/services/notification_recipient_service.rb index 1ce92c8cbdc..540e568fed2 100644 --- a/app/services/notification_recipient_service.rb +++ b/app/services/notification_recipient_service.rb @@ -308,11 +308,7 @@ module NotificationRecipientService end def read_ability - @read_ability ||= - case target - when Commit then nil - else :"read_#{target.class.model_name.name.underscore}" - end + @read_ability ||= :"read_#{target.class.model_name.name.underscore}" end def subject diff --git a/lib/declarative_policy.rb b/lib/declarative_policy.rb index b1eb1a6cef1..4936669a73a 100644 --- a/lib/declarative_policy.rb +++ b/lib/declarative_policy.rb @@ -28,7 +28,12 @@ module DeclarativePolicy subject = find_delegate(subject) - class_for_class(subject.class) + class_for_class(subject.class) \ + or raise "no policy for #{subject.class.name}" + end + + def has_policy?(subject) + !class_for_class(subject.class).nil? end private @@ -51,9 +56,7 @@ module DeclarativePolicy end end - policy_class = subject_class.instance_variable_get(CLASS_CACHE_IVAR) - raise "no policy for #{subject.class.name}" if policy_class.nil? - policy_class + subject_class.instance_variable_get(CLASS_CACHE_IVAR) end def compute_class_for_class(subject_class) @@ -71,6 +74,8 @@ module DeclarativePolicy nil end end + + nil end def find_delegate(subject)