[OAUTH-WG] Secdir last call review of draft-ietf-oauth-rar-15

[Carl Wallace via Datatracker]

Reviewer: Carl Wallace
Review result: Has Nits

Comments/questions
- Section 2.2 states "When different common data fields are used in
combination, the permissions the client requests are the cartesian product of
the values." What would that mean for the example in Figure 7 where geolocation
is asserted? Are data fields other than the common types not processed this
way? - The MUST in the first sentence of the third paragraph in Section 3.1
should probably be a SHOULD. The paragraph before recommends against using both
scope and authorization_details. The rest of the paragraph leaves open how to
handle the combination (presumably including ignoring one or the other). - It
is unclear to me how the "MUST consider both sets of requirements in
combination with each other" language in Section 3.2 squares with the "resource
parameter does not have any impact on the way the AS processes the
authorization_details" language in Section 3.3 for  a request that includes
scope, resource and authorization_details w/locations. If scope and resource
were used before, it seems odd to only consider scope during a migration
period. - In section 7, the first paragraph has a MUST regarding sending "the
authorization details as granted by the resource owner". The third paragraph
leaves room for discretion. Should the MUST be a SHOULD?

Nits
- Expand AS and RS on first use
- The security considerations should echo the requirement that the AS ensure
that there is no collision between different authorization details types that
it supports. - In section 6, the bulleted list of privileges is incomplete. The
example in Section 3 also allowed for checking the status of and canceling a
payment. - Section 11.1 should probably include a bullet regarding client use
of the authorization_details_types metadata. - In the second paragraph of
Section 12, "all strings" should probably be "all strings in an
authorization_details parameter".



___
OAuth mailing list
OAuth@ietf.org
https://www.ietf.org/mailman/listinfo/oauth

Re: [OAUTH-WG] Secdir last call review of draft-ietf-oauth-rar-15

[Torsten Lodderstedt]

Hi Carl,

thanks for your review comments.

> Am 16.11.2022 um 12:25 schrieb Carl Wallace via Datatracker 
> :
> 
> Reviewer: Carl Wallace
> Review result: Has Nits
> 
> Comments/questions
> - Section 2.2 states "When different common data fields are used in
> combination, the permissions the client requests are the cartesian product of
> the values." What would that mean for the example in Figure 7 where 
> geolocation
> is asserted?

The example in figure 7 asks for read and write access to metadata and images 
managed by a photo api offered at two different endpoints if the geo locations 
in the respective pictures are at one of the coordinates given in the 
geolocations array.

> Are data fields other than the common types not processed this
> way?

Yes, they are. 

> - The MUST in the first sentence of the third paragraph in Section 3.1
> should probably be a SHOULD.

From an interoperability standpoint, it is important the client can expect the 
AS to consider both sets of requirements.

> The paragraph before recommends against using both
> scope and authorization_details.

for the same API. However, if the same client asks for access to multiple APIs 
in the same authorization request and different APIs use one or the other 
method, that is nevertheless be supported by the spec. That’s also important to 
allow pre-existing APIs and new APIs to co-exist. 

For example, OpenID Connect uses scopes but newer APIs for Open Banking might 
use RAR. Asking for identity claims and the permission to execute a payment in 
the same transaction might make a lot of sense. 

> The rest of the paragraph leaves open how to
> handle the combination (presumably including ignoring one or the other).

The normative requirement is to support both in the same authorization request. 

> - It
> is unclear to me how the "MUST consider both sets of requirements in
> combination with each other" language in Section 3.2 squares with the 
> "resource
> parameter does not have any impact on the way the AS processes the
> authorization_details" language in Section 3.3 for  a request that includes
> scope, resource and authorization_details w/locations. If scope and resource
> were used before, it seems odd to only consider scope during a migration
> period.

good point. What we want to get across is: Whatever was valid before, is not 
rendered invalid by RAR. scope and resource can be combined as defined in RFC 
8707 - that's the first sentence. What 3.2 tries to convey is that while the 
resource has an impact on the interpretation of the scope, it does not have any 
impact on the authorization details - that’s the second sentence. 

> - In section 7, the first paragraph has a MUST regarding sending "the
> authorization details as granted by the resource owner". The third paragraph
> leaves room for discretion. Should the MUST be a SHOULD?

That makes sense. Changed the wording to MUST. 

> 
> Nits
> - Expand AS and RS on first use

done

> - The security considerations should echo the requirement that the AS ensure
> that there is no collision between different authorization details types that
> it supports.

added sentence to the second paragraph in the security considerations section

> - In section 6, the bulleted list of privileges is incomplete. The
> example in Section 3 also allowed for checking the status of and canceling a
> payment.

added

> - Section 11.1 should probably include a bullet regarding client use
> of the authorization_details_types metadata.

Added “(if needed) Entitle clients to use certain authorization details types” 

> - In the second paragraph of
> Section 12, "all strings" should probably be "all strings in an
> authorization_details parameter".

done 

I published a new revision with all changes 
https://www.ietf.org/archive/id/draft-ietf-oauth-rar-16.html 


best regards,
Torsten. 


> 
> 
> 

___
OAuth mailing list
OAuth@ietf.org
https://www.ietf.org/mailman/listinfo/oauth