[keycloak-dev] triple abstraction?

[Bill Burke]

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