Authorization FallbackPolicy is misleading

[jacob7395]

Is there an existing issue for this?

• I have searched the existing issues

Describe the bug

When using the Fall back policy like;

builder.Services.AddAuthorization(options =>
{
    options.FallbackPolicy = new AuthorizationPolicyBuilder()
        .RequireAuthenticatedUser()
        .Build();
});

And configuring the endpoint auth with either app.MapControllers().RequireAuthorization(); or adding an empty attribute [Authorize]. It uses the default auth policy. Even though another auth policy has not been defined.

This might be intended but could lead to some big headaches for teams. As the Attribute AuthorizeAttribute dose not require you to set the policy adding something like [Authorize(AuthenticationSchemes = "example")] would cause the endpoint to use the default not the fallback. This could lead to some sad times if it's not caught before prod.

This might be internecinal design, looking at the class responsible for this the occurs as by using any auth header or by setting an empty app.MapControllers().RequireAuthorization() the code loops though the authorizeData and as no policies have explicitly been set uses the default.

This seems unintuitive to me, and if you agree I think adding a check before adding the default to try and get the fallback policy, mean is used over the default. Since the user has explicitly set the fallback, but not named another policy, and can not have a null default policy.

Expected Behavior

The fallback will be used in all cases where no policy has been explicitly provided, including when a policy attribute has been provided but the policy property has not been set.

If the design is intended to be misleading in this way code docs on FallbackPolicy explaining this and updated docs would be useful.

Steps To Reproduce

Create a project, and add a fallback policy. Then add in an empty auth attribute on either a controller or when mapping the endpoints.

Exceptions (if any)

No response

.NET Version

Microsoft.AspNetCore.Authorization 8.0.13

also in the current [source].(https://github.com/dotnet/aspnetcore/blob/main/src/Security/Authorization/Core/src/AuthorizationPolicy.cs)

Anything else?

No response

[halter73]

This seems unintuitive to me, and if you agree I think adding a check before adding the default to try and get the fallback policy, mean is used over the default. Since the user has explicitly set the fallback, but not named another policy, and cannot have a null default policy.

Is the suggestion to always prefer the FallbackPolicy over the DefaultPolicy if it's specified? What would the point of DefaultPolicy be then?

Even if everyone agreed changing the behavior would make more sense, we couldn't make such a drastic breaking change without an opt-in of some sort. Otherwise, we'd risk introducing security vulnerabilities in an update.

Personally, I'm not a huge fan of the DefaultPolicy and FallbackPolicy either. Naming APIs is hard, but if we wanted to be super wordy maybe PolicyToUseWhenAuthorizationRequestedButNoSpecificPolicySpecified and PolicyToUseWhenAuthorizationNotExplicitelyRequested or something like that would be better, but even that's imprecise. What does it mean for authorization to be "requested"? It generally means the request endpoint has IAuthorizeMetadata.

This is where documentation comes into play. I think it could probably be improved, and this is an area we welcome contributions. The documentation on learn.microsoft.com is pulled directly from the doc comments in this repo, so if you have any improvements, feel free to open a PR.

aspnetcore/src/Security/Authorization/Core/src/AuthorizationOptions.cs

Lines 26 to 42 in a2e3677

	/// <summary>
	/// Gets or sets the default authorization policy. Defaults to require authenticated users.
	/// </summary>
	/// <remarks>
	/// The default policy used when evaluating <see cref="IAuthorizeData"/> with no policy name specified.
	/// </remarks>
	public AuthorizationPolicy DefaultPolicy { get; set; } = new AuthorizationPolicyBuilder().RequireAuthenticatedUser().Build();
	/// <summary>
	/// Gets or sets the fallback authorization policy used by <see cref="AuthorizationPolicy.CombineAsync(IAuthorizationPolicyProvider, IEnumerable{IAuthorizeData})"/>
	/// when no IAuthorizeData have been provided. As a result, the AuthorizationMiddleware uses the fallback policy
	/// if there are no <see cref="IAuthorizeData"/> instances for a resource. If a resource has any <see cref="IAuthorizeData"/>
	/// then they are evaluated instead of the fallback policy. By default the fallback policy is null, and usually will have no
	/// effect unless you have the AuthorizationMiddleware in your pipeline. It is not used in any way by the
	/// default <see cref="IAuthorizationService"/>.
	/// </summary>
	public AuthorizationPolicy? FallbackPolicy { get; set; }