Skip to content

Commit

Permalink
Merge pull request #1908 from govuk-one-login/bau-check-session-id-no…
Browse files Browse the repository at this point in the history 
…t-blank

BAU: Check ipvSessionId is not empty string
  • Loading branch information
@Wynndow
Wynndow authored May 13, 2024
2 parents c6676e8 + b8d3bec commit 31a73d2
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 24 deletions.
4 changes: 2 additions & 2 deletions ...ain/java/uk/gov/di/ipv/core/buildclientoauthresponse/BuildClientOauthResponseHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@
import static uk.gov.di.ipv.core.library.helpers.RequestHelper.getClientOAuthSessionId;
import static uk.gov.di.ipv.core.library.helpers.RequestHelper.getFeatureSet;
import static uk.gov.di.ipv.core.library.helpers.RequestHelper.getIpAddress;
import static uk.gov.di.ipv.core.library.helpers.RequestHelper.getIpvSessionIdAllowNull;
import static uk.gov.di.ipv.core.library.helpers.RequestHelper.getIpvSessionIdAllowBlank;
import static uk.gov.di.ipv.core.library.journeyuris.JourneyUris.JOURNEY_ERROR_PATH;

public class BuildClientOauthResponseHandler
Expand Down Expand Up @@ -92,7 +92,7 @@ public Map<String, Object> handleRequest(JourneyRequest input, Context context)
LogHelper.attachComponentId(configService);

try {
String ipvSessionId = getIpvSessionIdAllowNull(input);
String ipvSessionId = getIpvSessionIdAllowBlank(input);
String ipAddress = getIpAddress(input);
String clientSessionId = getClientOAuthSessionId(input);
List<String> featureSet = getFeatureSet(input);
Expand Down
35 changes: 18 additions & 17 deletions libs/common-services/src/main/java/uk/gov/di/ipv/core/library/helpers/RequestHelper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import java.util.stream.Stream;

import static com.nimbusds.oauth2.sdk.http.HTTPResponse.SC_BAD_REQUEST;
import static software.amazon.awssdk.utils.StringUtils.isBlank;
import static uk.gov.di.ipv.core.library.helpers.LogHelper.LogField.LOG_MESSAGE_DESCRIPTION;

public class RequestHelper {
Expand Down Expand Up @@ -51,15 +52,15 @@ public static String getHeaderByKey(Map<String, String> headers, String headerKe

public static String getIpvSessionId(APIGatewayProxyRequestEvent event)
throws HttpResponseExceptionWithErrorBody {
return getIpvSessionId(event.getHeaders(), false);
return getIpvSessionId(event.getHeaders());
}

public static String getIpvSessionId(JourneyRequest event)
throws HttpResponseExceptionWithErrorBody {
return getIpvSessionId(event, false);
}

public static String getIpvSessionIdAllowNull(JourneyRequest event)
public static String getIpvSessionIdAllowBlank(JourneyRequest event)
throws HttpResponseExceptionWithErrorBody {
return getIpvSessionId(event, true);
}
Expand Down Expand Up @@ -87,16 +88,6 @@ public static String getClientOAuthSessionId(JourneyRequest event) {
return StringUtils.isBlank(clientSessionId) ? null : clientSessionId;
}

public static String getIpvSessionId(JourneyRequest request, boolean allowNull)
throws HttpResponseExceptionWithErrorBody {
String ipvSessionId = request.getIpvSessionId();

validateIpvSessionId(ipvSessionId, "ipvSessionId not present in request", allowNull);

LogHelper.attachIpvSessionIdToLogs(ipvSessionId);
return ipvSessionId;
}

public static List<String> getFeatureSet(JourneyRequest request) {
String featureSet = request.getFeatureSet();
List<String> featureSetList =
Expand Down Expand Up @@ -184,22 +175,32 @@ private static <T> T extractValueFromLambdaInput(
return value;
}

private static String getIpvSessionId(Map<String, String> headers, boolean allowNull)
private static String getIpvSessionId(Map<String, String> headers)
throws HttpResponseExceptionWithErrorBody {
String ipvSessionId = RequestHelper.getHeaderByKey(headers, IPV_SESSION_ID_HEADER);
String message = String.format("%s not present in header", IPV_SESSION_ID_HEADER);

validateIpvSessionId(ipvSessionId, message, allowNull);
validateIpvSessionId(ipvSessionId, message, false);

LogHelper.attachIpvSessionIdToLogs(ipvSessionId);
return ipvSessionId;
}

private static String getIpvSessionId(JourneyRequest request, boolean allowBlank)
throws HttpResponseExceptionWithErrorBody {
String ipvSessionId = request.getIpvSessionId();

validateIpvSessionId(ipvSessionId, "ipvSessionId not present in request", allowBlank);

LogHelper.attachIpvSessionIdToLogs(ipvSessionId);
return ipvSessionId;
}

private static void validateIpvSessionId(
String ipvSessionId, String errorMessage, boolean allowNull)
String ipvSessionId, String errorMessage, boolean allowBlank)
throws HttpResponseExceptionWithErrorBody {
if (ipvSessionId == null) {
if (allowNull) {
if (isBlank(ipvSessionId)) {
if (allowBlank) {
LOGGER.warn(LogHelper.buildLogMessage(errorMessage));
} else {
LOGGER.error(LogHelper.buildLogMessage(errorMessage));
Expand Down
52 changes: 47 additions & 5 deletions libs/common-services/src/test/java/uk/gov/di/ipv/core/library/helpers/RequestHelperTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
import static uk.gov.di.ipv.core.library.helpers.RequestHelper.getHeaderByKey;
import static uk.gov.di.ipv.core.library.helpers.RequestHelper.getIpAddress;
import static uk.gov.di.ipv.core.library.helpers.RequestHelper.getIpvSessionId;
import static uk.gov.di.ipv.core.library.helpers.RequestHelper.getIpvSessionIdAllowNull;
import static uk.gov.di.ipv.core.library.helpers.RequestHelper.getIpvSessionIdAllowBlank;

class RequestHelperTest {
private final String TEST_IPV_SESSION_ID = "a-session-id";
Expand Down Expand Up @@ -74,7 +74,7 @@ void getIpvSessionIdShouldReturnSessionId() throws HttpResponseExceptionWithErro
}

@Test
void getIpvSessionIdShouldReturnSessionIdFromJourney()
void getIpvSessionIdShouldReturnSessionIdFromJourneyEvent()
throws HttpResponseExceptionWithErrorBody {
var event =
JourneyRequest.builder()
Expand All @@ -89,7 +89,49 @@ void getIpvSessionIdShouldReturnSessionIdFromJourney()
}

@Test
void getIpvSessionIdShouldThrowIfSessionIdIsNull() {
void getIpvSessionIdShouldThrowIfSessionIdIsNullInJourneyEvent() {
var event =
JourneyRequest.builder()
.ipvSessionId(null)
.ipAddress(TEST_IP_ADDRESS)
.clientOAuthSessionId(TEST_CLIENT_SESSION_ID)
.journey(TEST_JOURNEY)
.featureSet(TEST_FEATURE_SET)
.build();

var exception =
assertThrows(
HttpResponseExceptionWithErrorBody.class, () -> getIpvSessionId(event));

assertEquals(SC_BAD_REQUEST, exception.getResponseCode());
assertEquals(
ErrorResponse.MISSING_IPV_SESSION_ID.getMessage(),
exception.getErrorResponse().getMessage());
}

@Test
void getIpvSessionIdShouldThrowIfSessionIdIsEmptyStringInJourneyEvent() {
var event =
JourneyRequest.builder()
.ipvSessionId("")
.ipAddress(TEST_IP_ADDRESS)
.clientOAuthSessionId(TEST_CLIENT_SESSION_ID)
.journey(TEST_JOURNEY)
.featureSet(TEST_FEATURE_SET)
.build();

var exception =
assertThrows(
HttpResponseExceptionWithErrorBody.class, () -> getIpvSessionId(event));

assertEquals(SC_BAD_REQUEST, exception.getResponseCode());
assertEquals(
ErrorResponse.MISSING_IPV_SESSION_ID.getMessage(),
exception.getErrorResponse().getMessage());
}

@Test
void getIpvSessionIdShouldThrowIfSessionIdIsNullInEvent() {
var event = new APIGatewayProxyRequestEvent();
HashMap<String, String> headers = new HashMap<>();
headers.put(IPV_SESSION_ID_HEADER, null);
Expand All @@ -107,7 +149,7 @@ void getIpvSessionIdShouldThrowIfSessionIdIsNull() {
}

@Test
void getIpvSessionIdShouldThrowIfSessionIdIsEmptyString() {
void getIpvSessionIdShouldThrowIfSessionIdIsEmptyStringInEvent() {
var event = new APIGatewayProxyRequestEvent();

event.setHeaders(Map.of(IPV_SESSION_ID_HEADER, ""));
Expand Down Expand Up @@ -216,7 +258,7 @@ void forJourneyRequestShouldReturnNullIfSessionIdIsNull()
.featureSet(featureSet)
.build();

assertNull(getIpvSessionIdAllowNull(event));
assertNull(getIpvSessionIdAllowBlank(event));
}

@Test
Expand Down

0 comments on commit 31a73d2

Please sign in to comment.