Skip to content

Commit

Permalink
Check value of order filter (#951)
Browse files Browse the repository at this point in the history
* check value of order filter

To prevent parameters like:

* `{"order":"captured_at desc) UNION ALL SELECT NULL,NULL,NULL#","locale":"en-US","device_id":"100171"}`
* `{"order":"captured_at desc%' ORDER BY 1#","locale":"en-US","device_id":"100171"}`

* specify locale in spec
  • Loading branch information
eitoball committed Oct 13, 2022
1 parent 3ede879 commit 95a6194
Show file tree
Hide file tree
Showing 7 changed files with 28 additions and 17 deletions.
3 changes: 2 additions & 1 deletion app/controllers/bgeigie_imports_controller.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
# frozen_string_literal: true

class BgeigieImportsController < ApplicationController # rubocop:disable Metrics/ClassLength
include HasOrderScope

respond_to :html, :json

before_action :authenticate_user!, only: %i(new create edit update destroy)
Expand All @@ -14,7 +16,6 @@ class BgeigieImportsController < ApplicationController # rubocop:disable Metrics
has_scope :by_user_id
has_scope :by_rejected
has_scope :by_user_name
has_scope :order
has_scope :uploaded_after
has_scope :uploaded_before
has_scope :rejected_by
Expand Down
19 changes: 19 additions & 0 deletions app/controllers/concerns/has_order_scope.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
# frozen_string_literal: true

module HasOrderScope
extend ActiveSupport::Concern

VALID_SORT_ORDERS = %w(asc desc).freeze

included do
has_scope :order do |_controller, scope, value|
_column_name, sort_order = value.split(' ', 2)

sort_order = sort_order.to_s.strip.downcase || 'asc'
if VALID_SORT_ORDERS.include?(sort_order)
# TODO: Use `nulls_last` Arel method in Rails 6.1
scope.order("#{value} NULLS LAST")
end
end
end
end
5 changes: 1 addition & 4 deletions app/controllers/device_stories_controller.rb
Original file line number Diff line number Diff line change
@@ -1,10 +1,7 @@
# frozen_string_literal: true

class DeviceStoriesController < ApplicationController
has_scope :order do |_controller, scope, value|
# TODO: Use `nulls_last` Arel method in Rails 6.1
scope.order("#{value} NULLS LAST")
end
include HasOrderScope

before_action :authenticate_user!, only: %i(new create edit update destroy)
before_action :fetch_device_story, only: %i(show)
Expand Down
6 changes: 2 additions & 4 deletions app/controllers/devices_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,8 @@
# @topic Devices
#
class DevicesController < ApplicationController
has_scope :order do |_controller, scope, value|
# TODO: Use `nulls_last` Arel method in Rails 6.1
scope.order("#{value} NULLS LAST")
end
include HasOrderScope

has_scope :manufacturer do |_controller, scope, value|
scope.where('manufacturer LIKE ?', "%#{value}%")
end
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/measurements_controller.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# frozen_string_literal: true

class MeasurementsController < ApplicationController
include HasOrderScope
include SwaggerBlocks::Controllers::Measurements

class << self
Expand All @@ -26,7 +27,6 @@ def attribute_names_to_be_wrapped
has_scope :distance do |controller, scope, _value|
scope.nearby_to(controller.params[:latitude], controller.params[:longitude], controller.params[:distance])
end
has_scope :order
has_scope :original_id do |_controller, scope, value|
scope.where('original_id = :value OR id = :value', value: value)
end
Expand Down
6 changes: 1 addition & 5 deletions app/controllers/users_controller.rb
Original file line number Diff line number Diff line change
@@ -1,15 +1,11 @@
# frozen_string_literal: true

class UsersController < ApplicationController
include HasOrderScope
include Swagger::Blocks

before_action :authenticate_user!, only: %i(me)

has_scope :order do |_controller, scope, value|
# TODO: Use `nulls_last` Arel method in Rails 6.1
scope.order("#{value} NULLS LAST")
end

has_scope :name do |_controller, scope, value|
scope.by_name(value)
end
Expand Down
4 changes: 2 additions & 2 deletions spec/controllers/users_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,11 @@

context 'when not sign in' do
before do
get :me
get :me, params: { locale: 'en-US' }
end

it 'return HTTP 401 Unauthorized' do
expect(response).to redirect_to(new_user_session_path)
expect(response).to redirect_to(new_user_session_path(locale: 'en-US'))
end
end

Expand Down

0 comments on commit 95a6194

Please sign in to comment.