blowdart / AspNetAuthorizationWorkshop

A workshop for moving through the various new pieces in ASP.NET Core Authorization
Other
1.17k stars 245 forks source link

401 instead of 403 #4

Closed elmortega closed 8 years ago

elmortega commented 8 years ago

Hi,

I´m getting a 401 instead of 403 when using Azure Ad for authentication in aspnetcore.

I've set a global policy to RequireAuthenticatedUser, and it works just fine. When I access a any controller in my app, if the user is not authenticated it returns a 401.

I created a custom policy "AdminOnly" and applied it to a controller:

    [Authorize (Policy = "AdminOnly")]
    public class WebTestController : Controller
    {

If the user, is not authenticated, it redirects to https://login.microsoftonline.com/ as expected. If the user is authenticated and fulfills the policy requirements everything goes well, but if the user does not meet the requirements there's problems, and the problem depends on whether options.AutomaticChallenge = true or false. If true, the app get caught up in a loop:

Pieces from debug output:

Microsoft.AspNet.Mvc.Controllers.ControllerActionInvoker: Warning: Authorization failed for the request at filter 'Microsoft.AspNet.Mvc.Filters.AuthorizeFilter'.
Microsoft.AspNet.Authentication.OpenIdConnect.OpenIdConnectMiddleware: Information: AuthenticationScheme: OpenIdConnect was challenged.
Microsoft.AspNet.Mvc.ChallengeResult: Information: Executing ChallengeResult with authentication schemes ().

If options.AutomaticChallenge = false, it returns a 401

Any ideas how can I fix this?

Relevant pieces of Startup.cs

            services.AddAuthorization(options =>
            {
                options.AddPolicy("AdminOnly", policy => policy.Requirements.Add(new   AdminNameRequirement("Tyrion Lannister")));
            });

            services.AddMvc(config =>
            {
                var policy = new AuthorizationPolicyBuilder()
                    .RequireAuthenticatedUser()
                    .Build();
                config.Filters.Add(new AuthorizeFilter(policy));
            });
        }

        public void Configure(IApplicationBuilder app, IHostingEnvironment env, ILoggerFactory loggerFactory)
        {
//(...)
            app.UseCookieAuthentication(options =>
            {
                options.AutomaticAuthenticate = true;
            });

            app.UseOpenIdConnectAuthentication(options =>
            {
                options.AutomaticChallenge = true;
                options.ClientId = Configuration["Authentication:AzureAd:ClientId"];
                options.Authority = Configuration["Authentication:AzureAd:AADInstance"] + Configuration["Authentication:AzureAd:TenantId"];
                options.PostLogoutRedirectUri = Configuration["Authentication:AzureAd:PostLogoutRedirectUri"];
                options.SignInScheme = CookieAuthenticationDefaults.AuthenticationScheme;
            });

            app.UseMvc(routes =>
            {
                routes.MapRoute(
                    name: "default",
                    template: "{controller=Home}/{action=Index}/{id?}");
            });
        }

        // Entry point for the application.
        public static void Main(string[] args) => WebApplication.Run<Startup>(args);
    }
}
blowdart commented 8 years ago

That would be a bug to log in the github.com/aspnet/security repo - we did overhaul some of this in RC2.

Apologies for never noticing this, I didn't have github notifications turned on for this repo :(