authentication redirection improvement #1724

- add tests for #1561 behaviour
- use unscoped session key to enable store_location to be used for login #1724

Author: Alexander Wenzowski

Diff for: authentication/lib/refinery/authenticated_system.rb

@@ -7,19 +7,45 @@ def store_location
       session[:return_to] = request.fullpath.sub("//", "/")
     end
 
+    # Clear and return the stored location
+    def pop_stored_location
+      session.delete(:return_to)
+    end
+
     # Redirect to the URI stored by the most recent store_location call or
     # to the passed default.
     def redirect_back_or_default(default)
-      redirect_to(session[:return_to] || default)
-      session[:return_to] = nil
+      redirect_to(pop_stored_location || default)
     end
 
-    # This just defines the devise method for after sign in to support
-    # extension namespace isolation...
-    def after_sign_in_path_for(resource_or_scope)
+    # Overrides default devise paths with refinery routes
+    def signed_in_root_path(resource_or_scope)
       scope = Devise::Mapping.find_scope!(resource_or_scope)
       home_path = "#{scope}_root_path"
-      respond_to?(home_path, true) ? refinery.send(home_path) : refinery.admin_root_path
+      if respond_to?(home_path, true)
+        refinery.send(home_path)
+      elsif respond_to?(:admin_root_path)
+        refinery.admin_root_path
+      else
+        "/"
+      end
+    end
+
+    # Trims the sneaky "//" from the popped stored url and returns it.
+    #
+    # Making sure bad urls aren't stored should probably be
+    # a part of the Devise::FailureApp
+    def sanitized_stored_location_for(resource_or_scope)
+      location = stored_location_for(resource_or_scope)
+      location.sub!("//", "/") if location.respond_to?(:sub!)
+      location
+    end
+
+    # Adds support for unscoped redirect_back_or_default key to devise default
+    def after_sign_in_path_for(resource_or_scope)
+      pop_stored_location ||
+      sanitized_stored_location_for(resource_or_scope) ||
+      signed_in_root_path(resource_or_scope)
     end
 
     def after_sign_out_path_for(resource_or_scope)

Diff for: authentication/spec/requests/refinery/sessions_spec.rb

@@ -2,11 +2,15 @@
 
 module Refinery
   describe "sign in" do
+    let(:login_path) { refinery.new_refinery_user_session_path }
+    let(:login_retry_path) { refinery.refinery_user_session_path }
+    let(:admin_path) { refinery.admin_root_path }
+
     before(:each) do
       FactoryGirl.create(:refinery_user, :username => "ugisozols",
                               :password => "123456",
                               :password_confirmation => "123456")
-      visit refinery.new_refinery_user_session_path
+      visit login_path
     end
 
     it "shows login form" do
@@ -21,6 +25,7 @@ module Refinery
         fill_in "Password", :with => "123456"
         click_button "Sign in"
         page.should have_content("Signed in successfully.")
+        current_path.should == admin_path
       end
     end
 
@@ -30,6 +35,7 @@ module Refinery
         fill_in "Password", :with => "Hmmm"
         click_button "Sign in"
         page.should have_content("Sorry, your login or password was incorrect.")
+        current_path.should == login_retry_path
       end
     end
   end
@@ -59,4 +65,39 @@ module Refinery
       end
     end
   end
+
+  describe 'redirects' do
+    let(:protected_path) { refinery.new_admin_user_path }
+    let(:login_path) { refinery.new_refinery_user_session_path }
+
+    before(:each) do
+      FactoryGirl.create(:refinery_user,
+        :username => "ugisozols",
+        :password => "123456",
+        :password_confirmation => "123456"
+      )
+    end
+
+    context "when visiting a protected path" do
+      before(:each) { visit protected_path }
+
+      it "redirects to the login" do
+        current_path.should == login_path
+      end
+
+      it "shows login form" do
+        page.should have_content("Hello! Please sign in.")
+        page.should have_content("I forgot my password")
+        page.should have_selector("a[href*='/refinery/users/password/new']")
+      end
+
+      it "redirects to the protected path on login" do
+        fill_in "Login", :with => "ugisozols"
+        fill_in "Password", :with => "123456"
+        page.click_button "Sign in"
+        current_path.should == protected_path
+      end
+    end
+
+  end
 end