From c707ef49d9b13932f4d98c127ec3148a5cdc3479 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Sun, 15 Sep 2019 21:08:39 +0200 Subject: Fix 2FA challenge and password challenge for non-database users (#11831) * Fix 2FA challenge not appearing for non-database users Fix #11685 * Fix account deletion not working when using external login Fix #11691 --- app/controllers/auth/sessions_controller.rb | 61 ++++++++++++-------------- app/controllers/settings/deletes_controller.rb | 25 ++++++++--- app/models/form/delete_confirmation.rb | 2 +- app/views/settings/deletes/show.html.haml | 5 ++- 4 files changed, 51 insertions(+), 42 deletions(-) (limited to 'app') diff --git a/app/controllers/auth/sessions_controller.rb b/app/controllers/auth/sessions_controller.rb index 7e6dbf19e..3e93b2e68 100644 --- a/app/controllers/auth/sessions_controller.rb +++ b/app/controllers/auth/sessions_controller.rb @@ -8,8 +8,6 @@ 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 @@ -22,9 +20,22 @@ class Auth::SessionsController < Devise::SessionsController end def create - super do |resource| - remember_me(resource) - flash.delete(:notice) + 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) end end @@ -37,18 +48,6 @@ class Auth::SessionsController < Devise::SessionsController protected - def find_user - if session[:otp_user_id] - User.find(session[:otp_user_id]) - elsif user_params[:email] - if use_seamless_external_login? && Devise.check_at_sign && user_params[:email].index('@').nil? - User.joins(:account).find_by(accounts: { username: user_params[:email] }) - else - User.find_for_authentication(email: user_params[:email]) - end - end - end - def user_params params.require(:user).permit(:email, :password, :otp_attempt) end @@ -71,32 +70,17 @@ class Auth::SessionsController < Devise::SessionsController super end - def two_factor_enabled? - find_user.try(: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]) - rescue OpenSSL::Cipher::CipherError => _error + rescue OpenSSL::Cipher::CipherError 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&.valid_password?(user_params[:password]) - 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) - remember_me(user) - sign_in(user) + authenticate_and_respond(user) else flash.now[:alert] = I18n.t('users.invalid_otp_token') prompt_for_two_factor(user) @@ -108,6 +92,13 @@ class Auth::SessionsController < Devise::SessionsController 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 @@ -120,9 +111,11 @@ 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/controllers/settings/deletes_controller.rb b/app/controllers/settings/deletes_controller.rb index 97fe4d328..15a59c999 100644 --- a/app/controllers/settings/deletes_controller.rb +++ b/app/controllers/settings/deletes_controller.rb @@ -14,12 +14,11 @@ class Settings::DeletesController < Settings::BaseController end def destroy - if current_user.valid_password?(delete_params[:password]) - Admin::SuspensionWorker.perform_async(current_user.account_id, true) - sign_out + if challenge_passed? + destroy_account! redirect_to new_user_session_path, notice: I18n.t('deletes.success_msg') else - redirect_to settings_delete_path, alert: I18n.t('deletes.bad_password_msg') + redirect_to settings_delete_path, alert: I18n.t('deletes.challenge_not_passed') end end @@ -29,11 +28,25 @@ class Settings::DeletesController < Settings::BaseController redirect_to root_path unless Setting.open_deletion end - def delete_params - params.require(:form_delete_confirmation).permit(:password) + def resource_params + params.require(:form_delete_confirmation).permit(:password, :username) end def require_not_suspended! forbidden if current_account.suspended? end + + def challenge_passed? + if current_user.encrypted_password.blank? + current_account.username == resource_params[:username] + else + current_user.valid_password?(resource_params[:password]) + end + end + + def destroy_account! + current_account.suspend! + Admin::SuspensionWorker.perform_async(current_user.account_id, true) + sign_out + end end diff --git a/app/models/form/delete_confirmation.rb b/app/models/form/delete_confirmation.rb index 0884a09b8..99d04b331 100644 --- a/app/models/form/delete_confirmation.rb +++ b/app/models/form/delete_confirmation.rb @@ -3,5 +3,5 @@ class Form::DeleteConfirmation include ActiveModel::Model - attr_accessor :password + attr_accessor :password, :username end diff --git a/app/views/settings/deletes/show.html.haml b/app/views/settings/deletes/show.html.haml index 6e2ff31c5..08792e0af 100644 --- a/app/views/settings/deletes/show.html.haml +++ b/app/views/settings/deletes/show.html.haml @@ -20,7 +20,10 @@ %hr.spacer/ - = f.input :password, wrapper: :with_block_label, input_html: { :autocomplete => 'off' }, hint: t('deletes.confirm_password') + - if current_user.encrypted_password.present? + = f.input :password, wrapper: :with_block_label, input_html: { :autocomplete => 'off' }, hint: t('deletes.confirm_password') + - else + = f.input :username, wrapper: :with_block_label, input_html: { :autocomplete => 'off' }, hint: t('deletes.confirm_username') .actions = f.button :button, t('deletes.proceed'), type: :submit, class: 'negative' -- cgit