Hi Torsten! Thanks for the response. More inline ...
> -----Original Message----- > From: Torsten Lodderstedt <tors...@lodderstedt.net> > Sent: Tuesday, October 18, 2022 9:00 AM > To: Roman Danyliw <r...@cert.org> > Cc: oauth@ietf.org; Brian Campbell <bcampb...@pingidentity.com>; > jric...@mit.edu > Subject: Re: [OAUTH-WG] AD review of draft-ietf-oauth-rar-12 > > Hi Roman, > > thanks for you review. > > I’m trying to do the next round for the outstanding issues you raised. > > > Am 15.09.2022 um 00:30 schrieb Roman Danyliw <r...@cert.org>: > > > > Hi! > > > > I performed an AD review of draft-ietf-oauth-rar-12. Thanks for this > document. My feedback is as follows: > > > > ** Section 2. Editorial > > > > This field MUST be compared using an exact byte match of the string > > value against known types by the AS. > > > > Consider if you want to introduce how the lack of match will be handled here > - it is covered later. > > I’m not sure how to handle this as there is no direct consequence (in the > sense > of error condition) other then that different byte arrays constitute different > types. > > We could state something like this: "String values with different byte > representations constitute different types.“ ? Works for me. > > > > ** Section 2.2. > > > > All data fields are OPTIONAL for use by a given API > > definition. > > > > I don't follow how this is true in the general case. I was under the > > impression > that this section defined what is expected to be common fields. Couldn't some > AS with a particular type require their presence? > > > > ** Section 3. Editorial > > > > OLD > > In case of authorization requests as defined in [RFC6749], > > implementors MAY consider to use > > > > NEW > > In case of authorization requests as defined in [RFC6749], > > implementors MAY consider using ... > > > > ** Section 3. Typo. s/intiate/initiate/ > > > > ** Section 5. Typo. > > > > OLD > > authorization details type > > or authorization details > > > > NEW > > authorization_details type > > or authorization_details > > The spec uses authorization details types to refer to the general concept and > authorization_details to refer to the parameter. I think the former is > appropriate at this place. Understood. > > > > ** Section 6.1 > > > > However, when comparing a new request to an existing request, > > authorization servers can use the same processing techniques as used > > in granting the request in the first place to determine if a resource > > owner needs to authorize the request. > > > > Why is it possible to assess two arbitrary requests in this case to > > determine "if > a resource owner needs to authorize the request", but the prior paragraph > explicitly calls out that comparing two arbitrary requests is not feasible? > To me > is seems like comparing two requests to understand if more or less permissions > are being requested is equivalent to determining if a new request exceed the > current request to determine if going back to the resource owner is needed. > > > > ** Section 6.1. Typo. s/isaunce/issuance/ > > > > ** Section 7. > > > > If the client does not specify > > the authorization_details token request parameters, the AS determines > > the resulting authorization details at its discretion. The > > authorization server MAY consider the values of other parameters such > > as resource and scope if they are present during this processing, and > > the details of such considerations are outside the scope of this > > specification. > > > > This guidance seems to indicate the use of the scope parameter is optional > > in > determining the authorization details. Section 3.1 says "The AS MUST consider > both sets of requirements in combination with each other for the given > authorization request." My read is that this is conflicting guidance and > Section > 3.1 is correct. > > Justin: „You aren’t required to use “scope” in order to use > “authorization_details”, but we do want to say that the AS is allowed to (or > even is supposed to) consider both “scope” and “authorization_details” when > determining the resulting access for any given request that might have both. > The guidance in 3.1 should probably say “the AS MUST consider all > requirements present on a request” or something like that?“ > > Section 3 already states that the AS must consider scope and resource in > addition to authorisation details. I suggest to drop that sentence entirely. Works for me. > > > > ** Figure 15. The text prior to this figure says that for "For our running > example, this would look like this" indicating that this figure is similar to > previous examples. There is one key different - this is the first use of a > "payment_initiator" type with the API URL prepended. > > > > ** Section 7.1. Typo. s/sub set/subset/ > > > > ** Section 8. What is the difference between this section and Section 5 > beyond this text explicitly stating the name of the error value > (invalid_authorization_details). I'd recommend stating the normative behavior > twice; that is, why are both sections needed? > > 5 and 8 define the error behaviour of different endpoint. But you are right, > the > text is the same (with 5 being even more detailed). Suggest to remove the text > in 8 with a reference to 5. Works for me. Thanks, Roman > best regards, > Torsten. > > > > > ** Section 9.2. Editorial. There is some kind of rendering issues in the > RFC7622 reference. It reads "[!@RFC7662]". > > > > ** Section 11.2. > > > > Products supporting this specification should provide the following > > basic functions: > > > > Should this section be more tightly scoped to AS behavior instead of a > "products"? > > > > ** Section 11.2. > > > > Accept authorization_details parameter in authorization requests > > including basic syntax check for compliance with this > > specification > > > > Why only "basic syntax checking"? Perhaps "syntax checking"? > > > > ** Section 11.2 > > > > One option would be to have a mechanism allowing the registration of > > extension modules, each of them responsible for rendering the > > respective user consent and any transformation needed to provide the > > data needed to the resource server by way of structured access tokens > > or token introspection responses. > > > > I don't follow the flexibility being described here. "One option ..." with > respect to what? > > > > ** Section 11.3. Could this section provide an example of what it would > mean to use JSON schema ids in the type value. > > > > ** Section 12. Please note that the Security Considerations of RFC6749, > RFC7662, and RFC8414 apply. > > > > ** Section 13. > > > > Implementers MUST design and use authorization details in a privacy- > > preserving manner. > > > > I completely agree with the principle, but this design guidance cannot be > enforced without specifics. I recommend s/MUST/must/. The more specific > text in this section can use the normative MUST statements. > > > > ** Section 13. > > > > Any sensitive personal data included in authorization details MUST be > > prevented from leaking, e.g., through referrer headers. > > > > Is "leaking" the same as being sent unencrypted? Recommend being clear. > > > > ** Section 13. > > > > The AS SHOULD share this data with those parties on a "need to know" > > basis. > > > > Completely agree. Consider ending this sentence with "... as determined by > local policy", or the equivalent to make it clear that it will not be > document in > this specification. > > > > Thanks, > > Roman > > > > _______________________________________________ > > OAuth mailing list > > OAuth@ietf.org > > https://www.ietf.org/mailman/listinfo/oauth _______________________________________________ OAuth mailing list OAuth@ietf.org https://www.ietf.org/mailman/listinfo/oauth
