diff options
18 files changed, 137 insertions, 362 deletions
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 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 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 <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 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 |