-
Notifications
You must be signed in to change notification settings - Fork 58
[Prototype] Add multitenant version of dynamic schemes sample #44
Conversation
|
||
namespace AuthSamples.Options.MultiTenant | ||
{ | ||
public class TenantResolver |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interface rather than class? That way we could push a database into it, so people can map from config in a database
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So this isn't something we are shipping, this is just a sample, so there's not really much upside for an interface in the sample when they would just modify this to take any dependencies needed, if they wanted it to use a DbContext, they could just make it Scoped, although that would affect the lifetimes of the other things too
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks promising. It's hard to call this a supported scenario when you have to implement this many types.
3. You can also update any of the scheme options message via the add/update form. | ||
|
||
Index.cshtml and Controllers/AuthController.cs are the most interesting classes in the sample, | ||
as they demonstrate how to add/remove schemes and update the corresponding named options dynamically. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also TenantSchemeResolver and company
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah i still need to update the readmes
=> Task.FromResult<IEnumerable<AuthenticationScheme>>(GetMap(_resolver.ResolveTenant()).Values); | ||
|
||
public Task<AuthenticationScheme> GetDefaultAuthenticateSchemeAsync() | ||
=> null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why aren't these needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This sample doesn't actually do auth, the pages just list the schemes and their creds. Its just demonstrating how you configure different schemes in different tenants, its not functionally using auth.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hence its Options.Multitenant as opposed to Auth.Multitenant
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But basically this implementation doesn't support defaults, which makes sense since they hang off the singleton AuthenticationOptions anyways
But yeah this certainly isn't 'easy', but its doable via extensibility (not sure what the definition of 'supported' is) |
Any major concerns with this sample? If not I'll add some tests and call it good enough for now |
No, go ahead. |
Rebase? you picked up a lot of extra commits. |
This repo is about to be archived. AuthSamples are now part of aspnet/AspNetCore. |
So early cut of the multi-tenant + auth options sample.
I took the existing dynamic schemes samples and made it work for different tenants, I just used a lame
?tenant=id query string to simulate different tenants.
The sample has adds these services:
The result is each tenant has its own set of authentication schemes and options configurations, and can be updated dynamically using /?tenant=id
Thoughts @Tratcher @davidfowl @blowdart