From d781dbdd61d1d24ec4828859a28815b55310154d Mon Sep 17 00:00:00 2001 From: Azul Date: Tue, 2 Apr 2013 16:56:11 +0200 Subject: send more meaningful error message on completely failed login attempt --- .../controller_extension/authentication.rb | 16 +++++++++++++- users/config/locales/en.yml | 1 + users/test/functional/sessions_controller_test.rb | 11 ++++++++-- .../test/functional/v1/sessions_controller_test.rb | 16 -------------- users/test/integration/api/account_flow_test.rb | 6 ++++-- users/test/integration/api/login_test.rb | 25 ++++++++++++++++++++++ 6 files changed, 54 insertions(+), 21 deletions(-) create mode 100644 users/test/integration/api/login_test.rb diff --git a/users/app/controllers/controller_extension/authentication.rb b/users/app/controllers/controller_extension/authentication.rb index f2184d9..f0a6564 100644 --- a/users/app/controllers/controller_extension/authentication.rb +++ b/users/app/controllers/controller_extension/authentication.rb @@ -8,13 +8,27 @@ module ControllerExtension::Authentication end def authentication_errors - return unless errors = warden.winning_strategy.try(:message) + return unless attempted_login? + errors = get_warden_errors errors.inject({}) do |translated,err| translated[err.first] = I18n.t(err.last) translated end end + def get_warden_errors + if strategy = warden.winning_strategy + strategy.message + else + { login: :all_strategies_failed } + end + end + + def attempted_login? + request.env['warden.options'] && + request.env['warden.options'][:attempted_path] + end + def logged_in? !!current_user end diff --git a/users/config/locales/en.yml b/users/config/locales/en.yml index 9e7d4b2..2077858 100644 --- a/users/config/locales/en.yml +++ b/users/config/locales/en.yml @@ -6,6 +6,7 @@ en: login: "Login" login_message: "Please login with your account." invalid_user_pass: "Not a valid username/password combination" + all_strategies_failed: "Could not understand your login attempt. Please first send your login and a SRP ephemeral value A and then send the client_auth in the same session (using cookies)." update_login_and_password: "Update Login and Password" cancel_account: "Cancel your account" remove_account: "Remove Account" diff --git a/users/test/functional/sessions_controller_test.rb b/users/test/functional/sessions_controller_test.rb index f99c0d7..b22c3a3 100644 --- a/users/test/functional/sessions_controller_test.rb +++ b/users/test/functional/sessions_controller_test.rb @@ -11,7 +11,6 @@ class SessionsControllerTest < ActionController::TestCase end test "should get login screen" do - request.env['warden'].expects(:winning_strategy) get :new assert_response :success assert_equal "text/html", response.content_type @@ -19,13 +18,13 @@ class SessionsControllerTest < ActionController::TestCase end test "renders json" do - request.env['warden'].expects(:winning_strategy) get :new, :format => :json assert_response :success assert_json_error nil end test "renders warden errors" do + request.env['warden.options'] = {attempted_path: '/1/sessions/asdf.json'} strategy = stub :message => {:field => :translate_me} request.env['warden'].stubs(:winning_strategy).returns(strategy) I18n.expects(:t).with(:translate_me).at_least_once.returns("translation stub") @@ -34,6 +33,14 @@ class SessionsControllerTest < ActionController::TestCase assert_json_error :field => "translation stub" end + test "renders failed attempt message" do + request.env['warden.options'] = {attempted_path: '/1/sessions/asdf.json'} + request.env['warden'].stubs(:winning_strategy).returns(nil) + get :new, :format => :json + assert_response 422 + assert_json_error :login => I18n.t(:all_strategies_failed) + end + # Warden takes care of parsing the params and # rendering the response. So not much to test here. test "should perform handshake" do diff --git a/users/test/functional/v1/sessions_controller_test.rb b/users/test/functional/v1/sessions_controller_test.rb index 535da52..1226c9d 100644 --- a/users/test/functional/v1/sessions_controller_test.rb +++ b/users/test/functional/v1/sessions_controller_test.rb @@ -11,22 +11,6 @@ class V1::SessionsControllerTest < ActionController::TestCase @client_hex = 'a123' end - test "renders json" do - request.env['warden'].expects(:winning_strategy) - get :new, :format => :json - assert_response :success - assert_json_error nil - end - - test "renders warden errors" do - strategy = stub :message => {:field => :translate_me} - request.env['warden'].stubs(:winning_strategy).returns(strategy) - I18n.expects(:t).with(:translate_me).at_least_once.returns("translation stub") - get :new, :format => :json - assert_response 422 - assert_json_error :field => "translation stub" - end - # Warden takes care of parsing the params and # rendering the response. So not much to test here. test "should perform handshake" do diff --git a/users/test/integration/api/account_flow_test.rb b/users/test/integration/api/account_flow_test.rb index e618541..d1a97e9 100644 --- a/users/test/integration/api/account_flow_test.rb +++ b/users/test/integration/api/account_flow_test.rb @@ -75,7 +75,8 @@ class AccountFlowTest < ActiveSupport::TestCase test "signup and wrong password login attempt" do srp = SRP::Client.new @login, :password => "wrong password" server_auth = srp.authenticate(self) - assert_json_error({:login => "Not a valid username/password combination", :password => "Not a valid username/password combination"}) + assert_json_error login: "Not a valid username/password combination", + password: "Not a valid username/password combination" assert !last_response.successful? assert_nil server_auth["M2"] end @@ -86,7 +87,8 @@ class AccountFlowTest < ActiveSupport::TestCase assert_raises RECORD_NOT_FOUND do server_auth = srp.authenticate(self) end - assert_json_error({:login => "Not a valid username/password combination", :password => "Not a valid username/password combination"}) + assert_json_error login: "Not a valid username/password combination", + password: "Not a valid username/password combination" assert !last_response.successful? assert_nil server_auth end diff --git a/users/test/integration/api/login_test.rb b/users/test/integration/api/login_test.rb new file mode 100644 index 0000000..ba82c8e --- /dev/null +++ b/users/test/integration/api/login_test.rb @@ -0,0 +1,25 @@ +require 'test_helper' + +CONFIG_RU = (Rails.root + 'config.ru').to_s +OUTER_APP = Rack::Builder.parse_file(CONFIG_RU).first + +class AccountFlowTest < ActiveSupport::TestCase + include Rack::Test::Methods + include Warden::Test::Helpers + include LeapWebCore::AssertResponses + + def app + OUTER_APP + end + + def setup + @login = "integration_test_user" + end + + test "require json requests" do + put "http://api.lvh.me:3000/1/sessions/" + @login, + :client_auth => "This is not a valid login anyway" + assert_json_error login: I18n.t(:all_strategies_failed) + end + +end -- cgit v1.2.3