From b71930fba2d1932d50e9f52802a9badbfa078045 Mon Sep 17 00:00:00 2001 From: Pablo Brasero Date: Fri, 27 Dec 2019 14:32:07 +0100 Subject: [PATCH] Remove query from link, to avoid triggering banned params (#1507) With https://github.com/thoughtbot/administrate/pull/1457, we introduced a bug. Reproduction: 1. Visit the index page of a model with a has_many relationship (eg: customers). 2. Click on a table header to trigger some sorting. The current URL will gain query params. 3. Click on a row to see the show page of a record. Don't click on actual data on the table (eg: customer name), but instead click on a "blank" area of the row. This is so that the JS click handler kicks in and performs the "link" behaviour, as opposed to clicking on an actual link. 4. You'll get an error `ActionController::UnpermittedParameters`. This is triggered by the following: 1. At the index page, the URL is something like `/admin/customers` 2. When sorting the table, the current URL becomes something like `/admin/customers?customer%5Bdirection%5D=desc&customer%5Border%5D=name` 3. Clicking on the row, the JS gets us a URL like so `/admin/customers/123?customer%5Bdirection%5D=desc&customer%5Border%5D=name` 4. The partial that renders the has_many relationship in the show page can receive query params. It checks that the current ones are permitted. Turns out those aren't (they are in the index page, but not here), so it raises an exception. --- .../javascripts/administrate/components/table.js | 2 +- spec/features/index_page_spec.rb | 10 ++++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/app/assets/javascripts/administrate/components/table.js b/app/assets/javascripts/administrate/components/table.js index 81be98358d..493e691bce 100644 --- a/app/assets/javascripts/administrate/components/table.js +++ b/app/assets/javascripts/administrate/components/table.js @@ -13,7 +13,7 @@ $(function() { var dataUrl = $(event.target).closest("tr").data("url"); var selection = window.getSelection().toString(); if (selection.length === 0 && dataUrl) { - window.location.pathname = dataUrl; + window.location = window.location.protocol + '//' + window.location.host + dataUrl; } } }; diff --git a/spec/features/index_page_spec.rb b/spec/features/index_page_spec.rb index 57b181a6b1..faa8880cfc 100644 --- a/spec/features/index_page_spec.rb +++ b/spec/features/index_page_spec.rb @@ -95,6 +95,16 @@ def expect_to_appear_in_order(*elements) expect_to_appear_in_order("unique name one", "unique name two") end + it "allows clicking through after sorting", :js do + customer = create(:customer) + create(:order, customer: customer) + + visit admin_customers_path + click_on "Name" + find("[data-url]").click + expect(page).to have_header("Show #{customer.name}") + end + it "allows reverse sorting" do create(:customer, name: "unique name one") create(:customer, name: "unique name two")