Subject: [OAUTH-WG] draft-ietf-oauth-rar-08 review

From: Hannes Tschofenig <[email-masked]>
Date: Tue, 21 Dec 2021 12:06:48 +0000

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.