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-art@xxxxxxxx
https://www.ietf.org/mailman/listinfo/gen-art
--
last-call mailing list
last-call@xxxxxxxx
https://www.ietf.org/mailman/listinfo/last-call
