Re: [Last-Call] Opsdir last call review of draft-ietf-ace-revoked-token-notification-06

[Dhruv Dhody <[e-mail]>]

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 <
[e-mail]> 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
> [e-mail]
> https://www.ietf.org/mailman/listinfo/last-call
>