fix: problem of two-step flow find and authorize (#344)

danielnottingham · web-flow · commit 67c14f9ddc4b · 2026-01-16T10:15:09.000-03:00
diff --git a/app/controllers/api/v1/event_procedures_controller.rb b/app/controllers/api/v1/event_procedures_controller.rb
@@ -111,7 +111,10 @@ def event_procedure_params
       end
 
       def event_procedure
-        @event_procedure ||= EventProcedures::Find.result(id: params[:id]).event_procedure
+        @event_procedure ||= EventProcedures::Find.result(
+          id: params[:id],
+          scope: policy_scope(EventProcedure)
+        ).event_procedure
       end
 
       def serialized_event_procedures(event_procedures)
diff --git a/app/controllers/api/v1/hospitals_controller.rb b/app/controllers/api/v1/hospitals_controller.rb
@@ -48,7 +48,10 @@ def destroy
       private
 
       def hospital
-        @hospital ||= Hospitals::Find.result(id: params[:id]).hospital
+        @hospital ||= Hospitals::Find.result(
+          id: params[:id],
+          scope: policy_scope(Hospital)
+        ).hospital
       end
 
       def hospital_params
diff --git a/app/controllers/api/v1/medical_shifts_controller.rb b/app/controllers/api/v1/medical_shifts_controller.rb
@@ -80,7 +80,10 @@ def destroy
       private
 
       def medical_shift
-        @medical_shift ||= MedicalShifts::Find.result(id: params[:id]).medical_shift
+        @medical_shift ||= MedicalShifts::Find.result(
+          id: params[:id],
+          scope: policy_scope(MedicalShift)
+        ).medical_shift
       end
 
       def medical_shift_params
diff --git a/app/controllers/api/v1/patients_controller.rb b/app/controllers/api/v1/patients_controller.rb
@@ -52,7 +52,10 @@ def destroy
       private
 
       def patient
-        @patient ||= Patients::Find.result(id: params[:id]).patient
+        @patient ||= Patients::Find.result(
+          id: params[:id],
+          scope: policy_scope(Patient)
+        ).patient
       end
 
       def patient_params
diff --git a/app/controllers/api/v1/procedures_controller.rb b/app/controllers/api/v1/procedures_controller.rb
@@ -50,7 +50,10 @@ def destroy
       private
 
       def procedure
-        @procedure ||= Procedures::Find.result(id: params[:id]).procedure
+        @procedure ||= Procedures::Find.result(
+          id: params[:id],
+          scope: policy_scope(Procedure)
+        ).procedure
       end
 
       def procedure_params
diff --git a/app/operations/event_procedures/find.rb b/app/operations/event_procedures/find.rb
@@ -3,11 +3,12 @@
 module EventProcedures
   class Find < Actor
     input :id, type: String
+    input :scope, type: Enumerable, default: -> { EventProcedure.all }
 
     output :event_procedure, type: EventProcedure
 
     def call
-      self.event_procedure = EventProcedure.find(id)
+      self.event_procedure = scope.find(id)
     end
   end
 end
diff --git a/app/operations/hospitals/find.rb b/app/operations/hospitals/find.rb
@@ -3,11 +3,12 @@
 module Hospitals
   class Find < Actor
     input :id, type: String
+    input :scope, type: Enumerable, default: -> { Hospital.all }
 
     output :hospital, type: Hospital
 
     def call
-      self.hospital = Hospital.find(id)
+      self.hospital = scope.find(id)
     end
   end
 end
diff --git a/app/operations/medical_shifts/find.rb b/app/operations/medical_shifts/find.rb
@@ -3,11 +3,12 @@
 module MedicalShifts
   class Find < Actor
     input :id, type: String
+    input :scope, type: Enumerable, default: -> { MedicalShift.all }
 
     output :medical_shift, type: MedicalShift
 
     def call
-      self.medical_shift = MedicalShift.find(id)
+      self.medical_shift = scope.find(id)
     end
   end
 end
diff --git a/app/operations/patients/find.rb b/app/operations/patients/find.rb
@@ -3,11 +3,12 @@
 module Patients
   class Find < Actor
     input :id, type: String
+    input :scope, type: Enumerable, default: -> { Patient.all }
 
     output :patient, type: Patient
 
     def call
-      self.patient = Patient.find(id)
+      self.patient = scope.find(id)
     end
   end
 end
diff --git a/app/operations/procedures/find.rb b/app/operations/procedures/find.rb
@@ -3,11 +3,12 @@
 module Procedures
   class Find < Actor
     input :id, type: String
+    input :scope, type: Enumerable, default: -> { Procedure.all }
 
     output :procedure, type: Procedure
 
     def call
-      self.procedure = Procedure.find(id)
+      self.procedure = scope.find(id)
     end
   end
 end
diff --git a/app/policies/hospital_policy.rb b/app/policies/hospital_policy.rb
@@ -1,6 +1,12 @@
 # frozen_string_literal: true
 
 class HospitalPolicy < ApplicationPolicy
+  class Scope < ApplicationScope
+    def resolve
+      scope.all
+    end
+  end
+
   def index?
     user.present?
   end
diff --git a/app/policies/procedure_policy.rb b/app/policies/procedure_policy.rb
@@ -1,6 +1,9 @@
 # frozen_string_literal: true
 
 class ProcedurePolicy < ApplicationPolicy
+  class Scope < CurrentUserScope
+  end
+
   def index?
     user.present?
   end
diff --git a/spec/operations/event_procedures/find_spec.rb b/spec/operations/event_procedures/find_spec.rb
@@ -27,5 +27,29 @@
         end.to raise_error(ActiveRecord::RecordNotFound)
       end
     end
+
+    context "when using a scope" do
+      let(:user) { create(:user) }
+      let(:event_procedure) { create(:event_procedure, user: user) }
+      let(:other_event_procedure) { create(:event_procedure) }
+
+      it "returns found event_procedure when it exists in scope" do
+        result = described_class.result(
+          id: event_procedure.id.to_s,
+          scope: EventProcedure.where(user: user)
+        )
+
+        expect(result.event_procedure).to eq(event_procedure)
+      end
+
+      it "raises ActiveRecord::RecordNotFound when record exists but not in scope" do
+        expect do
+          described_class.result(
+            id: other_event_procedure.id.to_s,
+            scope: EventProcedure.where(user: user)
+          )
+        end.to raise_error(ActiveRecord::RecordNotFound)
+      end
+    end
   end
 end
diff --git a/spec/operations/hospitals/find_spec.rb b/spec/operations/hospitals/find_spec.rb
@@ -27,5 +27,28 @@
         end.to raise_error(ActiveRecord::RecordNotFound)
       end
     end
+
+    context "when using a scope" do
+      let!(:hospital) { create(:hospital, name: "Hospital A", address: "Address A") }
+      let!(:other_hospital) { create(:hospital, name: "Hospital B", address: "Address B") }
+
+      it "returns found hospital when it exists in scope" do
+        result = described_class.result(
+          id: hospital.id.to_s,
+          scope: Hospital.where(name: "Hospital A")
+        )
+
+        expect(result.hospital).to eq(hospital)
+      end
+
+      it "raises ActiveRecord::RecordNotFound when record exists but not in scope" do
+        expect do
+          described_class.result(
+            id: other_hospital.id.to_s,
+            scope: Hospital.where(name: "Hospital A")
+          )
+        end.to raise_error(ActiveRecord::RecordNotFound)
+      end
+    end
   end
 end
diff --git a/spec/operations/medical_shifts/find_spec.rb b/spec/operations/medical_shifts/find_spec.rb
@@ -29,5 +29,29 @@
         end.to raise_error(ActiveRecord::RecordNotFound)
       end
     end
+
+    context "when using a scope" do
+      let(:user) { create(:user) }
+      let(:medical_shift) { create(:medical_shift, user: user) }
+      let(:other_medical_shift) { create(:medical_shift) }
+
+      it "returns found medical_shift when it exists in scope" do
+        result = described_class.result(
+          id: medical_shift.id.to_s,
+          scope: MedicalShift.where(user: user)
+        )
+
+        expect(result.medical_shift).to eq(medical_shift)
+      end
+
+      it "raises ActiveRecord::RecordNotFound when record exists but not in scope" do
+        expect do
+          described_class.result(
+            id: other_medical_shift.id.to_s,
+            scope: MedicalShift.where(user: user)
+          )
+        end.to raise_error(ActiveRecord::RecordNotFound)
+      end
+    end
   end
 end
diff --git a/spec/operations/patients/find_spec.rb b/spec/operations/patients/find_spec.rb
@@ -29,5 +29,29 @@
         end.to raise_error(ActiveRecord::RecordNotFound)
       end
     end
+
+    context "when using a scope" do
+      let(:user) { create(:user) }
+      let(:patient) { create(:patient, user: user) }
+      let(:other_patient) { create(:patient) }
+
+      it "returns found patient when it exists in scope" do
+        result = described_class.result(
+          id: patient.id.to_s,
+          scope: Patient.where(user: user)
+        )
+
+        expect(result.patient).to eq(patient)
+      end
+
+      it "raises ActiveRecord::RecordNotFound when record exists but not in scope" do
+        expect do
+          described_class.result(
+            id: other_patient.id.to_s,
+            scope: Patient.where(user: user)
+          )
+        end.to raise_error(ActiveRecord::RecordNotFound)
+      end
+    end
   end
 end
diff --git a/spec/operations/procedures/find_spec.rb b/spec/operations/procedures/find_spec.rb
@@ -27,5 +27,30 @@
         end.to raise_error(ActiveRecord::RecordNotFound)
       end
     end
+
+    context "when using a scope" do
+      let(:user) { create(:user) }
+      let(:other_user) { create(:user) }
+      let(:procedure) { create(:procedure, custom: true, user: user) }
+      let(:other_procedure) { create(:procedure, custom: true, user: other_user) }
+
+      it "returns found procedure when it exists in scope" do
+        result = described_class.result(
+          id: procedure.id.to_s,
+          scope: Procedure.where(user: user)
+        )
+
+        expect(result.procedure).to eq(procedure)
+      end
+
+      it "raises ActiveRecord::RecordNotFound when record exists but not in scope" do
+        expect do
+          described_class.result(
+            id: other_procedure.id.to_s,
+            scope: Procedure.where(user: user)
+          )
+        end.to raise_error(ActiveRecord::RecordNotFound)
+      end
+    end
   end
 end
diff --git a/spec/requests/api/v1/event_procedures_request_spec.rb b/spec/requests/api/v1/event_procedures_request_spec.rb
@@ -463,7 +463,7 @@
       end
 
       context "with valid attributes and the record not belongs to the user" do
-        it "returns unauthorized" do
+        it "returns not_found to prevent ID enumeration" do
           other_user = create(:user)
           patient = create(:patient, user: other_user)
           health_insurance = create(:health_insurance)
@@ -495,7 +495,7 @@
 
           put "/api/v1/event_procedures/#{event_procedure.id}", params: params, headers: headers
 
-          expect(response).to have_http_status(:unauthorized)
+          expect(response).to have_http_status(:not_found)
         end
       end
 
diff --git a/spec/requests/api/v1/medical_shifts_request_spec.rb b/spec/requests/api/v1/medical_shifts_request_spec.rb
@@ -325,12 +325,12 @@
 
     context "when user is authenticated" do
       context "when updating another user's medical_shift" do
-        it "returns unauthorized status" do
+        it "returns not_found to prevent ID enumeration" do
           medical_shift = create(:medical_shift, workload: MedicalShifts::Workloads::SIX)
           params = { workload: MedicalShifts::Workloads::TWELVE }
           put api_v1_medical_shift_path(medical_shift), params: params, headers: headers
 
-          expect(response).to have_http_status(:unauthorized)
+          expect(response).to have_http_status(:not_found)
         end
       end
 
diff --git a/spec/requests/api/v1/patients_request_spec.rb b/spec/requests/api/v1/patients_request_spec.rb
@@ -168,30 +168,12 @@
     context "when user is authenticated" do
       include_examples "delete request returns ok", Patient
 
-      context "when patient cannot be destroyed" do
+      context "when trying to destroy another user's patient" do
         let(:patient) { create(:patient) }
-        let(:errors) do
-          instance_double(
-            ActiveModel::Errors,
-            full_messages: ["Cannot delete record because of dependent event_procedures"]
-          )
-        end
-
-        before do
-          allow(Patient).to receive(:destroy).with(patient.id.to_s).and_return(patient)
-          allow(patient).to receive_messages(destroy: false, errors: errors)
-          allow(patient).to receive(:errors).and_return(errors)
-        end
 
-        it "returns unauthorized" do
+        it "returns not_found to prevent ID enumeration" do
           delete path, headers: headers
-          expect(response).to have_http_status(:unauthorized)
-        end
-
-        it "returns errors" do
-          delete path, headers: headers
-
-          expect(response.parsed_body).to include({ "error" => "not allowed to destroy? this Patient" })
+          expect(response).to have_http_status(:not_found)
         end
       end
 
