make synapse permissions depend on topic1 and topic2 (#839)

* deep change to synapse policy - is this ok?

* make synapse policy resilient to nil topic1/topic2/map

* use a transaction to handle authorization vs invalid record in synapse controller

* more synapse controller tests

* inline documentation

* fix policy(Synapse).create?

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 }

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

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