Make IsAjaxRequest overridable or configurable

[TomGroeneboer]

Hi guys,

We are using the Cookie authentication in our application. This application makes use of API requests (as you do) and therefore the redirect to a login page is silly on these requests. However, we do not use jquery in our front-end. This causes the non-standard header X-Requested-With not to be set and that causes the API calls to redirect to the login page.

If we want our API requests to return a 401 instead of the redirect, we need to override the behavior in the OnRedirectToLogin, OnRedirectToLogout, OnRedirectToAccessDenied and OnRedirectToReturnUrl events.

Could you please make the IsAjaxRequest method overridable or make the behavior configurable in another way?

Thanks!

[davidfowl]

Can you set the header or do you have no control over the client?

[devatwork]

Making this configurable sounds good to me. One cannot always control the client and the header was never standardized. Also I've seen wild proxies eating this header.

The API change can be made non-breaking right?

[devatwork]

Note: If you're likely to approve this change, we'll be happy to create a PR.

[davidfowl]

@devatwork What would you make configurable?

[devatwork]

The logic in https://github.com/aspnet/Security/blob/rel/1.1.1/src/Microsoft.AspNetCore.Authentication.Cookies/Events/CookieAuthenticationEvents.cs#L104

I can see two ways of doing this:

1. Make IsAjaxRequest protected virtual allowing a subclass to override this.
2. Encapsulate the logic in a Func<HttpRequest, bool> which is set by default but allow overriding via a property. Similar to how OnRedirectToAccessDenied is already implemented.

I think the 2nd approach is more in line with the rest of the Option/Event classes in ASP.NET core.

The default behavior would still be the current X-Requested-With check, this will make this change non-breaking but still allow applications to override the behavior and implement it in any way they see fit.

[Tratcher]

If IsAjaxRequest were overridable, what would you change it to for your scenario?

[devatwork]

Depends on the application. A few months back we had to sniff the User-Agent header in order to correctly recognize a legacy application.

For another application we want the 'IsAjaxRequest' for all requests that run within the /api path. We currently solved this by overriding all the On* methods of CookieAuthenticationEvents with the exact same behavior as the ASP.NET Core impl but with the following check request.Path.StartsWithSegments("/api").

IMHO it is very likely that the current ASP.NET Core impl will need to be changed/overriden more often by consumers of that API in the future because the recently introduced Fetch standard does not prescribe the header for example.

Unfortunately, there is, at least to my knowledge, a standard to reliably detect whether something is an AJAX request or not.

Now there already is a way around this in the current API, one can simply overload all the On* methods on the *Events class. However, that leads to a lot of code duplication and maintenance burden.

Therefore I would prefer to be able to inject just the IsAjaxRequest detection strategy because that part is specific to my applications.

Hope this reasoning and the provided examples help. I'm looking forward to understand your concerns and reasoning.

[Tratcher]

Makes sense. That said, this is not going to be a high priority for us any time soon. You can already get the behavior you need by overriding the events, and we have several critical items we need to get done first.

[devatwork]

I understand this is not a high priority: noone is blocked because there is a workaround. So we are just talking about ergonomics here. I also understand that exposing a new public API needs careful evaluation.

Having said that, if I, or someone else, were to send a PR, are you likely to accept it? Of course assuming the quality requirements are met. Also I'm not saying it should be merged before the next release, I'm fine if it is merged for major+1 for example.

If so, is there a specific guidance you can provide?

[Tratcher]

You can send a PR after #1170 is complete, it's a major overhaul that would cause conflicts.

[Eilon]

Closing because there are no plans to implement this.