(2995) Fix: flaky sign in spec

doc/0_index_of_contents.md

@@ -4,6 +4,8 @@
 
 - **[Glossary of terms (glossary)](./glossary.md)**
 
+- **[Authentication](Authentication.md)**: Details about authentication and 2FA.
+
 - **[Importing new partner organisation data
   (importing-new-partner-organisation-data.md)](./importing-new-partner-organisation-data.md)**: We need to import
   legacy data for partner organisations, so they don't have to manually re-key it

doc/authentication.md

@@ -0,0 +1,30 @@
+Authentication
+
+The applications facilitates user management, authentication and
+authorisation itself.
+
+## Two factor authentication
+
+We rely on the following gems for our implementation of two factor
+authentication:
+
+- [Devise](https://github.com/heartcombo/devise)
+- [Devise-Two-Factor
+  Authentication](https://github.com/devise-two-factor/devise-two-factor)
+
+As there are a number of Devise 2FA gems, make sure you are referencing the
+correct documentation!
+
+Our one time passwords are time based. Generally speaking OTP have a concept of
+'drift' to allow for differences in the time on both the service and client.
+
+Whilst many 2FA Devise gems have separate setting for the expiry and drift,
+Devise-Two-Factor does not appear to, combining them into a single
+`[otp_allowed_drift](https://github.com/devise-two-factor/devise-two-factor#enabling-two-factor-authentication)` that we have set to 5 minutes in
+`[config/initializers/devise.rb](https://github.com/UKGovernmentBEIS/beis-report-official-development-assistance/blob/develop/config/initializers/devise.rb)`.
+
+Something appears to introduce an additional 30 seconds on top of this which is
+documented in our
+`[spec/features/users_can_sign_in_spec.rb](https://github.com/UKGovernmentBEIS/beis-report-official-development-assistance/blob/develop/spec/features/users_can_sign_in_spec.rb)`,
+at this point we can only assume this is part of the underlying
+[rotp](https://github.com/mdp/rotp) gem that is used.

spec/features/users_can_sign_in_spec.rb

@@ -45,37 +45,59 @@ def log_in_via_form(user, remember_me: false)
     end
 
     context "user has a confirmed mobile number" do
-      scenario "successful sign in via header link" do
-        # Given a user with 2FA enabled and a confirmed mobile number exists
-        user = create(:administrator, :mfa_enabled, mobile_number_confirmed_at: DateTime.current)
-
-        # When I log in with that user's email and password
-        visit root_path
-        log_in_via_form(user)
+      context "and the OTP is correct" do
+        scenario "successful sign in via header link" do
+          # Given a user with 2FA enabled and a confirmed mobile number exists
+          user = create(:administrator, :mfa_enabled, mobile_number_confirmed_at: DateTime.current)
+          allow_any_instance_of(User).to receive(:current_otp).and_return("123456")
+          allow_any_instance_of(User).to receive(:validate_and_consume_otp!).and_return(true)
+          allow(Notify::OTPMessage).to receive(:new).and_call_original
+
+          # When I log in with that user's email and password
+          travel_to Time.new(2023, 11, 29, 10, 0o0, 0o0) do
+            visit root_path
+            log_in_via_form(user)
+
+            # Then there should be no link to check my mobile number
+            expect(page).not_to have_link "Check your mobile number is correct"
+
+            # And I should receive an OTP to my mobile number
+            expect(Notify::OTPMessage).to have_received(:new).with(user.mobile_number, "123456")
+          end
+
+          # When I enter the one-time password before it expires
+          travel_to Time.new(2023, 11, 29, 10, 0o5, 29) do
+            fill_in "Please enter your six-digit verification code", with: "123456"
+            click_on "Continue"
+          end
+
+          # Then I should be logged in
+          expect(page).to have_content("Signed in successfully.")
+          expect(page).to have_link(t("header.link.sign_out"))
 
-        # Then there should be no link to check my mobile number
-        expect(page).not_to have_link "Check your mobile number is correct"
+          # And I should be at the home page
+          expect(page).to have_content("You can search by RODA, Partner Organisation, BEIS, or IATI identifier, or by the activity's title")
+        end
+      end
 
-        otp_at_time_of_login = user.current_otp
-        # And I should receive an OTP to my mobile number
-        expect(notify_client).to have_received(:send_sms).with({
-          phone_number: user.mobile_number,
-          template_id: ENV["NOTIFY_OTP_VERIFICATION_TEMPLATE"],
-          personalisation: {otp: otp_at_time_of_login}
-        })
+      context "and the OTP has expired (Configured as 5 minutes, but appears to be 5 minutes 30 seconds)" do
+        scenario "the see an error" do
+          user = create(:administrator, :mfa_enabled, mobile_number_confirmed_at: DateTime.current)
+          otp_at_time_of_login = nil
 
-        # When I enter the one-time password before it expires
-        travel 3.minutes do
-          fill_in "Please enter your six-digit verification code", with: otp_at_time_of_login
-          click_on "Continue"
-        end
+          travel_to Time.new(2023, 11, 29, 10, 0o0) do
+            visit root_path
+            log_in_via_form(user)
+            otp_at_time_of_login = user.current_otp
+          end
 
-        # Then I should be logged in
-        expect(page).to have_link(t("header.link.sign_out"))
-        expect(page).to have_content("Signed in successfully.")
+          travel_to Time.new(2023, 11, 29, 10, 0o5, 30) do
+            fill_in "Please enter your six-digit verification code", with: otp_at_time_of_login
+            click_on "Continue"
+          end
 
-        # And I should be at the home page
-        expect(page).to have_content("You can search by RODA, Partner Organisation, BEIS, or IATI identifier, or by the activity's title")
+          expect(page).to have_content("Invalid two-factor verification code.")
+        end
       end
 
       scenario "unsuccessful OTP attempt" do