swagger-express-middleware
swagger-express-middleware copied to clipboard
Add new middleware to handle non-CORS OPTIONS requests
I'd like to see a new middleware added to handle non-CORS OPTIONS
requests by responding 200 OK
with an Allow
response header detailing which HTTP methods are supported for that path.
This should be easy to add as that same logic is already used to construct the Access-Control-Allow-Methods
response header in the CORS middleware today: lib/cors.js#L50-L53
This new middleware should only end the response if there isn't an actual OPTIONS
route configured in the associated Swagger spec. It should add the Allow
response header no matter what, though.
Advanced Feature Idea (Extending the Original Idea): There could also be a config option for this new middleware to enable returning the JSON spec for each HTTP method based on the associated Swagger spec, a la the section "The response body and API documentation" from this intriguing blog post.
While this is a cool idea, and I like the blog post you linked to, I don't think this falls under the scope of what I want to include in Swagger Express Middleware. At least not unless Swagger starts incorporating Hypermedia and HATEOAS into the spec itself. Maybe it could go in a separate npm package called something like swagger-hypermedia-middleware
.
However, it won't be hard for you to write this middleware yourself. The metadata middleware decorates the Request
object with all the Swagger metadata for each request, so as long as your middleware comes after the metadata middleware, you'll be able to use that metadata to set whatever headers and/or response body you want - even for OPTIONS
requests. (as long as your middleware comes before the cors middleware)
Good point about me just making that middleware myself, though I do still see value in the first comment's core functionality (the non-hypermedia part) being a part of this module. Sure you're not open to that part?
If not, that's fine, and I'll create my own module to extend this middleware. shrugs
For the first part... I don't like the idea of creating a CORS-ish middleware for non-CORS requests. Seems like a pretty specific use case and not general-purpose enough for a Swagger library.
However, I could totally be open to adding an option to the existing CORS middleware to have it not automatically send the response. If that option is set, then the CORS middleware would just set the CORS headers (if the request is a CORS request) and would then pass-through to the next middleware, which would give you an opportunity to perform additional logic and send your own custom response.
To clarify, I'm just suggesting that there be a new middleware to handle responding to [non-CORS preflight] OPTIONS
requests in the standard/expected RESTful way:
- Responding with
200 OK
status - Including a response header named
Allow
that details all of the possible HTTP methods that can be accepted at that path, e.g.Allow: GET, POST, PUT, DELETE, OPTIONS
, based on the associated Swagger spec.
This seems very general purpose as it is just a normal [but oft-overlooked] standard response to an OPTIONS
request for HTTP/RESTful services, which pre-dates the creation of CORS.
P.S. Once again, I'm fine creating it myself but it seems like a pretty easy and standard functionality that would fit well in this collection of middleware since it is very easy to accomplish using the associated Swagger spec, just like you do in the CORS middleware.
Just want to make sure you know what I am referring to since I get the impression we're not on the same page.
To clarify, I'm just suggesting that there be a new middleware to handle responding to [non-CORS preflight] OPTIONS requests in the standard/expected RESTful way:
- Responding with 200 OK status
- Including a response header named Allow that details all of the possible HTTP methods that can be accepted at that path, e.g. Allow: GET, POST, PUT, DELETE, OPTIONS, based on the associated Swagger spec.
Isn't that what the CORS middleware does today? It (mistakenly) responds to all OPTIONS
requests - even non-CORS ones - with the Allow
header set, based on the Swagger spec
The CORS middleware currently sets the Access-Control-Request-Method
response header (e.g. Access-Control-Request-Method: GET, POST
) in this fashion for any HTTP request [with a CORS Origin
request header (#41, #42)] -- not just for HTTP OPTIONS
requests.
The proposed middleware would set the Allow
response header (e.g. Allow: GET, POST, OPTIONS
) in the same fashion for all HTTP OPTIONS
requests -- at least non-CORS requests, though you could also include CORS preflight requests without any known detriment.
Oh! NOW I see what you mean! Sorry... I guess I'm not firing on all cylinders today. Thanks for persevering.
I honestly thought the CORS middleware was already doing that. But I was mixing-up the Access-Control-Allow-Method
header and the Allow
header.
Yes, I 100% agree that automatically setting the Allow
header is a common use case and certainly generic enough for this library to handle. The only question now is whether it should be its own middleware, or just be part of the CORS middleware. That is, if the request is a CORS request (i.e. it has an Origin
header), then the CORS middleware would set the Access-Control-Allow-Method
header and immediately send the response, as it does today. But if the request is not a CORS request, then the CORS middleware would just set the Allow
header and then call next()
I think it feels a little strange to have to enable the CORS middleware if you really just wanted the normal OPTIONS
responses but did not want to enable CORS due to security reasons.
Yeah, agreed. I was just trying to be lazy. :)
So what should this new middleware be called? middleware.allow()
would certainly work, but maybe that's too narrow-focused? Maybe it should be more generic, like middleware.setHeaders()
, so it could be used to set other commonly-needed headers in the future? Can you think of any other headers besides Allow
that would be commonly needed and could be inferred from a Swagger API?
I'm pretty new to Swagger (but not HTTP/REST). That said, no other common HTTP response headers spring to mind that aren't either already set by Express internally (Content-Type
, Content-Length
, ETag
, Vary
, etc.) or else requiring customized values based on coded non-Swagger logic.
Probably the next most common HTTP response headers that are not dealt with would be those related to caching (Cache-Control
, Expires
, Pragma
(+ ETag
, Vary
)) but I don't think there are any obvious defaults for those. :question:
After those, maybe HSTS (Strict-Transport-Security
)...? But again, doesn't seem like there is a sensible default value for that, and setting that unbeknownst to those serving the site can actually end up causing some headaches. :confused: