[keycloak-dev] Simplifying realm model

[Marek Posolda]

On 19.6.2015 11:05, Stian Thorgersen wrote:
>
> ----- Original Message -----
>> From: "Marek Posolda" <mposolda at redhat.com>
>> To: "Stian Thorgersen" <stian at redhat.com>, "Bill Burke" <bburke at redhat.com>
>> Cc: keycloak-dev at lists.jboss.org
>> Sent: Friday, 19 June, 2015 10:40:35 AM
>> Subject: Re: [keycloak-dev] Simplifying realm model
>>
>> To me, it doesn't look like very big issue.
>>
>> IMO realm model is quite small and doesn't contain gazillion of objects
>> like users or userSession models. Everything in realm model can be
>> edited just by admin users and I think that many deployments have just
>> very small number of admin users (if not just one).
>>
>> With respect to all of this, the probability of concurrent edit seems to
>> be corner case. Hence I hope we can handle optimistic locking in one
>> place in the code in model and maybe just throw an exception with
>> message for the admin to retry it himself? I wouldn't do any retry logic
>> directly in the code of all rest endpoints. The increased complexity of
>> the code doesn't worth to handle the corner case IMO.
> You're probably right.
>
> Thinking about it some more the way that the admin console works it's actually broken now anyways and changing it to what's proposed with an optimistic lock would be an improvement.
>
> Take the following scenario for instance:
>
> 1. Admin1 opens the realm login settings page
> 2. Admin2 opens the realm login settings page
> 3. Admin1 enables self-registration and saves it
> 4. Admin2 enables remember-me and saves it
>
> Both step 3 and step 4 submit the same realm representation. In step 4 the realm representation will have self-registration disabled and remember-me enabled. This will override the change made in step 3.
>
> Now the same scenario above if we had optimistic locking in place and had the version in the realm representation sent to the admin console we could at least give an error to admin2 instead of overriding changes done by admin1. That's an improvement IMO.
+1

Marek
>
>> Marek
>>
>>
>> On 18.6.2015 16:08, Stian Thorgersen wrote:
>>> ----- Original Message -----
>>>> From: "Bill Burke" <bburke at redhat.com>
>>>> To: keycloak-dev at lists.jboss.org
>>>> Sent: Thursday, 18 June, 2015 2:47:04 PM
>>>> Subject: Re: [keycloak-dev] Simplifying realm model
>>>>
>>>> This would make things easier to store custom data too.  It could be
>>>> extended to places where custom data could be prevalent.  For example,
>>>> credential storage.
>>>>
>>>> But here are the disadvantages:
>>>>
>>>> * Do we have to worry about concurrency issues more though?  All you
>>>> need is 2 concurrent admins modifying different settings in the realm
>>>> for one to overwrite the other.  For example, one admin could be adding
>>>> a role, another could be configuring an identity provider.  Sure, these
>>>> kind of concurrency issues exist now, but they are isolated because
>>>> realm model data is in different tables.  Minimally, you would need some
>>>> kind of optimistic locking scheme that provided a non-cryptic error
>>>> message when there were collisions.
>>> Yep, that's one of the bigger issues.
>>>
>>> What I had in mind was to use optimistic locking and also make rest
>>> endpoints retry an operation before returning an error to the admin.
>>>
>>> For example adding an identity provider would still be a separate rest
>>> endpoint, so if it fails to add it to the realm due to an optimistic
>>> locking exception it can simply retry adding it.
>>>
>>>> * Admins would never be able to modify the database directly.
>>> I don't think that's a problem - in theory they could as it would just be a
>>> json doc so they could get it, change it, write it ;)
>>>
>>>> On 6/18/2015 5:19 AM, Stian Thorgersen wrote:
>>>>> ----- Original Message -----
>>>>>> From: "Marek Posolda" <mposolda at redhat.com>
>>>>>> To: "Stian Thorgersen" <stian at redhat.com>, "keycloak dev"
>>>>>> <keycloak-dev at lists.jboss.org>
>>>>>> Sent: Thursday, 18 June, 2015 10:59:42 AM
>>>>>> Subject: Re: [keycloak-dev] Simplifying realm model
>>>>>>
>>>>>> +1 to go this way for realm model. For users+userSessions I would likely
>>>>>> keep it in current form due to performance reasons, but for realm model
>>>>>> I am not seeing any issue to store it in blob as realm model doesn't
>>>>>> contain big amount of data. I am seeing just advantages and much simpler
>>>>>> migration and DB maintenance, which is currently pain.
>>>>> Yep, user model is much simpler in either case and isn't such a pain. We
>>>>> could probably clean it up a bit, but would certainly keep a proper
>>>>> schema
>>>>> for it with many tables and such.
>>>>>
>>>> --
>>>> Bill Burke
>>>> JBoss, a division of Red Hat
>>>> http://bill.burkecentral.com
>>>> _______________________________________________
>>>> keycloak-dev mailing list
>>>> keycloak-dev at lists.jboss.org
>>>> https://lists.jboss.org/mailman/listinfo/keycloak-dev
>>>>
>>> _______________________________________________
>>> keycloak-dev mailing list
>>> keycloak-dev at lists.jboss.org
>>> https://lists.jboss.org/mailman/listinfo/keycloak-dev
>>