Re: [Last-Call] [Gen-art] Genart last call review of draft-ietf-oauth-rar-15

[Lars Eggert <[e-mail]>]

Robert, thank you for your review and thank you all for the following discussion. I have entered a No Objection ballot for this document.

Lars


> On Nov 18, 2022, at 00:45, Robert Sparks <[e-mail]> wrote:
> 
> 
> On 11/17/22 4:44 PM, Robert Sparks wrote:
>> Apologies - an upload fumble left -00 empty. I've revised it to have content at -01.
> 
> Which I'll also paste here for convenience:
> 
> 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://trac.ietf.org/trac/gen/wiki/GenArtfaq>.
> 
> Document: draft-ietf-oauth-rar-14 (<- that was a typo - I really did review 15)
> Reviewer: Robert Sparks
> Review Date: 2022-11-17
> IETF LC End Date: 2022-11-17
> IESG Telechat date: Not scheduled for a telechat
> 
> Summary:
> 
> Major issues: Essentially ready for publication as a Proposed Standard RFC, but with issues to discuss before publication
> 
> I have two major issues that I think need discussion:
> 
> Major Issue 1) The document seems to be specifying a new way of comparing json names, claiming it is what RFC8259 requires, but I disagree that RFC8259 says what this document is claiming. If I'm correct, the document is trying to rely on the text in section 7 of RFC8259 to support the idea that implementation MUST NOT alter the json names (such as applying Unicode normalization) before comparison and that the comparison MUST be performed octet-by-octet. Rather, section 7 says something more like "you better escape and unescape stuff correctly if you’re going to do unicode codepoint by codepoint comparison" which is a completely different statement.
> 
> If I'm right, and this is a new comparison rule that goes beyond what JSON itself defines, I think the group should seek extra guidance from Unicode experts. If I'm wrong and this behavior is defined somewhere else, please provide a better pointer to the definition.
> 
> In many environments, its unusual for an implementation relying on a stack below it to have any say at all on whether normalization is going to be applied to the unicode before the application gets to look. Rather than trying to work around the problem you've identified with normalization by specifying the comparison algorithm, consider just making stronger statements about the strings used in the json names the document defines. Why _can't_ you restrict the authorization_details values to ascii? If it's because you want to present the string to a user, consider putting a presentation string elsewhere in the json that is not used for comparison.
> 
> Major Issue 2) The suggested pattern demonstrated starting in figure 16 (using [] to mean "let the user choose") seems underspecified. If the point is that different APIs may invent different mechanics _like_ this, and that this is only an example. Make it much clearer that this is an example. If this is a pattern you expect all APIs to follow, then more description is warranted. Is it intended that a user could add and remove things arbitrarily to such lists? For instance is it intended that this support an interaction where the client is asking for permission to operate on account A, and the user can say "no, but you can operate on account B"?
> 
> Minor issues:
> 
> Section 2: What does "The AS MUST ensure that there is no collision between different authorization details types that it supports." mean? How is this a 2119 requirement? I _think_ you're trying to point out that the authorization_details string is a unique-key at any AS and that that has consequences on the API _designer_, but I don't know what is expected of an AS coder here. Some clearer words are needed.
> 
> Section 7: Please expand on, or rephrase, "if these are deemed of no intended use for the client." Could you just delete the phrase? If you are only explaining why an AS might do this, make it clear that it's an example (and split the example into a separate sentence). If this is the _only_ reason an AS might omit values in the token Response, then more detail is needed.
> 
> 
> Nits/editorial comments:
> 
> Throughout the document, there is a mix of "authorization_details" and "authorization details". It is not completely consistent using one when talking about the parameter and the other when talking about the general concept of details about authorization. Please inspect each occurrence and verify that it is using the correct form.
> 
> Introduction: suggest changing 'defines the parameter scope' to 'defines the parameter "scope"'
> 
> Introduction just after figure 1 - expand AS (first use).
> 
> Section 2.2: Please rework the sentence where you mention "the cartesian product" and remove the reference - it is not helpful. Instead of "That is to say," just say it.
> 
> Section 6.1, discussion just after Figure 13. Consider rewriting the sentence to avoid the term "non-subsuming". Think about translation into other languages.
> 
> Section 9.2 intro to Figure 21: something is missing from this sentence. The RS: at the end is either not needed or needs more words?
> 
> Section 10: Please reconsider the first sentence. Write it in a way that doesn't imply the AS has agency (the AS can't "want"), and avoid the current "If...then MUST" construction. It would be better to say something like "To signal (foo) the AS (does these things)".
> 
> Section 10 just before Figure 23: Please clarify what it means for a client to announce types that they "use"? Do you mean "support"? Do you mean "intend to use in this interaction"? Or something else? Consider saying more about why allowing the client to announce these things is useful.
> 
> Appendix A.1 : expand ACR
> 
> Figure 30 caption : typo at eHaelth.
> 
> 
>> 
>> RjS
>> 
>> On 11/17/22 4:36 PM, Robert Sparks via Datatracker wrote:
>>> Reviewer: Robert Sparks
>>> 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://trac.ietf.org/trac/gen/wiki/GenArtfaq>.
>>> 
>>> Document: draft-ietf-oauth-rar-??
>>> Reviewer: Robert Sparks
>>> Review Date: 2022-11-17
>>> IETF LC End Date: 2022-11-17
>>> IESG Telechat date: Not scheduled for a telechat
>>> 
>>> Summary:
>>> 
>>> Major issues:
>>> 
>>> Minor issues:
>>> 
>>> Nits/editorial comments:
>>> 
>>> 
>>> 
>> 
>> _______________________________________________
>> Gen-art mailing list
>> [e-mail]
>> https://www.ietf.org/mailman/listinfo/gen-art
> 
> --
> last-call mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/last-call