diff --git a/CHANGELOG.rdoc b/CHANGELOG.rdoc index 0fdcb749..f4ab2aa6 100644 --- a/CHANGELOG.rdoc +++ b/CHANGELOG.rdoc @@ -1,3 +1,11 @@ +* enhancements + * Ensure fail! works inside strategies + * Make unauthenticated message (when you haven't logged in) different from + invalid message (when your credentials are invalid) + +* bug fix + * Do not redirect on invalid authenticate + == 0.2.2 * bug fix diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index 2ae35339..b02f1985 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -1,12 +1,19 @@ class SessionsController < ApplicationController include Devise::Controllers::Helpers + # Maps the messages types that comes from warden to a flash type. + WARDEN_MESSAGES = { + :unauthenticated => :success, + :unconfirmed => :failure + } + before_filter :require_no_authentication, :only => [ :new, :create ] # GET /resource/sign_in def new - unauthenticated! if params[:unauthenticated] - unconfirmed! if params[:unconfirmed] + WARDEN_MESSAGES.each do |message, type| + set_now_flash_message type, message if params.key?(message) + end build_resource end @@ -16,7 +23,7 @@ class SessionsController < ApplicationController set_flash_message :success, :signed_in redirect_back_or_to home_or_root_path else - unauthenticated! + set_now_flash_message :failure, :invalid build_resource render :new end @@ -29,14 +36,4 @@ class SessionsController < ApplicationController redirect_to root_path end - protected - - def unauthenticated! - set_now_flash_message :failure, :unauthenticated - end - - def unconfirmed! - set_now_flash_message :failure, :unconfirmed - end - end diff --git a/config/locales/en.yml b/config/locales/en.yml index 982de5a0..2702042e 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -3,8 +3,9 @@ en: sessions: signed_in: 'Signed in successfully.' signed_out: 'Signed out successfully.' - unauthenticated: 'Invalid email or password.' + unauthenticated: 'You need to sign in or sign up before continuing.' unconfirmed: 'Your account was not confirmed and your confirmation period has expired.' + invalid: 'Invalid email or password.' passwords: send_instructions: 'You will receive an email with instructions about how to reset your password in a few minutes.' updated: 'Your password was changed successfully. You are now signed in.' diff --git a/lib/devise/failure.rb b/lib/devise/failure.rb index 5e8133cf..52b89027 100644 --- a/lib/devise/failure.rb +++ b/lib/devise/failure.rb @@ -8,8 +8,12 @@ module Devise # to the default_url. def self.call(env) options = env['warden.options'] - params = options[:params] || {} scope = options[:scope] + params = if env['warden'].try(:message) + { env['warden'].message => true } + else + options[:params] + end redirect_path = if mapping = Devise.mappings[scope] "/#{mapping.as}/#{mapping.path_names[:sign_in]}" @@ -19,7 +23,7 @@ module Devise headers = {} headers["Location"] = redirect_path - headers["Location"] << "?" << Rack::Utils.build_query(params) unless params.empty? + headers["Location"] << "?" << Rack::Utils.build_query(params) if params headers["Content-Type"] = 'text/plain' message = options[:message] || "You are being redirected to #{redirect_path}" diff --git a/lib/devise/strategies/authenticable.rb b/lib/devise/strategies/authenticable.rb index 515b58c5..7db53a53 100644 --- a/lib/devise/strategies/authenticable.rb +++ b/lib/devise/strategies/authenticable.rb @@ -7,12 +7,16 @@ module Devise # Authenticate a user based on email and password params, returning to warden # success and the authenticated user if everything is okay. Otherwise redirect # to sign in page. + # + # Please notice the semantic difference between calling fail! and throw :warden. + # The first does not perform any action when calling authenticate, just + # when authenticate! is invoked. The second always perform the action. def authenticate! if valid_attributes? && resource = mapping.to.authenticate(attributes) success!(resource) else store_location - throw :warden, :scope => scope, :params => { :unauthenticated => true } + fail!(:unauthenticated) end end diff --git a/test/integration/authenticable_test.rb b/test/integration/authenticable_test.rb index d16c532c..f58ecc39 100644 --- a/test/integration/authenticable_test.rb +++ b/test/integration/authenticable_test.rb @@ -81,8 +81,6 @@ class AuthenticationTest < ActionController::IntegrationTest fill_in 'email', :with => 'wrongemail@test.com' end - assert_redirected_to new_admin_session_path(:unauthenticated => true) - follow_redirect! assert_contain 'Invalid email or password' assert_not warden.authenticated?(:admin) end @@ -92,8 +90,6 @@ class AuthenticationTest < ActionController::IntegrationTest fill_in 'password', :with => 'abcdef' end - assert_redirected_to new_admin_session_path(:unauthenticated => true) - follow_redirect! assert_contain 'Invalid email or password' assert_not warden.authenticated?(:admin) end @@ -101,13 +97,12 @@ class AuthenticationTest < ActionController::IntegrationTest test 'error message is configurable by resource name' do begin I18n.backend.store_translations(:en, :devise => { :sessions => - { :admin => { :unauthenticated => "Invalid credentials" } } }) + { :admin => { :invalid => "Invalid credentials" } } }) sign_in_as_admin do fill_in 'password', :with => 'abcdef' end - follow_redirect! assert_contain 'Invalid credentials' ensure I18n.reload! @@ -145,14 +140,14 @@ class AuthenticationTest < ActionController::IntegrationTest assert_not_contain 'Signed out successfully' end - test 'redirect from warden shows error message' do + test 'redirect from warden shows sign in or sign up message' do get admins_path warden_path = new_admin_session_path(:unauthenticated => true) assert_redirected_to warden_path get warden_path - assert_contain 'Invalid email or password.' + assert_contain 'You need to sign in or sign up before continuing.' end test 'render 404 on roles without permission' do diff --git a/test/test_helper.rb b/test/test_helper.rb index 35895cbd..16762141 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -13,6 +13,7 @@ ActionMailer::Base.default_url_options[:host] = 'test.com' ActiveRecord::Migration.verbose = false ActiveRecord::Base.logger = Logger.new(nil) ActiveRecord::Base.establish_connection(:adapter => "sqlite3", :database => ":memory:") + ActiveRecord::Schema.define(:version => 1) do [:users, :admins].each do |table| create_table table do |t|