[CORS] Adds W3C Cors Specification Support #665
Conversation
eliasjpr
changed the title
There was a problem hiding this comment.
I haven't looked into the deep magic of cores much but on the surface this looks fine to me. I'll defer to @drujensen or @fridgerator on this one.
| if allow_credentials | ||
| context.response.headers["Access-Control-Allow-credentials"] = "true" | ||
| end | ||
| private def put_response_headers(response) |
There was a problem hiding this comment.
these put_ method names are inconsistent with set_preflight_headers below, one pattern or the other should be consistent throughout the class
| end | ||
| end | ||
|
|
||
| struct Origin |
There was a problem hiding this comment.
Above OriginType is defined, but it's not used at all in this struct. Should it be?
spec/amber/pipes/cors_spec.cr
Outdated
| origins << "example.com" | ||
| context = cors_context( | ||
| "OPTIONS", | ||
| "Origin": "example.com", |
There was a problem hiding this comment.
should this and line 92 above point to a shared variable?
spec/amber/pipes/cors_spec.cr
Outdated
| "OPTIONS", | ||
| "Origin": "example.com", | ||
| "Access-Control-Request-Method": "PUT", | ||
| "Access-Control-Request-Headers": "Accept" |
There was a problem hiding this comment.
this could use the header constants
There was a problem hiding this comment.
How would you do this in crystal? The arguments are passed as a NamedTuple. I am not sure how to accomplish what you are asking.
spec/amber/pipes/cors_spec.cr
Outdated
| origin_header.should_not be_nil | ||
| end | ||
|
|
||
| def assert_cors_not_success(context) |
There was a problem hiding this comment.
this english of this method name is a little unexpected, how about refute_cors_success?
| context.response.headers["Access-Control-Allow-Origin"] = allow_origin | ||
| def forbidden(context) | ||
| context.response.headers["Content-Type"] = "text/plain" | ||
| context.response.respond_with_error FORBIDDEN, 403 |
There was a problem hiding this comment.
How will this interact with the Error pipe?
There was a problem hiding this comment.
This writes to the response without raising an error. Should we raise an error here?
There was a problem hiding this comment.
I think so, that way it can be caught by the errors pipe and rendered nicely
There was a problem hiding this comment.
So we do not need to raise error here just respond with 403 according to the CORS specs since the Pref-light request is done behind the scenes by the browser
eliasjpr
force-pushed
the
ep/cors
branch
8 times, most recently
from
99a4c83 to
95e21ec
Compare
Provides facilities for dealing with CORS requests and responses
- A Plug (Handler) that handles CORS request and responses.
\## How It Works
This handler is compliant with the W3C CORS specification. As per
this specification, It doesn’t put any CORS response headers
in a connection that holds an invalid CORS request. To know what
“invalid” CORS request means, have a look at the
“Validity of CORS requests” section below.
When some options that are not mandatory and have no default value
(such :max_age) at intialization, the relative header will often not be
sent at all. This is compliant with the specification and at the
same time it reduces the size of the response, even if just by a handful
of bytes.
The following is a list of all the CORS response headers supported by
the handler:
- Access-Control-Allow-Origin
- Access-Control-Allow-Methods
- Access-Control-Allow-Headers
- Access-Control-Allow-Credentials
- Access-Control-Expose-Headers
- Access-Control-Max-Age
\## Validity of CORS requests
“Invalid CORS request” can mean that a request doesn’t have an Origin
header (so it’s not a CORS request at all) or that it’s a CORS request but:
- The Origin request header doesn’t match any of the allowed origins
- The request is a preflight request but it requests to use a method
or some headers that are not allowed (via the Access-Control-Request-Method
and Access-Control-Request-Headers headers)
\## Responding to preflight requests
When the request is a preflight request and is a valid one (valid origin, valid
request method, and valid request headers), The CORS handler directly sends a
response to that request instead of just adding headers to the connection.
To do this, the handler halts the connection and sends a response.
eliasjpr
force-pushed
the
ep/cors
branch
3 times, most recently
from
0a3709e to
58bcae1
Compare
ISSUE #240
How It Works
This handler is compliant with the W3C CORS specification. As per this specification, It doesn’t put any CORS response headers in a connection that holds an invalid CORS request. To know what “invalid” CORS request means, have a look at the “Validity of CORS requests” section below.
When some options that are not mandatory and have no default value (such :max_age) at intialization, the relative header will often not be sent at all. This is compliant with the specification and at the same time it reduces the size of the response, even if just by a handful of bytes.
The following is a list of all the CORS response headers supported by the handler:
Validity of CORS requests
“Invalid CORS request” can mean that a request doesn’t have an Origin header (so it’s not a CORS request at all) or that it’s a CORS request but:
or some headers that are not allowed (via the Access-Control-Request-Method
and Access-Control-Request-Headers headers)
Responding to preflight requests
When the request is a preflight request and is a valid one (valid origin, valid request method, and valid request headers), The CORS handler directly sends a response to that request instead of just adding headers to the connection. To do this, the handler halts the connection and sends a response.
Requesting for inputs on the following