-
Notifications
You must be signed in to change notification settings - Fork 40
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.
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
Update validation endpoint and remove authentication #8056
Changes from 7 commits
932ff75
59e6c6a
7f259ae
5d25981
541749f
d500c46
27bd1a0
8635f03
742c68a
c3b373c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,67 @@ | ||
openapi: 3.0.2 | ||
info: | ||
title: Prime ReportStream | ||
description: A router of public health data from multiple senders and receivers | ||
contact: | ||
name: USDS at Centers for Disease Control and Prevention | ||
url: https://reportstream.cdc.gov | ||
email: reportstream@cdc.gov | ||
version: 0.2.0-oas3 | ||
tags: | ||
- name: validate | ||
description: ReportStream validation API | ||
|
||
paths: | ||
# The validation endpoints are public endpoints for validating payloads of various formats. | ||
/validate: | ||
post: | ||
tags: | ||
- validate | ||
summary: Validate a message using client information | ||
parameters: | ||
- in: header | ||
name: client | ||
description: The client.sender to validate against. If client is not known, use `schema` and `format` instead. | ||
schema: | ||
type: string | ||
example: simple_report.default | ||
- in: query | ||
name: schema | ||
description: > | ||
The schema path to validate the message against. Must be use with `format`. | ||
This parameter is incompatible with `client`. | ||
schema: | ||
type: string | ||
example: hl7/hcintegrations-covid-19 | ||
- in: query | ||
name: format | ||
description: > | ||
The format of the message. must be used with `schema`. | ||
This parameter is incompatible with `client`. | ||
schema: | ||
type: string | ||
enum: | ||
- CSV | ||
- HL7 | ||
- HL7_BATCH | ||
example: HL7 | ||
requestBody: | ||
description: The message to validate | ||
required: true | ||
content: | ||
text/csv: | ||
schema: | ||
type: string | ||
example: | ||
header1, header2 | ||
|
||
value1, value2 | ||
responses: | ||
'200': | ||
description: OK | ||
content: | ||
application/json: | ||
schema: | ||
$ref: 'https://raw.githubusercontent.com/CDCgov/prime-reportstream/master/prime-router/docs/api/reports.yml#/components/schemas/Report' | ||
'400': | ||
description: Bad Request |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,6 +6,7 @@ import gov.cdc.prime.router.ActionLogger | |
import gov.cdc.prime.router.DEFAULT_SEPARATOR | ||
import gov.cdc.prime.router.HasSchema | ||
import gov.cdc.prime.router.InvalidParamMessage | ||
import gov.cdc.prime.router.Metadata | ||
import gov.cdc.prime.router.ROUTE_TO_SEPARATOR | ||
import gov.cdc.prime.router.Schema | ||
import gov.cdc.prime.router.Sender | ||
|
@@ -17,6 +18,8 @@ const val DEFAULT_PARAMETER = "default" | |
const val ROUTE_TO_PARAMETER = "routeTo" | ||
const val ALLOW_DUPLICATES_PARAMETER = "allowDuplicate" | ||
const val TOPIC_PARAMETER = "topic" | ||
const val SCHEMA_PARAMETER = "schema" | ||
const val FORMAT_PARAMETER = "format" | ||
|
||
/** | ||
* Base class for ReportFunction and ValidateFunction | ||
|
@@ -31,7 +34,7 @@ abstract class RequestFunction( | |
val content: String = "", | ||
val defaults: Map<String, String> = emptyMap(), | ||
val routeTo: List<String> = emptyList(), | ||
val sender: Sender, | ||
val sender: Sender?, | ||
val topic: String = "covid-19" | ||
) | ||
|
||
|
@@ -70,27 +73,40 @@ abstract class RequestFunction( | |
routeTo.filter { workflowEngine.settings.findReceiver(it) == null } | ||
.forEach { actionLogs.error(InvalidParamMessage("Invalid receiver name: $it")) } | ||
|
||
var sender: Sender? = null | ||
var schema: Schema? = null | ||
val clientName = extractClient(request) | ||
if (clientName.isBlank()) { | ||
actionLogs.error(InvalidParamMessage("Expected a '$CLIENT_PARAMETER' query parameter")) | ||
} | ||
|
||
val sender = workflowEngine.settings.findSender(clientName) | ||
if (sender == null) { | ||
actionLogs.error(InvalidParamMessage("'$CLIENT_PARAMETER:$clientName': unknown sender")) | ||
} | ||
|
||
// verify schema if the sender is a topic sender | ||
var schema: Schema? = null | ||
if (sender != null && sender is HasSchema) { | ||
schema = workflowEngine.metadata.findSchema(sender.schemaName) | ||
if (schema == null) { | ||
// Find schema via SCHEMA_PARAMETER parameter | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note: You will need to refactor this as some point as this only supports the covid pipeline. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Noted, created #8073 to track this work. |
||
val schemaName = request.queryParameters.getOrDefault(SCHEMA_PARAMETER, null) | ||
if (schemaName == null) { | ||
actionLogs.error( | ||
InvalidParamMessage("'$CLIENT_PARAMETER:$clientName': unknown schema '${sender.schemaName}'") | ||
InvalidParamMessage("Expected a '$CLIENT_PARAMETER' or '$SCHEMA_PARAMETER' query parameter") | ||
) | ||
} | ||
schema = Metadata.getInstance().findSchema(schemaName) | ||
if (schema == null) { | ||
actionLogs.error(InvalidParamMessage("Failed to find schema with name '$schemaName'")) | ||
} | ||
} else { | ||
// Find schema via CLIENT_PARAMETER parameter | ||
sender = workflowEngine.settings.findSender(clientName) | ||
if (sender == null) { | ||
actionLogs.error(InvalidParamMessage("'$CLIENT_PARAMETER:$clientName': unknown sender")) | ||
} | ||
|
||
// verify schema if the sender is a topic sender | ||
if (sender != null && sender is HasSchema) { | ||
schema = workflowEngine.metadata.findSchema(sender.schemaName) | ||
if (schema == null) { | ||
actionLogs.error( | ||
InvalidParamMessage("'$CLIENT_PARAMETER:$clientName': unknown schema '${sender.schemaName}'") | ||
) | ||
} | ||
} | ||
} | ||
|
||
// validate content type | ||
val contentType = request.headers.getOrDefault(HttpHeaders.CONTENT_TYPE.lowercase(), "") | ||
if (contentType.isBlank()) { | ||
actionLogs.error(InvalidParamMessage("Missing ${HttpHeaders.CONTENT_TYPE} header")) | ||
|
@@ -111,7 +127,7 @@ abstract class RequestFunction( | |
} | ||
} | ||
|
||
if (sender == null || content.isEmpty() || actionLogs.hasErrors()) { | ||
if (content.isEmpty() || actionLogs.hasErrors()) { | ||
throw actionLogs.exception | ||
} | ||
|
||
|
@@ -125,7 +141,7 @@ abstract class RequestFunction( | |
} | ||
|
||
// only non full ELR senders will have a schema | ||
if (sender is HasSchema && schema != null) { | ||
if (schema != null) { | ||
val element = schema.findElement(parts[0]) | ||
if (element == null) { | ||
actionLogs.error(InvalidParamMessage("'${parts[0]}' is not a valid element name")) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,8 +12,10 @@ import com.microsoft.azure.functions.annotation.StorageAccount | |
import gov.cdc.prime.router.ActionError | ||
import gov.cdc.prime.router.ActionLog | ||
import gov.cdc.prime.router.ActionLogLevel | ||
import gov.cdc.prime.router.CustomerStatus | ||
import gov.cdc.prime.router.InvalidReportMessage | ||
import gov.cdc.prime.router.Sender | ||
import gov.cdc.prime.router.TopicSender | ||
import gov.cdc.prime.router.Translator | ||
import gov.cdc.prime.router.ValidationReceiver | ||
import gov.cdc.prime.router.azure.db.enums.TaskAction | ||
|
@@ -23,9 +25,6 @@ import gov.cdc.prime.router.history.DetailedActionLog | |
import gov.cdc.prime.router.history.DetailedReport | ||
import gov.cdc.prime.router.history.DetailedSubmissionHistory | ||
import gov.cdc.prime.router.history.ReportStreamFilterResultForResponse | ||
import gov.cdc.prime.router.tokens.AuthenticatedClaims | ||
import gov.cdc.prime.router.tokens.authenticationFailure | ||
import gov.cdc.prime.router.tokens.authorizationFailure | ||
import org.apache.logging.log4j.kotlin.Logging | ||
import java.time.OffsetDateTime | ||
|
||
|
@@ -44,30 +43,38 @@ class ValidateFunction( | |
*/ | ||
@FunctionName("validate") | ||
@StorageAccount("AzureWebJobsStorage") | ||
fun run( | ||
fun validate( | ||
@HttpTrigger( | ||
name = "validate", | ||
methods = [HttpMethod.POST], | ||
authLevel = AuthorizationLevel.ANONYMOUS | ||
) request: HttpRequestMessage<String?> | ||
): HttpResponseMessage { | ||
val senderName = extractClient(request) | ||
if (senderName.isBlank()) { | ||
return HttpUtilities.bad(request, "Expected a '$CLIENT_PARAMETER' query parameter") | ||
} | ||
return try { | ||
val claims = AuthenticatedClaims.authenticate(request) | ||
?: return HttpUtilities.unauthorizedResponse(request, authenticationFailure) | ||
|
||
// Sender should eventually be obtained directly from who is authenticated | ||
val sender = workflowEngine.settings.findSender(senderName) | ||
?: return HttpUtilities.bad(request, "'$CLIENT_PARAMETER:$senderName': unknown sender") | ||
|
||
if (!claims.authorizedForSendOrReceive(sender, request)) { | ||
return HttpUtilities.unauthorizedResponse(request, authorizationFailure) | ||
val sender: Sender? | ||
val senderName = extractClient(request) | ||
arnejduranovic marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if (senderName.isNotBlank()) { | ||
sender = workflowEngine.settings.findSender(senderName) | ||
?: return HttpUtilities.bad(request, "'$CLIENT_PARAMETER:$senderName': unknown sender") | ||
} else { | ||
val schemaName = request.queryParameters.getOrDefault(SCHEMA_PARAMETER, null) | ||
val format = request.queryParameters.getOrDefault(FORMAT_PARAMETER, null) | ||
if (schemaName != null && format != null) { | ||
sender = getDummySender(schemaName, format) | ||
if (sender == null) { | ||
return HttpUtilities.bad( | ||
request, "Could not find schema named '$schemaName" | ||
) | ||
} | ||
} else { | ||
return HttpUtilities.bad( | ||
request, | ||
"No client found in header so expected " + | ||
"'$SCHEMA_PARAMETER' and '$FORMAT_PARAMETER' query parameters" | ||
) | ||
} | ||
} | ||
actionHistory.trackActionParams(request) | ||
|
||
processRequest(request, sender) | ||
} catch (ex: Exception) { | ||
if (ex.message != null) { | ||
|
@@ -80,7 +87,7 @@ class ValidateFunction( | |
} | ||
|
||
/** | ||
* Handles an incoming validation request after it has been authenticated via the /validate endpoint. | ||
* Handles an incoming validation request from the /validate endpoint. | ||
* @param request The incoming request | ||
* @param sender The sender record, pulled from the database based on sender name on the request | ||
* @return Returns an HttpResponseMessage indicating the result of the operation and any resulting information | ||
|
@@ -96,9 +103,8 @@ class ValidateFunction( | |
try { | ||
val validatedRequest = validateRequest(request) | ||
|
||
// if the override parameter is populated, use that, otherwise use the sender value | ||
val allowDuplicates = if | ||
(!allowDuplicatesParam.isNullOrEmpty()) allowDuplicatesParam == "true" | ||
// if the override parameter is populated, use that, otherwise use the sender value. Default to false. | ||
val allowDuplicates = if (!allowDuplicatesParam.isNullOrEmpty()) allowDuplicatesParam == "true" | ||
else { | ||
sender.allowDuplicates | ||
} | ||
|
@@ -111,7 +117,6 @@ class ValidateFunction( | |
validatedRequest.routeTo, | ||
allowDuplicates, | ||
) | ||
|
||
// return OK status, report validation was successful | ||
HttpStatus.OK | ||
} catch (e: ActionError) { | ||
|
@@ -178,4 +183,28 @@ class ValidateFunction( | |
) | ||
.build() | ||
} | ||
|
||
/** | ||
* Return [TopicSender] for a given schema if that schema exists. This lets us wrap the data needed by | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🙏 |
||
* processRequest without making changes to the method | ||
* @param schemaName the name or path of the schema | ||
* @param format the message format that the schema supports | ||
* @return TopicSender if schema exists, null otherwise | ||
*/ | ||
internal fun getDummySender(schemaName: String, format: String): TopicSender? { | ||
val schema = workflowEngine.metadata.findSchema(schemaName) | ||
return if (schema != null) { | ||
TopicSender( | ||
"ValidationSender", | ||
"Internal", | ||
Sender.Format.valueOf(format), | ||
CustomerStatus.TESTING, | ||
schemaName, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there any possible issue of validating a custom schema error messages through this? Like if someone makes a call to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Correct, we concluded that this would be OK. We are also OK with a user calling validateWithClient and "guessing" clients. There is no personal info at risk. |
||
schema.topic | ||
) | ||
} else { | ||
// error schema not found | ||
null | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This new code is not covered by unit tests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This abstract class, and specifically this method, is exercised via the tests that test its subclasses: ReportFunctionTests and ValidateFunctionTests. The new (and old) ValidateFunctionTests in particular exercise the code highlighted here, through their calls to
ValidateFunction.processRequest
. We currently do not have any direct unit tests for the abstract class RequestFunction, but if you would like me to test that method directly in this ticket I can. I just didn't see the need to.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, I just simplified this code, moving almost all schema/format logic to
getDummySender
to reduce redundancy with the logic in ValidateFunction. So, I added tests forgetDummySender
since its more complicated now.