<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=Windows-1252">
<style type="text/css" style="display:none;"> P {margin-top:0;margin-bottom:0;} </style>
</head>
<body dir="ltr">
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
Hi Torsten,</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<br>
</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
Thank you so much for the review and the feedback.</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
I started a PR that incorporates your feedback: <a href="https://bitbucket.org/openid/connect/pull-requests/68" id="LPlnk985123">https://bitbucket.org/openid/connect/pull-requests/68</a></div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
It still needs to be worked on, but it shows the suggestions of how to update the text as I also summarized in-line in blue below.</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<br>
</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
Best Regards,</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
Kristina<br>
</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<br>
</div>
<hr tabindex="-1" style="display:inline-block; width:98%">
<div id="divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" color="#000000" style="font-size:11pt"><b>From:</b> Torsten Lodderstedt <torsten@lodderstedt.net><br>
<b>Sent:</b> Sunday, November 7, 2021 4:21<br>
<b>To:</b> Artifact Binding/Connect Working Group <openid-specs-ab@lists.openid.net><br>
<b>Cc:</b> Kristina Yasuda <Kristina.Yasuda@microsoft.com><br>
<b>Subject:</b> SIOP v2 Review</font>
<div> </div>
</div>
<div class="" style="word-wrap:break-word; line-break:after-white-space">Hi all,
<div class=""><br class="">
</div>
<div class="">I reviewed the current master revision of SIOP v2 with a special focus on SIOP discovery, client id resolution and sub types. </div>
<div class=""><br class="">
</div>
<div class="">Generally, I agree with the direction of travel. I think we need to find the right balance between flexibility and simplicity, esp. related to client ids and authentication requests. </div>
<div class=""><br class="">
</div>
<div class="">Here is my detailed feedback: </div>
<div class=""><br class="">
</div>
<div class="">- "Relying Parties typically cannot pre-establish registration with a Self-Issued OP, as each End-user might be represented by a different, locally-controlled Self-Issued OP instance. This specification extends the authentication request with
a mechanism with additional dynamic registration techniques for feature negotiation.“<br class="">
<br class="">
Other models are possible, e.g. all instances of a wallet app sharing the same RP registration data. This should be mentioned in the spec. </div>
<div class=""><span style="color: rgb(12, 100, 192);">-> below text added in this scope section. <span style="margin:0px"><span style="margin:0px">Still need to think how to add this to other sections such as the registration metadata section.</span></span><br>
<span style="margin:0px"></span></span></div>
<div class=""><span style="color: rgb(12, 100, 192);">"In another model, when all instances of Sel-Issued OPs share the same RP registration data, it is possible for the RPs to pre-establish registration with that set of Self-Issued OPs."</span></div>
<div class=""><font color="#0c64c0">Assuming that the method used by the RP in this case would be out-of-band or Dynamic Client Registration..?</font></div>
<div class=""><font color="#0c64c0"><br>
</font></div>
<div class="">- „## Self-Issued OpenID Provider Invocation“</div>
<div class=""><br class="">
</div>
<div class="">I this section should also mention that the current custom URI scheme `<a href="" class="">openid://`</a> does not work with web wallets. </div>
<div class=""><span style="color: rgb(12, 100, 192);">-> text added</span></div>
<div class=""><br class="">
"In the first scenario, request is encoded in a QR code, and the End-user scans it with the camera via the application that is intended to handle the request from the RP. In this scenario, the request does not need to be intended for a specfiic `authorization_endpoint`
of a Self-Issued OP. Note that this will not work in a same-device Self-Issued OP flow, when the RP and the Self-Issued OP are on the same device.“<br class="">
<br class="">
Having the authorization endpoint in the QR code allows scanning with an arbitrary cam app whereas. Just sending the parameters requires the user to use the wallet app for that purpose. I think we need to support both options. </div>
<div class="">Note: the „just parameters approach“ is not reflect the in the cross device section of the spec yet. </div>
<div class=""><span style="color: rgb(12, 100, 192);">-> added a text with this meaning to the cross-device section too.</span><br class="">
<br class="">
- "### Common identifier for one or more Self-Issued OPs (better title needed)<br class="">
<br class="">
This identifier should be communicated to the RP as an `authorization_endpoint` URI which every Self-Issued OP supports“<br class="">
<br class="">
How is this use case any different from "### Separate identifier per Self-Issued OP (better title needed)“ from a RP perspective? I’m asking since I don’t understand why this section deviates from the rest of the section by using the authorization endpoint
as identifier instead of the issuer URL? </div>
<div class=""><span style="color: rgb(12, 100, 192);">-> merged these two sections and clarified the text, but needs to be discussed if we want to keep this descriptive text.</span><br class="">
<br class="">
- "## Relying Party Registration“<br class="">
<br class="">
The new revision supports entity statements and DIDs as only client id. This mandates signed requests for SIOP, which makes SIOP significantly more complex to implement. How is this justified?</div>
<div class=""><br class="">
</div>
<div class="">I personally would feel better to retain the simple "client id is redirect uri“ approach as third option. </div>
<div class=""><span style="color: rgb(12, 100, 192);">-> The intention was not to remove "client id is redirect uri“ option. I added the text clarifying that when the request is not signed, "client id is redirect uri“.</span></div>
<div class=""><font color="#0c64c0">-> I also added a summary of the options in the very beginning of the registration and discovery section.</font></div>
<div class=""><br class="">
- "# Self-Issued OpenID Provider Response {#siop-authentication-response}" does not describe handling of SIOP responses with other than "<a href="https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fself-issued.me%2Fv2&data=04%7C01%7CKristina.Yasuda%40microsoft.com%7Ce8687928bf7848967c2c08d9a1e912f3%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637718844724683333%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=l1sGYdWOSvIDqjHKCJDSUJcai47TQ%2FVTAKL9hCXvVeE%3D&reserved=0" originalsrc="https://self-issued.me/v2" shash="aJEN6ynRQfK/8VPm7faR6Vypv501EXflB1DfszPI7RkEv3GMiexnrXl+pinMzz2cwo9rczpHqEhyZBHN1fKvIuFsgteGMGxcRnuOGaR/MYiJZyMF3OK5ytSzheXCtjSLCCoSKQnWrouPx5+6vnAlB1JHIvZvmMRtXOGIz/2ecKU=" class="">https://self-issued.me/v2</a>"
iss values<br class="">
<br class="">
- "1. The Relying Party (RP) MUST validate that the value of the `iss` (issuer) Claim equals to the `authorization_endpoint` in the Self-Issued OP metadata. When static Self-Issued OP Discovery metadata has been used, `iss` MUST be `<a href="https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fself-issued.me%2Fv2%2560&data=04%7C01%7CKristina.Yasuda%40microsoft.com%7Ce8687928bf7848967c2c08d9a1e912f3%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637718844724693333%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=5LktXgKYqbRVK3UT9dJzpg6eF%2FDYXZkdHj6VHNOayWw%3D&reserved=0" originalsrc="https://self-issued.me/v2%60" shash="kTNIvQc8bSEfDMHVPuIDSm4Y+sg23NHaZnDZNbcNgWNT9+PAU6Pr4PGmLZnVD7mDcRoKbSAWuWNyPiOW3PnJm1r2CVOyLC6kxcDqySzm2/cMA/giyN69DZR9wmg4ksON/qVLEjy32fgLV2/72sQoXh1vfzLTHyOjQcnOv5v/TBI=" class="">https://self-issued.me/v2`</a>. When
dynamic Self-Issued OP Discovery metadata has been performed, `iss` MUST exactly match the `issuer` identifier pre-obtained by the RP.“ </div>
<div class=""><br class="">
</div>
<div class="">The first sentence contradicts the last sentence (which I think ist correct). I suggest to remove the first sentence.</div>
<div class=""><span style="color: rgb(12, 100, 192);">-> Your understanding is correct. I removed the first sentence.</span><br class="">
<br class="">
- "1. The RP MUST validate that the `aud` (audience) Claim contains the value of the `redirect_uri` that the RP sent in the Authentication Request as an audience.“ </div>
<div class=""><br class="">
</div>
<div class="">I think this must be changed to incoporate the new possible values (DID, entity statement id).</div>
<div class=""><span style="color: rgb(12, 100, 192);">-> good catch! text updated.</span><br class="">
<br class="">
- "1. The RP MUST validate the signature of the ID Token. When Subject Syntax Type is `jkt`, …" </div>
<div class=""><br class="">
</div>
<div class="">It ist unclear to me how the RP determines what subject syntax type is used. Is it the existence of the „sub_jwk“ claim in the id token? </div>
<div class=""><span style="color: rgb(12, 100, 192);">-> Currently, if subject syntax type is did, sub=did:<method name>:<identifier>; if subject syntax type is jwk, sub_jwk is present. I added a text to this meaning, but feedback welcome if this is enough
and makes sense.</span></div>
<div class=""><span style="color: rgb(12, 100, 192);"><br>
</span></div>
<div class="">General findings</div>
<div class=""><br class="">
</div>
<div class="">- The draft currently lacks a couple of references to other specs just some examples: JWK, DID, Claims Aggregation specification, DID-CORE, VC-DATA, RFC6749 and OpenID-Core</div>
<div class=""><span style="color: rgb(12, 100, 192);">-> thank you for reminding. will do an editorial PR.</span></div>
<div class="">- I would strongly encourage to authors to add more examples. </div>
<div class=""><span style="color: rgb(12, 100, 192);">-> I added some examples in the text and added a placeholder for a separate examples sections, and what examples I think should go there - feedback welcome.</span></div>
<div class=""><br class="">
</div>
<div class="">best regards,</div>
<div class="">Torsten. </div>
<div class=""><br class="">
</div>
</div>
</body>
</html>