From dbb8052a17650744e2ef75ef1367ac7d3be51f2d Mon Sep 17 00:00:00 2001 From: Devin Howard Date: Mon, 14 Mar 2016 11:00:54 +0800 Subject: [PATCH 1/4] trifecta of policy tests --- spec/policies/map_policy_spec.rb | 31 +++++----- spec/policies/synapse_policy.rb | 92 ++++++++++++++++++++++++++++++ spec/policies/topic_policy_spec.rb | 92 ++++++++++++++++++++++++++++++ 3 files changed, 197 insertions(+), 18 deletions(-) create mode 100644 spec/policies/synapse_policy.rb create mode 100644 spec/policies/topic_policy_spec.rb diff --git a/spec/policies/map_policy_spec.rb b/spec/policies/map_policy_spec.rb index b160fead..7dd33707 100644 --- a/spec/policies/map_policy_spec.rb +++ b/spec/policies/map_policy_spec.rb @@ -7,12 +7,12 @@ RSpec.describe MapPolicy, type: :policy do context 'commons' do let(:map) { create(:map, permission: :commons) } permissions :show? do - it 'can view' do + it 'permits access' do expect(subject).to permit(nil, map) end end permissions :create?, :update?, :destroy? do - it 'can not modify' do + it 'denies access' do expect(subject).to_not permit(nil, map) end end @@ -21,7 +21,7 @@ RSpec.describe MapPolicy, type: :policy do context 'private' do let(:map) { create(:map, permission: :private) } permissions :show?, :create?, :update?, :destroy? do - it 'can not view or modify' do + it 'permits access' do expect(subject).to_not permit(nil, map) end end @@ -39,15 +39,15 @@ RSpec.describe MapPolicy, type: :policy do let(:owner) { create(:user) } let(:map) { create(:map, permission: :commons, user: owner) } permissions :show?, :create?, :update? do - it 'can view and modify' do + it 'permits access' do expect(subject).to permit(user, map) end end permissions :destroy? do - it 'can not destroy' do + it 'denies access' do expect(subject).to_not permit(user, map) end - it 'owner can destroy' do + it 'permits access to owner' do expect(subject).to permit(owner, map) end end @@ -56,21 +56,16 @@ RSpec.describe MapPolicy, type: :policy do 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 + permissions :show?, :create? do + it 'permits access' do expect(subject).to permit(user, map) end end permissions :update?, :destroy? do - it 'can not update/destroy' do + it 'denies access' do expect(subject).to_not permit(user, map) end - it 'owner can update/destroy' do + it 'permits access to owner' do expect(subject).to permit(owner, map) end end @@ -80,15 +75,15 @@ RSpec.describe MapPolicy, type: :policy do let(:owner) { create(:user) } let(:map) { create(:map, permission: :private, user: owner) } permissions :create? do - it 'can create' do + it 'permits access' do expect(subject).to permit(user, map) end end permissions :show?, :update?, :destroy? do - it 'can not view or modify' do + it 'denies access' do expect(subject).to_not permit(user, map) end - it 'owner can view and modify' do + it 'permits access to owner' do expect(subject).to permit(owner, map) end end diff --git a/spec/policies/synapse_policy.rb b/spec/policies/synapse_policy.rb new file mode 100644 index 00000000..4c725e37 --- /dev/null +++ b/spec/policies/synapse_policy.rb @@ -0,0 +1,92 @@ +require 'rails_helper' + +RSpec.describe SynapsePolicy, type: :policy do + subject { described_class } + + context 'unauthenticated' do + context 'commons' do + let(:synapse) { create(:synapse, permission: :commons) } + permissions :show? do + it 'permits access' do + expect(subject).to permit(nil, synapse) + end + end + permissions :create?, :update?, :destroy? do + it 'denies access' do + expect(subject).to_not permit(nil, synapse) + end + end + end + + context 'private' do + let(:synapse) { create(:synapse, permission: :private) } + permissions :show?, :create?, :update?, :destroy? do + it 'denies access' do + expect(subject).to_not permit(nil, synapse) + 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(:synapse) { create(:synapse, permission: :commons, user: owner) } + permissions :show?, :create?, :update? do + it 'permits access' do + expect(subject).to permit(user, synapse) + end + end + permissions :destroy? do + it 'denies access' do + expect(subject).to_not permit(user, synapse) + end + it 'permits access to owner' do + expect(subject).to permit(owner, synapse) + end + end + end + + context 'public' do + let(:owner) { create(:user) } + let(:synapse) { create(:synapse, permission: :public, user: owner) } + permissions :show?, :create? do + it 'permits access' do + expect(subject).to permit(user, synapse) + end + end + permissions :update?, :destroy? do + it 'denies access' do + expect(subject).to_not permit(user, synapse) + end + it 'permits access to owner' do + expect(subject).to permit(owner, synapse) + end + end + end + + context 'private' do + let(:owner) { create(:user) } + let(:synapse) { create(:synapse, permission: :private, user: owner) } + permissions :create? do + it 'permits access' do + expect(subject).to permit(user, synapse) + end + end + permissions :show?, :update?, :destroy? do + it 'denies access' do + expect(subject).to_not permit(user, synapse) + end + it 'permits access to owner' do + expect(subject).to permit(owner, synapse) + end + end + end + end +end diff --git a/spec/policies/topic_policy_spec.rb b/spec/policies/topic_policy_spec.rb new file mode 100644 index 00000000..7078496c --- /dev/null +++ b/spec/policies/topic_policy_spec.rb @@ -0,0 +1,92 @@ +require 'rails_helper' + +RSpec.describe TopicPolicy, type: :policy do + subject { described_class } + + context 'unauthenticated' do + context 'commons' do + let(:topic) { create(:topic, permission: :commons) } + permissions :show? do + it 'permits access' do + expect(subject).to permit(nil, topic) + end + end + permissions :create?, :update?, :destroy? do + it 'denies access' do + expect(subject).to_not permit(nil, topic) + end + end + end + + context 'private' do + let(:topic) { create(:topic, permission: :private) } + permissions :show?, :create?, :update?, :destroy? do + it 'denies access' do + expect(subject).to_not permit(nil, topic) + 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(:topic) { create(:topic, permission: :commons, user: owner) } + permissions :show?, :create?, :update? do + it 'permits access' do + expect(subject).to permit(user, topic) + end + end + permissions :destroy? do + it 'denies access' do + expect(subject).to_not permit(user, topic) + end + it 'permits access to owner' do + expect(subject).to permit(owner, topic) + end + end + end + + context 'public' do + let(:owner) { create(:user) } + let(:topic) { create(:topic, permission: :public, user: owner) } + permissions :show?, :create? do + it 'permits access' do + expect(subject).to permit(user, topic) + end + end + permissions :update?, :destroy? do + it 'denies access' do + expect(subject).to_not permit(user, topic) + end + it 'permits access to owner' do + expect(subject).to permit(owner, topic) + end + end + end + + context 'private' do + let(:owner) { create(:user) } + let(:topic) { create(:topic, permission: :private, user: owner) } + permissions :create? do + it 'permits access' do + expect(subject).to permit(user, topic) + end + end + permissions :show?, :update?, :destroy? do + it 'denies access' do + expect(subject).to_not permit(user, topic) + end + it 'permits access to owner' do + expect(subject).to permit(owner, topic) + end + end + end + end +end From c5009952f379aef3220a858d311e5bbfa03615ab Mon Sep 17 00:00:00 2001 From: Devin Howard Date: Mon, 14 Mar 2016 11:03:30 +0800 Subject: [PATCH 2/4] remove permissions tests --- spec/models/map_spec.rb | 29 ----------------------------- spec/models/synapse_spec.rb | 29 ----------------------------- spec/models/topic_spec.rb | 29 ----------------------------- 3 files changed, 87 deletions(-) diff --git a/spec/models/map_spec.rb b/spec/models/map_spec.rb index d1486192..27227188 100644 --- a/spec/models/map_spec.rb +++ b/spec/models/map_spec.rb @@ -5,34 +5,5 @@ RSpec.describe Map, type: :model do it { is_expected.to validate_presence_of :name } it { is_expected.to validate_presence_of :permission } it { is_expected.to validate_inclusion_of(:permission).in_array Perm::ISSIONS.map(&:to_s) } - - context 'permissions' do - let(:owner) { create :user } - let(:other_user) { create :user } - let(:map) { create :map, user: owner, permission: :commons } - let(:private_map) { create :map, user: owner, permission: :private } - let(:public_map) { create :map, user: owner, permission: :public } - - it 'prevents deletion by non-owner' do - expect(map.authorize_to_delete(other_user)).to eq false - expect(map.authorize_to_delete(owner)).to eq map - end - - it 'prevents visibility if private' do - expect(map.authorize_to_show(other_user)).to eq map - expect(map.authorize_to_show(owner)).to eq map - expect(private_map.authorize_to_show(owner)).to eq private_map - expect(private_map.authorize_to_show(other_user)).to eq false - end - - it 'only allows editing if commons or owned' do - expect(map.authorize_to_edit(other_user)).to eq map - expect(map.authorize_to_edit(owner)).to eq map - expect(private_map.authorize_to_edit(other_user)).to eq false - expect(private_map.authorize_to_edit(owner)).to eq private_map - expect(public_map.authorize_to_edit(other_user)).to eq false - expect(public_map.authorize_to_edit(owner)).to eq public_map - end - end end diff --git a/spec/models/synapse_spec.rb b/spec/models/synapse_spec.rb index dcf85358..6ba5ff22 100644 --- a/spec/models/synapse_spec.rb +++ b/spec/models/synapse_spec.rb @@ -10,33 +10,4 @@ RSpec.describe Synapse, type: :model do it { is_expected.to validate_inclusion_of(:permission).in_array Perm::ISSIONS.map(&:to_s) } it { is_expected.to validate_inclusion_of(:category).in_array ['from-to', 'both'] } it { is_expected.to validate_length_of(:desc).is_at_least(0) } # TODO don't allow nil - - context 'permissions' do - let(:owner) { create :user } - let(:other_user) { create :user } - let(:synapse) { create :synapse, user: owner, permission: :commons } - let(:private_synapse) { create :synapse, user: owner, permission: :private } - let(:public_synapse) { create :synapse, user: owner, permission: :public } - - it 'prevents deletion by non-owner' do - expect(synapse.authorize_to_delete(other_user)).to eq false - expect(synapse.authorize_to_delete(owner)).to eq synapse - end - - it 'prevents visibility if private' do - expect(synapse.authorize_to_show(other_user)).to eq synapse - expect(synapse.authorize_to_show(owner)).to eq synapse - expect(private_synapse.authorize_to_show(owner)).to eq private_synapse - expect(private_synapse.authorize_to_show(other_user)).to eq false - end - - it 'only allows editing if commons or owned' do - expect(synapse.authorize_to_edit(other_user)).to eq synapse - expect(synapse.authorize_to_edit(owner)).to eq synapse - expect(private_synapse.authorize_to_edit(other_user)).to eq false - expect(private_synapse.authorize_to_edit(owner)).to eq private_synapse - expect(public_synapse.authorize_to_edit(other_user)).to eq false - expect(public_synapse.authorize_to_edit(owner)).to eq public_synapse - end - end end diff --git a/spec/models/topic_spec.rb b/spec/models/topic_spec.rb index b499daac..ef8f0b7a 100644 --- a/spec/models/topic_spec.rb +++ b/spec/models/topic_spec.rb @@ -49,33 +49,4 @@ RSpec.describe Topic, type: :model do end end end - - context 'permssions' do - let(:owner) { create :user } - let(:other_user) { create :user } - let(:topic) { create :topic, user: owner, permission: :commons } - let(:private_topic) { create :topic, user: owner, permission: :private } - let(:public_topic) { create :topic, user: owner, permission: :public } - - it 'prevents deletion by non-owner' do - expect(topic.authorize_to_delete(other_user)).to eq false - expect(topic.authorize_to_delete(owner)).to eq topic - end - - it 'prevents visibility if private' do - expect(topic.authorize_to_show(other_user)).to eq topic - expect(topic.authorize_to_show(owner)).to eq topic - expect(private_topic.authorize_to_show(owner)).to eq private_topic - expect(private_topic.authorize_to_show(other_user)).to eq false - end - - it 'only allows editing if commons or owned' do - expect(topic.authorize_to_edit(other_user)).to eq topic - expect(topic.authorize_to_edit(owner)).to eq topic - expect(private_topic.authorize_to_edit(other_user)).to eq false - expect(private_topic.authorize_to_edit(owner)).to eq private_topic - expect(public_topic.authorize_to_edit(other_user)).to eq false - expect(public_topic.authorize_to_edit(owner)).to eq public_topic - end - end end From 3823c708fd0f65edba3b9f78801d676a3e3c4258 Mon Sep 17 00:00:00 2001 From: Devin Howard Date: Mon, 14 Mar 2016 11:09:27 +0800 Subject: [PATCH 3/4] update mapping policy --- app/policies/mapping_policy.rb | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/app/policies/mapping_policy.rb b/app/policies/mapping_policy.rb index ed93bc66..40b71f61 100644 --- a/app/policies/mapping_policy.rb +++ b/app/policies/mapping_policy.rb @@ -16,20 +16,28 @@ class MappingPolicy < ApplicationPolicy end def show? - map = Pundit.policy(user, record.map) - mappable = Pundit.policy(user, record.mappable) - map.show? && mappable.show? + map_policy.show? && mappable_policy.show? end def create? - Pundit.policy(user, record.map).update? + map_policy.update? end def update? - Pundit.policy(user, record.map).update? + record.mappable_type == 'Topic' && map_policy.update? end def destroy? - record.user == user || admin_override + map_policy.update? || admin_override + end + + # Helpers + + def map_policy + @map_policy ||= Pundit.policy(user, record.map) + end + + def mappable_policy + @mappable_policy ||= Pundit.policy(user, record.mappable) end end From f7201e048e68b80ba7fc0fadd8e2b68406f43b45 Mon Sep 17 00:00:00 2001 From: Devin Howard Date: Mon, 14 Mar 2016 11:15:10 +0800 Subject: [PATCH 4/4] pending mapping policy --- spec/policies/mapping_policy_spec.rb | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 spec/policies/mapping_policy_spec.rb diff --git a/spec/policies/mapping_policy_spec.rb b/spec/policies/mapping_policy_spec.rb new file mode 100644 index 00000000..291a9c17 --- /dev/null +++ b/spec/policies/mapping_policy_spec.rb @@ -0,0 +1,7 @@ +require 'rails_helper' + +RSpec.describe MappingPolicy, type: :policy do + subject { described_class } + + pending 'Implement some mapping tests!' +end