Raise an exception on open redirects

This way we'll add an extra layer of protection from attacks that might
cause our application to redirect to an external host.

There's one place where we're allowing redirects to external hosts,
though: administrators can link external resources in notifications, and
we're redirecting to them after marking the notification as read.

Since the tests for the remote translations controller were
(accidentally) using an external redirect, we're updating them to use a
relative URL.
This commit is contained in:
Javi Martín
2024-03-28 02:59:07 +01:00
parent 9b4525ac71
commit cc628f0363
3 changed files with 4 additions and 4 deletions

View File

@@ -11,7 +11,7 @@ class NotificationsController < ApplicationController
def show
@notification = current_user.notifications.find(params[:id])
@notification.mark_as_read
redirect_to linkable_resource_path(@notification)
redirect_to linkable_resource_path(@notification), allow_other_host: true
end
def read

View File

@@ -61,7 +61,7 @@ Rails.application.config.active_record.verify_foreign_keys_for_fixtures = true
Rails.application.config.active_record.partial_inserts = false
# Protect from open redirect attacks in `redirect_back_or_to` and `redirect_to`.
# Rails.application.config.action_controller.raise_on_open_redirects = true
Rails.application.config.action_controller.raise_on_open_redirects = true
# Change the variant processor for Active Storage.
# Changing this default means updating all places in your code that

View File

@@ -11,7 +11,7 @@ describe RemoteTranslationsController, :remote_translations do
end
before do
request.env["HTTP_REFERER"] = "any_path"
request.env["HTTP_REFERER"] = "/any_path"
end
it "create correctly remote translation" do
@@ -39,7 +39,7 @@ describe RemoteTranslationsController, :remote_translations do
it "redirect_to request referer after create" do
post :create, params: { remote_translations: remote_translations_params }
expect(subject).to redirect_to("any_path")
expect(subject).to redirect_to "/any_path"
end
end
end