Permalink
Browse files

Authentication redirection bug-fix which fixes #1561

* Fixed post-authentication redirect bug where users would
  always be redirected to the admin root after successful auth.
* Added tests for proper functionality.
  • Loading branch information...
1 parent ecfbfe8 commit 0b3b7a23e8a1fdcc0dd0f7dacf5f257fc8404b0d @wenzowski wenzowski committed with parndt Jun 3, 2012
@@ -7,19 +7,47 @@ 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 defines the devise method for refinery routes
+ def signed_in_root_path(resource_or_scope)
+ scope = Devise::Mapping.find_scope!(resource_or_scope)
+ home_path = "#{scope}_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
+
+ # Pops the stored url, trims the sneaky "//" from it, and returns it.
+ #
+ # Making sure bad urls aren't stored in the first place should probably be
+ # a part of the Devise::FailureApp
+ def sanitized_stored_location_for(resource_or_scope)
+ # `stored_location_for` is the devise method that pops the
+ # scoped `return_to` key
+ location = stored_location_for(resource_or_scope)
+ location.sub!("//", "/") if location.respond_to?(:sub!)
+ location
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)
- 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
+ sanitized_stored_location_for(resource_or_scope) ||
+ signed_in_root_path(resource_or_scope)
end
def after_sign_out_path_for(resource_or_scope)
@@ -30,7 +58,8 @@ def refinery_user?
refinery_user_signed_in? && current_refinery_user.has_role?(:refinery)
end
- protected :store_location, :redirect_back_or_default, :refinery_user?
+ protected :store_location, :pop_stored_location, :redirect_back_or_default,
+ :sanitized_stored_location_for, :refinery_user?
def self.included(base)
if base.respond_to? :helper_method
@@ -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
View
@@ -15,6 +15,7 @@
* Added `Refinery::Page#canonical` support which allows multiple translations to have one canonical version. [Philip Arndt](https://github.com/parndt)
* Usernames are validated case insensitively to ensure true uniqueness. [#1703](https://github.com/resolve/refinerycms/issues/1703). [Philip Arndt](https://github.com/parndt)
* Fixed bug with template selector for page where it would always default to parents template. [#1710](https://github.com/resolve/refinerycms/issues/1710). [Glenn Hoppe](https://github.com/ghoppe)
+* Fixed and added tests for post-authentication redirect bug where a user would always be redirected to the admin root after successful auth. [#1561](https://github.com/resolve/refinerycms/issues/1561). [Alexander Wenzowski](https://github.com/wenzowski)
## 2.0.4 [14 May 2012]
* IMPORTANT: Fixed a security issue whereby the user could bypass some access restrictions in the backend. [#1636](https://github.com/resolve/refinerycms/pull/1636). [Rob Yurkowski](https://github.com/robyurkowski) and [Uģis Ozols](https://github.com/ugisozols)

0 comments on commit 0b3b7a2

Please sign in to comment.