Subject: [GNAP] review of draft-ietf-gnap-resource-servers-05

From: Andrii Deinega <[email-masked]>
Date: Tue, 30 Apr 2024 16:30:49 -0700

Hello GNAP WG,

I've reviewed this document and came up with these comments below.

1. in the abstract section my preference would be to have

 "GNAP defines a mechanism for delegating authorization to a piece of
software, and conveying the results and artifacts of that delegation
to the software."

just like it's defined
https://www.ietf.org/archive/id/draft-ietf-gnap-core-protocol-20.html,
right now it says

"GNAP defines a mechanism for delegating authorization to a piece of
software, and conveying that delegation to the software."

2. sentence "Additionally, this document defines a general-purpose
model for access tokens, which can be used in structured, formatted
access tokens or in the API."

could be rephrased & simplified. What was the intent behind the "API"?
Was it "an API-like mechanism like token introspection"?

3. "implementors" should be probably changed to "implementers". The
same applies to the use of "implementor" in this as well as in the
core spec.

4. sentence "This is not intended to be a universal or comprehensive
list, but instead to provide guidance to implementors when developing
data structures and associated systems across a GNAP deployment.""

could be simplified to something like

"This list is not universal nor comprehensive but rather serves as
guidance for implementers in developing data structures and associated
systems across a GNAP deployment."

5. sentence "In the case of an asymmetric algorithm, the model for the
AS and RS need only contain the public key, while the client instance
will also need to know the private key in order to present the token
appropriately."

could be simplified to

"In the case of an asymmetric algorithm, the model for the AS and RS
needs only to contain the public key, while the client instance will
also need to know the private key in order to present the token
appropriately."

6. sentence "The source of this key information can vary depending on
circumstance and deployment" should be changed to "The source of this
key information can vary depending on the circumstances and
deployment."

7. section 7.3 should be called as "Caching Token Validation Result",
not "Cacheing Token Validation Result".

8. I suggest changing "the access_token request" and "the access_token
response" to "the access token request" and "the access token
response" accordingly in all places.

9. Some examples of HTTP requests and responses do not include the
Host HTTP header, as an example, the second example in section 3.3
(there are other similar places too).

10. "," in "is appropriate for the access indicated (if present),"
should be removed or changed to "." (section 3.3)

11. The normative section refers to
https://www.rfc-editor.org/rfc/rfc7519 for JWTs. However, it can also
refer to https://www.rfc-editor.org/rfc/rfc9068 from Vittorio, and
that, in my eyes, could be a little better fit. Mainly, because of the
"typ" headers set explicitly to "at+jwt".

12. sentence "Expressed as a integer seconds from UNIX Epoch." should
be probably changed to "Expressed as an integer number of seconds from
the UNIX Epoch." in all places.

All the best,
Andrii