refactor HTTP header verification

[niklasad1]

I'm struggling to understand ALWAYS_ALLOWED_HEADERS; I think that we allow any headers specified in AllowHeaders plus any in ALWAYS_ALLOWED_HEADERS.

I guess we have this list so that if the user doesn't allow any headers themselves via the access control stuff, standard requests will still work?

Some headers are also not completely black and white as to whether they are accepted or not. Eg with CORS, we'll always need to return "Content-Type" in the "Access-Control-Allowed-Headers" response, because otherwise I think the user couldn't set that to "application/json" without CORS disallowing it (see https://developer.mozilla.org/en-US/docs/Glossary/CORS-safelisted_request_header#additional_restrictions).

Also there is an Access-Control-Allow-Origin header in the list, which is what the server would respond with, so perhaps this doesn't need to be there?
Also a client can send an Access-Control-Request-Method header in a preflight request, and so perhaps that should be there?

Originally posted by @jsdw in #781 (comment)

[jsdw]

So I had a look at the code more, and this is my current understanding of the flow:

1. User provides access control list (either allowing any header or some set list).
2. Any new http connection hits this line, which checks the headers:
   

   jsonrpsee/http-server/src/server.rs

   Line 473 in a26f1fb

   if let Err(e) = acl.verify_headers(keys, cors_request_headers) {

3. That function calls this one, which makes sure headers are part of the ACL or in an ALWAYS_ALLOWED_HEADERS whitelist. This function returns the actual allowed headers (which seem to be ignored):
   

   jsonrpsee/core/src/server/access_control/cors.rs

   Line 270 in a26f1fb

   pub(crate) fn get_cors_allow_headers<T: AsRef<str>, O, F: Fn(T) -> O>(

Observations:

• This get_cors_allow_headers function returns a list of headers that we should respond with by the looks of it, but that list isn't actually used anywhere. It probably should be?
• Instead, the server responds with acl.allowed_headers().to_cors_header_value(), which I think responds with exactly the headers that the user has allowed, ignoring the ALWAYS_ALLOWED_HEADERS list by the looks of it and ignoring the headers that were actually requested?
• As noted above, the whitelist contains one or two headers it probably shouldn't, and could be tweaked?

So maybe we just need to:

• Tweak the ALWAYS_ALLOWED_HEADERS headers
• Tweak the access-control-allow-headers response that we return to use the list of headers that comes back from get_cors_allow_headers?

[jsdw]

Chatting with @lexnv and mulling over this more, there are a couple of ambiguities that stand out to me at the moment in the code, and I'm not clear enough on the intention of it to want to commit to anything here:

1. Is this ACL stuff specifically about CORS, or is it more general? We currently validate headers of every request for instance, but if we only cared about CORS I think we'd only need to validate anything in an OPTIONS request (browsers would always preflight the application/json POST requests that would need to be sent.)
2. Currently the AccessControl struct allows eithe all headers, or only a specific set of provided headers, and we respond to a CORS OPTIONS request with that set of headers, but just prior to this, we validate headers and allow any from either this AccessControl struct or a global whitelist. There's some inconsistency here I think.

So, are we focusing solely on handling CORS requests with this AccessControl stuff, or are we trying to be more general and support CORS but also just generally disallow certain headers and things?

I think if we are clear on what the goals are, it'll make it clearer what changes we should make to make it all consistent :)

So let's shelve this until @niklasad1 is around and have a chat about it then!

[lexnv]

This issue will be closed by #842.

We have decided to replace the CORS validation with tower middleware for HTTP purposes.

In doing so, JSONRPSee will validate only the header and origin to keep functionality parity.

And a huge chunk of code can be removed from JSONRPSee and maintained upstream in tower.
While at the same time, offering users a very generic middleware that would suffice all use-cases.