Add e-mail-based sign in challenge for users with disabled 2FA (#14013)

8 files changed, 281 insertions, 46 deletions

diff --git a/app/controllers/auth/sessions_controller.rb b/app/controllers/auth/sessions_controller.rb
index e95909447..2415e2ef3 100644
--- a/app/controllers/auth/sessions_controller.rb
+++ b/app/controllers/auth/sessions_controller.rb
@@ -8,7 +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]
+  include TwoFactorAuthenticationConcern
+  include SignInTokenAuthenticationConcern
 
   before_action :set_instance_presenter, only: [:new]
   before_action :set_body_classes
@@ -39,8 +40,8 @@ class Auth::SessionsController < Devise::SessionsController
   protected
 
   def find_user
-    if session[:otp_user_id]
-      User.find(session[:otp_user_id])
+    if session[:attempt_user_id]
+      User.find(session[:attempt_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
@@ -49,7 +50,7 @@ class Auth::SessionsController < Devise::SessionsController
   end
 
   def user_params
-    params.require(:user).permit(:email, :password, :otp_attempt)
+    params.require(:user).permit(:email, :password, :otp_attempt, :sign_in_token_attempt)
   end
 
   def after_sign_in_path_for(resource)
@@ -70,47 +71,6 @@ 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])
-  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.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)
-      remember_me(user)
-      sign_in(user)
-    else
-      flash.now[:alert] = I18n.t('users.invalid_otp_token')
-      prompt_for_two_factor(user)
-    end
-  end
-
-  def prompt_for_two_factor(user)
-    session[:otp_user_id] = user.id
-    @body_classes = 'lighter'
-    render :two_factor
-  end
-
   def require_no_authentication
     super
     # Delete flash message that isn't entirely useful and may be confusing in
diff --git a/app/controllers/concerns/sign_in_token_authentication_concern.rb b/app/controllers/concerns/sign_in_token_authentication_concern.rb
new file mode 100644
index 000000000..a177aacaf
--- /dev/null
+++ b/app/controllers/concerns/sign_in_token_authentication_concern.rb
@@ -0,0 +1,49 @@
+# frozen_string_literal: true
+
+module SignInTokenAuthenticationConcern
+  extend ActiveSupport::Concern
+
+  included do
+    prepend_before_action :authenticate_with_sign_in_token, if: :sign_in_token_required?, only: [:create]
+  end
+
+  def sign_in_token_required?
+    find_user&.suspicious_sign_in?(request.remote_ip)
+  end
+
+  def valid_sign_in_token_attempt?(user)
+    Devise.secure_compare(user.sign_in_token, user_params[:sign_in_token_attempt])
+  end
+
+  def authenticate_with_sign_in_token
+    user = self.resource = find_user
+
+    if user_params[:sign_in_token_attempt].present? && session[:attempt_user_id]
+      authenticate_with_sign_in_token_attempt(user)
+    elsif user.present? && user.external_or_valid_password?(user_params[:password])
+      prompt_for_sign_in_token(user)
+    end
+  end
+
+  def authenticate_with_sign_in_token_attempt(user)
+    if valid_sign_in_token_attempt?(user)
+      session.delete(:attempt_user_id)
+      remember_me(user)
+      sign_in(user)
+    else
+      flash.now[:alert] = I18n.t('users.invalid_sign_in_token')
+      prompt_for_sign_in_token(user)
+    end
+  end
+
+  def prompt_for_sign_in_token(user)
+    if user.sign_in_token_expired?
+      user.generate_sign_in_token && user.save
+      UserMailer.sign_in_token(user, request.remote_ip, request.user_agent, Time.now.utc.to_s).deliver_later!
+    end
+
+    session[:attempt_user_id] = user.id
+    @body_classes = 'lighter'
+    render :sign_in_token
+  end
+end
diff --git a/app/controllers/concerns/two_factor_authentication_concern.rb b/app/controllers/concerns/two_factor_authentication_concern.rb
new file mode 100644
index 000000000..cdd8d14af
--- /dev/null
+++ b/app/controllers/concerns/two_factor_authentication_concern.rb
@@ -0,0 +1,47 @@
+# frozen_string_literal: true
+
+module TwoFactorAuthenticationConcern
+  extend ActiveSupport::Concern
+
+  included do
+    prepend_before_action :authenticate_with_two_factor, if: :two_factor_enabled?, only: [:create]
+  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])
+  rescue OpenSSL::Cipher::CipherError
+    false
+  end
+
+  def authenticate_with_two_factor
+    user = self.resource = find_user
+
+    if user_params[:otp_attempt].present? && session[:attempt_user_id]
+      authenticate_with_two_factor_attempt(user)
+    elsif user.present? && user.external_or_valid_password?(user_params[:password])
+      prompt_for_two_factor(user)
+    end
+  end
+
+  def authenticate_with_two_factor_attempt(user)
+    if valid_otp_attempt?(user)
+      session.delete(:attempt_user_id)
+      remember_me(user)
+      sign_in(user)
+    else
+      flash.now[:alert] = I18n.t('users.invalid_otp_token')
+      prompt_for_two_factor(user)
+    end
+  end
+
+  def prompt_for_two_factor(user)
+    session[:attempt_user_id] = user.id
+    @body_classes = 'lighter'
+    render :two_factor
+  end
+end
diff --git a/app/mailers/user_mailer.rb b/app/mailers/user_mailer.rb
index 88a11f761..2cd58e60a 100644
--- a/app/mailers/user_mailer.rb
+++ b/app/mailers/user_mailer.rb
@@ -126,4 +126,21 @@ class UserMailer < Devise::Mailer
            reply_to: Setting.site_contact_email
     end
   end
+
+  def sign_in_token(user, remote_ip, user_agent, timestamp)
+    @resource   = user
+    @instance   = Rails.configuration.x.local_domain
+    @remote_ip  = remote_ip
+    @user_agent = user_agent
+    @detection  = Browser.new(user_agent)
+    @timestamp  = timestamp.to_time.utc
+
+    return if @resource.disabled?
+
+    I18n.with_locale(@resource.locale || I18n.default_locale) do
+      mail to: @resource.email,
+           subject: I18n.t('user_mailer.sign_in_token.subject'),
+           reply_to: Setting.site_contact_email
+    end
+  end
 end
diff --git a/app/models/user.rb b/app/models/user.rb
index ae0cdf29d..306e2d435 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -38,6 +38,8 @@
 #  chosen_languages          :string           is an Array
 #  created_by_application_id :bigint(8)
 #  approved                  :boolean          default(TRUE), not null
+#  sign_in_token             :string
+#  sign_in_token_sent_at     :datetime
 #
 
 class User < ApplicationRecord
@@ -113,7 +115,7 @@ class User < ApplicationRecord
            :advanced_layout, :use_blurhash, :use_pending_items, :trends, :crop_images,
            to: :settings, prefix: :setting, allow_nil: false
 
-  attr_reader :invite_code
+  attr_reader :invite_code, :sign_in_token_attempt
   attr_writer :external
 
   def confirmed?
@@ -167,6 +169,10 @@ class User < ApplicationRecord
     true
   end
 
+  def suspicious_sign_in?(ip)
+    !otp_required_for_login? && current_sign_in_at.present? && current_sign_in_at < 2.weeks.ago && !recent_ip?(ip)
+  end
+
   def functional?
     confirmed? && approved? && !disabled? && !account.suspended? && account.moved_to_account_id.nil?
   end
@@ -269,6 +275,13 @@ class User < ApplicationRecord
     super
   end
 
+  def external_or_valid_password?(compare_password)
+    # If encrypted_password is blank, we got the user from LDAP or PAM,
+    # so credentials are already valid
+
+    encrypted_password.blank? || valid_password?(compare_password)
+  end
+
   def send_reset_password_instructions
     return false if encrypted_password.blank?
 
@@ -304,6 +317,15 @@ class User < ApplicationRecord
     end
   end
 
+  def sign_in_token_expired?
+    sign_in_token_sent_at.nil? || sign_in_token_sent_at < 5.minutes.ago
+  end
+
+  def generate_sign_in_token
+    self.sign_in_token         = Devise.friendly_token(6)
+    self.sign_in_token_sent_at = Time.now.utc
+  end
+
   protected
 
   def send_devise_notification(notification, *args)
@@ -320,6 +342,10 @@ class User < ApplicationRecord
 
   private
 
+  def recent_ip?(ip)
+    recent_ips.any? { |(_, recent_ip)| recent_ip == ip }
+  end
+
   def send_pending_devise_notifications
     pending_devise_notifications.each do |notification, args|
       render_and_send_devise_message(notification, *args)
diff --git a/app/views/auth/sessions/sign_in_token.html.haml b/app/views/auth/sessions/sign_in_token.html.haml
new file mode 100644
index 000000000..8923203cd
--- /dev/null
+++ b/app/views/auth/sessions/sign_in_token.html.haml
@@ -0,0 +1,14 @@
+- content_for :page_title do
+  = t('auth.login')
+
+= simple_form_for(resource, as: resource_name, url: session_path(resource_name), method: :post) do |f|
+  %p.hint.otp-hint= t('users.suspicious_sign_in_confirmation')
+
+  .fields-group
+    = f.input :sign_in_token_attempt, type: :number, wrapper: :with_label, label: t('simple_form.labels.defaults.sign_in_token_attempt'), input_html: { 'aria-label' => t('simple_form.labels.defaults.sign_in_token_attempt'), :autocomplete => 'off' }, autofocus: true
+
+  .actions
+    = f.button :button, t('auth.login'), type: :submit
+
+  - if Setting.site_contact_email.present?
+    %p.hint.subtle-hint= t('users.generic_access_help_html', email: mail_to(Setting.site_contact_email, nil))
diff --git a/app/views/user_mailer/sign_in_token.html.haml b/app/views/user_mailer/sign_in_token.html.haml
new file mode 100644
index 000000000..826b34e7c
--- /dev/null
+++ b/app/views/user_mailer/sign_in_token.html.haml
@@ -0,0 +1,105 @@
+%table.email-table{ cellspacing: 0, cellpadding: 0 }
+  %tbody
+    %tr
+      %td.email-body
+        .email-container
+          %table.content-section{ cellspacing: 0, cellpadding: 0 }
+            %tbody
+              %tr
+                %td.content-cell.hero
+                  .email-row
+                    .col-6
+                      %table.column{ cellspacing: 0, cellpadding: 0 }
+                        %tbody
+                          %tr
+                            %td.column-cell.text-center.padded
+                              %table.hero-icon.alert-icon{ align: 'center', cellspacing: 0, cellpadding: 0 }
+                                %tbody
+                                  %tr
+                                    %td
+                                      = image_tag full_pack_url('media/images/mailer/icon_email.png'), alt: ''
+
+                              %h1= t 'user_mailer.sign_in_token.title'
+                              %p.lead= t 'user_mailer.sign_in_token.explanation'
+
+%table.email-table{ cellspacing: 0, cellpadding: 0 }
+  %tbody
+    %tr
+      %td.email-body
+        .email-container
+          %table.content-section{ cellspacing: 0, cellpadding: 0 }
+            %tbody
+              %tr
+                %td.content-cell.content-start
+                  %table.column{ cellspacing: 0, cellpadding: 0 }
+                    %tbody
+                      %tr
+                        %td.column-cell.input-cell
+                          %table.input{ align: 'center', cellspacing: 0, cellpadding: 0 }
+                            %tbody
+                              %tr
+                                %td= @resource.sign_in_token
+
+%table.email-table{ cellspacing: 0, cellpadding: 0 }
+  %tbody
+    %tr
+      %td.email-body
+        .email-container
+          %table.content-section{ cellspacing: 0, cellpadding: 0 }
+            %tbody
+              %tr
+                %td.content-cell
+                  .email-row
+                    .col-6
+                      %table.column{ cellspacing: 0, cellpadding: 0 }
+                        %tbody
+                          %tr
+                            %td.column-cell.text-center
+                              %p= t 'user_mailer.sign_in_token.details'
+                          %tr
+                            %td.column-cell.text-center
+                              %p
+                                %strong= "#{t('sessions.ip')}:"
+                                = @remote_ip
+                                %br/
+                                %strong= "#{t('sessions.browser')}:"
+                                %span{ title: @user_agent }= t 'sessions.description', browser: t("sessions.browsers.#{@detection.id}", default: "#{@detection.id}"), platform: t("sessions.platforms.#{@detection.platform.id}", default: "#{@detection.platform.id}")
+                                %br/
+                                = l(@timestamp)
+
+%table.email-table{ cellspacing: 0, cellpadding: 0 }
+  %tbody
+    %tr
+      %td.email-body
+        .email-container
+          %table.content-section{ cellspacing: 0, cellpadding: 0 }
+            %tbody
+              %tr
+                %td.content-cell
+                  .email-row
+                    .col-6
+                      %table.column{ cellspacing: 0, cellpadding: 0 }
+                        %tbody
+                          %tr
+                            %td.column-cell.text-center
+                              %p= t 'user_mailer.sign_in_token.further_actions'
+
+%table.email-table{ cellspacing: 0, cellpadding: 0 }
+  %tbody
+    %tr
+      %td.email-body
+        .email-container
+          %table.content-section{ cellspacing: 0, cellpadding: 0 }
+            %tbody
+              %tr
+                %td.content-cell
+                  %table.column{ cellspacing: 0, cellpadding: 0 }
+                    %tbody
+                      %tr
+                        %td.column-cell.button-cell
+                          %table.button{ align: 'center', cellspacing: 0, cellpadding: 0 }
+                            %tbody
+                              %tr
+                                %td.button-primary
+                                  = link_to edit_user_registration_url do
+                                    %span= t 'settings.account_settings'
diff --git a/app/views/user_mailer/sign_in_token.text.erb b/app/views/user_mailer/sign_in_token.text.erb
new file mode 100644
index 000000000..2539ddaf6
--- /dev/null
+++ b/app/views/user_mailer/sign_in_token.text.erb
@@ -0,0 +1,17 @@
+<%= t 'user_mailer.sign_in_token.title' %>
+
+===
+
+<%= t 'user_mailer.sign_in_token.explanation' %>
+
+=> <%= @resource.sign_in_token %>
+
+<%= t 'user_mailer.sign_in_token.details' %>
+
+<%= t('sessions.ip') %>: <%= @remote_ip %>
+<%= t('sessions.browser') %>: <%= t('sessions.description', browser: t("sessions.browsers.#{@detection.id}", default: "#{@detection.id}"), platform: t("sessions.platforms.#{@detection.platform.id}", default: "#{@detection.platform.id}")) %>
+<%= l(@timestamp) %>
+
+<%= t 'user_mailer.sign_in_token.further_actions' %>
+
+=> <%= edit_user_registration_url %>