-
-
Notifications
You must be signed in to change notification settings - Fork 4.7k
Introduce own method for calendar unsharing #52046
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
af9efb7 to
0198174
Compare
| * '{urn:ietf:params:xml:ns:caldav}schedule-calendar-transp': ScheduleCalendarTransp, | ||
| * '{DAV:}displayname': string, | ||
| * '{urn:ietf:params:xml:ns:caldav}calendar-timezone': ?string, | ||
| * '{http://nextcloud.com/ns}owner-displayname': string, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
owner-displayname is only set if we have displayname:
server/apps/dav/lib/CalDAV/CalDavBackend.php
Lines 3522 to 3535 in 2ea3491
| private function addOwnerPrincipalToCalendar(array $calendarInfo): array { | |
| $ownerPrincipalKey = '{' . \OCA\DAV\DAV\Sharing\Plugin::NS_OWNCLOUD . '}owner-principal'; | |
| $displaynameKey = '{' . \OCA\DAV\DAV\Sharing\Plugin::NS_NEXTCLOUD . '}owner-displayname'; | |
| if (isset($calendarInfo[$ownerPrincipalKey])) { | |
| $uri = $calendarInfo[$ownerPrincipalKey]; | |
| } else { | |
| $uri = $calendarInfo['principaluri']; | |
| } | |
| $principalInformation = $this->principalBackend->getPrincipalByPath($uri); | |
| if (isset($principalInformation['{DAV:}displayname'])) { | |
| $calendarInfo[$displaynameKey] = $principalInformation['{DAV:}displayname']; | |
| } | |
| return $calendarInfo; |
But we use it without an isset before and thus, marking it as optional, will trigger some new psalm warnings.
c79d01f to
6867a31
Compare
6867a31 to
a70bc08
Compare
apps/dav/tests/integration/DAV/Sharing/CalDavSharingBackendTest.php
Outdated
Show resolved
Hide resolved
a58aefa to
811af48
Compare
811af48 to
cb72b02
Compare
|
@SebastianKrupinski please test and review this PR. Thanks! |
SebastianKrupinski
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested Works.
- Introduces a `unshare` method in `CalDavBackend` to handle user unshares. - Implements check to determine if unshare entry is needed based on group/circle membership. - Ensures `updateShares` is only used when the calendar owner manages shares. - Resolves issue where unsharing a calendar as owner created an unshare entry in `oc_dav_shares`. Related PRs: - #43117 - #47737 Signed-off-by: Daniel Kesselberg <[email protected]>
Before: Find all entries in `dav_shares` with `access = 5` for the user's principal, as well as group and circle memberships. After: Find all entries in `dav_shares` with `access = 5` solely for the user's principal. Future support for unsharing group or circle principals could be considered as a feature enhancement. Signed-off-by: Daniel Kesselberg <[email protected]>
cb72b02 to
023b98c
Compare
|
/backport to stable31 |
Summary
I'm sorry the PR ended up being much larger than expected. The issue arose because managing calendar shares as an owner and unsharing a calendar as an owner used the same code. This led to a situation where removing an individual calendar share as an owner, while having at least one group share, created an unshare item for the individual user and made the calendar inaccessible.
I reworked the implementation to use a separate method for unsharing a user and reverted the changes for the "manage shares as calendar owner" method. We decided to implement an occ command to unset calendar unshares instead of a migration that drops all unshares. Additionally, there's now a command to list the calendar shares.
The second commit is a hardening of the unsharing logic. Previously, we considered all principal URIs (users, groups, and circles) when looking for unshares. Now, it only considers the user's principal URI, as we only support unshares for individuals but not for groups or teams yet.
Steps to reproduce are available at: #49834
The events are, among other things, used to write the activity, so please keep an eye on whether the activities look correct while testing.
TODO
Checklist