Skip to content

fix(CalDAV): set acls for DeletedCalendarObjectsCollection - #42850

Merged
ChristophWurst merged 1 commit into
masterfrom
fix/caldav/update-acls-deletedcalendarobjectscollection
Jan 18, 2024
Merged

fix(CalDAV): set acls for DeletedCalendarObjectsCollection#42850
ChristophWurst merged 1 commit into
masterfrom
fix/caldav/update-acls-deletedcalendarobjectscollection

Conversation

@JohannesGGE

@JohannesGGEJohannesGGE commented Jan 16, 2024

Copy link
Copy Markdown
Contributor
  • Resolves: #

Summary

TODO

  • ...

Checklist

@ChristophWurstChristophWurst left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense

tcitworld
tcitworld previously requested changes Jan 16, 2024

class DeletedCalendarObjectsCollection implements ICalendarObjectContainer {
class DeletedCalendarObjectsCollection implements ICalendarObjectContainer, IACL {
use ACLTrait;

@tcitworldtcitworldJan 16, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This brings

'privilege' => '{DAV:}all',

I think {DAV:}read should be enough, as the collection itself shouldn't be changed, but you'll need to test it.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean by adding:

publicfunctiongetACL(): array {
return [
[
'privilege' => '{DAV:}read',
'principal' => $this->getOwner(),
'protected' => true,
],
[
'privilege' => '{DAV:}unbind',
'principal' => '{DAV:}owner',
'protected' => true,
]
];
}

?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you need {DAV:}unbind ?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes. Otherwise the owner can no longer delete it themself.

@tcitworldtcitworldJan 17, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But the owner should be able to delete the elements in the collection, not the collection itself, right?

DeletedCalendarObject already has {DAV:}unbind.

And in any case the delete method here throws Forbidden.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The deletion does not work either via cadaver or via web interface without {DAV:}unbind for the owner. At least in my setup.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But the owner should be able to delete the elements in the collection, not the collection itself, right?

DeletedCalendarObject already has {DAV:}unbind.

And in any case the delete method here throws Forbidden.

It's a bit strange. If you drop unbind from the collection, objects in the collection can't be deleted neither. Sabre returns a Node with name 'objects' could not be found.

Since deletion is indeed protected by the Forbidden, I would like to move forward with this.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

\Sabre\DAVACL\Plugin::beforeUnbind checks the parent for unbind before a node is unbound.

Signed-off-by: Johannes Merkel <mail@johannesgge.de>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

4. to releaseReady to be released and/or waiting for tests to finishbug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@JohannesGGE@ChristophWurst@tcitworld@miaulalala@skjnldsv