+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 > 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

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 >>> 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 >> > -- 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

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.

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.

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>> 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(a)lists.jboss.org&gt; > >>> https://lists.jboss.org/mailman/listinfo/keycloak-dev > >> > >> > >> > >> _______________________________________________ > >> keycloak-dev mailing list > >> keycloak-dev(a)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> > https://lists.jboss.org/mailman/listinfo/keycloak-dev > > > -- Bill Burke JBoss, a division of Red Hat http://bill.burkecentral.com

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

On 11/23/2015 3:19 AM, Stian Thorgersen wrote: > 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. > > > Is it not actually bad that they are located using predetermined names? > If lookup is on id, you know for a fact that it's the correct client and > not just something with the same name. > > Remember, the predetermined names are "account", "broker" etc. These are non-unique names. They have to be predetermined or at least indexed in a realm attribute by a predetermined name.

> I never liked it when we had multiple entry points to the same resource. > What about using something like: > > GET ../users?username=<myusername>&single=true > GET ../users?email=<myemail>&single=true > > That returns a single UserRepresentation including 'self' > (../users/<user-id>). > > Same for groups: > > GET ../groups?path=<url encoded path>&single=true > > That works too.

-- Bill Burke JBoss, a division of Red Hat http://bill.burkecentral.com

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>> 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-likeadmin >> 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> >>> https://lists.jboss.org/mailman/listinfo/keycloak-dev >> >> >> >> _______________________________________________ >> keycloak-dev mailing list >> keycloak-dev(a)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> 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