Re: [OAUTH-WG] Lars Eggert's Discuss on draft-ietf-oauth-dpop-14: (with DISCUSS and COMMENT)
The smaller comments/nits are addressed in this PR https://github.com/danielfett/draft-dpop/pull/184/files On Wed, Apr 12, 2023 at 6:52 AM Brian Campbell wrote: > 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 >>
Re: [OAUTH-WG] Lars Eggert's Discuss on draft-ietf-oauth-dpop-14: (with DISCUSS and COMMENT)
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
[OAUTH-WG] Lars Eggert's Discuss on draft-ietf-oauth-dpop-14: (with DISCUSS and COMMENT)
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?
--
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?
### Too many authors
The document has six authors, which exceeds the recommended author limit. Has
the sponsoring AD agreed that this is appropriate?
### Missing references
No reference entries found for these items, which were mentioned in the text:
`[IANA.OAuth.Parameters]`.
### 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.)
### 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`
## 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":
}```
### Outdated references
Document references `draft-ietf-oauth-security-topics-21`, but `-22` is the
latest available revision.
### 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
## 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
___
OAuth mailing list
OAuth@ietf.org
https://www.ietf.org/mailman/listinfo/oauth
