Add MultiTenantAuthenticationManagerResolver #6977
Conversation
| * @since 5.2 | ||
| * @see AuthenticationManagerResolver | ||
| */ | ||
| public final class ServletAuthenticationManagerResolvers { |
There was a problem hiding this comment.
Let's rename this to AuthenticationManagerResolvers
There was a problem hiding this comment.
I'd like to also consider moving this to a class with a name more focused on multi tenancy. Please consider something like creating a class named MultiTenancyAuthenticationManagerResolver with static factory methods that do what is happening here. The MultiTenancyAuthenticationManagerResolver class allows injecting a strategy to resolve the tenant id from the request and then the manager from the tenant id.
jzheaux
force-pushed
the
servlet-multitenancy
branch
from
ba1e0ba to
20486a7
Compare
jzheaux
changed the title
jzheaux
force-pushed
the
servlet-multitenancy
branch
3 times, most recently
from
715625c to
49a77a9
Compare
jzheaux
changed the title
| * @return A hostname-resolving {@link AuthenticationManagerResolver} | ||
| */ | ||
| public static AuthenticationManagerResolver<HttpServletRequest> | ||
| resolveByFirstHostnameLabel(Converter<String, AuthenticationManager> resolver) { |
There was a problem hiding this comment.
I'm not sure about the phrasing for this. Perhaps it is just me, but I typically hear the phrase subdomain rather than hostname label. Have you given thought to resolveBySubdomain?
There was a problem hiding this comment.
I'm all for pragmatism, and "subdomain" may be more pragmatic. I think it reads more cleanly, too.
The reason that I avoided it is because of Wikipedia's definition of a subdomain:
A subdomain is a domain that is part of a larger domain; the only domain that is not also a subdomain is the root domain.[1] For example, west.example.com and east.example.com/ are subdomains of the example.com domain, which in turn is a subdomain of the com top-level domain (TLD)
If that's the case, then "west" or "east" are not subdomains, even though that's what we all often call them.
Knowing this, what are your thoughts? We could call it resolveBySubdomain (which is what I originally had), but would it cause cognitive dissonance?
There was a problem hiding this comment.
Thanks for the comment. I understand your reservations. I guess I wouldn't necessarily assume that the value of the tenant is the subdomain itself, but that the tenant is resolved from a subdomain. Perhaps a wording of resolveFromSubdomain would be better?
A class with a number of handy request-based implementations of AuthenticationManagerResolver targeted at common multi-tenancy scenarios. Fixes: spring-projectsgh-6976
jzheaux
force-pushed
the
servlet-multitenancy
branch
from
49a77a9 to
a048f36
Compare
jzheaux
added
in: oauth2
|
Thanks @jzheaux! This looks good to me |
A class with a number of handy request-based implementations of
AuthenticationManager Resolver
Fixes; gh-6976