diff options
32 files changed, 567 insertions, 50 deletions
diff --git a/app/controllers/admin/two_factor_authentications_controller.rb b/app/controllers/admin/two_factor_authentications_controller.rb index 2577a4b17..0652c3a7a 100644 --- a/app/controllers/admin/two_factor_authentications_controller.rb +++ b/app/controllers/admin/two_factor_authentications_controller.rb @@ -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 diff --git a/app/controllers/auth/challenges_controller.rb b/app/controllers/auth/challenges_controller.rb new file mode 100644 index 000000000..060944240 --- /dev/null +++ b/app/controllers/auth/challenges_controller.rb @@ -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 diff --git a/app/controllers/auth/sessions_controller.rb b/app/controllers/auth/sessions_controller.rb index 3e93b2e68..b3113bbef 100644 --- a/app/controllers/auth/sessions_controller.rb +++ b/app/controllers/auth/sessions_controller.rb @@ -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 diff --git a/app/controllers/concerns/challengable_concern.rb b/app/controllers/concerns/challengable_concern.rb new file mode 100644 index 000000000..b29d90b3c --- /dev/null +++ b/app/controllers/concerns/challengable_concern.rb @@ -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 diff --git a/app/controllers/settings/two_factor_authentication/confirmations_controller.rb b/app/controllers/settings/two_factor_authentication/confirmations_controller.rb index 46c90bf74..ef4df3339 100644 --- a/app/controllers/settings/two_factor_authentication/confirmations_controller.rb +++ b/app/controllers/settings/two_factor_authentication/confirmations_controller.rb @@ -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') diff --git a/app/controllers/settings/two_factor_authentication/recovery_codes_controller.rb b/app/controllers/settings/two_factor_authentication/recovery_codes_controller.rb index 09a759860..0c4f5bff7 100644 --- a/app/controllers/settings/two_factor_authentication/recovery_codes_controller.rb +++ b/app/controllers/settings/two_factor_authentication/recovery_codes_controller.rb @@ -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 diff --git a/app/controllers/settings/two_factor_authentications_controller.rb b/app/controllers/settings/two_factor_authentications_controller.rb index c93b17577..9118a7933 100644 --- a/app/controllers/settings/two_factor_authentications_controller.rb +++ b/app/controllers/settings/two_factor_authentications_controller.rb @@ -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') diff --git a/app/javascript/styles/mastodon/admin.scss b/app/javascript/styles/mastodon/admin.scss index 5d4fe4ef8..074eee2cd 100644 --- a/app/javascript/styles/mastodon/admin.scss +++ b/app/javascript/styles/mastodon/admin.scss @@ -233,32 +233,35 @@ hr.spacer { height: 1px; } -.muted-hint { - color: $darker-text-color; +body, +.admin-wrapper .content { + .muted-hint { + color: $darker-text-color; - a { - color: $highlight-text-color; + a { + color: $highlight-text-color; + } } -} -.positive-hint { - color: $valid-value-color; - font-weight: 500; -} + .positive-hint { + color: $valid-value-color; + font-weight: 500; + } -.negative-hint { - color: $error-value-color; - font-weight: 500; -} + .negative-hint { + color: $error-value-color; + font-weight: 500; + } -.neutral-hint { - color: $dark-text-color; - font-weight: 500; -} + .neutral-hint { + color: $dark-text-color; + font-weight: 500; + } -.warning-hint { - color: $gold-star; - font-weight: 500; + .warning-hint { + color: $gold-star; + font-weight: 500; + } } .filters { diff --git a/app/javascript/styles/mastodon/forms.scss b/app/javascript/styles/mastodon/forms.scss index 16352340b..80ef8797d 100644 --- a/app/javascript/styles/mastodon/forms.scss +++ b/app/javascript/styles/mastodon/forms.scss @@ -254,6 +254,10 @@ code { &-6 { max-width: 50%; } + + .actions { + margin-top: 27px; + } } .fields-group:last-child, diff --git a/app/mailers/user_mailer.rb b/app/mailers/user_mailer.rb index b41004acc..6b81f6873 100644 --- a/app/mailers/user_mailer.rb +++ b/app/mailers/user_mailer.rb @@ -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 diff --git a/app/models/form/challenge.rb b/app/models/form/challenge.rb new file mode 100644 index 000000000..40c99649c --- /dev/null +++ b/app/models/form/challenge.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +class Form::Challenge + include ActiveModel::Model + + attr_accessor :current_password, :current_username, + :return_to +end diff --git a/app/models/user.rb b/app/models/user.rb index 78b82a68f..b48455802 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -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 diff --git a/app/views/auth/challenges/new.html.haml b/app/views/auth/challenges/new.html.haml new file mode 100644 index 000000000..9aef2c35d --- /dev/null +++ b/app/views/auth/challenges/new.html.haml @@ -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' diff --git a/app/views/auth/shared/_links.html.haml b/app/views/auth/shared/_links.html.haml index e6c3f7cca..66ed5b93f 100644 --- a/app/views/auth/shared/_links.html.haml +++ b/app/views/auth/shared/_links.html.haml @@ -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' diff --git a/app/views/settings/two_factor_authentications/show.html.haml b/app/views/settings/two_factor_authentications/show.html.haml index 93509e022..f1eecd000 100644 --- a/app/views/settings/two_factor_authentications/show.html.haml +++ b/app/views/settings/two_factor_authentications/show.html.haml @@ -2,33 +2,35 @@ = t('settings.two_factor_authentication') - if current_user.otp_required_for_login - %p.positive-hint - = fa_icon 'check' - = ' ' - = t 'two_factor_authentication.enabled' + %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' diff --git a/app/views/user_mailer/two_factor_disabled.html.haml b/app/views/user_mailer/two_factor_disabled.html.haml new file mode 100644 index 000000000..651c6f940 --- /dev/null +++ b/app/views/user_mailer/two_factor_disabled.html.haml @@ -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') diff --git a/app/views/user_mailer/two_factor_disabled.text.erb b/app/views/user_mailer/two_factor_disabled.text.erb new file mode 100644 index 000000000..73be1ddc2 --- /dev/null +++ b/app/views/user_mailer/two_factor_disabled.text.erb @@ -0,0 +1,7 @@ +<%= t 'devise.mailer.two_factor_disabled.title' %> + +=== + +<%= t 'devise.mailer.two_factor_disabled.explanation' %> + +=> <%= edit_user_registration_url %> diff --git a/app/views/user_mailer/two_factor_enabled.html.haml b/app/views/user_mailer/two_factor_enabled.html.haml new file mode 100644 index 000000000..fc31bd979 --- /dev/null +++ b/app/views/user_mailer/two_factor_enabled.html.haml @@ -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') diff --git a/app/views/user_mailer/two_factor_enabled.text.erb b/app/views/user_mailer/two_factor_enabled.text.erb new file mode 100644 index 000000000..4319dddbf --- /dev/null +++ b/app/views/user_mailer/two_factor_enabled.text.erb @@ -0,0 +1,7 @@ +<%= t 'devise.mailer.two_factor_enabled.title' %> + +=== + +<%= t 'devise.mailer.two_factor_enabled.explanation' %> + +=> <%= edit_user_registration_url %> diff --git a/app/views/user_mailer/two_factor_recovery_codes_changed.html.haml b/app/views/user_mailer/two_factor_recovery_codes_changed.html.haml new file mode 100644 index 000000000..833708868 --- /dev/null +++ b/app/views/user_mailer/two_factor_recovery_codes_changed.html.haml @@ -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') diff --git a/app/views/user_mailer/two_factor_recovery_codes_changed.text.erb b/app/views/user_mailer/two_factor_recovery_codes_changed.text.erb new file mode 100644 index 000000000..6ed12fc08 --- /dev/null +++ b/app/views/user_mailer/two_factor_recovery_codes_changed.text.erb @@ -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 %> diff --git a/config/locales/devise.en.yml b/config/locales/devise.en.yml index 5defa6624..726d2426a 100644 --- a/config/locales/devise.en.yml +++ b/config/locales/devise.en.yml @@ -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: diff --git a/config/locales/en.yml b/config/locales/en.yml index f05fdd48b..da06b0e51 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -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" diff --git a/config/locales/simple_form.en.yml b/config/locales/simple_form.en.yml index c542377a9..c9ffcfc13 100644 --- a/config/locales/simple_form.en.yml +++ b/config/locales/simple_form.en.yml @@ -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: diff --git a/config/routes.rb b/config/routes.rb index a4dee2842..9ad1ea65d 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -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 diff --git a/spec/controllers/auth/challenges_controller_spec.rb b/spec/controllers/auth/challenges_controller_spec.rb new file mode 100644 index 000000000..2a6ca301e --- /dev/null +++ b/spec/controllers/auth/challenges_controller_spec.rb @@ -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 diff --git a/spec/controllers/auth/sessions_controller_spec.rb b/spec/controllers/auth/sessions_controller_spec.rb index 7ed5edde0..1950c173a 100644 --- a/spec/controllers/auth/sessions_controller_spec.rb +++ b/spec/controllers/auth/sessions_controller_spec.rb @@ -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 diff --git a/spec/controllers/concerns/challengable_concern_spec.rb b/spec/controllers/concerns/challengable_concern_spec.rb new file mode 100644 index 000000000..4db3b740d --- /dev/null +++ b/spec/controllers/concerns/challengable_concern_spec.rb @@ -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 diff --git a/spec/controllers/settings/two_factor_authentication/confirmations_controller_spec.rb b/spec/controllers/settings/two_factor_authentication/confirmations_controller_spec.rb index 2e5a9325c..336f13127 100644 --- a/spec/controllers/settings/two_factor_authentication/confirmations_controller_spec.rb +++ b/spec/controllers/settings/two_factor_authentication/confirmations_controller_spec.rb @@ -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 diff --git a/spec/controllers/settings/two_factor_authentication/recovery_codes_controller_spec.rb b/spec/controllers/settings/two_factor_authentication/recovery_codes_controller_spec.rb index c04760e53..630cec428 100644 --- a/spec/controllers/settings/two_factor_authentication/recovery_codes_controller_spec.rb +++ b/spec/controllers/settings/two_factor_authentication/recovery_codes_controller_spec.rb @@ -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' diff --git a/spec/controllers/settings/two_factor_authentications_controller_spec.rb b/spec/controllers/settings/two_factor_authentications_controller_spec.rb index 922231ded..9df9763fd 100644 --- a/spec/controllers/settings/two_factor_authentications_controller_spec.rb +++ b/spec/controllers/settings/two_factor_authentications_controller_spec.rb @@ -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) diff --git a/spec/mailers/previews/user_mailer_preview.rb b/spec/mailers/previews/user_mailer_preview.rb index ead3b3baa..464f177d0 100644 --- a/spec/mailers/previews/user_mailer_preview.rb +++ b/spec/mailers/previews/user_mailer_preview.rb @@ -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 |