+1 on UUIDs; we do same on UPS On Friday, February 7, 2014, Stian Thorgersen <stian(a)redhat.com <mailto:stian@redhat.com>> wrote: I propose we use java.util.UUID to IDs generated by DB (JPA @GeneratedValue). Reasoning behind this is: W/ the hibernate specific annotations? * IDs are the same independent of store used (JPA, Mongo, PicketLink, LDAP, etc) * Easy to support many RDBMS (some support sequence and/or identity, so it seems the recommended approach when you don't know what the db will be is table) * IDs can be generated without a "central" db Also, we'd like to be able to export all data to a json then import into any store. We then need to make sure there's no conflicts in IDs. For example you first use KC with H2, then export all data, import into MySQL, then export all data, import into Mongo. I can see that causing some issues with IDs generated by DB. This is related to DB issues (Mysql, PostgreSQL not working), Mongo store impl as well as move to using user id instead of username as the reference for a user. _______________________________________________ keycloak-dev mailing list keycloak-dev(a)lists.jboss.org <javascript:;> https://lists.jboss.org/mailman/listinfo/keycloak-dev -- Sent from Gmail Mobile _______________________________________________ keycloak-dev mailing list keycloak-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/keycloak-dev

----- Original Message ----- > From: "Bill Burke" <bburke(a)redhat.com&gt; > To: keycloak-dev(a)lists.jboss.org > Sent: Friday, 7 February, 2014 2:04:24 PM > Subject: Re: [keycloak-dev] Use UUID for IDs > > Not sure why I didn't use UUID in the first place. I've used it in > other code bases. I guess maybe I wasn't sure if UUID.randomUUID() > guaranteed that the UUID was always unique. Wasn't clear if a time > component is added to the UUID. AFAIK there's no time component, but the risk of collision is extremely small

----- Original Message ----- > From: "Bill Burke" <bburke(a)redhat.com&gt; > To: "Stian Thorgersen" <stian(a)redhat.com&gt; > Cc: keycloak-dev(a)lists.jboss.org > Sent: Friday, 7 February, 2014 2:53:43 PM > Subject: Re: [keycloak-dev] Use UUID for IDs > > > > On 2/7/2014 9:33 AM, Stian Thorgersen wrote: >> >> >> ----- Original Message ----- >>> From: "Bill Burke" <bburke(a)redhat.com&gt; >>> To: keycloak-dev(a)lists.jboss.org >>> Sent: Friday, 7 February, 2014 2:04:24 PM >>> Subject: Re: [keycloak-dev] Use UUID for IDs >>> >>> Not sure why I didn't use UUID in the first place. I've used it in >>> other code bases. I guess maybe I wasn't sure if UUID.randomUUID() >>> guaranteed that the UUID was always unique. Wasn't clear if a time >>> component is added to the UUID. >> >> AFAIK there's no time component, but the risk of collision is extremely >> small >> > > Hopefully no security weeny identifies that as a security risk. We can wrap it in our own UUID class, that way we'll be able to quickly change the underlying algorithm if some weenies complain

> I didn't mean use the UUID class as the id. I meant wrap the creation. So instead of: new User(java.util.UUID.randomUUID().toString()); we do: new User(org.keycloak.UUID.createUUID());

> IMO, should just reuse the static class we have now: String IdGenerator.generateId(); and use UUID as the impl instead of creating a dependency on UUID everywhere.

From: "Marek Posolda" <mposolda(a)redhat.com&gt; To: "Bruno Oliveira" <bruno(a)abstractj.org&gt;, "Bill Burke" <bburke(a)redhat.com&gt;, "Stian Thorgersen" <stian(a)redhat.com&gt; Cc: keycloak-dev(a)lists.jboss.org Sent: Tuesday, 11 February, 2014 11:38:56 PM Subject: Re: [keycloak-dev] Use UUID for IDs I wonder if it's not better to just use Long instead of String? In GateIn portal we use old Picketlink IDM backed by Hibernate and we have autogenerated Ids of type Long, which are "externally" seen as Strings, so it's not a problem to keep getId() method in API returning String. See the class here (and getter/setters for Strings): https://github.com/picketlink/picketlink-idm/blob/1.4/picketlink-idm-hibe... and Hibernate mapping: https://github.com/picketlink/picketlink-idm/blob/1.4/picketlink-idm-hibe... The most important thing is, that this is tested and works with all major databases (H2, MySQL, PostgreSQL, IBM DB2, Oracle, Sybase, MSSQL). Another thing is that long might have better performance than String. I am not sure about the usecase with import/export stuff from/into completely different DB (like have data in H2, then export them and then import same data into MySQL). This may not work as different DBs may have different rules for allowing ID (some may have unique ID just for single table, some may enforce to have unique ID globally). However I am also not sure if String Id like "1-898928392" is allowed in all RDBMS... Marek On 11.2.2014 15:13, Bruno Oliveira wrote: > +1 I guess TokenIdGenerator does the job well >

From: "Bill Burke" <bburke(a)redhat.com&gt; To: "Stian Thorgersen" <stian(a)redhat.com&gt;, "Marek Posolda" <mposolda(a)redhat.com&gt; Cc: "Bruno Oliveira" <bruno(a)abstractj.org&gt;, keycloak-dev(a)lists.jboss.org Sent: Wednesday, 12 February, 2014 1:54:20 PM Subject: Re: [keycloak-dev] Use UUID for IDs IIRC, String ids was an artifact from when we used Picketlink as the sole backend store. Why wouldn't we just use a Long and use whatever facility the persistence layer has for generating unique ids? (i.e. a sequence)? +1 that current IdGenerator sucks. My bad. I should know better (and have done better in the past, and have even used UUID in the past). Don't know what I was thinking... On 2/12/2014 6:30 AM, Stian Thorgersen wrote: > I'm confused - so let's clarify: > > Let's use IdGenerator to create IDs, but replace the implementation with: > > public class IdGenerator { > public static String generateId() { > return java.lang.UUID.randomUUID().toString(); > } > } > > * IdGenerator as it stands doesn't work. For example it won't work in a > cluster. It also could quite likely generated duplicates if a server is > restarted after the time has been changed (admin restarts server, realizes > time is 60 min wrong, fixes that and starts Keycloak). > > We'll then use this to generate IDs when creating new realms, apps, users, > roles, etc. (for all model implementations). In the model api and in the > db these will be 128-bits strings. > > I don't see the need for TokenIdGenerator and we should use IdGenerator for > that as well. > > If there is a demand for it in the future, we could investigate a better > solution. For example we could get away with a smaller id (64-bits?), we > could generate sequential UUIDs to improve db performance, etc. In the > short/medium term though, we have other higher priorities. > > ----- Original Message ----- >> From: "Marek Posolda" <mposolda(a)redhat.com&gt; >> To: "Bruno Oliveira" <bruno(a)abstractj.org&gt;, "Bill Burke" >> <bburke(a)redhat.com&gt;, "Stian Thorgersen" <stian(a)redhat.com&gt; >> Cc: keycloak-dev(a)lists.jboss.org >> Sent: Tuesday, 11 February, 2014 11:38:56 PM >> Subject: Re: [keycloak-dev] Use UUID for IDs >> >> I wonder if it's not better to just use Long instead of String? In >> GateIn portal we use old Picketlink IDM backed by Hibernate and we have >> autogenerated Ids of type Long, which are "externally" seen as Strings, >> so it's not a problem to keep getId() method in API returning String. >> >> See the class here (and getter/setters for Strings): >> https://github.com/picketlink/picketlink-idm/blob/1.4/picketlink-idm-hibe... >> and Hibernate mapping: >> https://github.com/picketlink/picketlink-idm/blob/1.4/picketlink-idm-hibe... >> >> The most important thing is, that this is tested and works with all >> major databases (H2, MySQL, PostgreSQL, IBM DB2, Oracle, Sybase, MSSQL). >> Another thing is that long might have better performance than String. >> >> I am not sure about the usecase with import/export stuff from/into >> completely different DB (like have data in H2, then export them and then >> import same data into MySQL). This may not work as different DBs may >> have different rules for allowing ID (some may have unique ID just for >> single table, some may enforce to have unique ID globally). However I am >> also not sure if String Id like "1-898928392" is allowed in all RDBMS... >> >> Marek >> >> >> On 11.2.2014 15:13, Bruno Oliveira wrote: >>> +1 I guess TokenIdGenerator does the job well >>> >> >> -- Bill Burke JBoss, a division of Red Hat http://bill.burkecentral.com

From: "Bill Burke" <bburke(a)redhat.com&gt; To: "Stian Thorgersen" <stian(a)redhat.com&gt; Cc: "Marek Posolda" <mposolda(a)redhat.com&gt;, "Bruno Oliveira" <bruno(a)abstractj.org&gt;, keycloak-dev(a)lists.jboss.org Sent: Wednesday, 12 February, 2014 2:56:50 PM Subject: Re: [keycloak-dev] Use UUID for IDs On 2/12/2014 9:18 AM, Stian Thorgersen wrote: > We've come full circle ;) > > My reasoning for not to use db generated ids are: > > * ID format would be different depending on the store (H2, MySQL, > PostgreSQL, Oracle, Mongo, etc.) / for example apps will want to refer to > users by id, and they need to know how to store it Dbs will support long primary keys :)

> * What happens to a "sequences" (or whatever mechanism the db chooses to > use) if you export to json, then import again. Especially if you change > the underlying db (e.g. from H2 to PostgreSQL) > Export to json does not have to contain ids. Our import format doesn't require ids. I'm not even sure it honors ids :)

> Further (this is a stupid reason) if we want to support huge deployments > shards will be involved, in which case it'd be simpler to use UUIDs. > That is beyond my experience so I don't have a counter argument to that. -- Bill Burke JBoss, a division of Red Hat http://bill.burkecentral.com