Ocelot icon indicating copy to clipboard operation
Ocelot copied to clipboard

Handle OPTIONS requests in gateway

Open ChrisSwinchatt opened this issue 5 years ago • 12 comments

(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:

  1. Have Ocelot restrict CORS to desired origins and the downstream services allow everything
  2. 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)
  3. 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

  1. 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)
  2. Same as above
  3. Ocelot would handle the OPTIONS request itself given it knows what methods and origins are allowed

Actual Behavior

  1. The Access-Control-Allowed-Origin header returned to the client contains * which is insecure
  2. 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
  3. 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 avatar Mar 05 '19 15:03 ChrisSwinchatt

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

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

ChrisSwinchatt avatar Mar 06 '19 09:03 ChrisSwinchatt

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

stefankip avatar Apr 04 '19 07:04 stefankip

Thanks @stefankip

ChrisSwinchatt avatar Apr 04 '19 09:04 ChrisSwinchatt

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

WeihanLi avatar May 22 '19 02:05 WeihanLi

@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 avatar Jan 14 '24 10:01 raman-m

@WeihanLi commented May 22, 2019

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

raman-m avatar Jan 14 '24 10:01 raman-m

Related

  • #993
  • #996

raman-m avatar Mar 16 '24 18:03 raman-m

@raman-m 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

https://github.com/dotnet/aspnetcore/blob/main/src/Middleware/CORS/src/Infrastructure/CorsMiddleware.cs

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

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

WeihanLi avatar Mar 17 '24 02:03 WeihanLi

@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 avatar Mar 17 '24 10:03 raman-m

@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.

raman-m avatar Mar 17 '24 10:03 raman-m

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()
        );

RaynaldM avatar Mar 18 '24 08:03 RaynaldM

@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 avatar Mar 25 '24 12:03 raman-m

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

raman-m avatar Jun 15 '24 12:06 raman-m