From 58bd9bdfbbcb27b67ebc3708f73d29aa7800af27 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=F0=9F=A5=B7=E2=9A=94=EF=B8=8F=F0=9F=92=80=F0=9F=91=BB?= Date: Tue, 21 May 2024 21:15:15 -0700 Subject: [PATCH] fix(cors): applies the passthroughBehavior and mime-type sync behind a new flag The new Open API setting is `syncCorsPreflightIntegration`. Added the docs as well. closes smithy-lang/smithy#2289 --- .../converting-to-openapi.rst | 23 ++ .../openapi/AddCorsPreflightIntegration.java | 42 ++-- .../aws/apigateway/openapi/CorsTest.java | 23 +- ...=> cors-with-multi-request-templates.json} | 0 ... => cors-with-preflight-sync.openapi.json} | 2 +- .../cors-without-preflight-sync.openapi.json | 219 ++++++++++++++++++ .../amazon/smithy/openapi/OpenApiConfig.java | 16 ++ 7 files changed, 302 insertions(+), 23 deletions(-) rename smithy-aws-apigateway-openapi/src/test/resources/software/amazon/smithy/aws/apigateway/openapi/{cors-with-multi-mime-types.json => cors-with-multi-request-templates.json} (100%) rename smithy-aws-apigateway-openapi/src/test/resources/software/amazon/smithy/aws/apigateway/openapi/{cors-with-multi-mime-types.openapi.json => cors-with-preflight-sync.openapi.json} (99%) create mode 100644 smithy-aws-apigateway-openapi/src/test/resources/software/amazon/smithy/aws/apigateway/openapi/cors-without-preflight-sync.openapi.json diff --git a/docs/source-2.0/guides/model-translations/converting-to-openapi.rst b/docs/source-2.0/guides/model-translations/converting-to-openapi.rst index 91746b394..7ff457f8d 100644 --- a/docs/source-2.0/guides/model-translations/converting-to-openapi.rst +++ b/docs/source-2.0/guides/model-translations/converting-to-openapi.rst @@ -691,6 +691,29 @@ onErrorStatusConflict (``String``) } } +.. _generate-openapi-setting-syncCorsPreflightIntegration: + +syncCorsPreflightIntegration (``boolean``) + Set to true to sync CORS preflight integration request templates with all combinations + of content-types from other methods within the same path resource. + + .. code-block:: json + :caption: smithy-build.json + + { + "version": "1.0", + "plugins": { + "openapi": { + "service": "example.weather#Weather", + "syncCorsPreflightIntegration": true + } + } + } + + With this enabled, the ``passthroughBehavior`` for the CORS preflight integration + will be set to "never". + + ---------------------------------- JSON schema configuration settings ---------------------------------- diff --git a/smithy-aws-apigateway-openapi/src/main/java/software/amazon/smithy/aws/apigateway/openapi/AddCorsPreflightIntegration.java b/smithy-aws-apigateway-openapi/src/main/java/software/amazon/smithy/aws/apigateway/openapi/AddCorsPreflightIntegration.java index b2732fd21..d66795d2d 100644 --- a/smithy-aws-apigateway-openapi/src/main/java/software/amazon/smithy/aws/apigateway/openapi/AddCorsPreflightIntegration.java +++ b/smithy-aws-apigateway-openapi/src/main/java/software/amazon/smithy/aws/apigateway/openapi/AddCorsPreflightIntegration.java @@ -98,7 +98,7 @@ final class AddCorsPreflightIntegration implements ApiGatewayMapper { LOGGER.fine(() -> "Adding CORS-preflight OPTIONS request and API Gateway integration for " + path); Map headers = deduceCorsHeaders(context, path, pathItem, corsTrait); return pathItem.toBuilder() - .options(createPreflightOperation(path, pathItem, headers)) + .options(createPreflightOperation(context, path, pathItem, headers)) .build(); } @@ -173,7 +173,7 @@ final class AddCorsPreflightIntegration implements ApiGatewayMapper { } private static OperationObject createPreflightOperation( - String path, PathItem pathItem, Map headers) { + Context context, String path, PathItem pathItem, Map headers) { return OperationObject.builder() .tags(ListUtils.of("CORS")) .security(Collections.emptyList()) @@ -181,7 +181,7 @@ final class AddCorsPreflightIntegration implements ApiGatewayMapper { .operationId(createOperationId(path)) .putResponse("200", createPreflightResponse(headers)) .parameters(findPathParameters(pathItem)) - .putExtension(INTEGRATION_EXTENSION, createPreflightIntegration(headers, pathItem)) + .putExtension(INTEGRATION_EXTENSION, createPreflightIntegration(context, headers, pathItem)) .build(); } @@ -217,7 +217,8 @@ final class AddCorsPreflightIntegration implements ApiGatewayMapper { return builder.build(); } - private static ObjectNode createPreflightIntegration(Map headers, PathItem pathItem) { + private static ObjectNode createPreflightIntegration( + Context context, Map headers, PathItem pathItem) { IntegrationResponse.Builder responseBuilder = IntegrationResponse.builder().statusCode("200"); // Add each CORS header to the mock integration response. @@ -225,27 +226,30 @@ final class AddCorsPreflightIntegration implements ApiGatewayMapper { responseBuilder.putResponseParameter("method.response.header." + e.getKey(), "'" + e.getValue() + "'"); } + boolean isPreflightSynced = Boolean.TRUE.equals(context.getConfig().getSyncCorsPreflightIntegration()); MockIntegrationTrait.Builder integration = MockIntegrationTrait.builder() // See https://forums.aws.amazon.com/thread.jspa?threadID=256140 .contentHandling("CONVERT_TO_TEXT") - .passThroughBehavior("when_no_match") + .passThroughBehavior(isPreflightSynced ? "never" : "when_no_match") .putResponse("default", responseBuilder.build()) .putRequestTemplate(API_GATEWAY_DEFAULT_ACCEPT_VALUE, PREFLIGHT_SUCCESS); - // Adds request template for every unique Content-Type supported by all path operations. - // This ensures that for Content-Type(s) other than 'application/json', the entire request payload - // is not sent to APIGW mock integration as stipulated by 'when_no_match' passthroughBehavior. - // APIGW throws an error if the mock integration request does not follow a set contract, - // example {"statusCode":200}. - for (OperationObject operation : pathItem.getOperations().values()) { - ObjectNode extensionNode = operation.getExtension(INTEGRATION_EXTENSION) - .flatMap(Node::asObjectNode) - .orElse(ObjectNode.EMPTY); - Set mimeTypes = extensionNode.getObjectMember(REQUEST_TEMPLATES_KEY) - .map(ObjectNode::getStringMap) - .map(Map::keySet) - .orElse(SetUtils.of()); - mimeTypes.forEach(mimeType -> integration.putRequestTemplate(mimeType, PREFLIGHT_SUCCESS)); + if (isPreflightSynced) { + // Adds request template for every unique Content-Type supported by all path operations. + // This ensures that for Content-Type(s) other than 'application/json', the entire request payload + // is not sent to APIGW mock integration as stipulated by 'when_no_match' passthroughBehavior. + // APIGW throws an error if the mock integration request does not follow a set contract, + // example {"statusCode":200}. + for (OperationObject operation : pathItem.getOperations().values()) { + ObjectNode extensionNode = operation.getExtension(INTEGRATION_EXTENSION) + .flatMap(Node::asObjectNode) + .orElse(ObjectNode.EMPTY); + Set mimeTypes = extensionNode.getObjectMember(REQUEST_TEMPLATES_KEY) + .map(ObjectNode::getStringMap) + .map(Map::keySet) + .orElse(SetUtils.of()); + mimeTypes.forEach(mimeType -> integration.putRequestTemplate(mimeType, PREFLIGHT_SUCCESS)); + } } // Add a request template for every mime-type of every response. diff --git a/smithy-aws-apigateway-openapi/src/test/java/software/amazon/smithy/aws/apigateway/openapi/CorsTest.java b/smithy-aws-apigateway-openapi/src/test/java/software/amazon/smithy/aws/apigateway/openapi/CorsTest.java index 84d24199c..1cd507ecd 100644 --- a/smithy-aws-apigateway-openapi/src/test/java/software/amazon/smithy/aws/apigateway/openapi/CorsTest.java +++ b/smithy-aws-apigateway-openapi/src/test/java/software/amazon/smithy/aws/apigateway/openapi/CorsTest.java @@ -114,17 +114,34 @@ public class CorsTest { } @Test - public void mapMultipleMimeTypesInRequestTemplates() { + public void withPreflightIntegrationSync() { Model model = Model.assembler(getClass().getClassLoader()) .discoverModels(getClass().getClassLoader()) - .addImport(getClass().getResource("cors-with-multi-mime-types.json")) + .addImport(getClass().getResource("cors-with-multi-request-templates.json")) + .assemble() + .unwrap(); + OpenApiConfig config = new OpenApiConfig(); + config.setService(ShapeId.from("example.smithy#MyService")); + config.setSyncCorsPreflightIntegration(true); + ObjectNode result = OpenApiConverter.create().config(config).convertToNode(model); + Node expectedNode = Node.parse(IoUtils.toUtf8String( + getClass().getResourceAsStream("cors-with-preflight-sync.openapi.json"))); + + Node.assertEquals(result, expectedNode); + } + + @Test + public void withoutPreflightIntegrationSync() { + Model model = Model.assembler(getClass().getClassLoader()) + .discoverModels(getClass().getClassLoader()) + .addImport(getClass().getResource("cors-with-multi-request-templates.json")) .assemble() .unwrap(); OpenApiConfig config = new OpenApiConfig(); config.setService(ShapeId.from("example.smithy#MyService")); ObjectNode result = OpenApiConverter.create().config(config).convertToNode(model); Node expectedNode = Node.parse(IoUtils.toUtf8String( - getClass().getResourceAsStream("cors-with-multi-mime-types.openapi.json"))); + getClass().getResourceAsStream("cors-without-preflight-sync.openapi.json"))); Node.assertEquals(result, expectedNode); } diff --git a/smithy-aws-apigateway-openapi/src/test/resources/software/amazon/smithy/aws/apigateway/openapi/cors-with-multi-mime-types.json b/smithy-aws-apigateway-openapi/src/test/resources/software/amazon/smithy/aws/apigateway/openapi/cors-with-multi-request-templates.json similarity index 100% rename from smithy-aws-apigateway-openapi/src/test/resources/software/amazon/smithy/aws/apigateway/openapi/cors-with-multi-mime-types.json rename to smithy-aws-apigateway-openapi/src/test/resources/software/amazon/smithy/aws/apigateway/openapi/cors-with-multi-request-templates.json diff --git a/smithy-aws-apigateway-openapi/src/test/resources/software/amazon/smithy/aws/apigateway/openapi/cors-with-multi-mime-types.openapi.json b/smithy-aws-apigateway-openapi/src/test/resources/software/amazon/smithy/aws/apigateway/openapi/cors-with-preflight-sync.openapi.json similarity index 99% rename from smithy-aws-apigateway-openapi/src/test/resources/software/amazon/smithy/aws/apigateway/openapi/cors-with-multi-mime-types.openapi.json rename to smithy-aws-apigateway-openapi/src/test/resources/software/amazon/smithy/aws/apigateway/openapi/cors-with-preflight-sync.openapi.json index d9acb0774..5f3b8154b 100644 --- a/smithy-aws-apigateway-openapi/src/test/resources/software/amazon/smithy/aws/apigateway/openapi/cors-with-multi-mime-types.openapi.json +++ b/smithy-aws-apigateway-openapi/src/test/resources/software/amazon/smithy/aws/apigateway/openapi/cors-with-preflight-sync.openapi.json @@ -107,7 +107,7 @@ } }, "type": "mock", - "passthroughBehavior": "when_no_match" + "passthroughBehavior": "never" } }, "put": { diff --git a/smithy-aws-apigateway-openapi/src/test/resources/software/amazon/smithy/aws/apigateway/openapi/cors-without-preflight-sync.openapi.json b/smithy-aws-apigateway-openapi/src/test/resources/software/amazon/smithy/aws/apigateway/openapi/cors-without-preflight-sync.openapi.json new file mode 100644 index 000000000..2ce6e10fa --- /dev/null +++ b/smithy-aws-apigateway-openapi/src/test/resources/software/amazon/smithy/aws/apigateway/openapi/cors-without-preflight-sync.openapi.json @@ -0,0 +1,219 @@ +{ + "openapi": "3.0.2", + "info": { + "title": "MyService", + "version": "2006-03-01" + }, + "paths": { + "/mock": { + "get": { + "operationId": "MockGet", + "responses": { + "200": { + "description": "MockGet 200 response", + "headers": { + "Access-Control-Allow-Origin": { + "schema": { + "type": "string" + } + }, + "Access-Control-Expose-Headers": { + "schema": { + "type": "string" + } + } + }, + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/MockGetResponseContent" + } + } + } + } + }, + "x-amazon-apigateway-integration": { + "requestTemplates": { + "application/json": "{\"statusCode\": 200}", + "application/x-www-form-urlencoded": "{\"statusCode\": 200}" + }, + "responses": { + "default": { + "statusCode": "200", + "responseTemplates": { + "application/json": "{\"extendedRequestId\": \"$context.extendedRequestId\"}" + }, + "responseParameters": { + "method.response.header.Access-Control-Allow-Origin": "'https://www.example.com'", + "method.response.header.Access-Control-Expose-Headers": "'Content-Length,Content-Type,X-Amzn-Errortype,X-Amzn-Requestid,X-Service-Output-Metadata'" + } + } + }, + "type": "mock", + "passthroughBehavior": "never" + } + }, + "options": { + "description": "Handles CORS-preflight requests", + "operationId": "CorsMock", + "responses": { + "200": { + "description": "Canned response for CORS-preflight requests", + "headers": { + "Access-Control-Allow-Headers": { + "schema": { + "type": "string" + } + }, + "Access-Control-Allow-Methods": { + "schema": { + "type": "string" + } + }, + "Access-Control-Allow-Origin": { + "schema": { + "type": "string" + } + }, + "Access-Control-Max-Age": { + "schema": { + "type": "string" + } + } + } + } + }, + "security": [], + "tags": [ + "CORS" + ], + "x-amazon-apigateway-integration": { + "contentHandling": "CONVERT_TO_TEXT", + "requestTemplates": { + "application/json": "{\"statusCode\":200}" + }, + "responses": { + "default": { + "responseParameters": { + "method.response.header.Access-Control-Max-Age": "'86400'", + "method.response.header.Access-Control-Allow-Headers": "'Amz-Sdk-Invocation-Id,Amz-Sdk-Request,Authorization,Date,Host,X-Amz-Content-Sha256,X-Amz-Date,X-Amz-Security-Token,X-Amz-Target,X-Amz-User-Agent,X-Amzn-Trace-Id,X-Service-Input-Metadata'", + "method.response.header.Access-Control-Allow-Origin": "'https://www.example.com'", + "method.response.header.Access-Control-Allow-Methods": "'GET,PUT'" + }, + "statusCode": "200" + } + }, + "type": "mock", + "passthroughBehavior": "when_no_match" + } + }, + "put": { + "operationId": "MockPut", + "responses": { + "201": { + "description": "MockPut 201 response", + "headers": { + "Access-Control-Allow-Origin": { + "schema": { + "type": "string" + } + }, + "Access-Control-Expose-Headers": { + "schema": { + "type": "string" + } + } + }, + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/MockPutResponseContent" + } + } + } + } + }, + "x-amazon-apigateway-integration": { + "requestTemplates": { + "text/plain": "{\"statusCode\": 200}", + "application/xml": "{\"statusCode\": 200}" + }, + "responses": { + "default": { + "statusCode": "200", + "responseTemplates": { + "application/json": "{\"extendedRequestId\": \"$context.extendedRequestId\"}" + }, + "responseParameters": { + "method.response.header.Access-Control-Allow-Origin": "'https://www.example.com'", + "method.response.header.Access-Control-Expose-Headers": "'Content-Length,Content-Type,X-Amzn-Errortype,X-Amzn-Requestid,X-Service-Output-Metadata'" + } + } + }, + "type": "mock", + "passthroughBehavior": "never" + } + } + } + }, + "components": { + "schemas": { + "MockGetResponseContent": { + "type": "object", + "properties": { + "extendedRequestId": { + "type": "string" + } + }, + "required": [ + "extendedRequestId" + ] + }, + "MockPutResponseContent": { + "type": "object", + "properties": { + "extendedRequestId": { + "type": "string" + } + }, + "required": [ + "extendedRequestId" + ] + } + }, + "securitySchemes": { + "aws.auth.sigv4": { + "type": "apiKey", + "description": "AWS Signature Version 4 authentication", + "name": "Authorization", + "in": "header", + "x-amazon-apigateway-authtype": "awsSigv4" + } + } + }, + "security": [ + { + "aws.auth.sigv4": [ ] + } + ], + "x-amazon-apigateway-gateway-responses": { + "DEFAULT_4XX": { + "responseTemplates": { + "application/json": "{\"message\":$context.error.messageString}" + }, + "responseParameters": { + "gatewayresponse.header.Access-Control-Allow-Origin": "'https://www.example.com'", + "gatewayresponse.header.Access-Control-Expose-Headers": "'X-Service-Output-Metadata'" + } + }, + "DEFAULT_5XX": { + "responseTemplates": { + "application/json": "{\"message\":$context.error.messageString}" + }, + "responseParameters": { + "gatewayresponse.header.Access-Control-Allow-Origin": "'https://www.example.com'", + "gatewayresponse.header.Access-Control-Expose-Headers": "'X-Service-Output-Metadata'" + } + } + } +} diff --git a/smithy-openapi/src/main/java/software/amazon/smithy/openapi/OpenApiConfig.java b/smithy-openapi/src/main/java/software/amazon/smithy/openapi/OpenApiConfig.java index cf0db1894..90b83e14d 100644 --- a/smithy-openapi/src/main/java/software/amazon/smithy/openapi/OpenApiConfig.java +++ b/smithy-openapi/src/main/java/software/amazon/smithy/openapi/OpenApiConfig.java @@ -101,6 +101,7 @@ public class OpenApiConfig extends JsonSchemaConfig { private List externalDocs = ListUtils.of( "Homepage", "API Reference", "User Guide", "Developer Guide", "Reference", "Guide"); private boolean disableIntegerFormat = false; + private boolean syncCorsPreflightIntegration = false; private ErrorStatusConflictHandlingStrategy onErrorStatusConflict; private OpenApiVersion version = OpenApiVersion.VERSION_3_0_2; @@ -355,6 +356,21 @@ public class OpenApiConfig extends JsonSchemaConfig { } + public boolean getSyncCorsPreflightIntegration() { + return this.syncCorsPreflightIntegration; + } + + /** + * Set true to sync CORS preflight integration request templates with the other + * methods of the same path resource and set passthroughBehavior to "never". + * + * @param syncCorsPreflightIntegration True to match CORS preflight integration. + */ + public void setSyncCorsPreflightIntegration(boolean syncCorsPreflightIntegration) { + this.syncCorsPreflightIntegration = syncCorsPreflightIntegration; + } + + public ErrorStatusConflictHandlingStrategy getOnErrorStatusConflict() { return onErrorStatusConflict; }