From 50ff026a06669850f3b1ca95290821e1284cf8ee Mon Sep 17 00:00:00 2001 From: Daniel Illi Date: Thu, 18 Aug 2022 18:01:22 +0200 Subject: [PATCH] Auto redirect on login page when single auth method configured, fixes #59739 --- .../omniauth_callbacks_controller.rb | 4 ++ .../employees/sessions_controller.rb | 34 +++++++++++ app/models/employee.rb | 7 --- app/views/devise/shared/_links.html.haml | 8 ++- config/routes.rb | 2 +- .../employees/sessions_controller_test.rb | 59 +++++++++++++++++++ .../integration/employees/new_session_test.rb | 57 ++++++++++++++++++ 7 files changed, 162 insertions(+), 9 deletions(-) create mode 100644 app/controllers/employees/sessions_controller.rb create mode 100644 test/controllers/employees/sessions_controller_test.rb create mode 100644 test/integration/employees/new_session_test.rb diff --git a/app/controllers/employees/omniauth_callbacks_controller.rb b/app/controllers/employees/omniauth_callbacks_controller.rb index d1aae90bb..d73d6460b 100644 --- a/app/controllers/employees/omniauth_callbacks_controller.rb +++ b/app/controllers/employees/omniauth_callbacks_controller.rb @@ -16,4 +16,8 @@ def default # TODO: Username wegspeichern alias keycloakopenid default alias saml default + + def after_omniauth_failure_path_for(scope) + new_session_path(scope, prevent_auto_login: true) + end end diff --git a/app/controllers/employees/sessions_controller.rb b/app/controllers/employees/sessions_controller.rb new file mode 100644 index 000000000..28a1fdfc2 --- /dev/null +++ b/app/controllers/employees/sessions_controller.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +# Copyright (c) 2006-2022, Puzzle ITC GmbH. This file is part of +# PuzzleTime and licensed under the Affero General Public License version 3 +# or later. See the COPYING file at the top-level directory or at +# https://github.com/puzzle/puzzletime. + +class Employees::SessionsController < Devise::SessionsController + helper_method :auto_redirect? + + private + + def no_local_auth? + !Settings.auth.db.active + end + + def omniauth_providers_active + Settings.auth&.omniauth&.map(&:second)&.map(&:active) + end + + def single_omniauth_provider? + omniauth_providers_active&.one? + end + + def auto_login_allowed? + return true unless prevent = params[:prevent_auto_login] + + !ActiveRecord::Type::Boolean.new.deserialize(prevent) + end + + def auto_redirect? + auto_login_allowed? && no_local_auth? && single_omniauth_provider? + end +end diff --git a/app/models/employee.rb b/app/models/employee.rb index 30079be8e..e687fdd8c 100644 --- a/app/models/employee.rb +++ b/app/models/employee.rb @@ -140,13 +140,6 @@ def providers end class << self - # Tries to login a user with the passed data. - # Returns the logged-in Employee or nil if the login failed. - def login(username, pwd) - find_by(shortname: username.upcase, passwd: encode(pwd)) || - LdapAuthenticator.new.login(username, pwd) - end - def employed_ones(period, sort = true) result = joins('left join employments em on em.employee_id = employees.id'). where('(em.end_date IS null or em.end_date >= ?) AND em.start_date <= ?', diff --git a/app/views/devise/shared/_links.html.haml b/app/views/devise/shared/_links.html.haml index 6b9c859dd..f8f1a53b8 100644 --- a/app/views/devise/shared/_links.html.haml +++ b/app/views/devise/shared/_links.html.haml @@ -25,6 +25,12 @@ = link_to "Mit #{provider_label} anmelden", public_send("#{resource_name}_#{provider}_omniauth_authorize_path"), method: :post, - class: 'btn btn-primary' + class: try(:auto_redirect?) ? 'btn btn-primary auto-login' : 'btn btn-primary' %br %br + +- if try(:auto_redirect?) + :coffeescript + $(document).on('turbolinks:load', -> + $('.auto-login').click(); + ) \ No newline at end of file diff --git a/config/routes.rb b/config/routes.rb index 24af575fa..e728bca0d 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -4,7 +4,7 @@ # https://github.com/puzzle/puzzletime. Rails.application.routes.draw do - devise_for :employees, controllers: { omniauth_callbacks: 'employees/omniauth_callbacks' }, skip: [:registrations] + devise_for :employees, controllers: { sessions: 'employees/sessions', omniauth_callbacks: 'employees/omniauth_callbacks' }, skip: [:registrations] as :employee do get 'employees/edit' => 'devise/registrations#edit', :as => 'edit_employee_registration' patch 'employees' => 'devise/registrations#update', :as => 'employee_registration' diff --git a/test/controllers/employees/sessions_controller_test.rb b/test/controllers/employees/sessions_controller_test.rb new file mode 100644 index 000000000..a6431c1fc --- /dev/null +++ b/test/controllers/employees/sessions_controller_test.rb @@ -0,0 +1,59 @@ +# Copyright (c) 2006-2022, Puzzle ITC GmbH. This file is part of +# PuzzleTime and licensed under the Affero General Public License version 3 +# or later. See the COPYING file at the top-level directory or at +# https://github.com/puzzle/puzzletime. + +require 'test_helper' + +class Employees::SessionsControllerTest < ActionController::TestCase + setup do + @request.env['devise.mapping'] = Devise.mappings[:employee] + end + + test "helper auto_redirect? with only omniauth keycloadopenid active" do + Settings.auth.db.active = false + Settings.auth.omniauth.keycloakopenid.active = true + Settings.auth.omniauth.saml.active = false + assert @controller.view_context.auto_redirect? + end + + test "helper auto_redirect? with only omniauth saml active" do + Settings.auth.db.active = false + Settings.auth.omniauth.keycloakopenid.active = false + Settings.auth.omniauth.saml.active = true + assert @controller.view_context.auto_redirect? + end + + test "helper auto_redirect? with only local auth active" do + Settings.auth.db.active = true + Settings.auth.omniauth.keycloakopenid.active = false + Settings.auth.omniauth.saml.active = false + refute @controller.view_context.auto_redirect? + end + + test "helper auto_redirect? with multiple omniauth active" do + Settings.auth.db.active = false + Settings.auth.omniauth.keycloakopenid.active = true + Settings.auth.omniauth.saml.active = true + refute @controller.view_context.auto_redirect? + end + + test "helper auto_redirect? with local auth and single omniauth active" do + Settings.auth.db.active = true + Settings.auth.omniauth.keycloakopenid.active = true + Settings.auth.omniauth.saml.active = false + refute @controller.view_context.auto_redirect? + end + + test "helper auto_redirect? depending on param prevent_auto_login" do + Settings.auth.db.active = false + Settings.auth.omniauth.keycloakopenid.active = true + Settings.auth.omniauth.saml.active = false + + get :new + assert @controller.view_context.auto_redirect? + + get :new, params: {prevent_auto_login: true} + refute @controller.view_context.auto_redirect? + end +end diff --git a/test/integration/employees/new_session_test.rb b/test/integration/employees/new_session_test.rb new file mode 100644 index 000000000..ce97ac5cc --- /dev/null +++ b/test/integration/employees/new_session_test.rb @@ -0,0 +1,57 @@ +# Copyright (c) 2006-2022, Puzzle ITC GmbH. This file is part of +# PuzzleTime and licensed under the Affero General Public License version 3 +# or later. See the COPYING file at the top-level directory or at +# https://github.com/puzzle/puzzletime. + +require 'test_helper' + +class Employees::NewSessionTest < ActionDispatch::IntegrationTest + def setup + # We use the rack_test driver as this one does not evaluate javascript. + # This is required as we want to test if the page contains the necessary class attribute and javascript snippet + # to execute the auto login. For this the auto login redirect can't actually happen. + Capybara.current_driver = :rack_test + end + + def teardown + # Let's restore the original driver. + Capybara.use_default_driver + end + + test 'login button has auto-login class if eligible' do + Settings.auth.db.active = false + Settings.auth.omniauth.keycloakopenid.active = true + Settings.auth.omniauth.saml.active = false + + visit new_employee_session_path + assert_selector 'a.auto-login', text: 'Mit Puzzle SSO anmelden' + end + + test 'login button does not have auto-login class if uneligible' do + Settings.auth.db.active = true + Settings.auth.omniauth.keycloakopenid.active = true + Settings.auth.omniauth.saml.active = false + + visit new_employee_session_path + assert_selector 'a', text: 'Mit Puzzle SSO anmelden' + assert_no_selector 'a.auto-login', text: 'Mit Puzzle SSO anmelden' + end + + test 'page includes auto-login javascript if eligible' do + Settings.auth.db.active = false + Settings.auth.omniauth.keycloakopenid.active = true + Settings.auth.omniauth.saml.active = false + + visit new_employee_session_path + assert page.text(:all).include? "$('.auto-login').click()" + end + + test 'page excludes auto-login javascript if uneligible' do + Settings.auth.db.active = true + Settings.auth.omniauth.keycloakopenid.active = true + Settings.auth.omniauth.saml.active = false + + visit new_employee_session_path + assert page.text(:all).exclude? "$('.auto-login').click()" + end +end