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 <rjspa...@nostrum.com> 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 >> gen-...@ietf.org >> https://www.ietf.org/mailman/listinfo/gen-art > > -- > last-call mailing list > last-c...@ietf.org > https://www.ietf.org/mailman/listinfo/last-call
signature.asc
Description: Message signed with OpenPGP
_______________________________________________ OAuth mailing list OAuth@ietf.org https://www.ietf.org/mailman/listinfo/oauth
