Hi all,

thanks for writing this document. I have read through it as part of my shepherd 
writeup and here are a few comments and questions.

Generic Comments:

As a style issue, it would be good to treat code segments as figures with a 
figure headings so that references in the text is easier to manage.

Editorial: s/This draft/This specification

Normative language: If you search for MUSTs in the document you will notice two 
things: First, they are not necessarily where you would expect them.

For example: This is from page 6, which explains an example.

"
   The JSON objects with type fields of account_information and
   payment_initiation represent the different authorization data to be
   used by the AS to ask for consent and MUST subsequently also be made
   available to the respective resource servers.
"

Another example from page 29: Here are two approaches offered to mitigate a 
specific threat. If there are two ways then a MUST is appropriate IMHO. If 
there are others, maybe it is useful to tell the reader that there are other 
approaches and when to use those mitigations so that a developer can make an 
informed decision.

"
In order to ensure their integrity, the
   client SHOULD send authorization details in a signed request object
   as defined in [I-D.ietf-oauth-jwsreq] or use the request_uri
   authorization request parameter as defined in [I-D.ietf-oauth-jwsreq]
   in conjunction with [I-D.ietf-oauth-par] to pass the URI of the
   request object to the authorization server.
"

Second, several MUST/SHOULD/MAYs are used with little guidance for the 
developer. Help developers to make the best decision.

For example:
"
   Implementers MUST design and use authorization details in a privacy-
   preserving manner.

   The AS MUST take into consideration the privacy implications when
   sharing authorization details with the client or resource servers.
"

In the rest of the review I will go through the individual sections and share 
my impressions.


Abstract

The abstract says:

"
   This document specifies a new parameter authorization_details that is
   used to carry fine-grained authorization data in the OAuth
   authorization request.
"

Later in the draft we will learn that the authorization_details parameter is 
not just carried in the authorization request but also in the token request.

Maybe you could say:

"
   This document specifies a new parameter authorization_details that is
   used to carry fine-grained authorization data in OAuth
   messages.
"

Section 1: Introduction

References to blog posts about the motivation for why this work is needed might 
be better replaced with a short summary, particularly if they motivate the 
solution in the document. If you don't want such a summary in the body of the 
document, then the appendix might also be a good place.

The last paragraph in the introduction covers one solution aspect. Is there a 
specific reason for putting a spotlight on it (compared to other solution 
aspects)? I think it comes too early in the document.

I missed one aspect in the introduction that might be relevant for the reader, 
namely that this specification itself is not sufficient for interoperability.  
Developers and those deploying RAR need to make various agreements about the 
JSON structure carried inside the authorization_details parameter since the 
spec does not do much more than to say that you must use a JSON-based structure 
in this newly defined parameter. The document is more a guide on how to specify 
this JSON-based structure rather than mandating a specific structure. I think 
the introduction would be a good place to set the tone for the reader about 
what to expect in the rest of the document.

Section 2: Request parameter "authorization_details"

At the start of the section I would have expected the structure of the payload 
being mandated. Something like "An implementation that claims conformance with 
this specification MUST use the following data model for the JSON structure 
carried inside the authorization_details parameter." I understand that you want 
to keep everything open for the developer.

Here are examples:

- "The array [of the JSON structure] MAY contain several elements of the same 
type."
- type: "This field MAY define which other elements are allowed in the request. 
This element is REQUIRED."
(Is this field required to be understood by parsers or is it required to 
include this field in any JSON structure included in the authorization_details 
parameter? Interestingly, the content of the type element is not mandated 
either.)
- None of the other fields seem to be required to be included nor implemented.

[Note: Throughout the spec you have used different ways to describe the 
"fields" (type, datatype, ) in the JSON structure. I think it would be useful 
to use the same term throughout the document to avoid confusion.]

In Section 2 you include Section 2.2 "Authorization Data Types", which is 
interesting since there are two elements defined: data type and type. While the 
subject heading of Section 2.2 gives the impression that you would be 
describing the data type you are actually describing the type element. IMHO the 
reason for this confusion is that you use the term "authorization data" to 
refer to authorization_details. Maybe you could use authorization_details 
throughout the document when you refer to the entire object.

Section 3: Authorization Request

You might want to give some reason to API designers about why they should be 
using the "scope" parameter together with the authorization_details parameter 
since it introduces extra complexity.

Section 6: Token Request

This section mostly consists of the text in Section 6.1 "Comparing 
authorization details", which essentially acknowledges that comparing any two 
arbitrary authorization details requests is difficult. Then, you give an 
example of how this could work. The use of RFC 2119 language is IMHO for the 
example not appropriate.

For example: The "MAY" in the paragraph below has to be replaced by a "may".

"
   For our running example, the client MAY ask for all permissions of
   the approved grant of type payment_iniation applicable to the
   resource server residing at https://example.com/payments
"

Regarding the use of the MAY in this paragraph I am not sure whether it is 
related to the example or a generic consideration. If it is generic, maybe you 
want to move it into Section 2 (as a description of the element).

"
   The predefined authorization data element locations MAY be used by
   the client to request an access token valid for a certain resource
   server, i.e. it is the recommended way to request issuance of
   audience restricted access tokens.
"

Section 10.  Metadata

In this section I have expected to see an example, as present in all other 
sections. For some reasons you decided not to include one.

Section 11.  Scope value "openid" and "claims" parameter

This section feels a bit out of place. It almost sounds like a design idea for 
use in the OpenID foundation.

Section 12.  Implementation Considerations

This section is good. Just a minor remark: In this paragraph the SHOULD appears 
to be inappropriate. I think you just want to make the reader aware of existing 
specifications to deal with concerns regarding large request sizes.

   Implementers SHOULD therefore consider using the request_uri
   parameter as defined in [I-D.ietf-oauth-jwsreq] in combination with
   the pushed request object mechanism as defined in
   [I-D.ietf-oauth-par] to pass authorization details in a reliable and
   secure manner.

Section 13.  Security Considerations

This paragraph is a repetition of text in Section 2.1:

   All strings MUST be compared using the exact byte representation of
   the characters as defined by [RFC8259].  This is especially true for
   the type field, which dictates which other fields and functions are
   allowed in the request.  The server MUST NOT perform any form of
   collation, transformation, or equivalence on the string values.

14.  Privacy Considerations

In this paragraph (reformatted) you are turning some of the informative 
references into normative:

   Any sensitive personal data included in authorization details MUST be
   prevented from leaking, e.g., through referrer headers.
   Implementation options include

   1) encrypted request objects as defined in [I-D.ietf-oauth-jwsreq],
   2) transmission of authorization details via end-to-end encrypted 
connections between client and authorization
   server by utilizing [I-D.ietf-oauth-par], and
   3) the request_uri authorization request parameter as defined in
   [I-D.ietf-oauth-jwsreq].

IMHO [I-D.ietf-oauth-jwsreq] needs to become a normative reference. 
[I-D.ietf-oauth-jwsreq] has been published as RFC 9101.
When you mandate the use of encryption (either through [I-D.ietf-oauth-jwsreq] 
or by [I-D.ietf-oauth-par]) then some guidance on when to use what mechanism 
would be good and how credential types influence the deployment.

The following paragraph makes me wonder how developers make design decisions 
since they have
to balance the privacy implications and the desire to make fine-grained 
authorization decisions:

   The AS MUST take into consideration the privacy implications when
   sharing authorization details with the client or resource servers.
   The AS SHOULD share this data with those parties on a "need to know"
   basis.

Is there any guidance you could give to developers to help them make a good 
decision?

Ciao
Hannes

IMPORTANT NOTICE: The contents of this email and any attachments are 
confidential and may also be privileged. If you are not the intended recipient, 
please notify the sender immediately and do not disclose the contents to any 
other person, use it for any purpose, or store or copy the information in any 
medium. Thank you.
_______________________________________________
OAuth mailing list
OAuth@ietf.org
https://www.ietf.org/mailman/listinfo/oauth

Reply via email to