From bb03b49d800c7d4b424fdd9b9459d2210326b824 Mon Sep 17 00:00:00 2001 From: Devin Howard Date: Sat, 12 Mar 2016 09:09:41 +0800 Subject: [PATCH 1/2] update main controller (searching) to use policies --- app/controllers/main_controller.rb | 112 +++++++++---------------- app/controllers/mappings_controller.rb | 1 + 2 files changed, 39 insertions(+), 74 deletions(-) diff --git a/app/controllers/main_controller.rb b/app/controllers/main_controller.rb index efebdce7..363c9c94 100644 --- a/app/controllers/main_controller.rb +++ b/app/controllers/main_controller.rb @@ -4,14 +4,13 @@ class MainController < ApplicationController include UsersHelper include SynapsesHelper -# after_action :verify_authorized, except: :index -# after_action :verify_policy_scoped, only: :index + after_action :verify_policy_scoped respond_to :html, :json # home page def home - @maps = Map.where("maps.permission != ?", "private").order("updated_at DESC").page(1).per(20) + @maps = policy_scope(Map).order("updated_at DESC").page(1).per(20) respond_to do |format| format.html { if authenticated? @@ -60,69 +59,35 @@ class MainController < ApplicationController filterByMetacode = m end end + + search = '%' + term.downcase + '%' + builder = policy_scope(Topic) if filterByMetacode if term == "" - @topics = [] + builder = builder.none else - search = term.downcase + '%' - - if user - @topics = Set.new(Topic.where('LOWER("name") like ?', search).where('metacode_id = ? AND user_id = ?', filterByMetacode.id, user).order('"name"')) - @topics2 = Set.new(Topic.where('LOWER("name") like ?', '%' + search).where('metacode_id = ? AND user_id = ?', filterByMetacode.id, user).order('"name"')) - @topics3 = Set.new(Topic.where('LOWER("desc") like ?', '%' + search).where('metacode_id = ? AND user_id = ?', filterByMetacode.id, user).order('"name"')) - @topics4 = Set.new(Topic.where('LOWER("link") like ?', '%' + search).where('metacode_id = ? AND user_id = ?', filterByMetacode.id, user).order('"name"')) - else - @topics = Set.new(Topic.where('LOWER("name") like ?', search).where('metacode_id = ?', filterByMetacode.id).order('"name"')) - @topics2 = Set.new(Topic.where('LOWER("name") like ?', '%' + search).where('metacode_id = ?', filterByMetacode.id).order('"name"')) - @topics3 = Set.new(Topic.where('LOWER("desc") like ?', '%' + search).where('metacode_id = ?', filterByMetacode.id).order('"name"')) - @topics4 = Set.new(Topic.where('LOWER("link") like ?', '%' + search).where('metacode_id = ?', filterByMetacode.id).order('"name"')) - end - - #get unique elements only through the magic of Sets - @topics = (@topics + @topics2 + @topics3 + @topics4).to_a + builder = builder.where('LOWER("name") like ? OR + LOWER("desc") like ? OR + LOWER("link") like ?', search, search, search) + builder = builder.where(metacode_id: filterByMetacode.id) end elsif desc - search = '%' + term.downcase + '%' - if !user - @topics = Topic.where('LOWER("desc") like ?', search).order('"name"') - elsif user - @topics = Topic.where('LOWER("desc") like ?', search).where('user_id = ?', user).order('"name"') - end + builder = builder.where('LOWER("desc") like ?', search) elsif link - search = '%' + term.downcase + '%' - if !user - @topics = Topic.where('LOWER("link") like ?', search).order('"name"') - elsif user - @topics = Topic.where('LOWER("link") like ?', search).where('user_id = ?', user).order('"name"') - end + builder = builder.where('LOWER("link") like ?', search) else #regular case, just search the name - search = term.downcase + '%' - if !user - @topics = Topic.where('LOWER("name") like ?', search).order('"name"') - @topics2 = Topic.where('LOWER("name") like ?', '%' + search).order('"name"') - @topics3 = Topic.where('LOWER("desc") like ?', '%' + search).order('"name"') - @topics4 = Topic.where('LOWER("link") like ?', '%' + search).order('"name"') - @topics = @topics + (@topics2 - @topics) - @topics = @topics + (@topics3 - @topics) - @topics = @topics + (@topics4 - @topics) - elsif user - @topics = Topic.where('LOWER("name") like ?', search).where('user_id = ?', user).order('"name"') - @topics2 = Topic.where('LOWER("name") like ?', '%' + search).where('user_id = ?', user).order('"name"') - @topics3 = Topic.where('LOWER("desc") like ?', '%' + search).where('user_id = ?', user).order('"name"') - @topics4 = Topic.where('LOWER("link") like ?', '%' + search).where('user_id = ?', user).order('"name"') - @topics = @topics + (@topics2 - @topics) - @topics = @topics + (@topics3 - @topics) - @topics = @topics + (@topics4 - @topics) - end + builder = builder.where('LOWER("name") like ? OR + LOWER("desc") like ? OR + LOWER("link") like ?', search, search, search) end + + builder = builder.where(user: user) if user + @topics = builder.order(:name) else @topics = [] end - - #read this next line as 'delete a topic if its private and you're either 1. logged out or 2. logged in but not the topic creator - @topics.to_a.delete_if {|t| t.permission == "private" && (!authenticated? || (authenticated? && current_user.id != t.user_id)) } - + render json: autocomplete_array_json(@topics) end @@ -142,21 +107,21 @@ class MainController < ApplicationController term = term[5..-1] desc = true end + search = '%' + term.downcase + '%' - query = desc ? 'LOWER("desc") like ?' : 'LOWER("name") like ?' - if !user - # !connor why is the limit 5 done here and not above? also, why not limit after sorting alphabetically? - @maps = Map.where(query, search).limit(5).order('"name"') - elsif user - @maps = Map.where(query, search).where('user_id = ?', user).order('"name"') + builder = policy_scope(Map) + + if desc + builder = builder.where('LOWER("desc") like ?', search) + else + builder = builder.where('LOWER("name") like ?', search) end + builder = builder.where(user: user) if user + @maps = builder.order(:name) else @maps = [] end - #read this next line as 'delete a map if its private and you're either 1. logged out or 2. logged in but not the map creator - @maps.to_a.delete_if {|m| m.permission == "private" && (!authenticated? || (authenticated? && current_user.id != m.user_id)) } - render json: autocomplete_map_array_json(@maps) end @@ -167,7 +132,10 @@ class MainController < ApplicationController #remove "mapper:" if appended at beginning term = term[7..-1] if term.downcase[0..6] == "mapper:" - @mappers = User.where('LOWER("name") like ?', term.downcase + '%').order('"name"') + search = term.downcase + '%' + builder = policy_scope(User) # TODO do I need to policy scope? I guess yes to verify_policy_scoped + builder = builder.where('LOWER("name") like ?', search) + @mappers = builder.order(:name) else @mappers = [] end @@ -182,7 +150,7 @@ class MainController < ApplicationController topic2id = params[:topic2id] if term && !term.empty? - @synapses = Synapse.where('LOWER("desc") like ?', '%' + term.downcase + '%').order('"desc"') + @synapses = policy_scope(Synapse).where('LOWER("desc") like ?', '%' + term.downcase + '%').order('"desc"') # remove any duplicate synapse types that just differ by # leading or trailing whitespaces @@ -196,22 +164,18 @@ class MainController < ApplicationController boolean = true end } - - #limit to 5 results - @synapses = @synapses.slice(0,5) elsif topic1id && !topic1id.empty? - @one = Synapse.where('node1_id = ? AND node2_id = ?', topic1id, topic2id) - @two = Synapse.where('node2_id = ? AND node1_id = ?', topic1id, topic2id) + @one = policy_scope(Synapse).where('node1_id = ? AND node2_id = ?', topic1id, topic2id) + @two = policy_scope(Synapse).where('node2_id = ? AND node1_id = ?', topic1id, topic2id) @synapses = @one + @two @synapses.sort! {|s1,s2| s1.desc <=> s2.desc }.to_a - - #permissions - @synapses.delete_if {|s| s.permission == "private" && !authenticated? } - @synapses.delete_if {|s| s.permission == "private" && authenticated? && current_user.id != s.user_id } else @synapses = [] end + #limit to 5 results + @synapses = @synapses.slice(0,5) + render json: autocomplete_synapse_array_json(@synapses) end end diff --git a/app/controllers/mappings_controller.rb b/app/controllers/mappings_controller.rb index cba1cb3f..02c28a54 100644 --- a/app/controllers/mappings_controller.rb +++ b/app/controllers/mappings_controller.rb @@ -18,6 +18,7 @@ class MappingsController < ApplicationController @mapping = Mapping.new(mapping_params) authorize @mapping @mapping.user = current_user + if @mapping.save render json: @mapping, status: :created else From 7e7ef173e501301d659105af1e47081d98ff0cbe Mon Sep 17 00:00:00 2001 From: Devin Howard Date: Sat, 12 Mar 2016 09:27:31 +0800 Subject: [PATCH 2/2] map policy spec --- spec/policies/map_policy_spec.rb | 97 ++++++++++++++++++++++++++++++++ spec/spec_helper.rb | 1 + 2 files changed, 98 insertions(+) create mode 100644 spec/policies/map_policy_spec.rb diff --git a/spec/policies/map_policy_spec.rb b/spec/policies/map_policy_spec.rb new file mode 100644 index 00000000..b160fead --- /dev/null +++ b/spec/policies/map_policy_spec.rb @@ -0,0 +1,97 @@ +require 'rails_helper' + +RSpec.describe MapPolicy, type: :policy do + subject { described_class } + + context 'unauthenticated' do + context 'commons' do + let(:map) { create(:map, permission: :commons) } + permissions :show? do + it 'can view' do + expect(subject).to permit(nil, map) + end + end + permissions :create?, :update?, :destroy? do + it 'can not modify' do + expect(subject).to_not permit(nil, map) + end + end + end + + context 'private' do + let(:map) { create(:map, permission: :private) } + permissions :show?, :create?, :update?, :destroy? do + it 'can not view or modify' do + expect(subject).to_not permit(nil, map) + end + end + end + end + + # + # Now begin the logged-in tests + # + + context 'logged in' do + let(:user) { create(:user) } + + context 'commons' do + let(:owner) { create(:user) } + let(:map) { create(:map, permission: :commons, user: owner) } + permissions :show?, :create?, :update? do + it 'can view and modify' do + expect(subject).to permit(user, map) + end + end + permissions :destroy? do + it 'can not destroy' do + expect(subject).to_not permit(user, map) + end + it 'owner can destroy' do + expect(subject).to permit(owner, map) + end + end + end + + context 'public' do + let(:owner) { create(:user) } + let(:map) { create(:map, permission: :public, user: owner) } + permissions :show? do + it 'can view' do + expect(subject).to permit(user, map) + end + end + permissions :create? do + it 'can create' do + expect(subject).to permit(user, map) + end + end + permissions :update?, :destroy? do + it 'can not update/destroy' do + expect(subject).to_not permit(user, map) + end + it 'owner can update/destroy' do + expect(subject).to permit(owner, map) + end + end + end + + context 'private' do + let(:owner) { create(:user) } + let(:map) { create(:map, permission: :private, user: owner) } + permissions :create? do + it 'can create' do + expect(subject).to permit(user, map) + end + end + permissions :show?, :update?, :destroy? do + it 'can not view or modify' do + expect(subject).to_not permit(user, map) + end + it 'owner can view and modify' do + expect(subject).to permit(owner, map) + end + end + end + end +end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 08ffd17f..d4028602 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -1,5 +1,6 @@ require 'simplecov' require 'support/controller_helpers' +require 'pundit/rspec' RSpec.configure do |config| config.expect_with :rspec do |expectations|