Skip to content

Commit

Permalink
ATO-1358: Redirect to RP on cross browser issue
Browse files Browse the repository at this point in the history
  • Loading branch information
@ethanmills
ethanmills committed Jan 30, 2025
1 parent 3604bd3 commit 5c5eb19
Show file tree
Hide file tree
Showing 4 changed files with 186 additions and 26 deletions.
69 changes: 63 additions & 6 deletions .../test/java/uk/gov/di/authentication/api/AuthenticationCallbackHandlerIntegrationTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,15 @@
import com.nimbusds.jwt.SignedJWT;
import com.nimbusds.oauth2.sdk.AuthorizationCode;
import com.nimbusds.oauth2.sdk.AuthorizationRequest;
import com.nimbusds.oauth2.sdk.ErrorObject;
import com.nimbusds.oauth2.sdk.OAuth2Error;
import com.nimbusds.oauth2.sdk.ParseException;
import com.nimbusds.oauth2.sdk.ResponseType;
import com.nimbusds.oauth2.sdk.Scope;
import com.nimbusds.oauth2.sdk.id.ClientID;
import com.nimbusds.oauth2.sdk.id.State;
import com.nimbusds.oauth2.sdk.id.Subject;
import com.nimbusds.openid.connect.sdk.AuthenticationErrorResponse;
import com.nimbusds.openid.connect.sdk.AuthenticationRequest;
import com.nimbusds.openid.connect.sdk.Nonce;
import com.nimbusds.openid.connect.sdk.OIDCScopeValue;
Expand Down Expand Up @@ -73,6 +75,7 @@
import java.util.Optional;

import static com.nimbusds.jose.JWSAlgorithm.ES256;
import static java.util.Collections.emptyMap;
import static java.util.Collections.singletonList;
import static org.awaitility.Awaitility.await;
import static org.hamcrest.MatcherAssert.assertThat;
Expand All @@ -88,6 +91,7 @@
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static uk.gov.di.authentication.oidc.domain.OrchestrationAuditableEvent.AUTH_UNSUCCESSFUL_CALLBACK_RESPONSE_RECEIVED;
import static uk.gov.di.orchestration.shared.entity.VectorOfTrust.parseFromAuthRequestAttribute;
import static uk.gov.di.orchestration.shared.helpers.ConstructUriHelper.buildURI;
import static uk.gov.di.orchestration.sharedtest.helper.AuditAssertionsHelper.assertTxmaAuditEventsReceived;
Expand Down Expand Up @@ -198,8 +202,7 @@ void shouldRedirectToRpWithErrorWhenStateIsInvalid() throws ParseException {
containsString(RP_STATE.getValue()));

assertTxmaAuditEventsReceived(
txmaAuditQueue,
List.of(OrchestrationAuditableEvent.AUTH_UNSUCCESSFUL_CALLBACK_RESPONSE_RECEIVED));
txmaAuditQueue, List.of(AUTH_UNSUCCESSFUL_CALLBACK_RESPONSE_RECEIVED));

Optional<UserInfo> userInfo =
userInfoStoreExtension.getAuthenticationUserInfo(SUBJECT_ID.getValue());
Expand Down Expand Up @@ -257,6 +260,46 @@ void shouldRedirectToFrontendErrorPageIfUnsuccessfulResponseReceivedFromUserInfo
OrchestrationAuditableEvent.AUTH_UNSUCCESSFUL_USERINFO_RESPONSE_RECEIVED));
}

@Test
void
shouldRedirectToRPWhenNoSessionCookieAndCallToNoSessionOrchestrationServiceReturnsNoSessionEntity()
throws Json.JsonException {
var scope = new Scope(OIDCScopeValue.OPENID);
var authRequestBuilder =
new AuthenticationRequest.Builder(
ResponseType.CODE, scope, new ClientID(CLIENT_ID), REDIRECT_URI)
.nonce(new Nonce())
.state(RP_STATE);
redis.createClientSession(
CLIENT_SESSION_ID, CLIENT_NAME, authRequestBuilder.build().toParameters());
redis.addClientSessionAndStateToRedis(ORCH_TO_AUTH_STATE, CLIENT_SESSION_ID);

var response =
makeRequest(
Optional.empty(),
emptyMap(),
new HashMap<>(
Map.of(
"state",
ORCH_TO_AUTH_STATE.getValue(),
"error",
"access_denied")));

var error =
new ErrorObject(
OAuth2Error.ACCESS_DENIED_CODE,
"Access denied for security reasons, a new authentication request may be successful");

var expectedURI =
new AuthenticationErrorResponse(REDIRECT_URI, error, RP_STATE, null)
.toURI()
.toString();
assertThat(response, hasStatus(302));
assertThat(response.getHeaders().get(ResponseHeaders.LOCATION), equalTo(expectedURI));
assertTxmaAuditEventsReceived(
txmaAuditQueue, singletonList(AUTH_UNSUCCESSFUL_CALLBACK_RESPONSE_RECEIVED));
}

@Test
void shouldRedirectToIPVWhenIdentityRequired()
throws ParseException, JOSEException, java.text.ParseException, Json.JsonException {
Expand All @@ -275,6 +318,7 @@ void shouldRedirectToIPVWhenIdentityRequired()

assertRedirectToIpv(response, false);
assertOrchSessionIsUpdatedWithUserInfoClaims();
assertInformationStoredForNoSessionService(response);
}

void accountInterventionSetup() throws Json.JsonException {
Expand Down Expand Up @@ -966,10 +1010,7 @@ private void assertRedirectToBlockedPage(APIGatewayProxyResponseEvent response)

private void assertRedirectToIpv(APIGatewayProxyResponseEvent response, boolean reproveIdentity)
throws java.text.ParseException, JOSEException, ParseException {
var authRequest = validateQueryRequestToIPVAndReturnAuthRequest(response);

var encryptedRequestObject = authRequest.getRequestObject();
var signedJWTResponse = decryptJWT((EncryptedJWT) encryptedRequestObject);
var signedJWTResponse = validateAndDecryptRequestObject(response);

validateClaimsInJar(signedJWTResponse, reproveIdentity);

Expand All @@ -984,6 +1025,22 @@ private void assertRedirectToIpv(APIGatewayProxyResponseEvent response, boolean
IPVAuditableEvent.IPV_AUTHORISATION_REQUESTED));
}

private SignedJWT validateAndDecryptRequestObject(APIGatewayProxyResponseEvent response)
throws ParseException, JOSEException {
var authRequest = validateQueryRequestToIPVAndReturnAuthRequest(response);

var encryptedRequestObject = authRequest.getRequestObject();
return decryptJWT((EncryptedJWT) encryptedRequestObject);
}

private void assertInformationStoredForNoSessionService(APIGatewayProxyResponseEvent response)
throws java.text.ParseException, ParseException, JOSEException {
var requestObject = validateAndDecryptRequestObject(response);
var state = requestObject.getJWTClaimsSet().getClaim("state");
var noSessionObject = redis.getFromRedis("state:" + state);
assertEquals(CLIENT_SESSION_ID, noSessionObject);
}

private void assertUserInfoStoredAndRedirectedToRp(APIGatewayProxyResponseEvent response)
throws ParseException {
assertThat(response, hasStatus(302));
Expand Down
4 changes: 4 additions & 0 deletions ...c/main/java/uk/gov/di/authentication/oidc/exceptions/AuthenticationCallbackException.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,8 @@ public class AuthenticationCallbackException extends RuntimeException {
public AuthenticationCallbackException(String message) {
super(message);
}

public AuthenticationCallbackException(String message, Throwable cause) {
super(message, cause);
}
}
48 changes: 44 additions & 4 deletions ...api/src/main/java/uk/gov/di/authentication/oidc/lambda/AuthenticationCallbackHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
import uk.gov.di.orchestration.shared.entity.Session;
import uk.gov.di.orchestration.shared.entity.Session.AccountState;
import uk.gov.di.orchestration.shared.entity.VectorOfTrust;
import uk.gov.di.orchestration.shared.exceptions.NoSessionException;
import uk.gov.di.orchestration.shared.exceptions.UnsuccessfulCredentialResponseException;
import uk.gov.di.orchestration.shared.helpers.CookieHelper;
import uk.gov.di.orchestration.shared.helpers.IpAddressHelper;
Expand Down Expand Up @@ -115,6 +116,7 @@ public class AuthenticationCallbackHandler
private final AccountInterventionService accountInterventionService;
private final LogoutService logoutService;
private final AuthFrontend authFrontend;
private final NoSessionOrchestrationService noSessionOrchestrationService;

public AuthenticationCallbackHandler() {
this(ConfigurationService.getInstance());
Expand Down Expand Up @@ -156,13 +158,15 @@ public AuthenticationCallbackHandler(ConfigurationService configurationService)
configurationService, cloudwatchMetricsService, auditService);
this.logoutService = new LogoutService(configurationService);
this.authFrontend = new AuthFrontend(configurationService);
this.noSessionOrchestrationService =
new NoSessionOrchestrationService(configurationService);
}

public AuthenticationCallbackHandler(
ConfigurationService configurationService, RedisConnectionService redis) {
ConfigurationService configurationService,
RedisConnectionService redisConnectionService) {

var kmsConnectionService = new KmsConnectionService(configurationService);
var redisConnectionService = redis;
this.configurationService = configurationService;
this.authorisationService = new AuthenticationAuthorizationService(redisConnectionService);
this.tokenService =
Expand Down Expand Up @@ -200,6 +204,8 @@ public AuthenticationCallbackHandler(
configurationService, cloudwatchMetricsService, auditService);
this.logoutService = new LogoutService(configurationService, redisConnectionService);
this.authFrontend = new AuthFrontend(configurationService);
this.noSessionOrchestrationService =
new NoSessionOrchestrationService(configurationService, redisConnectionService);
}

public AuthenticationCallbackHandler(
Expand All @@ -217,7 +223,8 @@ public AuthenticationCallbackHandler(
InitiateIPVAuthorisationService initiateIPVAuthorisationService,
AccountInterventionService accountInterventionService,
LogoutService logoutService,
AuthFrontend authFrontend) {
AuthFrontend authFrontend,
NoSessionOrchestrationService noSessionOrchestrationService) {
this.configurationService = configurationService;
this.authorisationService = responseService;
this.tokenService = tokenService;
Expand All @@ -233,6 +240,7 @@ public AuthenticationCallbackHandler(
this.accountInterventionService = accountInterventionService;
this.logoutService = logoutService;
this.authFrontend = authFrontend;
this.noSessionOrchestrationService = noSessionOrchestrationService;
}

public APIGatewayProxyResponseEvent handleRequest(
Expand All @@ -246,7 +254,7 @@ public APIGatewayProxyResponseEvent handleRequest(
CookieHelper.parseSessionCookie(input.getHeaders()).orElse(null);

if (sessionCookiesIds == null) {
throw new AuthenticationCallbackException("No session cookie found");
return handleMissingSession(input);
}

var sessionId = sessionCookiesIds.getSessionId();
Expand Down Expand Up @@ -623,6 +631,38 @@ public APIGatewayProxyResponseEvent handleRequest(
}
}

private APIGatewayProxyResponseEvent handleMissingSession(APIGatewayProxyRequestEvent input)
throws ParseException {
try {
return handleCrossBrowserError(input);
} catch (NoSessionException e) {
throw new AuthenticationCallbackException("No session cookie found", e);
}
}

private APIGatewayProxyResponseEvent handleCrossBrowserError(APIGatewayProxyRequestEvent input)
throws NoSessionException, ParseException {
var noSessionEntity =
noSessionOrchestrationService.generateNoSessionOrchestrationEntity(
input.getQueryStringParameters());
var authenticationRequest =
AuthenticationRequest.parse(
noSessionEntity.getClientSession().getAuthRequestParams());
auditService.submitAuditEvent(
OrchestrationAuditableEvent.AUTH_UNSUCCESSFUL_CALLBACK_RESPONSE_RECEIVED,
authenticationRequest.getClientID().getValue(),
TxmaAuditUser.user()
.withGovukSigninJourneyId(noSessionEntity.getClientSessionId()));
var errorResponse =
new AuthenticationErrorResponse(
authenticationRequest.getRedirectionURI(),
noSessionEntity.getErrorObject(),
authenticationRequest.getState(),
authenticationRequest.getResponseMode());
return generateApiGatewayProxyResponse(
302, "", Map.of(ResponseHeaders.LOCATION, errorResponse.toURI().toString()), null);
}

private boolean deduceUpliftRequired(UserInfo userInfo) {
Boolean upliftRequiredClaim =
userInfo.getBooleanClaim(AuthUserInfoClaims.UPLIFT_REQUIRED.getValue());
Expand Down
Loading

0 comments on commit 5c5eb19

Please sign in to comment.