
<html><head><meta http-equiv="Content-Type" content="text/html; charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class="">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=""><br class=""></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="openid://%60" class="">openid://`</a> does not work with web wallets. </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. <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? <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=""><br class="">- "# Self-Issued OpenID Provider Response {#siop-authentication-response}" does not describe handling of SIOP responses with other than "<a href="https://self-issued.me/v2" 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://self-issued.me/v2`" 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.<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).<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=""><br class=""></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="">- I would strongly encourage to authors to add more examples. </div><div class=""><br class=""></div><div class="">best regards,</div><div class="">Torsten. </div><div class=""><br class=""></div></body></html>