From c0955d7c5ea9592ecfac6b624bc6ca6a518617ba Mon Sep 17 00:00:00 2001 From: Connor Turland Date: Mon, 17 Oct 2016 01:20:48 -0400 Subject: [PATCH] multiple policy issues (#771) * multiple policy errors * make some things more explicit --- app/models/user.rb | 5 +++++ app/policies/mapping_policy.rb | 12 +++++++----- app/policies/message_policy.rb | 12 +++++++----- app/policies/synapse_policy.rb | 3 +-- app/policies/topic_policy.rb | 2 +- 5 files changed, 21 insertions(+), 13 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index 4f679c1b..52d6ef09 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -65,6 +65,11 @@ class User < ApplicationRecord json end + def all_accessible_maps + #TODO: is there a way to keep this an ActiveRecord relation? + maps + shared_maps + end + def recentMetacodes array = [] self.topics.sort{|a,b| b.created_at <=> a.created_at }.each do |t| diff --git a/app/policies/mapping_policy.rb b/app/policies/mapping_policy.rb index efcb798b..6cdb7e9b 100644 --- a/app/policies/mapping_policy.rb +++ b/app/policies/mapping_policy.rb @@ -8,11 +8,13 @@ class MappingPolicy < ApplicationPolicy # a private topic, since you can't see the private topic anyways visible = %w(public commons) permission = 'maps.permission IN (?)' - if user - scope.joins(:map).where(permission, visible).or(scope.joins(:map).where(user_id: user.id)) - else - scope.joins(:map).where(permission, visible) - end + return scope.joins(:map).where(permission, visible) unless user + + # if this is getting changed, the policy_scope for messages should also be changed + # as it is based entirely on the map to which it belongs + scope.joins(:map).where(permission, visible) + .or(scope.joins(:map).where('maps.id IN (?)', user.shared_maps.map(&:id))) + .or(scope.joins(:map).where('maps.user_id = ?', user.id)) end end diff --git a/app/policies/message_policy.rb b/app/policies/message_policy.rb index f35a2895..c32e29ed 100644 --- a/app/policies/message_policy.rb +++ b/app/policies/message_policy.rb @@ -4,11 +4,13 @@ class MessagePolicy < ApplicationPolicy def resolve visible = %w(public commons) permission = 'maps.permission IN (?)' - if user - scope.joins(:maps).where(permission + ' OR maps.user_id = ?', visible, user.id) - else - scope.where(permission, visible) - end + return scope.joins(:map).where(permission, visible) unless user + + # if this is getting changed, the policy_scope for mappings should also be changed + # as it is based entirely on the map to which it belongs + scope.joins(:map).where(permission, visible) + .or(scope.joins(:map).where('maps.id IN (?)', user.shared_maps.map(&:id))) + .or(scope.joins(:map).where('maps.user_id = ?', user.id)) end end diff --git a/app/policies/synapse_policy.rb b/app/policies/synapse_policy.rb index eae820b3..f3d2c997 100644 --- a/app/policies/synapse_policy.rb +++ b/app/policies/synapse_policy.rb @@ -3,11 +3,10 @@ class SynapsePolicy < ApplicationPolicy class Scope < Scope def resolve visible = %w(public commons) - return scope.where(permission: visible) unless user scope.where(permission: visible) - .or(scope.where(defer_to_map_id: user.shared_maps.map(&:id))) + .or(scope.where.not(defer_to_map_id: nil).where(defer_to_map_id: user.all_accessible_maps.map(&:id))) .or(scope.where(user_id: user.id)) end end diff --git a/app/policies/topic_policy.rb b/app/policies/topic_policy.rb index 7bcf585c..cf091662 100644 --- a/app/policies/topic_policy.rb +++ b/app/policies/topic_policy.rb @@ -6,7 +6,7 @@ class TopicPolicy < ApplicationPolicy return scope.where(permission: visible) unless user scope.where(permission: visible) - .or(scope.where(defer_to_map_id: user.shared_maps.map(&:id))) + .or(scope.where.not(defer_to_map_id: nil).where(defer_to_map_id: user.all_accessible_maps.map(&:id))) .or(scope.where(user_id: user.id)) end end