Skip to content
This repository was archived by the owner on Dec 13, 2018. It is now read-only.

Auth 2.0 using OptionsFactory (named options) #1144

Closed
wants to merge 56 commits into from
Closed

Conversation

HaoK
Copy link
Member

@HaoK HaoK commented Mar 11, 2017

  • New IOptionsFactory which provides named options in addition to validation, both of which are triggered via IOptionsFactory.Get("name"). Allows normal options configuration for schemes, Validation is just a different set of Actions that run after the Configures

IOptionsFactory Usage:

  services.ConfigureAll<GoogleOptions>(o => o.SignInScheme = "GoogleCookie");
  services.Configure<GoogleOptions>("Google", o => ClientId = "avsdlkj");
  services.Configure<GoogleOptions>("Google2" p => ClientId = "two");
  services.ValidateAll<GoogleOptions>(o => if (o.ClientId == null) throw new ArgumentException());

  optionsFactory.Get<GoogleOptions>("Google"); // OK
  optionsFactory.Get<GoogleOptions>("Default"); // throws ArgumentException
  • split HandleRequest into its own IAuthenticationRequestHandler interface instead, CallbackPaths routing is disabled for now (all IAuthenticationRequestHandler schemes are given a chance to handle the request)

TL:DR Auth 2.0 summary

  • New IAuthenticationService with the same Authenticate/Challenge/SignIn/Out/ForbidAsync API.
  • Made up of AuthenticationScheme instances which have: Name, Type, and Settings dictionary (Options instance + old description bag)
  • app.UseXyzAuth(new XyzOptions()) => services.AddXyzAuth(o => o.Foo = bar) adds an AuthenticationScheme instance for Xyz
  • Single UseAuthentication is required, which takes care of the old 'automatic' authenticate, in addition to giving each scheme a chance to handle the request like the old middlewares did.
  • Auth provider extensibility model is mostly unchanged, just tweaked (no more middleware, handler base class is cleaned up and activated from DI (see DbContext example below)
  • 'Automatic' authenticate and challenge are now a single Default[Authenticate/SignIn]Scheme in AuthenticationOptions, rather than a flag on each middleware. No ambiguity, altho there's one bit of magic where if none are specified and there's only a single scheme registered, we use that scheme as the default.

Pros of the new design

  • AuthN is now a service and 'feels' like AuthZ.
  • Auth handlers can now consume services from DI easily (e.g. see DbContext example).
  • ClaimsTransformation simplified (sane now, rather than wrapped AuthHandler + xform middleware fun), example below.
  • Dynamic schemes supported via replacing the IAuthenticationSchemeProvider service.
  • Eliminated a lot of design/complexity issues that we have consistently run into when trying to fix bugs (no more handler chaining, automatic gone)

Example of using scoped DbContext

This was not really easily accomplished in the old stack since handlers were created by the middleware via protected abstract AuthenticationHandler<TOptions> CreateHandler();

   // ConfigureServices:
   services.AddSchemeHandler<CookieAuthenticationOptions, MyCookieAuthenticationHandler>("Cookie1", o => { o.CookieName = "Cookie1" });
   services.AddSchemeHandler<CookieAuthenticationOptions, MyCookieAuthenticationHandler>("Cookie2", o => { o.CookieName = "Cookie2" });

public class MyCookieAuthenticationHandler : CookieAuthenticationSchemeHandler {
    public MyCookieAuthenticationHandler(DbContext context, ILoggerFactory logger, UrlEncoder encoder) : base(logger, encoder) {
       _dbContext = context;
    }
    
        protected async override Task<CookieAuthenticationOptions> CreateOptionsAsync()
        {
            var options = await base.CreateOptionsAsync();
            UseYourDbContextOnThese(options);
        }
}

Claims Transformation

A lot simpler, new IClaimsTransformation service which has a single method:
Task<ClaimsPrincipal> TransformAsync(ClaimsPrincipal principal)

We call this on any successful AuthenticateAsync call.

   services.AddAuthentication(o => o.ClaimsTransform = p => {
     p.AddIdentity(new ClaimsIdentity());
     return Task.FromResult(p);
  }

To use a more advanced scoped claims transform that uses DbContext's, they would just need to add their own IClaimsTransformation before any AddAuthentication calls.

@dnfclas
Copy link

dnfclas commented Mar 11, 2017

@HaoK,
Thanks for having already signed the Contribution License Agreement. Your agreement was validated by .NET Foundation. We will now review your pull request.
Thanks,
.NET Foundation Pull Request Bot

Senorsen and others added 4 commits March 13, 2017 07:35
To avoid confusion when reading the code
- do not have .NET 4.6.1 reference assemblies on all CI machines
- have corrected System.XML casing issue mentioned in 7637f2e

nit: sort dependencies
@HaoK
Copy link
Member Author

HaoK commented Mar 13, 2017

@Tratcher I Pushed the duplicated DataProtectionProvider code in all the RemoteAuths up to base to clean things up

@HaoK
Copy link
Member Author

HaoK commented Mar 13, 2017

@PinpointTownes do you prefer this iteration that is more similar to the old behavior, where any scheme that implements IAuthenticationRequestHandler will see all requests? I don't have an easy way right now to implement the old callback ghetto routing with the switch to using named options

@kevinchalet
Copy link
Contributor

@PinpointTownes do you prefer this iteration that is more similar to the old behavior, where any scheme that implements IAuthenticationRequestHandler will see all requests?

I'll take a deeper look tomorrow, but it looks really promising, I like it 😄

var handler = await handlers.ResolveHandlerAsync(context, scheme.Name);
if (await handler.HandleRequestAsync())
var handler = await handlers.GetHandlerAsync(context, scheme.Name) as IAuthenticationRequestHandler;
if (await handler?.HandleRequestAsync())
Copy link
Contributor

@kevinchalet kevinchalet Mar 13, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI: this line will null-ref if handler is null.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops thanks fixed

@@ -12,9 +12,6 @@ public interface IAuthenticationService
Task<AuthenticateResult> AuthenticateAsync(HttpContext context, string scheme);
Task ChallengeAsync(HttpContext context, string scheme, AuthenticationProperties properties, ChallengeBehavior behavior);
Task ForbidAsync(HttpContext context, string scheme, AuthenticationProperties properties);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Until we sort out Challenge vs Forbid, Forbid is not a 1st class thing, it's a subset of Challenge.

@HaoK
Copy link
Member Author

HaoK commented Mar 15, 2017

Rebased to dev replaced with #1151

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants