Thanks for the feedback, Jasdip! More below...

> -----Original Message-----
> From: regext <[email protected]> On Behalf Of Jasdip Singh
> Sent: Sunday, October 9, 2022 5:16 PM
> To: James Galvin <[email protected]>; [email protected]
> Subject: [EXTERNAL] Re: [regext] WGLC: draft-ietf-regext-rdap-openid-17
>
> Caution: This email originated from outside the organization. Do not click 
> links
> or open attachments unless you recognize the sender and know the content is
> safe.
>
> Hi.
>
> Overall, +1.
>
> While reviewing the latest draft, wanted to share few comments (sorry, if a 
> bit
> late):
>
> 1.2: "willing to share more information about them self" ... Minor: wouldn't
> "themselves" read better than "them self"?

[SAH] OK.

> 1.2: "It can also provide the ability to collect additional user 
> identification
> information, and that information can be shared with the consent of the 
> user."
> ... Not clear who that information could be shared with.

[SAH] "shared with the RDAP server operator with the consent of the user in 
order to help the server operator make access control decisions" is the 
intent.

> 3.1.2: "The RDAP server sends the RDAP client and Authentication Request"
> …Minor: Should "and" be "an"?

[SAH] Yes, you're right.

> 3.1.3.2: "as described in Section 3.1.2.2 of the OpenID Connect Core 
> protocol
> [OIDCC]" ... Minor: Just in case, noticed that such section links to the 
> Open ID
> documentation either point to within this doc (read the "htmlized" version), 
> or
> go nowhere.

[SAH] That sounds like a limitation of whatever is producing the "htmlized" 
version of the draft, but OK.

> 4.1.1: "An OPTIONAL "userClaims" object that contains the set of claims
> associated with the End-User's identity as used/requested by the RDAP server
> to make access control decisions." ... For consistency with other field
> definitions, should we mention that it is an array of strings?

[SAH] It's not necessarily an array of strings (see the example where the set 
of claims includes a URL, for example) , so I don't think so. I'd prefer to 
leave that description as-is, noting that "The set of possible values is 
determined by OP policy".

> 4.1.3: ""iss": (MANDATORY) a string value equal to Issuer Identifier of the 
> OP as
> per OpenID Connect Core specification [OIDCC]" ... Should it be clarified 
> that
> "iss" is a URI value?

[SAH] Yes, that should be consistent with the description in 4.1.1.

> 4.7: "RDAP servers MUST reject queries that include identification 
> information
> that is not associated with a supported OP by returning an HTTP 501 (Not
> Implemented) response." ... Should this not be a 401 (Unauthorized) instead? 
> ...
> I know Andy suggested a 400 (Bad Request). :)

[SAH] I prefer 401. "Unauthorized" would imply that an attempt was made to 
authorize the user, but that can't be done because the OP isn't supported.

> 4.8: "If a client sends any request that includes an unknown HTTP cookie, 
> the
> server MUST return an HTTP 409 (Conflict) error." ... Should this not be a 
> 401
> (Unauthorized) instead?

[SAH] I think that 400 that Andy suggested is the better response for the same 
reason noted above.

> 5: In some operational scenarios (such as a client that is providing a proxy
> service), an RP can receive tokens with an "aud" value that does not include 
> the
> RP's client_id." ... Should we further elaborate "a client that is providing 
> a proxy
> service"? ... Not clear to me. :)

[SAH] The text in this section was originally suggested by Mario. Let's see if 
he can help make it clearer.

> 8.3: "Value: academicPublicInterestDNSRResearch" … Minor: Is there an extra
> 'R' in "DNSRResearch"?

[SAH] There is! Will fix.

Scott
_______________________________________________
regext mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/regext

Reply via email to