Hi Marco, thank you for this very actionable review! Changes are in https://github.com/cabo/ace-aif/pull/7 (and in https://github.com/cabo/ace-aif/pull/5, see below). I plan to wait for comments on these PRs and then to submit an updated I-D. On 2022-02-22, at 19:56, Marco Tiloca via Datatracker <noreply@xxxxxxxx> wrote: > > Reviewer: Marco Tiloca > Review result: Ready with Nits > > Thanks for this good document! Please, find below a few minor comments. > > Best, > /Marco > > [Section 2.2] > > * The first paragraph has the only occurrence of "AIF object". I suppose that's > what you call "list of pairs" in Section 2, right before Figure 1. > > If so, it may help to define "AIF object" there in Section 2, or to rather > replace it with something like "list of (Toid, Tperm) pairs" here in Section > 2.2. Indeed, the term “AIF Object” may be confusing to people used to the JSON terminology, where “Object” refers to a map. Replaced it with “AIF data item”, as already used in two other places. > * In the second paragraph, an access is possibly subject to conditions, not > dictating them. Thus, shouldn't the right word be "conditionalized" rather than > "conditionalizing" ? > > [Section 3] Well, it is the information model that further conditionalizes here. Simplified this to >> This simple information model also does not allow expressing >> conditionalized access based on state outside the identification of >> objects (e.g., "opening a door is allowed if that is not locked"). > > * s/e.g., 35 for Dynamic-DELETE/e.g., 35 would be the number for Dynamic-DELETE Well, 35 actually *is* the number for Dynamic-DELETE. Now: been for X, plus a Dynamic-Offset chosen as 32 (e.g., 35 is the number for Dynamic-DELETE as the number for DELETE is 3). > [Sections 4, 5.1 and 5.2] > > * In Section 4 and later on when registering the two media-types in Section > 5.1, "local-uri" is used as default value for the Toid parameter. > > However, the new sub-registry defined in Section 5.2 (and where Toid values > have to be taken from) is populated with an entry with value "local-part". > Shouldn't they all indicate the same value? Yes, Klaus Hartke noticed this, too, PR #5. > Also, perhaps it is better to name it "URI-local-part". Good idea, added to PR #5. > > [Section 5.2] > > * Suggested rephrasing: > > For both media-types application/aif+cbor and application/aif+json, IANA is > requested to create a sub-registry within [IANA.media-type-sub-parameters] > for the two media-type parameters Toid and Tperm, populated with: s/both/the/, otherwise changed as proposed > > [Section 6] > > * The first bullet point has the only occurrence of "AIF information". Perhaps > this is another name for what you called "AIF object" in Section 2.2? (see > previous comment for similar considerations) Went to “AIF data item” again. > * The second bullet point says: "and that all parties understand Toid/Tperm > pairs to signify the same operations." > > Suggested rephrasing: "and that all parties have the same understanding of > each Toid/Tperm pair in terms of specified resources and operations on those" Nice! Changed. I changed “resources” to “objects (resources)”, because the objects of the access control matrix don’t actually need to be resources. > [Nits] > > * Section 1.1 > --- s/This memo uses terms from [RFC7252]/This specification uses terms from > CoAP [RFC7252] Good idea. I also expanded the Internet Security Glossary. > > * Section 2.2 > --- s/, however,/. However, > > * Section 6 > --- s/and provides (2) any/and (2) provides any > Thank you! Grüße, Carsten -- last-call mailing list last-call@xxxxxxxx https://www.ietf.org/mailman/listinfo/last-call
