[Openid-specs-ab] SIOP v2 Review

[Torsten Lodderstedt]

Hi all, 

I reviewed the current master revision of SIOP v2 with a special focus on SIOP discovery, client id resolution and sub types. 

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. 

Here is my detailed feedback: 

- "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.“

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. 

- „## Self-Issued OpenID Provider Invocation“

I this section should also mention that the current custom URI scheme `openid://` <openid://%60> does not work with web wallets. 

"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.“

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. 
Note: the „just parameters approach“ is not reflect the in the cross device section of the spec yet. 

- "### Common identifier for one or more Self-Issued OPs (better title needed)

This identifier should be communicated to the RP as an `authorization_endpoint` URI which every Self-Issued OP supports“

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? 

- "## Relying Party Registration“

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?

I personally would feel better to retain the simple "client id is redirect uri“ approach as third option. 

- "# Self-Issued OpenID Provider Response {#siop-authentication-response}" does not describe handling of SIOP responses with other than "https://self-issued.me/v2" iss values

- "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 `https://self-issued.me/v2`. When dynamic Self-Issued OP Discovery metadata has been performed, `iss` MUST exactly match the `issuer` identifier pre-obtained by the RP.“ 

The first sentence contradicts the last sentence (which I think ist correct). I suggest to remove the first sentence.

- "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.“ 

I think this must be changed to incoporate the new possible values (DID, entity statement id).

- "1. The RP MUST validate the signature of the ID Token. When Subject Syntax Type is `jkt`, …" 

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? 

General findings

- 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
- I would strongly encourage to authors to add more examples. 

best regards,
Torsten. 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openid.net/pipermail/openid-specs-ab/attachments/20211107/8a183154/attachment.html>