From 7ca35a9c33dbb9c5b5ec96e797b8a08c69b3a5af Mon Sep 17 00:00:00 2001
From: Martin Artnik <martin@scinote.net>
Date: Tue, 24 Sep 2024 12:32:34 +0200
Subject: [PATCH] Fix permission checks for nested storage locations
 [SCI-10865]

---
 ...storage_location_repository_rows_controller.rb |  4 ++--
 app/controllers/storage_locations_controller.rb   | 15 +++++++--------
 app/services/lists/storage_locations_service.rb   | 10 +++++++++-
 3 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/app/controllers/storage_location_repository_rows_controller.rb b/app/controllers/storage_location_repository_rows_controller.rb
index a1d3fc196..2f7309611 100644
--- a/app/controllers/storage_location_repository_rows_controller.rb
+++ b/app/controllers/storage_location_repository_rows_controller.rb
@@ -102,10 +102,10 @@ class StorageLocationRepositoryRowsController < ApplicationController
   end
 
   def load_storage_location
-    @storage_location = StorageLocation.viewable_by_user(current_user).find(
+    @storage_location = StorageLocation.find(
       storage_location_repository_row_params[:storage_location_id]
     )
-    render_404 unless @storage_location
+    render_404 unless can_read_storage_location?(@storage_location)
   end
 
   def load_repository_row
diff --git a/app/controllers/storage_locations_controller.rb b/app/controllers/storage_locations_controller.rb
index b239c9503..1e48a4e61 100644
--- a/app/controllers/storage_locations_controller.rb
+++ b/app/controllers/storage_locations_controller.rb
@@ -9,13 +9,12 @@ class StorageLocationsController < ApplicationController
   before_action :set_breadcrumbs_items, only: %i(index show)
 
   def index
+    @parent_location = StorageLocation.find(storage_location_params[:parent_id]) if storage_location_params[:parent_id]
+
+    render_403 if @parent_location && !can_read_storage_location?(@parent_location)
+
     respond_to do |format|
-      format.html do
-        if storage_location_params[:parent_id]
-          @parent_location = StorageLocation.viewable_by_user(current_user)
-                                            .find_by(id: storage_location_params[:parent_id])
-        end
-      end
+      format.html
       format.json do
         storage_locations = Lists::StorageLocationsService.new(current_user, current_team, params).call
         render json: storage_locations, each_serializer: Lists::StorageLocationSerializer,
@@ -183,8 +182,8 @@ class StorageLocationsController < ApplicationController
   end
 
   def load_storage_location
-    @storage_location = StorageLocation.viewable_by_user(current_user).find_by(id: storage_location_params[:id])
-    render_404 unless @storage_location
+    @storage_location = StorageLocation.find(storage_location_params[:id])
+    render_404 unless can_read_storage_location?(@storage_location)
   end
 
   def check_read_permissions
diff --git a/app/services/lists/storage_locations_service.rb b/app/services/lists/storage_locations_service.rb
index 7d497b245..0686b88db 100644
--- a/app/services/lists/storage_locations_service.rb
+++ b/app/services/lists/storage_locations_service.rb
@@ -2,6 +2,8 @@
 
 module Lists
   class StorageLocationsService < BaseService
+    include Canaid::Helpers::PermissionsHelper
+
     def initialize(user, team, params)
       @user = user
       @team = team
@@ -11,11 +13,15 @@ module Lists
     end
 
     def fetch_records
+      if @parent_id && !can_read_storage_location?(@user, StorageLocation.find(@parent_id))
+        @records = StorageLocation.none
+        return
+      end
+
       @records =
         StorageLocation.joins('LEFT JOIN storage_locations AS sub_locations ' \
                               'ON storage_locations.id = sub_locations.parent_id')
                        .left_joins(:team, :created_by)
-                       .viewable_by_user(@user, @team)
                        .select(shared_sql_select)
                        .select(
                          'storage_locations.*,
@@ -24,6 +30,8 @@ module Lists
                         CASE WHEN storage_locations.container THEN -1 ELSE COUNT(sub_locations.id) END AS sub_location_count'
                        )
                        .group(:id)
+
+      @records = @records.viewable_by_user(@user, @team) unless @parent_id
     end
 
     def filter_records