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 < [email protected]> 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 > [email protected] > https://www.ietf.org/mailman/listinfo/last-call >
_______________________________________________ Ace mailing list [email protected] https://www.ietf.org/mailman/listinfo/ace
