Use hash-based flash objects to render complex flash messages

Since flash objects can only be String, Hash or Array (notably excluding SafeBuffers), then this approach is necessary to render complex html in a safe manner.

Each local can be treated as an (unsafe) string, and therefore escaped normally when rendered into the template. The template (and translation strings) can
contain html since they are no longer stored in the flash as a plain string.

Fixes #3215

diff --git a/app/controllers/confirmations_controller.rb b/app/controllers/confirmations_controller.rb
index f9de54df0aa20559ada7d2e3b30bbe0be2bebe26..372ac2a70c0dc2277902fc43f71d3bee25a94fdc 100644 (file)
--- a/app/controllers/confirmations_controller.rb
+++ b/app/controllers/confirmations_controller.rb
@@ -66,7 +66,7 @@ class ConfirmationsController < ApplicationController
       flash[:error] = t "confirmations.confirm_resend.failure", :name => params[:display_name]
     else
       UserMailer.signup_confirm(user, user.tokens.create).deliver_later
-      flash[:notice] = t "confirmations.confirm_resend.success_html", :email => user.email, :sender => Settings.email_from
+      flash[:notice] = { :partial => "confirmations/resend_success_flash", :locals => { :email => user.email, :sender => Settings.email_from } }
     end
 
     redirect_to login_path

diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb
index ae90d980e81bc3de61da28000617f6103af3551d..faf538fa926917dc429136d1a960eebd4ba7e13e 100644 (file)
--- a/app/helpers/application_helper.rb
+++ b/app/helpers/application_helper.rb
@@ -68,4 +68,14 @@ module ApplicationHelper
 
     data
   end
+
+  # If the flash is a hash, then it will be a partial with a hash of locals, so we can call `render` on that
+  # This allows us to render html into a flash message in a safe manner.
+  def render_flash(flash)
+    if flash.is_a?(Hash)
+      render flash.with_indifferent_access
+    else
+      flash
+    end
+  end
 end

diff --git a/app/views/confirmations/_resend_success_flash.html.erb b/app/views/confirmations/_resend_success_flash.html.erb
new file mode 100644 (file)
index 0000000..1895de7
--- /dev/null
+++ b/app/views/confirmations/_resend_success_flash.html.erb
@@ -0,0 +1 @@
+<%= t "confirmations.confirm_resend.success_html", :email => email, :sender => sender %>

diff --git a/app/views/layouts/_flash.html.erb b/app/views/layouts/_flash.html.erb
index 0f39c4a47f3f7d0df5f415422e4fc749ff5d1615..f8a89d6dfcd97bda3894da52f9ae178d6c4467a2 100644 (file)
--- a/app/views/layouts/_flash.html.erb
+++ b/app/views/layouts/_flash.html.erb
@@ -4,7 +4,7 @@
       <source srcset="<%= image_path "notice.svg" %>" type="image/svg+xml" />
       <%= image_tag("notice.png", :srcset => image_path("notice.svg"), :class => "small_icon", :border => 0) %>
     </picture>
-    <div class="message"><%= flash[:error] %></div>
+    <div class="message"><%= render_flash(flash[:error]) %></div>
   </div>
 <% end %>
 
@@ -14,7 +14,7 @@
       <source srcset="<%= image_path "notice.svg" %>" type="image/svg+xml"></source>
       <%= image_tag("notice.png", :srcset => image_path("notice.svg"), :class => "small_icon", :border => 0) %>
     </picture>
-    <div class="message"><%= flash[:warning] %></div>
+    <div class="message"><%= render_flash(flash[:warning]) %></div>
   </div>
 <% end %>
 
@@ -24,6 +24,6 @@
       <source srcset="<%= image_path "notice.svg" %>" type="image/svg+xml"></source>
       <%= image_tag("notice.png", :srcset => image_path("notice.svg"), :class => "small_icon", :border => 0) %>
     </picture>
-    <div class="message"><%= flash[:notice] %></div>
+    <div class="message"><%= render_flash(flash[:notice]) %></div>
   </div>
 <% end %>

diff --git a/test/controllers/confirmations_controller_test.rb b/test/controllers/confirmations_controller_test.rb
index bae0158cdfddb9e2ed968d1a894686a0f38955c7..f583e5e39ca819e0218a98847d39c2b4c442a085 100644 (file)
--- a/test/controllers/confirmations_controller_test.rb
+++ b/test/controllers/confirmations_controller_test.rb
@@ -211,7 +211,8 @@ class UsersControllerTest < ActionDispatch::IntegrationTest
 
     assert_response :redirect
     assert_redirected_to login_path
-    assert_match(/sent a new confirmation/, flash[:notice])
+    assert_equal("confirmations/resend_success_flash", flash[:notice][:partial])
+    assert_equal({ :email => user.email, :sender => Settings.email_from }, flash[:notice][:locals])
 
     email = ActionMailer::Base.deliveries.last
 

diff --git a/test/system/confirmation_resend.rb b/test/system/confirmation_resend.rb
new file mode 100644 (file)
index 0000000..69103e6
--- /dev/null
+++ b/test/system/confirmation_resend.rb
@@ -0,0 +1,26 @@
+require "application_system_test_case"
+
+class ConfirmationResendSystemTest < ApplicationSystemTestCase
+  def setup
+    @user = build(:user)
+    visit user_new_path
+
+    fill_in "Email", :with => @user.email
+    fill_in "Email Confirmation", :with => @user.email
+    fill_in "Display Name", :with => @user.display_name
+    fill_in "Password", :with => "testtest"
+    fill_in "Confirm Password", :with => "testtest"
+    click_button "Sign Up"
+
+    check "I have read and agree to the above contributor terms"
+    check "I have read and agree to the Terms of Use"
+    click_button "Continue"
+  end
+
+  test "flash message should not contain raw html" do
+    visit user_confirm_resend_path(@user)
+
+    assert page.has_content?("sent a new confirmation")
+    assert_not page.has_content?("<br />")
+  end
+end