From 356b09431274c2c97a02376655278ea7414ebc1a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20Graakj=C3=A6r=20Grantzau?= Date: Wed, 2 Jan 2019 15:43:30 +0100 Subject: [PATCH] Downcase authentication keys and humanize error message (#4834) "Invalid Email or password." is grammatically incorrect, a change introduced a while ago by #4014. Signed-off-by: Carlos Antonio da Silva --- lib/devise/failure_app.rb | 8 ++++--- test/failure_app_test.rb | 24 +++++++++++++------ test/integration/authenticatable_test.rb | 4 ++-- test/integration/confirmable_test.rb | 2 +- .../database_authenticatable_test.rb | 4 ++-- test/integration/http_authenticatable_test.rb | 2 +- 6 files changed, 28 insertions(+), 16 deletions(-) diff --git a/lib/devise/failure_app.rb b/lib/devise/failure_app.rb index d0b50f7d..2f3e11e5 100644 --- a/lib/devise/failure_app.rb +++ b/lib/devise/failure_app.rb @@ -111,11 +111,13 @@ module Devise options[:scope] = "devise.failure" options[:default] = [message] auth_keys = scope_class.authentication_keys - keys = (auth_keys.respond_to?(:keys) ? auth_keys.keys : auth_keys).map { |key| scope_class.human_attribute_name(key) } + keys = (auth_keys.respond_to?(:keys) ? auth_keys.keys : auth_keys).map { |key| scope_class.human_attribute_name(key).downcase } options[:authentication_keys] = keys.join(I18n.t(:"support.array.words_connector")) options = i18n_options(options) - - I18n.t(:"#{scope}.#{message}", **options) + translated_message = I18n.t(:"#{scope}.#{message}", **options) + # only call `#humanize` when the message is `:invalid` to ensure the original format + # of other messages - like `:does_not_exist` - is kept. + message == :invalid ? translated_message.humanize : translated_message else message.to_s end diff --git a/test/failure_app_test.rb b/test/failure_app_test.rb index e8f316f0..b57f4e42 100644 --- a/test/failure_app_test.rb +++ b/test/failure_app_test.rb @@ -184,17 +184,27 @@ class FailureTest < ActiveSupport::TestCase test 'uses the proxy failure message as symbol' do call_failure('warden' => OpenStruct.new(message: :invalid)) - assert_equal 'Invalid Email or password.', @request.flash[:alert] + assert_equal 'Invalid email or password.', @request.flash[:alert] assert_equal 'http://test.host/users/sign_in', @response.second["Location"] end test 'supports authentication_keys as a Hash for the flash message' do swap Devise, authentication_keys: { email: true, login: true } do call_failure('warden' => OpenStruct.new(message: :invalid)) - assert_equal 'Invalid Email, Login or password.', @request.flash[:alert] + assert_equal 'Invalid email, login or password.', @request.flash[:alert] end end + test 'downcases authentication_keys for the flash message' do + call_failure('warden' => OpenStruct.new(message: :invalid)) + assert_equal 'Invalid email or password.', @request.flash[:alert] + end + + test 'humanizes the flash message' do + call_failure('warden' => OpenStruct.new(message: :invalid)) + assert_equal @request.flash[:alert], @request.flash[:alert].humanize + end + test 'uses custom i18n options' do call_failure('warden' => OpenStruct.new(message: :does_not_exist), app: FailureWithI18nOptions) assert_equal 'User Steve does not exist', @request.flash[:alert] @@ -288,7 +298,7 @@ class FailureTest < ActiveSupport::TestCase test 'uses the failure message as response body' do call_failure('formats' => Mime[:xml], 'warden' => OpenStruct.new(message: :invalid)) - assert_match 'Invalid Email or password.', @response.third.body + assert_match 'Invalid email or password.', @response.third.body end test 'respects the i18n locale passed via warden options when responding to HTTP request' do @@ -343,7 +353,7 @@ class FailureTest < ActiveSupport::TestCase } call_failure(env) assert_includes @response.third.body, '

Log in

' - assert_includes @response.third.body, 'Invalid Email or password.' + assert_includes @response.third.body, 'Invalid email or password.' end test 'calls the original controller if not confirmed email' do @@ -378,7 +388,7 @@ class FailureTest < ActiveSupport::TestCase } call_failure(env) assert_includes @response.third.body, '

Log in

' - assert_includes @response.third.body, 'Invalid Email or password.' + assert_includes @response.third.body, 'Invalid email or password.' assert_equal '/sample', @request.env["SCRIPT_NAME"] assert_equal '/users/sign_in', @request.env["PATH_INFO"] end @@ -409,7 +419,7 @@ class FailureTest < ActiveSupport::TestCase call_failure(env) assert_equal 422, @response.first - assert_includes @response.third.body, 'Invalid Email or password.' + assert_includes @response.third.body, 'Invalid email or password.' end end @@ -435,7 +445,7 @@ class FailureTest < ActiveSupport::TestCase call_failure(env) assert_equal 200, @response.first - assert_includes @response.third.body, 'Invalid Email or password.' + assert_includes @response.third.body, 'Invalid email or password.' end test 'users default hardcoded responder `redirect_status` for the status code since responders version does not support configuring it' do diff --git a/test/integration/authenticatable_test.rb b/test/integration/authenticatable_test.rb index ea338f6f..28d00399 100644 --- a/test/integration/authenticatable_test.rb +++ b/test/integration/authenticatable_test.rb @@ -563,7 +563,7 @@ class AuthenticationKeysTest < Devise::IntegrationTest test 'missing authentication keys cause authentication to abort' do swap Devise, authentication_keys: [:subdomain] do sign_in_as_user - assert_contain "Invalid Subdomain or password." + assert_contain "Invalid subdomain or password." assert_not warden.authenticated?(:user) end end @@ -602,7 +602,7 @@ class AuthenticationRequestKeysTest < Devise::IntegrationTest swap Devise, request_keys: [:subdomain] do sign_in_as_user - assert_contain "Invalid Email or password." + assert_contain "Invalid email or password." assert_not warden.authenticated?(:user) end end diff --git a/test/integration/confirmable_test.rb b/test/integration/confirmable_test.rb index c29d7aba..8e6f68ef 100644 --- a/test/integration/confirmable_test.rb +++ b/test/integration/confirmable_test.rb @@ -151,7 +151,7 @@ class ConfirmationTest < Devise::IntegrationTest fill_in 'password', with: 'invalid' end - assert_contain 'Invalid Email or password' + assert_contain 'Invalid email or password' assert_not warden.authenticated?(:user) end end diff --git a/test/integration/database_authenticatable_test.rb b/test/integration/database_authenticatable_test.rb index 20097a87..08011fe2 100644 --- a/test/integration/database_authenticatable_test.rb +++ b/test/integration/database_authenticatable_test.rb @@ -70,7 +70,7 @@ class DatabaseAuthenticationTest < Devise::IntegrationTest fill_in 'password', with: 'abcdef' end - assert_contain 'Invalid Email or password' + assert_contain 'Invalid email or password' assert_not warden.authenticated?(:admin) end @@ -82,7 +82,7 @@ class DatabaseAuthenticationTest < Devise::IntegrationTest end assert_not_contain 'Not found in database' - assert_contain 'Invalid Email or password.' + assert_contain 'Invalid email or password.' end end end diff --git a/test/integration/http_authenticatable_test.rb b/test/integration/http_authenticatable_test.rb index 707a0705..11e37332 100644 --- a/test/integration/http_authenticatable_test.rb +++ b/test/integration/http_authenticatable_test.rb @@ -52,7 +52,7 @@ class HttpAuthenticationTest < Devise::IntegrationTest sign_in_as_new_user_with_http("unknown") assert_equal 401, status assert_equal "application/json; charset=utf-8", headers["Content-Type"] - assert_match '"error":"Invalid Email or password."', response.body + assert_match '"error":"Invalid email or password."', response.body end test 'returns a custom response with www-authenticate and chosen realm' do