For some unholy reason, we delete all watchdog entries when a user cancels own account. Thats a security nightmare. Someone can wreak havoc and then cover all their tracks. Attached patch removes these watchdog deletes and smartens our dblog report to show rows where the uid is no longer found.

CommentFileSizeAuthor
#7 deleted_user.patch3.99 KBjbrown
#5 remove_user_cancel_tests.patch1.83 KBberdir
dblog.diff1.39 KBmoshe weitzman

Comments

catch’s picture

catch
he/him
Primary language English
commented
Status: Needs review » Reviewed & tested by the community

I think the idea is that people can completely remove their accounts from the system, but since logs get cleared by cron, and contrib could add this hook, I agree it's madness doing it like this. Also clearing watchdog logs can be an expensive operation if there's a lot of rows so best left for cron.

Patch looks great, needs a testbot (or manual) run before commit.

jbrown’s picture

jbrown commented

I totally agree.

dries’s picture

dries commented
Priority: Critical » Normal
Status: Reviewed & tested by the community » Fixed

Committed to CVS HEAD. Thanks!

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Component: user system » dblog.module
Status: Fixed » Needs review

Poor tests... everyone is forgetting these :) The attached page removes the tests that test this.

Another thing I noticed: Those events then show up as Anonymous, but link to the profile page of the deleted user. Maybe we should add something like "Deleted user (uid: 27)" instead?

Not covered by the patch, we need to get HEAD to pass again first :)

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
StatusFileSize
new remove_user_cancel_tests.patch1.83 KB

Erm, with patch this time.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Needs review » Fixed

Oopsie. :P

Committed to HEAD.

jbrown’s picture

jbrown commented
Status: Fixed » Needs review
StatusFileSize
new deleted_user.patch3.99 KB

I implemented the suggestion in #4.

ff1’s picture

ff1 commented

#7: deleted_user.patch queued for re-testing.

neale.wolfson’s picture

neale.wolfson commented
Version: 7.x-dev » 8.0.x-dev
Assigned: moshe weitzman » Unassigned
Issue summary: View changes
Status: Needs review » Needs work

Looks OK, but needs to match to code formatting standards (Curly brackets always and no empty lines with whitespace).
Needs a reroll for GIT.

This is still an issue in D8.

zaporylie’s picture

zaporylie
he/him
Primary language Polish
commented
Status: Needs work » Closed (cannot reproduce)
Issue tags: +drupaldevdays

This is not an issue any more, at least I can't reproduce it. All eventual code standard problems will be solved in https://www.drupal.org/node/1518116