Last Call Review of draft-ietf-oauth-rar-15
review-ietf-oauth-rar-15-genart-lc-sparks-2022-11-17-01

(-00 was essentially empty, apologies)

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
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.