What problem does this solve? It seems like an uncommon use case. Scott Rossillo Smartling | Senior Software Engineer srossillo(a)smartling.com <mailto:srossillo@smartling.com> > On Dec 2, 2015, at 11:48 AM, Bill Burke <bburke(a)redhat.com > <mailto:bburke@redhat.com>> 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. > > > > -- > Bill Burke > JBoss, a division of Red Hat > http://bill.burkecentral.com > _______________________________________________ > keycloak-dev mailing list > keycloak-dev(a)lists.jboss.org > https://lists.jboss.org/mailman/listinfo/keycloak-dev

I think it would also help if you could limit which users are imported when synchronizing with LDAP. If only a small number of enterprise users are going to use the application(s) it doesn't make sense to synchronize them all (could be tens of thousands in total).

Best regards, Thomas On Dec 2, 2015 7:33 PM, "Bill Burke" <bburke(a)redhat.com&gt; wrote: > Some are balking at importing every user when doing federation with > backends like ldap even though only minimal import is needed (i.e. > username/providerID). > > On 12/2/2015 12:27 PM, Scott Rossillo wrote: > > What problem does this solve? It seems like an uncommon use case. > > > > > > Scott Rossillo > > Smartling | Senior Software Engineer > > srossillo(a)smartling.com <mailto:srossillo@smartling.com> > > > > > >> On Dec 2, 2015, at 11:48 AM, Bill Burke <bburke(a)redhat.com > >> <mailto:bburke@redhat.com>> 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. > >> > >> > >> > >> -- > >> Bill Burke > >> JBoss, a division of Red Hat > >> http://bill.burkecentral.com > >> _______________________________________________ > >> keycloak-dev mailing list > >> keycloak-dev(a)lists.jboss.org > >> https://lists.jboss.org/mailman/listinfo/keycloak-dev > > > > -- > Bill Burke > JBoss, a division of Red Hat > http://bill.burkecentral.com > _______________________________________________ > keycloak-dev mailing list > keycloak-dev(a)lists.jboss.org > https://lists.jboss.org/mailman/listinfo/keycloak-dev > _______________________________________________ keycloak-dev mailing list keycloak-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/keycloak-dev

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.

On 12/2/2015 10:53 PM, Adam Young wrote: >> 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} > I'd recommend this, and increasing the size of the Database column. > We have many deployments already and some have quite large user databases. Was really concerned that expanding the column size would piss a lot of people off because of the DB migration required.

We were talking a few weeks ago about ALL of our IDs becoming URIs. roles, groups, realms, clients, etc. It would give us the flexibility of federating anything we wanted going further.

For example, currently admins have to define roles and groups in keycloak. This sucks if they are already defined and controlled someplace else. > Keystoen did this, but then SHA256 hashed it, makuing it a 64 character > string. We found we were OK so long as we stayed under 255, as that was > the limit mysql imposed for the string columns. > >> 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. > It gets trickier: you don;'t want one federation provider to step on > the login from an other: ayoung@coke is a different user from > ayoung@pepsi. Having the Id be something issued by Keycloak + something > that comes from the Federated IdP is necessary. > Not really. This is no different than a user logging into the user/password screen. Keycloak allows you to specify the order of federation providers, but doesn't handle conflicts, i.e. one LDAP server has the same username as another.

> I like in memory, but there are many questions to answer, when a user > comes back a second time, what happens with their Id. I pushed for > ephemeral for Keystone, and we couldn't make it work. > We track sessions, have offline sessions, and track consents. So an ID cannot be temporary.

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.

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.

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.

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. >> >> >> >

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. >>>> >>>> >>>> >>> >> >

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/... 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@lists.jboss.orghttps://lists.jboss.org/mailman/listinfo/keycloak-dev _______________________________________________ keycloak-dev mailing list keycloak-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/keycloak-dev

On 12/4/2015 5:36 AM, 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. > I'm also trying to figure out something that is backward compatible with the current federation SPI