Aspnetcore: [ASPNET Core 2.0]Authorization Policy ignoring Authentication

[blowdart]

_From @allevyMS on April 11, 2018 17:10_

I am trying to set up authentication and authorization but I am seeing unexpected behavior.
I am running an ASPNET Core 2.0 app on windows (tried this on windows 10 and in a docker container with the following image 2.0.6-sdk-2.1.104-nanoserver-sac2016) and I have my authentication and authorization set up like this:
using Nuget package: Microsoft.AspNetCore.All 2.0.0 and also tried with 2.0.6

    public void ConfigureServices(IServiceCollection services)
    {
        services.AddAuthorization(options =>
        {
            options.AddPolicy("Test", policy =>
            {
                policy.Requirements.Add(new TestRequirement());
            });
        });

        services.AddSingleton<IAuthorizationHandler, TestAuthorizationHandler>();

        services.AddMvc();

        services.AddAuthentication(JwtBearerDefaults.AuthenticationScheme)
            .AddJwtBearer();

        services.AddSingleton<IConfigureOptions<JwtBearerOptions>>(sp =>
        {
            return new ConfigureNamedOptions<JwtBearerOptions>(
                JwtBearerDefaults.AuthenticationScheme,
                options =>
                {
                    options.TokenValidationParameters = new TokenValidationParameters
                    {
                        RequireSignedTokens = true,
                        ValidateIssuer = false,
                        ValidateAudience = false,
                        ValidateLifetime = true,
                        IssuerSigningKey = rsaSecurityKey
                    };
                });
        });
    }

    public void Configure(IServiceProvider sp, IApplicationBuilder app, IHostingEnvironment env, ILoggerFactory loggerFactory)
    {
        loggerFactory.AddConsole(Configuration.GetSection("Logging"));

        app.UseAuthentication();

        app.UseMvc();
    }

My requirement is empty and this is my authorization handler:

public class TestAuthorizationHandler : AuthorizationHandler<TestRequirement>
{
    protected override Task HandleRequirementAsync(AuthorizationHandlerContext context,
                                                   TestRequirement requirement)
    {
        context.Succeed(requirement);
        return Task.CompletedTask;
    }
}

My Controller get method:

    [Authorize(Policy = "Test")]
    [HttpGet]
    [Route("Data")]
    public async Task<string> GetDataAsync()
    {
        return await Task.FromResult("data");
    }

The unexpected behavior:

Authentication is always ignored for that controller method, my authorization handler and controller are always reached, even if I don't supply an Authentication header or supply an invalid token.

Expected behavior:

Authorization should use the supplied default scheme in AddAuthentication to challenge the Authentication and not allow these calls through unless Authentication was successful.

Workaround:

Add the default scheme directly to the policy and require the user to be authenticated

policy.AuthenticationSchemes.Add(JwtBearerDefaults.AuthenticationScheme);
policy.RequireAuthenticatedUser();

When these are added I see the behavior I expect.

Am I misunderstanding the expected flow? Is this the expected behavior and RequireAuthenticatedUser is needed explicitly to block unauthenticated requests to go through authorization?

_Copied from original issue: aspnet/Home#3046_

[kevinchalet]

Am I misunderstanding the expected flow? Is this the expected behavior and RequireAuthenticatedUser is needed explicitly to block unauthenticated requests to go through authorization?

While not super convenient, it's the expected behavior (and it's a quite frequent question: https://stackoverflow.com/questions/49545706/adding-a-policy-based-authorization-skips-jwt-bearer-token-authentication-check/49545923#49545923).

In a better world (and IMHO), DenyAnonymousAuthorizationRequirement should always be added by default and RequireAuthenticatedUser() should be replaced by AllowAnonymousUser(). But hem... it's probably a breaking change :sweat_smile:

[blowdart]

Oh heck, I moved it and never replied. As @PinpointTownes says it's expected, and yes it'd be a big breaking change, like major version number breaking change.

[kevinchalet]

@blowdart do you mind re-opening this thread and putting it in the 3.0.0 milestone so it has a chance to be discussed for the next major version?

[blowdart]

I can, but as it's turning things on its head, it's going to be a hard sell :)

[kevinchalet]

So, who can I bribe to make that happen? @DamianEdwards maybe? :trollface:

The fact DenyAnonymousAuthorizationRequirement is not automatically added to new authorization policies (you have to explicitly call RequireAuthenticatedUser()) is a serious trap for too many people.

@DamianEdwards would you be opposed to changing that to make the default behavior more secure in 3.0?

[DamascenoRafael]

Does anyone know if the workaround no longer works on .NET Core 2.2.0?
I configured the policy as follows:
```c#
public void ConfigureServices(IServiceCollection services)
{
// ...
JWTExtension.AddJwtAuthentication(services, Configuration);
// ...
services.AddMvc().SetCompatibilityVersion(CompatibilityVersion.Version_2_1);
// ...
{
options.AddPolicy("AdminRole", policy =>
{
policy.AuthenticationSchemes.Add(CookieAuthenticationDefaults.AuthenticationScheme);
policy.RequireAuthenticatedUser();
policy.Requirements.Add(new UserRoleRequirement(ProfileEnum.Admin));
});
});
// ...
}

But the policy is always called before checking the existence and validity of JWT. In my case the token is validated through the Cookie so I put the corresponding schema.

My `Configure(IApplicationBuilder app, IHostingEnvironment env)` is as follows:

<details>

```c#
        public void Configure(IApplicationBuilder app, IHostingEnvironment env)
        {
            RewriteOptions options = new RewriteOptions().AddRewrite(@"^(?!(swagger|token|api|.+[.])).*", "/", skipRemainingRules: true);

            app.UseRewriter(options);

            if (env.IsDevelopment())
            {
                app.UseDeveloperExceptionPage();
            }
            else
            {
                app.UseHsts();
            }

            string[] allowedAppUrls = Configuration["AllowedAppUrls"].Split(','); 
            app.UseCorsAllow(allowedAppUrls);
            app.UseMiddleware(typeof(ExceptionHandlerMiddleware));
            app.UseAuthentication();
            app.UseMiddleware(typeof(TokenExpirationMiddleware));
            app.UseMvc(routes =>
            {
                routes.MapRoute
                (
                    name: "default",
                    template: "api/{controller=Home}/{action=Index}/{id?}" 
                );
            });
            app.UseDefaultFiles();
            app.UseStaticFiles();

            app.ConfigSwagger();

            app.UseHttpsRedirection();
        }

[MichaelHudgins]

I too would like to know if this workaround is still supported. I am having the same effect where my authorization policy is checked even when there is an invalid jwt. I am working around by checking the user again in the authorization policy, but it would be better if the authorization policy was never called given the failure of the authentication.

[tomidix]

Does anyone know if the workaround no longer works on .NET Core 2.2.0?
I configured the policy as follows:

        public void ConfigureServices(IServiceCollection services)
        {
            // ...
            JWTExtension.AddJwtAuthentication(services, Configuration);
            // ...
            services.AddMvc().SetCompatibilityVersion(CompatibilityVersion.Version_2_1);
            // ...
            {
                options.AddPolicy("AdminRole", policy =>
                {
                    policy.AuthenticationSchemes.Add(CookieAuthenticationDefaults.AuthenticationScheme);
                    policy.RequireAuthenticatedUser();
                    policy.Requirements.Add(new UserRoleRequirement(ProfileEnum.Admin));
                });
            });
            // ...
        }

But the policy is always called before checking the existence and validity of JWT. In my case the token is validated through the Cookie so I put the corresponding schema.

My Configure(IApplicationBuilder app, IHostingEnvironment env) is as follows:

I've had a same problem. My workaround was that I put controller under default [Authorize] attribute and then every action has it's own policy [Authorize(Policy = "FullAccess")]. To me it wasn't such a big deal because I had only one controller which action's was under the same policy. RequireAuthenticatedUser and similar wasn't necessary.

[HaoK]

Can you guys share a simple repro app so we can take a closer look at the behavior changes you are seeing?

In regards to "I am having the same effect where my authorization policy is checked even when there is an invalid jwt.". This is by design, all of the authorization logic is run always, the presence or lack of the .RequireAuthenticatedUser() requirement doesn't short circuit the rest of the authorization checks. So if you want to hard block that, you do need to have logic inside of your handlers to no-op if there's no user

[allevyMS]

@HaoK I am experiencing the same regression mentioned above

.RequireAuthenticatedUser() used to in fact short circuit the flow and block the authorization if the user was not authentication, now it no longer seems to do that.

What is the function of 'RequireAuthenticatedUser()' if it doesn't enforce authentication?

[allevyMS]

This is a clear behavior change between the versions, you can use my sample app in the start of this thread as a repro

[HaoK]

The likely behavior change you are seeing if its between 2.0 and 2.1/2.2/3.0 is most likely due to mvcOptions.AllowCombiningAuthorizeFilters, make sure that is false

[allevyMS]

I don't set that property and the documentation states that it is set to false by default

[allevyMS]

To clarify, I am using version 2.2

[HaoK]

Can you upload your project to a github repo in a minimal state that reproduces what you are seeing?

[allevyMS]

Thanks @HaoK I created a minimal repro with a noop handler and requirement

So after trying it out end to end, I can confirm that 'RequireAuthenticatedUser()' is still respected correctly.
The only difference is that in 2.2 the handlers are called regardless of the authentication status.
In earlier versions (2.0) the handlers weren't called at all if the authentication failed.
In both cases the calls result in a 401 return status when the authentication fails.

Here is the setup for my repro for posterity:

Startup:

public class Startup
{
    public Startup(IConfiguration configuration)
    {
        Configuration = configuration;
    }

    public IConfiguration Configuration { get; }

    // This method gets called by the runtime. Use this method to add services to the container.
    public void ConfigureServices(IServiceCollection services)
    {
        services.AddSingleton<IAuthorizationHandler, TestAuthorizationHandler>();

        services.AddMvc();

        services.AddAuthentication(JwtBearerDefaults.AuthenticationScheme)
            .AddJwtBearer();

        services.AddSingleton<IConfigureOptions<JwtBearerOptions>>(sp =>
        {
            return new ConfigureNamedOptions<JwtBearerOptions>(
                JwtBearerDefaults.AuthenticationScheme,
                options =>
                {
                    options.TokenValidationParameters = new TokenValidationParameters
                    {
                        RequireSignedTokens = false,
                        ValidateIssuer = false,
                        ValidateAudience = false,
                        ValidateLifetime = true,
                    };
                });
        });

        services.AddAuthorization(options =>
        {
            options.AddPolicy("Test", policy =>
            {
                policy.AuthenticationSchemes.Add(JwtBearerDefaults.AuthenticationScheme);
                policy.RequireAuthenticatedUser();
                policy.Requirements.Add(new TestRequirement());
            });
        });

        services.AddMvc().SetCompatibilityVersion(CompatibilityVersion.Latest);
    }

    public void Configure(IApplicationBuilder app, IHostingEnvironment env)
    {
        app.UseAuthentication();
        app.UseMvc();
    }
}

Main:

public class Program
{
    public static void Main(string[] args)
    {
        CreateWebHostBuilder(args).Build().Run();
    }

    public static IWebHostBuilder CreateWebHostBuilder(string[] args) =>
        WebHost.CreateDefaultBuilder(args)
            .UseStartup<Startup>();
}

AuthZ Handler:

public class TestRequirement : IAuthorizationRequirement
{
}

public class TestAuthorizationHandler : AuthorizationHandler<TestRequirement>
{
    protected override Task HandleRequirementAsync(AuthorizationHandlerContext context,
                                                   TestRequirement requirement)
    {
        context.Succeed(requirement);
        return Task.CompletedTask;
    }
}

Controller:

[ApiController]
public class TestController : ControllerBase
{
    [Authorize(Policy = "Test")]
    [HttpGet]
    [Route("data")]
    public string Get()
    {
        return "test";
    }
}

[allevyMS]

To be explicit:

making a call with an invalid/missing token when specifying policy.RequireAuthenticatedUser(); will trigger the TestAuthorizationHandler.HandleRequirementAsync method but the request will still result in a 401 http status

without the policy.RequireAuthenticatedUser(); it will go on to the controller

[jrovny]

Any update on this? Was this resolved in version 3.0?

Thanks,

[HaoK]

There are intentional behavior changes between 2.0/2.1 and 3.0, the best thing to do is to try your app on 2.2 and 3.0 and see if it behaves in a way you expect, if not we can try and explain any behavior differences you see

[mingazhev]

I can confirm @allevyMS 's answer, this flow works this way in .NET Core 3.1 too and this is not so obvious behavior.

I've expected that short circuiting (not walking till authorization policies with not authenticated user) could be done easily but it I couldn't find a perfect way.
So workaround for me was registering custom PolicyEvaluator wrapper to forbid requests manually:

Startup.cs
services.AddTransient<IPolicyEvaluator, ForbidUnauthenticatedPolicyEvaluator>(); services.AddTransient<PolicyEvaluator>();

ForbidUnauthenticatedPolicyEvaluator.cs

public class ForbidUnauthenticatedPolicyEvaluator : IPolicyEvaluator
    {
        public ForbidUnauthenticatedPolicyEvaluator(PolicyEvaluator defaultEvaluator)
        {
            _defaultEvaluator = defaultEvaluator;
        }


        public Task<AuthenticateResult> AuthenticateAsync(AuthorizationPolicy policy, HttpContext context)
            => _defaultEvaluator.AuthenticateAsync(policy, context);


        public Task<PolicyAuthorizationResult> AuthorizeAsync(AuthorizationPolicy policy, AuthenticateResult authenticationResult, HttpContext context,
            object resource)
        {
            if (!authenticationResult.Succeeded)
                return Task.FromResult(PolicyAuthorizationResult.Forbid());

            return _defaultEvaluator.AuthorizeAsync(policy, authenticationResult, context, resource);
        }


        private readonly PolicyEvaluator _defaultEvaluator;
    }

[rosdi]

Got bitten by this as well, I have a code like this:

services.AddAuthorization(options =>
  {
     options.DefaultPolicy = new AuthorizationPolicyBuilder()
        .RequireAuthenticatedUser()
        .AddRequirements(new MustBeSuperAdminRequirement())
        .Build();
  })

One would be forgiven to think that MustBeSuperAdminRequirement won't be called if user has not logged in... but it is not so!... it went straight in and I spent a few hours scratching my head...

As a workaround I did this in my requirement handler:

protected override async Task HandleRequirementAsync(
            AuthorizationHandlerContext context,
            MustBeSuperAdminRequirement requirement)
{
    var principal = context.User;
    if (!principal.IsAuthenticated())
    {
        return;  //user not logged in
    }

   //the rest of the codes
}

.. and here...

app.UseEndpoints(endpoints => 
{
    endpoints.MapControllers().RequireAuthorization();
});

UPDATE

I am not observing different behaviour between these two codes:

//code 1
services.AddAuthorization(options =>
  {
     options.DefaultPolicy = new AuthorizationPolicyBuilder()
        .RequireAuthenticatedUser()
        .AddRequirements(new MustBeSuperAdminRequirement())
        .Build();
  })

//code 2
services.AddAuthorization(options =>
  {
     options.DefaultPolicy = new AuthorizationPolicyBuilder()
        //.RequireAuthenticatedUser()  //<-- this is intentionally removed
        .AddRequirements(new MustBeSuperAdminRequirement())
        .Build();
  })

Both returning 401, now I am more puzzled in what scenario would I need RequireAuthenticatedUser() ?

Shouldn't we consider this a bug? I am on .NET Core 3.1 btw.

[IngoVals]

Just had this issue. Now we must write every Handler with the assumption that the user maybe isn't authenticated. So each handler is also a DenyAnonymousAuthorizationRequirement handler in theory. This is counter intuitive, and strange considering that a failed authentication gives us a 401, but authorization a 403, so it seems asp.net core is at least ensuring that authentication prevails anyways.

[mingazhev]

Just had this issue. Now we must write every Handler with the assumption that the user maybe isn't authenticated. So each handler is also a DenyAnonymousAuthorizationRequirement handler in theory. This is counter intuitive, and strange considering that a failed authentication gives us a 401, but authorization a 403, so it seems asp.net core is at least ensuring that authentication prevails anyways.

Just to update a workaround I've written before:
Now we return PolicyAuthorizationResult.Challenge() instead of PolicyAuthorizationResult.Forbid() and that returns 401 as expected for unauthenticated users.