CORS settings in default CORS middleware

[zekageri]

Platform

ESP32

IDE / Tooling

pioarduino

What happened?

When testing, I often run my react app in dev mode on localhost.
The frontend app api points to my esp32 ip address in this case, which is in the local network.
The following ( recommended ) cors config in this case is the following

corsMiddleware.setHeaders("Origin, X-Requested-With, Content-Type, Accept, Authorization, SysAuthorization");
corsMiddleware.setMethods("GET,HEAD,OPTIONS,POST,PUT,DELETE");
corsMiddleware.setOrigin("*");

When _credentials is true, the browser does not let "*" wildcard origin and it rejects the OPTIONS request. This is expected browser behaviour.
In order for this to work properly, we must set the origin to the request origin. This can not be done globally, but on individual requests.
I have cloned the AsyncCorsMiddleware and added a void checkAllowAnyOrigin(AsyncWebServerRequest *request, AsyncWebServerResponse *response); method. This method sets the Origin header to the request Origin, passes the browser check.

void ClonedCorsMiddleware::checkAllowAnyOrigin(AsyncWebServerRequest *request, AsyncWebServerResponse *response) {
    if (allowAnyOrigin) {
        std::string incomingOrigin = request->getHeader(asyncsrv::T_CORS_O)->value().c_str();
        response->addHeader(asyncsrv::T_CORS_ACAO, incomingOrigin.c_str());
    }
}

void ClonedCorsMiddleware::run(AsyncWebServerRequest *request, ArMiddlewareNext next) {
    // Origin header ? => CORS handling
    if (request->hasHeader(asyncsrv::T_CORS_O)) {
        // check if this is a preflight request => handle it and return
        if (request->method() == HTTP_OPTIONS) {
            AsyncWebServerResponse *response = request->beginResponse(200);
            addCORSHeaders(response);
            checkAllowAnyOrigin(request, response);
            request->send(response);
            return;
        }

        // CORS request, no options => let the request pass and add CORS headers after
        next();
        AsyncWebServerResponse *response = request->getResponse();
        if (response) {
            addCORSHeaders(response);
            checkAllowAnyOrigin(request, response);
        }

    } else {
        // NO Origin header => no CORS handling
        next();
    }
}

This is a hack right now because the _origin is set twice in the run method. I suggest a separate bool variable which can be toggled ( like allowAnyOrigin or allowCurrentOrigin ) which would allow the current Origin in the request.

Stack Trace

The code does not crash but browser does not care about the wildcard Origin header when credentials are true.

Minimal Reproductible Example (MRE)

Use the default CORS middleware with _credentials = true and try to reach the esp32 from an other ip address using origin as "*"

I confirm that:

• I have read the documentation.
• I have searched for similar discussions.
• I have searched for similar issues.
• I have looked at the examples.
• I have upgraded to the lasted version of ESPAsyncWebServer (and AsyncTCP for ESP32).

[mathieucarbou]

Thanks! Yes definitely something to improve...

So if I summarize:

• when _credentials is false, setting origin to * will work and allow any oriign
• when _credentials is true, * does not work, so in order to allow any origin, we have to send back in the response header the same origin as the one requested

?

[zekageri]

Yes

[zekageri]

This is ofc insecure because it will allow any origin so I suggest to indicate somewhere to use an array of origins to allow requests.
The most user friendly way would be to expose a "allowAnyOrigin()" method on the cors middleware which would internally check if the _credentials is true or not. If it is true and _allowAnyOrigin is also true, set the request Origin instead of the configured _origin

[zekageri]

This is the exact message which the browser prints to the console

(index):1 Access to XMLHttpRequest at 'http://192.168.1.14/test' from origin 'http://localhost:5173' has been blocked
by CORS policy: The value of the 'Access-Control-Allow-Origin' header in the response must not be the wildcard '*' when the
request's credentials mode is 'include'. The credentials mode of requests initiated by the XMLHttpRequest is controlled by the
withCredentials attribute.

[mathieucarbou]

This is ofc insecure because it will allow any origin so I suggest to indicate somewhere to use an array of origins to allow requests.

Of course!
But this would be the equiavlent as _credentials == false && _origin == *

The most user friendly way would be to expose a "allowAnyOrigin()" method on the cors middleware which would internally check if the _credentials is true or not. If it is true and _allowAnyOrigin is also true, set the request Origin instead of the configured _origin

Coulnd't we reuse the _origin value ?

Right now, the defaults are these:

String _origin = "";
String _methods = "";
String _headers = "*";
bool _credentials = true;
uint32_t _maxAge = 86400;

To me there are 2 cases:

• If the user sets credential to false and origin to *, then we just send back Access-Control-Allow-Origin: *

• If the user sets credential to true and origin to *, then we send back the Origin header

An optimization for the user to avoid too many CORS checks would be to configure the origins (setOrigin()) to a valid list

What do you think ?

[mathieucarbou]

@zekageri : would you be able to point to https://github.com/ESP32Async/ESPAsyncWebServer#issues/294 to test PR #295 and report back ? I have implemented the idea above, which is to allow everything when origin is set to *, and depending on the credential flasg, we allow everything but by using different way to answer in the haders.

[zekageri]

Checking

[zekageri]

This is ofc insecure because it will allow any origin so I suggest to indicate somewhere to use an array of origins to allow requests.

Of course! But this would be the equiavlent as _credentials == false && _origin == *

Yes, that is true.

if (request != nullptr && _credentials && _origin == "*") {
  // cannot use wildcard when allowing credentials
  response->addHeader(asyncsrv::T_CORS_ACAO, request->header(asyncsrv::T_CORS_O).c_str());
} else {
  response->addHeader(asyncsrv::T_CORS_ACAO, _origin.c_str());
}

This is the best way, I'm currently testing