Hi Travis, Thank you very much for your review! We have addressed it in the latest submitted version -01
https://tools.ietf.org/html/draft-tiloca-ace-revoked-token-notification-01 Please, see some detailed replies inline below. Best, /Marco On 2019-11-21 08:43, Travis Spencer wrote: > Hi All, > > I wanted to submit a review of "Notification of Revoked Access Tokens > in the Authentication and Authorization for Constrained Environments > (ACE) Framework" (draft-tiloca-ace-revoked-token-notification-00). > > I am an ACE noob, but hopefully my feedback will be constructive and > helpful. If they are not, it could be because of my ignorance of ACE. > That said, I am an expert in OAuth, so that is my frame of reference. > > * The draft mixes casing and has inconsistent use of acronyms like RS, > resource server, authorization server, AS, etc. This hangs me up as a > reader. I'd appreciate if one was used for all or if none were used. ==>MT We have revised the whole text accordingly, opting for expanded identifiers instead of acronyms. <== > > * I recommend an ASCII art be added to the protocol overview. I know > that there are some good diagrams in the examples, but an overview > figure in that section would be helpful as a reader as comes to terms > with what the doc specifies and decide if it's relevant. It would also > be helpful if this section ended with a link/reference to the example > sections below (e.g., something like “see examples in section X for > more detailed flows”). ==>MT We have added the new Figure 1, in Section 2. <== > > * In my strong opinion, It should not be assumed that the TRL endpoint > is at the root of the URL path of the AS. > https://example.com/tenant1/trl should be valid; also, > https://example.com/my-good-trl should be a valid URI. This should be > included in the OAuth metadata (if the AS supports this) or > communicated to the client/RS by some undefined method (e.g., > documentation). The spec may recommend some URI and the non-normative > examples can use whatever URI, but these must not be MTI. ==>MT Now the TRL endpoint is at /revoke/trl <== > > * The doc should not make any distinction between an OAuth client and > resource server. This makes it very confusing IMHO. Instead, it should > consider the caller/client of the TRL endpoint, regardless of what > role that caller plays in an OAuth flow. This is similar to how > introspection and other OAuth-related APIs are defined. ==>MT We have put the focus on tokens pertaining to a caller of the TRL endpoint. <== > > * The TRL endpoint need not be per client/RS. The endpoint only > exposes random hashes. Disclosing these to other parties will not pose > a security issue (that I can think of). This will allow clients/RSes > to collaborate in unforeseen ways that could be useful in certain > deployments. This also means that the TRL endpoint will be global and > not have a client-specific path segment in the path of the URI. ==>MT Based also on discussions with Jim, we have now a single TRL endpoint for all the authorized callers. The AS recognizes the caller based on the security identity used in their secure communication. That way, no query parameters are needed to identify the exact caller. <== > > * The TRL endpoint should accept some method for filter based, for > example, on client ID. This could be done using a query string > argument like "client_id" which can be supplied 0..INF times. This > will allow a monitoring client (like a NOC) to subscribed to > revocation messages for multiple clients/RSes. If the "client_id" (or > whatever) parameter is supplied, the caller of the TRL endpoint should > be notified of ALL revoked tokens. ==>MT As above, the AS does identify the endpoint caller, but without using query parameters. However, query parameters are now used by a caller to distinguish between full and diff queries (see Section 5). <== > > * If the idea above is accepted and the AS requires authentication of > the TRL endpoint client, then the AS may automatically subscribe the > client to notifications for just itself (which the AS knows the > identity of the client due to authentication). This implicitly > filtering may be beyond the scope of the doc, however, and I think > that authentication in general should be dropped. If it remains > though, this identification and implicit filter can be done > irrespective of authentication scheme used. ==>MT The CoAP client has in fact to register for an observation to the CoAP server. So it should not be possible for the AS to register observers by itself. <== > > * I think that the token name term should be changed to token hash, so > that it is more intuitively obvious what it is in case the term was > skimmed over or missed. ==>MT Done. <== > > * Because the draft is only about non-revoked tokens, the RS/client > still has to manage expired token checking. If the list included > expired tokens as well, the RS/client could off-load all of this to > the AS. This would make the development of the RS/client a lot simpler > and push that complexity to the AS. This is keeping in the spirit and > aim of OAuth. I've understood from Ludwig and Marco that this does > introduce some issues in a CoAP environment, that I know nothing > about. So, please take it as a suggestion and see what's best. ==>MT The problem is that, by doing so, the TRL resource would indefinitely grow in size, as well as every portion of it pertaining to each individual C/RS. Handling of expired Access Tokens should still be handled as defined and recommended in the ACE framework document. <== > > * Clarify that the hashes in the CBOR array are unordered. CBOR has no > set, I've been told, only arrays. Arrays are ordered though, so it > should be stated that this ordering is irrelevant, and the subscriber > should treat them as a set where the order has no significant meaning. ==>MT Done, both for the representation of the TRL resource and for the payload of responses to endpoint callers. <== > > HTH! ==>MT It really does, thanks a lot again! <== > > -- > > Travis Spencer > > _______________________________________________ > Ace mailing list > [email protected] > https://www.ietf.org/mailman/listinfo/ace -- Marco Tiloca Ph.D., Senior Researcher RISE Research Institutes of Sweden Division ICT Isafjordsgatan 22 / Kistagången 16 SE-164 40 Kista (Sweden) Phone: +46 (0)70 60 46 501 https://www.ri.se
signature.asc
Description: OpenPGP digital signature
_______________________________________________ Ace mailing list [email protected] https://www.ietf.org/mailman/listinfo/ace
