diff --git a/app/controllers/admin/sign_in_token_authentications_controller.rb b/app/controllers/admin/sign_in_token_authentications_controller.rb
deleted file mode 100644
index e620ab292..000000000
--- a/app/controllers/admin/sign_in_token_authentications_controller.rb
+++ /dev/null
@@ -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
diff --git a/app/controllers/auth/sessions_controller.rb b/app/controllers/auth/sessions_controller.rb
index 4d2695bf5..c4c8151e3 100644
--- a/app/controllers/auth/sessions_controller.rb
+++ b/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)
diff --git a/app/controllers/concerns/sign_in_token_authentication_concern.rb b/app/controllers/concerns/sign_in_token_authentication_concern.rb
deleted file mode 100644
index 384c5c50c..000000000
--- a/app/controllers/concerns/sign_in_token_authentication_concern.rb
+++ /dev/null
@@ -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
diff --git a/app/javascript/styles/mailer.scss b/app/javascript/styles/mailer.scss
index 34852178e..18fe522eb 100644
--- a/app/javascript/styles/mailer.scss
+++ b/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;
}
diff --git a/app/lib/suspicious_sign_in_detector.rb b/app/lib/suspicious_sign_in_detector.rb
new file mode 100644
index 000000000..1af5188c6
--- /dev/null
+++ b/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
diff --git a/app/mailers/user_mailer.rb b/app/mailers/user_mailer.rb
index 1a823328c..ce36dd6f5 100644
--- a/app/mailers/user_mailer.rb
+++ b/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,
- subject: I18n.t("user_mailer.warning.subject.#{@warning.action}", acct: "@#{user.account.local_username_and_domain}"),
- reply_to: ENV['SMTP_REPLY_TO']
+ mail to: @resource.email, subject: I18n.t("user_mailer.warning.subject.#{@warning.action}", acct: "@#{user.account.local_username_and_domain}")
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,
- subject: I18n.t('user_mailer.sign_in_token.subject'),
- reply_to: ENV['SMTP_REPLY_TO']
+ mail to: @resource.email, subject: I18n.t('user_mailer.suspicious_sign_in.subject')
end
end
end
diff --git a/app/models/user.rb b/app/models/user.rb
index e25c0ddb0..d19fe2c92 100644
--- a/app/models/user.rb
+++ b/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)
diff --git a/app/policies/user_policy.rb b/app/policies/user_policy.rb
index 92e2c4f4b..140905e1f 100644
--- a/app/policies/user_policy.rb
+++ b/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
diff --git a/app/views/admin/accounts/show.html.haml b/app/views/admin/accounts/show.html.haml
index 1230294fe..a69832b04 100644
--- a/app/views/admin/accounts/show.html.haml
+++ b/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
diff --git a/app/views/auth/sessions/sign_in_token.html.haml b/app/views/auth/sessions/sign_in_token.html.haml
deleted file mode 100644
index 8923203cd..000000000
--- a/app/views/auth/sessions/sign_in_token.html.haml
+++ /dev/null
@@ -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))
diff --git a/app/views/user_mailer/sign_in_token.html.haml b/app/views/user_mailer/suspicious_sign_in.html.haml
similarity index 55%
rename from app/views/user_mailer/sign_in_token.html.haml
rename to app/views/user_mailer/suspicious_sign_in.html.haml
index 826b34e7c..856f9fb7c 100644
--- a/app/views/user_mailer/sign_in_token.html.haml
+++ b/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'
- %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
+ %h1= t 'user_mailer.suspicious_sign_in.title'
+ %p= t 'user_mailer.suspicious_sign_in.explanation'
%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'
-
-%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'
+ %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)
diff --git a/app/views/user_mailer/sign_in_token.text.erb b/app/views/user_mailer/suspicious_sign_in.text.erb
similarity index 56%
rename from app/views/user_mailer/sign_in_token.text.erb
rename to app/views/user_mailer/suspicious_sign_in.text.erb
index 2539ddaf6..7d2ca28e8 100644
--- a/app/views/user_mailer/sign_in_token.text.erb
+++ b/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.sign_in_token.details' %>
+<%= t 'user_mailer.suspicious_sign_in.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 %>
diff --git a/config/locales/en.yml b/config/locales/en.yml
index 4a16098a8..4fa9abc51 100644
--- a/config/locales/en.yml
+++ b/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:
- details: 'Here are details of the attempt:'
- 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:'
- further_actions: 'If this wasn''t you, please change your password and enable two-factor authentication on your account. You can do so here:'
- subject: Please confirm attempted sign in
- title: Sign in attempt
+ suspicious_sign_in:
+ change_password: change your password
+ details: 'Here are details of the sign-in:'
+ explanation: We've detected a sign-in to your account from a new IP address.
+ further_actions_html: If this wasn't you, we recommend that you %{action} immediately and enable two-factor authentication to keep your account secure.
+ 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 verify yourself as the owner of the links in your profile metadata. For that, the linked website must contain a link back to your Mastodon profile. The link back must have a rel="me"
attribute. The text content of the link does not matter. Here is an example:'
verification: Verification
diff --git a/config/routes.rb b/config/routes.rb
index 7c9a13dc4..b05d31e4e 100644
--- a/config/routes.rb
+++ b/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
diff --git a/lib/mastodon/accounts_cli.rb b/lib/mastodon/accounts_cli.rb
index 2ef85d0a9..7256d1da9 100644
--- a/lib/mastodon/accounts_cli.rb
+++ b/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
diff --git a/spec/controllers/auth/sessions_controller_spec.rb b/spec/controllers/auth/sessions_controller_spec.rb
index 64ec7b794..1b8fd0b7b 100644
--- a/spec/controllers/auth/sessions_controller_spec.rb
+++ b/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
diff --git a/spec/lib/suspicious_sign_in_detector_spec.rb b/spec/lib/suspicious_sign_in_detector_spec.rb
new file mode 100644
index 000000000..101a18aa0
--- /dev/null
+++ b/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
diff --git a/spec/mailers/previews/user_mailer_preview.rb b/spec/mailers/previews/user_mailer_preview.rb
index 8de7d8669..95712e6cf 100644
--- a/spec/mailers/previews/user_mailer_preview.rb
+++ b/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
- def sign_in_token
- 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)
+ # Preview this email at http://localhost:3000/rails/mailers/user_mailer/suspicious_sign_in
+ def suspicious_sign_in
+ 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