[keycloak-dev] Client ID and Client ClientID - I propose we remove one
Stian Thorgersen
sthorger at redhat.com
Mon Nov 23 03:19:32 EST 2015
On 19 November 2015 at 19:26, Bill Burke <bburke at redhat.com> wrote:
>
>
> On 11/19/2015 9:58 AM, Stian Thorgersen wrote:
>
>>
>>
>> On 19 November 2015 at 15:43, Bill Burke <bburke at redhat.com
>> <mailto:bburke at 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.
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.
>
>
>
>> 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.
Didn't think of the additional stuff under each element. Even if it works
it would be messy.
>
>
>
> ??
>>
>> 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.
>
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
>
>
>
>>
>>
>> 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 at redhat.com
>> <mailto:bburke at redhat.com>
>> <mailto:bburke at redhat.com <mailto:bburke at 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 at lists.jboss.org
>> <mailto:keycloak-dev at lists.jboss.org>
>> <mailto:keycloak-dev at lists.jboss.org
>> <mailto:keycloak-dev at lists.jboss.org>>
>> >>> https://lists.jboss.org/mailman/listinfo/keycloak-dev
>> >>
>> >>
>> >>
>> >> _______________________________________________
>> >> keycloak-dev mailing list
>> >> keycloak-dev at lists.jboss.org
>> <mailto:keycloak-dev at lists.jboss.org>
>> <mailto:keycloak-dev at lists.jboss.org
>> <mailto:keycloak-dev at 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 at lists.jboss.org
>> <mailto:keycloak-dev at lists.jboss.org>
>> <mailto:keycloak-dev at lists.jboss.org
>> <mailto:keycloak-dev at 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.jboss.org/pipermail/keycloak-dev/attachments/20151123/4a53ae99/attachment-0001.html
More information about the keycloak-dev
mailing list