Let's say we want to force all of our client to use pairwise subs, but a
single "merchant" needs to implement several clients where subs should
remain the same for all those clients.
Merchant A
- client x
- client y
- client z
Merchant B
- client m
- client n
You can assume the sector_identifiers are the same across all clients owned
by a merchant
It should not be possible to correlate activities between Customer A and
Customer B (at least not from their side). They should, however, be able to
correlate user activities between their own clients.
Which implementation of pairwise subs is better suited for supporting this
scenario? I'm leaning towards the protocol mapper solution. It should be
easier to create custom mappers with merchant-wide configuration (e.g
salts).
On Mon, 22 Aug 2016 at 22:40 Marek Posolda <mposolda(a)redhat.com> wrote:
The question is, should we really introduce another SPI for that?
Doesn't
it mean uneccessary complexity? Also add the new options directly to the
client for the feature, which is likely interesting just for quite limited
amount of people?
IMO it's fine if this is implemented as protocolMapper?
Few thoughts:
- We can have abstract superclass like AbstractPairwiseSubGeneratorMapper
. The subclasses will just need to implement method "generateSub" . We can
have just one concrete impl, which will use SHA-256( sector_identifier ||
local_sub || salt )
- The sector_identifier_uri will be a configuration option of this
protocolMapper implementation.
- If protocolMapper is not added to client, the client will just use the
public subjects. By default it's not added, which ensures backwards
compatibility and public subjects by default. Note that with this approach,
we don't even need subject_type option on the client.
- The salt can be generated lazily at the first time when mapper is used.
- The validation can be done at the moment, when protocolMapper is going
to be created/updated. Right now, we don't have validation callback during
protocolMapper creation/update. However Bill is going to add support for
that into generic componentModel. So if we're going to refactor
protocolMapper to use the new generic component model, we will have
validation callback available to protocolMapper SPI. The validation will
fail if array of redirect_uri from sector_identifier_uri doesn't contain
the uris from redirect_uri of particular client (including wildcards etc).
- If client is updated and it's redirect_uri are changed, we won't be able
to catch this, however this is not strictly required per specs per my
understanding. At least the dynamic client registration specs sais [1]
"The values registered in redirect_uris MUST be included in the elements
of the array, or registration MUST fail. This MUST be validated at
registration time; there is no requirement for the OP to retain the
contents of this JSON file or to retrieve or revalidate its contents in the
future. "
[1]
http://openid.net/specs/openid-connect-registration-1_0.html#SectorIdenti...
Marek
On 22/08/16 15:50, Martin Hardselius wrote:
Ok, thanks for the clarification.
Where would it make sense to put the PairwiseSubGeneratorSpi? Which
package, that is?
On Mon, 22 Aug 2016 at 14:51 Stian Thorgersen <sthorger(a)redhat.com> wrote:
> On 22 August 2016 at 14:16, Martin Hardselius <
> martin.hardselius(a)gmail.com> wrote:
>
>> "IMO it's sufficient to document the algorithm to create the sub, which
>> should make it clear that the origin in the redirect uri will affect the
>> sub."
>>
>> Reasonable. :)
>>
>> "One client could also have multiple redirect uris with different
>> origins so could get different sub's generated depending on the redirect
>> uri used."
>>
>> That case is probably caught by
>> [...] If there are multiple hostnames in the registered redirect_uris,
>> the Client MUST register a sector_identifier_uri. [...]
>>
>
> Yes, but I meant from a documentation perspective. It should be clear
> from the documentation that is the case.
>
>
>>
>> On Mon, 22 Aug 2016 at 10:42 Stian Thorgersen <sthorger(a)redhat.com>
>> wrote:
>>
>>> IMO it's sufficient to document the algorithm to create the sub, which
>>> should make it clear that the origin in the redirect uri will affect the
>>> sub. One client could also have multiple redirect uris with different
>>> origins so could get different sub's generated depending on the redirect
>>> uri used.
>>>
>>> On 22 August 2016 at 09:58, Martin Hardselius <
>>> martin.hardselius(a)gmail.com> wrote:
>>>
>>>> Sounds fair enough.
>>>>
>>>> What about the case where you don't provide a sector_identifier_uri,
>>>> set up a single redirect uri on myhost and then later go on to change
that
>>>> redirect uri to something on myotherhost? That would change the
>>>> sector_identifier and subsequently all the user subs. Do we protect
against
>>>> such "mistakes" or do we warn people (in the docs and/or admin
gui) that
>>>> it's not a good idea to do that?
>>>>
>>>> On Mon, 22 Aug 2016 at 09:38 Stian Thorgersen
<sthorger(a)redhat.com>
>>>> wrote:
>>>>
>>>>> 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:
>>>>>
>>>>> [
https://www.mydomain.com/*,
https://www.myotherdomain.com/* ]
>>>>>
>>>>> 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...
>>>>>>>>>>
>>>>>>>>>> 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
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>
>>>