Thank you, Lars, for the review. I've endeavored to respond to your comments, especially the Discuss item, inline below. And I will soon make corresponding updates to the document source.
On Wed, Apr 12, 2023 at 4:03 AM Lars Eggert via Datatracker < nore...@ietf.org> wrote: > Lars Eggert has entered the following ballot position for > draft-ietf-oauth-dpop-14: Discuss > > When responding, please keep the subject line intact and reply to all > email addresses included in the To and CC lines. (Feel free to cut this > introductory paragraph, however.) > > > Please refer to > https://www.ietf.org/about/groups/iesg/statements/handling-ballot-positions/ > for more information about how to handle DISCUSS and COMMENT positions. > > > The document, along with other ballot positions, can be found here: > https://datatracker.ietf.org/doc/draft-ietf-oauth-dpop/ > > > > ---------------------------------------------------------------------- > DISCUSS: > ---------------------------------------------------------------------- > > # GEN AD review of draft-ietf-oauth-dpop-14 > > CC @larseggert > > ## Discuss > > ### Section 12.7.1, paragraph 3 > ``` > However, the initial registration of the nonce claim by [OpenID.Core] > used language that was contextually specific to that application, > which was potentially limiting to its general applicability. > > This specification therefore requests that the entry for nonce in the > IANA "JSON Web Token Claims" registry [IANA.JWT] be updated as > follows to reflect that the claim can be used appropriately in other > contexts. > ``` > Is OpenID as the change controller OK with the IETF changing the IANA > registry in this way? > I believe so, yes. The authors of this draft are all members of (or co-chairs) the OpenID WG that is the change controller for the registry. As such, we are reasonably comfortable representing the rough consensus of that WG to give the OK to the registry update. Is there something more that needs to happen? Admittedly, I've not seen or done an update like this before so am not sure what all is supposed to be involved. > > ---------------------------------------------------------------------- > COMMENT: > ---------------------------------------------------------------------- > > ## Comments > > ### Section 9, paragraph 5 > ``` > only at the issuing server. Developers should also take care to not > confuse DPoP nonces with the OpenID Connect [OpenID.Core] ID Token > nonce. > ``` > Could this ambiguity not be avoided by using a different term/claim? > Despite that text being included, I'd argue that the likelihood of confusion is pretty low as they are different tokens in different contexts. There was some debate over the claim name (with pros and cons one both sides) but ultimately the rough consensus outcome of the WG was to use/reuse the "nonce" claim in the DPoP proof JWT as the draft currently has it. ### Too many authors > > The document has six authors, which exceeds the recommended author limit. > Has > the sponsoring AD agreed that this is appropriate? > Roman can correct me, if I'm wrong, but has said previously that he is OK with it. ### Missing references > > No reference entries found for these items, which were mentioned in the > text: > `[IANA.OAuth.Parameters]`. > Will fix. > > ### DOWNREFs > > DOWNREF `[RFC8792]` from this Proposed Standard to Informational `RFC8792`. > (For IESG discussion. It seems this DOWNREF was not mentioned in the Last > Call > and also seems to not appear in the DOWNREF registry.) > I believe RFC8792 should have been an Informative Reference. I'll update. > > ### Inclusive language > > Found terminology that should be reviewed for inclusivity; see > https://www.rfc-editor.org/part2/#inclusive_language for background and > more > guidance: > > * Term `native`; alternatives might be `built-in`, `fundamental`, > `ingrained`, > `intrinsic`, `original` > * Term `blindly`; alternatives might be `visually impaired`, `unmindful > of`, > `unconcerned about`, `negligent of`, `unaware`, `uncomprehending`, > `unaware`, `uncritical`, `unthinking`, `hasty`, `blocked`, `opaque` > I'll look to update with alternatives or omit the words. > > ## Nits > > All comments below are about very minor potential issues that you may > choose to > address in some way - or ignore - as you see fit. Some were flagged by > automated tools (via https://github.com/larseggert/ietf-reviewtool), so > there > will likely be some false positives. There is no need to let me know what > you > did with these suggestions. > > > ### JSON > > ``` > > { > "error": "use_dpop_nonce" > ^ Expecting ',' delimiter > "error_description": > }``` > > Will fix. > ### Outdated references > > Document references `draft-ietf-oauth-security-topics-21`, but `-22` is the > latest available revision. > Okay. > > ### URLs > > These URLs in the document can probably be converted to HTTPS: > > * http://www.iana.org/assignments/jwt > * http://openid.net/specs/openid-connect-core-1_0.html Will convert to HTTPS. > > > ## Notes > > This review is in the ["IETF Comments" Markdown format][ICMF], You can use > the > [`ietf-comments` tool][ICT] to automatically convert this review into > individual GitHub issues. Review generated by the [`ietf-reviewtool`][IRT]. > > [ICMF]: https://github.com/mnot/ietf-comments/blob/main/format.md > [ICT]: https://github.com/mnot/ietf-comments > [IRT]: https://github.com/larseggert/ietf-reviewtool > > > > -- _CONFIDENTIALITY NOTICE: This email may contain confidential and privileged material for the sole use of the intended recipient(s). Any review, use, distribution or disclosure by others is strictly prohibited. If you have received this communication in error, please notify the sender immediately by e-mail and delete the message and any file attachments from your computer. Thank you._
_______________________________________________ OAuth mailing list OAuth@ietf.org https://www.ietf.org/mailman/listinfo/oauth
