From e1066cd4319a220d5be16e51ffaf5236a2f6e866 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Wed, 18 Sep 2019 16:37:27 +0200 Subject: Add password challenge to 2FA settings, e-mail notifications (#11878) Fix #3961 --- .../controllers/auth/challenges_controller_spec.rb | 46 +++++++++ spec/controllers/auth/sessions_controller_spec.rb | 2 +- .../concerns/challengable_concern_spec.rb | 114 +++++++++++++++++++++ .../confirmations_controller_spec.rb | 10 +- .../recovery_codes_controller_spec.rb | 2 +- .../two_factor_authentications_controller_spec.rb | 2 +- spec/mailers/previews/user_mailer_preview.rb | 15 +++ 7 files changed, 183 insertions(+), 8 deletions(-) create mode 100644 spec/controllers/auth/challenges_controller_spec.rb create mode 100644 spec/controllers/concerns/challengable_concern_spec.rb (limited to 'spec') 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 -- cgit