sabre-io / dav

sabre/dav is a CalDAV, CardDAV and WebDAV framework for PHP
http://sabre.io
BSD 3-Clause "New" or "Revised" License
1.53k stars 346 forks source link

FreeBusy-Report fails with Status 3.7;No calendar-home-set property found #1185

Open pk1234 opened 5 years ago

pk1234 commented 5 years ago

This happens with Sabre 3.2.2. When inviting people with thunderbird, FreeBusy-information is displayed only for some attendees.

In order to produce a FreeBusy-report principalSearch() from DAVACL/Plugin.php is called to find the principal by email-address and get some properties.

    function principalSearch(array $searchProperties, array $requestedProperties, $collectionUri = null, $test = 'allof') {
        if (!is_null($collectionUri)) {
            $uris = [$collectionUri];
        } else {
            $uris = $this->principalCollectionSet;
        }
        $lookupResults = [];
        foreach ($uris as $uri) {
            $principalCollection = $this->server->tree->getNodeForPath($uri);
            if (!$principalCollection instanceof IPrincipalCollection) {
                // Not a principal collection, we're simply going to ignore
                // this.
                continue;
            }
            $results = $principalCollection->searchPrincipals($searchProperties, $test);
            foreach ($results as $result) {
                $lookupResults[] = rtrim($uri, '/') . '/' . $result;
            }
        }
        $matches = [];
        foreach ($lookupResults as $lookupResult) {
            list($matches[]) = $this->server->getPropertiesForPath($lookupResult, $requestedProperties, 0);
        }

$result will contain one principal in all cases, but getPropertiesForPath() does not find the calendar-home-set property for some principals.

Any ideas why this is happening for some principals only?

The calendar-home-set property of principals/xxx should be $baseUri/calendars/xxx. It's not stored anywhere but will be calculated for every principal. So why should this calculation fail for some principals? So what am I missing here?

Peter

evert commented 5 years ago

The first thing that comes to mind is maybe it's a permissions issue? Are you able to (manually, with a browser) log in to sabre/dav as the intended user and find the target users' calendar-home-set property?

pk1234 commented 5 years ago

Hi Evert,

you are right: principals/aaa does not have read-permission on $baseUri/principals/xxx and therefore cannot read the calendar-home-set property of principals/xxx.

I changed function getFreeBusyForEmail() in CalDAV/Schedule/Plugin.php

        #if (!isset($result[0][200][$caldavNS . 'calendar-home-set'])) {
        #    return [
        #        'request-status' => '3.7;No calendar-home-set property found',
        #        'href'           => 'mailto:' . $email,
        #    ];
        #}
        #if (!isset($result[0][200][$caldavNS . 'schedule-inbox-URL'])) {
        #    return [
        #        'request-status' => '3.7;No schedule-inbox-URL property found',
        #        'href'           => 'mailto:' . $email,
        #    ];
        #}
        #$homeSet = $result[0][200][$caldavNS . 'calendar-home-set']->getHref();
        #$inboxUrl = $result[0][200][$caldavNS . 'schedule-inbox-URL']->getHref();
        $homeSet=sprintf("calendars/%s", substr($result[0]['href'],11));
        $inboxUrl=$homeSet."inbox/";

and with this dirty fix every principal can read the FreeBusy report of every other principal.

This leads me to the following questions:

Kind regards

Peter

pk1234 commented 4 years ago

I applied the above fix to out productive SabreDAV 3.2.2 installation. Now I'm moving to 4.0.3. and I have to reapply my fixes. Maybe that's a good moment to fix this permenently.

Any ideas what I should do?

Peter

m-a-v commented 4 years ago

and with this dirty fix every principal can read the FreeBusy report of every other principal.

This leads me to the following questions:

  • What permission does principals/aaa need to read the FreeBusy-report of principals/xxx?
  • How do I grant read permissions on principals/xxx to principals/aaa?
  • http://sabre.io/dav/acl/ says: Most ACL rules are 'hardcoded', but these can be overridden by extending correct node classes. How can I extend the node class for principals or change the hardcoded permissions for principals?

@pk1234 I think without code modification it's only possible to read the freebusy report if principal aaa has full read permissions for principals/aaa (using calendar-proxy-read). However, this results in principal aaa being able to read all the calendar entries of principal xxx, which may normally be undesirable. A more granular assignment of rights, as implemented in DAViCal for example, would be desirable.

evert commented 4 years ago

Messages like this really make it enticing to do a full rewrite in node.js/typescript one day.

Anyway, all of this is correct. We opted to make the system secure by default and use calendar-proxy-read as a means to control access to this kinda thing.

At fruux, we used these existing APIs and further customized ACL. Specifically we also enabled (some) read permissions when:

  1. You share at least 1 calendar
  2. You are part of the same 'team'.

To control this sort of thing in your own installation, you need to subclass the user principal. This is much less than a hack than the proposed solution, as this is an officially supported API.

And, it's not that hard. The two classes you need to extend are in this directory:

https://github.com/sabre-io/dav/tree/master/lib/CalDAV/Principal

Specifically, Collection and User.

The relevant function is User::getAcl() and the reason you need to subclass Collection, is so you can ensure that it's instantiating your User class.

So that's 2 classes, and 2 functions.

m-a-v commented 4 years ago

You write some read permissions. I don't quite understand how you would allow reading free busy operations with an own User::getAcl() method only.

evert commented 4 years ago

It's possible that I missed something, but if the issue was being just blocked at reading calendar-home-set, then that should solve it. Maybe there's another required permission?

m-a-v commented 4 years ago

@evert I get the point in extending the classes Collection and User, but I think it would be nicer if the sabre/dav core classes would handle this. In the scheduleLocalDelivery method we have almost the same situation. The solution chosen there is to temporarily switch off ACL. Shouldn't getFreeBusyForEmail handle it the same way? Is there a reason not to? This would at least be consistent with the current code base.

evert commented 4 years ago

It's an issue with privacy. The default system doesn't assume that everyone is allowed to read everyone's information. I agree it would be nice to allow people to control this, perhaps on a per-user basis. But sabre/dav is in the first place a framework with somewhat sensible defaults and examples.

m-a-v commented 4 years ago

Thanks for the clarification. I actually assumed that this could be uncritical, since only the properties calendar-home-set and schedule-inbox-URL would be queried without ACL. Afterwards the ACL is used to check if the permission rights for schedule-query-freebusy (although in the current implementation there is no return statement if this call fails).