ATO-983: Set InternalCommonSubjectIdentifier in AuthSession

govuk-one-login · Oct 24, 2024 · c7eaea4 · c7eaea4
1 parent 25447bb
commit c7eaea4
Show file tree

Hide file tree

Showing 7 changed files with 34 additions and 21 deletions.
diff --git a/frontend-api/src/main/java/uk/gov/di/authentication/frontendapi/helpers/SessionHelper.java b/frontend-api/src/main/java/uk/gov/di/authentication/frontendapi/helpers/SessionHelper.java
@@ -3,12 +3,11 @@
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 import uk.gov.di.authentication.frontendapi.lambda.VerifyMfaCodeHandler;
-import uk.gov.di.authentication.shared.entity.Session;
+import uk.gov.di.authentication.shared.entity.AuthSessionItem;
 import uk.gov.di.authentication.shared.entity.UserProfile;
 import uk.gov.di.authentication.shared.helpers.ClientSubjectHelper;
 import uk.gov.di.authentication.shared.services.AuthenticationService;
 import uk.gov.di.authentication.shared.services.ConfigurationService;
-import uk.gov.di.authentication.shared.services.SessionService;
 import uk.gov.di.authentication.shared.state.UserContext;
 
 public class SessionHelper {
@@ -18,9 +17,9 @@ public static void updateSessionWithSubject(
             UserContext userContext,
             AuthenticationService authenticationService,
             ConfigurationService configurationService,
-            SessionService sessionService,
-            Session session) {
+            AuthSessionItem authSession) {
         LOG.info("Calculating internal common subject identifier");
+        var session = userContext.getSession();
         UserProfile userProfile =
                 userContext.getUserProfile().isPresent()
                         ? userContext.getUserProfile().get()
@@ -34,9 +33,7 @@ public static void updateSessionWithSubject(
                                         authenticationService)
                                 .getValue();
         LOG.info("Setting internal common subject identifier in user session");
-        sessionService.storeOrUpdateSession(
-                userContext
-                        .getSession()
-                        .setInternalCommonSubjectIdentifier(internalCommonSubjectIdentifier));
+        session.setInternalCommonSubjectIdentifier(internalCommonSubjectIdentifier);
+        authSession.setInternalCommonSubjectIdentifier(internalCommonSubjectIdentifier);
     }
 }
diff --git a/frontend-api/src/main/java/uk/gov/di/authentication/frontendapi/lambda/LoginHandler.java b/frontend-api/src/main/java/uk/gov/di/authentication/frontendapi/lambda/LoginHandler.java
@@ -308,7 +308,9 @@ private APIGatewayProxyResponseEvent handleValidCredentials(
                         .setInternalCommonSubjectIdentifier(internalCommonSubjectIdentifier));
 
         authSessionService.updateSession(
-                authSessionItem.withAccountState(AuthSessionItem.AccountState.EXISTING));
+                authSessionItem
+                        .withAccountState(AuthSessionItem.AccountState.EXISTING)
+                        .withInternalCommonSubjectIdentifier(internalCommonSubjectIdentifier));
 
         var userMfaDetail =
                 getUserMFADetail(

diff --git a/frontend-api/src/main/java/uk/gov/di/authentication/frontendapi/lambda/SignUpHandler.java b/frontend-api/src/main/java/uk/gov/di/authentication/frontendapi/lambda/SignUpHandler.java
@@ -184,7 +184,10 @@ public APIGatewayProxyResponseEvent handleRequestWithUserContext(
                                     internalCommonSubjectIdentifier.getValue()));
 
             authSessionService.updateSession(
-                    authSessionItem.withAccountState(AuthSessionItem.AccountState.NEW));
+                    authSessionItem
+                            .withAccountState(AuthSessionItem.AccountState.NEW)
+                            .withInternalCommonSubjectIdentifier(
+                                    internalCommonSubjectIdentifier.getValue()));
             LOG.info("Successfully processed request");
             return generateApiGatewayProxyResponse(200, "");
         } else {

diff --git a/...tend-api/src/main/java/uk/gov/di/authentication/frontendapi/lambda/VerifyCodeHandler.java b/...tend-api/src/main/java/uk/gov/di/authentication/frontendapi/lambda/VerifyCodeHandler.java
@@ -237,8 +237,7 @@ public APIGatewayProxyResponseEvent handleRequestWithUserContext(
                         userContext,
                         authenticationService,
                         configurationService,
-                        sessionService,
-                        session);
+                        authSession.get());
             }
 
             processSuccessfulCodeRequest(

diff --git a/...d-api/src/main/java/uk/gov/di/authentication/frontendapi/lambda/VerifyMfaCodeHandler.java b/...d-api/src/main/java/uk/gov/di/authentication/frontendapi/lambda/VerifyMfaCodeHandler.java
@@ -308,11 +308,7 @@ private APIGatewayProxyResponseEvent verifyCode(
 
         if (JourneyType.PASSWORD_RESET_MFA.equals(codeRequest.getJourneyType())) {
             SessionHelper.updateSessionWithSubject(
-                    userContext,
-                    authenticationService,
-                    configurationService,
-                    sessionService,
-                    session);
+                    userContext, authenticationService, configurationService, authSession);
         }
 
         processCodeSession(

diff --git a/...-api/src/test/java/uk/gov/di/authentication/frontendapi/lambda/VerifyCodeHandlerTest.java b/...-api/src/test/java/uk/gov/di/authentication/frontendapi/lambda/VerifyCodeHandlerTest.java
@@ -558,8 +558,7 @@ void shouldReturn204ForValidMfaSmsRequestAndRemoveAccountRecoveryBlockWhenPresen
         assertThat(session.getVerifiedMfaMethodType(), equalTo(MFAMethodType.SMS));
         verify(codeStorageService).deleteOtpCode(EMAIL, MFA_SMS);
         verify(accountModifiersService).removeAccountRecoveryBlockIfPresent(expectedCommonSubject);
-        var saveSessionCount = journeyType == JourneyType.PASSWORD_RESET_MFA ? 3 : 2;
-        verify(sessionService, times(saveSessionCount)).storeOrUpdateSession(session);
+        verify(sessionService, times(2)).storeOrUpdateSession(session);
         verify(auditService)
                 .submitAuditEvent(
                         FrontendAuditableEvent.AUTH_CODE_VERIFIED,

diff --git a/shared/src/main/java/uk/gov/di/authentication/shared/entity/AuthSessionItem.java b/shared/src/main/java/uk/gov/di/authentication/shared/entity/AuthSessionItem.java
@@ -9,6 +9,9 @@ public class AuthSessionItem {
 
     public static final String ATTRIBUTE_SESSION_ID = "SessionId";
     public static final String ATTRIBUTE_IS_NEW_ACCOUNT = "isNewAccount";
+    public static final String ATTRIBUTE_VERIFIED_MFA_METHOD_TYPE = "VerifiedMfaMethodType";
+    public static final String ATTRIBUTE_INTERNAL_COMMON_SUBJECT_IDENTIFIER =
+            "InternalCommonSubjectIdentifier";
 
     public enum AccountState {
         NEW,
@@ -17,12 +20,11 @@ public enum AccountState {
         UNKNOWN
     }
 
-    public static final String ATTRIBUTE_VERIFIED_MFA_METHOD_TYPE = "VerifiedMfaMethodType";
-
     private String sessionId;
     private String verifiedMfaMethodType;
     private long timeToLive;
     private AccountState isNewAccount;
+    private String internalCommonSubjectIdentifier;
 
     public AuthSessionItem() {}
 
@@ -55,6 +57,21 @@ public AuthSessionItem withVerifiedMfaMethodType(String verifiedMfaMethodType) {
         return this;
     }
 
+    @DynamoDbAttribute(ATTRIBUTE_INTERNAL_COMMON_SUBJECT_IDENTIFIER)
+    public String getInternalCommonSubjectIdentifier() {
+        return internalCommonSubjectIdentifier;
+    }
+
+    public void setInternalCommonSubjectIdentifier(String internalCommonSubjectIdentifier) {
+        this.internalCommonSubjectIdentifier = internalCommonSubjectIdentifier;
+    }
+
+    public AuthSessionItem withInternalCommonSubjectIdentifier(
+            String internalCommonSubjectIdentifier) {
+        this.internalCommonSubjectIdentifier = internalCommonSubjectIdentifier;
+        return this;
+    }
+
     @DynamoDbAttribute("ttl")
     public long getTimeToLive() {
         return timeToLive;