[keycloak-dev] in-memory only federated users

[Stian Thorgersen]

What about splitting the federation logic from the store logic?

We'd have multiple user store implementations:

* JPA
* Mongo
* LDAP
* Maybe something similar to PL IDM to allow people to easily use their own
schema for users

We could have store mappers as well in a mapping layer.

This would allow users to configure the user store for a specific realm. So
they can store everything about users in LDAP and not have to worry about
federation.

Then we'd have separate federation providers which could be re-used for
different store types. So it would look something like:

cache -> federation -> mappers -> store

In theory you could even use that to share users between multiple realms.
Have multiple separate sets of users in different dbs, ldap, etc..

I think it's an idea worth considering?



On 4 December 2015 at 11:41, Marek Posolda <mposolda at redhat.com> wrote:

> On 04/12/15 11:36, Marek Posolda wrote:
>
> Why it's bad to do simpler things? :-)
>
> AFAIK filter pattern (or interceptor/chain whatever you call it) is proven
> to work in many places. The provider at level X can always decide if it
> delegates call to method "getUserByXXX" to next provider (and then
> proxy/cache or do whatever according to his logic) or return something by
> itself.
>
> If I understand correctly your proposal, it requires
> UserFederationProvider to decide, if it wants to import or just return
> InMemoryUserModel . So if we want to support in-memory for LDAP, we will
> need to have 2 versions of LDAPFederationProvider (current which imports
> into userStorage() and another, which will return InMemoryUserModel
> instances ). That's not ideal IMO.
>
>
> As I mentioned before, there are also 2 additional usecases, which are
> important to support IMO:
>
> 1) Case when admin changes some user attributes directly in LDAP and he
> wants the LDAP data to be immediately visible in Keycloak. This is what we
> currently support (see
> FederationProvidersIntegrationTest.testDirectLDAPUpdate() ). Maybe I am
> missing something with your proposal, but if we hardcode CacheProvider to
> be always first, we lost this.
>
> 2) Case when admin doesn't change user attributes in LDAP directly, but
> rather prefer to save performance and read data from cache. In this case,
> admin configures the chain like you proposed: CacheProvider =>
> UserFederationManager => UserProvider
>
>
> IMO it will be cool to have single implementation of
> LDAPFederationProvider (and others), which works for both those cases and
> also for in-memory at the same time. Just let admin to decide how he want
> to configure chain of UserProviders, but not require UserFederationProvider
> itself to care about it.
>
>
> For my proposal, I assume that UserProvider will have just 2 new methods:
>
> UserProvider getNext();
> void setNext(UserProvider next);
>
>
> Only change in current UserFederationManager and DefaultCacheUserProvider
> is, that they will call "getNext()" when they need delegate. They don't
> care about what the delegate actually is, that's not their responsibility.
>
> For the "in-memory" provider, it might be the per-request in-memory
> provider (users stored just per-request). So if you have chain like:
> userFederationManager => inMemory
>
> Then assume the session.users().getUserByUsername is called:
> 1) First delegate is UserFederationManager, so calling
> UserFederationManager.getUserByUsername
> 2) This line
> https://github.com/keycloak/keycloak/blob/master/model/api/src/main/java/org/keycloak/models/UserFederationManager.java#L180
> will call getNext().getUserByUsername() and returns null as the user was
> not yet looked  for this request.
> 3) Going to federationProviders and call
> LDAPFederationProvider.getUserByUsername
> 4) LDAPFederationProvider query user in LDAP and calls importUserFromLDAP
> . This calls session.userStorage().addUser, which will put user into
> in-memory provider (I assume that session.userStorage() will be kept and
> will always point to the next delegate after UserFederationManager ). The
> LDAPFederationProvider will then return LDAP proxy of UserModel.
>
> The in-memory provider will also work with searching ( searchUser ) as
> federationLoad will first pre-load users into in-memory and then calls
> "query" and proxy users.
>
> The only limitation I can see now is calling of session.users().getUsers()
> as this doesn't preload users from federation. But if people add cache and
> use chain like:
> cache => federationManager => inMemory
>
> it will work fine and find all users retrieved from LDAP in any previous
> requests.
>
>
> In summary: UserProvider chaining is:
>
> 1) Very flexible
>
> 2) Supports in-memory, but also other use-cases too. It's all up to admin
> preference how to configure chain
>
> 3) No dependencies of providers on each other
>
> 4) Minimal changes to UserFederationManager and DefaultCacheUserProvider .
> Just need to call "getNext()" to retrieve next provider
>
> 5) Current UserFederationProvider will work fine for all cases and
> automatically gains "in-memory" support without need to change anything in
> their code. Assuming that for backwards compatibility, we will keep
> "session.userStorage()" to always point to next delegate of
> UserFederationManager . If it's JPA, then imports user like now. If it will
> be "in-memory" it will just return cache user for this request and return
> per-request inMemory user.
>
> 6) No need to increase the size of ID column for user table :-)
>
>
>
> Marek
>
>
> On 03/12/15 18:58, Bill Burke wrote:
>
> Still don't think it is as simple as you state.  We don't need an "in
> memory" provider.  We want UserFederationProvider to create a temporary
> request/only in-memory UserModel for federation providers that don't want
> to import.  This UserModel may be proxied for any write operations.
>
> My current thinking is that we change the flow from:
>
> UserFederationManager=>CacheProvider=>UserProvider
>
> to
>
> CacheProvider->UserFederationManager->UserProvider/UserFederationProvider
>
> KeycloakSession changes:
>
> * users() returns the CacheUserProvider instead of UserFederationManager
> * userStorage() is not changed
> * federationManager() returns UserFederationManager
>
> UserCacheProvider changes:
>
> * Gets a new method cache(UserModel user);
> * References UserFederationManager instead of the DB provider directly
>
> UserFederationManager changes:
> * Instead of calling userStorage(), it gets the DB provider directly
>
> UserFederationProvider:
> * Imports using userStorage() or, allocates a new class InMemoryUserModel
> (or extension of that class).  This class is just an in memory
> implementation of UserModel
> * Returns the imported UserModel or the InMemoryUserModel
>
> So
>
> session.users().getUserByXXXX() does the folloing:
>
> 1. UserCacheProvider.getUserByXXX is invoked
> 2. It looks looks in cache, if found return
> 3. invoke UserFederationManager.getUserByXXX
> 4. If UserFederationManager finds it return UserModel
> 5. Look in UserFederationProvider
> 6. UserFederationProvider imports or returns InMemoryUserModel
> 7. UserCacheProvider.getUserByXXX caches the user.
>
> cache.UserAdapter.getDelegateForUpdate() does the following:
> 1. invokes UserFederationManager.getUserById()
> 2. ID is parsed to see if it is a federated ID or not (see original post)
>
> Alternatively, we could just invoke
> userFederationManager.getUserByusername() since username can't be null.
>
>
>
> On 12/3/2015 11:59 AM, Marek Posolda wrote:
>
> On 03/12/15 16:57, Bill Burke wrote:
>
> Either we redo the federation SPI or work with the current one.
>
> It is just not as simple as you state.  You can't just chain in a
> generic InMemoryProvider.  Federation needs to be able to proxy the
> UserModel so that it can handle write methods if it wants to. Or
> delegate lookup of certain things to LDAP.
>
> I am not seeing why it's an issue? The InMemory will be kind of same
> thing like currently JPA. It just won't store the things into database,
> but into memory. That's the only difference. It will just be the
> provider at the end of the chain. UserFederationManager can proxy users
> exactly like now and doesn't require any code changes.
>
> So when admin configure:
>
> userFederationMAnager => inMemory
>
> The call of "session.userStorage()" from UserFederationManager will
> return underlying InMemory instead of current JPA.
>
> Also, UserFederationManager has to be first in the chain so that if
> something is found in cache, it can let the federation provider proxy
> the cache if it wants to.
>
> That's not a problem as well.
>
> What I mean is the flexibility to configure things exactly how you want
> for various cases.
>
> 3 basic setups:
>
> 1. userFederation => cache => JPA
>
> This is what we have now and it will be the default setup.  It 's useful
> for deployments when admins are often doing changes directly in their
> LDAP and they want the change imediatelly visible in Keycloak. So the
> UserFederationProvider proxy is always the top and when you call:
>
> user.getFirstName()
>
> you will retrieve the firstName from LDAP. The disadvantage of this
> setup is performance (each HTTP request needs to query LDAP like it's now)
>
>
> 2. cache => userFederation => JPA
>
> This will be useful for deployments when amins are not doing changes
> directly in their LDAP. So once you retrieve the user from
> LDAP+KeycloakDB, you will save him to cache and call to:
>
> user.getFirstName()
>
> will always return the value from cache. Hence when admin changes
> directly in LDAP, it won't be immediately visible in Keycloak.
>
> But on the other hand update from Keycloak to LDAP is not an issue. When
> you call:
>
> user.setFirstName("foo")
>
> the cache will call getDelegateForUpdate (exactly like now) and it will
> return proxy object, so the saving of firstName is propagated to LDAP
> (if it's writable) and to Keycloak JPA DB as well.
>
>
> 3. userFederation => inMemory
>
> The federation backed by pure in-memory storage. The federation proxy is
> on top, writing and reading to/from LDAP is not a problem and has
> preference over the content from memory. The only difference from (1) is
> that underlying backend is pure memory (infinispan) instead of JPA DB
>
> There is also alternative to use combination of 2 and 3:
> cache => userFederation => inMemory
>
> etc etc.
>
>
> I can see this as most flexible approach without dependencies of various
> providers on each other.
>
> Marek
>
>
> What we need is a special interface for the cache:
>
> cache.cacheUser(UserModel user);
>
> The cache would also work with UserFederationManager rather than a
> generic UserProvider. UserFederationManager would gain methods like:
> UserFederationManager.getUncachedUserById() which the cache would
> invoke.  UserFederationManager would break down the user id and either
> know it was local storage or something that would have to be delegated
> to a UserProvider.
>
>
>
>
>
> On 12/3/2015 10:32 AM, Marek Posolda wrote:
>
> IMO the more important use-case that in-memory federated users is the
> caching of federated users.
>
> Currently if you call: session.users().getUserById() and the user with
> ID "123" is LDAP (or other federationProvider) user, there is always
> call to UserFederationProvider.validateAndProxy , which results in LDAP
> query.
>
> If we introduce the chaining of UserProvider (something I already
> proposed earlier), you can switch UserFederationProvider with cache, so
> you will have:
> cache => userFederationManager => JPA
>
> instead of current:
>
> userFederationManager => cache => JPA
>
>
> With that in mind, we can easily implement in-memory as another
> implementation of UserProvider, which will hold users purely in-memory.
> Our current DefaultCacheUserProvider always require delegate to call
> write operations. But this in-memory provider would be something
> different. It won't use any delegate as it will be in the end of the
> chain. So for in-memory federation you will just configure:
>
> userFederationManager => inMemoryProvider
>
> and you're done. No needs for special ID handling or something like
> that.
>
> With chaining of UserProvider we have biggest flexibility for various
> needs IMO. That's why I would rather go this way TBH.
> Marek
>
> On 02/12/15 17:48, Bill Burke wrote:
>
> I'm looking into in-memory only no-import federated users.  What we
> would want to do is allow the UserFederationProvider to create an
> in-memory UserModel and allow for that UserModel to be cached via our
> current architecture.
>
> The current design assumes that all federated users are imported.  This
> includes our caching layer too!  To add to that, the user isn't cached
> until the 2nd request i.e.:
>
> 1. username/password page would hit the UserFederationProvider and the
> user would be imported into Keycloak.  This imported user is not
> cached,
> only imported into the database for this request's KeycloakSession
> 2. OTP Page or code 2 token would then want to lookup the user by id as
> that is what is stored in the ClientSession.  It would hit the keycloak
> database as it is not cached yet.  This lookup loads the cache for
> the user.
>
> Getting in-memory zero-import to work is even more tricky. The issue is
> that ClientSession and UserSession need to lookup clients by id.  If
> the
> user is not in cache, then the cache needs to lookup the user by id
> within storage.  This lookup also needs to happen if a write operation
> is performed on a cache user (getDelegateForUpdate()).  So, Keycloak
> needs to know that that ID is not in local storage and must be
> looked up
> from a fed provider.  The ID must be formed so that the provider fed
> provider can resolve the lookup.  I could use a URI for the ID i.e.
>
> fed:{providerId}:{login-name}
>
> The problem with this is that this id would need to be larger than 36
> characters which is the current column size for UserEntity.id and any
> other table that references users.   I could possibly do:
>
> fed:{providerAlias}:{login-name}
>
> But its quite possible that combination would be larger than 36
> characters.  We could also just shrink it to:
>
> fed:{login-name}
>
> But then we would have to iterate over every federation provider to
> find
> and load the user.
>
> So in summary:
> * IDs need to expand from 36 characters to something larger. (255
> maybe).  Don't some DBs have constraints on string primary key
> size?  DB
> scripts could possibly be
> * CachedUserProvider and UserFederationManager interfaces would need to
> be refactored
> * I don't think UserFederationProvider interface would need to change.
> But users would have to code for in-memory rather than throwing a
> switch
> to just turn it on.
>
>
>
>
>
>
>
>
>
>
> _______________________________________________
> keycloak-dev mailing listkeycloak-dev at lists.jboss.orghttps://lists.jboss.org/mailman/listinfo/keycloak-dev
>
>
>
> _______________________________________________
> keycloak-dev mailing list
> keycloak-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/keycloak-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.jboss.org/pipermail/keycloak-dev/attachments/20151204/38c1a4ef/attachment-0001.html