mirror of
https://github.com/openfoodfoundation/openfoodnetwork
synced 2026-02-17 00:07:24 +00:00
Refactor line items search to improve security and maintainability
Moves search field configuration from frontend to backend to prevent potential security issues with exposing internal field names. The change also improves maintainability by centralizing search logic in the controller. Adds conditional logic to use name_alias for non-admin users when searching distributor names, enhancing data access control.
This commit is contained in:
@@ -98,6 +98,7 @@ Metrics/ClassLength:
|
||||
- 'lib/reporting/reports/enterprise_fee_summary/enterprise_fees_with_tax_report_by_producer.rb'
|
||||
- 'lib/reporting/reports/enterprise_fee_summary/scope.rb'
|
||||
- 'lib/reporting/reports/xero_invoices/base.rb'
|
||||
- 'app/services/permissions/order.rb'
|
||||
|
||||
# Offense count: 30
|
||||
# Configuration parameters: AllowedMethods, AllowedPatterns, Max.
|
||||
|
||||
@@ -19,18 +19,6 @@ angular.module("admin.lineItems").controller 'LineItemsCtrl', ($scope, $timeout,
|
||||
$scope.page = 1
|
||||
$scope.per_page = $scope.per_page_options[0].id
|
||||
$scope.filterByVariantId = null
|
||||
searchThrough = ["order_distributor_name_alias",
|
||||
"order_bill_address_phone",
|
||||
"order_bill_address_firstname",
|
||||
"order_bill_address_lastname",
|
||||
"order_bill_address_full_name",
|
||||
"order_bill_address_full_name_reversed",
|
||||
"order_bill_address_full_name_with_comma",
|
||||
"order_bill_address_full_name_with_comma_reversed",
|
||||
"variant_supplier_name",
|
||||
"order_email",
|
||||
"order_number",
|
||||
"product_name"].join("_or_") + "_cont"
|
||||
|
||||
$scope.confirmRefresh = ->
|
||||
LineItems.allSaved() || confirm(t("unsaved_changes_warning"))
|
||||
@@ -75,11 +63,10 @@ angular.module("admin.lineItems").controller 'LineItemsCtrl', ($scope, $timeout,
|
||||
[formattedStartDate, formattedEndDate] = $scope.formatDates($scope.startDate, $scope.endDate)
|
||||
|
||||
RequestMonitor.load LineItems.index(
|
||||
"q[#{searchThrough}]": $scope.query,
|
||||
"q[variant_id_eq]": $scope.filterByVariantId if $scope.filterByVariantId,
|
||||
"q[order_state_not_eq]": "canceled",
|
||||
"q[order_shipment_state_not_eq]": "shipped",
|
||||
"q[order_completed_at_not_null]": "true",
|
||||
"q[variant_id_eq]": $scope.filterByVariantId if $scope.filterByVariantId,
|
||||
"q[order_distributor_id_eq]": $scope.distributorFilter,
|
||||
"q[variant_supplier_id_eq]": $scope.supplierFilter,
|
||||
"q[order_order_cycle_id_eq]": $scope.orderCycleFilter,
|
||||
@@ -87,7 +74,8 @@ angular.module("admin.lineItems").controller 'LineItemsCtrl', ($scope, $timeout,
|
||||
"q[order_completed_at_lt]": if formattedEndDate then formattedEndDate else undefined,
|
||||
"q[s]": "order_completed_at desc",
|
||||
"page": $scope.page,
|
||||
"per_page": $scope.per_page
|
||||
"per_page": $scope.per_page,
|
||||
"search_query": $scope.query
|
||||
)
|
||||
|
||||
$scope.formatDates = (startDate, endDate) ->
|
||||
|
||||
@@ -12,7 +12,7 @@ module Admin
|
||||
@line_items = order_permissions.
|
||||
editable_line_items.where(order_id: orders).
|
||||
includes(:variant).
|
||||
ransack(params[:q]).result.order(:id)
|
||||
ransack(line_items_search_query).result.order(:id)
|
||||
|
||||
@pagy, @line_items = pagy(@line_items) if pagination_required?
|
||||
|
||||
@@ -88,5 +88,27 @@ module Admin
|
||||
def page
|
||||
params[:page] || 1
|
||||
end
|
||||
|
||||
def line_items_search_query
|
||||
query = params.permit(q: {}).to_h[:q] || {}
|
||||
|
||||
search_fields_string = [
|
||||
spree_current_user.admin? ? "order_distributor_name" : "order_distributor_name_alias",
|
||||
"order_bill_address_phone",
|
||||
"order_bill_address_firstname",
|
||||
"order_bill_address_lastname",
|
||||
"order_bill_address_full_name",
|
||||
"order_bill_address_full_name_reversed",
|
||||
"order_bill_address_full_name_with_comma",
|
||||
"order_bill_address_full_name_with_comma_reversed",
|
||||
"variant_supplier_name",
|
||||
"order_email",
|
||||
"order_number",
|
||||
"product_name"
|
||||
].join("_or_")
|
||||
search_query = "#{search_fields_string}_cont"
|
||||
|
||||
query.merge({ search_query => params[:search_query] })
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
@@ -27,7 +27,9 @@ RSpec.describe Permissions::Order do
|
||||
before { allow(OpenFoodNetwork::Permissions).to receive(:new) { basic_permissions } }
|
||||
|
||||
context "with user cannot only manage line_items in orders" do
|
||||
let(:user) { instance_double('Spree::User', can_manage_line_items_in_orders_only?: false) }
|
||||
let(:user) do
|
||||
instance_double('Spree::User', can_manage_line_items_in_orders_only?: false, admin?: false)
|
||||
end
|
||||
|
||||
describe "finding orders that are visible in reports" do
|
||||
let(:random_enterprise) { create(:distributor_enterprise) }
|
||||
|
||||
Reference in New Issue
Block a user