From 304600306c973fe51ed1555c8e42994d58204bf9 Mon Sep 17 00:00:00 2001 From: Michael Dowling Date: Mon, 7 Dec 2020 13:36:31 -0800 Subject: [PATCH] Fix Access-Control-Expose-Headers This commit ensures that Access-Control-Expose-Headers is correctly populated based on all of the headers used in an operation, including any headers that might be added manually during the OpenAPI conversion independent of a Smithy shape. I added a new ApiGatewayMapper method called postProcessOperation to allow transformations to better control when they are applied, in particular, transforms sometimes need to occur on an operation after the things that the operation contains have been transformed. This was not previously possible even with the ordering semantics that already exist. This change was necessary in order to ensure that the correct CORs headers are added to operations based on all of the transforms applied to the operation, and it makes it easier to separate CORS headers from modeled headers when constructing Access-Control-Expose-Headers. --- .../openapi/AddCorsResponseHeaders.java | 44 +++++++++++++----- .../apigateway/openapi/AddIntegrations.java | 10 +++-- .../apigateway/openapi/ApiGatewayMapper.java | 12 +++++ .../aws/apigateway/openapi/CorsHeader.java | 12 ++++- .../cors-explicit-options.openapi.json | 3 +- .../openapi/cors-model.openapi.json | 2 +- .../cors-with-additional-headers.openapi.json | 2 +- ...stom-gateway-response-headers.openapi.json | 3 +- .../openapi/fromsmithy/OpenApiConverter.java | 11 +++-- .../openapi/fromsmithy/OpenApiMapper.java | 45 ++++++++++++++++++- 10 files changed, 118 insertions(+), 26 deletions(-) diff --git a/smithy-aws-apigateway-openapi/src/main/java/software/amazon/smithy/aws/apigateway/openapi/AddCorsResponseHeaders.java b/smithy-aws-apigateway-openapi/src/main/java/software/amazon/smithy/aws/apigateway/openapi/AddCorsResponseHeaders.java index 74aede9c2b7..cdbdcde755c 100644 --- a/smithy-aws-apigateway-openapi/src/main/java/software/amazon/smithy/aws/apigateway/openapi/AddCorsResponseHeaders.java +++ b/smithy-aws-apigateway-openapi/src/main/java/software/amazon/smithy/aws/apigateway/openapi/AddCorsResponseHeaders.java @@ -1,5 +1,5 @@ /* - * Copyright 2019 Amazon.com, Inc. or its affiliates. All Rights Reserved. + * Copyright 2020 Amazon.com, Inc. or its affiliates. All Rights Reserved. * * Licensed under the Apache License, Version 2.0 (the "License"). * You may not use this file except in compliance with the License. @@ -17,12 +17,14 @@ import java.util.ArrayList; import java.util.List; +import java.util.Map; import java.util.logging.Logger; import software.amazon.smithy.jsonschema.Schema; import software.amazon.smithy.model.shapes.OperationShape; import software.amazon.smithy.model.traits.CorsTrait; import software.amazon.smithy.model.traits.Trait; import software.amazon.smithy.openapi.fromsmithy.Context; +import software.amazon.smithy.openapi.model.OperationObject; import software.amazon.smithy.openapi.model.ParameterObject; import software.amazon.smithy.openapi.model.Ref; import software.amazon.smithy.openapi.model.ResponseObject; @@ -48,25 +50,43 @@ public List getApiTypes() { } @Override - public ResponseObject updateResponse( + public OperationObject postProcessOperation( Context context, OperationShape shape, - String status, + OperationObject operation, String method, - String path, - ResponseObject response + String path ) { return context.getService().getTrait(CorsTrait.class) - .map(corsTrait -> addCorsHeadersToResponse(context, shape, response, corsTrait, method)) - .orElse(response); + .map(trait -> addCorsHeadersToResponses(context, shape, operation, method, trait)) + .orElse(operation); } - private ResponseObject addCorsHeadersToResponse( + private OperationObject addCorsHeadersToResponses( + Context context, + OperationShape shape, + OperationObject operationObject, + String method, + CorsTrait trait + ) { + OperationObject.Builder builder = operationObject.toBuilder(); + + for (Map.Entry entry : operationObject.getResponses().entrySet()) { + ResponseObject updated = createUpdatedResponseWithCorsHeaders( + context, shape, operationObject, method, trait, entry.getValue()); + builder.putResponse(entry.getKey(), updated); + } + + return builder.build(); + } + + private ResponseObject createUpdatedResponseWithCorsHeaders( Context context, OperationShape operation, - ResponseObject response, - CorsTrait corsTrait, - String method + OperationObject operationObject, + String method, + CorsTrait trait, + ResponseObject response ) { // Determine which headers have been added to the response. List headers = new ArrayList<>(); @@ -108,7 +128,7 @@ private ResponseObject addCorsHeadersToResponse( // An HTTP response to a CORS request that is *not* a // CORS-preflight request can also include the following header: // `Access-Control-Expose-Headers` - if (!CorsHeader.deduceOperationHeaders(context, operation, corsTrait).isEmpty()) { + if (!CorsHeader.deduceOperationResponseHeaders(context, operationObject, operation, trait).isEmpty()) { headers.add(CorsHeader.EXPOSE_HEADERS.toString()); } diff --git a/smithy-aws-apigateway-openapi/src/main/java/software/amazon/smithy/aws/apigateway/openapi/AddIntegrations.java b/smithy-aws-apigateway-openapi/src/main/java/software/amazon/smithy/aws/apigateway/openapi/AddIntegrations.java index 859b1300457..37f680091d0 100644 --- a/smithy-aws-apigateway-openapi/src/main/java/software/amazon/smithy/aws/apigateway/openapi/AddIntegrations.java +++ b/smithy-aws-apigateway-openapi/src/main/java/software/amazon/smithy/aws/apigateway/openapi/AddIntegrations.java @@ -1,5 +1,5 @@ /* - * Copyright 2019 Amazon.com, Inc. or its affiliates. All Rights Reserved. + * Copyright 2020 Amazon.com, Inc. or its affiliates. All Rights Reserved. * * Licensed under the Apache License, Version 2.0 (the "License"). * You may not use this file except in compliance with the License. @@ -74,7 +74,7 @@ public OperationObject updateOperation( IntegrationTraitIndex index = IntegrationTraitIndex.of(context.getModel()); return index.getIntegrationTrait(context.getService(), shape) .map(trait -> operation.toBuilder() - .putExtension(EXTENSION_NAME, createIntegration(context, shape, trait)) + .putExtension(EXTENSION_NAME, createIntegration(context, operation, shape, trait)) .build()) .orElseGet(() -> { LOGGER.warning("No API Gateway integration trait found for " + shape.getId()); @@ -84,6 +84,7 @@ public OperationObject updateOperation( private ObjectNode createIntegration( Context context, + OperationObject operationObject, OperationShape shape, Trait integration ) { @@ -91,7 +92,7 @@ private ObjectNode createIntegration( return context.getService().getTrait(CorsTrait.class) .map(cors -> { LOGGER.fine(() -> String.format("Adding CORS to `%s` operation responses", shape.getId())); - return updateIntegrationWithCors(context, shape, integrationObject, cors); + return updateIntegrationWithCors(context, operationObject, shape, integrationObject, cors); }) .orElse(integrationObject); } @@ -122,6 +123,7 @@ private static ObjectNode getIntegrationAsObject( private ObjectNode updateIntegrationWithCors( Context context, + OperationObject operationObject, OperationShape shape, ObjectNode integrationNode, CorsTrait cors @@ -141,7 +143,7 @@ private ObjectNode updateIntegrationWithCors( LOGGER.finer(() -> String.format("Adding the following CORS headers to the API Gateway integration of %s: %s", shape.getId(), corsHeaders)); - Set deducedHeaders = CorsHeader.deduceOperationHeaders(context, shape, cors); + Set deducedHeaders = CorsHeader.deduceOperationResponseHeaders(context, operationObject, shape, cors); LOGGER.fine(() -> String.format("Detected the following headers for operation %s: %s", shape.getId(), deducedHeaders)); diff --git a/smithy-aws-apigateway-openapi/src/main/java/software/amazon/smithy/aws/apigateway/openapi/ApiGatewayMapper.java b/smithy-aws-apigateway-openapi/src/main/java/software/amazon/smithy/aws/apigateway/openapi/ApiGatewayMapper.java index c761f032951..57641b71c6c 100644 --- a/smithy-aws-apigateway-openapi/src/main/java/software/amazon/smithy/aws/apigateway/openapi/ApiGatewayMapper.java +++ b/smithy-aws-apigateway-openapi/src/main/java/software/amazon/smithy/aws/apigateway/openapi/ApiGatewayMapper.java @@ -112,6 +112,18 @@ public OperationObject updateOperation( : operation; } + @Override + public OperationObject postProcessOperation( + Context context, + OperationShape shape, + OperationObject operation, + String httpMethodName, String path + ) { + return matchesApiType(context) + ? delegate.postProcessOperation(context, shape, operation, httpMethodName, path) + : operation; + } + @Override public PathItem updatePathItem(Context context, String path, PathItem pathItem) { return matchesApiType(context) diff --git a/smithy-aws-apigateway-openapi/src/main/java/software/amazon/smithy/aws/apigateway/openapi/CorsHeader.java b/smithy-aws-apigateway-openapi/src/main/java/software/amazon/smithy/aws/apigateway/openapi/CorsHeader.java index c1dc21b90ba..b87de285ca9 100644 --- a/smithy-aws-apigateway-openapi/src/main/java/software/amazon/smithy/aws/apigateway/openapi/CorsHeader.java +++ b/smithy-aws-apigateway-openapi/src/main/java/software/amazon/smithy/aws/apigateway/openapi/CorsHeader.java @@ -1,5 +1,5 @@ /* - * Copyright 2019 Amazon.com, Inc. or its affiliates. All Rights Reserved. + * Copyright 2020 Amazon.com, Inc. or its affiliates. All Rights Reserved. * * Licensed under the Apache License, Version 2.0 (the "License"). * You may not use this file except in compliance with the License. @@ -22,6 +22,8 @@ import software.amazon.smithy.model.traits.Trait; import software.amazon.smithy.openapi.fromsmithy.Context; import software.amazon.smithy.openapi.fromsmithy.SecuritySchemeConverter; +import software.amazon.smithy.openapi.model.OperationObject; +import software.amazon.smithy.openapi.model.ResponseObject; enum CorsHeader { @@ -43,8 +45,9 @@ public String toString() { return headerName; } - static Set deduceOperationHeaders( + static Set deduceOperationResponseHeaders( Context context, + OperationObject operationObject, OperationShape shape, CorsTrait cors ) { @@ -58,6 +61,11 @@ static Set deduceOperationHeaders( result.addAll(getSecuritySchemeResponseHeaders(context, converter)); } + // Include all headers found in the generated OpenAPI response. + for (ResponseObject responseObject : operationObject.getResponses().values()) { + result.addAll(responseObject.getHeaders().keySet()); + } + return result; } diff --git a/smithy-aws-apigateway-openapi/src/test/resources/software/amazon/smithy/aws/apigateway/openapi/cors-explicit-options.openapi.json b/smithy-aws-apigateway-openapi/src/test/resources/software/amazon/smithy/aws/apigateway/openapi/cors-explicit-options.openapi.json index 591d59c1e73..1d42e983e78 100644 --- a/smithy-aws-apigateway-openapi/src/test/resources/software/amazon/smithy/aws/apigateway/openapi/cors-explicit-options.openapi.json +++ b/smithy-aws-apigateway-openapi/src/test/resources/software/amazon/smithy/aws/apigateway/openapi/cors-explicit-options.openapi.json @@ -63,7 +63,8 @@ "default": { "statusCode": "200", "responseParameters": { - "method.response.header.Access-Control-Allow-Origin": "'https://foo.com'" + "method.response.header.Access-Control-Allow-Origin": "'https://foo.com'", + "method.response.header.Access-Control-Expose-Headers": "'X-Hd'" } } } diff --git a/smithy-aws-apigateway-openapi/src/test/resources/software/amazon/smithy/aws/apigateway/openapi/cors-model.openapi.json b/smithy-aws-apigateway-openapi/src/test/resources/software/amazon/smithy/aws/apigateway/openapi/cors-model.openapi.json index cbd7e4d8357..6b03c019cb6 100644 --- a/smithy-aws-apigateway-openapi/src/test/resources/software/amazon/smithy/aws/apigateway/openapi/cors-model.openapi.json +++ b/smithy-aws-apigateway-openapi/src/test/resources/software/amazon/smithy/aws/apigateway/openapi/cors-model.openapi.json @@ -199,7 +199,7 @@ "default": { "statusCode": "200", "responseParameters": { - "method.response.header.Access-Control-Expose-Headers": "'X-Service-Output-Metadata'", + "method.response.header.Access-Control-Expose-Headers": "'X-Foo-Header,X-Service-Output-Metadata'", "method.response.header.Access-Control-Allow-Origin": "'https://www.example.com'" } } diff --git a/smithy-aws-apigateway-openapi/src/test/resources/software/amazon/smithy/aws/apigateway/openapi/cors-with-additional-headers.openapi.json b/smithy-aws-apigateway-openapi/src/test/resources/software/amazon/smithy/aws/apigateway/openapi/cors-with-additional-headers.openapi.json index b68d0b78c1e..73af2a4cde2 100644 --- a/smithy-aws-apigateway-openapi/src/test/resources/software/amazon/smithy/aws/apigateway/openapi/cors-with-additional-headers.openapi.json +++ b/smithy-aws-apigateway-openapi/src/test/resources/software/amazon/smithy/aws/apigateway/openapi/cors-with-additional-headers.openapi.json @@ -199,7 +199,7 @@ "default": { "statusCode": "200", "responseParameters": { - "method.response.header.Access-Control-Expose-Headers": "'X-Service-Output-Metadata'", + "method.response.header.Access-Control-Expose-Headers": "'X-Foo-Header,X-Service-Output-Metadata'", "method.response.header.Access-Control-Allow-Origin": "'https://www.example.com'" } } diff --git a/smithy-aws-apigateway-openapi/src/test/resources/software/amazon/smithy/aws/apigateway/openapi/cors-with-custom-gateway-response-headers.openapi.json b/smithy-aws-apigateway-openapi/src/test/resources/software/amazon/smithy/aws/apigateway/openapi/cors-with-custom-gateway-response-headers.openapi.json index e604a632e94..3ced368ac52 100644 --- a/smithy-aws-apigateway-openapi/src/test/resources/software/amazon/smithy/aws/apigateway/openapi/cors-with-custom-gateway-response-headers.openapi.json +++ b/smithy-aws-apigateway-openapi/src/test/resources/software/amazon/smithy/aws/apigateway/openapi/cors-with-custom-gateway-response-headers.openapi.json @@ -63,7 +63,8 @@ "default": { "statusCode": "200", "responseParameters": { - "method.response.header.Access-Control-Allow-Origin": "'https://foo.com'" + "method.response.header.Access-Control-Allow-Origin": "'https://foo.com'", + "method.response.header.Access-Control-Expose-Headers": "'X-Hd'" } } } diff --git a/smithy-openapi/src/main/java/software/amazon/smithy/openapi/fromsmithy/OpenApiConverter.java b/smithy-openapi/src/main/java/software/amazon/smithy/openapi/fromsmithy/OpenApiConverter.java index 2fae2eb2de8..b681ac9c408 100644 --- a/smithy-openapi/src/main/java/software/amazon/smithy/openapi/fromsmithy/OpenApiConverter.java +++ b/smithy-openapi/src/main/java/software/amazon/smithy/openapi/fromsmithy/OpenApiConverter.java @@ -426,10 +426,12 @@ private void addPaths( String method = result.getMethod(); String path = result.getUri(); PathItem.Builder pathItem = paths.computeIfAbsent(result.getUri(), (uri) -> PathItem.builder()); + // Mark the operation deprecated if the trait's present. if (shape.hasTrait(DeprecatedTrait.class)) { result.getOperation().deprecated(true); } + // Add security requirements to the operation. addOperationSecurity(context, result.getOperation(), shape, plugin); @@ -438,9 +440,10 @@ private void addPaths( .map(DocumentationTrait::getValue) .ifPresent(description -> result.getOperation().description(description)); - // Pass the operation through the plugin system and then build it. - OperationObject builtOperation = plugin.updateOperation( - context, shape, result.getOperation().build(), method, path); + OperationObject builtOperation = result.getOperation().build(); + + // Pass the operation through the plugin system. + builtOperation = plugin.updateOperation(context, shape, builtOperation, method, path); // Add tags that are on the operation. builtOperation = addOperationTags(context, shape, builtOperation); // Update each parameter of the operation and rebuild if necessary. @@ -449,6 +452,8 @@ private void addPaths( builtOperation = updateResponses(context, shape, builtOperation, method, path, plugin); // Update the request body of the operation and rebuild if necessary. builtOperation = updateRequestBody(context, shape, builtOperation, method, path, plugin); + // Pass the operation through the plugin system for post-processing. + builtOperation = plugin.postProcessOperation(context, shape, builtOperation, method, path); switch (method.toLowerCase(Locale.ENGLISH)) { case "get": diff --git a/smithy-openapi/src/main/java/software/amazon/smithy/openapi/fromsmithy/OpenApiMapper.java b/smithy-openapi/src/main/java/software/amazon/smithy/openapi/fromsmithy/OpenApiMapper.java index fe128f399bf..1f28bf7e1a5 100644 --- a/smithy-openapi/src/main/java/software/amazon/smithy/openapi/fromsmithy/OpenApiMapper.java +++ b/smithy-openapi/src/main/java/software/amazon/smithy/openapi/fromsmithy/OpenApiMapper.java @@ -71,7 +71,10 @@ default byte getOrder() { default void updateDefaultSettings(Model model, OpenApiConfig config) {} /** - * Updates an operation. + * Updates an operation before invoking the plugin system on the contents + * of the operation (specifically, before {@link #updateParameter}, + * {@link #updateRequestBody}, {@link #updateResponse}, + * {@link #updateRequestBody}, and {@link #postProcessOperation}). * * @param context Conversion context. * @param shape Operation being converted. @@ -90,6 +93,29 @@ default OperationObject updateOperation( return operation; } + /** + * Updates an operation after invoking the plugin system on the contents + * of the operation (specifically, after {@link #updateOperation}, + * {@link #updateParameter}, {@link #updateRequestBody}, + * {@link #updateResponse}, and {@link #updateRequestBody}). + * + * @param context Conversion context. + * @param shape Operation being converted. + * @param operation OperationObject being built. + * @param httpMethodName The HTTP method of the operation. + * @param path The HTTP URI of the operation. + * @return Returns the updated operation object. + */ + default OperationObject postProcessOperation( + Context context, + OperationShape shape, + OperationObject operation, + String httpMethodName, + String path + ) { + return operation; + } + /** * Updates a path item. * @@ -269,6 +295,23 @@ public OperationObject updateOperation( return operation; } + @Override + public OperationObject postProcessOperation( + Context context, + OperationShape shape, + OperationObject operation, + String httpMethodName, + String path + ) { + for (OpenApiMapper plugin : sorted) { + if (operation == null) { + return null; + } + operation = plugin.postProcessOperation(context, shape, operation, httpMethodName, path); + } + return operation; + } + @Override public PathItem updatePathItem(Context context, String path, PathItem pathItem) { for (OpenApiMapper plugin : sorted) {