Skip to content
This repository has been archived by the owner on Nov 7, 2018. It is now read-only.

Options 2.0 changes (named/validation/caching) for Auth 2.0 #161

Closed
HaoK opened this issue Jan 18, 2017 · 9 comments
Closed

Options 2.0 changes (named/validation/caching) for Auth 2.0 #161

HaoK opened this issue Jan 18, 2017 · 9 comments

Comments

@HaoK
Copy link
Member

HaoK commented Jan 18, 2017

See if there's a way to reintroduce named options in a nicer way, such that we can consume it the same way today via IOptions/ISnapshotOptions.Value, where a new IOptionsNameSelector service would be the customization point where name(aka tenant) can be selected.

High level usage:

    // Add some kind of NamedInstance string property to IConfigure<TOptions>

    // Configure
    services.Configure<MyOptions>("Tenant1", o => o.Connection = "conn1");
    services.Configure<MyOptions>("Tenant2", o => o.Connection = "conn2");
    services.AddScoped<IOptionsNameSelector, MyTenantSelector>();
    
    public MyTenantSelector : IOptionsNameSelector {
        public MyTenantSelector(IHttpContextAccessor accessor);
        public string ResolveOptionsName() {
             return GetTenantNameFromRequest(accessor);
        }
    }

    public Controller(IOptions<MyOptions> options) {
       var tenantAwareOptions = options.Value;
    }

    public Controller(ISnapshotOptions<MyOptions> options) {
       var tenantAwareOptions = options.Value;
    }

The options implementations would use the IOptionsNameSelector and NamedInstance to return the appropriate options instance

Hopefully we can do this simply by enhancing the existing implementations of the current interfaces... But its possible we might need to introduce INamedOptions<MyOptions>.

This would dovetail nicely with the auth changes that could take advantage of this feature via named auth scheme options

cc @divega @glennc

@HaoK HaoK self-assigned this Jan 18, 2017
@HaoK HaoK added this to the 2.0.0 milestone Jan 18, 2017
@HaoK
Copy link
Member Author

HaoK commented Jan 19, 2017

@divega Looks like you might get that IOptionsCache that you were hoping for. Since IOptionsNameSelector potentially needs to be scoped, we can no longer register any of our options managers as singletons, and instead they will need to be scoped as well, which means we will need to push the cache instances into a shared singleton service instead to survive across requests since the lifetime of IOptions<T>, IOptionsMonitor<T> will now be scoped (but the actual options instances will continue to be stable, unless they are reloaded)

@divega
Copy link

divega commented Jan 19, 2017

@HaoK cool 😄

@HaoK
Copy link
Member Author

HaoK commented Jan 20, 2017

Looks like we will need to have more advanced cache capabilities with eviction to support multi-tenancy, we should be able to leverage the memory cache.

@HaoK
Copy link
Member Author

HaoK commented Jan 23, 2017

@divega updated the PR adding support for named options to IOptionsMonitor. I also added a direct GetNamedInstance back to IOptions for a direct way to get a sibling.

After refactoring the code a bit, the options monitor now inherits from OptionsManager and adds on the reload/change notification stuff, and CurrentValue is just an alias for Value (maybe we should take this opportunity to revisit the montior API?

@HaoK
Copy link
Member Author

HaoK commented Mar 7, 2017

Desired Auth 2.0 usage pattern:

   // Validation registered in DI as part services.AddGoogleAuthentication(), and triggered whenever IOptions.Value is created
   services.ValidateAll<GoogleOptions>(o => o.Validate()); // Validate throws on errors
   services.Validate<GoogleOptions>("Name1", o => o.Validate()); // Named validation target

        // RemoteAuth validation
        public override void Validate()
        {
            base.Validate();
            if (CallbackPath == null || !CallbackPath.HasValue)
            {
                throw new ArgumentException(Resources.FormatException_OptionMustBeProvided(nameof(CallbackPath)), nameof(CallbackPath));
            }
        }

        // OAuth validation
        public override void Validate()
        {
            base.Validate();

            if (string.IsNullOrEmpty(ClientId))
            {
                throw new ArgumentException(string.Format(CultureInfo.CurrentCulture, Resources.Exception_OptionMustBeProvided, nameof(ClientId)), nameof(ClientId));
            }

            if (string.IsNullOrEmpty(ClientSecret))
            {
                throw new ArgumentException(string.Format(CultureInfo.CurrentCulture, Resources.Exception_OptionMustBeProvided, nameof(ClientSecret)), nameof(ClientSecret));
            }

            if (string.IsNullOrEmpty(AuthorizationEndpoint))
            {
                throw new ArgumentException(string.Format(CultureInfo.CurrentCulture, Resources.Exception_OptionMustBeProvided, nameof(AuthorizationEndpoint)), nameof(AuthorizationEndpoint));
            }

            if (string.IsNullOrEmpty(TokenEndpoint))
            {
                throw new ArgumentException(string.Format(CultureInfo.CurrentCulture, Resources.Exception_OptionMustBeProvided, nameof(TokenEndpoint)), nameof(TokenEndpoint));
            }

            if (!CallbackPath.HasValue)
            {
                throw new ArgumentException(string.Format(CultureInfo.CurrentCulture, Resources.Exception_OptionMustBeProvided, nameof(CallbackPath)), nameof(CallbackPath));
            }

       // Alternatives to Validate throwing: Validate could return a List<Exception>


   // Configuration of all instances
  services.ConfigureAll<GoogleOptions>(o => o.SignInScheme = "whatever");

  // Configure only name1 instance
  services.Configure<GoogleOptions>("Name1", o => o.ClientId = "abcd");

   public class GoogleHandler {
        public GoogleHandler(IOptions<GoogleOptions> options) { OptionsAccessor = options; }
        public virtual Task InitializeAsync(AuthenticationScheme scheme, HttpContext context) {
             Options = OptionsAccessor.GetNamed(scheme); // Also validates the scheme at this point
        }
   }

@HaoK HaoK changed the title Investigate named(aka multi-tenant) options Implement named options for Auth 2.0 [OptionsFactory] Mar 16, 2017
@HaoK
Copy link
Member Author

HaoK commented Mar 16, 2017

cc @glennc @davidfowl

At the Auth 2.0 meeting with @Tratcher @Eilon @DamianEdwards @ajcvickers we decided to go ahead and add named options support back to Options since its required functionality for Auth 2.0.

This will evolve from the prototype in: aspnet/Security#1144

Basic current shape is:

   services.Configure<TOption>("name"); // configure single named instance
   services.ConfigureAll<TOption>() // configure all instances regardless of name
   services.Validate<TOption>("name"); // validation for a single named instance
   services.ValidateAll<TOption>() // validates all instances

   IOptionsFactory.Get<TOption>("name"); // which will trigger the appropriate Configure and Validates

This should be fully backwards compatable with no effect on existing IOptions, we can choose to preserve the current unnamed option as a special Default value (null name?) in the Factory.

We can also consider switching to Async Configures at this point which would better enable DbContext driven configuration.

@HaoK HaoK changed the title Implement named options for Auth 2.0 [OptionsFactory] Options 2.0 changes (named/validation/caching) for Auth 2.0 Mar 18, 2017
@HaoK
Copy link
Member Author

HaoK commented Mar 29, 2017

Initial changes added via bf76cdb to unblock Auth.

@HaoK HaoK closed this as completed Mar 29, 2017
@HaoK HaoK added the 3 - Done label Mar 29, 2017
@jdom
Copy link
Contributor

jdom commented Apr 14, 2017

Good to see that named options is becoming a thing. Do you have an estimated timeframe for shipping 2.0? We are considering using this in Orleans: dotnet/orleans#2936

@Eilon
Copy link
Member

Eilon commented Apr 14, 2017

@jdom you can see a rough roadmap here: https://github.com/aspnet/Home/wiki/Roadmap

2.0 Q3 2017

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

No branches or pull requests

4 participants