Added API Gateway multi-origin CORS example #468

loganzartman · 2018-06-11T22:21:49Z

Issue #, if available:

Description of changes:
Created an example of using Lambda to provide CORS headers on API Gateway endpoints given a list of allowed origins. The Lambda function checks the origin of incoming requests against a list, and only provides CORS headers if there is a match.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

jackrk · 2018-06-11T22:39:54Z

examples/2016-10-31/api_multiple_origin_cors/template.yaml

+          Properties:
+            Path: '/test'
+            Method: 'options'
+        PostTest:


I see there is some special case for DELETE methods in the function. Is it useful to have a DELETE method in the example API? Up to you.

I chose DELETE arbitrarily to demonstrate how to handle preflight requests, since it is one of the methods that is guaranteed to trigger one. So I think it is more a specific example of a general case.
I do notice now that the event is still named "PostTest", whereas it should maybe be something like "DeleteTest".

brettstack · 2018-06-11T22:49:17Z

examples/2016-10-31/api_multiple_origin_cors/cors-util.js

+ * @param allowedOrigins A list of strings or regexes representing allowed origin URLs
+ * @return {Object} an object containing a header => value mapping, or nothing
+ */
+exports.makeOriginHeader = (event, allowedOrigins) => {


Pass in origin instead of event

brettstack · 2018-06-11T22:53:36Z

examples/2016-10-31/api_multiple_origin_cors/cors-util.js

@@ -0,0 +1,65 @@
+const DEFAULT_ALLOWED_HEADERS = [
+    "Content-Type",
+    "X-Amz-Date",


Comments on what some of the X-Amz headers are would be useful

brettstack · 2018-06-11T22:57:22Z

examples/2016-10-31/api_multiple_origin_cors/README.md

+
+### Local development
+
+`sam local start-api`


Link to SAM CLI and mention these instructions assume it is set up

brettstack · 2018-06-11T23:07:08Z

examples/2016-10-31/api_multiple_origin_cors/cors-util.js

+ * Otherwise, return an empty object literal.
+ * @param event Lambda event event associated with a request
+ * @param allowedOrigins A list of strings or regexes representing allowed origin URLs
+ * @return {Object} an object containing a header => value mapping, or nothing


"containing allowed headers and their value" and remove ", or nothing"

brettstack · 2018-06-11T23:11:02Z

examples/2016-10-31/api_multiple_origin_cors/cors-util.js

+
+    // look for origin in list of allowed origins
+    const allowedPattern = allowedOrigins.find(pattern => origin.match(pattern));
+    if (typeof allowedPattern === "undefined")


if (allowedPattern === undefined); same for origin check above

brettstack · 2018-06-11T23:23:08Z

examples/2016-10-31/api_multiple_origin_cors/cors-util.js

+ * @param {Array} allowedHeaders (optional) a list of strings representing allowed headers
+ * @return {Object} an object containing several header => value mappings
+ */
+exports.makeHeaders = (event, allowedOrigins, allowedMethods, allowedHeaders = DEFAULT_ALLOWED_HEADERS) => {


This should also just be origin

brettstack · 2018-06-11T23:24:30Z

examples/2016-10-31/api_multiple_origin_cors/cors-util.js

+ */
+exports.compileURLWildcards = (path) => {
+    const parts = path.split("*");
+    const escapeRegex = str => str.replace(/([.?*+^$(){}|[\-\]\\])/g, "\\$1");


Add comment to make this clearer; maybe use an example.

brettstack · 2018-06-11T23:25:13Z

examples/2016-10-31/api_multiple_origin_cors/cors-util.test.js

+    return {"headers": {"origin": origin}};
+};
+
+// makeOriginHeader


brettstack · 2018-06-11T23:29:57Z

examples/2016-10-31/api_multiple_origin_cors/index.js

+ * handle OPTIONS requests and provide the CORS headers.
+ * When the browser makes the DELETE request, we only need to provide the origin.
+ */
+const handleTest = (event, context, callback) => {


With nodejs8.10 you should be able to define handlers like

async () => { return { headers, statusCode } }

brettstack · 2018-06-11T23:31:44Z

examples/2016-10-31/api_multiple_origin_cors/cors-util.js

@@ -0,0 +1,65 @@
+const DEFAULT_ALLOWED_HEADERS = [


Is there a library that exists that might be better than rolling your own?

brettstack · 2018-06-12T17:17:07Z

examples/2016-10-31/api_multiple_origin_cors/index.js

+    "http://127.0.0.1",
+    "https://*.example.com",
+    "https://*.amazon.com"
+].map(cors.compileURLWildcards);


Simpler interface would be cors.compileURLWildcards([]). Even better if you just do this for them behind the scenes (inside cors.makeHeaders)

My goal was to isolate the wildcard support from the rest of the implementation, in case a user wants to use some other library or provide their own regex for the origins.

They can easily modify themselves. cors-utils is baked in; not a library.

Also updated old test to reflect desired behavior

brettstack · 2018-06-12T23:17:34Z

examples/apps/api-gateway-multiple-origin-cors/cors-util.js

+ * @return {Object} an object containing allowed header and its value
+ */
+exports.makeOriginHeader = (origin, allowedOrigins) => {
+    if (origin === undefined || origin === null)


if (!origin)

brettstack · 2018-06-12T23:18:22Z

examples/apps/api-gateway-multiple-origin-cors/cors-util.js

+ */
+exports.makeOriginHeader = (origin, allowedOrigins) => {
+    if (origin === undefined || origin === null)
+        return {}; // no CORS headers necessary; browser will load resource


"no CORS headers necessary" if no origin? Why?

Firefox, for example, will not send an Origin header in a same-origin request, but will always send one in a cross-origin request. If there is no origin, I don't think there's anything else to be done here.

brettstack · 2018-06-12T23:18:59Z

examples/apps/api-gateway-multiple-origin-cors/cors-util.js

+        return {"Access-Control-Allow-Origin": origin};
+
+    // the origin does not match any allowed origins
+    return {}; // return no CORS headers; browser will not load resource


Nice. This is all much simpler.

brettstack · 2018-06-12T23:19:28Z

examples/apps/api-gateway-multiple-origin-cors/cors-util.js

+exports.compileURLWildcards = (url) => {
+    // unreserved characters as per https://tools.ietf.org/html/rfc3986#section-2.3
+    const url_unreserved_pattern = "[A-Za-z0-9\-._~]";
+    const wildcard_pattern = url_unreserved_pattern + "*";


This isn't python ;)

brettstack · 2018-06-12T23:20:50Z

examples/apps/api-gateway-multiple-origin-cors/index.js

+    "http://127.0.0.1",
+    "https://*.example.com",
+    "https://*.amazon.com"
+].map(cors.compileURLWildcards);


Still want cors-util to do wildcard logic for me.

I'll change that

brettstack · 2018-06-12T23:21:36Z

examples/apps/api-gateway-multiple-origin-cors/index.js

+ * We include all CORS headers in the GET response.
+ */
+exports.handleRoot = async (event, context) => {
+    const origin = event.headers.Origin || event.headers.origin;


Move this logic to a getOriginFromHeaders function

brettstack · 2018-06-12T23:24:58Z

examples/apps/api-gateway-multiple-origin-cors/index.js

+    if (event.httpMethod === "OPTIONS") {
+        // return an empty response, with all CORS headers
+        return {
+            headers: cors.makeHeaders(origin, allowedOrigins, allowedMethods),


Better name might be getPreflightRequestHeaders. Could also add a higher abstraction: return getPreflightResponse(origin, allowedOrigins, allowedMethods) since this is likely something customers will want to do a lot (every path/method that requires preflight)

A higher level of abstraction seems reasonable. I'm not sure if there would be a case where a user might want to add extra headers to the response. It might be sufficient to add a parameter for Access-Control-Max-Age, which seems to be the only other header relevant to a preflight response.

I've gone ahead and done that

brettstack · 2018-06-14T17:51:40Z

Looking good. The only other things (as communicated offline) is to split this into multiple Lambda Functions instead of implementing routing. Alternatively, if you prefer to use a single function, use aws-serverless-express (and in this case you could probably just use the cors middleware)

loganzartman · 2018-06-14T21:49:18Z

Improved the routing

brettstack

You've split into additional Lambda functions which is great, but the handlers should be in separate files. Maybe something like routes/root.js and routes/test.js and move any common logic into a /service|helpers|utils.js file.

Also, just 2 Lambda functions here is fine; 1 per route; let routes/test Lambda function/handler also handle preflight. It's a common pattern to split on path/resource but perform different logic based on the method. This seems like a good middleground between being too aggressive with individual functions and jamming everything into a single function.

brettstack · 2018-06-15T18:44:18Z

examples/apps/api-gateway-multiple-origin-cors/template.yaml

            Path: '/test'
            Method: 'delete'
-        OptionsTest:
+  ExampleTestPreflight:


This should go in the same Lambda function as delete.

brettstack

Awesome work. Thanks!

Added API Gateway multi-origin CORS example

1a485b4

jackrk approved these changes Jun 11, 2018

View reviewed changes

brettstack suggested changes Jun 11, 2018

View reviewed changes

brettstack reviewed Jun 12, 2018

View reviewed changes

Logan Zartman added 7 commits June 12, 2018 10:53

Updated docs and improved naming of endpoint

dc7235e

Created new test for compileURLWildcards

0926287

Also updated old test to reflect desired behavior

Fixed compileURLWildcards

c40408b

Refactored to avoid passing event into cors-util; improved clarity

d4a6db8

Improve routing, use async syntax for handlers

ca85e79

Moved multi-origin cors example to examples/apps

7fe3ae0

Move multiple origin CORS example to more uniformly-named directory

1cb6bb0

brettstack suggested changes Jun 12, 2018

View reviewed changes

Logan Zartman added 3 commits June 12, 2018 17:03

Refactor multiple-origin-cors example

293027f

Automatically compile wildcards for multiple-origin-cors

f71d07f

makeHeaders replaced with createPreflightResponse

65ec9eb

Route using API Gateway, refactor

c8585ac

brettstack suggested changes Jun 15, 2018

View reviewed changes

improve routing

2376325

brettstack approved these changes Jun 16, 2018

View reviewed changes

Simon2228 approved these changes Jun 21, 2018

View reviewed changes

Simon2228 merged commit 21e9063 into aws:develop Jun 21, 2018

Uh oh!

Added API Gateway multi-origin CORS example #468

Added API Gateway multi-origin CORS example #468

Uh oh!

Conversation

loganzartman commented Jun 11, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

brettstack commented Jun 14, 2018

Uh oh!

loganzartman commented Jun 14, 2018

Uh oh!

brettstack left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

brettstack left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!