[Openid-specs-ab] SIOP v2 review comments

[Kristina Yasuda]

Hi Edmund,

I created a PR addressing the comments you have sent below to the ML. Please review and approve.
openid / connect / Pull Request #91: addressing Edmund's siop-v2 comments sent to the ML — Bitbucket<https://bitbucket.org/openid/connect/pull-requests/91/addressing-edmunds-siop-v2-comments-sent>

A lot of good catches. Thank you very much!
Kristina

________________________________
From: Edmund Jay <ejay at mgi1.com>
Sent: Friday, December 10, 2021 9:47
To: Kristina Yasuda <Kristina.Yasuda at microsoft.com>; openid-specs-ab at lists.openid.net <openid-specs-ab at lists.openid.net>
Subject: SIOP v2 review comments

Hi Kristina,

I've reviewed the SIOP v2 and here are some comments.

I"ve also created a pull request with some minor editorial/grammer fixes.

General comments:

The html document bookmarks don’t work e.g. {#rp-metadata}, etc..
-> addressed


2.4
, and hard to be verified in a digital form.

Are the digitally scanned documents hard to verify?
Or the scanned documents are hard to verify electronically?
-> Clarified. scanned docs are hard to be verified digitally

3.1
Cryptographically Verifiable Identifiers - an identifier that is either based upon or can be resolved to cryptographic key material and is used by the Self-Issued OPs in the sub Claim of the ID Token to prove possession of the cryptographic key used to sign the ID Token. Types of such identifiers are defined in this specification as Subject Syntax Types.

The definition is already defined in 4, so therefore we can rewrite this part as:

Subject Syntax Types - defines types of cryptographically verifiable identifiers.
-> I like it! thank you for the suggestion!

4.1
RP and OP are defined in OIDC, so don’t need to relist them here
Self-issued OP is not an abbreviation. OP is already listed in OIDC.
-> l agree, I took out Self-Issued OP, but kept OP/RP, keeping in mind that some of the readers of this specification are not familiar with OIDC.Core. Can take out if there are strong counter-opinions.

6.
Same-Device Self-Issued OP model: Self-Issued OP is on the same device on which the End-User's user interactions are occurring.

“The RP might be a Web site on a different machine and use the same-device Self-Issued OP flow for authentication.”

This sentence seems to be describing the Cross device model where the RP and SIOP are on different devices.
-> Absolutely. Thank you for catching it.

7.
If the Self-Issued OP is able to perform pre-registration of the RP, client_id MUST equal to the client identifier the RP has pre-obtained using [OpenID.Registration] or out-of-band mechanisms, and registration nor registration_uri parameters MUST NOT be present in the Self-Issued OP Request. If the Self-Issued OP Request is signed, the public key for verification MUST be obtained during the pre-registration process.

I think this paragraph needs to be in 11 since it’s about the authentication request. It's also repeated 2/3 times in this document.
-> I deleted the part about client_id since it is about the authn per your comments. the part is repeated twice - in the overview and in the description of the pre-registration part. the overview was added upon the request to give more clarify. I think mentioning this twice reads ok..?

9.2
Need a short sentence to precede the metadata definitions to explain what they’re for.
-> Added.

10.1
This whole section seems to be about the authentication request and not registration.
-> Good point. moved relevant parts to the request section.

10.2
Maybe add some context to say that registration parameters are sent along with the authentication request
-> Added.

10.2.2.2
To obtain the DID Document, Self-Issued OP MUST use DID Resolution defined by the DID method must be used by the RP.

Too many “MUST” in that sentence.
-> fixed

11.
client_id
REQUIRED. Client ID value for the Client, which in this case contains the redirect_uri value of the RP.

Various places in the doc mentions client_id must also be the registered value.
-> fixed. very good catch, thank you!

11.1
Is response_mode=post required for all cross-device flows?
If yes, the example does not have one.
-> Already fixed in a previous PR. When we introduce PARM, not all cross-device flows will have response_mode=post, but for now they do)
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openid.net/pipermail/openid-specs-ab/attachments/20211215/f0aecf7a/attachment.html>