From a1f04c1e3497e9dff5970038461d9f454f2650df Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Tue, 24 Sep 2019 04:35:36 +0200 Subject: Fix authentication before 2FA challenge (#11943) Regression from #11831 --- app/controllers/auth/sessions_controller.rb | 61 +++++++++++++++++------------ app/models/concerns/ldap_authenticable.rb | 44 ++++++++++++++++----- 2 files changed, 70 insertions(+), 35 deletions(-) (limited to 'app') diff --git a/app/controllers/auth/sessions_controller.rb b/app/controllers/auth/sessions_controller.rb index b3113bbef..f48b17c79 100644 --- a/app/controllers/auth/sessions_controller.rb +++ b/app/controllers/auth/sessions_controller.rb @@ -8,6 +8,8 @@ class Auth::SessionsController < Devise::SessionsController skip_before_action :require_no_authentication, only: [:create] skip_before_action :require_functional! + prepend_before_action :authenticate_with_two_factor, if: :two_factor_enabled?, only: [:create] + before_action :set_instance_presenter, only: [:new] before_action :set_body_classes @@ -20,22 +22,9 @@ class Auth::SessionsController < Devise::SessionsController end def create - self.resource = begin - if user_params[:email].blank? && session[:otp_user_id].present? - User.find(session[:otp_user_id]) - else - warden.authenticate!(auth_options) - end - end - - if resource.otp_required_for_login? - if user_params[:otp_attempt].present? && session[:otp_user_id].present? - authenticate_with_two_factor_via_otp(resource) - else - prompt_for_two_factor(resource) - end - else - authenticate_and_respond(resource) + super do |resource| + remember_me(resource) + flash.delete(:notice) end end @@ -49,6 +38,16 @@ class Auth::SessionsController < Devise::SessionsController protected + def find_user + if session[:otp_user_id] + User.find(session[:otp_user_id]) + else + user = User.authenticate_with_ldap(user_params) if Devise.ldap_authentication + user ||= User.authenticate_with_pam(user_params) if Devise.pam_authentication + user ||= User.find_for_authentication(email: user_params[:email]) + end + end + def user_params params.require(:user).permit(:email, :password, :otp_attempt) end @@ -71,6 +70,10 @@ class Auth::SessionsController < Devise::SessionsController super end + def two_factor_enabled? + find_user&.otp_required_for_login? + end + def valid_otp_attempt?(user) user.validate_and_consume_otp!(user_params[:otp_attempt]) || user.invalidate_otp_backup_code!(user_params[:otp_attempt]) @@ -78,10 +81,24 @@ class Auth::SessionsController < Devise::SessionsController false end + def authenticate_with_two_factor + user = self.resource = find_user + + if user_params[:otp_attempt].present? && session[:otp_user_id] + authenticate_with_two_factor_via_otp(user) + elsif user.present? && (user.encrypted_password.blank? || user.valid_password?(user_params[:password])) + # If encrypted_password is blank, we got the user from LDAP or PAM, + # so credentials are already valid + + prompt_for_two_factor(user) + end + end + def authenticate_with_two_factor_via_otp(user) if valid_otp_attempt?(user) session.delete(:otp_user_id) - authenticate_and_respond(user) + remember_me(user) + sign_in(user) else flash.now[:alert] = I18n.t('users.invalid_otp_token') prompt_for_two_factor(user) @@ -90,16 +107,10 @@ class Auth::SessionsController < Devise::SessionsController def prompt_for_two_factor(user) session[:otp_user_id] = user.id + @body_classes = 'lighter' render :two_factor end - def authenticate_and_respond(user) - sign_in(user) - remember_me(user) - - respond_with user, location: after_sign_in_path_for(user) - end - private def set_instance_presenter @@ -112,11 +123,9 @@ class Auth::SessionsController < Devise::SessionsController def home_paths(resource) paths = [about_path] - if single_user_mode? && resource.is_a?(User) paths << short_account_path(username: resource.account) end - paths end diff --git a/app/models/concerns/ldap_authenticable.rb b/app/models/concerns/ldap_authenticable.rb index 84ff84c4b..117993947 100644 --- a/app/models/concerns/ldap_authenticable.rb +++ b/app/models/concerns/ldap_authenticable.rb @@ -3,24 +3,50 @@ module LdapAuthenticable extend ActiveSupport::Concern - def ldap_setup(_attributes) - self.confirmed_at = Time.now.utc - self.admin = false - self.external = true + class_methods do + def authenticate_with_ldap(params = {}) + ldap = Net::LDAP.new(ldap_options) + filter = format(Devise.ldap_search_filter, uid: Devise.ldap_uid, email: params[:email]) - save! - end + if (user_info = ldap.bind_as(base: Devise.ldap_base, filter: filter, password: params[:password])) + ldap_get_user(user_info.first) + end + end - class_methods do def ldap_get_user(attributes = {}) resource = joins(:account).find_by(accounts: { username: attributes[Devise.ldap_uid.to_sym].first }) if resource.blank? - resource = new(email: attributes[:mail].first, agreement: true, account_attributes: { username: attributes[Devise.ldap_uid.to_sym].first }) - resource.ldap_setup(attributes) + resource = new(email: attributes[:mail].first, agreement: true, account_attributes: { username: attributes[Devise.ldap_uid.to_sym].first }, admin: false, external: true, confirmed_at: Time.now.utc) + resource.save! end resource end + + def ldap_options + opts = { + host: Devise.ldap_host, + port: Devise.ldap_port, + base: Devise.ldap_base, + + auth: { + method: :simple, + username: Devise.ldap_bind_dn, + password: Devise.ldap_password, + }, + + connect_timeout: 10, + } + + if [:simple_tls, :start_tls].include?(Devise.ldap_method) + opts[:encryption] = { + method: Devise.ldap_method, + tls_options: OpenSSL::SSL::SSLContext::DEFAULT_PARAMS.tap { |options| options[:verify_mode] = OpenSSL::SSL::VERIFY_NONE if Devise.ldap_tls_no_verify }, + } + end + + opts + end end end -- cgit