Improve access_denied handling

[kevinchalet]

Moved from aspnet-contrib/AspNet.Security.OAuth.Providers#81.

---

Not sure if this has been addressed yet:

ASP.NET Core RC1 Sample application (VS 2015 Community - not update 1)

project.json (public nuget)
"AspNet.Security.OAuth.BattleNet": "1.0.0-alpha3",

Clean user - never been to site - no identity db entries - no battle.net correlation to OAuth Application (Battlenet/identity account combo have never seen this application).

Reproduction:

1. Log In (upper right nav)
2. Log Into Battle.Net (main body link)
3. Press Cancel button (On Battle.Net's signin page)
   3a) (note) unselecting wow-profile and clicking continue works as expected

Redirects to our /signin-battlenet(?) which is the standard url for a successful signin and is handled by the 3rd party provider... 3rd party provider does not properly pick up on the "cancel" and throws a 500 error. Regardless of Debug mode, details of this error are not provided.

• Is a redirect to me happening beyond /signin-battlenet that I haven't detected? Let me know....
• Or is it an issue with the base OAuth Provider? (quickly! RC2 is imminent ;)

Startup ConfigureServices:

           services.AddIdentity<ApplicationUser, IdentityRole>(options => {
                options.Cookies.ApplicationCookie.ExpireTimeSpan = TimeSpan.FromDays(29);
            })
            .AddEntityFrameworkStores<ApplicationDbContext>()
            .AddDefaultTokenProviders();

Startup Configure:

        app.UseBattleNetAuthentication(options =>
        {
            options.SaveTokensAsClaims = true;
            options.Scope.Add("wow.profile");
            options.ClientSecret = Configuration["xxxxxxxxx:BattleNet:Secret"];
            options.ClientId = Configuration["xxxxxxxxx:BattleNet:Key"];
        });

        app.UseCookieAuthentication(options =>
        {
            options.AutomaticAuthenticate = true;
            options.AuthenticationScheme = "PublicAuth";
            options.ClaimsIssuer = "xxxxxxxxx";
            options.CookieSecure = CookieSecureOption.Never;
            options.ExpireTimeSpan = TimeSpan.FromDays(30);
        });

I doubt the CookieAuth is conflicting - it's a separate cookie, separate auth process that brings insecure claims down to where I can access certain info without https. No calls are made in regards to this prior to battle.net redirecting back to /signin-battlenet.

[kevinchalet]

Or is it an issue with the base OAuth Provider? (quickly! RC2 is imminent ;)

Yep, though it's actually by-design: RemoteAuthenticationHandler and OAuthHandler (the classes that handle the callback path dance) don't consider access_denied as a special error and simply return a 500 response to indicate that the external authentication process failed.

In RC2, the handler no longer returns a 500 response but directly throws an exception. If the exception is not caught by one of the middleware registered before the social provider, the server is responsible of intercepting it and returning a 500 response.

To stop using the default logic (which is... well; not super user-friendly 😄), the recommended approach is to use the RemoteError event to redirect the user agent to an error page.

app.UseBattleNetAuthentication(options => {
    options.SaveTokensAsClaims = true;
    options.Scope.Add("wow.profile");
    options.ClientSecret = Configuration["xxxxxxxxx:BattleNet:Secret"];
    options.ClientId = Configuration["xxxxxxxxx:BattleNet:Key"];
    options.Events = new OAuthEvents {
        OnRemoteError = context => {
            context.Response.Redirect("/oauth-error-page");
            context.HandleResponse();

            return Task.FromResult(0);
        }
    };
});

That said, I agree the default experience sucks. I'm pretty sure we had plans to introduce an AccessDeniedPath property, but I can't find the corresponding work item.

I'll move your ticket to the aspnet/Security repository.

[kevinchalet]

/cc @thewyzard44 @HaoK @Tratcher

[thewyzard44]

Ahh OK so its returning 500 rather than throwing? Got it ;). No wonder the debug page never showed up. If rc2 does throw now then that'll help with diagnosis in the future. And wiring up that event sounds perfect. Thank you!!

[Eilon]

To do this properly would require a fair bit of design. We'd need a way to perhaps redirect or re-execute another page but we need a way to pass the error state to that page in order for it to do anything meaningful. For now using the event with the code above should be fine.

[kevinchalet]

A simple design might be to add a new AccessDeniedPath property, intercept access_denied errors and redirect the user agent to this access denied page.

[Tratcher]

@PinpointTownes give it a try and see how it works.

[NikasZalias]

@PinpointTownes your answer saved me! I was looking for an answer to this question for a week! :))

[Tratcher]

Closing as duplicate of #1165