Re: [Last-Call] Genart last call review of draft-ietf-jmap-sharing-07

Susan Hares <shares@xxxxxxxx> · Fri, 12 Apr 2024 15:09:11 +0000

Neil: 

Thank you for your prompt response.   All of the revisions are fine with me.

Two other comments may be helpful to you:

You might want to send the formatting issues to the RFC editor and to the IETF tools team.

On #14 below (copied below for quick email reading) – The requirements for list formats in IETF documents differ from Professional journals.

The RFC editor’s acceptance of various types of list formats varies from the professional journal formatting standards (e.g. APA or MLA).  Recently, per my Area’s AD requests I have
 begun to utilize the grammar checks from my professional journal reviews.  If you find a style guide for IETF lists, let me know.

Cheers, Sue 

------------------

14. Section 5.3, paragraph 1

Why: Normally, lists use a "," or a ";" to separate the clauses.
In addition, the context usually suggests "and" or an "or" in the
list.

Please consider whether you should follow this list format in this section.

From: Neil Jenkins <neilj@xxxxxxxxxxxxxxxx>

Sent: Thursday, April 11, 2024 11:13 PM

To: Susan Hares <shares@xxxxxxxx>; gen-art@xxxxxxxx

Cc: draft-ietf-jmap-sharing.all@xxxxxxxx; IETF JMAP Mailing List <jmap@xxxxxxxx>; last-call@xxxxxxxx

Subject: Re: Genart last call review of draft-ietf-jmap-sharing-07

Hi Susan,

Thank you for your review. I have 
published an updated draft addressing your comments, details below.

1. Section 1.5.1, paragraph 1

Why: Sentence structure makes the reference unclear

Old text:/

   Represents support for the Principal and ShareNotification data types

   and associated API methods. /

New text:/ The property "urn:ietf:params:jmap:principals" indicates

  support for the Principal and ShareNotification data types

  and associated API methods./

I have rewritten this to:

The urn:ietf:params:jmap:principals capability represents support… [etc]

------

2. Section 1.5.2, paragraph 1

Why: Text is unclear as which URI and what is supported.

Old text:/

   This URI is solely used as a key in an account’s accountCapabilities

   property; it does not appear in the JMAP Session capabilities.

   Support is implied by the urn:ietf:params:jmap:principals session

   capability.

   If present, the account (and data therein) is owned by a principal.

   Some accounts may not be owned by a principal (e.g., the account that

   contains the data for the principals themselves), in which case this

   property is omitted. /

New text:/

   The URI urn:ietf:params:jmap:principals:owner is solely used as a

   key in an account’s accountCapabilities property, so it does not

   appear in the JMAP Session Capabilities. A urn of

   "urn:ietf:params:jmap:principals session capability" indicates support for

   this capability.

   If this urn is present, the account (and data therein) is owned by a

   principal. Some accounts may not be owned by a principal (e.g., the account

   that contains the data for the principals themselves), in which case this

   property is omitted./

I have rewritten this to:

The URI urn:ietf:params:jmap:principals:owner is solely used as a key in an account’s accountCapabilities
 property. It does not appear in the JMAP Session capabilities — support is indicated by the
urn:ietf:params:jmap:principals URI being present in the session capabilities.

If urn:ietf:params:jmap:principals:owner is a key in an account’s accountCapabilities, that
 account (and data therein) is owned by a principal. Some accounts may not be owned by a principal (e.g., the account that contains the data for the principals themselves), in which case this property is omitted.

------

3. section 2.3, paragraph 3

Why: Not clear how this fits in with the first 2 paragraphs.  Below

I have given text based on the fact the changes are properties of

Principal. However, the authors should validate.

old text:/

   However, the server may reject this change, and probably will reject

   any other change, with a forbidden SetError.  Managing principals is

   likely tied to a directory service or some other vendor-specific

   solution, and may occur out-of-band, or via an additional capability

   defined elsewhere./

New text:/

   However, the server may reject these changes to the properties of Principal

   with a response of forbidden SetError. Such servers will probably reject

   any other change with a forbidden SetError.  Managing principals is

   likely tied to a directory service or some other vendor-specific

   solution.  This management may occur out-of-band or via an additional

   capability defined elsewhere./

This section has come up in all the reviews I think! I have rewritten this as follows:

Managing principals is likely tied to a directory service or some other vendor-specific solution. This management may occur out-of-band, or via an additional capability defined elsewhere. Allowing direct user modification of properties
 has security considerations, as noted in [Security considerations section]. Servers
MUST reject any change it doesn’t allow with a forbidden SetError.

Where a server does support changes via this API, it SHOULD allow an update to the "name", "description" and "timeZone" properties of the Principal with the same id as the "currentUserPrincipalId" in the Account capabilities. This
 allows the user to update their own details.

4. section 4, paragraph 4

Why issue: Unclarity of section may lead to

issues in the security processing.

Old text:/

   *  *shareWith*: Id[String[Boolean]]|null

      A map of principal id to rights to give that principal (in the

      same format as the myRights property), or null if not shared with

      anyone.  The account id for the principal id can be found in the

      capabilities of the Account this object is in (see Section 1.5.2).

      Users with appropriate permission may set this property to modify

      who the data is shared with.  The principal that owns the account

      this data is in MUST NOT be in the set of sharees; their rights

      are implicit./

New text:/

   *  *shareWith*: Id[String[Boolean]]|null

      The "sharewith" attribute provides a map of principal id to rights

      to give each principal id listed (in the same format as the

      myRights property), or null if not shared with anyone.  The

      account id for the principal id can be found in the

      capabilities of the Account this object is in (see Section 1.5.2).

      Users with appropriate permission may set this property to modify

      who the data is shared with. The principal id that owns the account

      this data is in MUST NOT be in the set of shareWith" since the owner's

      rights are implicit./

I have tried to clarify this as follows:

The value of this property is null if the data is not shared with anyone. Otherwise, it is a map where each key is the id of a principal with which this data is shared, and the value associated with that key is the rights to give that principal,
 in the same format as the myRights property. The account id for the principal id can be found in the capabilities of the Account this object is in (see

Section 1.5.2).

Users with appropriate permission may set this property to modify who the data is shared with. The principal that owns the account this data is in
MUST NOT be in the set of sharees since the owner's rights are implicit.

Nits/editorial comments:

1. Abstract

Old text:/ Future documents can reference this one when defining

   data types to support a consistent model of sharing./

New text:/  Future documents can reference this document when defining

   data types to support a consistent model of sharing./

Why: Unclear what "this one" is in the text.

Adopted.

-------

2. section 1.4, paragraph 1

reason: English Grammar's definition of semi-colon

Old text:/ Clients will often want to differentiate

   the two; for example, a company may share mailing list archives for

   all departments with all employees, but a user may only generally be

   interested in the few they belong to./

New text:/ Clients will often want to differentiate

   the two. For example, a company may share mailing list archives for

   all departments with all employees, but a user may only generally be

   interested in the few they belong to./

Adopted.

-----

3. Section 1.4, paragraph 3.

Why:  The example is not part of the normative text.

Old text:/The server MAY reject the user's attempt to subscribe to some

   resources even if they have permission to access them, e.g., a

   calendar representing a location./

Next text:/   The server MAY reject the user's attempt to subscribe to some

   resources even if they have permission to access them, (e.g., a

   calendar representing a location). /

Adopted.

-----

4. Section 1.5.1, paragraph 4

Why: formatting issues

Old text:/ *  *currentUserPrincipalId*: Id|null

      The id of the principal in this account that corresponds to the

      user fetching this object, if any./

New text:/*currentUserPrincipalId*: Id|null

      The id of the principal in this account which corresponds to the

      user fetching this object, if any./

-------

5. section 1.5.2 - formatting

Reason: Why are there two * * in ASCII Text and odd spacing in pdf?

Old text:/

  *  *accountIdForPrincipal*: Id

      The id of an account with the urn:ietf:params:jmap:principals

      capability that contains the corresponding Principal object.

   *  *principalId*: Id

      The id of the Principal that owns this account.

/

This is an issue with the plain text version of the document — the first asterisk is because this is a bullet list (there's only one item in the list, but it's consistent with how properties are defined throughout the document), and then
 the bold text used for the property name causes it to also wrap that in asterisks. I will ask the RFC editors what they recommend for formatting here.

6. Section 2, paragraph 1.

Why: Run on sentence without purpose.

Old text:/ Sharing in JMAP is generally configured by assigning

   rights to certain data within an account to other principals, for

   example a user may assign permission to read their calendar to a

   principal representing another user, or their team./

New text:/ Sharing in JMAP is generally configured by assigning

   rights to certain data within an account to other principals. For

   example, a user may assign permission to read their calendar to a

   principal representing another user or their team./

Adopted.

7. Section 2, paragraph 3

Why: Run-on Sentence and unclear context.

Old text:/In most systems the user will have access to a single Account

   containing Principal objects, but they may have access to multiple

   if, for example, aggregating data from different places./

New text:/   In most systems, the user will have access to a single Account

   containing Principal objects, but they may have access to multiple portions

   of it. For example, clients aggregating data from different places./

I have rewritten as:

In most systems, the user will have access to a single Account containing Principal objects. In some situations, for example when aggregating data from different places, there may be multiple Accounts containing Principal objects.

7. Section 2, 2.4.1, 3.2, 3.6.1, 4, and 4.1 in descriptions of objects

In ASCII text, the text has multiple * per defined value.

In PDF version, the text requires formatting.

Please check your original text and fix so ASCII and PDF are correct.

(Same as 4/5 above.)

8. section 2.2, paragraph 1

why: English errors relating to comma

Old text:/  Note, implementations backed by an external directory

   may be unable to calculate changes, in which they will always return

   a "cannotCalculateChanges" error, as described in the core JMAP

   specification./

New text:/ Note: implementations backed by an external directory

   maybe unable to calculate changes, and in this case, they will always return

   a "cannotCalculateChanges" error as described in the core JMAP

   specification./

Adopted.

9. Section 2.5

Why: Sentence clarity

Old text:/ Note, implementations backed by an external directory

   may be unable to calculate changes, in which they will always return

   a "cannotCalculateChanges" error, as described in the core JMAP

   specification./

New text:/ Note: implementations backed by an external directory

   may be unable to calculate changes.  In this case, they will always return

   a "cannotCalculateChanges" error, as described in the core JMAP

   specification./

Adopted.

10. Section 3.2, formatting of objections

What is issue: In section 3.2 your objects use a format of:

   *  *id*: String (immutable; server-set)

It is normal to use the format:

    * id: String (immutable, server-set)

Why are you using your format?

(Same as 4/5 above.)

11. section 3.2, last paragraph

Old text:/Determining the name will depend on the data type in question, for

      example it might be the "title" property of a CalendarEvent or the

      "name" of a Mailbox, and is implementation specific.  The name is

      to show to users who have had their access rights to the object

      removed, so that they know what it is they can no longer access./

New text:/ Determining the name will depend on the data type in question.

      For example, it might be the "title" property of a CalendarEvent or the

      "name" of a Mailbox. The name is to show to users who have had

      their access rights to the object removed, so that these users

      know what it is they can no longer access./

Adopted.

---

12. Section 4, IsSubscribed definition

Why: Starting the definition off with a question creates a vague

understanding for people not involved in the writing of the specification.

Suggested change:

Old text:/

  *  *isSubscribed*: Boolean

      Has the user indicated they wish to see this data?  The initial

      value for this when data is shared by another user is

      implementation dependent, although data types may give advice on

      appropriate defaults./

New text:/

  *isSubscribed: Boolean

      The value true indicates the user wishes to subscribe to see this data.

      The value false indicates the user does not wish to subscribe to see

      this data.  The initial value for this variable when data is

      shared by another user is implementation dependent, although data types

      may give advice on appropriate defaults.

     /

If you accept this change, please also change the example in section 4.1.

Adopted.

-----

13. Section 5.2

Why: definition of e.g. (For example) and spelling error

Old text:/Sharing data with another user allows someone to turn a transitory

   account compromise (e.g., brief access to an unlocked, logged in

   client) into a persistant compromise (by setting up sharing with a

   user controlled by the attacker)./

New text:/ Sharing data with another user allows someone to turn a transitory

   account compromise (e.g., brief access to an unlocked or logged-in

   client) into a persistent compromise (by setting up sharing with a

   user-controlled by the attacker). /

Adopted.

14. Section 5.3, paragraph 1

Why: Normally, lists use a "," or a ";" to separate the clauses.

In addition, the context usually suggests "and" or an "or" in the

list.

Please consider whether you should follow this list format in this section.

I think the current format is fine, and don't believe this change would increase clarity here.

Cheers,

Neil.

-- 
last-call mailing list
last-call@xxxxxxxx
https://www.ietf.org/mailman/listinfo/last-call