Avoid calendar & addressbook activities being created on user being deleted #32869
Conversation
tcitworld
added
bug
3. to review
| } | ||
|
|
||
| foreach ($users as $user) { | ||
| if (!$this->userManager->userExists($user)) { |
There was a problem hiding this comment.
Maybe prefix with a check for the $action, so we don't run a query per user all the time but only in case of a delete.
But even then I think the impact on normal usage is not worth the benefit on deleting a user?
There was a problem hiding this comment.
But even then I think the impact on normal usage is not worth the benefit on deleting a user?
Since we avoid the creation of X activities, it might be.
There was a problem hiding this comment.
Alternate approach would be to add the information that this is a subsequent delete due to a user delete to the Dav events?
There was a problem hiding this comment.
That would require changing the signatures function for deleteCalendar/deleteAddressbook methods in the backend just for this, which I'm not very comfortable with.
tcitworld
force-pushed
the
dav-no-deleted-user-activities
branch
from
83dec24 to
efaac42
Compare
…eleted Addressbooks and Calendar data are destroyed through hook OC_User::pre_deleteUser, which when it reaches the backends sends AddressBookDeletedEvent/CalendarDeletedEvent typed events, which in turns generates activities that aren't deleted until they expire. This can probably lead to old activities being visible for a new user created with the same uid. Signed-off-by: Thomas Citharel <[email protected]>
tcitworld
force-pushed
the
dav-no-deleted-user-activities
branch
from
efaac42 to
4129089
Compare
|
Drone failure unrelated: |
Addressbooks and Calendar data are destroyed through hook
OC_User::post_deleteUser(legacy stuff), which when it reaches the CardDAV/CalDAV backends sendsAddressBookDeletedEvent/CalendarDeletedEventtyped events, which in turns generates activities that aren't deleted until they expire.This can probably lead to old activities being visible for a new user created with the same uid.
Another way to avoid adding this call to
IUserManager::userExistswould be to have server make sure the activities table is correctly emptied at the end ofOC/User::delete, but there's no event likeAfterUserDeletedEventthat would allow the activity app to hook in.Another possibility is another activity background job that cleans activity entries for users which don't exist anymore, but that's dirty.