Re: [keycloak-dev] Client ID and Client ClientID - I propose we remove one

On 11/19/2015 9:58 AM, Stian Thorgersen wrote: > > > On 19 November 2015 at 15:43, Bill Burke <bburke(a)redhat.com > <mailto:bburke@redhat.com>> wrote: > > FYI, there's other problems too. Built in clients are looked up via > their hardcoded ClientIds "admin-console" "broker" etc, again > because DB ids are generated. > > I just don't see how you can remove ID or ClientID. ID is URL > friendly, ClientID isn't and needs to be encoded. ID is pretty much > hidden from the user until they want to use the REST interface. If > you remove ID from the REST interface then ClientID @PathParams are > gonna have to be encoded which could be a real pain for users, > especially ones using simple HTTP. While most libraries will encode > query and form params, users will have to encode by hand other > mechanisms. > > > For clients I was thinking we should remove ClientID, not ID. We'd allow > specifying the ID when creating a new client, but not changing it. The > ID would also have to be URL friendly. > > Removing clientID doesn't work for built-in clients like account, broker, admin-console, etc. These all need to be located using a predetermined name. You'd have to figure out an additional alternative to refactor that.

> There's a similar problem with Groups. Groups are looked up via a > "path" name "/parent/child" in the REST interface, then you have > use the ID to reference the rest of the GROUP rest interface i.e. > groups/{ID}/children. > > > We'll have similar problems with role namespaces if you want to > incorporate them into the REST interface. > > ClientId should be forced to become a URL friendly string (no > special characters or '/' or "?", etc.). SAML can introduce an > Issuer URL or something. After that, you can remove ID. That's the > only solution I can see that will work well. > > > For groups and roles couldn't the ID be the full path? Then the REST > endpoints would be aware of "hierarchies" so you could just do: > > get /auth/realms/groups/group-parent/group-child > > And: > > get /auth/realms/roles/role/path/to/role/myrole > > I didn't even try to see if that worked. The issue is that there are additional endpoints after the path: .../children .../composites And methods to add remove group children and role composites. I'm pretty sure there are some JAX-RS and/or regular-expression mapping issues with that. At least that's what I remember, might be a false memory.

?? > > I think the current approach of having REST endpoints that retrieve > based on ID is a pain point for a lot of users. They create a client > with a named client-id or role with a name, then have to use a different > ID to retrieve from the rest endpoints > > The way it works for groups now is that i have a /group-by-path lookup mechanism which returns a GroupRepresentation, the id is extracted from that, then you invoke from there, i.e.: GroupRepresentation topGroup = realm.getGroupByPath("/the/top"); List<RoleRepresentation> roles = new LinkedList<>(); roles.add(topRole); realm.groups().group(topGroup.getId()).roles().realmLevel().add(roles); Alternatively we could add links to the GroupRepresentation like 'self' that points to the resource endpoint.

> > > On 11/19/2015 8:59 AM, Stian Thorgersen wrote: > > How about: > > * IDs are usually generated, but can be specified. It should not > be > possible to change them though > > * Instead of alias, name and all other ways of setting a human > readable > name we add Display Name and Display Class > > Not sure how to deal with migration, but if we agree on those > points, > then we can figure that out > > On 19 November 2015 at 14:49, Bill Burke <bburke(a)redhat.com > <mailto:bburke@redhat.com> > <mailto:bburke@redhat.com <mailto:bburke@redhat.com>>> wrote: > > One more thing. Another problem is that if you switch to > ID for > everything, then demos etc. will no longer run out of the > box and you'll > have to modify adapter configuration to set the newly > generated DB id. > Really, any piece of configuration that references things > by client ID > will no longer work. > > So, there's really 2 issues: > > * Using ClientID in admin-console URLs doesn't work well as > the client > IDs could have '/' characters in them > * Using ID only messes up pre-defined, pre-packaged adapter > config and > you have to edit these files after the keycloak DB is set up. > > On 11/19/2015 8:40 AM, Bill Burke wrote: > > We currently actually have 3 client identifers: ID, > ClientID, > and Name. > > > > Also, I think there are a lot of duplicate clientID > names between > > realms. i.e. "admin-console" "broker", etc... > > > > A search by clientID is (realm + clientID). > > > > Marek is right about why I switched everything to ID in > the admin > > console. For SAML I just didn't want to add yet another > client > > identifier since we already had 3. > > > > On 11/19/2015 7:52 AM, Marek Posolda wrote: > >> +1 for this change. > >> > >> I am just not sure if we should set the "id" to the > current value of > >> "client-id" ? Few things to note: > >> > >> - SAML clients currently use clientId in form of URL. > For example in > >> SAML demo there are clientIds like > "http://localhosT:8080/employee-sig" > >> . I don't know if it's requirement, maybe it's possible > to solve it > >> somehow (ie. introduce different attribute for SAML > client to store > >> these URLs). But from what I remember, Bill changed admin > console to use > >> "id" instead of "clientId" because there were issues > with URL-like > >> clientId in admin console . So if we overwrite the "id" > with current > >> "client-id" the issue will be back. > >> > >> - Migration might be a pain. Many tables (roles, > protocolMappers, user > >> consents, offline clientSessions ...) references client > by "id" . > >> Overwriting "id" with "client-id" means that we will > need to > change all > >> those DB records. And there are things like foreign > keys etc... > >> > >> Shouldn't do vice-versa and just remove current > "client-id" and ask > >> people to update their keycloak.json adapter > configurations? On the > >> other hand, removing "client-id" might break migration > of JSON > exported > >> realms as the JSON entities are using "client-id" for > referencing client. > >> > >> It seems the migration will be a pain regardless of > whatever > direction > >> we choose :-( > >> > >> Marek > >> > >> On 16/11/15 14:54, Stian Thorgersen wrote: > >>> We have both "id" and "client-id" for clients in > Keycloak at the > >>> moment. This seems unnecessary and complex. > >>> > >>> The model can retrieve clients on either value. In token > endpoints the > >>> "client-id" is used. In admin endpoints the "id" is > used. > >>> > >>> Also, in most cases it would be simpler for users to > just have a > >>> generated id than having to come up with one > themselves. The id > >>> doesn't have to be human readable either as we have > name for that. > >>> > >>> OpenID Connect expects "client-id" to be generated by > the IdP and > >>> can't be changed once created. > >>> > >>> I propose we remove "client-id" and only keep id. > >>> > >>> For migration of existing clients we would set the > "id" value > to the > >>> current value of "client-id". This would require no > changes to > adapter > >>> configs. When creating new clients from the admin > console we > would not > >>> allow setting the "client-id", instead just display it > after the > >>> client was created. When importing clients it would be > possible > to set > >>> the id (and for backwards compatibility we would set > "id" equal > to the > >>> "client-id" field. > >>> > >>> > >>> _______________________________________________ > >>> keycloak-dev mailing list > >>> keycloak-dev(a)lists.jboss.org > <mailto:keycloak-dev@lists.jboss.org> > <mailto:keycloak-dev@lists.jboss.org > <mailto:keycloak-dev@lists.jboss.org>> > >>> https://lists.jboss.org/mailman/listinfo/keycloak-dev > >> > >> > >> > >> _______________________________________________ > >> keycloak-dev mailing list > >> keycloak-dev(a)lists.jboss.org > <mailto:keycloak-dev@lists.jboss.org> > <mailto:keycloak-dev@lists.jboss.org > <mailto:keycloak-dev@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 > <mailto:keycloak-dev@lists.jboss.org> > <mailto:keycloak-dev@lists.jboss.org > <mailto:keycloak-dev@lists.jboss.org>> > https://lists.jboss.org/mailman/listinfo/keycloak-dev > > > > -- > Bill Burke > JBoss, a division of Red Hat > http://bill.burkecentral.com > > > -- Bill Burke JBoss, a division of Red Hat http://bill.burkecentral.com