Re: [Last-Call] Secdir last call review of draft-ietf-oauth-rar-15

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Carl,

thanks for your review comments.

Am 16.11.2022 um 12:25 schrieb Carl Wallace via Datatracker <noreply@xxxxxxxx>:

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. 






-- 
last-call mailing list
last-call@xxxxxxxx
https://www.ietf.org/mailman/listinfo/last-call

[Index of Archives]     [IETF Annoucements]     [IETF]     [IP Storage]     [Yosemite News]     [Linux SCTP]     [Linux Newbies]     [Mhonarc]     [Fedora Users]

  Powered by Linux