Permalink
Browse files

Redirect to the previous URL on timeout, closes #1596

  • Loading branch information...
josevalim committed Jan 24, 2012
1 parent 477d9fb commit 275c480f89f814af7135093a507ab57397729faf
Showing with 39 additions and 6 deletions.
  1. +1 −0 CHANGELOG.rdoc
  2. +20 −2 lib/devise/failure_app.rb
  3. +18 −4 test/integration/timeoutable_test.rb
View
@@ -9,6 +9,7 @@ Notes: https://github.com/plataformatec/devise/wiki/How-To:-Upgrade-to-Devise-2.
* Do not run validations unless on reconfirmable branch
* enhancements
+ * Redirect to the previous URL on timeout
* Inherit from the same Devise parent controller (by @sj26)
* Allow parent_controller to be customizable via Devise.parent_controller, useful for engines
* Allow router_name to be customizable via Devise.router_name, useful for engines
View
@@ -53,14 +53,19 @@ def recall
def redirect
store_location!
- flash[:alert] = i18n_message
+ if flash[:timedout] && flash[:alert]
+ flash.keep(:timedout)
+ flash.keep(:alert)
+ else
+ flash[:alert] = i18n_message
+ end
redirect_to redirect_url
end
protected
def i18n_message(default = nil)
- message = warden.message || warden_options[:message] || default || :unauthenticated
+ message = warden_message || default || :unauthenticated
if message.is_a?(Symbol)
I18n.t(:"#{scope}.#{message}", :resource_name => scope,
@@ -71,6 +76,15 @@ def i18n_message(default = nil)
end
def redirect_url
+ if warden_message == :timeout
+ flash[:timedout] = true

This comment has been minimized.

Show comment
Hide comment
@thrillcall

thrillcall Feb 2, 2012

This line kind of tripped us up a bit when we upgraded to Devise 2.0. Evertime we visit a page after our session has timed out we have been receiving two flash messages. One that is expected "Your session expired, please sign in again to continue." and one that is not, just the word : "true". In our app we have a helper that displays flash messages, even non-standard ones, and this :timedout flash message was always displaying. Is it needed? Or do we need to remove that extra flash message handling from our app to fix the issue?

@thrillcall

thrillcall Feb 2, 2012

This line kind of tripped us up a bit when we upgraded to Devise 2.0. Evertime we visit a page after our session has timed out we have been receiving two flash messages. One that is expected "Your session expired, please sign in again to continue." and one that is not, just the word : "true". In our app we have a helper that displays flash messages, even non-standard ones, and this :timedout flash message was always displaying. Is it needed? Or do we need to remove that extra flash message handling from our app to fix the issue?

This comment has been minimized.

Show comment
Hide comment
@josevalim

josevalim Feb 2, 2012

Member

Yes, it is needed. Flash is not only used for storing messages, you should probably check if something is a string or responds to to_str before printing it.

@josevalim

josevalim Feb 2, 2012

Member

Yes, it is needed. Flash is not only used for storing messages, you should probably check if something is a string or responds to to_str before printing it.

+ attempted_path || scope_path
+ else
+ scope_path
+ end
+ end
+
+ def scope_path
opts = {}
route = :"new_#{scope}_session_path"
opts[:format] = request_format unless skip_format?
@@ -139,6 +153,10 @@ def warden_options
env['warden.options']
end
+ def warden_message
+ @message ||= warden.message || warden_options[:message]
+ end
+
def scope
@scope ||= warden_options[:scope] || Devise.default_scope
end
@@ -41,7 +41,7 @@ def last_request_at
assert_not_nil last_request_at
get users_path
- assert_redirected_to new_user_session_path
+ assert_redirected_to users_path
assert_not warden.authenticated?(:user)
end
@@ -68,12 +68,25 @@ def last_request_at
get expire_user_path(user)
get users_path
- assert_redirected_to new_user_session_path
+ assert_redirected_to users_path
assert_not warden.authenticated?(:user)
end
end
test 'error message with i18n' do
+ store_translations :en, :devise => {
+ :failure => { :user => { :timeout => 'Session expired!' } }
+ } do
+ user = sign_in_as_user
+
+ get expire_user_path(user)
+ get root_path
+ follow_redirect!
+ assert_contain 'Session expired!'
+ end
+ end
+
+ test 'error message with i18n with double redirect' do
store_translations :en, :devise => {
:failure => { :user => { :timeout => 'Session expired!' } }
} do
@@ -82,15 +95,16 @@ def last_request_at
get expire_user_path(user)
get users_path
follow_redirect!
+ follow_redirect!
assert_contain 'Session expired!'
end
end
-
+
test 'time out not triggered if remembered' do
user = sign_in_as_user :remember_me => true
get expire_user_path(user)
assert_not_nil last_request_at
-
+
get users_path
assert_response :success
assert warden.authenticated?(:user)

0 comments on commit 275c480

Please sign in to comment.