On 11/19/2015 9:58 AM, Stian Thorgersen wrote:
On 19 November 2015 at 15:43, Bill Burke <bburke@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@redhat.com
<mailto:bburke@redhat.com><mailto:keycloak-dev@lists.jboss.org<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@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@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@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