HangfireIO / Hangfire

An easy way to perform background job processing in .NET and .NET Core applications. No Windows Service or separate process required
https://www.hangfire.io
Other
9.4k stars 1.7k forks source link

Hangfire 1.6.20 AntiForgeryToken breaks dashboard on load balanced servers #1248

Closed steveland83 closed 5 years ago

steveland83 commented 6 years ago

Hangfire.Core: 1.6.20

As of 1.6.20, the hangfire dashboard implemented AntiForgeryToken validation on its controllers, however the current implementation breaks the dashboard when running across multiple load balanced servers, resulting in 403 responses whenever requests are not routed to the same server that served the initial page.

The issue was introduced by this commit: https://github.com/HangfireIO/Hangfire/commit/50cb3b91c055aa15b29dd01b94ae664f8c21ceb4

Using a sticky load balancer works around the issue, but perhaps providing a configuration option would be a fairly quick fix - e.g. following the same pattern as

services.AddMvc(options => { options.Filters.Add(new IgnoreAntiforgeryTokenAttribute()); })

so that we could specify something like:

app.UseHangfireDashboard("hangfire", new DashboardOptions() {Filters = new[] {new IgnoreAntiforgeryTokenAttribute()}});

steveland83 commented 6 years ago

I've had a quick look at how this could be implemented. Hangfire is pulling the available implementation of the standard .Net IAntiforgery type from HttpContext.RequestServices.GetService<IAntiforgery>()

The above means that adding a property for FilterCollection Filters to the DashboardOptions and propogating any added Filters through to the underlying Owin middleware should handle both disabling and overriding of the default AntiforgeryToken behaviour fairly transparently. One snag is that there is no existing dependency on Microsoft.AspNetCore.Mvc.Core, which would be needed to do this.

@odinserj do you have any comments/preferences/suggestions on how you would want this to be handled (if at all)?

vonkertis commented 6 years ago

I encounter the same issue with dashboards after upgrading to 1.6.20 version, but with Hangfire running in a docker container. Downgrading to 1.6.19 helped :(

simonjduff commented 6 years ago

We've hit this as well. Downgrading temporarily until either we can share keys between machines or disable antiforgery.

dbredvick commented 6 years ago

We are reverting our version until this is fixed.

rfrancisco commented 6 years ago

Any updates regarding this?

pieceofsummer commented 6 years ago

You should set a matching machineKey on all your servers for anti-forgery tokens (and also Forms authentication) to work correctly. This isn't something specific to Hangfire itself.

simonjduff commented 6 years ago

@pieceofsummer I don't believe machineKeys are supported in dotnet core, and certainly not on *nix.

pieceofsummer commented 6 years ago

@simonjduff on .NET Core you need to configure Data Protection instead. It's just nobody has mentioned Core in the discussion, so I've assumed we were talking about IIS hosting.

simonjduff commented 6 years ago

@pieceofsummer setting up data protection is a bigger piece of work than we can engage with at the moment. This is the reason for the downgrade, until either we can disable the antiforgery requirements (service runs in a private network with security groups allowing access to specific service accounts), or until we can share keys via data protection.

pieceofsummer commented 6 years ago

@simonjduff the default data protection settings are only appropriate for single-machine configurations, so you would still need to configure it when doing a multi-machine setup. Not only for antiforgery, but also for authentication and sessions to work – I doubt you have a load-balanced web farm running solely for a Hangfire dashboard :)

simonjduff commented 6 years ago

@pieceofsummer This is an api not a web application, so the only session, authentication etc is for the hangire dashboard attached to the api. We could move the dashboard to its own service, but that's not the configuration at the moment, each api has its dashboard cohosted.

steveland83 commented 6 years ago

That's a similar setup to what we have - .Net core Middleware applications accessed by api only, with the exception of the hangfire dashboard. In our case we always have two servers behind a load balancer to ensure redundancy and allow rolling deployments, even though the load on most of the services is generally minimal.

On Wed, 17 Oct 2018, 07:38 Simon Duff, notifications@github.com wrote:

@pieceofsummer https://github.com/pieceofsummer This is an api not a web application, so the only session, authentication etc is for the hangire dashboard attached to the api. We could move the dashboard to its own service, but that's not the configuration at the moment, each api has its dashboard cohosted.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/HangfireIO/Hangfire/issues/1248#issuecomment-430578719, or mute the thread https://github.com/notifications/unsubscribe-auth/AKnnrwvyhGxD0iKaHotOW9_jbWpY2zPEks5ulwiWgaJpZM4WBfv0 .

pieceofsummer commented 6 years ago

Even if the authentication is only for Hangfire dashboard, it still needs to be configured to share keys across instances, so the cookie/token generated by one instance would be accepted by any of them.

You already deploy your app on servers, or synchronizing appconfig.json changes between machines, so it probably won’t be a problem copying the keys as well. Or keep them on a network share/Azure/whatever.

simonjduff commented 6 years ago

My point is that there's a hurdle here for implementation. Yes, we can solve these problems with time and effort. We would prefer not to, spend that time and effort though. Hence the downgrade until we have the time and effort available, or until the problem goes away with a configuration switch. Whichever comes first.

tafs7 commented 5 years ago

any updates on this issue?

is the final word: "works as intended - to resolve you must configure data protection per ASP.NET Core docs"?

bakester14 commented 5 years ago

This still appears to be an issue, anyone found the right thing to do yet? Novice at Antiforgery/DataProtection

PauliusLe commented 5 years ago

Possible workaround if you don't use default AntiForgery protection. Remove IAntiforgery from ServiceCollection and it will skip validation: https://github.com/HangfireIO/Hangfire/blob/053afff4b7fcfa2be060760b14879c5ac7a93204/src/Hangfire.AspNetCore/Dashboard/AspNetCoreDashboardMiddleware.cs#L79 Just add services.Remove(services.First(x => x.ServiceType == typeof(IAntiforgery))); after services.AddMvc()

danroot commented 5 years ago

I'm running into this also attempting to use OIDC auth even on the development server (no load balancing). I am able to auth and GET to the dashboard, but any POST/DELETE/etc. returns a 403. Debug shows it's the anti-forgery code complaining that the token was for another user. I am able to do non-hangfire POSTs just find, so I suspect it's a combination of hangfire's auth and antiforgery. Any help getting past this would be appreciated.

bakester14 commented 5 years ago

@danroot If you don't require antiforgery see the workaround in the comment just above yours. Otherwise downgrading to earlier versions of hangfire has worked for others and for me.

danroot commented 5 years ago

I tried @PauliusLe 's fix, but got an error - Mvc apparently still tried to issue a token and failed when no service was found. So as a temporary fix I made a fake implementation of IAntiforgery. I really don't like that though - ideally we would keep antiforgery untouched hangfire would work. I really think there is a bug when using a custom auth filter and >= 1.6.2. The error I get in this scenario is:

Antiforgery validation failed with message 'The provided antiforgery token was meant for a different claims-based user than the current user.'

latop2604 commented 5 years ago

Instead of removing the service, you can create a custom IAntiForgery implementation. And return true in all validation method. In the following sample I use the decorator patern to reuse the default implementation.

public class DisabledAntiforgery : IAntiforgery
    {
        private readonly IAntiforgery _baseAntiforgery;

        public DisabledAntiforgery(IAntiforgery baseAntiforgery)
        {
            _baseAntiforgery = baseAntiforgery ?? throw new ArgumentNullException(nameof(baseAntiforgery));
        }

        public AntiforgeryTokenSet GetAndStoreTokens(HttpContext httpContext)
        {
            return _baseAntiforgery.GetAndStoreTokens(httpContext);
        }

        public AntiforgeryTokenSet GetTokens(HttpContext httpContext)
        {
            return _baseAntiforgery.GetTokens(httpContext);
        }

        public Task<bool> IsRequestValidAsync(HttpContext httpContext)
        {
            return Task.FromResult(true);
        }

        public void SetCookieTokenAndHeader(HttpContext httpContext)
        {
            _baseAntiforgery.SetCookieTokenAndHeader(httpContext);
        }

        public Task ValidateRequestAsync(HttpContext httpContext)
        {
            return Task.CompletedTask;
        }
    }
steveland83 commented 5 years ago

Looks like this is now handled - for .Net Core :)

The release notes for https://github.com/HangfireIO/Hangfire/releases/tag/v1.7.1 include:

Added – DashboardOptions.IgnoreAntiforgeryToken property to disable token validation in Dashboard UI.

So from the looks of things you should now be able to configure hangfire to ignore AntiForgery Tokens with:

app.UseHangfireDashboard("/hangfire", new DashboardOptions
            {
                Authorization = new[] {new HangfireAuthFilter()},
                IgnoreAntiforgeryToken = true                                 // <--This
            });