9:27 a.m.
Geez,
We now have a 6 levels of indirection:
KeycloakSession->ModelProvider->Model API->Hybrid API->Adapter->Entity
Too...much...abstraction.... It needs to be consolidated. When you're
done with the mongo store, I'd like to refactor and merge the model and
hybrid apis and get services/ to use it. We need this done before
beta-4 and beyond. I can't really finish my cache work until the
underlying API is on a solid foundation, and right now, IMO, it isn't.
--
Bill Burke
JBoss, a division of Red Hat
http://bill.burkecentral.com
9:35 a.m.
I've removed the model methods from KeycloakSession, so that's one level of
indirection gone.
Are you talking about dropping Model API?
----- Original Message -----
From: "Bill Burke" <bburke(a)redhat.com>
To: keycloak-dev(a)lists.jboss.org
Sent: Tuesday, 8 July, 2014 3:27:11 PM
Subject: [keycloak-dev] triple abstraction?
Geez,
We now have a 6 levels of indirection:
KeycloakSession->ModelProvider->Model API->Hybrid API->Adapter->Entity
Too...much...abstraction.... It needs to be consolidated. When you're
done with the mongo store, I'd like to refactor and merge the model and
hybrid apis and get services/ to use it. We need this done before
beta-4 and beyond. I can't really finish my cache work until the
underlying API is on a solid foundation, and right now, IMO, it isn't.
--
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
9:52 a.m.
On 7/8/2014 10:35 AM, Stian Thorgersen wrote:
I've removed the model methods from KeycloakSession, so
that's one level of indirection gone.
Are you talking about dropping Model API?
Dropping Model API or Hybrid API. Would probably be easier to drop
Hybrid and replace some Model API methods (i.e. addUser()) with
ModelProvider equivalent.
Maybe you can't because of cross-provider joins for things like roles?
Then again, maybe you should let the UserProvider decide how to convert
a role mapping to a referenced RoleModel and vice versa.
--
Bill Burke
JBoss, a division of Red Hat
http://bill.burkecentral.com
10:16 a.m.
Dropping Hybrid API sounds like pretty much undoing all the work I've done the last
couple weeks :(
IMO what we should do is to drop the Model API/SPI. Instead make it into a single
implementation that delegates to the various providers (RealmProvider, UserProvider,
SessionProvider and probably also CacheProvider). We can then merge this with
RealmManager, ApplicationManager, etc. and instead have a single ModelManager.
I don't think it's a good idea to make the UserProvider create RoleModels. The
UserProvider should be as simple as possible IMO, and shouldn't deal with Realms,
Apps, Roles, etc, other than through simple id's.
----- Original Message -----
From: "Bill Burke" <bburke(a)redhat.com>
To: "Stian Thorgersen" <stian(a)redhat.com>
Cc: keycloak-dev(a)lists.jboss.org
Sent: Tuesday, 8 July, 2014 3:52:10 PM
Subject: Re: [keycloak-dev] triple abstraction?
On 7/8/2014 10:35 AM, Stian Thorgersen wrote:
> I've removed the model methods from KeycloakSession, so that's one level of
> indirection gone.
>
> Are you talking about dropping Model API?
>
Dropping Model API or Hybrid API. Would probably be easier to drop
Hybrid and replace some Model API methods (i.e. addUser()) with
ModelProvider equivalent.
Maybe you can't because of cross-provider joins for things like roles?
Then again, maybe you should let the UserProvider decide how to convert
a role mapping to a referenced RoleModel and vice versa.
--
Bill Burke
JBoss, a division of Red Hat
http://bill.burkecentral.com
10:29 a.m.
On 7/8/2014 11:16 AM, Stian Thorgersen wrote:
Dropping Hybrid API sounds like pretty much undoing all the work
I've done the last couple weeks :(
I honestly have some serious concerns that this split will solve much.
See my previous email.
IMO what we should do is to drop the Model API/SPI. Instead make it
into a single implementation that delegates to the various providers (RealmProvider,
UserProvider, SessionProvider and probably also CacheProvider). We can then merge this
with RealmManager, ApplicationManager, etc. and instead have a single ModelManager.
I don't think it's a good idea to make the UserProvider create RoleModels. The
UserProvider should be as simple as possible IMO, and shouldn't deal with Realms,
Apps, Roles, etc, other than through simple id's.
That's not what I said. UserProvider may have to map between an
existing static user role mapping in some existing LDAP store to a role
defined in Keycloak. It may be impossible to map one to one a Keycloak
role id and the role mapping stored in the customer's user database.
Federating an existing user store is pretty hard to do even with the
current split. Not only does the UserProvider make some pretty steep
assumptions, tts pretty hard, IMO, to understand exactly where
AuthenticationProvider begins and the UserProvider takes over, or the
relationship between the two.
--
Bill Burke
JBoss, a division of Red Hat
http://bill.burkecentral.com
10:39 a.m.
----- Original Message -----
From: "Bill Burke" <bburke(a)redhat.com>
To: "Stian Thorgersen" <stian(a)redhat.com>
Cc: keycloak-dev(a)lists.jboss.org
Sent: Tuesday, 8 July, 2014 4:29:03 PM
Subject: Re: [keycloak-dev] triple abstraction?
On 7/8/2014 11:16 AM, Stian Thorgersen wrote:
> Dropping Hybrid API sounds like pretty much undoing all the work I've done
> the last couple weeks :(
>
I honestly have some serious concerns that this split will solve much.
See my previous email.
Which email?
> IMO what we should do is to drop the Model API/SPI. Instead make it into a
> single implementation that delegates to the various providers
> (RealmProvider, UserProvider, SessionProvider and probably also
> CacheProvider). We can then merge this with RealmManager,
> ApplicationManager, etc. and instead have a single ModelManager.
>
> I don't think it's a good idea to make the UserProvider create RoleModels.
> The UserProvider should be as simple as possible IMO, and shouldn't deal
> with Realms, Apps, Roles, etc, other than through simple id's.
>
That's not what I said. UserProvider may have to map between an
existing static user role mapping in some existing LDAP store to a role
defined in Keycloak. It may be impossible to map one to one a Keycloak
role id and the role mapping stored in the customer's user database.
Sure, I assumed the UserProvider would do that by returning the id of the Keycloak role it
maps to.
Federating an existing user store is pretty hard to do even with the
current split. Not only does the UserProvider make some pretty steep
assumptions, tts pretty hard, IMO, to understand exactly where
AuthenticationProvider begins and the UserProvider takes over, or the
relationship between the two.
See my previous email where I proposed that AuthenticationProvider could be dropped and
replaced by a delegating UserProvider with the same functionality.
--
Bill Burke
JBoss, a division of Red Hat
http://bill.burkecentral.com
11:01 a.m.
I thought we where at an agreement with splitting the model. If that's not the case,
and to be able to actually do a release sometime soon, I propose we just revert back to
how it was before.
For argument sake the benefits for this split include:
* Users are what we need to federate, this split makes that a lot simpler IMO
* Users are what people may want to store in their own schema, again this makes it a lot
simpler IMO
* Can store different things in different store types. For example realms in a json file,
users in a relational database and sessions in a in-mem data-grid
* Realms store can be changed if we need to. Relative quick and easy to export/import to
update.
* Users store shouldn't be changed frequently. Updating a database with 20 million
entries is non-trivial
* Model was getting pretty big and complex, with loads of tables. Splitting makes it
simpler IMO
----- Original Message -----
From: "Stian Thorgersen" <stian(a)redhat.com>
To: "Bill Burke" <bburke(a)redhat.com>
Cc: keycloak-dev(a)lists.jboss.org
Sent: Tuesday, 8 July, 2014 4:39:02 PM
Subject: Re: [keycloak-dev] triple abstraction?
----- Original Message -----
> From: "Bill Burke" <bburke(a)redhat.com>
> To: "Stian Thorgersen" <stian(a)redhat.com>
> Cc: keycloak-dev(a)lists.jboss.org
> Sent: Tuesday, 8 July, 2014 4:29:03 PM
> Subject: Re: [keycloak-dev] triple abstraction?
>
>
>
> On 7/8/2014 11:16 AM, Stian Thorgersen wrote:
> > Dropping Hybrid API sounds like pretty much undoing all the work I've
> > done
> > the last couple weeks :(
> >
>
> I honestly have some serious concerns that this split will solve much.
> See my previous email.
Which email?
>
> > IMO what we should do is to drop the Model API/SPI. Instead make it into
> > a
> > single implementation that delegates to the various providers
> > (RealmProvider, UserProvider, SessionProvider and probably also
> > CacheProvider). We can then merge this with RealmManager,
> > ApplicationManager, etc. and instead have a single ModelManager.
> >
> > I don't think it's a good idea to make the UserProvider create
> > RoleModels.
> > The UserProvider should be as simple as possible IMO, and shouldn't deal
> > with Realms, Apps, Roles, etc, other than through simple id's.
> >
>
> That's not what I said. UserProvider may have to map between an
> existing static user role mapping in some existing LDAP store to a role
> defined in Keycloak. It may be impossible to map one to one a Keycloak
> role id and the role mapping stored in the customer's user database.
Sure, I assumed the UserProvider would do that by returning the id of the
Keycloak role it maps to.
>
> Federating an existing user store is pretty hard to do even with the
> current split. Not only does the UserProvider make some pretty steep
> assumptions, tts pretty hard, IMO, to understand exactly where
> AuthenticationProvider begins and the UserProvider takes over, or the
> relationship between the two.
See my previous email where I proposed that AuthenticationProvider could be
dropped and replaced by a delegating UserProvider with the same
functionality.
>
> --
> 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
12:14 p.m.
On 7/8/2014 12:01 PM, Stian Thorgersen wrote:
I thought we where at an agreement with splitting the model. If
that's not the case, and to be able to actually do a release sometime soon, I propose
we just revert back to how it was before.
I did not know you were going to write a completely new persistence
abstraction underneath our existing persistence abstraction. I thought
the existing Model interfaces were going to be retained/refactored and
you were just going to split out how UserModel, Realm, and UserSessions
were looked up.
For argument sake the benefits for this split include:
* Users are what we need to federate, this split makes that a lot simpler IMO
* Users are what people may want to store in their own schema, again this makes it a lot
simpler IMO
* Can store different things in different store types. For example realms in a json file,
users in a relational database and sessions in a in-mem data-grid
* Realms store can be changed if we need to. Relative quick and easy to export/import to
update.
* Users store shouldn't be changed frequently. Updating a database with 20 million
entries is non-trivial
All you did to improve the 20 million use case was remove secondary key
constraints from UserEntity and UserRoleMappingEntity, which could have
been done in the existing model.
* Model was getting pretty big and complex, with loads of tables.
Splitting makes it simpler IMO
Bah! our data model is not complex!
Summary of my perspective
* Model API was written so that the implementation details of storage
wouldn't be exposed to the service layer. So, if we retired the Model
API in favor of using the Hybrid API, the service layer would be dealing
with user ids, role ids, realm ids, client ids, instead of UserModel,
RoleModel, RealmModel, and ClientModel.
* The Adapter classes existed to bridge between the Model API and the
persistence API. I think you lost that notion in the Hybrid model as it
is completely String ID based.
* The line between AuthentcationProvider and UserProvider is now blurred
and confusing as they both provide a mechanism to create and look up
users. I'm not even sure you could have a LDAP store that is both a
UserProvider and AuthenticationProvider.
--
Bill Burke
JBoss, a division of Red Hat
http://bill.burkecentral.com
5 a.m.
Okay, this all hybrid stuff ended up taking more time and becoming more complex than I
hoped. There's also still work to be done, and even once that's completed
there'd still be more to do.
I think the correct path forward is to:
1. Recover the JPA model provider
2. Branch the Hybrid model provider
3. Delete the Hybrid model provider from master
Then we can consider what changes if any to do to the model. I'd like to keep the
Hybrid model provider in a branch for a while as there are some things in there that may
be useful.
A few things that we should consider include:
* Can we reduce the need to update database schemas in the future? I think by cleaning up
the entities, and also replacing a lot of columns with attributes we can. For example
adding an additional config field to RealmModel shouldn't require us updating the
database schema
* If we need to update database schemas how do we do this? Export/import would work for
realms, but not for users I think (users would just require a too big downtime if
there's millions)
* Do we want to store sessions in memory?
* Cache deals with realms well, but does this work for users or do we just need to make
sure loading users from the database performs well?
* With regards to the above I was wondering if we could prevent loading users from
cache/db to refresh a token, or to logout a user. If a user is deleted or disabled the
session should be deleted, so the only reason I can see to check this is if role mappings
have changed, and we could add a dirtyRoles flag to the session for this. Basically
dirtyRoles is set to true on all users sessions whenever role-mappings are updated, we
would then reload the user/role-mappings on refresh tokens only if dirtyRoles is true..
This would result in us only having to load the user from db once for a
"session", and hence there shouldn't really be a need to cache it.
* Do we want to be able to store realms and users separately?
----- Original Message -----
From: "Bill Burke" <bburke(a)redhat.com>
To: "Stian Thorgersen" <stian(a)redhat.com>
Cc: keycloak-dev(a)lists.jboss.org
Sent: Tuesday, 8 July, 2014 6:14:35 PM
Subject: Re: [keycloak-dev] triple abstraction?
On 7/8/2014 12:01 PM, Stian Thorgersen wrote:
> I thought we where at an agreement with splitting the model. If that's not
> the case, and to be able to actually do a release sometime soon, I propose
> we just revert back to how it was before.
>
I did not know you were going to write a completely new persistence
abstraction underneath our existing persistence abstraction. I thought
the existing Model interfaces were going to be retained/refactored and
you were just going to split out how UserModel, Realm, and UserSessions
were looked up.
> For argument sake the benefits for this split include:
>
> * Users are what we need to federate, this split makes that a lot simpler
> IMO
> * Users are what people may want to store in their own schema, again this
> makes it a lot simpler IMO
> * Can store different things in different store types. For example realms
> in a json file, users in a relational database and sessions in a in-mem
> data-grid
> * Realms store can be changed if we need to. Relative quick and easy to
> export/import to update.
> * Users store shouldn't be changed frequently. Updating a database with 20
> million entries is non-trivial
All you did to improve the 20 million use case was remove secondary key
constraints from UserEntity and UserRoleMappingEntity, which could have
been done in the existing model.
> * Model was getting pretty big and complex, with loads of tables. Splitting
> makes it simpler IMO
>
Bah! our data model is not complex!
Summary of my perspective
* Model API was written so that the implementation details of storage
wouldn't be exposed to the service layer. So, if we retired the Model
API in favor of using the Hybrid API, the service layer would be dealing
with user ids, role ids, realm ids, client ids, instead of UserModel,
RoleModel, RealmModel, and ClientModel.
* The Adapter classes existed to bridge between the Model API and the
persistence API. I think you lost that notion in the Hybrid model as it
is completely String ID based.
* The line between AuthentcationProvider and UserProvider is now blurred
and confusing as they both provide a mechanism to create and look up
users. I'm not even sure you could have a LDAP store that is both a
UserProvider and AuthenticationProvider.
--
Bill Burke
JBoss, a division of Red Hat
http://bill.burkecentral.com
8:33 a.m.
On 7/9/2014 6:00 AM, Stian Thorgersen wrote:
Okay, this all hybrid stuff ended up taking more time and becoming
more complex than I hoped. There's also still work to be done, and even once
that's completed there'd still be more to do.
I think the correct path forward is to:
1. Recover the JPA model provider
We should. Is that hard to do?
2. Branch the Hybrid model provider
3. Delete the Hybrid model provider from master
Then we can consider what changes if any to do to the model. I'd like to keep the
Hybrid model provider in a branch for a while as there are some things in there that may
be useful.
Are you sure? After thinking about things overnight, there's a lot of
things hybrid model solved. What freaked me out was that I thought the
Model API was just going to be refactored and the ModelProvider split
into RealmProvider, UserProvider, and UserSessionProvider.
A few things that we should consider include:
* Can we reduce the need to update database schemas in the future? I think by cleaning up
the entities, and also replacing a lot of columns with attributes we can. For example
adding an additional config field to RealmModel shouldn't require us updating the
database schema
Your hybrid implementation made me realize how important this will be to
do across the board for Realm, Apps, Users, etc. But this should be a
storage implementation detail, right? And not part of the Model API?
Please? :)
i.e. the brute force settings could be stored as attributes, but the
model API would still expose them as getter methods. I hate doing
things like map.get("maxFailures") vs. realm.getMaxfailures(). Having
actual methods in the model api makes refactoring much much easier too.
* If we need to update database schemas how do we do this?
Export/import would work for realms, but not for users I think (users would just require a
too big downtime if there's millions)
If we change, for instance, how user "required actions" are stored,
wouldn't we have to write a migration script anyways? IIRC, adding
tables or altering a table to add a column isn't such a big deal. What
is a big deal is removing a column.
Maybe we should just provide migration scripts in between releases that
require them?
* Do we want to store sessions in memory?
Don't we need both persistence and in-memory storage for sessions? For
1.0 we're not going to test clustering configurations, but, shouldn't it
still be possible to cluster Keycloak if the cache is shut off and
persistence session implementation is used?
* Cache deals with realms well, but does this work for users or do we
just need to make sure loading users from the database performs well?
It works for users.
* With regards to the above I was wondering if we could prevent
loading users from cache/db to refresh a token, or to logout a user. If a user is deleted
or disabled the session should be deleted, so the only reason I can see to check this is
if role mappings have changed, and we could add a dirtyRoles flag to the session for this.
Basically dirtyRoles is set to true on all users sessions whenever role-mappings are
updated, we would then reload the user/role-mappings on refresh tokens only if dirtyRoles
is true.. This would result in us only having to load the user from db once for a
"session", and hence there shouldn't really be a need to cache it.
The cache implementation handles grantRole() by invalidating the user in
the cache. It doesn't handle role removal. But it does lookup the
role to build the role mapping list.
@Override
public Set<RoleModel> getRoleMappings() {
if (updated != null) return updated.getRoleMappings();
Set<RoleModel> roles = new HashSet<RoleModel>();
for (String id : cached.getRoleMappings()) {
roles.add(cacheSession.getRoleById(id, realm));
}
return roles;
}
Maybe it could be as easy as just invalidating the user and delegating
to the persistence model if the role from the role mapping doesn't exist?
* Do we want to be able to store realms and users separately?
I don't know. We need a sync API architecture to be able to support all
keycloak features. This means we need to be able to store user metadata
for each user.
These steps should be taken IMO:
* bring back JPA model
* Retire Hybrid....BUT...ARE YOU SURE?!!! You usually make better
decisions than me...
* Split Model API and JPA/Mongo impls into RealmProvider, UserProvider,
and UserSessionProvider.
* Keep Hybrid in codebase until we're done with ModelProvider refactoring
* Move Hybrid to branch
* Change JPA implementation to be more generic. Attribute based for
many settings.
* Rethink AuthenticationProvider SPI. Users really want to be able to
delegate to their own database or LDAP storage and we need to provide
better flexibility.
--
Bill Burke
JBoss, a division of Red Hat
http://bill.burkecentral.com
9:19 a.m.
Oki, so how about:
1. Recover the JPA model provider, and set that as the default
2. Retire, but keep Hybrid model around until we're done
3. Extract user sessions from ModelProvider into UserSessionProvider, and add
mem/jpa/mongo implementations
4. Review what we have, and make sure everyone is happy with the approach taken for
UserSessionProvider
5. Extract users and role-mappings from ModelProvider into UserProvider, and add jpa/mongo
implementations
6. Again, let's review and make sure everyone is happy
7. Rename ModelProvider to RealmProvider
8. Consider refactorings to the models (such as using attributes instead of columns)
I also think we'll need a EntityManagerProvider and a MongoClientProvider to make sure
we have a single connection/transaction per-request.
For 1.0.final I think we could introduce a limitation that we'll only allow one store
at a time, so we don't have to deal with multiple-transactions (or 2pc).
More comments inline
----- Original Message -----
From: "Bill Burke" <bburke(a)redhat.com>
To: "Stian Thorgersen" <stian(a)redhat.com>
Cc: keycloak-dev(a)lists.jboss.org
Sent: Wednesday, 9 July, 2014 2:33:48 PM
Subject: Re: [keycloak-dev] triple abstraction?
On 7/9/2014 6:00 AM, Stian Thorgersen wrote:
> Okay, this all hybrid stuff ended up taking more time and becoming more
> complex than I hoped. There's also still work to be done, and even once
> that's completed there'd still be more to do.
>
> I think the correct path forward is to:
>
> 1. Recover the JPA model provider
We should. Is that hard to do?
Nopes, already have it in a branch just need to merge it ;)
> 2. Branch the Hybrid model provider
> 3. Delete the Hybrid model provider from master
>
> Then we can consider what changes if any to do to the model. I'd like to
> keep the Hybrid model provider in a branch for a while as there are some
> things in there that may be useful.
>
Are you sure? After thinking about things overnight, there's a lot of
things hybrid model solved. What freaked me out was that I thought the
Model API was just going to be refactored and the ModelProvider split
into RealmProvider, UserProvider, and UserSessionProvider.
Yes - I attacked this the wrong way around, and should have done it in the way I've
suggested above.
> A few things that we should consider include:
>
> * Can we reduce the need to update database schemas in the future? I think
> by cleaning up the entities, and also replacing a lot of columns with
> attributes we can. For example adding an additional config field to
> RealmModel shouldn't require us updating the database schema
Your hybrid implementation made me realize how important this will be to
do across the board for Realm, Apps, Users, etc. But this should be a
storage implementation detail, right? And not part of the Model API?
Please? :)
i.e. the brute force settings could be stored as attributes, but the
model API would still expose them as getter methods. I hate doing
things like map.get("maxFailures") vs. realm.getMaxfailures(). Having
actual methods in the model api makes refactoring much much easier too.
How about:
int maxFailures = map.containsKey("maxFailures") ?
Integer.parseInt(map.get("maxFailures)) : 0;
Just kidding, it would be insane to do that in services ;)
> * If we need to update database schemas how do we do this? Export/import
> would work for realms, but not for users I think (users would just require
> a too big downtime if there's millions)
If we change, for instance, how user "required actions" are stored,
wouldn't we have to write a migration script anyways? IIRC, adding
tables or altering a table to add a column isn't such a big deal. What
is a big deal is removing a column.
Maybe we should just provide migration scripts in between releases that
require them?
If stored as plain attributes, Keycloak could rewrite them on a live server.
Migration scripts may be the only feasible way to go, but I'm worried about the
complexity of writing and testing these scripts for all the different DB variations,
including Mongo.
> * Do we want to store sessions in memory?
Don't we need both persistence and in-memory storage for sessions? For
1.0 we're not going to test clustering configurations, but, shouldn't it
still be possible to cluster Keycloak if the cache is shut off and
persistence session implementation is used?
Sure, let's do both. In the future we could consider Infinispan, or something
similar.
> * Cache deals with realms well, but does this work for users or do we just
> need to make sure loading users from the database performs well?
It works for users.
My point was, if we can tweak things so we only have to retrieve users from the database
on login do we even need to cache users? It would make the overhead of a distributed cache
significantly smaller, as admins are not going to update realms and such frequently, but
if you have a large number of users they'll be changing passwords, profiles, etc, all
the time.
I'm thinking:
* Realms - are own cache with the http invalidation messages
* Sessions - Infinispan
* Users - no cache, we can retrieve the info we need from tokens and session
> * With regards to the above I was wondering if we could prevent loading
> users from cache/db to refresh a token, or to logout a user. If a user is
> deleted or disabled the session should be deleted, so the only reason I
> can see to check this is if role mappings have changed, and we could add a
> dirtyRoles flag to the session for this. Basically dirtyRoles is set to
> true on all users sessions whenever role-mappings are updated, we would
> then reload the user/role-mappings on refresh tokens only if dirtyRoles is
> true.. This would result in us only having to load the user from db once
> for a "session", and hence there shouldn't really be a need to cache
it.
The cache implementation handles grantRole() by invalidating the user in
the cache. It doesn't handle role removal. But it does lookup the
role to build the role mapping list.
@Override
public Set<RoleModel> getRoleMappings() {
if (updated != null) return updated.getRoleMappings();
Set<RoleModel> roles = new HashSet<RoleModel>();
for (String id : cached.getRoleMappings()) {
roles.add(cacheSession.getRoleById(id, realm));
}
return roles;
}
Maybe it could be as easy as just invalidating the user and delegating
to the persistence model if the role from the role mapping doesn't exist?
> * Do we want to be able to store realms and users separately?
>
I don't know. We need a sync API architecture to be able to support all
keycloak features. This means we need to be able to store user metadata
for each user.
These steps should be taken IMO:
* bring back JPA model
* Retire Hybrid....BUT...ARE YOU SURE?!!! You usually make better
decisions than me...
* Split Model API and JPA/Mongo impls into RealmProvider, UserProvider,
and UserSessionProvider.
* Keep Hybrid in codebase until we're done with ModelProvider refactoring
* Move Hybrid to branch
* Change JPA implementation to be more generic. Attribute based for
many settings.
* Rethink AuthenticationProvider SPI. Users really want to be able to
delegate to their own database or LDAP storage and we need to provide
better flexibility.
--
Bill Burke
JBoss, a division of Red Hat
http://bill.burkecentral.com
10:24 a.m.
I've reverted the JPA model provider, which is now the default in master.
I also started a user session provider and changed the mem session provider from hybrid to
implement this instead. Have a look at it at:
https://github.com/stianst/keycloak/tree/user-session-provider
Specifically at:
https://github.com/stianst/keycloak/blob/user-session-provider/model/api/...
How simple this was pretty much verifies to me that the approach I took previously was
utterly wrong!
Let's have a Hangout tomorrow?
----- Original Message -----
From: "Stian Thorgersen" <stian(a)redhat.com>
To: "Bill Burke" <bburke(a)redhat.com>
Cc: keycloak-dev(a)lists.jboss.org
Sent: Wednesday, 9 July, 2014 3:19:28 PM
Subject: Re: [keycloak-dev] triple abstraction?
Oki, so how about:
1. Recover the JPA model provider, and set that as the default
2. Retire, but keep Hybrid model around until we're done
3. Extract user sessions from ModelProvider into UserSessionProvider, and add
mem/jpa/mongo implementations
4. Review what we have, and make sure everyone is happy with the approach
taken for UserSessionProvider
5. Extract users and role-mappings from ModelProvider into UserProvider, and
add jpa/mongo implementations
6. Again, let's review and make sure everyone is happy
7. Rename ModelProvider to RealmProvider
8. Consider refactorings to the models (such as using attributes instead of
columns)
I also think we'll need a EntityManagerProvider and a MongoClientProvider to
make sure we have a single connection/transaction per-request.
For 1.0.final I think we could introduce a limitation that we'll only allow
one store at a time, so we don't have to deal with multiple-transactions (or
2pc).
More comments inline
----- Original Message -----
> From: "Bill Burke" <bburke(a)redhat.com>
> To: "Stian Thorgersen" <stian(a)redhat.com>
> Cc: keycloak-dev(a)lists.jboss.org
> Sent: Wednesday, 9 July, 2014 2:33:48 PM
> Subject: Re: [keycloak-dev] triple abstraction?
>
>
>
> On 7/9/2014 6:00 AM, Stian Thorgersen wrote:
> > Okay, this all hybrid stuff ended up taking more time and becoming more
> > complex than I hoped. There's also still work to be done, and even once
> > that's completed there'd still be more to do.
> >
> > I think the correct path forward is to:
> >
> > 1. Recover the JPA model provider
>
> We should. Is that hard to do?
Nopes, already have it in a branch just need to merge it ;)
>
> > 2. Branch the Hybrid model provider
> > 3. Delete the Hybrid model provider from master
> >
> > Then we can consider what changes if any to do to the model. I'd like to
> > keep the Hybrid model provider in a branch for a while as there are some
> > things in there that may be useful.
> >
>
> Are you sure? After thinking about things overnight, there's a lot of
> things hybrid model solved. What freaked me out was that I thought the
> Model API was just going to be refactored and the ModelProvider split
> into RealmProvider, UserProvider, and UserSessionProvider.
Yes - I attacked this the wrong way around, and should have done it in the
way I've suggested above.
>
> > A few things that we should consider include:
> >
> > * Can we reduce the need to update database schemas in the future? I
> > think
> > by cleaning up the entities, and also replacing a lot of columns with
> > attributes we can. For example adding an additional config field to
> > RealmModel shouldn't require us updating the database schema
>
> Your hybrid implementation made me realize how important this will be to
> do across the board for Realm, Apps, Users, etc. But this should be a
> storage implementation detail, right? And not part of the Model API?
> Please? :)
>
> i.e. the brute force settings could be stored as attributes, but the
> model API would still expose them as getter methods. I hate doing
> things like map.get("maxFailures") vs. realm.getMaxfailures(). Having
> actual methods in the model api makes refactoring much much easier too.
How about:
int maxFailures = map.containsKey("maxFailures") ?
Integer.parseInt(map.get("maxFailures)) : 0;
Just kidding, it would be insane to do that in services ;)
>
>
> > * If we need to update database schemas how do we do this? Export/import
> > would work for realms, but not for users I think (users would just
> > require
> > a too big downtime if there's millions)
>
> If we change, for instance, how user "required actions" are stored,
> wouldn't we have to write a migration script anyways? IIRC, adding
> tables or altering a table to add a column isn't such a big deal. What
> is a big deal is removing a column.
>
> Maybe we should just provide migration scripts in between releases that
> require them?
If stored as plain attributes, Keycloak could rewrite them on a live server.
Migration scripts may be the only feasible way to go, but I'm worried about
the complexity of writing and testing these scripts for all the different DB
variations, including Mongo.
>
> > * Do we want to store sessions in memory?
>
> Don't we need both persistence and in-memory storage for sessions? For
> 1.0 we're not going to test clustering configurations, but, shouldn't it
> still be possible to cluster Keycloak if the cache is shut off and
> persistence session implementation is used?
Sure, let's do both. In the future we could consider Infinispan, or something
similar.
>
> > * Cache deals with realms well, but does this work for users or do we
> > just
> > need to make sure loading users from the database performs well?
>
> It works for users.
My point was, if we can tweak things so we only have to retrieve users from
the database on login do we even need to cache users? It would make the
overhead of a distributed cache significantly smaller, as admins are not
going to update realms and such frequently, but if you have a large number
of users they'll be changing passwords, profiles, etc, all the time.
I'm thinking:
* Realms - are own cache with the http invalidation messages
* Sessions - Infinispan
* Users - no cache, we can retrieve the info we need from tokens and session
>
> > * With regards to the above I was wondering if we could prevent loading
> > users from cache/db to refresh a token, or to logout a user. If a user is
> > deleted or disabled the session should be deleted, so the only reason I
> > can see to check this is if role mappings have changed, and we could add
> > a
> > dirtyRoles flag to the session for this. Basically dirtyRoles is set to
> > true on all users sessions whenever role-mappings are updated, we would
> > then reload the user/role-mappings on refresh tokens only if dirtyRoles
> > is
> > true.. This would result in us only having to load the user from db once
> > for a "session", and hence there shouldn't really be a need to
cache it.
>
> The cache implementation handles grantRole() by invalidating the user in
> the cache. It doesn't handle role removal. But it does lookup the
> role to build the role mapping list.
>
> @Override
> public Set<RoleModel> getRoleMappings() {
> if (updated != null) return updated.getRoleMappings();
> Set<RoleModel> roles = new HashSet<RoleModel>();
> for (String id : cached.getRoleMappings()) {
> roles.add(cacheSession.getRoleById(id, realm));
>
> }
> return roles;
> }
>
>
>
> Maybe it could be as easy as just invalidating the user and delegating
> to the persistence model if the role from the role mapping doesn't exist?
>
> > * Do we want to be able to store realms and users separately?
> >
>
> I don't know. We need a sync API architecture to be able to support all
> keycloak features. This means we need to be able to store user metadata
> for each user.
>
> These steps should be taken IMO:
>
> * bring back JPA model
> * Retire Hybrid....BUT...ARE YOU SURE?!!! You usually make better
> decisions than me...
> * Split Model API and JPA/Mongo impls into RealmProvider, UserProvider,
> and UserSessionProvider.
> * Keep Hybrid in codebase until we're done with ModelProvider refactoring
> * Move Hybrid to branch
> * Change JPA implementation to be more generic. Attribute based for
> many settings.
> * Rethink AuthenticationProvider SPI. Users really want to be able to
> delegate to their own database or LDAP storage and we need to provide
> better flexibility.
>
> --
> 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
10:35 a.m.
On 7/9/2014 11:24 AM, Stian Thorgersen wrote:
I've reverted the JPA model provider, which is now the default in
master.
I also started a user session provider and changed the mem session provider from hybrid
to implement this instead. Have a look at it at:
https://github.com/stianst/keycloak/tree/user-session-provider
Specifically at:
https://github.com/stianst/keycloak/blob/user-session-provider/model/api/...
How simple this was pretty much verifies to me that the approach I took previously was
utterly wrong!
Looks good so far. Let me know if I can help with anything as again,
I"m kind of in a holding pattern.
Bill
--
Bill Burke
JBoss, a division of Red Hat
http://bill.burkecentral.com
2:19 a.m.
Ping me when you're online and we can have a Hangout to divide this work
----- Original Message -----
From: "Bill Burke" <bburke(a)redhat.com>
To: "Stian Thorgersen" <stian(a)redhat.com>
Cc: keycloak-dev(a)lists.jboss.org
Sent: Wednesday, 9 July, 2014 4:35:47 PM
Subject: Re: [keycloak-dev] triple abstraction?
On 7/9/2014 11:24 AM, Stian Thorgersen wrote:
> I've reverted the JPA model provider, which is now the default in master.
>
> I also started a user session provider and changed the mem session provider
> from hybrid to implement this instead. Have a look at it at:
>
> https://github.com/stianst/keycloak/tree/user-session-provider
>
> Specifically at:
>
>
https://github.com/stianst/keycloak/blob/user-session-provider/model/api/...
>
> How simple this was pretty much verifies to me that the approach I took
> previously was utterly wrong!
>
Looks good so far. Let me know if I can help with anything as again,
I"m kind of in a holding pattern.
Bill
--
Bill Burke
JBoss, a division of Red Hat
http://bill.burkecentral.com
5:43 a.m.
A suggestion to how we can split this work:
Stian: Finish the UserSessionProvider (including mem, jpa and mongo implementations). Then
I can do the EntityManagerProvider and MongoConnectionProvider, then look at
transactions.
Bill: Split the remainder of the ModelProvider into RealmProvider and UserProvider, and at
the same time do the cache work.
Marek: Work on import/export to use same representations as admin endpoints.
----- Original Message -----
From: "Stian Thorgersen" <stian(a)redhat.com>
To: "Bill Burke" <bburke(a)redhat.com>
Cc: keycloak-dev(a)lists.jboss.org
Sent: Thursday, 10 July, 2014 8:19:16 AM
Subject: Re: [keycloak-dev] triple abstraction?
Ping me when you're online and we can have a Hangout to divide this work
----- Original Message -----
> From: "Bill Burke" <bburke(a)redhat.com>
> To: "Stian Thorgersen" <stian(a)redhat.com>
> Cc: keycloak-dev(a)lists.jboss.org
> Sent: Wednesday, 9 July, 2014 4:35:47 PM
> Subject: Re: [keycloak-dev] triple abstraction?
>
>
>
> On 7/9/2014 11:24 AM, Stian Thorgersen wrote:
> > I've reverted the JPA model provider, which is now the default in master.
> >
> > I also started a user session provider and changed the mem session
> > provider
> > from hybrid to implement this instead. Have a look at it at:
> >
> > https://github.com/stianst/keycloak/tree/user-session-provider
> >
> > Specifically at:
> >
> >
https://github.com/stianst/keycloak/blob/user-session-provider/model/api/...
> >
> > How simple this was pretty much verifies to me that the approach I took
> > previously was utterly wrong!
> >
>
> Looks good so far. Let me know if I can help with anything as again,
> I"m kind of in a holding pattern.
>
> Bill
>
> --
> 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
6:46 a.m.
On 10.7.2014 12:43, Stian Thorgersen wrote:
A suggestion to how we can split this work:
Stian: Finish the UserSessionProvider (including mem, jpa and mongo implementations).
Then I can do the EntityManagerProvider and MongoConnectionProvider, then look at
transactions.
Bill: Split the remainder of the ModelProvider into RealmProvider and UserProvider, and
at the same time do the cache work.
Marek: Work on import/export to use same representations as admin endpoints.
Yeah,
I am already working in export/import now. I am using
ModelProvider atm I believe it won't be hard to refactor later once
model split is fully finished.
Marek
----- Original Message -----
> From: "Stian Thorgersen" <stian(a)redhat.com>
> To: "Bill Burke" <bburke(a)redhat.com>
> Cc: keycloak-dev(a)lists.jboss.org
> Sent: Thursday, 10 July, 2014 8:19:16 AM
> Subject: Re: [keycloak-dev] triple abstraction?
>
> Ping me when you're online and we can have a Hangout to divide this work
>
> ----- Original Message -----
>> From: "Bill Burke" <bburke(a)redhat.com>
>> To: "Stian Thorgersen" <stian(a)redhat.com>
>> Cc: keycloak-dev(a)lists.jboss.org
>> Sent: Wednesday, 9 July, 2014 4:35:47 PM
>> Subject: Re: [keycloak-dev] triple abstraction?
>>
>>
>>
>> On 7/9/2014 11:24 AM, Stian Thorgersen wrote:
>>> I've reverted the JPA model provider, which is now the default in
master.
>>>
>>> I also started a user session provider and changed the mem session
>>> provider
>>> from hybrid to implement this instead. Have a look at it at:
>>>
>>> https://github.com/stianst/keycloak/tree/user-session-provider
>>>
>>> Specifically at:
>>>
>>>
https://github.com/stianst/keycloak/blob/user-session-provider/model/api/...
>>>
>>> How simple this was pretty much verifies to me that the approach I took
>>> previously was utterly wrong!
>>>
>> Looks good so far. Let me know if I can help with anything as again,
>> I"m kind of in a holding pattern.
>>
>> Bill
>>
>> --
>> 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
10:51 a.m.
Added UserProvider with mem implementation. JPA and Mongo coming up next.
----- Original Message -----
From: "Marek Posolda" <mposolda(a)redhat.com>
To: "Stian Thorgersen" <stian(a)redhat.com>, "Bill Burke"
<bburke(a)redhat.com>
Cc: keycloak-dev(a)lists.jboss.org
Sent: Thursday, 10 July, 2014 12:46:32 PM
Subject: Re: [keycloak-dev] triple abstraction?
On 10.7.2014 12:43, Stian Thorgersen wrote:
> A suggestion to how we can split this work:
>
> Stian: Finish the UserSessionProvider (including mem, jpa and mongo
> implementations). Then I can do the EntityManagerProvider and
> MongoConnectionProvider, then look at transactions.
>
> Bill: Split the remainder of the ModelProvider into RealmProvider and
> UserProvider, and at the same time do the cache work.
>
> Marek: Work on import/export to use same representations as admin
> endpoints.
Yeah, I am already working in export/import now. I am using
ModelProvider atm I believe it won't be hard to refactor later once
model split is fully finished.
Marek
>
> ----- Original Message -----
>> From: "Stian Thorgersen" <stian(a)redhat.com>
>> To: "Bill Burke" <bburke(a)redhat.com>
>> Cc: keycloak-dev(a)lists.jboss.org
>> Sent: Thursday, 10 July, 2014 8:19:16 AM
>> Subject: Re: [keycloak-dev] triple abstraction?
>>
>> Ping me when you're online and we can have a Hangout to divide this work
>>
>> ----- Original Message -----
>>> From: "Bill Burke" <bburke(a)redhat.com>
>>> To: "Stian Thorgersen" <stian(a)redhat.com>
>>> Cc: keycloak-dev(a)lists.jboss.org
>>> Sent: Wednesday, 9 July, 2014 4:35:47 PM
>>> Subject: Re: [keycloak-dev] triple abstraction?
>>>
>>>
>>>
>>> On 7/9/2014 11:24 AM, Stian Thorgersen wrote:
>>>> I've reverted the JPA model provider, which is now the default in
>>>> master.
>>>>
>>>> I also started a user session provider and changed the mem session
>>>> provider
>>>> from hybrid to implement this instead. Have a look at it at:
>>>>
>>>> https://github.com/stianst/keycloak/tree/user-session-provider
>>>>
>>>> Specifically at:
>>>>
>>>>
https://github.com/stianst/keycloak/blob/user-session-provider/model/api/...
>>>>
>>>> How simple this was pretty much verifies to me that the approach I took
>>>> previously was utterly wrong!
>>>>
>>> Looks good so far. Let me know if I can help with anything as again,
>>> I"m kind of in a holding pattern.
>>>
>>> Bill
>>>
>>> --
>>> 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
10:28 a.m.
On 7/9/2014 10:19 AM, Stian Thorgersen wrote:
Oki, so how about:
1. Recover the JPA model provider, and set that as the default
2. Retire, but keep Hybrid model around until we're done
3. Extract user sessions from ModelProvider into UserSessionProvider, and add
mem/jpa/mongo implementations
4. Review what we have, and make sure everyone is happy with the approach taken for
UserSessionProvider
5. Extract users and role-mappings from ModelProvider into UserProvider, and add
jpa/mongo implementations
6. Again, let's review and make sure everyone is happy
7. Rename ModelProvider to RealmProvider
8. Consider refactorings to the models (such as using attributes instead of columns)
9. We need to rethink AuthenticationProvider.
I'm willing to do any/all of that work. It overlaps with cache changes
I'd need to make. I'm kind of in a holding pattern anyways until all
the above is resolved.
I also think we'll need a EntityManagerProvider and a
MongoClientProvider to make sure we have a single connection/transaction per-request.
For 1.0.final I think we could introduce a limitation that we'll only allow one store
at a time, so we don't have to deal with multiple-transactions (or 2pc).
We will have 2pc problems when we eventually have a distributed cache.
More comments inline.
>
>> * Cache deals with realms well, but does this work for users or do we just
>> need to make sure loading users from the database performs well?
>
> It works for users.
My point was, if we can tweak things so we only have to retrieve users from the database
on login do we even need to cache users? It would make the overhead of a distributed cache
significantly smaller, as admins are not going to update realms and such frequently, but
if you have a large number of users they'll be changing passwords, profiles, etc, all
the time.
IMO, users will usually be static too. Not as static as realm metadata,
but they won't change daily, even weekly.
The only overhead of a distributed cache for users is when a user is
updated. And the only overhead is a simple invalidation message to
invalidate the user.
I'm thinking:
* Realms - are own cache with the http invalidation messages
* Sessions - Infinispan
* Users - no cache, we can retrieve the info we need from tokens and session
User cache is very useful, even with millions of users and a much
smaller cache. Between processLogin, accessCodeToToken, refreshToken,
UserModel is needed for all those requests. So caching an active user
is not a waste, especially if access code timeouts are much shorter than
average active session times.
FYI, beside checking session logout/timeout, refreshToken also exists to
get the latest and greatest scope and role mappings.
Also, IMO, we could probably cache 100s of thousands of users maybe
millions on average machines. 1 million users at 1k per user is 1
gigabyte ram. I'm not convinced that millions of users will be the norm
for keycloak.
--
Bill Burke
JBoss, a division of Red Hat
http://bill.burkecentral.com
3822
days inactive
3824
days old
17 comments
3 participants
participants (3)
-
Bill Burke -
Marek Posolda -
Stian Thorgersen