[Jmap] Orie Steele's Discuss on draft-ietf-jmap-sharing-07: (with DISCUSS and COMMENT)

[Orie Steele via Datatracker <[e-mail]>]

Orie Steele has entered the following ballot position for
draft-ietf-jmap-sharing-07: Discuss

When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)


Please refer to https://www.ietf.org/about/groups/iesg/statements/handling-ballot-positions/ 
for more information about how to handle DISCUSS and COMMENT positions.


The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-jmap-sharing/



----------------------------------------------------------------------
DISCUSS:
----------------------------------------------------------------------

# Orie Steele, ART AD, comments for draft-ietf-jmap-sharing-07
CC @OR13

https://author-tools.ietf.org/api/idnits?url=https://www.ietf.org/archive/id/draft-ietf-jmap-sharing-07.txt&submitcheck=True

Thanks to the authors for this document, and to Arnt Gulbrandsen for the
shepherd write up.

## Discuss

```
243        *  *id*: Id (immutable; server-set)
244           The id of the principal.
245        *  *type*: String
246           This MUST be one of the following values:
247           -  individual: This represents a single person.
248           -  group: This represents a group of people.
249           -  resource: This represents some resource, e.g., a projector.
250           -  location: This represents a location.
251           -  other: This represents some other undefined principal.
252        *  *name*: String
```

The repeated '*' make this section difficult to read, I initially assumed this
syntax indicated the field is not optional.

After reviewing RFC8620, it seems to simply be a "bolding" / "emphasis" effect.

This section (and similar ones) contain many instances of "String" which is
defined in https://datatracker.ietf.org/doc/html/rfc8620#section-1.1

As has been discussed on the list regarding a related document:
https://mailarchive.ietf.org/arch/msg/jmap/nEumbk7DKcV6AOg6q-7yD_qetU0/

Unicode / i18n processing of these fields is possibly non trivial...

The document would be greatly improved by a section commenting on what kinds of
JSON Strings / UTF-8 strings must remain supported. Examples in an appendix
demonstrating interesting edge cases / unicode characters might help.

```
310        *  *accountIds*: String[]
311           A list of account ids.  The Principal matches if any of the ids in
312           this list are keys in the Principal's "accounts" property (i.e.,
313           if any of the account ids belong to the principal).
```

Should this not be Id[] ? Is full UTF-8 equality check expected here?

```
354        The server MAY limit the maximum number of notifications it will
355        store for a user.  When the limit is reached, any new notification
356        will cause the previously oldest notification to be automatically
357        deleted.
```

Why not SHOULD or MUST? What happens if there is no limit?

```
359        The server MAY coalesce notifications if appropriate, or remove
360        notifications that it deems are no longer relevant or after a certain
361        period of time.  The server SHOULD automatically destroy a
362        notification about an object if the user subscribes to that object.
```

Why not MUST? Should 362 say "unsubscribes" ?

```
439        *  *objectAccountId*: String
440           The objectAccountId value must be identical to the given value to
441           match the condition.
```

Should this be `*objectAccountId*: Id` ?


----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------


## Comments

```
173        The server MAY reject the user's attempt to subscribe to some
174        resources even if they have permission to access them, e.g., a
175        calendar representing a location.
```

Can this be strengthened to a SHOULD with clear conditions for when the server
MUST allow subscription?

```
177        A user may query the set of Principals they have access to with
178        "Principal/query" (see Section 2.4).  The Principal object may then
179        provide Account objects if the user has permission to access data for
180        that principal, even if they are not yet subscribed.
```

Can this be strengthened with normative language?
For example:
The Principle object MUST contain / provide when...
The Principle object MUST NOT contain / provide when...

```
295        However, the server may reject this change, and probably will reject
296        any other change, with a forbidden SetError.  Managing principals is
297        likely tied to a directory service or some other vendor-specific
298        solution, and may occur out-of-band, or via an additional capability
299        defined elsewhere.
```

Can this language be strengthened?

For example: When the server rejects updates it MUST use a SetError of type
forbidden as described in Section 5.3 of RFC8620.

```
314        *  *email*: String
315           Looks for the text in the email property.
```

I suggest a stronger reference for IDNA compatible email identifiers.
There are many combinations of unicode characters that will not result in a
valid email address. Consider: https://datatracker.ietf.org/doc/html/rfc6531 If
this field is not required to be a well formed email (possibly one build of an
IDNA), perhaps note that directly.

```
318        *  *text* String
319           Looks for the text in the name, email, and description properties.
```

The use of the word text is slightly misleading here without inclusion of the
charset, since strictly speaking this is filtering on octets (encoding some
UTF-8 string), correct?

Thanks to Yaron Sheffer for his comments to this effect in the SEC DIR review.

```
347        The server SHOULD create a ShareNotification whenever the user's
348        permissions change on an object.  It SHOULD NOT create a notification
349        for permission changes to a group principal, even if the user is in
350        the group.
```

Why not MUST? The way this is written, an implementation could easily decide to
reverse these recommendations.

```
614     5.  Security Considerations
```

Please consider adding security considerations regarding deceptive use of
unicode characters, perhaps drawing from previous work, for example from:
https://datatracker.ietf.org/doc/html/rfc5894#section-1.4

"""
   The introduction of the larger repertoire of characters
   potentially makes the set of misspellings larger, especially given
   that in some cases the same appearance, for example on a business
   card, might visually match several Unicode code points or several
   sequences of code points.
"""

```
639        The set of principals within a shared environment SHOULD be strictly
640        controlled.  If adding a new principal is open to the public, risks
641        include:
```

Why not MUST?

Is consent from existing principles required or recommended when adding new
principles?

## Nits

```
536        "u12345678": {
537            "name": "[e-mail]"
538            "isPersonal": true
539            "isReadOnly": false
540            "accountCapabilities": {
541                "urn:com.example:jmap:todo": {},
542                "urn:ietf:params:jmap:principals:owner": {
543                    accountIdForPrincipal: "u33084183",
544                    principalId: "P105aga511jaa"
545                }
546            }
547        }
```

This example is malformed JSON. accountIdForPrincipal and principalId are
missing quotes, and the there is no indication of elision, consider starting
with { ... "u12345678": ... }

Its also strange that "name" is an email in this example, consider an
interesting unicode example like:

"Voß" or some user chosen string that includes emojis (just make sure they are
black or white).