Handle OPTIONS requests in gateway

ChrisSwinchatt commented 5 years ago

(This is kind of a combined bug report and feature request; apologies if it's confusing.)

When using CORS with Ocelot and a bunch of microservices, we would like to have Ocelot handle all the CORS stuff and disable it in the downstream services.

I've tried to achieve this a few ways:

Have Ocelot restrict CORS to desired origins and the downstream services allow everything
Have Ocelot restrict CORS to desired origins and spoof the Origin header so we can disable CORS in the downstream services, with Ocelot forwarding OPTIONS requests to the downstream services to handle (this doesn't make sense in hindsight)
Have Ocelot restrict CORS to desired origins and spoof the Origin header so we can disable CORS in the downstream services, with Ocelot responding to OPTIONS requests itself based on the configuration

Expected Behavior

Ocelot would pass the CORS request on, the downstream service would return Access-Control-Allowed-Origin *, which Ocelot would replace with the correct value (the origin that was allowed)
Same as above
Ocelot would handle the OPTIONS request itself given it knows what methods and origins are allowed

Actual Behavior

The Access-Control-Allowed-Origin header returned to the client contains * which is insecure
The downstream services seem to return the correct Access-Control-Allowed-Origin header, but return 405 to the OPTIONS request because the Asp.Net Core CORS middleware isn't running
Ocelot returns 404 to the OPTIONS request because it's not specified in the configuration

I think 3 would be the best solution. I can probably simulate this by writing a middleware of my own but think this should be handled by Ocelot.

ChrisSwinchatt commented 5 years ago

I've solved this problem using a PreErrorResponderMiddleware for now.

Code for anyone else with the same problem: https://pastebin.com/T2rMDPuT

stefankip commented 5 years ago

With latest ocelot it doesn't work in the PreErrorResponderMiddleware delegate, because the context.DownstreamReRoute property is null. I used the PreQueryStringBuilderMiddleware delegate instead.

ChrisSwinchatt commented 5 years ago

Thanks @stefankip

WeihanLi commented 5 years ago

You can define a custom Middleware to set cors reponse header behine the HttpRequester.

sample code here:

CorsMiddleware:

    public class CorsMiddleware : OcelotMiddleware
    {
        private readonly GatewayOptions _gatewayOptions;
        private readonly OcelotRequestDelegate _next;

        public CorsMiddleware(OcelotRequestDelegate next, IOptions<GatewayOptions> options, IOcelotLoggerFactory loggerFactory)
            : base(loggerFactory.CreateLogger<UrlBasedAuthenticationMiddleware>())
        {
            _next = next;

            _gatewayOptions = options.Value;
        }

        public async Task Invoke(DownstreamContext context)
        {
            context.DownstreamResponse.Headers.Add(string.IsNullOrWhiteSpace(_gatewayOptions.AllowedOrigins)
                ? new Header(HeaderNames.AccessControlAllowOrigin, new[] { "*" })
                : new Header(HeaderNames.AccessControlAllowOrigin,
                    _gatewayOptions.AllowedOrigins.Split(new[] { ',', ';' }, StringSplitOptions.RemoveEmptyEntries)));

            context.DownstreamResponse.Headers.Add(new Header(HeaderNames.AccessControlAllowHeaders,
                new[] { "*" }));
            context.DownstreamResponse.Headers.Add(new Header(HeaderNames.AccessControlRequestMethod,
                new[] { "*" }));

            await _next(context);
        }

Ocelot Pipeline:

// ...
// We fire off the request and set the response on the scoped data repo
builder.UseMiddleware<HttpRequesterMiddleware>();
// cors
builder.UseMiddleware<CorsMiddleware>();

------- update ------ btw, use Ocelot version at 13.2 or above

raman-m commented 8 months ago

@ChrisSwinchatt commented Mar 5, 2019

Hello, Chris! If you want new feature, then you need to contribute! We have no additional developers. Our team capacity is low, only 2 developers on part-time.

raman-m commented 8 months ago

@WeihanLi commented May 22, 2019

Hi Weihan! Nice code sample! Could we develop your idea more?

raman-m commented 6 months ago

@WeihanLi Welcome back to Ocelot world! :wink: Seems the author @ChrisSwinchatt is not with Ocelot anymore... So, Weihan, do you have intention to contribute? Will you open a PR with your own vision for feature design? We need to care about 3 scenarios by the author, and + your own design enhancements.

Here are my answers

the main idea is to set the cors headers for a options request so that the client could continue with the following request such as GET/POST

Why GET and POST only? It should work for any verb I guess which are defined in the route

Microsoft.AspNetCore.Cors.Infrastructure.CorsMiddleware

I've reviewed the middleware quickly and... the class methods are not virtual, impossible to override, and I have no whole picture how we could reuse it. We cannot make reflection calls for sure, and the last chance to reuse the functionality :point_down:

copy & and paste the current class implementations and re-develop the code for our needs
define Sidecar, or Decorator, or Adapter GoF pattern class and use CorsMiddleware class as an aggregated service object with decorated behavior by our needs.

To be discussed... Also, our team needs to look into Yarp code... I'm curious how did Yarp team designed processing OPTIONS. We can borrow ideas :stuck_out_tongue_winking_eye:

The previous code sample is going to allow any origin, any http method and any headers, maybe we could reuse the cors policy defined with AddCors, then support configuring the route cors policy, configure the cors headers when necessary

Yep, it is a good start, we can develop more your old code sample... I'm sure we need to re-use MS native implementation somehow.

Or we could forward the options request to the downstream also? when downstream does not support OPTIONS request, configure response with default cors headers

We have to consider all 3 scenarios by the author! In one of those scenarios Opt request is forwarded to downstream service to make further decisions based on response. To be fair, we must forward because no other ways to check the service by OPTIONS. Regarding default behavior, I'm open for discussions...

raman-m commented 6 months ago

@ggnaegi @RaynaldM Our community expresses high interest in correct processing of OPTIONS requests. There are 2-3 issues now which are related to preflight. I will find then later. So, seems we need to assign high priority, or at least to add the issue to upcoming releases. Also, a design of feature can be dependent with Health Check feature. So, in Load Balancer scenarios seems we have not to forward OPTIONS at all.

RaynaldM commented 6 months ago

To support the OPTIONS and pre-flight request, we just add an AddCors

        services.AddCors();
        services.AddOcelot(_config)

        // global cors policy
        app.UseCors(static x => x
            .AllowAnyOrigin()
            .AllowAnyMethod()
            .AllowAnyHeader()
            .DisallowCredentials()
        );

raman-m commented 6 months ago

@WeihanLi commented on Mar 17

Or we could forward the options request to the downstream also? when downstream does not support OPTIONS request, configure response with default CORS headers

Good idea! We could implement it as a part of Health Checks feature and/or as separate one.

raman-m commented 3 months ago

@ChrisSwinchatt Your answer is :point_right: RaynaldM commented on Mar 18, 2024

ThreeMammals / Ocelot