diff --git a/app/controllers/synapses_controller.rb b/app/controllers/synapses_controller.rb index e0b8f727..c3aed2bc 100644 --- a/app/controllers/synapses_controller.rb +++ b/app/controllers/synapses_controller.rb @@ -22,10 +22,18 @@ class SynapsesController < ApplicationController @synapse = Synapse.new(synapse_params) @synapse.desc = '' if @synapse.desc.nil? @synapse.desc.strip! # no trailing/leading whitespace - authorize @synapse + + # we want invalid params to return :unprocessable_entity + # so we have to authorize AFTER saving. But if authorize + # fails, we need to rollback the SQL transaction + success = nil + ActiveRecord::Base.transaction do + success = @synapse.save + success ? authorize(@synapse) : skip_authorization + end respond_to do |format| - if @synapse.save + if success format.json { render json: @synapse, status: :created } else format.json { render json: @synapse.errors, status: :unprocessable_entity } diff --git a/app/policies/synapse_policy.rb b/app/policies/synapse_policy.rb index f3d2c997..145f7432 100644 --- a/app/policies/synapse_policy.rb +++ b/app/policies/synapse_policy.rb @@ -16,16 +16,16 @@ class SynapsePolicy < ApplicationPolicy end def create? - user.present? - # TODO: add validation against whether you can see both topics + if record.try(:topic1) && record.try(:topic2) + topic1_show? && topic2_show? && user.present? + else + # allows us to use policy(Synapse).create? + user.present? + end end def show? - if record.defer_to_map.present? - map_policy.show? - else - record.permission == 'commons' || record.permission == 'public' || record.user == user - end + topic1_show? && topic2_show? && synapse_show? end def update? @@ -43,7 +43,26 @@ class SynapsePolicy < ApplicationPolicy end # Helpers + def map_policy @map_policy ||= Pundit.policy(user, record.defer_to_map) end + + def topic1_show? + @topic1_policy ||= Pundit.policy(user, record.topic1) + @topic1_policy&.show? + end + + def topic2_show? + @topic2_policy ||= Pundit.policy(user, record.topic2) + @topic2_policy&.show? + end + + def synapse_show? + if record.defer_to_map.present? + map_policy&.show? + else + record.permission == 'commons' || record.permission == 'public' || record.user == user + end + end end diff --git a/spec/controllers/synapses_controller_spec.rb b/spec/controllers/synapses_controller_spec.rb index 3a5310e4..511971ad 100644 --- a/spec/controllers/synapses_controller_spec.rb +++ b/spec/controllers/synapses_controller_spec.rb @@ -35,6 +35,16 @@ RSpec.describe SynapsesController, type: :controller do end end + context 'with private topics' do + it 'redirects to /' do + post :create, format: :json, params: { + synapse: valid_attributes.merge(topic1_id: create(:topic, permission: 'private'), topic2_id: create(:topic, permission: 'private')) + } + expect(response.status).to eq 302 + expect(response).to redirect_to('/') + end + end + context 'with invalid params' do it 'returns 422 UNPROCESSABLE ENTITY' do post :create, format: :json, params: { @@ -42,6 +52,13 @@ RSpec.describe SynapsesController, type: :controller do } expect(response.status).to eq 422 end + it 'does not create a synapse' do + expect { + post :create, format: :json, params: { synapse: invalid_attributes } + }.to change { + Synapse.count + }.by 0 + end end end