Skip to content

Commit

Permalink
[ALS-4980] pic sure medium findings (#144)
Browse files Browse the repository at this point in the history
[ALS-4980] Add additional validation of parameters
[ALS-4980] Use converter to enforce UUID
  • Loading branch information
Gcolon021 authored Sep 15, 2023
1 parent 187d477 commit 697e8aa
Show file tree
Hide file tree
Showing 10 changed files with 93 additions and 39 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -163,31 +163,31 @@ public QueryStatus query(QueryRequest queryRequest) {

}

@POST
@Path("/query/{resourceQueryId}/status")
@Override
public QueryStatus queryStatus(@PathParam("resourceQueryId") String queryId, QueryRequest statusRequest) {
logger.debug("Calling Aggregate Data Sharing Resource queryStatus() for query {}", queryId);
checkQuery(statusRequest);
HttpResponse response = postRequest(statusRequest, "/query/" + queryId + "/status");
return readObjectFromResponse(response, QueryStatus.class);
}

@POST
@Path("/query/{resourceQueryId}/result")
@Override
public Response queryResult(@PathParam("resourceQueryId") String queryId, QueryRequest resultRequest) {
logger.debug("Calling Aggregate Data Sharing Resource queryResult() for query {}", queryId);
checkQuery(resultRequest);
HttpResponse response = postRequest(resultRequest, "/query/" + queryId + "/result");
try {
return Response.ok(response.getEntity().getContent()).build();
} catch (IOException e) {
throw new ApplicationException(
"Error encoding query for resource with id " + resultRequest.getResourceUUID()
);
}
}
@POST
@Path("/query/{resourceQueryId}/status")
@Override
public QueryStatus queryStatus(@PathParam("resourceQueryId") UUID queryId, QueryRequest statusRequest) {
logger.debug("Calling Aggregate Data Sharing Resource queryStatus() for query {}", queryId);
checkQuery(statusRequest);
HttpResponse response = postRequest(statusRequest, "/query/" + queryId + "/status");
return readObjectFromResponse(response, QueryStatus.class);
}

@POST
@Path("/query/{resourceQueryId}/result")
@Override
public Response queryResult(@PathParam("resourceQueryId") UUID queryId, QueryRequest resultRequest) {
logger.debug("Calling Aggregate Data Sharing Resource queryResult() for query {}", queryId);
checkQuery(resultRequest);
HttpResponse response = postRequest(resultRequest, "/query/" + queryId + "/result");
try {
return Response.ok(response.getEntity().getContent()).build();
} catch (IOException e) {
throw new ApplicationException(
"Error encoding query for resource with id " + resultRequest.getResourceUUID()
);
}
}

private HttpResponse postRequest(QueryRequest statusRequest, String pathName) {
try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -346,14 +346,17 @@ public void shouldPostQueryStatus() {
QueryStatus expectedResponse = new QueryStatus();
expectedResponse.setResourceID(targetResourceId);

UUID randomUUID = UUID.randomUUID();
String requestUUID = randomUUID.toString();

ProxyPostEndpointMocker.start(wireMockRule)
.withPath("/query/aaaaaaaaaaaaaah/status")
.withPath("/query/" + requestUUID + "/status")
.withRequestBody(postedRequest)
.withResponseBody(expectedResponse)
.withStatusCode(200)
.commit();

QueryStatus actual = subject.queryStatus("aaaaaaaaaaaaaah", originalRequest);
QueryStatus actual = subject.queryStatus(randomUUID, originalRequest);

// equality isn't defined for QueryRequest, and I'm scared to define it, so let's
// just compare resource IDs
Expand All @@ -370,14 +373,17 @@ public void shouldPostQueryResult() {

Response expectedResponse = new OutboundJaxrsResponse(Response.Status.OK, new OutboundMessageContext());

UUID randomUUID = UUID.randomUUID();
String requestUUID = randomUUID.toString();

ProxyPostEndpointMocker.start(wireMockRule)
.withPath("/query/aaaaaaaaaaaaaah/result")
.withPath("/query/" + requestUUID + "/result")
.withRequestBody(postedRequest)
.withResponseBody(expectedResponse)
.withStatusCode(200)
.commit();

Response actual = subject.queryResult("aaaaaaaaaaaaaah", originalRequest);
Response actual = subject.queryResult(randomUUID, originalRequest);

assertEquals(expectedResponse.getStatus(), actual.getStatus());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ public QueryStatus query(QueryRequest queryJson) {
@POST
@Path("/query/{resourceQueryId}/status")
@Override
public QueryStatus queryStatus(@PathParam("resourceQueryId") String queryId, QueryRequest statusRequest) {
public QueryStatus queryStatus(@PathParam("resourceQueryId") UUID queryId, QueryRequest statusRequest) {
logger.debug("Getting status for for queryId {}", queryId);

retrieveTargetUrl(statusRequest);
Expand Down Expand Up @@ -351,7 +351,7 @@ public QueryStatus queryStatus(@PathParam("resourceQueryId") String queryId, Que
@POST
@Path("/query/{dataObjectId}/result")
@Override
public Response queryResult(@PathParam("dataObjectId") String dataObjectId, QueryRequest statusRequest) {
public Response queryResult(@PathParam("dataObjectId") UUID dataObjectId, QueryRequest statusRequest) {
logger.debug("queryResult() calling dataobject/{}", dataObjectId);

retrieveTargetUrl(statusRequest);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ public QueryStatus query(QueryRequest queryJson) {
@POST
@Path("/query/{resourceQueryId}/status")
@Override
public QueryStatus queryStatus(@PathParam("resourceQueryId") String queryId, QueryRequest statusQuery) {
public QueryStatus queryStatus(@PathParam("resourceQueryId") UUID queryId, QueryRequest statusQuery) {
logger.debug("calling HSAPI Resource queryStatus() for query {}", queryId);
throw new UnsupportedOperationException("Query status is not implemented in this resource. Please use query/sync");

Expand All @@ -147,7 +147,7 @@ public QueryStatus queryStatus(@PathParam("resourceQueryId") String queryId, Que
@POST
@Path("/query/{resourceQueryId}/result")
@Override
public Response queryResult(@PathParam("resourceQueryId") String queryId, QueryRequest statusQuery) {
public Response queryResult(@PathParam("resourceQueryId") UUID queryId, QueryRequest statusQuery) {
logger.debug("calling HSAPI Resource queryResult() for query {}", queryId);
throw new UnsupportedOperationException("Query result is not implemented in this resource. Please use query/sync");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ void testQueryResult() throws Exception {
when(httpClient.retrievePostResponse(anyString(), any(Header[].class), anyString())).thenReturn(httpResponse);

assertThrows(ProtocolException.class, () -> {
resource.queryResult(null, null);
resource.queryResult("", null);
}, "QueryID is required");

assertThrows(ProtocolException.class, () -> {
Expand Down Expand Up @@ -185,7 +185,7 @@ void testQueryStatus() throws Exception {
when(httpClient.retrievePostResponse(anyString(), any(Header[].class), anyString())).thenReturn(httpResponse);

assertThrows(ProtocolException.class, () -> {
resource.queryStatus(null, null);
resource.queryStatus("", null);
}, "QueryID is required");

assertThrows(ProtocolException.class, () -> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
import edu.harvard.dbmi.avillach.domain.*;
import io.swagger.v3.oas.annotations.Operation;

import java.util.UUID;

@Path("/pic-sure")
@Produces("application/json")
@Consumes("application/json")
Expand Down Expand Up @@ -36,14 +38,14 @@ default QueryStatus query(QueryRequest queryJson) {
@POST
@Path("/query/{resourceQueryId}/status")
@Operation(hidden = true)
default QueryStatus queryStatus(String queryId, QueryRequest statusRequest) {
default QueryStatus queryStatus(UUID queryId, QueryRequest statusRequest) {
throw new NotSupportedException();
}

@POST
@Path("/query/{resourceQueryId}/result")
@Operation(hidden = true)
default Response queryResult(String queryId, QueryRequest resultRequest) {
default Response queryResult(UUID queryId, QueryRequest resultRequest) {
throw new NotSupportedException();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -231,8 +231,13 @@ private Map<String, Map<String, String>> getOpenContinuousCrossCounts(QueryReque
* @return Response - the binned data
*/
public Response generateContinuousBin(QueryRequest continuousData) {
// validate the continuous data
if (continuousData == null || continuousData.getQuery() == null) {
return Response.status(Response.Status.BAD_REQUEST).entity("Continuous data is required.").build();
}

logger.info("Continuous data: " + continuousData.getQuery());
Map<String, Map<String, Integer>> continuousDataMap = mapper.convertValue(continuousData.getQuery(), new TypeReference<Map<String, Map<String, Integer>>>() {});
Map<String, Map<String, Integer>> continuousDataMap = mapper.convertValue(continuousData.getQuery(), new TypeReference<>() {});
Map<String, Map<String, Integer>> continuousProcessedData = dataProcessingServices.binContinuousData(continuousDataMap);
return Response.ok(continuousProcessedData).build();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,7 @@ public static JsonNode simpleGetWithConfig(
private static HttpClient getConfiguredHttpClient() {
try {
SSLConnectionSocketFactory.getSocketFactory();
SSLContext sslContext = SSLContext.getInstance("TLS");
SSLContext sslContext = SSLContext.getInstance("TLSv1.2");
sslContext.init(null, null, null);
String[] defaultCiphers = sslContext.getServerSocketFactory().getDefaultCipherSuites();

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
package edu.harvard.dbmi.avillach.util.converter;

import edu.harvard.dbmi.avillach.util.exception.ProtocolException;

import javax.ws.rs.ext.ParamConverter;
import java.util.UUID;

public class UUIDParamConverter implements ParamConverter<UUID> {

@Override
public UUID fromString(String value) {
try {
return UUID.fromString(value);
} catch (IllegalArgumentException e) {
throw new ProtocolException(ProtocolException.INCORRECTLY_FORMATTED_REQUEST);
}
}

@Override
public String toString(UUID value) {
return value.toString();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package edu.harvard.dbmi.avillach.util.converter;

import javax.ws.rs.ext.ParamConverter;
import javax.ws.rs.ext.ParamConverterProvider;
import javax.ws.rs.ext.Provider;
import java.util.UUID;

@Provider
public class UUIDParamConverterProvider implements ParamConverterProvider {

@Override
public <T> ParamConverter<T> getConverter(Class<T> rawType, java.lang.reflect.Type genericType, java.lang.annotation.Annotation[] annotations) {
if (rawType.equals(UUID.class)) {
return (ParamConverter<T>) new UUIDParamConverter();
}
return null;
}
}

0 comments on commit 697e8aa

Please sign in to comment.