INC-760: Do not return an mfa method when none exists

We were defaulting to returning an MFA method of SMS even when none existed. This meant that when a user with a partially created account was going through a password reset journey, they were hitting an error (because we think they have an MFA method of SMS but actually they do not have a verified phone number.
govuk-one-login · Apr 26, 2024 · 133549e · 133549e
1 parent 763ffd5
commit 133549e
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 11 deletions.
diff --git a/frontend-api/src/test/java/uk/gov/di/authentication/frontendapi/lambda/LoginHandlerTest.java b/frontend-api/src/test/java/uk/gov/di/authentication/frontendapi/lambda/LoginHandlerTest.java
@@ -76,6 +76,8 @@
 import static uk.gov.di.authentication.frontendapi.lambda.StartHandlerTest.CLIENT_SESSION_ID;
 import static uk.gov.di.authentication.frontendapi.lambda.StartHandlerTest.CLIENT_SESSION_ID_HEADER;
 import static uk.gov.di.authentication.shared.entity.CredentialTrustLevel.LOW_LEVEL;
+import static uk.gov.di.authentication.shared.entity.MFAMethodType.AUTH_APP;
+import static uk.gov.di.authentication.shared.entity.MFAMethodType.NONE;
 import static uk.gov.di.authentication.shared.entity.MFAMethodType.SMS;
 import static uk.gov.di.authentication.shared.services.AuditService.MetadataPair.pair;
 import static uk.gov.di.authentication.sharedtest.helper.JsonArrayHelper.jsonArrayOf;
@@ -277,7 +279,7 @@ void shouldReturn200IfLoginIsSuccessfulAndTermsAndConditionsNotAccepted(
     }
 
     @Test
-    void shouldReturn200WithCorrectMfaMethodVerifiedStatus() throws Json.JsonException {
+    void shouldReturn200WithCorrectMfaDetailsWhenAVerifiedMethodExists() throws Json.JsonException {
         var userProfile = generateUserProfile(null);
         var userCredentials =
                 new UserCredentials()
@@ -286,7 +288,7 @@ void shouldReturn200WithCorrectMfaMethodVerifiedStatus() throws Json.JsonExcepti
                         .setMfaMethod(
                                 new MFAMethod()
                                         .withMfaMethodType(MFAMethodType.AUTH_APP.getValue())
-                                        .withMethodVerified(false)
+                                        .withMethodVerified(true)
                                         .withEnabled(true));
         when(authenticationService.login(userCredentials, PASSWORD)).thenReturn(true);
         when(authenticationService.getUserProfileByEmailMaybe(EMAIL))
@@ -303,15 +305,40 @@ void shouldReturn200WithCorrectMfaMethodVerifiedStatus() throws Json.JsonExcepti
         assertThat(result, hasStatus(200));
 
         var response = objectMapper.readValue(result.getBody(), LoginResponse.class);
-        assertThat(response.getMfaMethodType(), equalTo(SMS));
+        assertThat(response.getMfaMethodType(), equalTo(AUTH_APP));
         assertThat(response.isMfaMethodVerified(), equalTo(true));
+    }
 
-        assertAuditServiceCalledWith(
-                FrontendAuditableEvent.LOG_IN_SUCCESS,
-                pair("internalSubjectId", INTERNAL_SUBJECT_ID.getValue()));
-        verifyNoInteractions(cloudwatchMetricsService);
+    @Test
+    void shouldReturn200WithCorrectMfaDetailsWhenNoVerifiedMethodExists()
+            throws Json.JsonException {
+        var userProfile = generateUserProfile(null);
+        var userCredentials =
+                new UserCredentials()
+                        .withEmail(EMAIL)
+                        .withPassword(PASSWORD)
+                        .setMfaMethod(
+                                new MFAMethod()
+                                        .withMfaMethodType(MFAMethodType.AUTH_APP.getValue())
+                                        .withMethodVerified(false)
+                                        .withEnabled(true));
+        when(authenticationService.login(userCredentials, PASSWORD)).thenReturn(true);
+        when(authenticationService.getUserProfileByEmailMaybe(EMAIL))
+                .thenReturn(Optional.of(userProfile));
+        when(authenticationService.getUserCredentialsFromEmail(EMAIL)).thenReturn(userCredentials);
+        when(clientSession.getAuthRequestParams()).thenReturn(generateAuthRequest().toParameters());
+        usingValidSession();
 
-        verifySessionIsSaved();
+        usingDefaultVectorOfTrust();
+
+        var event = eventWithHeadersAndBody(VALID_HEADERS, validBodyWithEmailAndPassword);
+        APIGatewayProxyResponseEvent result = handler.handleRequest(event, context);
+
+        assertThat(result, hasStatus(200));
+
+        var response = objectMapper.readValue(result.getBody(), LoginResponse.class);
+        assertThat(response.getMfaMethodType(), equalTo(NONE));
+        assertThat(response.isMfaMethodVerified(), equalTo(false));
     }
 
     @ParameterizedTest

diff --git a/shared/src/main/java/uk/gov/di/authentication/shared/conditions/MfaHelper.java b/shared/src/main/java/uk/gov/di/authentication/shared/conditions/MfaHelper.java
@@ -44,10 +44,9 @@ public static UserMfaDetail getUserMFADetail(
             String phoneNumber,
             boolean isPhoneNumberVerified) {
         var isMfaRequired = mfaRequired(userContext.getClientSession().getAuthRequestParams());
-        var mfaMethodVerified = isPhoneNumberVerified;
-
+        var mfaMethodVerified = false;
+        var mfaMethodType = MFAMethodType.NONE;
         var mfaMethod = getPrimaryMFAMethod(userCredentials);
-        var mfaMethodType = MFAMethodType.SMS;
         if (mfaMethod.filter(MFAMethod::isMethodVerified).isPresent()) {
             mfaMethodVerified = true;
             mfaMethodType = MFAMethodType.valueOf(mfaMethod.get().getMfaMethodType());