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

Support for custom authentication scheme without adding data protection #33779

Closed
rocklan opened this issue Jun 23, 2021 · 4 comments
Closed
Labels
area-auth Includes: Authn, Authz, OAuth, OIDC, Bearer ✔️ Resolution: Answered Resolved because the question asked by the original author has been answered. Status: Resolved

Comments

@rocklan
Copy link
Contributor

rocklan commented Jun 23, 2021

If you are building a custom authentication scheme that doesn't need data protection, there is currently no way to add authentication without adding data protection services. This means that at startup, keys are generated and logging is created (if they aren't persisted or encrypted) - when it's completely unnecessary.

This is because inside Microsoft.Extensions.DependencyInjection.AuthenticationServiceCollectionExtensions the code looks like this:

public static AuthenticationBuilder AddAuthentication(this IServiceCollection services)
{
    ...

    services.AddAuthenticationCore();
    services.AddDataProtection();
    services.AddWebEncoders();
    services.TryAddSingleton<ISystemClock, SystemClock>();
    return new AuthenticationBuilder(services);
}

The call to AddDataProtection is added indiscriminately. From a developers point of view this can be worked around by replicating this code and the call to AddDataProtection() is removed:

    services.AddAuthenticationCore();
    services.AddWebEncoders();
    services.TryAddSingleton<ISystemClock, SystemClock>();
    var authBuilder = new AuthenticationBuilder(services);

But I expect this code to break in the future as it's not documented or supported.

As a possible solution maybe a new boolean property named RequiresDataProtection that defaults to true could be added to the AuthenticationOptions class, and if set to false, Data Protection is not added?

@Tratcher Tratcher added the area-auth Includes: Authn, Authz, OAuth, OIDC, Bearer label Jun 23, 2021
@blowdart
Copy link
Contributor

If the problem here is the key creating you can disable key creation

Copying the code out of the extension is unlikely to break, but we have data protection in there because we use it in a lot of places, from cookies to oauth to csrf. Its loaded at startup to avoid deadlocks on the first request that hits the service. It's unlikely that we'd consider a different extension method, because it's usually needed by other services, so you're a bit of an edge case. If we see more requests we can consider it.

@blowdart blowdart added the ✔️ Resolution: Answered Resolved because the question asked by the original author has been answered. label Jun 24, 2021
@ghost ghost added the Status: Resolved label Jun 24, 2021
@rocklan
Copy link
Contributor Author

rocklan commented Jun 24, 2021

Disabling key creation still gives me a The key ring does not contain a valid default key, and the key manager is configured with auto-generation of keys disabled. error at startup because according to the code "The key ring must contain at least one active non-revoked key" - but appreciate the thought.

@blowdart
Copy link
Contributor

Darn it. Well it was worth a shot.

@ghost
Copy link

ghost commented Jun 26, 2021

This issue has been resolved and has not had any activity for 1 day. It will be closed for housekeeping purposes.

See our Issue Management Policies for more information.

@ghost ghost closed this as completed Jun 26, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Jul 26, 2021
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-auth Includes: Authn, Authz, OAuth, OIDC, Bearer ✔️ Resolution: Answered Resolved because the question asked by the original author has been answered. Status: Resolved
Projects
None yet
Development

No branches or pull requests

3 participants