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

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&gt; 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&gt; 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

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&gt; 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&gt; 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&gt; 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&gt; 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 >> > >

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&gt; 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&gt; 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&gt; >> 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&gt; 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&gt; 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&gt; >>>>> 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 >>>> >>> >>> >

"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. [...]

On Mon, 22 Aug 2016 at 10:42 Stian Thorgersen <sthorger(a)redhat.com&gt; 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&gt; 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&gt; >> 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&gt; 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&gt; >>>> 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&gt; 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&gt; 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&gt; >>>>>>> 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 >>>>>> >>>>> >>>>> >>> >

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 <mailto:sthorger@redhat.com>> wrote: On 22 August 2016 at 14:16, Martin Hardselius <martin.hardselius(a)gmail.com <mailto:martin.hardselius@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 registeredredirect_uris, the Client MUST register asector_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 <mailto:sthorger@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 <mailto:martin.hardselius@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 <mailto:sthorger@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 <mailto:martin.hardselius@gmail.com>> wrote: Speaking of subject_identifier_uri From the spec: "When asector_identifier_uriis provided, the host component of that URL is used as the Sector Identifier for the pairwise identifier calculation. The value of thesector_identifier_uriMUST be a URL using thehttpsscheme that points to a JSON file containing an array ofredirect_urivalues. The values of the registeredredirect_urisMUST 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 <mailto:sthorger@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 <mailto:martin.hardselius@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 <mailto:martin.hardselius@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 <mailto:mposolda@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 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

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 <mailto:mposolda@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 <mailto:sthorger@redhat.com>> wrote: > > On 22 August 2016 at 14:16, Martin Hardselius > <martin.hardselius(a)gmail.com > <mailto:martin.hardselius@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 > registeredredirect_uris, the Client MUST register > asector_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 <mailto:sthorger@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 > <mailto:martin.hardselius@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 > <mailto:sthorger@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 > <mailto:martin.hardselius@gmail.com>> wrote: > > Speaking of subject_identifier_uri > > From the spec: > > "When asector_identifier_uriis provided, > the host component of that URL is used as > the Sector Identifier for the pairwise > identifier calculation. The value of > thesector_identifier_uriMUST be a URL > using thehttpsscheme that points to a > JSON file containing an array > ofredirect_urivalues. The values of the > registeredredirect_urisMUST 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 > <mailto:sthorger@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 > <mailto:martin.hardselius@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 > <mailto:martin.hardselius@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 > <mailto:mposolda@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 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 > > > >

Sounds good. I'll look into it and see what I can do. Possibly during next week. :) On Tue, 23 Aug 2016 at 15:38 Marek Posolda <mposolda(a)redhat.com <mailto:mposolda@redhat.com>> wrote: We discussed it with Stian a bit and we discussed to: - Implement as protocolMapper - There will be possibility to configure "salt" as parameter to protocolMapper. It may not be mandatory parameter. If it's used, then client will use it as salt, otherwise salt will be generated. This allows the use-case you mentioned. You can use same salt for clients x,y,z but different salt for clients m,n. If you let generate salt, then the subject identifiers will be really unique just to the particular client. - We need to validate sector_identifier_uri during create/update of protocolMapper, but also during update of client. Otherwise someone can register sector_identifier_uri and register the client with valid redirect_uris (for example URLs "http://good-host/url1" <http://good-host/url1> "http://good-host/url2" <http://good-host/url2> , which are both specified under sector_identifier_uri ) but then later update just the client redirect_uris to "http://evil-host/url1" <http://evil-host/url1> . Basically he will be able to update redirect_uris to anything he wants regardless of redirect_uris under sector_identifier_uri, so the whole validation will be bypassed. For updating client, I've just sent PR, which adds RealmModel.ClientUpdatedEvent . You can register to that event similarly like https://github.com/keycloak/keycloak/blob/master/server-spi/src/main/java... . The creation/update listener for protocolMapper doesn't yet exists, however we are likely going to refactor protocolMapper to generic component, which will have validation support. But that will be likely available in few weeks, so for now, we can add something temporary for protocolMappers similar to what is available for example for UserFederationMapperFactory (See https://github.com/keycloak/keycloak/blob/master/services/src/main/java/o... ) Martin, will you have some time to eventually look into this and send PR ? Marek On 23/08/16 14:25, Martin Hardselius wrote: > 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 > <mailto:mposolda@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 <mailto:sthorger@redhat.com>> wrote: >> >> On 22 August 2016 at 14:16, Martin Hardselius >> <martin.hardselius(a)gmail.com >> <mailto:martin.hardselius@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 >> registeredredirect_uris, the Client MUST register >> asector_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 <mailto:sthorger@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 >> <mailto:martin.hardselius@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 >> <mailto:sthorger@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 >> <mailto:martin.hardselius@gmail.com>> wrote: >> >> Speaking of subject_identifier_uri >> >> From the spec: >> >> "When asector_identifier_uriis >> provided, the host component of that >> URL is used as the Sector Identifier >> for the pairwise identifier >> calculation. The value of >> thesector_identifier_uriMUST be a >> URL using thehttpsscheme that points >> to a JSON file containing an array >> ofredirect_urivalues. The values of >> the registeredredirect_urisMUST 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 >> <mailto:sthorger@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 >> <mailto:martin.hardselius@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 >> <mailto:martin.hardselius@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 >> <mailto:mposolda@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 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 >> >> >> >> >

Just had the chance to start looking at this. Wanted to check with you if this is the direction you had in mind? public abstract class AbstractPairwiseSubGeneratorMapperextends AbstractOIDCProtocolMapperimplements OIDCAccessTokenMapper, OIDCIDTokenMapper{ @Override public IDToken transformIDToken(IDToken token, ProtocolMapperModel mappingModel, KeycloakSession session, UserSessionModel userSession, ClientSessionModel clientSession) { token.setSubject(generateSub(mappingModel, userSession.getUser().getId())); return token; } @Override public AccessToken transformAccessToken(AccessToken token, ProtocolMapperModel mappingModel, KeycloakSession session, UserSessionModel userSession, ClientSessionModel clientSession) { token.setSubject(generateSub(mappingModel, userSession.getUser().getId())); return token; } public abstract String generateSub(ProtocolMapperModel mappingModel, String localSub); } On Wed, 24 Aug 2016 at 12:03 Marek Posolda <mposolda(a)redhat.com <mailto:mposolda@redhat.com>> wrote: Sounds great. Thanks! Marek On 24/08/16 11:08, Martin Hardselius wrote: > Sounds good. I'll look into it and see what I can do. Possibly > during next week. :) > > On Tue, 23 Aug 2016 at 15:38 Marek Posolda <mposolda(a)redhat.com > <mailto:mposolda@redhat.com>> wrote: > > We discussed it with Stian a bit and we discussed to: > > - Implement as protocolMapper > > - There will be possibility to configure "salt" as parameter > to protocolMapper. It may not be mandatory parameter. If it's > used, then client will use it as salt, otherwise salt will be > generated. This allows the use-case you mentioned. You can > use same salt for clients x,y,z but different salt for > clients m,n. If you let generate salt, then the subject > identifiers will be really unique just to the particular client. > > - We need to validate sector_identifier_uri during > create/update of protocolMapper, but also during update of > client. Otherwise someone can register sector_identifier_uri > and register the client with valid redirect_uris (for example > URLs "http://good-host/url1" <http://good-host/url1> > "http://good-host/url2" <http://good-host/url2> , which are > both specified under sector_identifier_uri ) but then later > update just the client redirect_uris to > "http://evil-host/url1" <http://evil-host/url1> . Basically > he will be able to update redirect_uris to anything he wants > regardless of redirect_uris under sector_identifier_uri, so > the whole validation will be bypassed. > > For updating client, I've just sent PR, which adds > RealmModel.ClientUpdatedEvent . You can register to that > event similarly like > https://github.com/keycloak/keycloak/blob/master/server-spi/src/main/java... > . The creation/update listener for protocolMapper doesn't yet > exists, however we are likely going to refactor > protocolMapper to generic component, which will have > validation support. But that will be likely available in few > weeks, so for now, we can add something temporary for > protocolMappers similar to what is available for example for > UserFederationMapperFactory (See > https://github.com/keycloak/keycloak/blob/master/services/src/main/java/o... > ) > > Martin, will you have some time to eventually look into this > and send PR ? > > > Marek > > > On 23/08/16 14:25, Martin Hardselius wrote: >> 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 <mailto:mposolda@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 <mailto:sthorger@redhat.com>> wrote: >>> >>> On 22 August 2016 at 14:16, Martin Hardselius >>> <martin.hardselius(a)gmail.com >>> <mailto:martin.hardselius@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 >>> registeredredirect_uris, the Client MUST >>> register asector_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 >>> <mailto:sthorger@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 >>> <mailto:martin.hardselius@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 >>> <mailto:sthorger@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 >>> <mailto:martin.hardselius@gmail.com>> >>> wrote: >>> >>> Speaking of subject_identifier_uri >>> >>> From the spec: >>> >>> "When asector_identifier_uriis >>> provided, the host component of >>> that URL is used as the Sector >>> Identifier for the pairwise >>> identifier calculation. The >>> value of >>> thesector_identifier_uriMUST be >>> a URL using thehttpsscheme that >>> points to a JSON file >>> containing an array >>> ofredirect_urivalues. The >>> values of the >>> registeredredirect_urisMUST 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 >>> <mailto:sthorger@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 <mailto:martin.hardselius@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 >>> <mailto:martin.hardselius@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 <mailto:mposolda@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 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 >>> >>> >>> >>> >> >