cyrusimap / cyrus-imapd

Cyrus IMAP is an email, contacts and calendar server
http://cyrusimap.org
Other
549 stars 150 forks source link

2.5 imap/http_caldav.c:caldav_put() ensure ORGANIZER is consistent #1768

Closed dilyanpalauzov closed 7 years ago

dilyanpalauzov commented 7 years ago

If the first element of a caldav object has no ORGANIZER, make sure that no other element of the same caldav object has an ORGANIZER (RFC 6638 Section 3.2.4.2 CALDAV:same-organizer-in-all-components Precondition)

diff --git a/imap/http_caldav.c b/imap/http_caldav.c
index 5a2a62b30..302822f6e 100644
--- a/imap/http_caldav.c
+++ b/imap/http_caldav.c
@@ -2705,18 +2705,17 @@ static int caldav_put(struct transaction_t *txn,
            goto done;
        }

-       if (organizer) {
-           const char *nextorg = NULL;
+       const char *nextorg = NULL;

-           prop = icalcomponent_get_first_property(nextcomp,
-                                                   ICAL_ORGANIZER_PROPERTY);
-           if (prop) nextorg = icalproperty_get_organizer(prop);
-           if (!nextorg || strcmp(organizer, nextorg)) {
+       prop = icalcomponent_get_first_property(nextcomp,
+                                               ICAL_ORGANIZER_PROPERTY);
+       if (prop) nextorg = icalproperty_get_organizer(prop);
+       if ((!organizer && nextorg)
+           || (organizer && (!nextorg || strcmp(organizer, nextorg)))) {
                txn->error.precond = CALDAV_SAME_ORGANIZER;
                ret = HTTP_FORBIDDEN;
                goto done;
            }
-       }
     }

     switch (kind) {
elliefm commented 7 years ago

@ksmurchison - what do you think of this?

The patch is against 2.5 but it looks like it might apply to 3.0 too (with some finessing). I'm happy to do the wrangling if you're happy for it to be merged?

ksmurchison commented 7 years ago

If this is compliant with RFCs 5545, 5546, and 6638 then let's apply it to both 2.5 and 3.0

elliefm commented 7 years ago

https://tools.ietf.org/html/rfc5545#section-3.8.4.3:

Conformance: This property MUST be specified in an iCalendar object that specifies a group-scheduled calendar entity. This property MUST be specified in an iCalendar object that specifies the publication of a calendar user's busy time. This property MUST NOT be specified in an iCalendar object that specifies only a time zone definition or that defines calendar components that are not group-scheduled components, but are components only on a single user's calendar.

That's kind of ... unenlightening? Seems like this behaviour is neither explicitly verboten nor explicitly allowed

I don't see anything in the other two RFCs regarding when organizer is/isn't appropriate (but I might have missed something, I'm not familiar with these in any depth)

dilyanpalauzov commented 7 years ago

https://tools.ietf.org/html/rfc6638#section-3.2.4.2

3.2.4.2. CALDAV:same-organizer-in-all-components Precondition

Apply to: PUT, COPY, and MOVE

Use with: 403 Forbidden

Purpose: (precondition) -- All the calendar components in a scheduling object resource MUST contain the same "ORGANIZER" property value when present.

ksmurchison commented 7 years ago

Yes. That's the correct reference. The patch looks sane to me.

elliefm commented 7 years ago

Patch applied, thanks :)

I copied your author line from one of your other commits, hope that's okay