Add CORS support for MiniProfiler

[pmccloghrylaing]

Related to #332

MiniProfiler doesn't work out of the box when the UI application is on a different origin to the API application due to missing CORS support. In order to get MiniProfiler working for an Angular application running from a different origin I had to set up the following:

• Add CORS headers for MiniProfiler and handle OPTIONS requests so the MiniProfiler endpoints would succeed:

  app.Use(async (context, next) =>
  {
    if (context.Request.Path.StartsWithSegments(new PathString("/profiler"))) // Must match RouteBasePath
    {
        if (context.Request.Headers.TryGetValue("Origin", out var origin))
        {
            context.Response.Headers.Add("Access-Control-Allow-Origin", origin);
            if (context.Request.Method == "OPTIONS")
            {
                context.Response.StatusCode = 200;
                context.Response.Headers.Add("Access-Control-Allow-Headers", "Content-Type");
                context.Response.Headers.Add("Access-Control-Allow-Methods", "OPTIONS, GET");
                await context.Response.CompleteAsync();
                return;
            }
        }
    }
  
    await next();
  });
  app.UseMiniProfiler();

• Update our CORS policy to have .WithExposedHeaders("X-MiniProfiler-Ids"), otherwise X-MiniProfiler-Ids isn't available to the MiniProfiler UI:

  services.AddCors(options =>
  {
    options.AddPolicy("CorsPolicy",
        builder => builder
            .WithOrigins(...)
            .WithMethods(...)
            .AllowAnyHeader()
            .WithExposedHeaders("X-MiniProfiler-Ids"));
  });

• Add the MiniProfiler includes script to the UI application (data-path needed to include the host):

  <script async type="text/javascript" id="mini-profiler"
    src="http://localhost:15000/profiler/includes.min.js"
    data-path="http://localhost:15000/profiler/"
    data-position="Left"
    data-scheme="Light"
    data-controls="true"
    data-authorized="true"
  ></script>

Suggestions:

• Add an option to UseMiniProfiler for setting a CORS policy for MiniProfiler
• Add an extension method for CorsPolicyBuilder to Expose the X-MiniProfiler-Ids header
• Update documentation with steps to support CORS including script snippet

I'd be happy to open up a PR for this issue if the suggestions are OK.

[pmccloghrylaing]

To get this working for Angular, I also need to implement a custom HttpInterceptor. I tried setting window.angular = true but zone-js bypasses the patched XMLHttpRequest.prototype.send so MiniProfiler is unable to listen to the response.

Here's the custom HttpInterceptor:

import { HttpEvent, HttpHandler, HttpInterceptor, HttpRequest, HttpResponse } from '@angular/common/http';
import { Injectable } from '@angular/core';
import { NavigationStart, Router } from '@angular/router';
import { Observable } from 'rxjs';
import { filter, tap } from 'rxjs/operators';

declare const MiniProfiler:
  | {
      container?: HTMLElement;
      fetchResults(ids: string[]): void;
    }
  | undefined;

@Injectable()
export class HttpProfilerInterceptor implements HttpInterceptor {
  constructor(router: Router) {
    // Clear profiler results on navigation
    router.events.pipe(filter(ev => ev instanceof NavigationStart)).subscribe(() => {
      if (typeof MiniProfiler !== 'undefined' && MiniProfiler.container) {
        const clearButton = MiniProfiler.container.querySelector<HTMLSpanElement>(
          '.mp-controls .mp-clear',
        );
        if (clearButton) {
          clearButton.click();
        }
      }
    });
  }

  intercept(req: HttpRequest<any>, next: HttpHandler): Observable<HttpEvent<any>> {
    return next.handle(req).pipe(
      tap((event: HttpEvent<any>) => {
        // if the event is for http request
        if (event instanceof HttpResponse) {
          if (typeof MiniProfiler !== 'undefined') {
            try {
              const miniProfilerIds = event.headers.get('X-MiniProfiler-Ids');
              if (miniProfilerIds) {
                MiniProfiler.fetchResults(JSON.parse(miniProfilerIds));
              }
            } catch (err) {}
          }
        }
      }),
    );
  }
}

[NickCraver]

I'm not opposed to adding this - sounds like a useful feature if we can make it neat! I'm mostly unsure about the CORS header bits (need to poke more). For OPTIONS it seems like we'd just handle those in middleware if the new option is enabled, right? I'm not sure if we can do CORS policy there in middleware or have to globally register. Or is the CORS policy an issue because you're registering one at all? Just trying to understand the specifics more :)

[pmccloghrylaing]

These issues are because our UI and API have different origins - so all requests to the API are cross-origin. That means they need to handle the pre-flight OPTIONS request and set the necessary CORS headers for the requests to work at all.

• MiniProfiler doesn't send an OPTIONS response or set the necessary CORS headers for this - that can hopefully be done by using the existing CORS middleware (as opposed to the custom code I've posted). I'd like to be able to call something like app.UseMiniProfiler(c => c.CorsPolicy(...)) or app.UseMiniProfilerWithCorsPolicy(...).
• By default most headers aren't accessible for CORS requests, like the X-MiniProfiler-Ids header. Headers need to be "exposed" through the CORS policy - an extension method on the CorsPolicyBuilder would easily set that up - internally it would just do builder.WithExposedHeaders("X-MiniProfiler-Ids").

It would be useful to document this so anyone looking to do a similar setup can easily solve it - I think it's probably enough to give an example UI snippet.

I'm not sure what to do about Angular - that really seems to be an issue with zone-js, not MiniProfiler.

As I said, I'm happy to open a PR if that makes it easier to continue the discussion.

[NickCraver]

I need to fire this up local when time allows to better understand it. Is a CORS policy always in effect? We're not using that functionality atm, so not familiar with it. MiniProfiler middleware handles all requests to its routes though, so it should be able to respond to OPTIONS in there, just by the presence of a bool in the AspNetCore settings (rather than code elsewhere). That seems like the simplest solution here, but I'm not totally sure why X-MiniProfiler-Ids needs to be exposed, is that because your API calls are coming from a different domain than the page, so the header is being sent from elsewhere?

I'm not sure what the cleanest API is there, but definitely not for another top level method, it should be an option or on a specific builder (imagine if we have a .UseMiniProfilerWithX for every combination). You're more than welcome to PR this (always!), I just wanted to be very forward: I'm not sure what this API could/should look like for those pieces, may need to simplify there a bit.

[pmccloghrylaing]

A CORS policy isn't always in effect, it's only required and used when you serve cross-origin requests.

Here's the MDN article on exposed headers - the take away is this (for cross-origin requests):

If you want clients to be able to access other headers, you have to list them using the Access-Control-Expose-Headers header.

The reason I would use the existing CORS middleware is so you can specify which origins are allowed access and which aren't.

I agree with your point with using an option rather than a new top level method.

[NickCraver]

Gotcha - I propose: a new boolean on MiniProfilerOptions (specific to .AspNetCore) for EnableCors, by that alone we should be able to register a change to CorsOptions, via an IConfigureOptions<CorsOptions>. In .AddMiniProfiler() you can see existing pre-3.1 code for views which does similar.

This setup keeps the API super simple and we'll take care of the rest. I'm curious if that works though, or if it needs to be not a bool but a list of origins or some such. Thoughts on that API surface area?

[pmccloghrylaing]

It would be worth a try like that.

My example code opens MiniProfiler to all origins - are you concerned about doing that?

[NickCraver]

@pmccloghrylaing That's the part I mean about "maybe it's not a bool" - maybe it's something other type (e.g. list) to support multiple origins (and it not being there/null is "off")...whatever format we need to add a policy correctly (not in front of a comp to check APIs atm) - that make sense?

[pmccloghrylaing]

I've added a draft PR that still needs testing. @NickCraver I've added a single property to options and the rest is taken care of by the middleware. I didn't find that I could do much with IConfigureOptions<CorsOptions> as it only allows you to create or get named CORS policies - it doesn't provide access to all policies.