Remove sign-in token authentication, instead send e-mail about new sign-in (#17970)

app/controllers/admin/sign_in_token_authentications_controller.rb

@@ -1,27 +0,0 @@
-# frozen_string_literal: true
-
-module Admin
-  class SignInTokenAuthenticationsController < BaseController
-    before_action :set_target_user
-
-    def create
-      authorize @user, :enable_sign_in_token_auth?
-      @user.update(skip_sign_in_token: false)
-      log_action :enable_sign_in_token_auth, @user
-      redirect_to admin_account_path(@user.account_id)
-    end
-
-    def destroy
-      authorize @user, :disable_sign_in_token_auth?
-      @user.update(skip_sign_in_token: true)
-      log_action :disable_sign_in_token_auth, @user
-      redirect_to admin_account_path(@user.account_id)
-    end
-
-    private
-
-    def set_target_user
-      @user = User.find(params[:user_id])
-    end
-  end
-end

app/controllers/auth/sessions_controller.rb

@@ -8,7 +8,6 @@ class Auth::SessionsController < Devise::SessionsController
   skip_before_action :update_user_sign_in
 
   include TwoFactorAuthenticationConcern
-  include SignInTokenAuthenticationConcern
 
   before_action :set_instance_presenter, only: [:new]
   before_action :set_body_classes
@@ -66,7 +65,7 @@ class Auth::SessionsController < Devise::SessionsController
   end
 
   def user_params
-    params.require(:user).permit(:email, :password, :otp_attempt, :sign_in_token_attempt, credential: {})
+    params.require(:user).permit(:email, :password, :otp_attempt, credential: {})
   end
 
   def after_sign_in_path_for(resource)
@@ -142,6 +141,12 @@ class Auth::SessionsController < Devise::SessionsController
       ip: request.remote_ip,
       user_agent: request.user_agent
     )
+
+    UserMailer.suspicious_sign_in(user, request.remote_ip, request.user_agent, Time.now.utc).deliver_later! if suspicious_sign_in?(user)
+  end
+
+  def suspicious_sign_in?(user)
+    SuspiciousSignInDetector.new(user).suspicious?(request)
   end
 
   def on_authentication_failure(user, security_measure, failure_reason)

app/controllers/concerns/sign_in_token_authentication_concern.rb

@@ -1,56 +0,0 @@
-# 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
-    if user_params[:email].present?
-      user = self.resource = find_user_from_params
-      prompt_for_sign_in_token(user) if user&.external_or_valid_password?(user_params[:password])
-    elsif session[:attempt_user_id]
-      user = self.resource = User.find_by(id: session[:attempt_user_id])
-      return if user.nil?
-
-      if session[:attempt_user_updated_at] != user.updated_at.to_s
-        restart_session
-      elsif user_params.key?(:sign_in_token_attempt)
-        authenticate_with_sign_in_token_attempt(user)
-      end
-    end
-  end
-
-  def authenticate_with_sign_in_token_attempt(user)
-    if valid_sign_in_token_attempt?(user)
-      on_authentication_success(user, :sign_in_token)
-    else
-      on_authentication_failure(user, :sign_in_token, :invalid_sign_in_token)
-      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
-
-    set_attempt_session(user)
-
-    @body_classes = 'lighter'
-
-    set_locale { render :sign_in_token }
-  end
-end

app/javascript/styles/mailer.scss

@@ -435,6 +435,10 @@ h5 {
     background: $success-green;
   }
 
+  &.warning-icon td {
+    background: $gold-star;
+  }
+
   &.alert-icon td {
     background: $error-red;
   }

app/lib/suspicious_sign_in_detector.rb

@@ -0,0 +1,42 @@
+# frozen_string_literal: true
+
+class SuspiciousSignInDetector
+  IPV6_TOLERANCE_MASK = 64
+  IPV4_TOLERANCE_MASK = 16
+
+  def initialize(user)
+    @user = user
+  end
+
+  def suspicious?(request)
+    !sufficient_security_measures? && !freshly_signed_up? && !previously_seen_ip?(request)
+  end
+
+  private
+
+  def sufficient_security_measures?
+    @user.otp_required_for_login?
+  end
+
+  def previously_seen_ip?(request)
+    @user.ips.where('ip <<= ?', masked_ip(request)).exists?
+  end
+
+  def freshly_signed_up?
+    @user.current_sign_in_at.blank?
+  end
+
+  def masked_ip(request)
+    masked_ip_addr = begin
+      ip_addr = IPAddr.new(request.remote_ip)
+
+      if ip_addr.ipv6?
+        ip_addr.mask(IPV6_TOLERANCE_MASK)
+      else
+        ip_addr.mask(IPV4_TOLERANCE_MASK)
+      end
+    end
+
+    "#{masked_ip_addr}/#{masked_ip_addr.prefix}"
+  end
+end

app/mailers/user_mailer.rb

@@ -167,9 +167,7 @@ class UserMailer < Devise::Mailer
     @statuses = @warning.statuses.includes(:account, :preloadable_poll, :media_attachments, active_mentions: [:account])
 
     I18n.with_locale(@resource.locale || I18n.default_locale) do
-      mail to: @resource.email,
+      mail to: @resource.email, subject: I18n.t("user_mailer.warning.subject.#{@warning.action}", acct: "@#{user.account.local_username_and_domain}")
-           subject: I18n.t("user_mailer.warning.subject.#{@warning.action}", acct: "@#{user.account.local_username_and_domain}"),
-           reply_to: ENV['SMTP_REPLY_TO']
     end
   end
 
@@ -193,7 +191,7 @@ class UserMailer < Devise::Mailer
     end
   end
 
-  def sign_in_token(user, remote_ip, user_agent, timestamp)
+  def suspicious_sign_in(user, remote_ip, user_agent, timestamp)
     @resource   = user
     @instance   = Rails.configuration.x.local_domain
     @remote_ip  = remote_ip
@@ -201,12 +199,8 @@ class UserMailer < Devise::Mailer
     @detection  = Browser.new(user_agent)
     @timestamp  = timestamp.to_time.utc
 
-    return unless @resource.active_for_authentication?
-
     I18n.with_locale(@resource.locale || I18n.default_locale) do
-      mail to: @resource.email,
+      mail to: @resource.email, subject: I18n.t('user_mailer.suspicious_sign_in.subject')
-           subject: I18n.t('user_mailer.sign_in_token.subject'),
-           reply_to: ENV['SMTP_REPLY_TO']
     end
   end
 end

app/models/user.rb

@@ -47,6 +47,7 @@ class User < ApplicationRecord
     remember_token
     current_sign_in_ip
     last_sign_in_ip
+    skip_sign_in_token
   )
 
   include Settings::Extend
@@ -132,7 +133,7 @@ class User < ApplicationRecord
            :disable_swiping,
            to: :settings, prefix: :setting, allow_nil: false
 
-  attr_reader :invite_code, :sign_in_token_attempt
+  attr_reader :invite_code
   attr_writer :external, :bypass_invite_request_check
 
   def confirmed?
@@ -200,10 +201,6 @@ class User < ApplicationRecord
     !account.memorial?
   end
 
-  def suspicious_sign_in?(ip)
-    !otp_required_for_login? && !skip_sign_in_token? && current_sign_in_at.present? && !ips.where(ip: ip).exists?
-  end
-
   def functional?
     confirmed? && approved? && !disabled? && !account.suspended? && !account.memorial? && account.moved_to_account_id.nil?
   end
@@ -368,15 +365,6 @@ class User < ApplicationRecord
     setting_display_media == 'hide_all'
   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, **kwargs)

app/policies/user_policy.rb

@@ -13,14 +13,6 @@ class UserPolicy < ApplicationPolicy
     admin? && !record.staff?
   end
 
-  def disable_sign_in_token_auth?
-    staff?
-  end
-
-  def enable_sign_in_token_auth?
-    staff?
-  end
-
   def confirm?
     staff? && !record.confirmed?
   end

app/views/admin/accounts/show.html.haml

@@ -128,17 +128,11 @@
             %td{ rowspan: can?(:reset_password, @account.user) ? 2 : 1 }
               - if @account.user&.two_factor_enabled?
                 = t 'admin.accounts.security_measures.password_and_2fa'
-              - elsif @account.user&.skip_sign_in_token?
-                = t 'admin.accounts.security_measures.only_password'
               - else
-                = t 'admin.accounts.security_measures.password_and_sign_in_token'
+                = t 'admin.accounts.security_measures.only_password'
             %td
               - if @account.user&.two_factor_enabled?
                 = table_link_to 'unlock', t('admin.accounts.disable_two_factor_authentication'), admin_user_two_factor_authentication_path(@account.user.id), method: :delete if can?(:disable_2fa, @account.user)
-              - elsif @account.user&.skip_sign_in_token?
-                = table_link_to 'lock', t('admin.accounts.enable_sign_in_token_auth'), admin_user_sign_in_token_authentication_path(@account.user.id), method: :post if can?(:enable_sign_in_token_auth, @account.user)
-              - else
-                = table_link_to 'unlock', t('admin.accounts.disable_sign_in_token_auth'), admin_user_sign_in_token_authentication_path(@account.user.id), method: :delete if can?(:disable_sign_in_token_auth, @account.user)
 
           - if can?(:reset_password, @account.user)
             %tr

app/views/auth/sessions/sign_in_token.html.haml

@@ -1,14 +0,0 @@
-- 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))

app/views/user_mailer/suspicious_sign_in.html.haml

@@ -13,32 +13,14 @@
                         %tbody
                           %tr
                             %td.column-cell.text-center.padded
-                              %table.hero-icon.alert-icon{ align: 'center', cellspacing: 0, cellpadding: 0 }
+                              %table.hero-icon.warning-icon{ align: 'center', cellspacing: 0, cellpadding: 0 }
                                 %tbody
                                   %tr
                                     %td
-                                      = image_tag full_pack_url('media/images/mailer/icon_email.png'), alt: ''
+                                      = image_tag full_pack_url('media/images/mailer/icon_lock_open.png'), alt: ''
 
-                              %h1= t 'user_mailer.sign_in_token.title'
+                              %h1= t 'user_mailer.suspicious_sign_in.title'
-                              %p.lead= t 'user_mailer.sign_in_token.explanation'
+                              %p= t 'user_mailer.suspicious_sign_in.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
@@ -55,7 +37,7 @@
                         %tbody
                           %tr
                             %td.column-cell.text-center
-                              %p= t 'user_mailer.sign_in_token.details'
+                              %p= t 'user_mailer.suspicious_sign_in.details'
                           %tr
                             %td.column-cell.text-center
                               %p
@@ -82,24 +64,4 @@
                         %tbody
                           %tr
                             %td.column-cell.text-center
-                              %p= t 'user_mailer.sign_in_token.further_actions'
+                              %p= t 'user_mailer.suspicious_sign_in.further_actions_html', action: link_to(t('user_mailer.suspicious_sign_in.change_password'), edit_user_registration_url)
-
-%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'

app/views/user_mailer/suspicious_sign_in.text.erb

@@ -1,17 +1,15 @@
-<%= t 'user_mailer.sign_in_token.title' %>
+<%= t 'user_mailer.suspicious_sign_in.title' %>
 
 ===
 
-<%= t 'user_mailer.sign_in_token.explanation' %>
+<%= t 'user_mailer.suspicious_sign_in.explanation' %>
 
-=> <%= @resource.sign_in_token %>
+<%= t 'user_mailer.suspicious_sign_in.details' %>
-
-<%= 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' %>
+<%= t 'user_mailer.suspicious_sign_in.further_actions_html', action: t('user_mailer.suspicious_sign_in.change_password') %>
 
 => <%= edit_user_registration_url %>

config/locales/en.yml

@@ -199,7 +199,6 @@ en:
       security_measures:
         only_password: Only password
         password_and_2fa: Password and 2FA
-        password_and_sign_in_token: Password and e-mail token
       sensitive: Force-sensitive
       sensitized: Marked as sensitive
       shared_inbox_url: Shared inbox URL
@@ -1634,12 +1633,13 @@ en:
       explanation: You requested a full backup of your Mastodon account. It's now ready for download!
       subject: Your archive is ready for download
       title: Archive takeout
-    sign_in_token:
+    suspicious_sign_in:
-      details: 'Here are details of the attempt:'
+      change_password: change your password
-      explanation: 'We detected an attempt to sign in to your account from an unrecognized IP address. If this is you, please enter the security code below on the sign in challenge page:'
+      details: 'Here are details of the sign-in:'
-      further_actions: 'If this wasn''t you, please change your password and enable two-factor authentication on your account. You can do so here:'
+      explanation: We've detected a sign-in to your account from a new IP address.
-      subject: Please confirm attempted sign in
+      further_actions_html: If this wasn't you, we recommend that you %{action} immediately and enable two-factor authentication to keep your account secure.
-      title: Sign in attempt
+      subject: Your account has been accessed from a new IP address
+      title: A new sign-in
     warning:
       appeal: Submit an appeal
       appeal_description: If you believe this is an error, you can submit an appeal to the staff of %{instance}.
@@ -1690,13 +1690,10 @@ en:
       title: Welcome aboard, %{name}!
   users:
     follow_limit_reached: You cannot follow more than %{limit} people
-    generic_access_help_html: Trouble accessing your account? You may get in touch with %{email} for assistance
     invalid_otp_token: Invalid two-factor code
-    invalid_sign_in_token: Invalid security code
     otp_lost_help_html: If you lost access to both, you may get in touch with %{email}
     seamless_external_login: You are logged in via an external service, so password and e-mail settings are not available.
     signed_in_as: 'Signed in as:'
-    suspicious_sign_in_confirmation: You appear to not have logged in from this device before, so we're sending a security code to your e-mail address to confirm that it's you.
   verification:
     explanation_html: 'You can <strong>verify yourself as the owner of the links in your profile metadata</strong>. For that, the linked website must contain a link back to your Mastodon profile. The link back <strong>must</strong> have a <code>rel="me"</code> attribute. The text content of the link does not matter. Here is an example:'
     verification: Verification

config/routes.rb

@@ -295,7 +295,6 @@ Rails.application.routes.draw do
 
     resources :users, only: [] do
       resource :two_factor_authentication, only: [:destroy]
-      resource :sign_in_token_authentication, only: [:create, :destroy]
     end
 
     resources :custom_emojis, only: [:index, :new, :create] do

lib/mastodon/accounts_cli.rb

@@ -55,7 +55,6 @@ module Mastodon
     option :email, required: true
     option :confirmed, type: :boolean
     option :role, default: 'user', enum: %w(user moderator admin)
-    option :skip_sign_in_token, type: :boolean
     option :reattach, type: :boolean
     option :force, type: :boolean
     desc 'create USERNAME', 'Create a new user'
@@ -69,9 +68,6 @@ module Mastodon
       With the --role option one of  "user", "admin" or "moderator"
       can be supplied. Defaults to "user"
 
-      With the --skip-sign-in-token option, you can ensure that
-      the user is never asked for an e-mailed security code.
-
       With the --reattach option, the new user will be reattached
       to a given existing username of an old account. If the old
       account is still in use by someone else, you can supply
@@ -81,7 +77,7 @@ module Mastodon
     def create(username)
       account  = Account.new(username: username)
       password = SecureRandom.hex
-      user     = User.new(email: options[:email], password: password, agreement: true, approved: true, admin: options[:role] == 'admin', moderator: options[:role] == 'moderator', confirmed_at: options[:confirmed] ? Time.now.utc : nil, bypass_invite_request_check: true, skip_sign_in_token: options[:skip_sign_in_token])
+      user     = User.new(email: options[:email], password: password, agreement: true, approved: true, admin: options[:role] == 'admin', moderator: options[:role] == 'moderator', confirmed_at: options[:confirmed] ? Time.now.utc : nil, bypass_invite_request_check: true)
 
       if options[:reattach]
         account = Account.find_local(username) || Account.new(username: username)
@@ -125,7 +121,6 @@ module Mastodon
     option :disable_2fa, type: :boolean
     option :approve, type: :boolean
     option :reset_password, type: :boolean
-    option :skip_sign_in_token, type: :boolean
     desc 'modify USERNAME', 'Modify a user'
     long_desc <<-LONG_DESC
       Modify a user account.
@@ -147,9 +142,6 @@ module Mastodon
 
       With the --reset-password option, the user's password is replaced by
       a randomly-generated one, printed in the output.
-
-      With the --skip-sign-in-token option, you can ensure that
-      the user is never asked for an e-mailed security code.
     LONG_DESC
     def modify(username)
       user = Account.find_local(username)&.user
@@ -171,7 +163,6 @@ module Mastodon
       user.disabled = true if options[:disable]
       user.approved = true if options[:approve]
       user.otp_required_for_login = false if options[:disable_2fa]
-      user.skip_sign_in_token = options[:skip_sign_in_token] unless options[:skip_sign_in_token].nil?
       user.confirm if options[:confirm]
 
       if user.save

spec/controllers/auth/sessions_controller_spec.rb

@@ -225,22 +225,6 @@ RSpec.describe Auth::SessionsController, type: :controller do
           end
         end
 
-        context 'using email and password after an unfinished log-in attempt with a sign-in token challenge' do
-          let!(:other_user) do
-            Fabricate(:user, email: 'z@y.com', password: 'abcdefgh', otp_required_for_login: false, current_sign_in_at: 1.month.ago)
-          end
-
-          before do
-            post :create, params: { user: { email: other_user.email, password: other_user.password } }
-            post :create, params: { user: { email: user.email, password: user.password } }
-          end
-
-          it 'renders two factor authentication page' do
-            expect(controller).to render_template("two_factor")
-            expect(controller).to render_template(partial: "_otp_authentication_form")
-          end
-        end
-
         context 'using upcase email and password' do
           before do
             post :create, params: { user: { email: user.email.upcase, password: user.password } }
@@ -266,21 +250,6 @@ RSpec.describe Auth::SessionsController, type: :controller do
           end
         end
 
-        context 'using a valid OTP, attempting to leverage previous half-login to bypass password auth' do
-          let!(:other_user) do
-            Fabricate(:user, email: 'z@y.com', password: 'abcdefgh', otp_required_for_login: false, current_sign_in_at: 1.month.ago)
-          end
-
-          before do
-            post :create, params: { user: { email: other_user.email, password: other_user.password } }
-            post :create, params: { user: { email: user.email, otp_attempt: user.current_otp } }, session: { attempt_user_updated_at: user.updated_at.to_s }
-          end
-
-          it "doesn't log the user in" do
-            expect(controller.current_user).to be_nil
-          end
-        end
-
         context 'when the server has an decryption error' do
           before do
             allow_any_instance_of(User).to receive(:validate_and_consume_otp!).and_raise(OpenSSL::Cipher::CipherError)
@@ -401,126 +370,6 @@ RSpec.describe Auth::SessionsController, type: :controller do
         end
       end
     end
-
-    context 'when 2FA is disabled and IP is unfamiliar' do
-      let!(:user) { Fabricate(:user, email: 'x@y.com', password: 'abcdefgh', current_sign_in_at: 3.weeks.ago) }
-
-      before do
-        request.remote_ip  = '10.10.10.10'
-        request.user_agent = 'Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:75.0) Gecko/20100101 Firefox/75.0'
-
-        allow(UserMailer).to receive(:sign_in_token).and_return(double('email', deliver_later!: nil))
-      end
-
-      context 'using email and password' do
-        before do
-          post :create, params: { user: { email: user.email, password: user.password } }
-        end
-
-        it 'renders sign in token authentication page' do
-          expect(controller).to render_template("sign_in_token")
-        end
-
-        it 'generates sign in token' do
-          expect(user.reload.sign_in_token).to_not be_nil
-        end
-
-        it 'sends sign in token e-mail' do
-          expect(UserMailer).to have_received(:sign_in_token)
-        end
-      end
-
-      context 'using email and password after an unfinished log-in attempt to a 2FA-protected account' do
-        let!(:other_user) do
-          Fabricate(:user, email: 'z@y.com', password: 'abcdefgh', otp_required_for_login: true, otp_secret: User.generate_otp_secret(32))
-        end
-
-        before do
-          post :create, params: { user: { email: other_user.email, password: other_user.password } }
-          post :create, params: { user: { email: user.email, password: user.password } }
-        end
-
-        it 'renders sign in token authentication page' do
-          expect(controller).to render_template("sign_in_token")
-        end
-
-        it 'generates sign in token' do
-          expect(user.reload.sign_in_token).to_not be_nil
-        end
-
-        it 'sends sign in token e-mail' do
-          expect(UserMailer).to have_received(:sign_in_token)
-        end
-      end
-
-      context 'using email and password after an unfinished log-in attempt with a sign-in token challenge' do
-        let!(:other_user) do
-          Fabricate(:user, email: 'z@y.com', password: 'abcdefgh', otp_required_for_login: false, current_sign_in_at: 1.month.ago)
-        end
-
-        before do
-          post :create, params: { user: { email: other_user.email, password: other_user.password } }
-          post :create, params: { user: { email: user.email, password: user.password } }
-        end
-
-        it 'renders sign in token authentication page' do
-          expect(controller).to render_template("sign_in_token")
-        end
-
-        it 'generates sign in token' do
-          expect(user.reload.sign_in_token).to_not be_nil
-        end
-
-        it 'sends sign in token e-mail' do
-          expect(UserMailer).to have_received(:sign_in_token).with(user, any_args)
-        end
-      end
-
-      context 'using a valid sign in token' do
-        before do
-          user.generate_sign_in_token && user.save
-          post :create, params: { user: { sign_in_token_attempt: user.sign_in_token } }, session: { attempt_user_id: user.id, attempt_user_updated_at: user.updated_at.to_s }
-        end
-
-        it 'redirects to home' do
-          expect(response).to redirect_to(root_path)
-        end
-
-        it 'logs the user in' do
-          expect(controller.current_user).to eq user
-        end
-      end
-
-      context 'using a valid sign in token, attempting to leverage previous half-login to bypass password auth' do
-        let!(:other_user) do
-          Fabricate(:user, email: 'z@y.com', password: 'abcdefgh', otp_required_for_login: false, current_sign_in_at: 1.month.ago)
-        end
-
-        before do
-          user.generate_sign_in_token && user.save
-          post :create, params: { user: { email: other_user.email, password: other_user.password } }
-          post :create, params: { user: { email: user.email, sign_in_token_attempt: user.sign_in_token } }, session: { attempt_user_updated_at: user.updated_at.to_s }
-        end
-
-        it "doesn't log the user in" do
-          expect(controller.current_user).to be_nil
-        end
-      end
-
-      context 'using an invalid sign in token' do
-        before do
-          post :create, params: { user: { sign_in_token_attempt: 'wrongotp' } }, session: { attempt_user_id: user.id, attempt_user_updated_at: user.updated_at.to_s }
-        end
-
-        it 'shows a login error' do
-          expect(flash[:alert]).to match I18n.t('users.invalid_sign_in_token')
-        end
-
-        it "doesn't log the user in" do
-          expect(controller.current_user).to be_nil
-        end
-      end
-    end
   end
 
   describe 'GET #webauthn_options' do

spec/lib/suspicious_sign_in_detector_spec.rb

@@ -0,0 +1,57 @@
+require 'rails_helper'
+
+RSpec.describe SuspiciousSignInDetector do
+  describe '#suspicious?' do
+    let(:user) { Fabricate(:user, current_sign_in_at: 1.day.ago) }
+    let(:request) { double(remote_ip: remote_ip) }
+    let(:remote_ip) { nil }
+
+    subject { described_class.new(user).suspicious?(request) }
+
+    context 'when user has 2FA enabled' do
+      before do
+        user.update!(otp_required_for_login: true)
+      end
+
+      it 'returns false' do
+        expect(subject).to be false
+      end
+    end
+
+    context 'when exact IP has been used before' do
+      let(:remote_ip) { '1.1.1.1' }
+
+      before do
+        user.update!(sign_up_ip: remote_ip)
+      end
+
+      it 'returns false' do
+        expect(subject).to be false
+      end
+    end
+
+    context 'when similar IP has been used before' do
+      let(:remote_ip) { '1.1.2.2' }
+
+      before do
+        user.update!(sign_up_ip: '1.1.1.1')
+      end
+
+      it 'returns false' do
+        expect(subject).to be false
+      end
+    end
+
+    context 'when IP is completely unfamiliar' do
+      let(:remote_ip) { '2.2.2.2' }
+
+      before do
+        user.update!(sign_up_ip: '1.1.1.1')
+      end
+
+      it 'returns true' do
+        expect(subject).to be true
+      end
+    end
+  end
+end

spec/mailers/previews/user_mailer_preview.rb

@@ -87,8 +87,8 @@ class UserMailerPreview < ActionMailer::Preview
     UserMailer.appeal_approved(User.first, Appeal.last)
   end
 
-  # Preview this email at http://localhost:3000/rails/mailers/user_mailer/sign_in_token
+  # Preview this email at http://localhost:3000/rails/mailers/user_mailer/suspicious_sign_in
-  def sign_in_token
+  def suspicious_sign_in
-    UserMailer.sign_in_token(User.first.tap { |user| user.generate_sign_in_token }, '127.0.0.1', 'Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:75.0) Gecko/20100101 Firefox/75.0', Time.now.utc)
+    UserMailer.suspicious_sign_in(User.first, '127.0.0.1', 'Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:75.0) Gecko/20100101 Firefox/75.0', Time.now.utc)
   end
 end