Remove support for using request.referer as redirect URL. (#5)

* Remove support for using `request.referer` as URL.

Redirects preserve the original `HTTP_REFERER` which isn't what we want.

You must either use `params[:redirect_url]` or pass an explicit URL with something like this:

```ruby
stash_redirect_for :sign_in, on: :new, url: :root_url
stash_redirect_for :sign_in, on: :new, url: -> { request.url }
```

* Remove use of referer and cut down some glue code

* People will be passing a block to this most likely

Author: kaspth

README.md

@@ -12,13 +12,14 @@ class ApplicationController < ActionController::Base
 
   private
     def authenticate
-      redirect_to new_session_url unless Current.user
+      # Pass `redirect_url:` to pass the URL we're currently on.
+      redirect_to new_session_url(redirect_url: request.url) unless Current.user
     end
 end
 
 class SessionsController < ApplicationController
   # Stash a redirect at the start of the session authentication flow,
-  # from either params[:redirect_url] or request.referer in that order.
+  # from `params[:redirect_url]` automatically.
   stash_redirect_for :sign_in, on: :new
 
   def new

lib/action_controller/stashed_redirects.rb

@@ -29,10 +29,9 @@ def initialize(purpose)
     #
     #   stash_redirect_for :sign_in, on: :new
     #   stash_redirect_for :sign_in, on: %i[ new edit ]
-    #   stash_redirect_for :sign_in, on: :new, from: :referer
-    #   stash_redirect_for :sign_in, on: :new, from: -> { update_post_path(@post) }
-    def stash_redirect_for(purpose, on:, from: DEFAULT_FROM)
-      before_action(-> { stash_redirect_for(purpose, from: from.respond_to?(:call) ? instance_exec(&from) : from) }, only: on)
+    #   stash_redirect_for :sign_in, on: :new, url: -> { update_post_path(@post) }
+    def stash_redirect_for(purpose, on:, url: DEFAULT_URL)
+      before_action(-> { stash_redirect_for(purpose, url: url) }, only: on)
     end
   end
 
@@ -46,8 +45,8 @@ def stash_redirect_for(purpose, on:, from: DEFAULT_FROM)
     #   stash_redirect_for :sign_in, from: url_from(params[:redirect_url]) || root_url
     #   stash_redirect_for :sign_in, from: :param   # Only derive the redirect URL from `params[:redirect_url]`.
     #   stash_redirect_for :sign_in, from: :referer # Only derive the redirect URL from `request.referer`.
-    def stash_redirect_for(purpose, from: DEFAULT_FROM)
-      if url = derive_stash_redirect_url_from(from)
+    def stash_redirect_for(purpose, url: DEFAULT_URL)
+      if url = derive_stash_redirect_url_from(url)
         session[KEY_GENERATOR.(purpose)] = url
       else
         raise ArgumentError, "missing a redirect_url to stash, pass one via from: or via a redirect_url URL param"
@@ -76,11 +75,12 @@ def stashed_redirect_url_for(purpose)
       url_from(discard_stashed_redirect_for(purpose)) or raise MissingRedirectError, purpose
     end
 
-    def derive_stash_redirect_url_from(from)
-      from = %i[ param referer ] if from == DEFAULT_FROM
-      possible_urls = { param: params[:redirect_url], referer: request.get? && request.referer }
-
-      url_from(possible_urls.values_at(*from).find(&:present?) || from)
+    def derive_stash_redirect_url_from(url)
+      case url
+      when DEFAULT_URL  then redirect_url
+      when String       then url_from url
+      when Symbol, Proc then url_from instance_exec(self, &url)
+      end
     end
 
     # Looks up a redirect URL from `params[:redirect_url]` using
@@ -91,7 +91,7 @@ def derive_stash_redirect_url_from(from)
     #   redirect_to redirect_url || users_url
     def redirect_url = url_from(params[:redirect_url])
 
-    DEFAULT_FROM = Object.new
+    DEFAULT_URL = Object.new
 
     KEY_GENERATOR = ->(purpose) { "__url_stash_#{purpose}" }
     private_constant :KEY_GENERATOR

test/action_controller/stashed_redirects_test.rb

@@ -15,15 +15,7 @@ class ActionController::StashedRedirectsTest < ActionDispatch::IntegrationTest
     assert_redirected_to users_url
   end
 
-  test "stash and recall redirect from the referer" do
-    get new_session_url, headers: { HTTP_REFERER: users_url }
-    assert_response :no_content
-
-    post sessions_url
-    assert_redirected_to users_url
-  end
-
-  test "passing from: block accesses instance context" do
+  test "passing url: block accesses instance context" do
     delete session_url(id: 1)
     assert_redirected_to users_url
   end
@@ -44,52 +36,28 @@ class ActionController::StashedRedirectsTest < ActionDispatch::IntegrationTest
 
 class ActionController::StashedRedirects::HooksTest < ActiveSupport::TestCase
   module Context
-    def session
-      @session ||= {}
-    end
-
-    def params
-      @params ||= { redirect_url: "/users/param" }
-    end
-
-    def request
-      @request ||= Struct.new(:host, :referer) { def get? = true }.new "http://example.com", "/users/referer"
-    end
+    def request = @request ||= Struct.new(:host).new("http://example.com")
+    def session = @session ||= {}
+    def params  = { redirect_url: "/users/param" }
 
     def redirect_to(url, *) = url
-
-    def url_from(url)
-      if url.present?
-        url if URI(url.to_s).host.then { _1.nil? || _1 == request.host }
-      end
-    end
+    def url_from(url) = URI(url.to_s).host.then { _1.nil? || _1 == request.host } && url
   end
-
   include Context, ActionController::StashedRedirects
 
-  test "param takes precedence over referer" do
+  test "from redirect_url" do
     stash_redirect_for :sign_in
     assert_equal "/users/param", redirect_from_stashed(:sign_in)
   end
 
-  test "from param" do
-    stash_redirect_for :sign_in, from: :param
-    assert_equal "/users/param", redirect_from_stashed(:sign_in)
-  end
-
-  test "from referer" do
-    stash_redirect_for :sign_in, from: :referer
-    assert_equal "/users/referer", redirect_from_stashed(:sign_in)
-  end
-
   test "explicit url override" do
-    stash_redirect_for :sign_in, from: "/users/explicit"
+    stash_redirect_for :sign_in, url: "/users/explicit"
     assert_equal "/users/explicit", redirect_from_stashed(:sign_in)
   end
 
   test "passing the wrong URL raises" do
-    assert_raises(ArgumentError) { stash_redirect_for :sign_in, from: nil }
-    assert_raises(ArgumentError) { stash_redirect_for :sign_in, from: "http://google.com" }
+    assert_raises(ArgumentError) { stash_redirect_for :sign_in, url: -> { nil } }
+    assert_raises(ArgumentError) { stash_redirect_for :sign_in, url: "http://google.com" }
   end
 
   test "no stashed redirect raises" do

test/boot/action_controller.rb

@@ -4,7 +4,7 @@ class ApplicationController < ActionController::Base
 
 class SessionsController < ApplicationController
   stash_redirect_for :sign_in,  on: :new
-  stash_redirect_for :sign_out, on: :destroy, from: -> { users_url }
+  stash_redirect_for :sign_out, on: :destroy, url: :users_url
 
   def new
     head :no_content