Re: [GNAP] Genart last call review of draft-ietf-gnap-core-protocol-16

[Justin Richer <[e-mail]>]

Hi Dan, some responses inline.

> On Nov 28, 2023, at 6:54 AM, Dan Romascanu via Datatracker <[e-mail]> wrote:
> 
> Reviewer: Dan Romascanu
> Review result: Ready with Issues
> 
> I am the assigned Gen-ART reviewer for this draft. The General Area
> Review Team (Gen-ART) reviews all IETF documents being processed
> by the IESG for the IETF Chair.  Please treat these comments just
> like any other last call comments.
> 
> For more information, please see the FAQ at
> 
> <https://wiki.ietf.org/en/group/gen/GenArtFAQ>.
> 
> Document: draft-ietf-gnap-core-protocol-16
> Reviewer: Dan Romascanu
> Review Date: 2023-11-28
> IETF LC End Date: 2023-11-21
> IESG Telechat date: Not scheduled for a telechat
> 
> Summary: Ready with Issues
> 
> A complex and detailed document. Well written, but demands relevant expertise
> to read, understand, use and implement. There are a couple of minor issues and
> a few nits that I would recommend addressing before approval for publication.
> 
> Major issues:
> 
> Minor issues:
> 
> 1. Section 2.3.2
> 
>> If the client instance has additional information to display to the
>   RO during any interactions at the AS, it MAY send that information in
>   the "display" field.  This field is a JSON object that declares
>   information to present to the RO during any interactive sequences.
> 
>   name (string):  Display name of the client software.  RECOMMENDED.
> 
> MAY and RECOMMENDED do not seem to be in tune here. Maybe the MAY needs to be a
> SHOULD?

The goal here is that the overall object is a MAY but within that object the display name is a SHOULD. So you don’t have to send the object, but if you do, then you should send the name.

> 
> 2. Section 3.1
> 
>> wait (integer):  The amount of time in integer seconds the client
>      instance MUST wait after receiving this request continuation
>      response and calling the continuation URI.  The value SHOULD NOT
>      be less than five seconds, and omission of the value MUST NOT be
>      interpreted as zero (i.e., no delay between requests).
>      RECOMMENDED.
> 
> I do not understand how this works from an operational point of view. I assume
> there is some logic in picking 5 sec as a minimal value, but then why is this
> limitation a SHOULD and not a MUST?

This was discussed on another review - the editors are going to plan to make an omission mean 5 seconds to align with OAuth 2.

> 
> Nits/editorial comments:
> 
> 1. It's probably too late for a change, but the name of the new protocol does
> not exactly fit it's purpose, as this is not an authorization protocol, but
> rather a delegation for authorization protocol. Well ...

Naming is hard and you’re not wrong on it not being a perfect fit and also being a bit too late to re-do it. There was a huge discussion at the start of the WG on the name, and this was the least-contentious option at the time.

> 
> 2.Section 1.2 - in the Legend of the figure what does 'potential equivalence'
> mean? (same question in legend of figure in 1.6.1)

It simply means they might be the same person, but they might not be.

> 
> 3. General remark - I would recommend numbering the Figures in the document.

Thanks, I think we’re missing figure captions throughout so we should probably go fix that!

> 
> 4. There are no definition of the lines in the figures inserted in 1.6.2,
> 1.6.3, etc. I assume the conventions are the same as in 1.6.1, but this should
> be better clearly specified.

Thanks, we can call that out.

> 
> 5. Section 3.6 s/Additional error codes can be defined in the Error Code
> Registry/Additional error codes can be defined in the Error Codes Registry/
> 
> 6. Section 7.3.2 - MTLS needs to be expanded and a reference must be provided.
> 
> 7. Section 7.3.3 - JWS needs to be expanded and a reference must be provided.
> 


Thanks, we’ll take a look at all those as well, these make sense to fix and should be straight forward.


 — Justin