Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add methods for registering both authentication and authorization middlewares/services #42047

Open
captainsafia opened this issue Jun 6, 2022 · 6 comments
Assignees
Labels
api-needs-work API needs work before it is approved, it is NOT ready for implementation area-auth Includes: Authn, Authz, OAuth, OIDC, Bearer
Milestone

Comments

@captainsafia
Copy link
Member

captainsafia commented Jun 6, 2022

Background and Motivation

To provide an abstraction of registering both authentication and authorization-related middlewares/services in an app with fewer lines of code and build on the foundation of automatically registering middlewares/services that we started in preview5, we would like to add extension methods for registering both authentication and authorization-related middlewares/services via one overload.

Proposed API

namespace Microsoft.Extensions.DependencyInjection;

public static class AuthServiceCollectionExtensions 
{
  public static IServiceCollection AddAuthenticationAndAuthorization(this IServiceCollection services);
  public static IServiceCollection AddAuthenticationAndAuthorization(
    this IServiceCollection services,
    Action<AuthorizationOptions> configureAuthorizationOptions);
  public static IServiceCollection AddAuthenticationAndAuthorization(
    this IServiceCollection services,
    Action<AuthenticationOptions> configureAuthenticationOptions);
  public static IServiceCollection AddAuthenticationAndAuthorization(
    this IServiceCollection services,
    Action<AuthenticationOptions> configureAuthenticationOptions,
    Action<AuthorizationOptions> configureAuthorizationOptions);
}
namespace Microsoft.AspNetCore.Builder;

public static class AuthAppBuilderExtensions
{
  public static IApplicationBuilder UseAuthenticationAndAuthorization(this IApplicationBuilder app)
}

Usage Examples

Before

var builder = WebApplication.CreateBuilder(args);
builder.Services.AddAuthentication();
builder.Services.AddAuthorization();
var app = builder.Build();
app.UseAuthentication();
app.UseAuthorization();

After

var builder = WebApplication.CreateBuilder(args);
builder.Services.AddAuthenticationAndAuthorization();
var app = builder.Build();
app.UseAuthenticationAndAuthorization();

After with Options

var builder = WebApplication.CreateBuilder(args);
builder.Services.AddAuthenticationAndAuthorization(options => {
  options.DefaultScheme = "foobar";
});
var app = builder.Build();
app.UseAuthenticationAndAuthorization();
@captainsafia captainsafia added the api-ready-for-review API is ready for formal API review - https://github.com/dotnet/apireviews label Jun 6, 2022
@ghost
Copy link

ghost commented Jun 6, 2022

Thank you for submitting this for API review. This will be reviewed by @dotnet/aspnet-api-review at the next meeting of the ASP.NET Core API Review group. Please ensure you take a look at the API review process documentation and ensure that:

  • The PR contains changes to the reference-assembly that describe the API change. Or, you have included a snippet of reference-assembly-style code that illustrates the API change.
  • The PR describes the impact to users, both positive (useful new APIs) and negative (breaking changes).
  • Someone is assigned to "champion" this change in the meeting, and they understand the impact and design of the change.

@captainsafia captainsafia added area-auth Includes: Authn, Authz, OAuth, OIDC, Bearer area-web-frameworks labels Jun 6, 2022
@davidfowl
Copy link
Member

Not sure about the adds as both need to be configured. The API sample should show a before and after with options.

@halter73
Copy link
Member

halter73 commented Jun 13, 2022

API Review Notes:

  • Do we need AddAuthenticationAndAuthorization given you can call AddAuthentication and AddAuthorization in either order? And that there are two different options objects to configure?
    • No
  • What about UseAuthenticationAndAuthorization? Do we need it?
    • Yes. It doesn't make sense to use authz without authn.
  • Can we come up with a better name?
    • UseAuthNAndAuthZ
    • UseAuth
    • UseAuthStar 😆
    • UseAuths
  • UseAuth wins. We think it implies both authn and authz.
  • What assembly should UseAuth live in?
    • We can't find a good one

API would be approved if we can find a good assembly for it. Until then, it needs work.

namespace Microsoft.AspNetCore.Builder;

public static class AuthAppBuilderExtensions
{
+  public static IApplicationBuilder UseAuth(this IApplicationBuilder app);
}

@halter73 halter73 added api-needs-work API needs work before it is approved, it is NOT ready for implementation and removed api-ready-for-review API is ready for formal API review - https://github.com/dotnet/apireviews labels Jun 13, 2022
@captainsafia captainsafia modified the milestones: 7.0-preview6, Backlog Jun 13, 2022
@ghost
Copy link

ghost commented Jun 13, 2022

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.

@Kahbazi
Copy link
Member

Kahbazi commented Jun 14, 2022

It doesn't make sense to use authz without authn.

@halter73 Just FYI I don't use authn middleware in the applications that are just APIs. I don't have a default authn scheme and I also don't need any remote authentication handler. Also all endpoints are marked with Authorize attribute with different scheme.

@halter73
Copy link
Member

Thanks for pointing that out it's possible to use authz middleware without authn middleware. It's not a scenario I considered. Fortunately, we are not planning on removing any existing auth APIs, so you should be able to continue using just the authz middleware by itself.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-needs-work API needs work before it is approved, it is NOT ready for implementation area-auth Includes: Authn, Authz, OAuth, OIDC, Bearer
Projects
None yet
Development

No branches or pull requests

4 participants