diff options
Diffstat (limited to 'spec')
11 files changed, 866 insertions, 250 deletions
diff --git a/spec/controllers/admin/two_factor_authentications_controller_spec.rb b/spec/controllers/admin/two_factor_authentications_controller_spec.rb index 4c1aa88d7..b0e82d3d6 100644 --- a/spec/controllers/admin/two_factor_authentications_controller_spec.rb +++ b/spec/controllers/admin/two_factor_authentications_controller_spec.rb @@ -1,20 +1,51 @@ require 'rails_helper' +require 'webauthn/fake_client' describe Admin::TwoFactorAuthenticationsController do render_views - let(:user) { Fabricate(:user, otp_required_for_login: true) } + let(:user) { Fabricate(:user) } before do sign_in Fabricate(:user, admin: true), scope: :user end describe 'DELETE #destroy' do - it 'redirects to admin accounts page' do - delete :destroy, params: { user_id: user.id } + context 'when user has OTP enabled' do + before do + user.update(otp_required_for_login: true) + end - user.reload - expect(user.otp_required_for_login).to eq false - expect(response).to redirect_to(admin_accounts_path) + it 'redirects to admin accounts page' do + delete :destroy, params: { user_id: user.id } + + user.reload + expect(user.otp_enabled?).to eq false + expect(response).to redirect_to(admin_accounts_path) + end + end + + context 'when user has OTP and WebAuthn enabled' do + let(:fake_client) { WebAuthn::FakeClient.new('http://test.host') } + + before do + user.update(otp_required_for_login: true, webauthn_id: WebAuthn.generate_user_id) + + public_key_credential = WebAuthn::Credential.from_create(fake_client.create) + Fabricate(:webauthn_credential, + user_id: user.id, + external_id: public_key_credential.id, + public_key: public_key_credential.public_key, + nickname: 'Security Key') + end + + it 'redirects to admin accounts page' do + delete :destroy, params: { user_id: user.id } + + user.reload + expect(user.otp_enabled?).to eq false + expect(user.webauthn_enabled?).to eq false + expect(response).to redirect_to(admin_accounts_path) + end end end end diff --git a/spec/controllers/auth/sessions_controller_spec.rb b/spec/controllers/auth/sessions_controller_spec.rb index c387842cd..8ad9e74fc 100644 --- a/spec/controllers/auth/sessions_controller_spec.rb +++ b/spec/controllers/auth/sessions_controller_spec.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require 'rails_helper' +require 'webauthn/fake_client' RSpec.describe Auth::SessionsController, type: :controller do render_views @@ -183,90 +184,170 @@ RSpec.describe Auth::SessionsController, type: :controller do end context 'using two-factor authentication' do - let!(:user) do - Fabricate(:user, email: 'x@y.com', password: 'abcdefgh', otp_required_for_login: true, otp_secret: User.generate_otp_secret(32)) - end - - let!(:recovery_codes) do - codes = user.generate_otp_backup_codes! - user.save - return codes - end - - context 'using email and password' do - before do - post :create, params: { user: { email: user.email, password: user.password } } + context 'with OTP enabled as second factor' do + let!(:user) do + Fabricate(:user, email: 'x@y.com', password: 'abcdefgh', otp_required_for_login: true, otp_secret: User.generate_otp_secret(32)) end - it 'renders two factor authentication page' do - expect(controller).to render_template("two_factor") + let!(:recovery_codes) do + codes = user.generate_otp_backup_codes! + user.save + return codes end - end - context 'using upcase email and password' do - before do - post :create, params: { user: { email: user.email.upcase, password: user.password } } - end + context 'using email and password' do + before do + 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") + 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 - end - context 'using a valid OTP' do - before do - post :create, params: { user: { otp_attempt: user.current_otp } }, session: { attempt_user_id: user.id } - end + context 'using upcase email and password' do + before do + post :create, params: { user: { email: user.email.upcase, password: user.password } } + end - it 'redirects to home' do - expect(response).to redirect_to(root_path) + 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 - it 'logs the user in' do - expect(controller.current_user).to eq user + context 'using a valid OTP' do + before do + post :create, params: { user: { otp_attempt: user.current_otp } }, 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 - 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) - post :create, params: { user: { otp_attempt: user.current_otp } }, session: { attempt_user_id: user.id } + 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: { attempt_user_id: user.id } + end + + it 'shows a login error' do + expect(flash[:alert]).to match I18n.t('users.invalid_otp_token') + end + + it "doesn't log the user in" do + expect(controller.current_user).to be_nil + end end - it 'shows a login error' do - expect(flash[:alert]).to match I18n.t('users.invalid_otp_token') + context 'using a valid recovery code' do + before do + post :create, params: { user: { otp_attempt: recovery_codes.first } }, 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 - it "doesn't log the user in" do - expect(controller.current_user).to be_nil + context 'using an invalid OTP' do + before do + post :create, params: { user: { otp_attempt: 'wrongotp' } }, session: { attempt_user_id: user.id } + end + + it 'shows a login error' do + expect(flash[:alert]).to match I18n.t('users.invalid_otp_token') + end + + it "doesn't log the user in" do + expect(controller.current_user).to be_nil + end end end - context 'using a valid recovery code' do - before do - post :create, params: { user: { otp_attempt: recovery_codes.first } }, session: { attempt_user_id: user.id } + context 'with WebAuthn and OTP enabled as second factor' do + let!(:user) do + Fabricate(:user, email: 'x@y.com', password: 'abcdefgh', otp_required_for_login: true, otp_secret: User.generate_otp_secret(32)) end - it 'redirects to home' do - expect(response).to redirect_to(root_path) + let!(:recovery_codes) do + codes = user.generate_otp_backup_codes! + user.save + return codes end - it 'logs the user in' do - expect(controller.current_user).to eq user + let!(:webauthn_credential) do + user.update(webauthn_id: WebAuthn.generate_user_id) + public_key_credential = WebAuthn::Credential.from_create(fake_client.create) + user.webauthn_credentials.create( + nickname: 'SecurityKeyNickname', + external_id: public_key_credential.id, + public_key: public_key_credential.public_key, + sign_count: '1000' + ) + user.webauthn_credentials.take end - end - context 'using an invalid OTP' do - before do - post :create, params: { user: { otp_attempt: 'wrongotp' } }, session: { attempt_user_id: user.id } + let(:domain) { "#{Rails.configuration.x.use_https ? 'https' : 'http' }://#{Rails.configuration.x.web_domain}" } + + let(:fake_client) { WebAuthn::FakeClient.new(domain) } + + let(:challenge) { WebAuthn::Credential.options_for_get.challenge } + + let(:sign_count) { 1234 } + + let(:fake_credential) { fake_client.get(challenge: challenge, sign_count: sign_count) } + + context 'using email and password' do + before do + post :create, params: { user: { email: user.email, password: user.password } } + end + + it 'renders webauthn authentication page' do + expect(controller).to render_template("two_factor") + expect(controller).to render_template(partial: "_webauthn_form") + end end - it 'shows a login error' do - expect(flash[:alert]).to match I18n.t('users.invalid_otp_token') + context 'using upcase email and password' do + before do + post :create, params: { user: { email: user.email.upcase, password: user.password } } + end + + it 'renders webauthn authentication page' do + expect(controller).to render_template("two_factor") + expect(controller).to render_template(partial: "_webauthn_form") + end end - it "doesn't log the user in" do - expect(controller.current_user).to be_nil + context 'using a valid webauthn credential' do + before do + @controller.session[:webauthn_challenge] = challenge + + post :create, params: { user: { credential: fake_credential } }, session: { attempt_user_id: user.id } + end + + it 'instructs the browser to redirect to home' do + expect(body_as_json[:redirect_path]).to eq(root_path) + end + + it 'logs the user in' do + expect(controller.current_user).to eq user + end + + it 'updates the sign count' do + expect(webauthn_credential.reload.sign_count).to eq(sign_count) + 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 336f13127..cdfeef8d6 100644 --- a/spec/controllers/settings/two_factor_authentication/confirmations_controller_spec.rb +++ b/spec/controllers/settings/two_factor_authentication/confirmations_controller_spec.rb @@ -5,8 +5,6 @@ require 'rails_helper' describe Settings::TwoFactorAuthentication::ConfirmationsController do render_views - let(:user) { Fabricate(:user, email: 'local-part@domain', otp_secret: 'thisisasecretforthespecofnewview') } - let(:user_without_otp_secret) { Fabricate(:user, email: 'local-part@domain') } shared_examples 'renders :new' do it 'renders the new view' do @@ -20,87 +18,101 @@ describe Settings::TwoFactorAuthentication::ConfirmationsController do end end - describe 'GET #new' do - context 'when signed in' do - subject do - sign_in user, scope: :user - get :new, session: { challenge_passed_at: Time.now.utc } - end + [true, false].each do |with_otp_secret| + let(:user) { Fabricate(:user, email: 'local-part@domain', otp_secret: with_otp_secret ? 'oldotpsecret' : nil) } - include_examples 'renders :new' - end + describe 'GET #new' do + context 'when signed in and a new otp secret has been setted in the session' do + subject do + sign_in user, scope: :user + get :new, session: { challenge_passed_at: Time.now.utc, new_otp_secret: 'thisisasecretforthespecofnewview' } + end - it 'redirects if not signed in' do - get :new - expect(response).to redirect_to('/auth/sign_in') - end + include_examples 'renders :new' + end - it 'redirects if user do not have otp_secret' do - sign_in user_without_otp_secret, scope: :user - get :new, session: { challenge_passed_at: Time.now.utc } - expect(response).to redirect_to('/settings/two_factor_authentication') - end - end + it 'redirects if not signed in' do + get :new + expect(response).to redirect_to('/auth/sign_in') + end - describe 'POST #create' do - context 'when signed in' do - before do + it 'redirects if a new otp_secret has not been setted in the session' do sign_in user, scope: :user + get :new, session: { challenge_passed_at: Time.now.utc } + expect(response).to redirect_to('/settings/otp_authentication') end + end - describe 'when form_two_factor_confirmation parameter is not provided' do - it 'raises ActionController::ParameterMissing' do - post :create, params: {}, session: { challenge_passed_at: Time.now.utc } - expect(response).to have_http_status(400) + describe 'POST #create' do + context 'when signed in' do + before do + sign_in user, scope: :user end - end - describe 'when creation succeeds' do - it 'renders page with success' do - otp_backup_codes = user.generate_otp_backup_codes! - expect_any_instance_of(User).to receive(:generate_otp_backup_codes!) do |value| - expect(value).to eq user - otp_backup_codes - end - expect_any_instance_of(User).to receive(:validate_and_consume_otp!) do |value, arg| - expect(value).to eq user - expect(arg).to eq '123456' - true + describe 'when form_two_factor_confirmation parameter is not provided' do + it 'raises ActionController::ParameterMissing' do + post :create, params: {}, session: { challenge_passed_at: Time.now.utc, new_otp_secret: 'thisisasecretforthespecofnewview' } + expect(response).to have_http_status(400) end + end - 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' - expect(response).to have_http_status(200) - expect(response).to render_template('settings/two_factor_authentication/recovery_codes/index') + describe 'when creation succeeds' do + it 'renders page with success' do + otp_backup_codes = user.generate_otp_backup_codes! + expect_any_instance_of(User).to receive(:generate_otp_backup_codes!) do |value| + expect(value).to eq user + otp_backup_codes + end + expect_any_instance_of(User).to receive(:validate_and_consume_otp!) do |value, code, options| + expect(value).to eq user + expect(code).to eq '123456' + expect(options).to eq({ otp_secret: 'thisisasecretforthespecofnewview' }) + true + end + + expect do + post :create, + params: { form_two_factor_confirmation: { otp_attempt: '123456' } }, + session: { challenge_passed_at: Time.now.utc, new_otp_secret: 'thisisasecretforthespecofnewview' } + end.to change { user.reload.otp_secret }.to 'thisisasecretforthespecofnewview' + + expect(assigns(:recovery_codes)).to eq otp_backup_codes + expect(flash[:notice]).to eq 'Two-factor authentication successfully enabled' + expect(response).to have_http_status(200) + expect(response).to render_template('settings/two_factor_authentication/recovery_codes/index') + end end - end - describe 'when creation fails' do - subject do - expect_any_instance_of(User).to receive(:validate_and_consume_otp!) do |value, arg| - expect(value).to eq user - expect(arg).to eq '123456' - false + describe 'when creation fails' do + subject do + expect_any_instance_of(User).to receive(:validate_and_consume_otp!) do |value, code, options| + expect(value).to eq user + expect(code).to eq '123456' + expect(options).to eq({ otp_secret: 'thisisasecretforthespecofnewview' }) + false + end + + expect do + post :create, + params: { form_two_factor_confirmation: { otp_attempt: '123456' } }, + session: { challenge_passed_at: Time.now.utc, new_otp_secret: 'thisisasecretforthespecofnewview' } + end.to not_change { user.reload.otp_secret } end - post :create, params: { form_two_factor_confirmation: { otp_attempt: '123456' } }, session: { challenge_passed_at: Time.now.utc } - end + it 'renders the new view' do + subject + expect(response.body).to include 'The entered code was invalid! Are server time and device time correct?' + end - it 'renders the new view' do - subject - expect(response.body).to include 'The entered code was invalid! Are server time and device time correct?' + include_examples 'renders :new' end - - include_examples 'renders :new' end - end - context 'when not signed in' do - it 'redirects if not signed in' do - post :create, params: { form_two_factor_confirmation: { otp_attempt: '123456' } } - expect(response).to redirect_to('/auth/sign_in') + context 'when not signed in' do + it 'redirects if not signed in' do + post :create, params: { form_two_factor_confirmation: { otp_attempt: '123456' } } + expect(response).to redirect_to('/auth/sign_in') + end end end end diff --git a/spec/controllers/settings/two_factor_authentication/otp_authentication_controller_spec.rb b/spec/controllers/settings/two_factor_authentication/otp_authentication_controller_spec.rb new file mode 100644 index 000000000..17e8fa9b8 --- /dev/null +++ b/spec/controllers/settings/two_factor_authentication/otp_authentication_controller_spec.rb @@ -0,0 +1,99 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe Settings::TwoFactorAuthentication::OtpAuthenticationController do + render_views + + let(:user) { Fabricate(:user) } + + describe 'GET #show' do + context 'when signed in' do + before do + sign_in user, scope: :user + end + + describe 'when user has OTP enabled' do + before do + user.update(otp_required_for_login: true) + end + + it 'redirects to two factor authentciation methods list page' do + get :show + + expect(response).to redirect_to settings_two_factor_authentication_methods_path + end + end + + describe 'when user does not have OTP enabled' do + before do + user.update(otp_required_for_login: false) + end + + it 'returns http success' do + get :show + + expect(response).to have_http_status(200) + end + end + end + + context 'when not signed in' do + it 'redirects' do + get :show + + expect(response).to redirect_to new_user_session_path + end + end + end + + describe 'POST #create' do + context 'when signed in' do + before do + sign_in user, scope: :user + end + + describe 'when user has OTP enabled' do + before do + user.update(otp_required_for_login: true) + end + + describe 'when creation succeeds' do + it 'redirects to code confirmation page without updating user secret and setting otp secret in the session' do + expect do + post :create, session: { challenge_passed_at: Time.now.utc } + end.to not_change { user.reload.otp_secret } + .and change { session[:new_otp_secret] } + + expect(response).to redirect_to(new_settings_two_factor_authentication_confirmation_path) + end + end + end + + describe 'when user does not have OTP enabled' do + before do + user.update(otp_required_for_login: false) + end + + describe 'when creation succeeds' do + it 'redirects to code confirmation page without updating user secret and setting otp secret in the session' do + expect do + post :create, session: { challenge_passed_at: Time.now.utc } + end.to not_change { user.reload.otp_secret } + .and change { session[:new_otp_secret] } + + expect(response).to redirect_to(new_settings_two_factor_authentication_confirmation_path) + end + end + end + end + + context 'when not signed in' do + it 'redirects to login' do + get :show + + expect(response).to redirect_to new_user_session_path + end + end + end +end diff --git a/spec/controllers/settings/two_factor_authentication/webauthn_credentials_controller_spec.rb b/spec/controllers/settings/two_factor_authentication/webauthn_credentials_controller_spec.rb new file mode 100644 index 000000000..fe53b4dfc --- /dev/null +++ b/spec/controllers/settings/two_factor_authentication/webauthn_credentials_controller_spec.rb @@ -0,0 +1,374 @@ +# frozen_string_literal: true + +require 'rails_helper' +require 'webauthn/fake_client' + +describe Settings::TwoFactorAuthentication::WebauthnCredentialsController do + render_views + + let(:user) { Fabricate(:user) } + let(:domain) { "#{Rails.configuration.x.use_https ? 'https' : 'http' }://#{Rails.configuration.x.web_domain}" } + let(:fake_client) { WebAuthn::FakeClient.new(domain) } + + def add_webauthn_credential(user) + Fabricate(:webauthn_credential, user_id: user.id, nickname: 'USB Key') + end + + describe 'GET #new' do + context 'when signed in' do + before do + sign_in user, scope: :user + end + + context 'when user has otp enabled' do + before do + user.update(otp_required_for_login: true) + end + + it 'returns http success' do + get :new + + expect(response).to have_http_status(200) + end + end + + context 'when user does not have otp enabled' do + before do + user.update(otp_required_for_login: false) + end + + it 'requires otp enabled first' do + get :new + + expect(response).to redirect_to settings_two_factor_authentication_methods_path + expect(flash[:error]).to be_present + end + end + end + end + + describe 'GET #index' do + context 'when signed in' do + before do + sign_in user, scope: :user + end + + context 'when user has otp enabled' do + before do + user.update(otp_required_for_login: true) + end + + context 'when user has webauthn enabled' do + before do + user.update(webauthn_id: WebAuthn.generate_user_id) + add_webauthn_credential(user) + end + + it 'returns http success' do + get :index + + expect(response).to have_http_status(200) + end + end + + context 'when user does not has webauthn enabled' do + it 'redirects to 2FA methods list page' do + get :index + + expect(response).to redirect_to settings_two_factor_authentication_methods_path + expect(flash[:error]).to be_present + end + end + end + + context 'when user does not have otp enabled' do + before do + user.update(otp_required_for_login: false) + end + + it 'requires otp enabled first' do + get :index + + expect(response).to redirect_to settings_two_factor_authentication_methods_path + expect(flash[:error]).to be_present + end + end + end + + context 'when not signed in' do + it 'redirects to login' do + delete :index + + expect(response).to redirect_to new_user_session_path + end + end + end + + describe 'GET /options #options' do + context 'when signed in' do + before do + sign_in user, scope: :user + end + + context 'when user has otp enabled' do + before do + user.update(otp_required_for_login: true) + end + + context 'when user has webauthn enabled' do + before do + user.update(webauthn_id: WebAuthn.generate_user_id) + add_webauthn_credential(user) + end + + it 'returns http success' do + get :options + + expect(response).to have_http_status(200) + end + + it 'stores the challenge on the session' do + get :options + + expect(@controller.session[:webauthn_challenge]).to be_present + end + + it 'does not change webauthn_id' do + expect { get :options }.to_not change { user.webauthn_id } + end + + it "includes existing credentials in list of excluded credentials" do + get :options + + excluded_credentials_ids = JSON.parse(response.body)['excludeCredentials'].map { |credential| credential['id'] } + expect(excluded_credentials_ids).to match_array(user.webauthn_credentials.pluck(:external_id)) + end + end + + context 'when user does not have webauthn enabled' do + it 'returns http success' do + get :options + + expect(response).to have_http_status(200) + end + + it 'stores the challenge on the session' do + get :options + + expect(@controller.session[:webauthn_challenge]).to be_present + end + + it 'sets user webauthn_id' do + get :options + + expect(user.reload.webauthn_id).to be_present + end + end + end + + context 'when user has not enabled otp' do + before do + user.update(otp_required_for_login: false) + end + + it 'requires otp enabled first' do + get :options + + expect(response).to redirect_to settings_two_factor_authentication_methods_path + expect(flash[:error]).to be_present + end + end + end + + context 'when not signed in' do + it 'redirects to login' do + get :options + + expect(response).to redirect_to new_user_session_path + end + end + end + + describe 'POST #create' do + let(:nickname) { 'SecurityKeyNickname' } + + let(:challenge) do + WebAuthn::Credential.options_for_create( + user: { id: user.id, name: user.account.username, display_name: user.account.display_name } + ).challenge + end + + let(:new_webauthn_credential) { fake_client.create(challenge: challenge) } + + context 'when signed in' do + before do + sign_in user, scope: :user + end + + context 'when user has enabled otp' do + before do + user.update(otp_required_for_login: true) + end + + context 'when user has enabled webauthn' do + before do + user.update(webauthn_id: WebAuthn.generate_user_id) + add_webauthn_credential(user) + end + + context 'when creation succeeds' do + it 'returns http success' do + @controller.session[:webauthn_challenge] = challenge + + post :create, params: { credential: new_webauthn_credential, nickname: nickname } + + expect(response).to have_http_status(200) + end + + it 'adds a new credential to user credentials' do + @controller.session[:webauthn_challenge] = challenge + + expect do + post :create, params: { credential: new_webauthn_credential, nickname: nickname } + end.to change { user.webauthn_credentials.count }.by(1) + end + + it 'does not change webauthn_id' do + @controller.session[:webauthn_challenge] = challenge + + expect do + post :create, params: { credential: new_webauthn_credential, nickname: nickname } + end.to_not change { user.webauthn_id } + end + end + + context 'when the nickname is already used' do + it 'fails' do + @controller.session[:webauthn_challenge] = challenge + + post :create, params: { credential: new_webauthn_credential, nickname: 'USB Key' } + + expect(response).to have_http_status(500) + expect(flash[:error]).to be_present + end + end + + context 'when the credential already exists' do + before do + user2 = Fabricate(:user) + public_key_credential = WebAuthn::Credential.from_create(new_webauthn_credential) + Fabricate(:webauthn_credential, + user_id: user2.id, + external_id: public_key_credential.id, + public_key: public_key_credential.public_key) + end + + it 'fails' do + @controller.session[:webauthn_challenge] = challenge + + post :create, params: { credential: new_webauthn_credential, nickname: nickname } + + expect(response).to have_http_status(500) + expect(flash[:error]).to be_present + end + end + end + + context 'when user have not enabled webauthn' do + context 'creation succeeds' do + it 'creates a webauthn credential' do + @controller.session[:webauthn_challenge] = challenge + + expect do + post :create, params: { credential: new_webauthn_credential, nickname: nickname } + end.to change { user.webauthn_credentials.count }.by(1) + end + end + end + end + + context 'when user has not enabled otp' do + before do + user.update(otp_required_for_login: false) + end + + it 'requires otp enabled first' do + post :create, params: { credential: new_webauthn_credential, nickname: nickname } + + expect(response).to redirect_to settings_two_factor_authentication_methods_path + expect(flash[:error]).to be_present + end + end + end + + context 'when not signed in' do + it 'redirects to login' do + post :create, params: { credential: new_webauthn_credential, nickname: nickname } + + expect(response).to redirect_to new_user_session_path + end + end + end + + describe 'DELETE #destroy' do + context 'when signed in' do + before do + sign_in user, scope: :user + end + + context 'when user has otp enabled' do + before do + user.update(otp_required_for_login: true) + end + + context 'when user has webauthn enabled' do + before do + user.update(webauthn_id: WebAuthn.generate_user_id) + add_webauthn_credential(user) + end + + context 'when deletion succeeds' do + it 'redirects to 2FA methods list and shows flash success' do + delete :destroy, params: { id: user.webauthn_credentials.take.id } + + expect(response).to redirect_to settings_two_factor_authentication_methods_path + expect(flash[:success]).to be_present + end + + it 'deletes the credential' do + expect do + delete :destroy, params: { id: user.webauthn_credentials.take.id } + end.to change { user.webauthn_credentials.count }.by(-1) + end + end + end + + context 'when user does not have webauthn enabled' do + it 'redirects to 2FA methods list and shows flash error' do + delete :destroy, params: { id: '1' } + + expect(response).to redirect_to settings_two_factor_authentication_methods_path + expect(flash[:error]).to be_present + end + end + end + + context 'when user does not have otp enabled' do + it 'requires otp enabled first' do + delete :destroy, params: { id: '1' } + + expect(response).to redirect_to settings_two_factor_authentication_methods_path + expect(flash[:error]).to be_present + end + end + end + + context 'when not signed in' do + it 'redirects to login' do + delete :destroy, params: { id: '1' } + + expect(response).to redirect_to new_user_session_path + end + end + end +end diff --git a/spec/controllers/settings/two_factor_authentication_methods_controller_spec.rb b/spec/controllers/settings/two_factor_authentication_methods_controller_spec.rb new file mode 100644 index 000000000..66ffe89f3 --- /dev/null +++ b/spec/controllers/settings/two_factor_authentication_methods_controller_spec.rb @@ -0,0 +1,49 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe Settings::TwoFactorAuthenticationMethodsController do + render_views + + let(:user) { Fabricate(:user) } + + describe 'GET #index' do + context 'when signed in' do + before do + sign_in user, scope: :user + end + + describe 'when user has enabled otp' do + before do + user.update(otp_required_for_login: true) + end + + it 'returns http success' do + get :index + + expect(response).to have_http_status(200) + end + end + + describe 'when user has not enabled otp' do + before do + user.update(otp_required_for_login: false) + end + + it 'redirects to enable otp' do + get :index + + expect(response).to redirect_to(settings_otp_authentication_path) + end + end + end + + context 'when not signed in' do + it 'redirects' do + get :index + + expect(response).to redirect_to '/auth/sign_in' + end + end + end +end diff --git a/spec/controllers/settings/two_factor_authentications_controller_spec.rb b/spec/controllers/settings/two_factor_authentications_controller_spec.rb deleted file mode 100644 index 9df9763fd..000000000 --- a/spec/controllers/settings/two_factor_authentications_controller_spec.rb +++ /dev/null @@ -1,125 +0,0 @@ -# frozen_string_literal: true - -require 'rails_helper' - -describe Settings::TwoFactorAuthenticationsController do - render_views - - let(:user) { Fabricate(:user) } - - describe 'GET #show' do - context 'when signed in' do - before do - sign_in user, scope: :user - end - - describe 'when user requires otp for login already' do - it 'returns http success' do - user.update(otp_required_for_login: true) - get :show - - expect(response).to have_http_status(200) - end - end - - describe 'when user does not require otp for login' do - it 'returns http success' do - user.update(otp_required_for_login: false) - get :show - - expect(response).to have_http_status(200) - end - end - end - - context 'when not signed in' do - it 'redirects' do - get :show - expect(response).to redirect_to '/auth/sign_in' - end - end - end - - describe 'POST #create' do - context 'when signed in' do - before do - sign_in user, scope: :user - end - - describe 'when user requires otp for login already' do - it 'redirects to show page' do - user.update(otp_required_for_login: true) - post :create - - expect(response).to redirect_to(settings_two_factor_authentication_path) - end - end - - describe 'when creation succeeds' do - it 'updates user secret' do - before = user.otp_secret - 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) - end - end - end - - context 'when not signed in' do - it 'redirects' do - get :show - expect(response).to redirect_to '/auth/sign_in' - end - end - end - - describe 'POST #destroy' do - before do - user.update(otp_required_for_login: true) - end - - context 'when signed in' do - before do - sign_in user, scope: :user - end - - it 'turns off otp requirement with correct code' do - expect_any_instance_of(User).to receive(:validate_and_consume_otp!) do |value, arg| - expect(value).to eq user - expect(arg).to eq '123456' - true - end - - post :destroy, params: { form_two_factor_confirmation: { otp_attempt: '123456' } } - - expect(response).to redirect_to(settings_two_factor_authentication_path) - user.reload - expect(user.otp_required_for_login).to eq(false) - end - - it 'does not turn off otp if code is incorrect' do - expect_any_instance_of(User).to receive(:validate_and_consume_otp!) do |value, arg| - expect(value).to eq user - expect(arg).to eq '057772' - false - end - - post :destroy, params: { form_two_factor_confirmation: { otp_attempt: '057772' } } - - user.reload - expect(user.otp_required_for_login).to eq(true) - end - - it 'raises ActionController::ParameterMissing if code is missing' do - post :destroy - expect(response).to have_http_status(400) - end - end - - it 'redirects if not signed in' do - get :show - expect(response).to redirect_to '/auth/sign_in' - end - end -end diff --git a/spec/fabricators/webauthn_credential_fabricator.rb b/spec/fabricators/webauthn_credential_fabricator.rb new file mode 100644 index 000000000..496a7a735 --- /dev/null +++ b/spec/fabricators/webauthn_credential_fabricator.rb @@ -0,0 +1,7 @@ +Fabricator(:webauthn_credential) do + user_id { Fabricate(:user).id } + external_id { Base64.urlsafe_encode64(SecureRandom.random_bytes(16)) } + public_key { OpenSSL::PKey::EC.new("prime256v1").generate_key.public_key } + nickname 'USB key' + sign_count 0 +end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 5686ec909..cded4c99b 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -151,6 +151,12 @@ RSpec.describe User, type: :model do expect(user.reload.otp_required_for_login).to be false end + it 'saves nil for otp_secret' do + user = Fabricate.build(:user, otp_secret: 'oldotpcode') + user.disable_two_factor! + expect(user.reload.otp_secret).to be nil + end + it 'saves cleared otp_backup_codes' do user = Fabricate.build(:user, otp_backup_codes: %w(dummy dummy)) user.disable_two_factor! diff --git a/spec/models/webauthn_credentials_spec.rb b/spec/models/webauthn_credentials_spec.rb new file mode 100644 index 000000000..9289c371e --- /dev/null +++ b/spec/models/webauthn_credentials_spec.rb @@ -0,0 +1,80 @@ +require 'rails_helper' + +RSpec.describe WebauthnCredential, type: :model do + describe 'validations' do + it 'is invalid without an external id' do + webauthn_credential = Fabricate.build(:webauthn_credential, external_id: nil) + + webauthn_credential.valid? + + expect(webauthn_credential).to model_have_error_on_field(:external_id) + end + + it 'is invalid without a public key' do + webauthn_credential = Fabricate.build(:webauthn_credential, public_key: nil) + + webauthn_credential.valid? + + expect(webauthn_credential).to model_have_error_on_field(:public_key) + end + + it 'is invalid without a nickname' do + webauthn_credential = Fabricate.build(:webauthn_credential, nickname: nil) + + webauthn_credential.valid? + + expect(webauthn_credential).to model_have_error_on_field(:nickname) + end + + it 'is invalid without a sign_count' do + webauthn_credential = Fabricate.build(:webauthn_credential, sign_count: nil) + + webauthn_credential.valid? + + expect(webauthn_credential).to model_have_error_on_field(:sign_count) + end + + it 'is invalid if already exist a webauthn credential with the same external id' do + existing_webauthn_credential = Fabricate(:webauthn_credential, external_id: "_Typ0ygudDnk9YUVWLQayw") + new_webauthn_credential = Fabricate.build(:webauthn_credential, external_id: "_Typ0ygudDnk9YUVWLQayw") + + new_webauthn_credential.valid? + + expect(new_webauthn_credential).to model_have_error_on_field(:external_id) + end + + it 'is invalid if user already registered a webauthn credential with the same nickname' do + user = Fabricate(:user) + existing_webauthn_credential = Fabricate(:webauthn_credential, user_id: user.id, nickname: 'USB Key') + new_webauthn_credential = Fabricate.build(:webauthn_credential, user_id: user.id, nickname: 'USB Key') + + new_webauthn_credential.valid? + + expect(new_webauthn_credential).to model_have_error_on_field(:nickname) + end + + it 'is invalid if sign_count is not a number' do + webauthn_credential = Fabricate.build(:webauthn_credential, sign_count: 'invalid sign_count') + + webauthn_credential.valid? + + expect(webauthn_credential).to model_have_error_on_field(:sign_count) + end + + it 'is invalid if sign_count is negative number' do + webauthn_credential = Fabricate.build(:webauthn_credential, sign_count: -1) + + webauthn_credential.valid? + + expect(webauthn_credential).to model_have_error_on_field(:sign_count) + end + + it 'is invalid if sign_count is greater 2**32 - 1' do + webauthn_credential = Fabricate.build(:webauthn_credential, sign_count: 2**32) + + webauthn_credential.valid? + + expect(webauthn_credential).to model_have_error_on_field(:sign_count) + end + end +end diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index 40ddf1f95..86c2a9c52 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -70,6 +70,8 @@ RSpec::Sidekiq.configure do |config| config.warn_when_jobs_not_processed_by_sidekiq = false end +RSpec::Matchers.define_negated_matcher :not_change, :change + def request_fixture(name) File.read(Rails.root.join('spec', 'fixtures', 'requests', name)) end |