test: update specs to not require OTP for enabling WebAuthn as 2FA

Co-authored-by: Santiago Rodriguez <santiago.rodriguez@cedarcode.com>

spec/controllers/auth/sessions_controller_spec.rb

@@ -349,9 +349,9 @@ RSpec.describe Auth::SessionsController do
         end
       end
 
-      context 'with WebAuthn and OTP enabled as second factor' do
+      context 'with WebAuthn 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)
+          Fabricate(:user, email: 'x@y.com', password: 'abcdefgh')
         end
 
         let!(:webauthn_credential) do

spec/controllers/settings/two_factor_authentication/webauthn_credentials_controller_spec.rb

@@ -20,31 +20,12 @@ RSpec.describe Settings::TwoFactorAuthentication::WebauthnCredentialsController
         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
@@ -53,11 +34,6 @@ RSpec.describe Settings::TwoFactorAuthentication::WebauthnCredentialsController
         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)
@@ -81,20 +57,6 @@ RSpec.describe Settings::TwoFactorAuthentication::WebauthnCredentialsController
       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
@@ -110,23 +72,30 @@ RSpec.describe Settings::TwoFactorAuthentication::WebauthnCredentialsController
         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 'includes existing credentials in list of excluded credentials', :aggregate_failures do
+        it 'returns http success' do
-            expect { get :options }.to_not change(user, :webauthn_id)
+          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 = response.parsed_body['excludeCredentials'].pluck('id')
           expect(excluded_credentials_ids).to match_array(user.webauthn_credentials.pluck(:external_id))
@@ -134,26 +103,22 @@ RSpec.describe Settings::TwoFactorAuthentication::WebauthnCredentialsController
       end
 
       context 'when user does not have webauthn enabled' do
-          it 'stores the challenge on the session and sets user webauthn_id', :aggregate_failures do
+        it 'returns http success' do
           get :options
 
           expect(response).to have_http_status(200)
-            expect(controller.session[:webauthn_challenge]).to be_present
-            expect(user.reload.webauthn_id).to be_present
-          end
-        end
         end
 
-      context 'when user has not enabled otp' do
+        it 'stores the challenge on the session' 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(controller.session[:webauthn_challenge]).to be_present
-          expect(flash[:error]).to be_present
+        end
+
+        it 'sets user webauthn_id' do
+          get :options
+
+          expect(user.reload.webauthn_id).to be_present
         end
       end
     end
@@ -183,29 +148,40 @@ RSpec.describe Settings::TwoFactorAuthentication::WebauthnCredentialsController
         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
 
-          it 'adds a new credential to user credentials and does not change webauthn_id when creation succeeds', :aggregate_failures do
+        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)
-                                                             .and not_change(user, :webauthn_id)
-
-            expect(response).to have_http_status(200)
           end
 
-          it 'fails when the nickname is already used' do
+          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' }
@@ -213,14 +189,19 @@ RSpec.describe Settings::TwoFactorAuthentication::WebauthnCredentialsController
             expect(response).to have_http_status(422)
             expect(flash[:error]).to be_present
           end
+        end
 
-          it 'fails when the credential already exists' do
+        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: Fabricate(:user).id,
+                      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 }
@@ -230,7 +211,8 @@ RSpec.describe Settings::TwoFactorAuthentication::WebauthnCredentialsController
           end
         end
 
-        context 'when user have not enabled webauthn and creation succeeds' do
+        context 'when user have not enabled webauthn' do
+          context 'when creation succeeds' do
             it 'creates a webauthn credential' do
               controller.session[:webauthn_challenge] = challenge
 
@@ -240,18 +222,6 @@ RSpec.describe Settings::TwoFactorAuthentication::WebauthnCredentialsController
             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
 
@@ -270,25 +240,26 @@ RSpec.describe Settings::TwoFactorAuthentication::WebauthnCredentialsController
         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 'redirects to 2FA methods list and shows flash success and deletes the credential when deletion succeeds', :aggregate_failures do
+        context 'when deletion succeeds' do
-            expect do
+          it 'redirects to 2FA methods list and shows flash success' do
             delete :destroy, params: { id: user.webauthn_credentials.take.id }
-            end.to change { user.webauthn_credentials.count }.by(-1)
 
             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
@@ -301,16 +272,6 @@ RSpec.describe Settings::TwoFactorAuthentication::WebauthnCredentialsController
       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' }

spec/requests/settings/two_factor_authentication_methods_spec.rb

@@ -13,23 +13,4 @@ RSpec.describe 'Settings TwoFactorAuthenticationMethods' do
       end
     end
   end
-
-  context 'when signed in' do
-    let(:user) { Fabricate(:user) }
-
-    before { sign_in user }
-
-    describe 'GET to /settings/two_factor_authentication_methods' do
-      describe 'when user has not enabled otp' do
-        before { user.update(otp_required_for_login: false) }
-
-        it 'redirects to enable otp' do
-          get settings_two_factor_authentication_methods_path
-
-          expect(response)
-            .to redirect_to(settings_otp_authentication_path)
-        end
-      end
-    end
-  end
 end

spec/system/admin/users/two_factor_authentications_spec.rb

@@ -26,15 +26,14 @@ RSpec.describe 'Admin Users TwoFactorAuthentications' do
       end
     end
 
-    context 'when user has OTP and WebAuthn enabled' do
+    context 'when user has WebAuthn enabled' do
-      before { user.update(otp_required_for_login: true, webauthn_id: WebAuthn.generate_user_id) }
+      before { user.update(webauthn_id: WebAuthn.generate_user_id) }
 
       it 'disables OTP and webauthn and redirects to admin account page' do
         visit admin_account_path(user.account.id)
 
         expect { disable_two_factor }
-          .to change { user.reload.otp_enabled? }.to(false)
+          .to change { user.reload.webauthn_enabled? }.to(false)
-          .and(change { user.reload.webauthn_enabled? }.to(false))
         expect(page)
           .to have_title(user.account.pretty_acct)
       end