Re: [OAUTH-WG] AD review of draft-ietf-oauth-rar-12

[Roman Danyliw <[e-mail]>]

Hi Justin!

https://github.com/oauthstuff/draft-oauth-rar/pull/88 looks good to me.  Thanks.
Roman

From: Justin Richer <[e-mail]>
Sent: Monday, October 17, 2022 4:35 PM
To: Roman Danyliw <[e-mail]>
Cc: [e-mail]
Subject: Re: [OAUTH-WG] AD review of draft-ietf-oauth-rar-12

Thank you, that’s a much better way to say that. I think more of that paragraph can actually be pulled back, so I’m proposing this as the opening paragraph to that section:

This specification defines a set of common data fields that are designed to be usable across different types of APIs. This specification does not require the use of any of these common fields by an API definition, but instead provides them as reusable generic components for API designers to make use of. The allowable values of all fields are determined by the API being protected, as defined by a particular "type" value.

This removes the problematic normative language but, I believe, still gets the intent across. I’ve put in a PR that makes this change here:

https://github.com/oauthstuff/draft-oauth-rar/pull/88
Please let us know if this works for you. Thanks for your helpful input!

 — Justin


On Oct 14, 2022, at 12:50 PM, Roman Danyliw <[e-mail]<mailto:[e-mail]>> wrote:

Hi Justin!


-----Original Message-----
From: Justin Richer <[e-mail]<mailto:[e-mail]>>
Sent: Thursday, September 15, 2022 11:20 AM
To: Roman Danyliw <[e-mail]<mailto:[e-mail]>>
Cc: [e-mail]<mailto:[e-mail]>
Subject: Re: [OAUTH-WG] AD review of draft-ietf-oauth-rar-12

Hi Roman, some responses inline.


On Sep 14, 2022, at 6:30 PM, Roman Danyliw <[e-mail]<mailto:[e-mail]>> wrote:


[snip]


** 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?

Yes — a particular “type” value can require any of these fields, but a type is
itself not *required* to use any of these fields. That’s what we were trying to
convey with the OPTIONAL here. Is there a better way to communicate this?
Maybe we should just leave off the “OPTIONAL” flag here, but we wanted
people to realize that the common elements don’t have to be used by each
“type”, while every request needs to have a “type”.

We're on the same page.  It's up to the API/particular type field to decide which fields to use and this specification doesn't require any of them.  They are presented here for the possibility of reuse in common use cases.  I would recommend:

OLD
   All data fields are OPTIONAL for use by a given API
  definition.  The allowable values of all fields are determined by the
  API being protected.

NEW
The allowable values of all fields are determined by the API (as defined by a particular "type" value) being protected.  This specification does not require the use of any of these common fields.


[snip]


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



It might be possible to do such a comparison in a specific case, but we can’t add
logic in the general case. In OAuth, scopes are supposed to be purely additive,
so if you have another scope it’s for “more” things. We know in practice that
that’s not always how it works. Things get much more complex with RAR
because you could have an object with :more: fields in it that makes things
more :strict: by the presence of those fields. That’s all going to be up to the
“type” definition though, so if you understand the “type” definition you could
do a comparison based on that. To me the text is clear, can you suggest how
we could clarify this?

OLD 1
Since the nature of an authorization details request
  is based solely on the API or APIs that it is describing, there is
  not a simple means of comparing any two arbitrary authorization
  details requests.

NEW 1
Since the semantics of the fields in the authorization details result will be implementation specific to a given API or set of APIs, there is a no standardized mechanism to compare two arbitrary authorization detail requests.

OLD 2
  However, when comparing a new request to an existing request,

NEW 2

When comparing a new request to an existing request, ...

[snip]


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

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?

That softer language of "use what is presented" makes sense to me.

[snip]


** 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"?

I’m not positive, but I think the guidance here is meant for “basic” to mean
more like “make sure it’s a JSON object and that it has a type field” as opposed
to “check the type field’s value and run it against a JSON Schema definition”.

I don't think this is worth unpacking and would just recommend:

OLD
*  Accept authorization_details parameter in authorization requests
     including basic syntax check for compliance with this
     specification

NEW
*  Accept authorization_details parameter in authorization requests
     Conformant with this this specification



** 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?

With respect to having certain types hard-coded (like someone like Facebook or
GitHub might do because their API is specific) or having some kind of
mechanism that just prints out the RAR objects verbatim.

Ah, you mean relative to customization.  Maybe s/One option would be/One option to support customization/

Regards,
Roman