Best way to do CORS preflight

[theelims]

What is the recommended way to perform a CORS preflight test? I want to avoid adding a dedicated OPTIONS endpoint to any POST endpoint I have.

I tried a special CORSHandler, but it never got triggered.

And I tried to tap into the not found mechanism, but this resulted in a core panic.

[playmiel]

Hi, I'm in the same situation as you, have you found a solution?

[staviq]

I think I found a workaround for this

If instead of using request->reply() you construct a PsychicResponse and send that, you can add custom headers to the reply, and bypass CORS.

So this:

return request->reply(200, "application/json", jsonBuffer.c_str()); });

becomes this:

PsychicResponse response(request);
response.setCode(200);
response.setContentType("application/json");
response.setContent(jsonBuffer.c_str());
response.addHeader("Access-Control-Allow-Origin", "*");
return response.send();

You have to do that for each handler you want to bypass CORS for though.

[hoeken]

You could also extend the PsychicResponse class and add the custom stuff if you have a lot of handlers.

On Tue, May 28, 2024, 16:54 staviq @.***> wrote:

I think I found a workaround for this

If instead of using request->reply() you construct a PsychicResponse and send that, you can add custom headers to the reply, and bypass CORS.

So this:

return request->reply(200, "application/json", jsonBuffer.c_str()); });

becomes this:

PsychicResponse response(request); response.setCode(200); response.setContentType("application/json"); response.setContent(jsonBuffer.c_str()); response.addHeader("Access-Control-Allow-Origin", "*"); return response.send();

You have to do that for each handler you want to bypass CORS for though.

— Reply to this email directly, view it on GitHub https://github.com/hoeken/PsychicHttp/issues/72#issuecomment-2136087956, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABEHSUG4UY5YBGU22GSHYDZETVJHAVCNFSM6AAAAABCVWXHDCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMZWGA4DOOJVGY . You are receiving this because you are subscribed to this thread.Message ID: @.***>

[villszer]

But how do you handle preflight requests?

For the Access-Control-Allow-Origin header you can just add it as a default to everything like this

DefaultHeaders::Instance().addHeader("Access-Control-Allow-Origin", "*");

But there is a preflight request which fails

[hitecSmartHome]

I have found the solution: https://github.com/hoeken/PsychicHttp/issues/102#issuecomment-2238563979

Basically you have to add these default headers

DefaultHeaders::Instance().addHeader("Access-Control-Allow-Origin", "*");
DefaultHeaders::Instance().addHeader("Access-Control-Allow-Methods", "GET, POST, PUT, DELETE, OPTIONS");
DefaultHeaders::Instance().addHeader("Access-Control-Allow-Headers", "Content-Type, Authorization, Accept");

And a wildcard handler for the HTTP_OPTIONS requets

server.on("/*", HTTP_OPTIONS, [](PsychicRequest *request) {
  return request->reply(200);
});

[hitecSmartHome]

Nevermind. If it is in place, the web app works which is not on the same host, but if the esp32 is the server it does not work.

[hitecSmartHome]

A middleware would be the best option where we could manipulate the headers also.

[hoeken]

Hey all, I'm not really up to date on CORS. Can someone send me a tutorial on how the client/server need to communicate? It seems like you need to check for the headers and respond with other headers in a callback no?

[staviq]

Hey all, I'm not really up to date on CORS. Can someone send me a tutorial on how the client/server need to communicate? It seems like you need to check for the headers and respond with other headers in a callback no?

Basically, whenever your browser loads a page, every image, link, css file, and so on, gets verified against the url of that page itself.

If you include an image in your html, and that image is hosted somewhere else ( different host/domain ) the browser will block this and refuse to load the image.

This is generally a good thing, and prevents cross site exploits etc.

Except, if you are testing your own web page, via localhost, the browser will block pretty much everything outside that particular html file. That's the main problem. CORS ( in the browser ) is in fully restrictive mode if you are trying to test things locally.

The only reasonable, and the simplest way to get around this, is to make the server serve the html page, together with a custom http header ( mentioned in other comments here )

The only thing you need to fiddle with and modify, is the request handler on the server side, and you just need to convince it to add another http header, beside the default ones. No extra parsing, no overriding, just one more http header, so the browser won't use cors when talking with your own server.

[mathieucarbou]

Example: https://github.com/espressif/arduino-esp32/blob/master/libraries/WebServer/src/WebServer.cpp#L553-L557

[hitecSmartHome]

The problem is that there is a preflight request for every request you make on the client side. Your server should send a response to this preflight request without content and a 200 response. It is an HTTP-OPTIONS request

[mathieucarbou]

The problem is that there is a preflight request for every request you make on the client side. Your server should send a response to this preflight request without content and a 200 response. It is an HTTP-OPTIONS request

This is what the arduino server is doing. The same thing would need to be implemented in Psychic. That's not complicated, just a global handler on OPTIONS and add the headers back, Ideally, expose that in a configuration.

(https://developer.mozilla.org/en-US/docs/Glossary/Preflight_request)

I remember that well I was developping js libraries years ago around ajax (when browsers still were not all supporting cors and some like IE were problematic).

What I don't see from Arduino WS is the Access-Control-Max-Age header which is super important to set otherwise the browser will keep sending preflight requests (each 5 sec for chrome).

[hitecSmartHome]

Yeah, thats why I said it would be a good idea to implement a way to use express.js style middlewares. You could attach a global middleware where you listen to all OPTIONS requests, also you could use these for authentication on specific routes. You can have one authentication function and attach it to any route you want. In node express js, a lot of things is done this way. For example CORS.

[hoeken]

Okay, so I see what you guys mean now about the CORS. I was experimenting with it, and it seems like sometimes the browser will skip the OPTIONS request and look for the headers in the GET request. I came up with this kludge that seems to work:

    //this is for SUPER ULTRA PERMISSIVE CORS requests
    DefaultHeaders::Instance().addHeader("Access-Control-Allow-Origin", "*");
    DefaultHeaders::Instance().addHeader("Access-Control-Allow-Methods", "GET, POST, PUT, DELETE, OPTIONS");
    DefaultHeaders::Instance().addHeader("Access-Control-Allow-Headers", "Content-Type, Authorization, Accept");
    DefaultHeaders::Instance().addHeader("Access-Control-Max-Age", "86400");
    server.on("/*", HTTP_OPTIONS, [](PsychicRequest* request) {
      PsychicResponse response(request);
      response.addHeader("Access-Control-Allow-Origin", "*");
      response.addHeader("Access-Control-Allow-Methods", "GET, POST, PUT, DELETE, OPTIONS");
      response.addHeader("Access-Control-Allow-Headers", "Content-Type, Authorization, Accept");
      response.addHeader("Access-Control-Max-Age", "86400");
      response.setCode(200);

      return response.send();
    });

PS. You will need to run it with the v2-dev branch, since I discovered a bug with setting headers multiple times, fixed now.

Moving forward, its clear we need some kind of endpoint->middleware() functionality to enable this sort of logic. I'm still trying to think how we can pull that off without breaking compatibility too much.

[proddy]

A handy way to test this is using the "CORS Unblock" browser extension (for Chrome and Edge). It basically interrupts the headers and mangles them, which is what we're doing in code. The site shows which access-control headers they are setting. Just interesting for comparison I guess.

[hoeken]

I've been thinking about this a bit, and I think we could add an easy temporary fix that wont break anything without going full-on middleware that will require fairly large breaking changes.

It seems that most of the things people want to are working with headers in a filter(). Its a bit of a hack, but adding a request->addResponseHeader() would allow you to do that without changing the API. we could then scan those headers in the PsychicResponse object and copy them over. The request object gets passed to every new Response, so it should work pretty nicely.

Thoughts?

[hitecSmartHome]

Well, if the response headers are passed to every endpoint then its a middleware :D

[mathieucarbou]

@hoeken @hitecSmartHome : note sure DefaultHeaders::Instance().addHeader() is a good solution because it adds overhead on all requests including ones where CORS is not needed.

We only need to answer to CORS preflight request (OPTIONS) with 4 headers including a max age to avoid the browser to resend the request 5 sec later.

This should be enough:

server.on("/*", HTTP_OPTIONS, [](PsychicRequest* request) {
      PsychicResponse response(request);
      response.addHeader("Access-Control-Allow-Origin", "*");
      response.addHeader("Access-Control-Allow-Methods", "*");
      response.addHeader("Access-Control-Allow-Headers", "*");
      response.addHeader("Access-Control-Max-Age", "86400");
      response.setCode(200);
      return response.send();
    });

Also not that to allow everything, Access-Control-Allow-Headers and Access-Control-Allow-Methods should be set to *.

Also, pay attention that whatever solution we provide, it must be customisable by the user, because it really depends on the application.

That is why I was saying earlier:

just a global handler on OPTIONS and add the headers back

To be, the code above, which only ned to be documented, would be enough. No need to add specific things in the framework for now.

If we really want to help users, we could do a CORSEndpoint maybe ? new CORSEndpoint(origin, methods, headers, maxAge) to facilitate user's life..

Refs:

• https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Access-Control-Allow-Methods
• https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Access-Control-Allow-Headers etc

[hitecSmartHome]

That is true. Would be good to be able to add an OPTIONS endpoint

[hitecSmartHome]

But it should work for websockets and events too :/

[hoeken]

@mathieucarbou @hitecSmartHome okay, i looked into this a bit more. i see a way forward.

1. i added request->addResponseHeader() and request->getResponseHeaders() to the PsychicRequest object. these get copied into whatever PsychicResponse object is created down the line and sent to the user.

2. CORS does a preflight with OPTIONS, but sometimes it just sends the Origin: header in a get request. I'd like a way to handle all of this. We can create a simple filter to check that header and add the CORS stuff.

bool PERMISSIVE_CORS(PsychicRequest* request)
{
  if (request->hasHeader("Origin")) {
    DefaultHeaders::Instance().addHeader("Access-Control-Allow-Origin", "*");
    DefaultHeaders::Instance().addHeader("Access-Control-Allow-Methods", "GET, POST, PUT, DELETE, OPTIONS");
    DefaultHeaders::Instance().addHeader("Access-Control-Allow-Headers", "Content-Type, Authorization, Accept");
    DefaultHeaders::Instance().addHeader("Access-Control-Max-Age", "86400");
  }

  return true;
}

3. The problem with that, is that currently it is only possible to set a single filter on an endpoint or handler.

For the solution, I think we should do 2 things:

1. Extend the internal filter stuff to be a list of callbacks instead of a single callback. You can then call ->setFilter() multiple times. Filters are added in the order they are added. Filter processing will stop as soon as there is a filter that returns false.

2. I think we need a global server.setFilter() option that would work the same, but just at the server level. It would be the very first thing the server checks on a request. If you have dozens of endpoints, you want to be able to apply this to all of them. Same internal stucture of a list of callbacks.

I think it will be pretty straightforward to implement and allow some pretty flexible processing that verges on middleware.

[hoeken]

Here's a simple html page that will trigger CORS with a GET:

<!DOCTYPE html>
<html lang="en">
<head>
    <meta charset="UTF-8">
    <meta name="viewport" content="width=device-width, initial-scale=1.0">
    <title>Fetch Image Example</title>
    <style>
        img {
            max-width: 100%;
            height: auto;
        }
    </style>
</head>
<body>
    <h1>Image Fetch Example</h1>
    <div id="image-container"></div>

    <script>
        // URL of the image
        const imageUrl = 'http://psychic.local/alien.png';

        // Function to fetch and display the image
        async function fetchAndDisplayImage(url) {
            try {
                const response = await fetch(url);
                if (!response.ok) {
                    throw new Error(`Network response was not ok: ${response.statusText}`);
                }
                const blob = await response.blob();
                const imageUrl = URL.createObjectURL(blob);
                const imgElement = document.createElement('img');
                imgElement.src = imageUrl;
                document.getElementById('image-container').appendChild(imgElement);
            } catch (error) {
                console.error('Error fetching the image:', error);
            }
        }

        // Call the function
        fetchAndDisplayImage(imageUrl);
    </script>
</body>
</html>

[hoeken]

then in my server setup there is this:

    PsychicStaticFileHandler* handler = server.serveStatic("/", LittleFS, "/www/");
    handler->setFilter(PERMISSIVE_CORS);

    // this will send CORS headers on options requests
    server.on("*", HTTP_OPTIONS, [](PsychicRequest* request) { return request->reply(200); })
      ->setFilter(PERMISSIVE_CORS);

we really need the chain filtering, since the StaticFileHandler gets used with handler->setFilter(ON_STA_FILTER); quite a bit.

[mathieucarbou]

5. The problem with that, is that currently it is only possible to set a single filter on an endpoint or handler.

For the solution, I think we should do 2 things:

1. Extend the internal filter stuff to be a list of callbacks instead of a single callback. You can then call ->setFilter() multiple times. Filters are added in the order they are added. Filter processing will stop as soon as there is a filter that returns false.
2. I think we need a global server.setFilter() option that would work the same, but just at the server level. It would be the very first thing the server checks on a request. If you have dozens of endpoints, you want to be able to apply this to all of them. Same internal stucture of a list of callbacks.

You are essentially describing the FilterChain and RequestFlter design I talked about in Discord...

IMO, one way or another, this design is required and has a lot of benefits, one being to avoid duplicating auth (username passowrd) on each endpoints for example. Only one RequestFilter for authc or CORS can be created and held by the user, and added to many endpoints.

A FilterChain is a sequence of several RequestFilters, a RequestFilter takes as parameter a request and a pre-built response (to be able to act on it, add headers ,etc) and has the ability to stop the chain and commit the response eagerly. The FilterChain ends by giving back control to the handler which has the responsibility to handle app specific code linked to the request.

[hoeken]

@mathieucarbou now that i've studied this a bit, i understand it a lot better. i would like to see this added for sure. to do it properly will require some breaking changes.

As an intermediary step, i've added the filter chaining to both PsychicHandler and PsychicServer. It was pretty easy and is working nicely.

Here is how the code that handles CORS is currently setup:

bool PERMISSIVE_CORS(PsychicRequest* request)
{
  if (request->hasHeader("Origin")) {
    request->addResponseHeader("Access-Control-Allow-Origin", "*");
    request->addResponseHeader("Access-Control-Allow-Methods", "GET, POST, PUT, DELETE, OPTIONS");
    request->addResponseHeader("Access-Control-Allow-Headers", "Content-Type, Authorization, Accept");
    request->addResponseHeader("Access-Control-Max-Age", "86400");
  }

  return true;
}

// this will send CORS headers on every request that contains the Origin: header
server.setFilter(PERMISSIVE_CORS);

// this will respond to CORS requests (note: the global server filter will automatically add the CORS headers)
server.on("*", HTTP_OPTIONS, [](PsychicRequest* request) { return request->reply(200); });

To do it properly, I think there are 2 big challenges:

1. the PsychicHttpRequestCallback function definition will need to be modified to add a PsychicResponse object. This will break lots of code beyond our control.

2. There are many different types of PsychicResponse objects that currently are created independently by the users onRequest callback functions. C++ inheritance gives me a headache, but we will need to find a way to allow the user to easily take the existing PsychicRequest object, 'extend' it, then use it. I'm not really sure how to do this, but I'm also not a C++ OOP expert by any stretch.

[mathieucarbou]

1. the PsychicHttpRequestCallback function definition will need to be modified to add a PsychicResponse object. This will break lots of code beyond our control.

I don't think so: I think it might be possible to overload and support both in a union, like I did in my previous PR. Either you use a callback with the response, or the other one, or none. IF we really want to keep those. Like already said, I do not ee any value of the current Filter implementation. This should be something like ActivateFilter that takes no argument and returns a bool IMO.

There are many different types of PsychicResponse objects that currently are created independently by the users

I saw. Personally I am not fan of this part and I think this is a a bullet that we shoot ourselves in the foot. This gives too much flexibility where it might not be required and keeping backward compat' will be a hell if extension si allowed.

IMO the only way to derive a response should be from a request, like with the beginReply() methods.

Through a filter chain, the initial response could be created as equivalent as beginReply(404) (equivalent of not found), then passed onto the chain.

The response object needs to be flexible enough to support different types of response and I think this is doable, whether the answer is bytes, chars, json, message pack, or a stream.

[hoeken]

We're starting to get a bit off-track from the CORS discussion. I created issue #161 to discuss how to best implement the middleware functionality.