Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

authentication redirection improvement #1724 #1728

Closed
wants to merge 3 commits into from

3 participants

@wenzowski
  • add tests for #1561 behaviour
  • use unscoped session key to enable store_location to be used for login #1724
@wenzowski wenzowski authentication redirection improvement #1724
- add tests for #1561 behaviour
- use unscoped session key to enable store_location to be used for login #1724
47dde82
@parndt
Owner

Thank you - can you please specify all methods that need to be protected as such? Line 59 on your file version

@travisbot

This pull request passes (merged 47dde82 into ecfbfe8).

@wenzowski

@parndt whoops, done

@parndt
Owner

Oh also as this is a bugfix can you please add this to the changelog.md file?

@wenzowski

@parndt explanation added

@travisbot

This pull request passes (merged 1987f58 into ecfbfe8).

@wenzowski wenzowski closed this
@wenzowski

Split this pull into bugfix and feature commits.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jun 4, 2012
  1. @wenzowski

    authentication redirection improvement #1724

    wenzowski authored
    - add tests for #1561 behaviour
    - use unscoped session key to enable store_location to be used for login #1724
  2. @wenzowski

    specify protected methods

    wenzowski authored
  3. @wenzowski
This page is out of date. Refresh to see the latest.
View
41 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)
@@ -30,7 +56,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
View
43 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
View
1  changelog.md
@@ -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 users would always be redirected to the admin root after successful auth. [#1561](https://github.com/resolve/refinerycms/issues/1561), [#1728](https://github.com/resolve/refinerycms/issues/1728). [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)
Something went wrong with that request. Please try again.