Skip to content

Commit

Permalink
error on disallowed ransack parameters
Browse files Browse the repository at this point in the history
  • Loading branch information
timcowlishaw committed Oct 10, 2023
1 parent 3d75c00 commit bad8e03
Show file tree
Hide file tree
Showing 11 changed files with 52 additions and 34 deletions.
8 changes: 8 additions & 0 deletions app/controllers/v0/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,14 @@ def check_missing_params *params_list

private

def raise_ransack_errors_as_bad_request(&block)
begin
block.call
rescue ArgumentError => e
raise ActionController::BadRequest.new(e.message)
end
end

def prepend_view_paths
# is this still necessary?
prepend_view_path "app/views/v0"
Expand Down
9 changes: 5 additions & 4 deletions app/controllers/v0/devices_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,11 @@ def show
end

def index
@q = policy_scope(Device)
.includes(:owner, :tags, kit: [:components, :sensors])
.ransack(params[:q], auth_object: (current_user&.is_admin? ? :admin : nil))

raise_ransack_errors_as_bad_request do
@q = policy_scope(Device)
.includes(:owner, :tags, kit: [:components, :sensors])
.ransack(params[:q], auth_object: (current_user&.is_admin? ? :admin : nil))
end
# We are here customly adding multiple tags into the Ransack query.
# Ransack supports this, but how do we add multiple tag names in URL string? Which separator to use?
# See Issue #186 https://github.com/fablabbcn/smartcitizen-api/issues/186
Expand Down
4 changes: 3 additions & 1 deletion app/controllers/v0/sensors_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@ def show
end

def index
@q = Sensor.includes(:measurement, :tag_sensors).ransack(params[:q])
raise_ransack_errors_as_bad_request do
@q = Sensor.includes(:measurement, :tag_sensors).ransack(params[:q])
end
@q.sorts = 'id asc' if @q.sorts.empty?
@sensors = @q.result(distinct: true)
@sensors = paginate @sensors
Expand Down
4 changes: 3 additions & 1 deletion app/controllers/v0/users_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@ def show
end

def index
@q = User.includes(:devices, :profile_picture_attachment).ransack(params[:q])
raise_ransack_errors_as_bad_request do
@q = User.includes(:devices, :profile_picture_attachment).ransack(params[:q])
end
@q.sorts = 'id asc' if @q.sorts.empty?
@users = @q.result(distinct: true)
@users = paginate(@users)
Expand Down
4 changes: 4 additions & 0 deletions app/models/postprocessing.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,8 @@ class Postprocessing < ApplicationRecord
def self.ransackable_attributes(auth_object = nil)
["blueprint_url", "created_at", "device_id", "forwarding_params", "hardware_url", "id", "latest_postprocessing", "meta", "updated_at"]
end

def self_ransackable_associations(auth_object = nil)
[]
end
end
4 changes: 4 additions & 0 deletions app/models/sensor.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ def self.ransackable_attributes(auth_object = nil)
["ancestry", "created_at", "description", "id", "measurement_id", "name", "unit", "updated_at", "uuid"]
end

def self.ransackable_associations(auth_object = nil)
[]
end

def tags
tag_sensors.map(&:name)
end
Expand Down
4 changes: 4 additions & 0 deletions app/models/tag.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,4 +14,8 @@ class Tag < ActiveRecord::Base
def self.ransackable_attributes(auth_object = nil)
["created_at", "description", "id", "name", "updated_at", "uuid"]
end

def ransackable_associations(auth_object = nil)
[]
end
end
5 changes: 5 additions & 0 deletions config/initializers/ransack.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Ransack.configure do |config|
# Raise errors if a query contains an unknown predicate or attribute.
# Default is true (do not raise error on unknown conditions).
config.ignore_unknown_conditions = false
end
6 changes: 4 additions & 2 deletions spec/models/device_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -298,8 +298,10 @@
expect(Device.count).to eq(3)
# Admins can ransack on device_token_contains
expect(Device.ransack({device_token_cont: '123'}, auth_object: :admin).result.count).to eq(1)
# Normal users get all deviecs returned. They cannot ransack on device_token
expect(Device.ransack({device_token_cont: '123'}).result.count).to eq(3)
# Normal users cannot ransack on device_token
expect {
Device.ransack({device_token_cont: '123'})
}.to raise_error(ArgumentError)
end

it 'does not validate uniqueness when nil' do
Expand Down
31 changes: 8 additions & 23 deletions spec/requests/v0/devices_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -124,16 +124,6 @@
expect(response.status).to eq(200)
end

it "allows searching by tag name" do
json = api_get "devices?q[tag_name_in]=Barcelona"
expect(response.status).to eq(200)
end

it "allows searching by tag names" do
json = api_get "devices?with_tags=Barcelona%7CAmsterdam"
expect(response.status).to eq(200)
end

it "allows searching by name" do
json = api_get "devices?q[name_eq]=Name"
expect(response.status).to eq(200)
Expand All @@ -144,11 +134,17 @@
expect(response.status).to eq(200)
end

it "allows searching by mac address" do
json = api_get "devices?q[mac_address_eq]=00:00:00:00:00:00"
it "allows searching by mac address by admins" do
json = api_get "devices?q[mac_address_eq]=00:00:00:00:00:00&access_token=#{admin_token.token}"
expect(response.status).to eq(200)
end

it "does not allow searching by mac address by non-admins" do
expect {
api_get "devices?q[mac_address_eq]=00:00:00:00:00:00"
}.to raise_error(ActionController::BadRequest)
end

it "allows searching by created_at" do
json = api_get "devices?q[created_at_lt]=2023-09-26"
expect(response.status).to eq(200)
Expand Down Expand Up @@ -178,17 +174,6 @@
json = api_get "devices?q[state_eq]=state"
expect(response.status).to eq(200)
end

it "allows searching by postprocessing_id" do
json = api_get "devices?q[postprocessing_id_eq]=postprocessing_id"
expect(response.status).to eq(200)
end

it "allows searching by hardware_info" do
json = api_get "devices?q[hardware_info_eq]=hardware_info"
expect(response.status).to eq(200)
end

end
end

Expand Down
7 changes: 4 additions & 3 deletions spec/requests/v0/users_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,10 @@
end

describe "smoke tests for ransack" do
it "allows searching by first name" do
json = api_get "users?q[first_name_eq]=Tim"
expect(response.status).to eq(200)
it "does not allow searching by first name" do
expect {
api_get "users?q[first_name_eq]=Tim"
}.to raise_error(ActionController::BadRequest)
end

it "allows searching by username" do
Expand Down

0 comments on commit bad8e03

Please sign in to comment.