treehouse
/
mastodon
22
8
Fork 5
Code Issues 32 Pull Requests Projects 1 Releases Wiki Activity

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

lolsob-rspec
Eugen Rochko 2020-06-09 10:23:06 +02:00 committed by GitHub
parent 654c13ecfb
commit 2dbf6bc5ad
14 changed files with 368 additions and 51 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

50
app/controllers/auth/sessions_controller.rb
View File

@ -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

49
app/controllers/concerns/sign_in_token_authentication_concern.rb Normal file
View File

@ -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

47
app/controllers/concerns/two_factor_authentication_concern.rb Normal file
View File

@ -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

17
app/mailers/user_mailer.rb
View File

@ -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

28
app/models/user.rb
View File

@ -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)

14
app/views/auth/sessions/sign_in_token.html.haml Normal file
View File

@ -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))

105
app/views/user_mailer/sign_in_token.html.haml Normal file
View File

@ -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'

17
app/views/user_mailer/sign_in_token.text.erb Normal file
View File

@ -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 %>

9
config/locales/en.yml
View File

@ -1273,6 +1273,12 @@ 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
warning:
explanation:
disable: While your account is frozen, your account data remains intact, but you cannot perform any actions until it is unlocked.
@ -1310,11 +1316,14 @@ 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_email: The e-mail address is invalid
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, and you haven't logged in for a while, 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

1
config/locales/simple_form.en.yml
View File

@ -151,6 +151,7 @@ en:
setting_use_blurhash: Show colorful gradients for hidden media
setting_use_pending_items: Slow mode
severity: Severity
sign_in_token_attempt: Security code
type: Import type
username: Username
username_or_email: Username or Email

6
db/migrate/20200608113046_add_sign_in_token_to_users.rb Normal file
View File

@ -0,0 +1,6 @@
class AddSignInTokenToUsers < ActiveRecord::Migration[5.2]
def change
add_column :users, :sign_in_token, :string
add_column :users, :sign_in_token_sent_at, :datetime
end
end

5
db/schema.rb
View File

@ -10,7 +10,8 @@
#
# It's strongly recommended that you check this file into your version control system.
ActiveRecord::Schema.define(version: 2020_06_05_155027) do
ActiveRecord::Schema.define(version: 2020_06_08_113046) do
# These are extensions that must be enabled in order to support this database
enable_extension "plpgsql"
@ -869,6 +870,8 @@ ActiveRecord::Schema.define(version: 2020_06_05_155027) do
t.string "chosen_languages", array: true
t.bigint "created_by_application_id"
t.boolean "approved", default: true, null: false
t.string "sign_in_token"
t.datetime "sign_in_token_sent_at"
t.index ["account_id"], name: "index_users_on_account_id"
t.index ["confirmation_token"], name: "index_users_on_confirmation_token", unique: true
t.index ["created_by_application_id"], name: "index_users_on_created_by_application_id"

66
spec/controllers/auth/sessions_controller_spec.rb
View File

@ -215,7 +215,7 @@ RSpec.describe Auth::SessionsController, type: :controller do
context 'using a valid OTP' do
before do
post :create, params: { user: { otp_attempt: user.current_otp } }, session: { otp_user_id: user.id }
post :create, params: { user: { otp_attempt: user.current_otp } }, session: { attempt_user_id: user.id }
end
it 'redirects to home' do
@ -230,7 +230,7 @@ RSpec.describe Auth::SessionsController, type: :controller do
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)
post :create, params: { user: { otp_attempt: user.current_otp } }, session: { otp_user_id: user.id }
post :create, params: { user: { otp_attempt: user.current_otp } }, session: { attempt_user_id: user.id }
end
it 'shows a login error' do
@ -244,7 +244,7 @@ RSpec.describe Auth::SessionsController, type: :controller do
context 'using a valid recovery code' do
before do
post :create, params: { user: { otp_attempt: recovery_codes.first } }, session: { otp_user_id: user.id }
post :create, params: { user: { otp_attempt: recovery_codes.first } }, session: { attempt_user_id: user.id }
end
it 'redirects to home' do
@ -258,7 +258,7 @@ RSpec.describe Auth::SessionsController, type: :controller do
context 'using an invalid OTP' do
before do
post :create, params: { user: { otp_attempt: 'wrongotp' } }, session: { otp_user_id: user.id }
post :create, params: { user: { otp_attempt: 'wrongotp' } }, session: { attempt_user_id: user.id }
end
it 'shows a login error' do
@ -270,5 +270,63 @@ 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, current_sign_in_ip: '0.0.0.0') }
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 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 }
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 an invalid sign in token' do
before do
post :create, params: { user: { sign_in_token_attempt: 'wrongotp' } }, session: { attempt_user_id: user.id }
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
end

5
spec/mailers/previews/user_mailer_preview.rb
View File

@ -59,4 +59,9 @@ class UserMailerPreview < ActionMailer::Preview
def warning
UserMailer.warning(User.first, AccountWarning.new(text: '', action: :silence), [Status.first.id])
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)
end
end