diff --git a/app/controllers/admin/hidden_budget_investments_controller.rb b/app/controllers/admin/hidden_budget_investments_controller.rb index c55ab3ef6..6b977cdd3 100644 --- a/app/controllers/admin/hidden_budget_investments_controller.rb +++ b/app/controllers/admin/hidden_budget_investments_controller.rb @@ -15,14 +15,14 @@ class Admin::HiddenBudgetInvestmentsController < Admin::BaseController def confirm_hide @investment.confirm_hide - redirect_to request.query_parameters.merge(action: :index) + redirect_with_query_params_to(action: :index) end def restore @investment.restore(recursive: true) @investment.ignore_flag Activity.log(current_user, :restore, @investment) - redirect_to request.query_parameters.merge(action: :index) + redirect_with_query_params_to(action: :index) end private diff --git a/app/controllers/admin/hidden_comments_controller.rb b/app/controllers/admin/hidden_comments_controller.rb index b3f64688f..fc778a1a6 100644 --- a/app/controllers/admin/hidden_comments_controller.rb +++ b/app/controllers/admin/hidden_comments_controller.rb @@ -10,14 +10,14 @@ class Admin::HiddenCommentsController < Admin::BaseController def confirm_hide @comment.confirm_hide - redirect_to request.query_parameters.merge(action: :index) + redirect_with_query_params_to(action: :index) end def restore @comment.restore(recursive: true) @comment.ignore_flag Activity.log(current_user, :restore, @comment) - redirect_to request.query_parameters.merge(action: :index) + redirect_with_query_params_to(action: :index) end private diff --git a/app/controllers/admin/hidden_debates_controller.rb b/app/controllers/admin/hidden_debates_controller.rb index de9610972..b5a4ea502 100644 --- a/app/controllers/admin/hidden_debates_controller.rb +++ b/app/controllers/admin/hidden_debates_controller.rb @@ -13,14 +13,14 @@ class Admin::HiddenDebatesController < Admin::BaseController def confirm_hide @debate.confirm_hide - redirect_to request.query_parameters.merge(action: :index) + redirect_with_query_params_to(action: :index) end def restore @debate.restore!(recursive: true) @debate.ignore_flag Activity.log(current_user, :restore, @debate) - redirect_to request.query_parameters.merge(action: :index) + redirect_with_query_params_to(action: :index) end private diff --git a/app/controllers/admin/hidden_proposals_controller.rb b/app/controllers/admin/hidden_proposals_controller.rb index 9dea18a07..8ee137140 100644 --- a/app/controllers/admin/hidden_proposals_controller.rb +++ b/app/controllers/admin/hidden_proposals_controller.rb @@ -14,14 +14,14 @@ class Admin::HiddenProposalsController < Admin::BaseController def confirm_hide @proposal.confirm_hide - redirect_to request.query_parameters.merge(action: :index) + redirect_with_query_params_to(action: :index) end def restore @proposal.restore(recursive: true) @proposal.ignore_flag Activity.log(current_user, :restore, @proposal) - redirect_to request.query_parameters.merge(action: :index) + redirect_with_query_params_to(action: :index) end private diff --git a/app/controllers/admin/hidden_users_controller.rb b/app/controllers/admin/hidden_users_controller.rb index 31578c94e..fef56a94e 100644 --- a/app/controllers/admin/hidden_users_controller.rb +++ b/app/controllers/admin/hidden_users_controller.rb @@ -15,13 +15,13 @@ class Admin::HiddenUsersController < Admin::BaseController def confirm_hide @user.confirm_hide - redirect_to request.query_parameters.merge(action: :index) + redirect_with_query_params_to(action: :index) end def restore @user.restore Activity.log(current_user, :restore, @user) - redirect_to request.query_parameters.merge(action: :index) + redirect_with_query_params_to(action: :index) end private diff --git a/app/controllers/admin/organizations_controller.rb b/app/controllers/admin/organizations_controller.rb index a001e6875..1a5367286 100644 --- a/app/controllers/admin/organizations_controller.rb +++ b/app/controllers/admin/organizations_controller.rb @@ -19,11 +19,11 @@ class Admin::OrganizationsController < Admin::BaseController def verify @organization.verify - redirect_to request.query_parameters.merge(action: :index) + redirect_with_query_params_to(action: :index) end def reject @organization.reject - redirect_to request.query_parameters.merge(action: :index) + redirect_with_query_params_to(action: :index) end end diff --git a/app/controllers/admin/proposal_notifications_controller.rb b/app/controllers/admin/proposal_notifications_controller.rb index a591eef83..cff3d6f9d 100644 --- a/app/controllers/admin/proposal_notifications_controller.rb +++ b/app/controllers/admin/proposal_notifications_controller.rb @@ -12,14 +12,14 @@ class Admin::ProposalNotificationsController < Admin::BaseController def confirm_hide @proposal_notification.confirm_hide - redirect_to request.query_parameters.merge(action: :index) + redirect_with_query_params_to(action: :index) end def restore @proposal_notification.restore @proposal_notification.ignore_flag Activity.log(current_user, :restore, @proposal_notification) - redirect_to request.query_parameters.merge(action: :index) + redirect_with_query_params_to(action: :index) end private diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 3c0f27c98..c560ee5ed 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -120,4 +120,11 @@ class ApplicationController < ActionController::Base def current_budget Budget.current end + + def redirect_with_query_params_to(options, response_status = {}) + path_options = { controller: params[:controller] }.merge(options).merge(only_path: true) + path = url_for(request.query_parameters.merge(path_options)) + + redirect_to path, response_status + end end diff --git a/app/controllers/concerns/moderate_actions.rb b/app/controllers/concerns/moderate_actions.rb index d6c35e777..a96c2d6a6 100644 --- a/app/controllers/concerns/moderate_actions.rb +++ b/app/controllers/concerns/moderate_actions.rb @@ -28,7 +28,7 @@ module ModerateActions User.where(id: author_ids).accessible_by(current_ability, :block).each { |user| block_user user } end - redirect_to request.query_parameters.merge(action: :index) + redirect_with_query_params_to(action: :index) end private diff --git a/app/controllers/debates_controller.rb b/app/controllers/debates_controller.rb index ee2103ee2..4d4eb0e1d 100644 --- a/app/controllers/debates_controller.rb +++ b/app/controllers/debates_controller.rb @@ -37,12 +37,12 @@ class DebatesController < ApplicationController def unmark_featured @debate.update!(featured_at: nil) - redirect_to request.query_parameters.merge(action: :index) + redirect_to debates_path end def mark_featured @debate.update!(featured_at: Time.current) - redirect_to request.query_parameters.merge(action: :index) + redirect_to debates_path end def disable_recommendations diff --git a/app/controllers/images_controller.rb b/app/controllers/images_controller.rb index 665c7ce64..2e1093f0b 100644 --- a/app/controllers/images_controller.rb +++ b/app/controllers/images_controller.rb @@ -11,7 +11,7 @@ class ImagesController < ApplicationController else flash[:alert] = t "images.actions.destroy.alert" end - redirect_to params[:from] + redirect_to request.referer end format.js do if @image.destroy diff --git a/app/controllers/moderation/users_controller.rb b/app/controllers/moderation/users_controller.rb index d0ee29f83..f07268348 100644 --- a/app/controllers/moderation/users_controller.rb +++ b/app/controllers/moderation/users_controller.rb @@ -9,7 +9,7 @@ class Moderation::UsersController < Moderation::BaseController def hide_in_moderation_screen block_user - redirect_to request.query_parameters.merge(action: :index), notice: I18n.t("moderation.users.notice_hide") + redirect_with_query_params_to({ action: :index }, { notice: I18n.t("moderation.users.notice_hide") }) end def hide diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 255534a88..44b5410cc 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -3,7 +3,7 @@ module ApplicationHelper # notice: if query_params have a param which also exist in current path, # it "overrides" (query_params is merged last) def current_path_with_query_params(query_parameters) - url_for(request.query_parameters.merge(query_parameters)) + url_for(request.query_parameters.merge(query_parameters).merge(only_path: true)) end def markdown(text) diff --git a/app/views/budgets/investments/_investment_show.html.erb b/app/views/budgets/investments/_investment_show.html.erb index 98d18e655..e112342b7 100644 --- a/app/views/budgets/investments/_investment_show.html.erb +++ b/app/views/budgets/investments/_investment_show.html.erb @@ -26,7 +26,7 @@

<%= t("budgets.investments.show.author") %>

- <%= link_to image_path(investment.image, from: request.url), + <%= link_to image_path(investment.image), method: :delete, class: "button hollow alert expanded" do %> diff --git a/spec/controllers/admin/hidden_debates_controller_spec.rb b/spec/controllers/admin/hidden_debates_controller_spec.rb new file mode 100644 index 000000000..c55617a94 --- /dev/null +++ b/spec/controllers/admin/hidden_debates_controller_spec.rb @@ -0,0 +1,25 @@ +require "rails_helper" + +describe Admin::HiddenDebatesController do + before { sign_in create(:administrator).user } + + describe "PUT confirm_hide" do + it "keeps query parameters while using protected redirects" do + debate = create(:debate, :hidden) + + get :confirm_hide, params: { id: debate, filter: "all", host: "evil.dev" } + + expect(response).to redirect_to "/admin/hidden_debates?filter=all" + end + end + + describe "PUT restore" do + it "keeps query parameters while using protected redirects" do + debate = create(:debate, :hidden, :with_confirmed_hide) + + get :restore, params: { id: debate, filter: "all", host: "evil.dev" } + + expect(response).to redirect_to "/admin/hidden_debates?filter=all" + end + end +end diff --git a/spec/controllers/admin/organizations_controller_spec.rb b/spec/controllers/admin/organizations_controller_spec.rb new file mode 100644 index 000000000..bafa1819c --- /dev/null +++ b/spec/controllers/admin/organizations_controller_spec.rb @@ -0,0 +1,25 @@ +require "rails_helper" + +describe Admin::OrganizationsController do + before { sign_in create(:administrator).user } + + describe "PUT verify" do + it "keeps query parameters while using protected redirects" do + organization = create(:organization) + + get :verify, params: { id: organization, filter: "pending", host: "evil.dev" } + + expect(response).to redirect_to "/admin/organizations?filter=pending" + end + end + + describe "PUT reject" do + it "keeps query parameters while using protected redirects" do + organization = create(:organization) + + get :reject, params: { id: organization, filter: "pending", host: "evil.dev" } + + expect(response).to redirect_to "/admin/organizations?filter=pending" + end + end +end diff --git a/spec/controllers/debates_controller_spec.rb b/spec/controllers/debates_controller_spec.rb index af108f906..3f8774ce6 100644 --- a/spec/controllers/debates_controller_spec.rb +++ b/spec/controllers/debates_controller_spec.rb @@ -50,4 +50,15 @@ describe DebatesController do end.not_to change { debate.reload.votes_for.size } end end + + describe "PUT mark_featured" do + it "ignores query parameters" do + debate = create(:debate) + sign_in create(:administrator).user + + get :mark_featured, params: { id: debate, controller: "proposals" } + + expect(response).to redirect_to debates_path + end + end end diff --git a/spec/controllers/images_controller_spec.rb b/spec/controllers/images_controller_spec.rb new file mode 100644 index 000000000..5b83ac1c8 --- /dev/null +++ b/spec/controllers/images_controller_spec.rb @@ -0,0 +1,17 @@ +require "rails_helper" + +describe ImagesController do + let(:user) { create(:user) } + before { sign_in user } + + describe "DELETE destroy" do + it "redirects to the referer URL" do + image = create(:image, imageable: create(:proposal, author: user)) + request.env["HTTP_REFERER"] = "/proposals" + + delete :destroy, params: { id: image, from: "http://evil.dev" } + + expect(response).to redirect_to "/proposals" + end + end +end diff --git a/spec/controllers/moderation/budgets/investments_controller_spec.rb b/spec/controllers/moderation/budgets/investments_controller_spec.rb new file mode 100644 index 000000000..eacba5a2e --- /dev/null +++ b/spec/controllers/moderation/budgets/investments_controller_spec.rb @@ -0,0 +1,15 @@ +require "rails_helper" + +describe Moderation::Budgets::InvestmentsController do + before { sign_in create(:moderator).user } + + describe "PUT moderate" do + it "keeps query parameters while using protected redirects" do + id = create(:budget_investment).id + + get :moderate, params: { resource_ids: [id], filter: "all", host: "evil.dev" } + + expect(response).to redirect_to "/moderation/budget_investments?filter=all&resource_ids%5B%5D=#{id}" + end + end +end diff --git a/spec/controllers/moderation/users_controller_spec.rb b/spec/controllers/moderation/users_controller_spec.rb new file mode 100644 index 000000000..424e41c8b --- /dev/null +++ b/spec/controllers/moderation/users_controller_spec.rb @@ -0,0 +1,15 @@ +require "rails_helper" + +describe Moderation::UsersController do + before { sign_in create(:moderator).user } + + describe "PUT hide_in_moderation_screen" do + it "keeps query parameters while using protected redirects" do + user = create(:user, email: "user@consul.dev") + + get :hide_in_moderation_screen, params: { id: user, name_or_email: "user@consul.dev", host: "evil.dev" } + + expect(response).to redirect_to "/moderation/users?name_or_email=user%40consul.dev" + end + end +end diff --git a/spec/features/localization_spec.rb b/spec/features/localization_spec.rb index 47cf53528..4056cb8dd 100644 --- a/spec/features/localization_spec.rb +++ b/spec/features/localization_spec.rb @@ -40,6 +40,15 @@ describe "Localization" do expect(page).to have_select("locale-switcher", selected: "Español") end + scenario "Keeps query parameters while using protected redirects", :js do + visit "/debates?order=created_at&host=evil.dev" + + select("Español", from: "locale-switcher") + + expect(current_host).to eq "http://127.0.0.1" + expect(page).to have_current_path "/debates?locale=es&order=created_at" + end + context "Only one locale" do before do allow(I18n).to receive(:available_locales).and_return([:en])