Add password challenge to 2FA settings, e-mail notifications (#11878)

Fix #3961
This commit is contained in:
Eugen Rochko 2019-09-18 16:37:27 +02:00 committed by GitHub
parent d0c2c52783
commit e1066cd431
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
32 changed files with 567 additions and 50 deletions

View file

@ -8,6 +8,7 @@ module Admin
authorize @user, :disable_2fa?
@user.disable_two_factor!
log_action :disable_2fa, @user
UserMailer.two_factor_disabled(@user).deliver_later!
redirect_to admin_accounts_path
end

View file

@ -0,0 +1,22 @@
# frozen_string_literal: true
class Auth::ChallengesController < ApplicationController
include ChallengableConcern
layout 'auth'
before_action :authenticate_user!
skip_before_action :require_functional!
def create
if challenge_passed?
session[:challenge_passed_at] = Time.now.utc
redirect_to challenge_params[:return_to]
else
@challenge = Form::Challenge.new(return_to: challenge_params[:return_to])
flash.now[:alert] = I18n.t('challenge.invalid_password')
render_challenge
end
end
end

View file

@ -42,6 +42,7 @@ class Auth::SessionsController < Devise::SessionsController
def destroy
tmp_stored_location = stored_location_for(:user)
super
session.delete(:challenge_passed_at)
flash.delete(:notice)
store_location_for(:user, tmp_stored_location) if continue_after?
end

View file

@ -0,0 +1,65 @@
# frozen_string_literal: true
# This concern is inspired by "sudo mode" on GitHub. It
# is a way to re-authenticate a user before allowing them
# to see or perform an action.
#
# Add `before_action :require_challenge!` to actions you
# want to protect.
#
# The user will be shown a page to enter the challenge (which
# is either the password, or just the username when no
# password exists). Upon passing, there is a grace period
# during which no challenge will be asked from the user.
#
# Accessing challenge-protected resources during the grace
# period will refresh the grace period.
module ChallengableConcern
extend ActiveSupport::Concern
CHALLENGE_TIMEOUT = 1.hour.freeze
def require_challenge!
return if skip_challenge?
if challenge_passed_recently?
session[:challenge_passed_at] = Time.now.utc
return
end
@challenge = Form::Challenge.new(return_to: request.url)
if params.key?(:form_challenge)
if challenge_passed?
session[:challenge_passed_at] = Time.now.utc
return
else
flash.now[:alert] = I18n.t('challenge.invalid_password')
render_challenge
end
else
render_challenge
end
end
def render_challenge
@body_classes = 'lighter'
render template: 'auth/challenges/new', layout: 'auth'
end
def challenge_passed?
current_user.valid_password?(challenge_params[:current_password])
end
def skip_challenge?
current_user.encrypted_password.blank?
end
def challenge_passed_recently?
session[:challenge_passed_at].present? && session[:challenge_passed_at] >= CHALLENGE_TIMEOUT.ago
end
def challenge_params
params.require(:form_challenge).permit(:current_password, :return_to)
end
end

View file

@ -3,9 +3,12 @@
module Settings
module TwoFactorAuthentication
class ConfirmationsController < BaseController
include ChallengableConcern
layout 'admin'
before_action :authenticate_user!
before_action :require_challenge!
before_action :ensure_otp_secret
skip_before_action :require_functional!
@ -22,6 +25,8 @@ module Settings
@recovery_codes = current_user.generate_otp_backup_codes!
current_user.save!
UserMailer.two_factor_enabled(current_user).deliver_later!
render 'settings/two_factor_authentication/recovery_codes/index'
else
flash.now[:alert] = I18n.t('two_factor_authentication.wrong_code')

View file

@ -3,16 +3,22 @@
module Settings
module TwoFactorAuthentication
class RecoveryCodesController < BaseController
include ChallengableConcern
layout 'admin'
before_action :authenticate_user!
before_action :require_challenge!, on: :create
skip_before_action :require_functional!
def create
@recovery_codes = current_user.generate_otp_backup_codes!
current_user.save!
UserMailer.two_factor_recovery_codes_changed(current_user).deliver_later!
flash.now[:notice] = I18n.t('two_factor_authentication.recovery_codes_regenerated')
render :index
end
end

View file

@ -2,10 +2,13 @@
module Settings
class TwoFactorAuthenticationsController < BaseController
include ChallengableConcern
layout 'admin'
before_action :authenticate_user!
before_action :verify_otp_required, only: [:create]
before_action :require_challenge!, only: [:create]
skip_before_action :require_functional!
@ -23,6 +26,7 @@ module Settings
if acceptable_code?
current_user.otp_required_for_login = false
current_user.save!
UserMailer.two_factor_disabled(current_user).deliver_later!
redirect_to settings_two_factor_authentication_path
else
flash.now[:alert] = I18n.t('two_factor_authentication.wrong_code')

View file

@ -233,6 +233,8 @@ hr.spacer {
height: 1px;
}
body,
.admin-wrapper .content {
.muted-hint {
color: $darker-text-color;
@ -260,6 +262,7 @@ hr.spacer {
color: $gold-star;
font-weight: 500;
}
}
.filters {
display: flex;

View file

@ -254,6 +254,10 @@ code {
&-6 {
max-width: 50%;
}
.actions {
margin-top: 27px;
}
}
.fields-group:last-child,

View file

@ -57,6 +57,39 @@ class UserMailer < Devise::Mailer
end
end
def two_factor_enabled(user, **)
@resource = user
@instance = Rails.configuration.x.local_domain
return if @resource.disabled?
I18n.with_locale(@resource.locale || I18n.default_locale) do
mail to: @resource.email, subject: I18n.t('devise.mailer.two_factor_enabled.subject')
end
end
def two_factor_disabled(user, **)
@resource = user
@instance = Rails.configuration.x.local_domain
return if @resource.disabled?
I18n.with_locale(@resource.locale || I18n.default_locale) do
mail to: @resource.email, subject: I18n.t('devise.mailer.two_factor_disabled.subject')
end
end
def two_factor_recovery_codes_changed(user, **)
@resource = user
@instance = Rails.configuration.x.local_domain
return if @resource.disabled?
I18n.with_locale(@resource.locale || I18n.default_locale) do
mail to: @resource.email, subject: I18n.t('devise.mailer.two_factor_recovery_codes_changed.subject')
end
end
def welcome(user)
@resource = user
@instance = Rails.configuration.x.local_domain

View file

@ -0,0 +1,8 @@
# frozen_string_literal: true
class Form::Challenge
include ActiveModel::Model
attr_accessor :current_password, :current_username,
:return_to
end

View file

@ -264,17 +264,20 @@ class User < ApplicationRecord
end
def password_required?
return false if Devise.pam_authentication || Devise.ldap_authentication
return false if external?
super
end
def send_reset_password_instructions
return false if encrypted_password.blank? && (Devise.pam_authentication || Devise.ldap_authentication)
return false if encrypted_password.blank?
super
end
def reset_password!(new_password, new_password_confirmation)
return false if encrypted_password.blank? && (Devise.pam_authentication || Devise.ldap_authentication)
return false if encrypted_password.blank?
super
end

View file

@ -0,0 +1,15 @@
- content_for :page_title do
= t('challenge.prompt')
= simple_form_for @challenge, url: request.get? ? auth_challenge_path : '' do |f|
= f.input :return_to, as: :hidden
.field-group
= f.input :current_password, wrapper: :with_block_label, input_html: { :autocomplete => 'off', :autofocus => true }, label: t('challenge.prompt'), required: true
.actions
= f.button :button, t('challenge.confirm'), type: :submit
%p.hint.subtle-hint= t('challenge.hint_html')
.form-footer= render 'auth/shared/links'

View file

@ -11,7 +11,7 @@
- if controller_name != 'passwords' && controller_name != 'registrations'
%li= link_to t('auth.forgot_password'), new_user_password_path
- if controller_name != 'confirmations'
- if controller_name != 'confirmations' && (!user_signed_in? || !current_user.confirmed? || current_user.unconfirmed_email.present?)
%li= link_to t('auth.didnt_get_confirmation'), new_user_confirmation_path
- if user_signed_in? && controller_name != 'setup'

View file

@ -2,33 +2,35 @@
= t('settings.two_factor_authentication')
- if current_user.otp_required_for_login
%p.positive-hint
%p.hint
%span.positive-hint
= fa_icon 'check'
= ' '
= t 'two_factor_authentication.enabled'
%hr/
%hr.spacer/
= simple_form_for @confirmation, url: settings_two_factor_authentication_path, method: :delete do |f|
= f.input :otp_attempt, wrapper: :with_label, hint: t('two_factor_authentication.code_hint'), label: t('simple_form.labels.defaults.otp_attempt'), input_html: { :autocomplete => 'off' }, required: true
.fields-group
= f.input :otp_attempt, wrapper: :with_block_label, hint: t('two_factor_authentication.code_hint'), label: t('simple_form.labels.defaults.otp_attempt'), input_html: { :autocomplete => 'off' }, required: true
.actions
= f.button :button, t('two_factor_authentication.disable'), type: :submit
= f.button :button, t('two_factor_authentication.disable'), type: :submit, class: 'negative'
%hr/
%hr.spacer/
%h6= t('two_factor_authentication.recovery_codes')
%p.muted-hint
= t('two_factor_authentication.lost_recovery_codes')
= link_to t('two_factor_authentication.generate_recovery_codes'),
settings_two_factor_authentication_recovery_codes_path,
data: { method: :post }
%h3= t('two_factor_authentication.recovery_codes')
%p.muted-hint= t('two_factor_authentication.lost_recovery_codes')
%hr.spacer/
.simple_form
= link_to t('two_factor_authentication.generate_recovery_codes'), settings_two_factor_authentication_recovery_codes_path, data: { method: :post }, class: 'block-button'
- else
.simple_form
%p.hint= t('two_factor_authentication.description_html')
= link_to t('two_factor_authentication.setup'),
settings_two_factor_authentication_path,
data: { method: :post },
class: 'block-button'
%hr.spacer/
= link_to t('two_factor_authentication.setup'), settings_two_factor_authentication_path, data: { method: :post }, class: 'block-button'

View file

@ -0,0 +1,43 @@
%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_lock_open.png'), alt: ''
%h1= t 'devise.mailer.two_factor_disabled.title'
%p.lead= t 'devise.mailer.two_factor_disabled.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.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')

View file

@ -0,0 +1,7 @@
<%= t 'devise.mailer.two_factor_disabled.title' %>
===
<%= t 'devise.mailer.two_factor_disabled.explanation' %>
=> <%= edit_user_registration_url %>

View file

@ -0,0 +1,43 @@
%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{ align: 'center', cellspacing: 0, cellpadding: 0 }
%tbody
%tr
%td
= image_tag full_pack_url('media/images/mailer/icon_lock_open.png'), alt: ''
%h1= t 'devise.mailer.two_factor_enabled.title'
%p.lead= t 'devise.mailer.two_factor_enabled.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.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')

View file

@ -0,0 +1,7 @@
<%= t 'devise.mailer.two_factor_enabled.title' %>
===
<%= t 'devise.mailer.two_factor_enabled.explanation' %>
=> <%= edit_user_registration_url %>

View file

@ -0,0 +1,43 @@
%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_lock_open.png'), alt: ''
%h1= t 'devise.mailer.two_factor_recovery_codes_changed.title'
%p.lead= t 'devise.mailer.two_factor_recovery_codes_changed.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.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')

View file

@ -0,0 +1,7 @@
<%= t 'devise.mailer.two_factor_recovery_codes_changed.title' %>
===
<%= t 'devise.mailer.two_factor_recovery_codes_changed.explanation' %>
=> <%= edit_user_registration_url %>

View file

@ -46,6 +46,18 @@ en:
extra: If you didn't request this, please ignore this email. Your password won't change until you access the link above and create a new one.
subject: 'Mastodon: Reset password instructions'
title: Password reset
two_factor_disabled:
explanation: Two-factor authentication for your account has been disabled. Login is now possible using only e-mail address and password.
subject: 'Mastodon: Two-factor authentication disabled'
title: 2FA disabled
two_factor_enabled:
explanation: Two-factor authentication has been enabled for your account. A token generated by the paired TOTP app will be required for login.
subject: 'Mastodon: Two-factor authentication enabled'
title: 2FA enabled
two_factor_recovery_codes_changed:
explanation: The previous recovery codes have been invalidated and new ones generated.
subject: 'Mastodon: Two-factor recovery codes re-generated'
title: 2FA recovery codes changed
unlock_instructions:
subject: 'Mastodon: Unlock instructions'
omniauth_callbacks:

View file

@ -621,6 +621,11 @@ en:
return: Show the user's profile
web: Go to web
title: Follow %{acct}
challenge:
confirm: Continue
hint_html: "<strong>Tip:</strong> We won't ask you for your password again for the next hour."
invalid_password: Invalid password
prompt: Confirm password to continue
datetime:
distance_in_words:
about_x_hours: "%{count}h"

View file

@ -43,6 +43,8 @@ en:
domain: This domain will be able to fetch data from this server and incoming data from it will be processed and stored
featured_tag:
name: 'You might want to use one of these:'
form_challenge:
current_password: You are entering a secure area
imports:
data: CSV file exported from another Mastodon server
invite_request:

View file

@ -41,6 +41,7 @@ Rails.application.routes.draw do
namespace :auth do
resource :setup, only: [:show, :update], controller: :setup
resource :challenge, only: [:create], controller: :challenges
end
end

View file

@ -0,0 +1,46 @@
# frozen_string_literal: true
require 'rails_helper'
describe Auth::ChallengesController, type: :controller do
render_views
let(:password) { 'foobar12345' }
let(:user) { Fabricate(:user, password: password) }
before do
sign_in user
end
describe 'POST #create' do
let(:return_to) { edit_user_registration_path }
context 'with correct password' do
before { post :create, params: { form_challenge: { return_to: return_to, current_password: password } } }
it 'redirects back' do
expect(response).to redirect_to(return_to)
end
it 'sets session' do
expect(session[:challenge_passed_at]).to_not be_nil
end
end
context 'with incorrect password' do
before { post :create, params: { form_challenge: { return_to: return_to, current_password: 'hhfggjjd562' } } }
it 'renders challenge' do
expect(response).to render_template('auth/challenges/new')
end
it 'displays error' do
expect(response.body).to include 'Invalid password'
end
it 'does not set session' do
expect(session[:challenge_passed_at]).to be_nil
end
end
end
end

View file

@ -80,7 +80,7 @@ RSpec.describe Auth::SessionsController, type: :controller do
let(:user) do
account = Fabricate.build(:account, username: 'pam_user1')
account.save!(validate: false)
user = Fabricate(:user, email: 'pam@example.com', password: nil, account: account)
user = Fabricate(:user, email: 'pam@example.com', password: nil, account: account, external: true)
user
end

View file

@ -0,0 +1,114 @@
# frozen_string_literal: true
require 'rails_helper'
RSpec.describe ChallengableConcern, type: :controller do
controller(ApplicationController) do
include ChallengableConcern
before_action :require_challenge!
def foo
render plain: 'foo'
end
def bar
render plain: 'bar'
end
end
before do
routes.draw do
get 'foo' => 'anonymous#foo'
post 'bar' => 'anonymous#bar'
end
end
context 'with a no-password user' do
let(:user) { Fabricate(:user, external: true, password: nil) }
before do
sign_in user
end
context 'for GET requests' do
before { get :foo }
it 'does not ask for password' do
expect(response.body).to eq 'foo'
end
end
context 'for POST requests' do
before { post :bar }
it 'does not ask for password' do
expect(response.body).to eq 'bar'
end
end
end
context 'with recent challenge in session' do
let(:password) { 'foobar12345' }
let(:user) { Fabricate(:user, password: password) }
before do
sign_in user
end
context 'for GET requests' do
before { get :foo, session: { challenge_passed_at: Time.now.utc } }
it 'does not ask for password' do
expect(response.body).to eq 'foo'
end
end
context 'for POST requests' do
before { post :bar, session: { challenge_passed_at: Time.now.utc } }
it 'does not ask for password' do
expect(response.body).to eq 'bar'
end
end
end
context 'with a password user' do
let(:password) { 'foobar12345' }
let(:user) { Fabricate(:user, password: password) }
before do
sign_in user
end
context 'for GET requests' do
before { get :foo }
it 'renders challenge' do
expect(response).to render_template('auth/challenges/new')
end
# See Auth::ChallengesControllerSpec
end
context 'for POST requests' do
before { post :bar }
it 'renders challenge' do
expect(response).to render_template('auth/challenges/new')
end
it 'accepts correct password' do
post :bar, params: { form_challenge: { current_password: password } }
expect(response.body).to eq 'bar'
expect(session[:challenge_passed_at]).to_not be_nil
end
it 'rejects wrong password' do
post :bar, params: { form_challenge: { current_password: 'dddfff888123' } }
expect(response.body).to render_template('auth/challenges/new')
expect(session[:challenge_passed_at]).to be_nil
end
end
end
end

View file

@ -24,7 +24,7 @@ describe Settings::TwoFactorAuthentication::ConfirmationsController do
context 'when signed in' do
subject do
sign_in user, scope: :user
get :new
get :new, session: { challenge_passed_at: Time.now.utc }
end
include_examples 'renders :new'
@ -37,7 +37,7 @@ describe Settings::TwoFactorAuthentication::ConfirmationsController do
it 'redirects if user do not have otp_secret' do
sign_in user_without_otp_secret, scope: :user
get :new
get :new, session: { challenge_passed_at: Time.now.utc }
expect(response).to redirect_to('/settings/two_factor_authentication')
end
end
@ -50,7 +50,7 @@ describe Settings::TwoFactorAuthentication::ConfirmationsController do
describe 'when form_two_factor_confirmation parameter is not provided' do
it 'raises ActionController::ParameterMissing' do
post :create, params: {}
post :create, params: {}, session: { challenge_passed_at: Time.now.utc }
expect(response).to have_http_status(400)
end
end
@ -68,7 +68,7 @@ describe Settings::TwoFactorAuthentication::ConfirmationsController do
true
end
post :create, params: { form_two_factor_confirmation: { otp_attempt: '123456' } }
post :create, params: { form_two_factor_confirmation: { otp_attempt: '123456' } }, session: { challenge_passed_at: Time.now.utc }
expect(assigns(:recovery_codes)).to eq otp_backup_codes
expect(flash[:notice]).to eq 'Two-factor authentication successfully enabled'
@ -85,7 +85,7 @@ describe Settings::TwoFactorAuthentication::ConfirmationsController do
false
end
post :create, params: { form_two_factor_confirmation: { otp_attempt: '123456' } }
post :create, params: { form_two_factor_confirmation: { otp_attempt: '123456' } }, session: { challenge_passed_at: Time.now.utc }
end
it 'renders the new view' do

View file

@ -15,7 +15,7 @@ describe Settings::TwoFactorAuthentication::RecoveryCodesController do
end
sign_in user, scope: :user
post :create
post :create, session: { challenge_passed_at: Time.now.utc }
expect(assigns(:recovery_codes)).to eq otp_backup_codes
expect(flash[:notice]).to eq 'Recovery codes successfully regenerated'

View file

@ -58,7 +58,7 @@ describe Settings::TwoFactorAuthenticationsController do
describe 'when creation succeeds' do
it 'updates user secret' do
before = user.otp_secret
post :create
post :create, session: { challenge_passed_at: Time.now.utc }
expect(user.reload.otp_secret).not_to eq(before)
expect(response).to redirect_to(new_settings_two_factor_authentication_confirmation_path)

View file

@ -18,6 +18,21 @@ class UserMailerPreview < ActionMailer::Preview
UserMailer.password_change(User.first)
end
# Preview this email at http://localhost:3000/rails/mailers/user_mailer/two_factor_disabled
def two_factor_disabled
UserMailer.two_factor_disabled(User.first)
end
# Preview this email at http://localhost:3000/rails/mailers/user_mailer/two_factor_enabled
def two_factor_enabled
UserMailer.two_factor_enabled(User.first)
end
# Preview this email at http://localhost:3000/rails/mailers/user_mailer/two_factor_recovery_codes_changed
def two_factor_recovery_codes_changed
UserMailer.two_factor_recovery_codes_changed(User.first)
end
# Preview this email at http://localhost:3000/rails/mailers/user_mailer/reconfirmation_instructions
def reconfirmation_instructions
user = User.first