We need to follow the spec and verify that sector_identifier_uri points to
a URL that contains the corresponding URIs. As an enhancement we could
support wildcards in the JSON returned by sector_identifier_uri. For
example if it returns:
[
]
A client with the redirect uri 'https://www.myotherdomain.com/myapp' would
work
On 18 August 2016 at 15:09, Martin Hardselius <martin.hardselius(a)gmail.com>
wrote:
Speaking of subject_identifier_uri
From the spec:
"When a sector_identifier_uri is provided, the host component of that URL
is used as the Sector Identifier for the pairwise identifier calculation.
The value of the sector_identifier_uri MUST be a URL using the https scheme
that points to a JSON file containing an array of redirect_uri values.
The values of the registered redirect_uris MUST be included in the
elements of the array."
What's your stance on sanity/health checking the sector_identifier_uri?
URI validation is one thing, but should we also make a request to the uri
in order to validate the response?
The spec also mentions that the sector_identifier_uri isn't strictly
required if a client has all it's redirect_uris under the same domain. How
do we deal with changes to clients if the sector_identifier_uri isn't
required for enabling pairwise subs?
Example:
I create a client, enabling pairwise subs. Valid redirect_uris are [
https://www.mydomain.com/* ]. The sector_identifier would be mydomain.
Later on, I update the redirect_uris to [
https://www.mydomain.com/*,
https://www.myotherdomain.com/* ] Do we
* keep the old sector_identifier, or
* reject the update, or
* allow the update if a valid subject_identifier_uri is provided at
mydomain, or
* just allow it and let the client devs deal with the consequences, or
* take some other path you can think of ?
Having the sector_identifier_uri as a hard requirement seems safer, but
it's also means more work implementing a client. Leaving it optional is a
lot more scary.
On Thu, 18 Aug 2016 at 10:45 Stian Thorgersen <sthorger(a)redhat.com> wrote:
> Makes sense to make this pluggable. The default should probably SHA-256(
> sector_identifier || local_sub || salt ).
>
> We would love a PR for this, but there's a few bits required:
>
> * OIDC clients need option for subject type and sector_identifier_uri. If
> not set it should default to public so existing clients continue to work.
> These options can just be set as client attributes so there's no need to
> update db schema
> * Admin console update for the above
> * PairwiseSubGeneratorSpi and default implementation
> * Generation of salt for clients. This should be done when a client sets
> subject type to pairwise
> * Tests and docs
>
> I'd say the PairwiseSubGeneratorSpi signature should probably be:
>
> * public String getPairwiseSub(UserModel user, ClientModel client)
>
> It might even be an option to let the default PairwiseSubGenerator
> provider create the salt and store it as an attribute on the client, making
> that part pluggable as well.
>
> On 17 August 2016 at 15:59, Martin Hardselius <
> martin.hardselius(a)gmail.com> wrote:
>
>> I'm going to bump this, as I want to continue the discussion/provide
>> some input.
>>
>> Does it make sense to support more than type of pairwise subject
>> identifier generator? E.g through a PairwiseSubGeneratorSpi?
>>
>> Let's say I want to generate the pairwise sub as a salted hash: sub =
>> SHA-256( sector_identifier || local_sub || salt )
>> To me, it makes sense to allow for per-client salts. These salts should
>> probably be generated and persisted during client creation. Thoughts?
>>
>> On Fri, 12 Aug 2016 at 09:57 Martin Hardselius <
>> martin.hardselius(a)gmail.com> wrote:
>>
>>> Thank you for your response. Did not see that ticket before. Great news!
>>>
>>> I looked into using protocol mappers to achieve this, and while it
>>> would work I'm worried that once KEYCLOAK-3422 has been resolved and
>>> included in a proper release we would run into migration issues if the
>>> method used for calculating "native" pairwise subs is different
from what
>>> we implement. Clients could loose / be forced to re-register all their
>>> users if we decide to switch. The example methods in the spec are just
>>> that. Examples. Maybe the method/alg for computing the pairwise sub should
>>> be pluggable?
>>>
>>> --
>>> Martin
>>>
>>> On Thu, 11 Aug 2016 at 17:15 Marek Posolda <mposolda(a)redhat.com>
wrote:
>>>
>>>> Sorry for late response.
>>>>
>>>> We have JIRA created for that. You can possibly add yourself as a
>>>> watcher. See
https://issues.jboss.org/browse/KEYCLOAK-3422
>>>>
>>>> Maybe an alternative for you is to use protocolMappers. That should
>>>> allow you to "construct" the token for particular client
exactly how you
>>>> want and also use the different value for "sub" claim.
>>>>
>>>> Another possibility is, to handle this on adapter side. We already
>>>> have an adapter option "principal-attribute", which specifies
that
>>>> application will see the different attribute instead of "sub"
as subject.
>>>> For example when in appllication you call
"httpServletRequest.getRemoteUser()"
>>>> it will return "john" instead of
"123456-unique-johns-uuid" . See
>>>>
https://keycloak.gitbooks.io/securing-client-applications-
>>>> guide/content/v/2.1/topics/oidc/java/java-adapter-config.html
>>>>
>>>> Hopefully some of the options can be useful for you?
>>>>
>>>> Marek
>>>>
>>>>
>>>> On 02/08/16 14:13, Martin Hardselius wrote:
>>>>
>>>> Me and my team are working towards getting Keycloak, customized for
>>>> our needs, into production but we've identified the need for
Pairwise
>>>> Subject Identifiers as we don't want to expose internal user ids.
>>>>
>>>> Right now, the only subject_types_supported seems to be
"public". Are
>>>> there any near-future plans to include "pairwise"? Can we pitch
in with a
>>>> PR to make this happen as soon as possible?
>>>>
>>>> Links to relevant sections in the spec:
>>>>
>>>>
http://openid.net/specs/openid-connect-core-1_0.html#SubjectIDTypes
>>>>
http://openid.net/specs/openid-connect-core-1_0.html#PairwiseAlg
>>>>
>>>> --
>>>> Martin
>>>>
>>>>
>>>> _______________________________________________
>>>> keycloak-dev mailing
listkeycloak-dev@lists.jboss.orghttps://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
>>
>
>