Skip to content

ATO-2024 Multi-session recovery #7264

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

ATO-2024 Multi-session recovery #7264

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 37 additions & 19 deletions ipv-api/src/main/java/uk/gov/di/authentication/ipv/lambda/IPVCallbackHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import uk.gov.di.orchestration.shared.entity.AccountIntervention;
import uk.gov.di.orchestration.shared.entity.AuthUserInfoClaims;
import uk.gov.di.orchestration.shared.entity.ClientRegistry;
import uk.gov.di.orchestration.shared.entity.CrossBrowserEntity;
import uk.gov.di.orchestration.shared.entity.DestroySessionsRequest;
import uk.gov.di.orchestration.shared.entity.OrchSessionItem;
import uk.gov.di.orchestration.shared.entity.ResponseHeaders;
Expand Down Expand Up @@ -198,37 +199,54 @@ public APIGatewayProxyResponseEvent handleRequest(
var persistentId =
PersistentIdHelper.extractPersistentIdFromCookieHeader(input.getHeaders());
attachLogFieldToLogs(PERSISTENT_SESSION_ID, persistentId);
var clientSessionId = sessionCookiesIds.getClientSessionId();
attachLogFieldToLogs(CLIENT_SESSION_ID, clientSessionId);
attachLogFieldToLogs(GOVUK_SIGNIN_JOURNEY_ID, clientSessionId);
var orchClientSession =
var clientSessionIdFromCookie = sessionCookiesIds.getClientSessionId();
attachLogFieldToLogs(CLIENT_SESSION_ID, clientSessionIdFromCookie);
attachLogFieldToLogs(GOVUK_SIGNIN_JOURNEY_ID, clientSessionIdFromCookie);
var orchClientSessionFromCookie =
orchClientSessionService
.getClientSession(clientSessionId)
.getClientSession(clientSessionIdFromCookie)
.orElseThrow(
() ->
new IPVCallbackNoSessionException(
"ClientSession not found"));

var mismatchedEntity =
crossBrowserOrchestrationService.generateEntityForMismatchInClientSessionId(
input.getQueryStringParameters(), clientSessionId, orchSession);
input.getQueryStringParameters(), clientSessionIdFromCookie);

if (mismatchedEntity.isPresent()) {

var authRequestFromStateDerivedRP =
AuthenticationRequest.parse(
mismatchedEntity.get().getClientSession().getAuthRequestParams());
attachLogFieldToLogs(
CLIENT_ID, authRequestFromStateDerivedRP.getClientID().getValue());

return ipvCallbackHelper.generateAuthenticationErrorResponse(
authRequestFromStateDerivedRP,
mismatchedEntity.get().getErrorObject(),
false,
mismatchedEntity.get().getClientSessionId(),
AuditService.UNKNOWN);
if (orchSession
.getClientSessions()
.contains(mismatchedEntity.get().getClientSessionId())) {
LOG.info("Recovering client session from state");
Copy link
Contributor Author

@Joe-Edwards-GDS Joe-Edwards-GDS Oct 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might be where we need to update the gs cookie and possibly other state to make the SPOT spinner work.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, we'll need to work out how the auth frontend deals with this as well on the spinner page

} else {
var authRequestFromStateDerivedRP =
AuthenticationRequest.parse(
mismatchedEntity
.get()
.getClientSession()
.getAuthRequestParams());
attachLogFieldToLogs(
CLIENT_ID, authRequestFromStateDerivedRP.getClientID().getValue());

return ipvCallbackHelper.generateAuthenticationErrorResponse(
authRequestFromStateDerivedRP,
mismatchedEntity.get().getErrorObject(),
false,
mismatchedEntity.get().getClientSessionId(),
AuditService.UNKNOWN);
}
}

var clientSessionId =
mismatchedEntity
.map(CrossBrowserEntity::getClientSessionId)
.orElse(clientSessionIdFromCookie);
var orchClientSession =
mismatchedEntity
.map(CrossBrowserEntity::getClientSession)
.orElse(orchClientSessionFromCookie);

var authRequest = AuthenticationRequest.parse(orchClientSession.getAuthRequestParams());
var clientId = authRequest.getClientID().getValue();
attachLogFieldToLogs(CLIENT_ID, clientId);
Expand Down
93 changes: 86 additions & 7 deletions ipv-api/src/test/java/uk/gov/di/authentication/ipv/lambda/IPVCallbackHandlerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -980,7 +980,7 @@ void shouldRedirectToFrontendErrorPageWhenTokenResponseIsNotSuccessful() throws

@Test
void
shouldRedirectToRPWhenNoSessionCookieAndCallToNoSessionOrchestrationServiceReturnsNoSessionEntity()
shouldRedirectToRPWhenNoSessionCookieAndCallToCrossBrowserOrchestrationServiceReturnsNoSessionEntity()
throws NoSessionException, ParseException {
usingValidSession();
usingValidClientSession();
Expand Down Expand Up @@ -1020,7 +1020,7 @@ void shouldRedirectToFrontendErrorPageWhenTokenResponseIsNotSuccessful() throws

@Test
void
shouldRedirectToFrontendErrorPageWhenNoSessionCookieButCallToNoSessionOrchestrationServiceThrowsException()
shouldRedirectToFrontendErrorPageWhenNoSessionCookieButCallToCrossBrowserOrchestrationServiceThrowsException()
throws NoSessionException, ParseException {
usingValidSession();
usingValidClientSession();
Expand Down Expand Up @@ -1145,7 +1145,7 @@ void shouldLogoutUserWhenAISReturnsBlockedAccountInTokenStep()
class EnhancedCrossBrowserHandling {

private final String clientSessionIdFromState = "state-client-session-id";
private final OrchClientSessionItem clientSessionFromState =
private final OrchClientSessionItem orchClientSessionFromState =
new OrchClientSessionItem(
clientSessionIdFromState,
authRequestParams,
Expand All @@ -1165,10 +1165,18 @@ class EnhancedCrossBrowserHandling {

@BeforeEach
void setup() throws ParseException {
// Set up default session (from cookie)
usingValidSession();
usingValidClientSession();
usingValidAuthUserInfo();

// Set up other client session from state
when(orchClientSessionService.getClientSession(clientSessionIdFromState))
.thenReturn(Optional.of(orchClientSessionFromState));
when(authUserInfoStorageService.getAuthenticationUserInfo(
TEST_INTERNAL_COMMON_SUBJECT_IDENTIFIER, clientSessionIdFromState))
.thenReturn(Optional.of(authUserInfo));

when(ipvCallbackHelper.generateAuthenticationErrorResponse(
any(), any(), anyBoolean(), anyString(), anyString()))
.thenReturn(
Expand All @@ -1186,7 +1194,7 @@ void itDoesNotReturnToTheRpIfCrossBrowserServiceReturnsEmptyForMismatchInClientS
Json.JsonException {

when(crossBrowserOrchestrationService.generateEntityForMismatchInClientSessionId(
anyMap(), anyString(), any()))
anyMap(), anyString()))
.thenReturn(Optional.empty());

Map<String, Object> userIdentityAdditionalClaims = new HashMap<>();
Expand Down Expand Up @@ -1247,13 +1255,13 @@ void itReturnsToRpIfTheCrossBrowserServiceReturnsAMismatchEntity()
throws NoSessionException, Json.JsonException {

when(crossBrowserOrchestrationService.generateEntityForMismatchInClientSessionId(
anyMap(), anyString(), any()))
anyMap(), anyString()))
.thenReturn(
Optional.of(
new CrossBrowserEntity(
clientSessionIdFromState,
errorObject,
clientSessionFromState)));
orchClientSessionFromState)));

Map<String, String> queryParams = new HashMap<>();
queryParams.put("error", OAuth2Error.ACCESS_DENIED_CODE);
Expand All @@ -1278,6 +1286,72 @@ void itReturnsToRpIfTheCrossBrowserServiceReturnsAMismatchEntity()
.saveIdentityClaimsToDynamo(
any(String.class), any(Subject.class), any(UserInfo.class));
}

@Test
void itDoesNotReturnToTheRpIfMismatchedClientSessionIsRecoverable() throws Exception {
// Use orch session linked to both client sessions
var combinedOrchSession =
new OrchSessionItem(SESSION_ID)
.withInternalCommonSubjectId(TEST_INTERNAL_COMMON_SUBJECT_IDENTIFIER)
.addClientSession(CLIENT_SESSION_ID)
.addClientSession(clientSessionIdFromState);
when(orchSessionService.getSession(SESSION_ID))
.thenReturn(Optional.of(combinedOrchSession));

when(crossBrowserOrchestrationService.generateEntityForMismatchInClientSessionId(
anyMap(), anyString()))
.thenReturn(
Optional.of(
new CrossBrowserEntity(
clientSessionIdFromState,
errorObject,
orchClientSessionFromState)));

var claims =
new HashMap<String, Object>(
Map.of(
"sub",
"sub-val",
"vot",
"P2",
"vtm",
OIDC_BASE_URL + "/trustmark",
IdentityClaims.CORE_IDENTITY.getValue(),
CORE_IDENTITY_CLAIM,
IdentityClaims.CREDENTIAL_JWT.getValue(),
CREDENTIAL_JWT_CLAIM));

var response =
makeHandlerRequest(
getApiGatewayProxyRequestEvent(
new UserInfo(new JSONObject(claims)), clientRegistry));

assertDoesRedirectToFrontendPage(response, FRONT_END_IPV_CALLBACK_URI);
verify(ipvCallbackHelper)
.queueSPOTRequest(
any(),
anyString(),
eq(authUserInfo),
eq(new Subject(TEST_RP_PAIRWISE_ID)),
any(UserInfo.class),
eq(CLIENT_ID.getValue()));
verify(ipvCallbackHelper)
.saveIdentityClaimsToDynamo(
any(String.class), any(Subject.class), any(UserInfo.class));

verifyAuditEventWithClientSessionId(
IPVAuditableEvent.IPV_AUTHORISATION_RESPONSE_RECEIVED,
clientSessionIdFromState);
verifyAuditEventWithClientSessionId(
IPVAuditableEvent.IPV_SUCCESSFUL_TOKEN_RESPONSE_RECEIVED,
clientSessionIdFromState);
verifyAuditEventWithClientSessionId(
IPVAuditableEvent.IPV_SUCCESSFUL_IDENTITY_RESPONSE_RECEIVED,
clientSessionIdFromState);
verifyAuditEventWithClientSessionId(
IPVAuditableEvent.IPV_SPOT_REQUESTED, clientSessionIdFromState);
verifyNoMoreInteractions(auditService);
}
}

static Stream<Arguments> getFrontendCases() {
Expand Down Expand Up @@ -1367,12 +1441,17 @@ public static AuthenticationRequest generateAuthRequest(OIDCClaimsRequest oidcCl
}

private void verifyAuditEvent(IPVAuditableEvent auditableEvent) {
verifyAuditEventWithClientSessionId(auditableEvent, CLIENT_SESSION_ID);
}

private void verifyAuditEventWithClientSessionId(
IPVAuditableEvent auditableEvent, String clientSessionId) {
verify(auditService)
.submitAuditEvent(
auditableEvent,
CLIENT_ID.getValue(),
TxmaAuditUser.user()
.withGovukSigninJourneyId(CLIENT_SESSION_ID)
.withGovukSigninJourneyId(clientSessionId)
.withSessionId(SESSION_ID)
.withUserId(TEST_INTERNAL_COMMON_SUBJECT_IDENTIFIER)
.withEmail(TEST_EMAIL_ADDRESS)
Expand Down
11 changes: 1 addition & 10 deletions ...c/main/java/uk/gov/di/orchestration/shared/services/CrossBrowserOrchestrationService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
import uk.gov.di.orchestration.shared.entity.CrossBrowserEntity;
import uk.gov.di.orchestration.shared.entity.CrossBrowserItem;
import uk.gov.di.orchestration.shared.entity.OrchClientSessionItem;
import uk.gov.di.orchestration.shared.entity.OrchSessionItem;
import uk.gov.di.orchestration.shared.exceptions.NoSessionException;

import java.util.Map;
Expand Down Expand Up @@ -89,9 +88,7 @@ && isStatePresentInQueryParams(queryStringParameters)) {
}

public Optional<CrossBrowserEntity> generateEntityForMismatchInClientSessionId(
Map<String, String> queryStringParameters,
String clientSessionIdFromCookie,
OrchSessionItem orchSession)
Map<String, String> queryStringParameters, String clientSessionIdFromCookie)
throws NoSessionException {
if (!isStatePresentInQueryParams(queryStringParameters)) {
LOG.warn("No state value in query params");
Expand Down Expand Up @@ -133,12 +130,6 @@ public Optional<CrossBrowserEntity> generateEntityForMismatchInClientSessionId(
LOG.warn("Failed to attach client details to logs");
}

if (!isAccessDeniedErrorPresent(queryStringParameters)) {
LOG.info(
"Client session may be recoverable. Client session linked to active session: {}",
orchSession.getClientSessions().contains(clientSessionIdFromState));
}

var errorObject =
new ErrorObject(
OAuth2Error.ACCESS_DENIED_CODE,
Expand Down
15 changes: 6 additions & 9 deletions ...st/java/uk/gov/di/orchestration/shared/services/CrossBrowserOrchestrationServiceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
import org.junit.jupiter.api.Test;
import uk.gov.di.orchestration.shared.entity.CrossBrowserItem;
import uk.gov.di.orchestration.shared.entity.OrchClientSessionItem;
import uk.gov.di.orchestration.shared.entity.OrchSessionItem;
import uk.gov.di.orchestration.shared.exceptions.NoSessionException;
import uk.gov.di.orchestration.shared.helpers.IdGenerator;

Expand Down Expand Up @@ -46,8 +45,6 @@ class CrossBrowserOrchestrationServiceTest {
private static final State STATE = new State();
private static final Nonce NONCE = new Nonce();
private static final String CLIENT_SESSION_ID = "a-client-session-id";
private static final OrchSessionItem ORCH_SESSION_ITEM =
new OrchSessionItem("a-session-id").addClientSession(CLIENT_SESSION_ID);

private CrossBrowserOrchestrationService crossBrowserOrchestrationService;

Expand Down Expand Up @@ -241,7 +238,7 @@ void itShouldThrowNoSessionExceptionIfAccessDeniedErrorWithNoState() {
() ->
crossBrowserOrchestrationService
.generateEntityForMismatchInClientSessionId(
queryParams, CLIENT_SESSION_ID, ORCH_SESSION_ITEM));
queryParams, CLIENT_SESSION_ID));
}

@Test
Expand All @@ -258,7 +255,7 @@ void itShouldReturnANoSessionEntityIfThereIsMismatchInCSIDFromSuccessfulIPVCallb

var noSessionEntity =
crossBrowserOrchestrationService.generateEntityForMismatchInClientSessionId(
queryParams, IdGenerator.generate(), ORCH_SESSION_ITEM);
queryParams, IdGenerator.generate());

assertTrue(noSessionEntity.isPresent());
assertThat(
Expand Down Expand Up @@ -292,7 +289,7 @@ void itShouldGenerateANoSessionEntityWhenCSIDCookieDoesNotMatchStateValue()
queryParams.put("error_description", OAuth2Error.ACCESS_DENIED.getDescription());
var noSessionEntity =
crossBrowserOrchestrationService.generateEntityForMismatchInClientSessionId(
queryParams, cookieClientSessionID, ORCH_SESSION_ITEM);
queryParams, cookieClientSessionID);

assertTrue(noSessionEntity.isPresent());
assertThat(
Expand Down Expand Up @@ -325,7 +322,7 @@ void itShouldReturnEmptyIfClientSessionIdInCookieMatchesStateValue()
queryParams.put("error_description", OAuth2Error.ACCESS_DENIED.getDescription());
var noSessionEntity =
crossBrowserOrchestrationService.generateEntityForMismatchInClientSessionId(
queryParams, CLIENT_SESSION_ID, ORCH_SESSION_ITEM);
queryParams, CLIENT_SESSION_ID);

assertTrue(noSessionEntity.isEmpty());
}
Expand All @@ -346,7 +343,7 @@ void itShouldThrowIfThereIsNoClientSessionIdAssociatedWithStateValue() {
() ->
crossBrowserOrchestrationService
.generateEntityForMismatchInClientSessionId(
queryParams, CLIENT_SESSION_ID, ORCH_SESSION_ITEM));
queryParams, CLIENT_SESSION_ID));
}

@Test
Expand All @@ -368,7 +365,7 @@ void itShouldThrowIfThereIsNoClientSessionIdAssociatedWithStateValue() {
() ->
crossBrowserOrchestrationService
.generateEntityForMismatchInClientSessionId(
queryParams, CLIENT_SESSION_ID, ORCH_SESSION_ITEM));
queryParams, CLIENT_SESSION_ID));
}
}

Expand Down
Loading