Hi,
I see that the list formatting got messed up somehow. Well you can find the review also at - https://notes.ietf.org/draft-ietf-ace-revoked-token-notification-06?view
Hope that helps!
Thanks!
Dhruv
On Mon, Apr 1, 2024 at 11:21 PM Dhruv Dhody via Datatracker <noreply@xxxxxxxx> wrote:
Reviewer: Dhruv Dhody
Review result: Has Issues
# OPSDIR review of draft-ietf-ace-revoked-token-notification-06
I have reviewed this document as part of the Operational directorate's ongoing
effort to review all IETF documents being processed by the IESG. These
comments were written with the intent of improving the operational aspects of
the IETF drafts. Comments that are not addressed in the last call may be
included in AD reviews during the IESG review. Document editors and WG chairs
should treat these comments just like any other last-call comments.
The document provides a mechanism for the Authorization Server to notify about
revoked access tokens via access to a Token Revocation List (TRL) on the
Authorization Server via CoAP. The document is clear and well-written. The
motivation is described well. The document is almost ready for publication.
## Operational Concern
- I am worried that the admin authorization is kept out of scope. I could not
find how it is handled in the ACE framework and if this is acceptable. At the
very least some hint or example can be provided... ````
* Administrator: entity authorized to get full access to the TRL at
the AS, and acting as a requester towards the TRL endpoint. An
administrator is not necessarily a registered device as defined
above, i.e., a Client requesting access tokens or an RS consuming
access tokens. How the administrator authorization is established
and verified is out of the scope of this specification.
````
- Section 5.1, if the TRL maintains MAX_N items only and if the diff is larger
than that wouldn't some of the diffs be lost when the requester finally makes a
request? I would have assumed that in such a case we indicate it that to the
requester and it could issue a full query instead. But MAX_N is applicable for
full query as well. Perhaps that is okay for most deployments, but let's be
explicit about that in the document.
## Minor
- Thanks for including the description of "endpoint" in Section 1.1 but the
term is used in the Abstract and Introduction where it confused me on first
reading. I suggest adding a forward reference in the Introduction to avoid
confusion. Also in section 1.1, it would be better if we have a reference to
the OAuth document where the term originates from. - Is there a suitable
reference for "CBOR diagnostic notation"? - Section 3, I suggest mentioning
that 0x58 identifies byte string with one-byte for n. - Section 5.1 "The AS
SHOULD provide requesters with the value of MAX_N, upon their registration",
how about we make it a MUST and quality it with if diff is supported? - Section
5.1.1 "If supporting the "Cursor" extension, the AS SHOULD provide registered
devices and administrators with the value of MAX_DIFF_BATCH...", why SHOULD? I
think this should be a MUST! - Section 7, "SIZE <= MAX_N" but it is unclear
what exactly SIZE is, please add explicit text. - Section 14.1, should the
change controller be IETF instead?
## Nits
- Don't use "the CoAP protocol", the P in CoAP is already protocol!
- s/as interested to receive notifications/as interested in receiving
notifications/ - s/recent updates occurred over the list/recent updates that
occurred over the list/ - s/with value the token hash of an access token/with
the value of the token hash of an access token/ - s/The processing of a full
query and the related response format are defined in/The processing of a full
query and the related response format is defined in/ - s/other than 0 or than a
positive integer/other than 0 or a positive integer/ - s/registers at the AS
until a first update/registers at the AS until the first update/ - s/pertaining
to the requester occurred to the TRL./pertaining to the requester that occurred
to the TRL./ - s/registers at the AS until a first update/registers at the AS
until the first update/ - s/Once completed the registration procedure at the
AS,/Once the registration procedure is completed at the AS,/ - s/the Client is
not aware about that yet,/the Client is not aware of that yet,/
--
last-call mailing list
last-call@xxxxxxxx
https://www.ietf.org/mailman/listinfo/last-call
-- last-call mailing list last-call@xxxxxxxx https://www.ietf.org/mailman/listinfo/last-call
