Elwyn, Thanks for this thorough review! Assume everything is done below. I have made some comments most of which are explanatory.
I will post the updates in draft 11 as soon as I include the other input from this round of review. Cheers, Phil [email protected] > On Sep 25, 2025, at 2:28 PM, Elwyn Davies via Datatracker <[email protected]> > wrote: > > Document: draft-ietf-scim-events > Title: SCIM Profile for Security Event Tokens > Reviewer: Elwyn Davies > Review result: Almost Ready > > I am the assigned Gen-ART reviewer for this draft. The General Area > Review Team (Gen-ART) reviews all IETF documents being processed > by the IESG for the IETF Chair. Please treat these comments just > like any other last call comments. > > For more information, please see the FAQ at > > <https://wiki.ietf.org/en/group/gen/GenArtFAQ>. > > Document: draft-ietf-scim-events-10 > Reviewer: Elwyn Davies > Review Date: 2025-09-25 > IETF LC End Date: 2025-09-24 > IESG Telechat date: Not scheduled for a telechat > > Summary: Almost Ready. Sorry to be a little late with this review. It took > me > rather longer than expected due to unfamiliarity with the plethora of > documents > in SCIM stable. Most of my comments are at the nit/editorial issue level but > there are a number of places where concepts appear without either truly prior > definition or pointers to such definition and there are some areas of > interaction with the other specifications in the SCIM stable that appear to > need clarifying or correcting. > > Major issues: > > Minor issues: > (This almost rises to the level of a minor issue) > s2.2, txn item: I observe that the definitions of the txn and jti claims in > Section 2.2 of RFC 8417 reverse the mandatoriness of txn and jti (txn optional > and jti mandatory in RFC 8417). If this is fully intended, I think it would > be > good to emphasise the reversal. To relieve my curiosity, is there a good > reason why this has been done assuming this *is* what was intended please? > Possibly to do with the advent of the Set-txn HTTP response header? > > Nits/editorial comments: > > Global: s/i.e. /i.e., / (1 instance in s1.3). s/e.g. /e.g., / (12 instances) > > Global: References to Sections in other RFCs are normally expressed in the > form > Section <n.m...> of [RFC xyzm] - this makes it clearer that they are pointers > to other documents as opposed to a local cross-ref plus a RFC with a missing > comma/ > > Abstract/s1: Expand SCIM on first use (it isn't in the RFC Editor well known > category) and refer to RFC 7642 which is the base specification for the SCIM > story. > > s1, 1st sentence: The SET spec specifies a format for the events. Suggest > s/as > specified by/in the format defined by/ > > s1, para 4: I think it would be helpful to introduce the concept of Event > Streams/Event Feeds here. Currently they appear in s1.3 without any previous > connection and the word 'quality' is used there which I found very confusing. > > OLD: > Security Event Tokens [RFC8417] and SCIM Events offer the ability to > exchange messages that act as triggers for receivers to monitor over > time in an asynchronous approach. This enables greater scale and > timeliness, where only changed information is exchanged between > parties. > > NEW: > Security Event Tokens [RFC8417] and SCIM Events offer the ability to > exchange messages that act as triggers for receivers to monitor changes > to resources over > time in an asynchronous approach. This enables greater scale and > timeliness, where only changed information is exchanged between > parties. The messages would be exchanged over Event Feeds (also > known as Event Streams) linking SCIM clients and SCIM service > providers. > > s1, para 5: I found this phrase slightly confusing - I initially read it as > the > event being delivered to the SCIM Client: s/delivered asynchronously to the > originating SCIM Client/delivered asynchronously relative to the event in the > originating SCIM Client/ > > s1, para 5, last word: s/domain/domains/ > > s1, para 6: s/defined/that are defined/ (it is the extra attributes that are > defined in s3, not ...ServiceProviderConfig). <PH> Moved to abstract and fixed > > s1.3: The following text appears to have duplicate discussions of the common > meaning of claims and attributes - it needs tidying up. > >> In JSON Web Tokens and Security Event Tokens, the term "claim" refers >> to JSON attribute values in a JSON Web Token [RFC7519] structure. >> The term "claim" in tokens is used to indicate that an attribute >> value may not be verified and its accuracy can be questioned. In the >> context of SCIM, claims are referred to as attributes. For the >> purposes of this specification claims and attributes are inter- >> changeable. For consistency, JWT and SET IANA registered attributes >> will continue to be called claims, while event attributes (i.e. those >> in an event payload) will be referred to as attributes. >> >> Additionally, the following terms are defined: >> >> Attributes and Claims >> The JWT specification [RFC7519] upon which SET is based uses the >> term "claims" to refer to attributes in a JSON token. SCIM in >> contrast uses the term "attributes" to refer to JSON attributes. >> For the purposes of this draft, the terms "attributes" and >> "claims" are equivalent. > <PH> I am not sure I succeeded but I put in some slight modifications. > s1.3, CP discussion: I think this should mention 'notice mode' with a pointer > to Section 2.4.1 where this mode is defined. > > s1.3, DBR discussion: Add a pointer to Section 2.4.1 where 'full mode' is > defined. > > s1.3, Event Feed discussion: s/aka/equivalently/. I suggest the following > alternative first sentence (I know what you mean by 'quality' but it takes > some > thinking about!): An Event Feed (equivalently Event Stream) is a logical > connection between an Event Publisher and a client implementing the > possibility > that notification of resource state changes MAY be managed per receiving > client. <PH> I modified this from a previous feedback item. Hopefully the new one fits > > s1.3, SCIM Client discussion: Possibly add 'via one or more Event Feeds'? <PH> Actually the SCIM client is often what causes a SCIM Event. I added some clarification. I also modified SCIM Service Provider > > s2.1, uri item: A reference to Section 3.2 of RFC 7644 for the definition of > resource type endpoint would help. > > s2.1, para 1, last sentence: Is this the 'Subject Identification' constraint > in > s3? Might be worth saying as there are several sub discussions in RFC 8417. > > s2.1, externalId and id items: Presumably these attributes share definitions > with the same items in RFC 7463 Section 3.1. If so a reference would help. > > s2,1, "emails, username..." item: Are the allowed values here constrained to > those defined in Section 4.1 of RFC 7463? Whether or not this is the case > should be stated here. > > s2.2, data item: s/SCIM Bulk/the SCIM Bulk/ > > s2.2, txn item: s/is a SET defined claim/txn is a SET defined claim/ It may > be > worth mentioning that it has a STRING value. > > s2.2, txn item: I observe that the definitions of the txn and jti claims in > Section 2.2 of RFC 8417 reverse the mandatoriness of txn and jti (txn optional > and jti mandatory in RFC 8417). If this is fully intended, I think it would > be > good to emphasise the reversal. To relieve my curiosity, is there a good > reason why this has been done assuming this *is* what was intended please? > Possibly to do with the advent of the Set-txn HTTP response header? <PH> See proposal above. I attempted to give a better explanation as to why. My real thinking is the cost of generating a unique id seem fairly low and getting into complex logic about when txn could be omitted would likely do more harm to interoperability. RFC8417 had to recognize that some events aren’t necessarily triggered by a state transfer change but may in fact be an observational event. In our discussions for 8417, we did anticipate a lot of event definitions would require this value. > > s2.2, 'attributes' item: I don't understand why the second sentence (starting > "Values of...") is relevant (or correct here). The item appears to be an array > of claim names. To clarify this s/array of attributes/array of names of > attributes/ in any case. If the second sentence is in fact appropriate, then I > think some explanation is needed and also s/path>/path/ - otherwise it should > be deleted as the list of attribute names does not have any interest in the > format of the individual claim values. <PH> Good catch. I switched s/Values/Names/ > > s2.4.1, 3rd sentence: "final state" - Surely the set of values represents the > 'current' set of values at the time the event occurred/was reported? Suggest > s/final state/current state/ <PH> Current causes issue as in current in the perspective of the client or the server. This is meant to cover scenarios where the attribute is generated or normalized by the SCIM server. To help I modified as follows: For each of the following events when the data payload attribute is included, the event URI SHALL end with full, otherwise the event URI ends with notice. In full mode, the set of values reflecting the final representation of the resource (such as would be returned in a SCIM protocol response) at the Service Provider are provided using the data attribute (see Figure 4). In notice mode, the attributes attribute is returned listing the set of attributes created or modified in the request (see Figure 5). > > s2.4.1, 4th sentence: add '(see Figure 4)' for consistency with notice mode. <PH> see previous > > s2.4.1, Figure 4 title: s/Example SCIM Create (Full)/Example SCIM Create Event > (Full)/ (for consistency with Figure 5.) > > s2.4.1, last para: s/The event above/The example event shown in Figure 5/ > (Both > Figure 4 and Figure 5 are above). > > s2.4.3 and s2.4.4: It may be a bit nit picking but it would be good to > indicate > explicitly which figures are referred to in the introductory text of these two > sections as in Section 2.4.2. > > s2.4.6, "minimal disclosure requirements": Firstly Section 2.4.5 says nothing > explicitly about disclosure events. Secondly, what is meant by a disclosure > event? There is no definition of this term. <PH> I have removed. It is certainly true that some events (full) reveal more data than others (notice) which is discussed elsewhere. > > s2.5.1.1, para 2 and also in s3, None : For consistency with the title of > Section 2.5.1 s/async SCIM request/asynchronous SCIM request/ <PH> I performed a global s/Asynch /Asynchronous / > > s2.5.1.1, respond-async: This is defined in RFC 7240 (along with the Prefer > header) and not RFC 9110. > > s2.5.1.1, respond-async: RFC 7240 provides the 'wait' preference allowing the > server to decide whether to respond synchronously or asynchronously depending > on expected response delay. Should this be honoured or MUST the server > ignore the wait preference in SCIM cases as it is allowed to do? <PH> Good suggestion. I added support for wait to the text. I think this is ok since 7240 can choose whether to respond synchronously or asynchronously anyway. > > s2.5.1.1, Servicing of asynchronous requests by proxies: Does anything need > to be said about this for the SCIM case? <PH> I think it is ok. The language in 7240 works. Do you think we should remind the reader here? > > s2.5.1.1, Set-txn header (para after Figure 12): The specification needs an > explicit definition of the Set-txn HTTP header. It appears here without any > definition. This definition needs to be referenced in Section 6.1. <PH> I will create a new section and adjust the async request and iana sectinos > > s2.5.1.3, 2nd sentence: s/is the same a single/is the same as a single/ > > Appendix C: Should contain an RFC Editor note requesting the deletion of > Appendix C before publication. > > Appendix D: Acknowledgements and authors addresses are normally sections in > the > body of the document. > > >
_______________________________________________ Gen-art mailing list -- [email protected] To unsubscribe send an email to [email protected]
