From 5561825584f1f187b25b75e81966745f7ce9da92 Mon Sep 17 00:00:00 2001 From: Artur Beljajev Date: Wed, 18 Jul 2018 20:02:05 +0300 Subject: [PATCH] Use standard Devise sessions controller in admin area Fixes a bug when retrying to login with correct credentials --- app/controllers/admin/sessions_controller.rb | 22 -------------- app/models/admin_user.rb | 3 +- app/views/admin/sessions/_links.html.erb | 29 +++++++++++++++++++ app/views/admin/sessions/new.haml | 15 ---------- app/views/admin/sessions/new.html.erb | 30 ++++++++++++++++++++ app/views/admin/shared/_errors.haml | 5 ---- config/locales/admin/sessions.en.yml | 6 ++++ test/integration/admin/login_test.rb | 25 +++++++++++++--- 8 files changed, 88 insertions(+), 47 deletions(-) create mode 100644 app/views/admin/sessions/_links.html.erb delete mode 100644 app/views/admin/sessions/new.haml create mode 100644 app/views/admin/sessions/new.html.erb delete mode 100644 app/views/admin/shared/_errors.haml create mode 100644 config/locales/admin/sessions.en.yml diff --git a/app/controllers/admin/sessions_controller.rb b/app/controllers/admin/sessions_controller.rb index a4a286cc1..f4feefc3a 100644 --- a/app/controllers/admin/sessions_controller.rb +++ b/app/controllers/admin/sessions_controller.rb @@ -1,27 +1,5 @@ module Admin class SessionsController < Devise::SessionsController - def new - @admin_user = AdminUser.new - end - - def create - if params[:admin_user].blank? - @admin_user = AdminUser.new - flash[:alert] = 'Something went wrong' - return render :new - end - - @admin_user = AdminUser.find_by(username: params[:admin_user][:username]) - @admin_user ||= AdminUser.new(username: params[:admin_user][:username]) - - if @admin_user.valid_password?(params[:admin_user][:password]) - sign_in_and_redirect(:admin_user, @admin_user, event: :authentication) - else - flash[:alert] = 'Authorization error' - render :new - end - end - private def after_sign_in_path_for(_resource_or_scope) diff --git a/app/models/admin_user.rb b/app/models/admin_user.rb index b53d6531a..07686e921 100644 --- a/app/models/admin_user.rb +++ b/app/models/admin_user.rb @@ -9,7 +9,8 @@ class AdminUser < User ROLES = %w(user customer_service admin) # should not match to api_users roles - devise :database_authenticatable, :trackable, :validatable, :timeoutable + devise :database_authenticatable, :trackable, :validatable, :timeoutable, + authentication_keys: [:username] def self.min_password_length Devise.password_length.min diff --git a/app/views/admin/sessions/_links.html.erb b/app/views/admin/sessions/_links.html.erb new file mode 100644 index 000000000..93dadb0d8 --- /dev/null +++ b/app/views/admin/sessions/_links.html.erb @@ -0,0 +1,29 @@ +<%- if controller_name != 'sessions' %> + <%= link_to "Log in", new_session_path(resource_name) %>
+<% end -%> + +<%- if devise_mapping.registerable? && controller_name != 'registrations' %> + <%= link_to "Sign up", new_registration_path(resource_name) %>
+<% end -%> + +<%- if devise_mapping.recoverable? && controller_name != 'passwords' && + controller_name != 'registrations' %> + <%= link_to "Forgot your password?", new_password_path(resource_name) %>
+<% end -%> + +<%- if devise_mapping.confirmable? && controller_name != 'confirmations' %> + <%= link_to "Didn't receive confirmation instructions?", new_confirmation_path(resource_name) %> +
+<% end -%> + +<%- if devise_mapping.lockable? && resource_class.unlock_strategy_enabled?(:email) && + controller_name != 'unlocks' %> + <%= link_to "Didn't receive unlock instructions?", new_unlock_path(resource_name) %>
+<% end -%> + +<%- if devise_mapping.omniauthable? %> + <%- resource_class.omniauth_providers.each do |provider| %> + <%= link_to "Sign in with #{OmniAuth::Utils.camelize(provider)}", + omniauth_authorize_path(resource_name, provider) %>
+ <% end -%> +<% end -%> diff --git a/app/views/admin/sessions/new.haml b/app/views/admin/sessions/new.haml deleted file mode 100644 index d37461c85..000000000 --- a/app/views/admin/sessions/new.haml +++ /dev/null @@ -1,15 +0,0 @@ -.row - .form-signin.col-md-6.center-block.text-center - %h2.form-signin-heading.text-center Eesti Interneti SA - %hr - .form-signin - = form_for(@admin_user, url: admin_user_session_path, html: {class: 'form-signin'}) do |f| - = render 'admin/shared/errors', object: f.object - - - error_class = f.object.errors.any? ? 'has-error' : '' - %div{class: error_class} - = f.text_field :username, class: 'form-control', placeholder: t(:username), required: true - = f.password_field :password, class: 'form-control', - autocomplete: 'off', placeholder: t(:password), required: true - %button.btn.btn-lg.btn-primary.btn-block{:type => 'submit'}= t(:log_in) - diff --git a/app/views/admin/sessions/new.html.erb b/app/views/admin/sessions/new.html.erb new file mode 100644 index 000000000..5e655d9a6 --- /dev/null +++ b/app/views/admin/sessions/new.html.erb @@ -0,0 +1,30 @@ +
+ <%= form_for resource, as: resource_name, url: session_path(resource_name), + html: { class: 'col-md-6 form-signin center-block text-center' } do |f| %> + + +
+ + <%= f.label :username, class: 'sr-only' %> + <%= f.text_field :username, placeholder: AdminUser.human_attribute_name(:username), + required: true, + autofocus: true, + class: 'form-control' %> + + <%= f.label :password, class: 'sr-only' %> + <%= f.password_field :password, placeholder: AdminUser.human_attribute_name(:password), + required: true, + autocomplete: 'off', + class: 'form-control' %> + + <% if devise_mapping.rememberable? -%> +
+ +
+ <% end -%> + + <%= f.submit t('.sign_in_btn'), class: 'btn btn-lg btn-primary btn-block' %> + <% end %> +
+ +<%= render 'links' %> \ No newline at end of file diff --git a/app/views/admin/shared/_errors.haml b/app/views/admin/shared/_errors.haml deleted file mode 100644 index 50eb6de12..000000000 --- a/app/views/admin/shared/_errors.haml +++ /dev/null @@ -1,5 +0,0 @@ -- if object.errors.any? - %p.text-danger - - object.errors.each do |attr, err| - = err - %br diff --git a/config/locales/admin/sessions.en.yml b/config/locales/admin/sessions.en.yml new file mode 100644 index 000000000..154227b65 --- /dev/null +++ b/config/locales/admin/sessions.en.yml @@ -0,0 +1,6 @@ +en: + admin: + sessions: + new: + sign_in_btn: Sign in + remember_checkbox: Remember me \ No newline at end of file diff --git a/test/integration/admin/login_test.rb b/test/integration/admin/login_test.rb index 898c8b148..c5aa98a11 100644 --- a/test/integration/admin/login_test.rb +++ b/test/integration/admin/login_test.rb @@ -9,9 +9,9 @@ class AdminAreaLoginTest < ActionDispatch::IntegrationTest visit new_admin_user_session_url fill_in 'admin_user_username', with: @user.username fill_in 'admin_user_password', with: 'testtest' - click_button 'Log in' + click_button 'Sign in' - assert_text 'Log out' + assert_text 'Signed in successfully' assert_current_path admin_root_path end @@ -19,9 +19,26 @@ class AdminAreaLoginTest < ActionDispatch::IntegrationTest visit new_admin_user_session_url fill_in 'admin_user_username', with: @user.username fill_in 'admin_user_password', with: 'wrong' - click_button 'Log in' + click_button 'Sign in' - assert_text 'Authorization error' + assert_text 'Invalid Username or password' assert_current_path new_admin_user_session_path end + + def test_retry_with_correct_credentials + visit new_admin_user_session_url + fill_in 'admin_user_username', with: @user.username + fill_in 'admin_user_password', with: 'wrong' + click_button 'Sign in' + + assert_text 'Invalid Username or password' + assert_current_path new_admin_user_session_path + + fill_in 'admin_user_username', with: @user.username + fill_in 'admin_user_password', with: 'testtest' + click_button 'Sign in' + + assert_text 'Signed in successfully' + assert_current_path admin_root_path + end end \ No newline at end of file