From 7de89cfa0fb3bb6db239c4be487a0781ae80631d Mon Sep 17 00:00:00 2001 From: Connor Turland Date: Fri, 2 Sep 2016 12:37:01 -0400 Subject: [PATCH 1/3] shouldn't reference relatives that are connected in private ways --- app/controllers/topics_controller.rb | 4 ++-- app/models/topic.rb | 34 +++++++++++++++++++++------- 2 files changed, 28 insertions(+), 10 deletions(-) diff --git a/app/controllers/topics_controller.rb b/app/controllers/topics_controller.rb index 253f1ec1..b0fd4057 100644 --- a/app/controllers/topics_controller.rb +++ b/app/controllers/topics_controller.rb @@ -24,7 +24,7 @@ class TopicsController < ApplicationController respond_to do |format| format.html do - @alltopics = [@topic].concat(policy_scope(Topic.relatives1(@topic.id)).to_a).concat(policy_scope(Topic.relatives2(@topic.id)).to_a) + @alltopics = [@topic].concat(policy_scope(Topic.relatives1(@topic.id, current_user)).to_a).concat(policy_scope(Topic.relatives2(@topic.id, current_user)).to_a) @allsynapses = policy_scope(Synapse.for_topic(@topic.id)).to_a puts @alltopics.length puts @allsynapses.length @@ -42,7 +42,7 @@ class TopicsController < ApplicationController @topic = Topic.find(params[:id]) authorize @topic - @alltopics = [@topic].concat(policy_scope(Topic.relatives1(@topic.id)).to_a).concat(policy_scope(Topic.relatives2(@topic.id)).to_a) + @alltopics = [@topic].concat(policy_scope(Topic.relatives1(@topic.id, current_user)).to_a).concat(policy_scope(Topic.relatives2(@topic.id, current_user)).to_a) @allsynapses = policy_scope(Synapse.for_topic(@topic.id)) @allcreators = @alltopics.map(&:user).uniq diff --git a/app/models/topic.rb b/app/models/topic.rb index 4ff6ac50..4cb58f65 100644 --- a/app/models/topic.rb +++ b/app/models/topic.rb @@ -42,16 +42,34 @@ class Topic < ActiveRecord::Base topics1 + topics2 end - scope :relatives1, ->(topic_id = nil) { - includes(:topics1) - .where('synapses.node1_id = ?', topic_id) - .references(:synapses) + scope :relatives1, ->(topic_id = nil, user = nil) { + visible = %w(public commons) + permission = 'synapses.permission IN (?)' + if user + synapse_permission = permission + ' OR synapses.defer_to_map_id IN (?) OR synapses.user_id = ?' + return includes(:topics1) + .where('synapses.node1_id = ? AND (' + synapse_permission + ')', topic_id, visible, user.shared_maps.map(&:id), user.id) + .references(:synapses) + else + return includes(:topics1) + .where('synapses.node1_id = ? AND (' + permission + ')', topic_id, visible) + .references(:synapses) + end } - scope :relatives2, ->(topic_id = nil) { - includes(:topics2) - .where('synapses.node2_id = ?', topic_id) - .references(:synapses) + scope :relatives2, ->(topic_id = nil, user = nil) { + visible = %w(public commons) + permission = 'synapses.permission IN (?)' + if user + synapse_permission = permission + ' OR synapses.defer_to_map_id IN (?) OR synapses.user_id = ?' + return includes(:topics2) + .where('synapses.node2_id = ? AND (' + synapse_permission + ')', topic_id, visible, user.shared_maps.map(&:id), user.id) + .references(:synapses) + else + return includes(:topics2) + .where('synapses.node2_id = ? AND (' + permission + ')', topic_id, visible) + .references(:synapses) + end } delegate :name, to: :user, prefix: true From e761e1693c3818f6acab158693f612352ae2d547 Mon Sep 17 00:00:00 2001 From: Devin Howard Date: Mon, 5 Sep 2016 11:52:35 +0800 Subject: [PATCH 2/3] use Topic.relatives scope to get all relatives --- app/controllers/topics_controller.rb | 4 +--- app/models/topic.rb | 6 ++++++ 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/app/controllers/topics_controller.rb b/app/controllers/topics_controller.rb index b0fd4057..fa7eada8 100644 --- a/app/controllers/topics_controller.rb +++ b/app/controllers/topics_controller.rb @@ -24,10 +24,8 @@ class TopicsController < ApplicationController respond_to do |format| format.html do - @alltopics = [@topic].concat(policy_scope(Topic.relatives1(@topic.id, current_user)).to_a).concat(policy_scope(Topic.relatives2(@topic.id, current_user)).to_a) + @alltopics = [@topic].concat(policy_scope(Topic.relatives(@topic.id, current_user)).to_a) @allsynapses = policy_scope(Synapse.for_topic(@topic.id)).to_a - puts @alltopics.length - puts @allsynapses.length @allcreators = @alltopics.map(&:user).uniq @allcreators += @allsynapses.map(&:user).uniq diff --git a/app/models/topic.rb b/app/models/topic.rb index 4cb58f65..6ca214fe 100644 --- a/app/models/topic.rb +++ b/app/models/topic.rb @@ -42,6 +42,12 @@ class Topic < ActiveRecord::Base topics1 + topics2 end + scope :relatives, ->(topic_id = nil, user = nil) { + synapses = Pundit.policy_scope(user, Synapse.where(node1_id: topic_id)).pluck(:node2_id) + synapses += Pundit.policy_scope(user, Synapse.where(node2_id: topic_id)).pluck(:node1_id) + where(id: synapses.uniq) + } + scope :relatives1, ->(topic_id = nil, user = nil) { visible = %w(public commons) permission = 'synapses.permission IN (?)' From 3e38fba2153fe2c3576b8bb495752b68589a5f08 Mon Sep 17 00:00:00 2001 From: Devin Howard Date: Mon, 5 Sep 2016 11:55:19 +0800 Subject: [PATCH 3/3] remove relatives1 and relatives2 --- app/controllers/topics_controller.rb | 6 +++--- app/models/topic.rb | 32 ++-------------------------- 2 files changed, 5 insertions(+), 33 deletions(-) diff --git a/app/controllers/topics_controller.rb b/app/controllers/topics_controller.rb index fa7eada8..846c5469 100644 --- a/app/controllers/topics_controller.rb +++ b/app/controllers/topics_controller.rb @@ -40,7 +40,7 @@ class TopicsController < ApplicationController @topic = Topic.find(params[:id]) authorize @topic - @alltopics = [@topic].concat(policy_scope(Topic.relatives1(@topic.id, current_user)).to_a).concat(policy_scope(Topic.relatives2(@topic.id, current_user)).to_a) + @alltopics = [@topic].concat(policy_scope(Topic.relatives(@topic.id, current_user)).to_a) @allsynapses = policy_scope(Synapse.for_topic(@topic.id)) @allcreators = @alltopics.map(&:user).uniq @@ -64,7 +64,7 @@ class TopicsController < ApplicationController topicsAlreadyHas = params[:network] ? params[:network].split(',').map(&:to_i) : [] - @alltopics = policy_scope(Topic.relatives1(@topic.id)).to_a.concat(policy_scope(Topic.relatives2(@topic.id)).to_a).uniq + @alltopics = policy_scope(Topic.relatives(@topic.id, current_user)).to_a @alltopics.delete_if do |topic| !topicsAlreadyHas.index(topic.id).nil? end @@ -86,7 +86,7 @@ class TopicsController < ApplicationController topicsAlreadyHas = params[:network] ? params[:network].split(',').map(&:to_i) : [] - alltopics = policy_scope(Topic.relatives1(@topic.id)).to_a.concat(policy_scope(Topic.relatives2(@topic.id)).to_a).uniq + alltopics = policy_scope(Topic.relatives(@topic.id)).to_a alltopics.delete_if do |topic| !topicsAlreadyHas.index(topic.id.to_s).nil? end diff --git a/app/models/topic.rb b/app/models/topic.rb index 6ca214fe..a91c75fc 100644 --- a/app/models/topic.rb +++ b/app/models/topic.rb @@ -43,41 +43,13 @@ class Topic < ActiveRecord::Base end scope :relatives, ->(topic_id = nil, user = nil) { + # should only see topics through *visible* synapses + # e.g. Topic A (commons) -> synapse (private) -> Topic B (commons) must be filtered out synapses = Pundit.policy_scope(user, Synapse.where(node1_id: topic_id)).pluck(:node2_id) synapses += Pundit.policy_scope(user, Synapse.where(node2_id: topic_id)).pluck(:node1_id) where(id: synapses.uniq) } - scope :relatives1, ->(topic_id = nil, user = nil) { - visible = %w(public commons) - permission = 'synapses.permission IN (?)' - if user - synapse_permission = permission + ' OR synapses.defer_to_map_id IN (?) OR synapses.user_id = ?' - return includes(:topics1) - .where('synapses.node1_id = ? AND (' + synapse_permission + ')', topic_id, visible, user.shared_maps.map(&:id), user.id) - .references(:synapses) - else - return includes(:topics1) - .where('synapses.node1_id = ? AND (' + permission + ')', topic_id, visible) - .references(:synapses) - end - } - - scope :relatives2, ->(topic_id = nil, user = nil) { - visible = %w(public commons) - permission = 'synapses.permission IN (?)' - if user - synapse_permission = permission + ' OR synapses.defer_to_map_id IN (?) OR synapses.user_id = ?' - return includes(:topics2) - .where('synapses.node2_id = ? AND (' + synapse_permission + ')', topic_id, visible, user.shared_maps.map(&:id), user.id) - .references(:synapses) - else - return includes(:topics2) - .where('synapses.node2_id = ? AND (' + permission + ')', topic_id, visible) - .references(:synapses) - end - } - delegate :name, to: :user, prefix: true def user_image