in current cvs, deleting a user account is, as i can see, only possible for an admin. as deleting one owns account is a basic privacy requirement (see http://www.truste.org/webpublishers/pub_sitecoordinatorsguide.html , "4.4 Delete/Deactivate"), i would urgently (;) recommend to add this feature.

do you like sites where you cannot canvel your membership? i dont.

Files: 
CommentFileSizeAuthor
#168 drupal.user-cancel-8-RTBC.patch70.44 KBsun
Unable to apply patch drupal.user-cancel-8-RTBC.patch View
#167 drupal.user-cancel-8-167.patch70.44 KBsun
Passed: 8378 passes, 0 fails, 0 exceptions View
#165 drupal.user-cancel-8-165.patch70.41 KBsun
Passed: 8378 passes, 0 fails, 0 exceptions View
#163 drupal.user-cancel-8-163.patch70 KBsun
Passed: 8377 passes, 0 fails, 0 exceptions View
#160 drupal.user-cancel-8-160.patch69.38 KBsun
Failed: 8349 passes, 1 fail, 0 exceptions View
#159 drupal.user-cancel-8-159.patch69.36 KBsun
Failed: 8349 passes, 1 fail, 0 exceptions View
#158 drupal.user-cancel-8-158.patch69.01 KBsun
Failed: 8349 passes, 1 fail, 2 exceptions View
#157 drupal.user-cancel-8-157.patch66.56 KBsun
Failed: Failed to install HEAD. View
#156 drupal.user-cancel-8-156.patch66.19 KBsun
Failed: Failed to install HEAD. View
#154 drupal.user-cancel-8-154.patch65.26 KBsun
Failed: Failed to install HEAD. View
#153 drupal.user-cancel-8-153.patch65.15 KBsun
Failed: Failed to install HEAD. View
#150 drupal.user-cancel-8-150.patch64.34 KBsun
Failed: Failed to install HEAD. View
#148 drupal.user-cancel-8-148.patch63.5 KBsun
Failed: Failed to install HEAD. View
#147 drupal.user-cancel-8-146.patch63.15 KBsun
Failed: Failed to install HEAD. View
#137 drupal.user-cancel-8-137.patch62.88 KBsun
Failed: Failed to install HEAD. View
#138 drupal.user-cancel-8-138.patch63.06 KBsun
Failed: Failed to install HEAD. View
#136 drupal.user-cancel-8-136.patch62.82 KBsun
Failed: Failed to install HEAD. View
#136 drupal.user-cancel-8-136-convenience.patch31.16 KBsun
Failed: Failed to install HEAD. View
#134 drupal.user-cancel-8-134.patch63.86 KBsun
Failed: Failed to install HEAD. View
#133 remove-own-account-no-permission.png10.21 KBsun
#130 drupal.user-cancel-8-130.patch57.73 KBsun
Failed: Failed to install HEAD. View
#129 drupal.user-cancel-8-129.patch56.19 KBsun
Failed: Failed to install HEAD. View
#129 remove-user-account.png17.95 KBsun
#129 user-account.png22.95 KBsun
#129 remove-own-account.png17.51 KBsun
#129 user-settings.png8.99 KBsun
#126 drupal.user-cancel-8-126.patch54.35 KBsun
Failed: Failed to install HEAD. View
#126 irc-log-banning-cancelling-users.txt5.4 KBsun
#122 drupal.user-cancel-8-122.patch50.98 KBsun
Failed: Failed to install HEAD. View
#121 drupal.user-cancel-8-121.patch42.57 KBsun
Failed: Failed to install HEAD. View
#118 drupal.user-remove-8-118.patch30.64 KBsun
Failed: Failed to install HEAD. View
#116 drupal.user-remove-8-116.patch28.21 KBsun
Failed: Failed to install HEAD. View
#115 drupal.user-remove-8-115.patch28.03 KBkeith.smith
Failed: Failed to install HEAD. View
#115 patchdiff.txt2.67 KBkeith.smith
#107 drupal.user-remove-8-107.patch28.07 KBsun
Failed: Failed to install HEAD. View
#106 drupal.user-remove-8-106.patch21.49 KBsun
Failed: Failed to install HEAD. View
#106 permissions.png8.7 KBsun
#106 remove-own-account.png14.26 KBsun
#106 remove-own-account-no-permission.png10 KBsun
#106 remove-user-account.png14.85 KBsun
#106 user-account.png17.23 KBsun
#106 user-settings.png16.68 KBsun
#105 drupal.user-remove.patch21.49 KBsun
Failed: Failed to install HEAD. View
#57 user_delete_own.patch2.65 KBDriesK
#47 patch_43.txt2.21 KBwebernet
#42 user_64.patch8.86 KBsfarestam
#37 user_61.patch8.87 KBsimon rawson
#36 user_60.patch8.85 KBsimon rawson
#32 user_58.patch8.55 KBsimon rawson
#29 user_56.patch7.48 KBsimon rawson
#26 user_55.patch7.2 KBsimon rawson
#25 user_54.patch7.36 KBsimon rawson
#11 user.module_5.diff1.55 KBNeoChucky

Comments

moshe weitzman’s picture

Assigned: Unassigned » moshe weitzman

ax convinced me by referring to user privacy ... i've assigned this one to myself.

ax’s picture

just curious - any progress on this?

moshe weitzman’s picture

i tried and gave up on this one. user_edit() is my nemesis.

ax’s picture

anyone else then? it should be possible to delete a user account, shouldnt it?

moshe weitzman’s picture

Assigned: moshe weitzman » Unassigned

assigning this feature to 'unassigned' [again - sorry]. any takers?

Anonymous’s picture

Assigned: Unassigned » Kjartan

I'll have a look at it.

Any ideas on how secure this should be? Should a mail be sent to the user telling them their account was deleted? The account will never be removed completely from the database, just a status change.

moshe weitzman’s picture

Assigned: Kjartan » moshe weitzman

i've completed this one. patch forthcoming. the implemetation is that a new link appears beside 'view' and 'edit' on the geenral user pages. it reads 'delete account'. after following that link, the user confirms that he really wants to delete his account. after agreeing, the user records are actually deleted from the database. i believe this is wha tthe user really wants, and is the most 'privacy friendly' option. the watchdog logs sufficient information such that an admin could reconstruct the user account if he really needed to.

Anonymous’s picture

Committed to CVS. Just needs some more testing before this can be closed.

Anonymous’s picture

Priority: Major » Normal

Closed.

ax’s picture

Component: Code » user.module

in an <quote>Improvement</quote> to the user module, dries <quote>also removed the "delete account" link/functionality (for now).</quote> [1]. i'm not the only one who thinks that this is a bad idea [2] and hope i can trust in the "for now" and get this back as soon as possible.

[1] http://lists.drupal.org/archives/drupal-cvs/2003-10/msg00431.html
[2] http://lists.drupal.org/archives/drupal-devel/2003-10/msg01580.html

NeoChucky’s picture

Category: feature » task
FileSize
1.55 KB

I've modified the user.module included in the Drupal 4.4.2 Debian package.

Basically, I've undone an adapted the patch in which the delete functionality was removed. Now a user can "delete" her own account, but in fact it remains only deactivated.

I hope this patch to be useful for everyone. Please, feel free to add it to the Drupal mainstream if you like. :-)

beginner’s picture

Version: » 4.6.3

The "for now" is still lasting...

+1 for the principle of allowing user to delete account.
But:
What happens to all the posts that user has posted? Do they get deleted too?
What about the comments?
What about to all the replies by other posters to those posts and comments?

The link to the privacy document is dead.
Any alternative one?

beginner’s picture

Version: 4.6.3 »
fgm’s picture

Title: let a user delete its own account » what *should* happen

When a user asks for removal, what should ideally happen after a confirmation phase, preferably through an email instead of just on site, seems to be :

  • any nominative information is removed from the system. This includes IP logging if a module has been used to track it.
  • the user login uid is flagged as deactivated (status ? data ?) and cannot be reused, and a generic "unsubscribed" name is used to replace name
  • choice is given to the outgoing user to remove his postings or not.
    • If he leaves them in place
      • as the uid is still there, the generic "unsubscribed" name is used as the author name in all nodes
      • ideally, all comments in a node for which the user has posted are parsed to replace the user name by the generic replacement (which is probably error-prone, alas)
    • otherwise
      • comments by that user are replaced by an empty comment marking "deleted" and with the generic name, to maintain threading
      • nodes by that user are removed with all their comments, and comments pertaining to it are emailed to their respective authors so they can choose to repost them with an explanation instead of losing them (or could be directly removed, choice given to the administrator in a site setting)

Note that IANAL and, especially in the EU, personal data is regarded as highly sensitive (much more so than in the USA), and any webmaster implementing drupal should probably consult a lawyer for the implications of setting up a site and choosing to keep personal data (think backups). At the very least, for EU Drupal users, this is a must-read:

http://www.europarl.eu.int/stoa/ta/privacy_and_data_security/privacy_and...

breyten’s picture

Title: what *should* happen » let a user delete its own account

Please don't change the title. putting it back :).

Robin Monks’s picture

Um... This is node ***8*** on this site, and it still remains to be fixed.

Please let's get this fixed. It's a privacy and legal issue.

Robin

Robin Monks’s picture

http://drupal.org/node/29373 has been marked as a duplicate of this bug.

webchick’s picture

Marked http://drupal.org/node/43674 a duplicate of this bug.

Btw, the truste link at the beginning seems dead, but here is a relevant excerpt from COPPA (The Children's Online Privacy Protection Act) for sites which collect data from children: http://www.ftc.gov/bcp/conline/pubs/buspubs/coppa.htm

Relevant excerpt:

"At any time, a parent may revoke his/her consent, refuse to allow an operator to further use or collect their child's personal information, and direct the operator to delete the information."

So this is still a legal issue, at least in certain circumstances, and should be addressed.

chx’s picture

I can see two options: a) we clean the user data, his email, his password, his username (set it to 'deleted user') his profile data and then and block b) we delete the user and all his nodes, his comments in faith that it's all his content.

simon rawson’s picture

I posted this in http://drupal.org/node/43674 but since that's a duplicate, I suppose I should post it here...

If you are running your site as a web community, then I can understand the need for users to be able to delete their own accounts and (possibly) content. However, if you are running a "commericial" content management system then you really don't want users to be able to delete themselves or, worse still, delete their content. Drupal must, therefore, cope with both situations.

Proposed Solution

A configurable setting which allows administrators to determine:

  1. Are users allowed to delete their own account?
  2. Are users allowed to delete all their own content when they delete their account?

This could be in the form of permissions under access control, or it could be in the form of a setting (like for registrations) on admin/settings/user. My feeling is the 1 should be a permission ("delete own account") and 2 should be a setting ("delete own content on account deletion").

So, when the user profile form is editted, drupal needs to check whether the current user has the permission "administer users" or "delete own account" and if either is granted then we should allow them to commence the deletion process.

At the deletion confirmation page, drupal would check if the user has "administer users" permission or if the setting "delete own content on account deletion" is true (and they are deleting their own account!). If it is, then we should display checkboxes for optional deletion of content. Otherwise we don't.

Deleting content means totally removing it from the database.

Not deleting content when a user is removed, remains the difficult question.

Possible Solutions

A) The user remains in the database. Remove all of the personal information except their username. Set the account status to a new (third) status: "deleted". Pros: content can still be attributed to a name, particularly nice when the post information is displayed. Possibility of "undeleting" users, if the need arose. Cons: username cannot be reused by new registrants.

B) The user is deleted from the database. Nodes which were attributed to that user should have uid set = 0. Pros: content left after deletion will behave nicely; true respect for user privacy. Cons: Don't know who created content and lots of posts show as "unknown"; possible issue not crediting the true author of content on your site.

I personally prefer A). But either works for me.

m3avrck’s picture

simon, excellent post! i agree 100% and was about to post similar.

as for what to do about the user's account, i think an hybrid approach is best, let the user or admin choose upon deletion. when they arrive at the confirm delete page, there should be some checkboxes:

"delete all content (nodes)"
"delete all comments"
"delete all forum posts"
"delete anything else i'm forgetting"

then there should be another checkbox with 2 sub-radio selections:

"delete user name"
( ) remove all personal information but leave username in tact
( ) remove account completely and set all posts to 'Anonymous'

i think these options should happen on the confirm delete page, because i can see many situations where these settings are differ greatly from user to user, especially as an admin deletes various users. there is really no global setting IMO, really depends on a per user to user basis.

however, with all of these options, all privacy laws can be met, across all borders. additionally, content can still be left in tact and this is left up to the author or admin.

i'd lets get a patch going and simon let's coordinate our efforts :-)

simon rawson’s picture

I'm open to the idea of having all of those checkboxes on the delete confirm page, however I would maintain the requirement for the two permissions/settings which are controlled by an administrator. Namely, 1. Are users allowed to delete their own account?, and 2. Are users allowed to delete all their own content when they delete their account?

You couldn't have a cms operating in a commercial environment where their content is deleted, when a member of staff (user) is deleted!

I'm willing to have a go at writing this, but wouldn't be making a start until the weekend. If you make headway before then, please let me know!

yktdan’s picture

Nothing should impede the administrator doing whatever needs to be done, fairly quickly, e.g. completely delete a spammer and all his postings. The same my be said of flamers, and others consistently making inappropriate remarks.

Another possible option for the content of a deleted user is to assign it all (under a unique but nonmeaningful name) to someone whose permissions are reviewer who can selectively keep and delete the items.

m3avrck’s picture

Simon, I agree 100%. Those checkboxes would only be available if the right permissions have been granted as you stated. I can't start work on this patch till this weekend either so let's shoot for them. Catch me on #drupal as m3avrck, cheers!

simon rawson’s picture

Assigned: moshe weitzman » simon rawson
FileSize
7.36 KB

I've attached a first attempt at this patch. What I've done:

Introduced two new permissions. "Delete own account" and "Delete content with account(s)".

So there are four scenarious now. Providing a user has suffiicient permissions they can:

1. Delete personal info excluding username and leave content.
2. Delete personal info including username and leave content (will become anonymous).
3. Delete personal info excluding username and delete content.
4. Delete personal info including username and delete content.

It all works, as far as I can tell. If you view a deleted user's profile, you get a message telling you they are deleted. If you have sufficient permissions (administer users) you can bring them back from the dead by going to the edit tab and entering a password, email address and marking them "active".

I can't imagine a situation when scenario 3 would be useful. Perhaps I should make this impossible.

The patch adds lots of queries, but only when you are actually deleting stuff. Otherwise it is fairly slim. And I think it is all security tight, with the new permissions.

Let me know what you think. The code still needs tidying up. One thing I haven't looked at yet is $destinations.

simon rawson’s picture

Status: Active » Needs work
FileSize
7.2 KB

Slightly updated patch and setting to code needs work.

m3avrck’s picture

Simon, great start!!! This is certainly much needed, but here's a few things after looking at the patch.

I'm not sure what this means:
drupal_set_message(t('Your deletion instructions have been actioned.')); ... perhaps just saying "The user %user has been deleted" should suffice? Or, even depending on what options were checked, having different messages saying "The user %user and all content has been deleted" etc...

What about forum posts?

We shouldn't introduce hardcode HTML into core like this:
$form['warning'] = array ('#value' => '<strong>This action is irreversible.</strong>');

Perhaps something like: drupal_set_message(t('Warning: this action cannot be undone.'), 'error'); String should be translatable to and having 'undone' follows more along the lines of other core messages.

Upon user deletion we should *not* go back to the node page: drupal_goto('node'); but back to the admin > users page.

Shouldn't this query use UPDATE / SET syntax instead? db_query("REPLACE INTO {users} (uid, name, status) VALUES (%d,'%s',2)", $account->uid, $account->name);

This code:

    } elseif ($account->status == 2) {
      return 'This user has been deleted.';
    }

Should have: else if on the following line. Also, it should return a translatable string: return t('This user has been deleted');

After a user is deleted but if I leave the username intact and I goto view the list of users, it says Member for: 36 years 3 weeks ... perhaps we should record in the database a timestamp of when the user was created? And this field can be the difference of when the user was created till user was deleted? Or for simplicity, maybe this member for should just return 'n/a' or 'never'.

Otherwise, patch looks good and is much delete!

m3avrck’s picture

Also with that last comment, in the users table, the status should say 'deleted', right now it just comes up blank.

simon rawson’s picture

FileSize
7.48 KB

m3avrck,

Thanks very much for all those points. I have much to learn about the core... it is a beautiful thing...

Anyway - I've addressed all of you points, I think, in this version. Which is must more robust in terms of deleting content and comments. Most notably I now use calls to node_delete($nid) and _comment_delete_thread($comment) rather than using my own SQL. This is much nicer, and invokes the appropriate hooks.

Still to do:

1. Add a 'Cancel' link to the delete confirm page. (possibly by reintroducing the use of confirm_form()).
2. Think about $destinations.
3. Properly think through and check what happens when we delete revisions which are authored by deletee. Could we end up with some nasty situations? (e.g. if deletee authored current version... what happens?)

m3avrck’s picture

Simon, new patch looks great! A few more things :-)

1. Biggest thing I overlooked on first review and what you mentioned is that we should move this to the confirm_form() screen. Check out filter_admin_delete() for an example. This doens't really require much code change, just shifting things around a bit and building the form into $form and passing this to the confirm_form()

2. user_edit_delete I'm guessing should be renamed user_delete

3. If I delete a user but leave his username, it still shows up in the list of users (which is good). However, I *cannot* delete that username/account anymore as the delete button is gone. Shouldn't it still be there so that account can be permanently deleted if need be as well?

Looks like this patch is almost ready for commit, keep up the awesome work!

dopry’s picture

Priority: Normal » Critical

I'm setting this bug back to critical due to the privacy and legal issues involved. I think resolving it would close a couple of other user deletion related bugs. Or at least pave the way, a couple of flags on hook_user $op = delete for delete or archive content... so other modules can know what to do...

simon rawson’s picture

FileSize
8.55 KB

Well, I'm quite happy to provide an updated patch (against HEAD), and with a fix for issue 2 mentioned by m3avrck in #30. I decided against fixing 1 and 3 in that post for now. I will await more feedback.

I believe this patch to be fully functional - however, as always, it will need review and testing thoroughly.

Please post feedback.

simon rawson’s picture

Sorry. What I should have said is that 1 is fixed. 2 and 3 are not. Apologies.

m3avrck’s picture

Simon, problem with the patch, seems the last portion of it got truncated somewhere, any idea? Patch is looking better ready to start testing it here.

puregin’s picture

Good to see progress being made on this.

Perhaps it doesn't matter much if things are progressing, but it seems that there are multiple issues involved here, so perhaps if it makes it easier, we should consider splitting some of them out to separate issues.

With respect to the last patch, wouldn't it make more sense to deal with deletion of content in node.module via a hook, rather than doing it in user.module. Better to let node.module deal with the complexities. For example, this patch does not deal with revisions.

Similarly, if we're interested in completely obliterating all traces of the user, we have to look a little farther. There are also instances of 'uid' in the following tables.

accesslog
comments
node_comment_statistics
history
profile values
poll_votes
watchdog

So, I'd suggest that we set up appropriate hooks in the associated modules to deal with user deletion.

simon rawson’s picture

FileSize
8.85 KB

Generally I agree about the hooks.

I've reattached the patch, but it isn't against head.

When you choose to completely delete content with the patch it does invoke node_delete, so in essence it does things "properly" where it can.

Let me know if it works this time.

simon rawson’s picture

FileSize
8.87 KB

Right!

Here's a patch against HEAD. I've had my eye off the ball for a couple of weeks and the core has changed a lot - most importantly the user module. So, although I've tested the patch and it appears to work, it will need thorough testing... and possible improvement.

Simon

danzell’s picture

I am a newbie and not familiar with patch things.
For testing purpose prior to upload the altered file to live server, how do I apply this patch and other patches out there on my windos machine?
What tools should I use?
Sorry for this silly question. Have I mentioned I'm a newbie? ;)
I need this function to delete user(s), and I currently am using Drupal 4.6.5.
Would this patch work on this version?
Is there any module available for 4.6.5?

Thanks

simon rawson’s picture

Here is a guide of how to patch on Windows. http://drupal.org/node/23409. I use Cygwin.

Unfortunately this patch does not apply to 4.6.5.

chx’s picture

module_invoke_all('user', 'delete', $edit, $account); i thought we have user_invoke.

Jose Reyero’s picture

sfarestam’s picture

Status: Needs work » Needs review
FileSize
8.86 KB

I updated patch 61 for Drupal 4.7.2. It seems to work fine for me, but it could very well be that I have messed things up or additional changes should be introduced in the patch. So, use it with care!

drumm’s picture

Status: Needs review » Needs work

- This needs to be a patch for CVS (the 4.7.x version may apply, I haven't tried).
- The indentation is inconsistent, always use two spaces.
- All form arrays should follow the standards at http://drupal.org/node/318.
- I don't see how the user's page is changed if his status is 2. (I think there should be an informative message.)
- Can you post a screenshot of the new delete screen?

Fusion_Sushi’s picture

Wow, just a comment.. this was opened in 2001??? not a high priority huh? I think users should be able to delete their own accounts. Saves us the bother if they don't want it, we should not keep them.

fgm’s picture

Fusion Sushi: you're ignoring the legal aspects. In some situations (and I'm thinking very specifically of drupal sites being run by some public agencies, or possibly public companies bound by Sarbanes-Oxley or similar texts), deletion is not an option: every content must be kept archived and safe, although it may be mandatory to either keep it available or on the contrary hide it depending on the legal context.

As always, consult your lawyer for any decision in that field, but as the developing community, we must take into account most usual regulatory conditions regarding content archival/deletion.

drumm’s picture

Category: task » feature
webernet’s picture

Title: let a user delete its own account » Let a user delete their own account
Status: Needs work » Needs review
FileSize
2.21 KB

Attached is a simple patch which adds a new permission ('delete own account') and provides users with that permission the ability to delete their own accounts.

This patch does not implement the ability to delete a users content - that can be a separate patch.

rkerr’s picture

Status: Needs review » Reviewed & tested by the community

Patch applies and works as it should.
This is a great new feature for user privacy.
(After 5 years!)

Dries’s picture

Priority: Critical » Normal

Not critical. Will have to wait for Drupal 6.0.

webernet’s picture

Version: » 5.0-beta1
Status: Reviewed & tested by the community » Postponed
webernet’s picture

Version: 5.0-beta1 » 6.x-dev
Status: Postponed » Needs review
drupalok’s picture

Hi everyone. Is there a working patch for Drupal 5.1 ?
Would be really cool... i really cant launch my site without the users having the possibility to delete their account.
Greetings, Finn

StevenPatz’s picture

keith.smith’s picture

Status: Needs review » Needs work

Patch no longer cleanly applies.

# patch -p0 < patch_43.txt
(Stripping trailing CRs from patch.)
patching file modules/user/user.module
Hunk #1 succeeded at 489 (offset 87 lines).
Hunk #2 FAILED at 896.
Hunk #3 succeeded at 1505 with fuzz 2 (offset 106 lines).
Hunk #4 succeeded at 1511 (offset 88 lines).
1 out of 4 hunks FAILED -- saving rejects to file modules/user/user.module.rej

Christefano-oldaccount’s picture

@keith.smith: Is that against HEAD? The patch from #43 still works on v1.745.2.1 (from the official 5.1 release):

Hunk #1 succeeded at 405 (offset 3 lines).
Hunk #2 succeeded at 811 (offset 2 lines).
Hunk #3 succeeded at 1411 (offset 12 lines).
Hunk #4 succeeded at 1435 (offset 12 lines).
keith.smith’s picture

I was attempting to patch against HEAD, per drumm's comment in #43, since this was a feature request (features get implemented in the latest version of Drupal), and since the issue was flagged with 6.x-dev.

DriesK’s picture

FileSize
2.65 KB

IMO, what should happen with the user's data is unrelated to this issue since this is independent on who deletes the user (i.e. admin or the user herself).

I rerolled the patch from #43 against head, and added some additional text in the confirm form to make it more user-friendly. Now let's hope this makes it into D6 :)

Notice: the patch only applies after this patch has been committed.

DriesK’s picture

Status: Needs work » Needs review

changing status to needs review -- sorry

chx’s picture

Status: Needs review » Needs work

DriesK, the difference between an admin and a user deleting a user is that it's likely that the user admin is a node admin as well and can change the authors of all nodes belong to said user.

DriesK’s picture

that's true - so you feel this patch can't make it until there is a more general "reassign or delete all user content upon user deletion" patch (which could be used in together with this patch to let the user choose to delete all its data)?

I still think that's a different issue - in this patch the user _is_ warned that his content will become unassigned. If _webmasters_ don't want this to happen, they simply shouldn't grant this permission...

DriesK’s picture

let me elaborate a bit further:

in my opinion, there is no difference between an admin choosing to allow users to delete their account, with the known consequence that all content of that user will then be assigned to anonymous, and the same admin assigning the 'create content' or 'post comments' permission to anonymous (which in all Drupal versions has been a valid option). In both cases, the admin knows that content can be anonymous. It's an admin decision.

Christefano-oldaccount’s picture

I agree with DriesK. Admins have choice to enable user deletion or not (and even use something like Node Adoption), so I really don't see this as an issue.

However, the current text "All submissions you made to this website will no longer be linked to your account" is problematic. It's easy enough to change on a per site basis (using locale or displaying a custom block at user/*/delete in the content region), but I'd really like to see an admin setting for customizing the message.

sun’s picture

Version: 6.x-dev » 7.x-dev
drupalgirl’s picture

Version: 7.x-dev » 5.2

Looks like this thing has been going on for a few years now. That's too long. I just realized that a user cannot cancel an account without engaging the Admin. That's not good. We want to be certified for privacy. User not being able to cancel eliminates us. This goes directly against the right to privacy.

There are watchdogs such as Truste that consider this a breach of privacy. Users can report violations such as this. Visit this website for an August 2007 report. I don't want to be among the violators! http://www.truste.org/consumers/reports/August2007.html

Using 5.1 on this site and new 5.2 doesn't address this. Any solution to prevent being in violation of privacy rights????

Drupalgirl

webchick’s picture

Version: 5.2 » 7.x-dev

This can't be solved until 7.x because everything before that is locked for new features.

Christefano-oldaccount’s picture

drupalgirl, there are the User Cancellation and User Delete modules that can handle this for now. I would like to see this functionality in core, too, but until that happens I've been using User Cancellation with great success (well, not that anyone has deleted their account yet but it does work).

fgm’s picture

Drupalgirl, read higher in the thread : enabling this feature can put a webmaster in an illegal situation depending on the country his site is running in (IANAL, but it seems this would be the case in France unless alternate contribution mechanisms are in place, thanks to the LCEN, which mandates keeping track of user interaction).

This seems to be an issue where the legal situation is significantly different between countries, and Drupal must be able to address all possible configurations (required, authorized, forbidden), which is probably part of the reason why this has lasted so long already.

sun’s picture

I'm trying to summarize, since reading through all above comments is quite confusing.

Requirements / considerations:

  1. A user needs to be able to remove her own account.
  2. Laws for certain services/countries force the operator to delete all user related information.
  3. Laws for certain services/countries force the operator to keep all information of removed users. This even applies to user deletions by site administrators.
  4. If laws require to keep all user related information, a user should still be able to disable (block) her own account.
  5. Independently from 2 and 3, a site administrator should be able to globally define
    • whether the username should be replaced by "Anonymous" (default), "Deleted" or a custom term
    • whether all contents of that user should be deleted (with implications regarding threaded and related/referenced contents), zero'ed, unpublished or kept as is.
  6. A user might need to be informed that her account has been cancelled (in whatever fashion).
  7. Independently from all previous points, users with a certain permission should be able to completely delete a user, optionally including all content created by that user (e.g. a spammer) and without notifying that user.

Based on the legal aspects, this issue could certainly be classified as a bug.

rkendall’s picture

wow! I can't believe this issue has been around so looooong! what a classic.

after having a read through I can appreciate there are hidden depths, but still.

fgm’s picture

I think one point of view has not been given enough attention: what typically matters is the actual retention/erasure of the user and content, not necessarily actual removal from archives, although this can exist too (and possibly clash with legal requirements).

However, the actual online visibility of these data without administration privileges and/or a legal mandate is not considered and might be what helps solve this issue, in two distinct stages:

  • a site-level setting defining the overall data retention policy for deleted users and content
  • a user-accessible UI allowing users to request disappearance of their pseudo and its created content, and obtain removal of the related data from publication

IF and only if the site-level setting allows it, apply the various removal techniques discussed in the thread, otherwise remain at the "hidden" level above, with information effectively no longer visible, but still kept if required.

Gábor Hojtsy’s picture

I think it is very late now to go with this in Drupal 6. This imaginably requires such huge changes which we should not make now as far as I see.

Chris Johnson’s picture

I believe the proper solution requires a new site administrator pane called "User Deletion Settings" where all of the points in #68 are addressed and controlled.

The organization I work for has a number of sponsors who need this functionality. Unfortunately, we are still on 4.7 and will be migrating to D5 or D6 between now and next spring. If we spend money to implement this, it would be to put it into D5 or D6, not D7.

cburschka’s picture

It seems like a trivial problem at first sight, but I suppose that is part of its deviousness.

But since D6 is looming, and any patch that changes the API now will have one hell of a time getting in (if at all), what about the path of least resistance?

I've encountered hook_user($op='delete') every now and then in contrib; it usually serves to remove configuration values or personal information they entered, but never content. It seems to be safe to trigger both by users and by admins.

The user-deletion functionality is obviously *there* in the API; the admins can use it. What is wrong with slapping one or two perms and a form (with admin&email confirmation, if so configured) onto it? I find last-minute half-measures distasteful myself, but adding the possibility for admins to make user accounts deletable (it would be an optional permission, of course) seems like it would solve far more problems than it can create. True, optional content-deletion wouldn't be bad either, but it's the only part that isn't trivial?

With this reasoning, I'm really relying on the site admin to take care of all the legal issues of #68. I'm sure it's obvious that Drupal cannot be programmed in a way that makes it impossible to break the law with it - nor can it without difficulty use default values that can be used everywhere without being at risk. Can't we trust the owner of the root account to be responsible enough to let a user delete or not delete their information?

--

It sounds as simple as adding these three permissions to the access table:

* Block own account
* Delete own account pending approval
* Delete own account without approval

Perhaps with an additional setting:
* Require email validation for account deletion.

--

But perhaps I'm seeing the problem too simplistically - it must be bigger than that if it's lasted for six years...

sun’s picture

Title: Let a user delete their own account » Let a user remove her own account
Category: feature » task

It seems like the summarized requirements were unclear.

Requirements / considerations (revised):

  1. A user needs to be able to remove her own account.
  2. Laws for certain services or countries force the operator to
    • either delete all user related information
    • or keep all information of removed users. This applies to all user deletions (even those by site administrators) except those mentioned in 6). In this case, a user should be blocked upon removal request.
  3. Independently from 2), a site administrator needs to globally define for all removed users
    • whether the username should be replaced by "Anonymous" (default), "Deleted" or a custom term
    • whether all contents of that user should be
      • deleted (with implications regarding threaded and related/referenced contents),
      • zero'ed,
      • unpublished
      • or kept as is.
  4. A user might need to confirm (double-opt-in) her removal request (see #73).
  5. A user might need to be informed that her account has been cancelled.
  6. Independently from all previous points, users with a certain permission should be able to completely delete a user, optionally including all content created by that user (e.g. a spammer) and without notifying that user.
    • Note: After upgrading from D5, this would be the default for all removals.

Basically, I agree with some of the previous posters - it should not be that hard to implement and we can default to the current behavior. After tinkering about this a bit more, I even do not think, it would change the current APIs that much - it would rather extend them.
Some further considerations:

  1. Content display modules in core would need to check whether a content may be displayed depending on a user's status [maybe dependent from c)].
  2. Because blocked and removed users are not the same, we may use users.status = -1 [tinyint(4) signed] to assert removed users.
  3. Required user permissions:
    • Remove own account
    • Permanently delete user accounts & contents [see 6) above]
    • optional: Display contents of removed users
  4. Required user hooks: delete [means removal request now; optionally using double-opt-in], remove.
    • Note: Existing hooks should be renamed.
  5. UI:
    • New tab/task in user/# (i.e. user/#/remove) for 1).
    • New fieldset with radios in admin/user/settings for 2) and 3).
    • New textarea(s) in admin/user/settings for 4) and 5) [maybe two to supply different text templates].
    • New form action in user/#/remove and confirmation page with checkboxes (i.e. user/#/delete) for 6).
    • New content (based on user) status filter options in admin/content/node.
    • New user status filter options in admin/user/user.
  6. For now, we might limit options for removal of user's content to 'zero', 'unpublish' and 'keep' to circumvent any implications of deleting threaded contents.
  7. If a user needs to be able to view a page directly upon/after removal, we need to ensure that she is actually able to access it.
Leeteq’s picture

Title: Let a user deactivate or remove their own account » Let a user remove her own account

7. Give the admins a way of managing whether the user name should be made available to use by new user, or should be blocked and not be available to anyone even after the deletion.

8. If that same user, identified by confirming the original email address, comes back after a while, when the account and perhaps all content is deleted, it is a question whether he/she should be able to re-enable that user(name)/account.

9. Other users that has been communicating with the one that has been set as inactive (removed from active users/"deleted"), should perhaps have a way of checking the activity status and the last logged-in date, and perhaps even the status if the user has requested manually to be either set as inactive or removed (from view).

Some of these may suggest that once used, that particular user name should be blocked for usage by others unless made available by the user him/herself (by for example changing his/her user name.

If so, then there is a need to retain all the user names in the database after "deletion", so that the system has a way of preventing them from being re-used.

I agree about the privacy concerns which suggest that the sensitive information (the information about the user on the user page), should be deleted from the database. However, as far as I can see, that does not apply to the content posted in public. I think that the content (admin policy) should be possible to retain, along with the following details about the user account:
- user name, original/last email used (to enable reactivation by the user), active status, registration date, and last activity date.

(Retaining the emails could be an admin policy, and only used for enabling re-activation by the user, and obviously not accessible by either the public, nor (ideally) easily by the admins from the GUI. It should also be moved to a different place in the database where those emails cannot accidentally take part of future outgoing emails to active users, if some module has a bug or forgets to check the status, etc.)

When we get further down the road, several concerns seems to demand higher attention, for example related to digital identities, openid, search engine cache/internet archives, reputation management, etc,

The more I think along these lines, the more I tend to lean towards no actual/complete deletion of the user (but private data, yes), but rather flexible ways of managing inactivity status and appearance (content/user name/status).

Edit: This comment was initially just a subscription to the thread. I chose to add these points to this old comment of mine, as it seemed practical to have it posted right beneath the list of the Sun's other 6 points.

momper’s picture

subscribing

mrgoltra’s picture

subscribing

Jose Reyero’s picture

Funny this has been going on for years and the reason IMHO is that we are taking all the time the wrong approach here. All these requirements we have compiled may very well be a new drupaluserlegalrequirements.module :-)

Come on, it cannot be that difficult, we just need to add some simple cool features with reasonable defaults. And this issue was simple enough at the beginning: "Let a user remove her own account".

And this simple feature would allow our sites to be ok for about half of the world countries and, most important, keep your site users happy because they can remove their accounts.

Then if someone really needs to comply with all his/her country's legal requirements then you better hire a lawyer and a coder to work together on specific site customizations... And possibly in most countries you'll find out that you better do no web site at all to be 100% safe. Really, setting up a web site used to be (should be) easier :-(

So well, I just wanted to add my two cents to the discussion, specially since I'm getting notifications for all posts here from some time ago, but I won't be throwing any code at this one the way it is currently going. I'd just +1 any patch that gets something done.

zilla’s picture

i'm so fascinated that this has been a 6 year conversation with no resolve - data portability is a hot topic, and with it comes account shut-downs....i believe that this should be in core, not a popin module...

webchick’s picture

zilla: Looking forward to your patches.

zilla’s picture

@webchick - i unfortunately can not write code, but i DO wonder if this could be resolved with a simple workflow solution - here's the idea:

using the 'user request role' module - an admin creates a user type called "Deactivate" and in 'my account' all users can see the ability to request access to that role - a user requests such a change, and the request is 'automatically approved' and the admin is notified of the request at which point the admin logs in and deletes the users manually by request...a "trigger" perhaps notifies the admin and/or 'removes the user' if that is possible with a trigger???

that's a solution for less busy sites, because it wouldn't work with thousands of requests per day - though of course, if thousands want to deactivate per day then you might want to rethink your whole site idea ;)

what do you think? perhaps this would allow for some basic framework for a process to support account deactivation...

if you think this makes sense, let me know and i'll write up a detailed workflow type of thing in the handbook for people to examine...

Wim Leers’s picture

Subscribing.

Fayna’s picture

This is interesting! Subscribing

ztyx’s picture

Subscribing.

cburschka’s picture

Having "deactivated" as an extra role seems not to match the purpose of the user roles (a hack, in other words). Rather, I think there should be an extra account state next to "active" and "blocked" (currently a boolean state), named "resigned" or "withdrawn".

A "resigned" user account has the following properties:

- it can log in (unlike a blocked account)
- it can change its own state back to active
- it cannot create content or comments

Depending on admin configuration, a resigned account's

- profile cannot be viewed
- nodes are unpublished
- comments are unpublished

-------

I believe this will allow maximal control to the user about their own data, while being minimally destructive and completely reversible.

Possible criticism: Some jurisdictions require data to be deleted permanently at the user's request; this solution retains it outside public view.

Counter: The admin can delete the account manually if required by law in their jurisdiction, along with any content. There is hardly any content management system that allows a user to delete their own submissions permanently (in fact, most don't even allow hiding it). Our job is to make it as easy as possible for the admin to comply with local regulations, not to follow these regulations by default or make it impossible to break them.

So: Allow users to de-activate their accounts, but keep their data and allow them to re-activate until the admin manually deletes it. Also allow the admin to disable the hiding of data.

BioALIEN’s picture

Title: Let a user remove her own account » Let a user deactivate or remove their own account
baronmunchowsen’s picture

subscribing

oc’s picture

Let me propose a user interface for this feature. I agree that this needs to be made more of a priority, but I haven't seen anyone put forth a solution that would be straightforward from the perspective of the user and the administrator.

Obviously this discussion has not come to any resolution, so if you want it to reach closure and you like what I have to say, help me out a bit on this!

THE PROPOSAL:

Basically at the bottom of the page
http://www.site.org/user/*/edit

there ought to be an Account Removal section containing just one button with small print underneath it:

[ Delete Account ]
Removes this account so that it can no longer be used. You will be given the option to choose whether or not to remove any posts associated with this account.

When user clicks on the delete account button they should be taken to the page:
http://www.site.org/user/*/delete

and the content of this page would be 3 radio buttons:

Please confirm this action; you can choose whether to also remove posts created by this user.

[*] Remove account entirely as well as all posts associated with this user

[ ] Remove account entirely but reassign posts by this user to the 'anonymous user'

[ ] Deactivate the account. Associated posts remain, but logins, messaging, etc. are disabled

[ Confirm ] [ Cancel ]

If user had no posts, then only two choices should appear:

[*] Remove account entirely (no posts are associated with this account)"
[ ] Deactivate the account. Logins, messaging, etc. will disabled"

I think that leaving it up to the user is fine. If there really were a need to prevent users from deleting their content, then permanent delete requests could be be put into a "delete review queue" and there could be a setting to "Enable admin review of account removal" or something like that, but perhaps that is a bell and whistle that is unnecessary.

I don't think that allowing the user to log in to a deactivated account is necessary; it complicates things. Probably it should be possible for an admin to reactivate a deactivated account (an admin might deactivate someone for bad behavior and thus should be able to restore as well). Sure a user might mistakenly deactivate their account but this would be so rare that it is no big deal to force them to get admin help to restore things. A pay site might be deactivated after a certain number of months, so the message that you get when your account has been deactivated is something that ought to be customizable. "Your account was deactivated by _________ on this site. Please contact customer service to renew your subscription/ reinstate your account."

I don't think you need any more than 2 permissions "Delete own account" and "Delete/deactivate others' accounts"... "keep it simple stupid"

It would be great to see this issue fixed in versions 5 and 6.

-rich cowan
Organizers' Collaborative

Susurrus’s picture

Why would we offer the user the option to deactivate the account or disable it; If a user wants to deactivate an account, they just leave it be, I'm sure everyone here has an account on a website that's in that state. They don't want email messages from the site, they can deactivate them all (hopefully).

I see this being more of an administrative decision than a user decision. The user should be able to affect their account in the only way allowed by the administrator. I don't see the need to make this more complicated for the user.

Maybe I haven't been following this thread as closely as I thought, though, since most posts seem intent on leaving the "deactivated" state in this. I would think this option could be easily handled via contrib, however, and this issue was originally only about deleting the account.

Leeteq’s picture

At the very least, there should be a tick mark available for the user to set the account as disabled.
For example to stop being listed on user/country pages, etc., and also potentially to not be counted among "active users" if such a statistics is used on the site.

Administrative deactivation:
I also think there should be a separate tick mark for the administrator and even the system itself (automated) to set an account as "marked for deactivation" based on user activity. When that is set, an email would be sent to the user informing about a scheduled deactivation of the account and prompt to re-visit the site if wanting to prevent the account from being deactivated. The admin settings for that should include time frame settings: how long time before the system deems an account as inactive, and how long time to wait for the user to respond to the system email before deactivating it. (so, two tick marks).

RobLoach’s picture

I can't believe this is still around.

mrintegrity’s picture

yeah, can't wait for d7 when this might show up :)

A user should never be able to sign up with a previously deleted username otherwise your going to have issues of stolen identity.

Leeteq’s picture

Yes, it is important that the administration of users includes features to prevent issues like stolen identity or confusion related to it.

So the record should not be deleted, only deactivated in a way that prevents it from being pulled into any queries etc.

How to prevent mistakes in future snippets etc. that may query the users table without taking this issue into consideration? Somehow "enforce" calling through an API or the like for any access to the user table, so that it effectively prevents access to deactivated accounts for other purposes than user account administration (for administrators)?

It would be practical with the possibility for a user to set his/her account deactive temporarily, but still be able to re-activate it later without administrator interference, simply by having access to the email address registered with that account.

For any account that has been in use and has posted comments or other, it should not be possible for others to use that user name even if the account is deleted or deactivated. But if an account has been registered but never been used, it should be possible to deleted it in a way that makes that user name available to either others that want to change their user name, or to be used with new user registrations.

dnewkerk’s picture

Subscribing... thinking about some ideas to share if I can refine them (I have this kind of request often on my site, and it would be great to eventually put in the hands of the user rather than mine... in the mean time I'm trying to modify the user cancellation or user delete module to add more checks to guide the user and prevent accidental deletion).

Michael Phipps’s picture

Subscribing...

From a user point of view, I was trying out a website (kwippy, an app similar to twitter that gives you a public presence) a few days ago that I decided I wasn't interested in. I wanted to delete my account to remove the public profile, and to indicate I was not interested in using the service to the admin.

When I think about it, facebook allows for add / remove for their applications, and I think the workflow for that process works quite well. I'd love to see User Delete Account get to core.

christefano’s picture

When I think about it, facebook allows for add / remove for their applications, and I think the workflow for that process works quite well.

I really don't see how this is relevant. Facebook accounts are manually deleted by Facebook staff (which means Facebook just uses a fancy form and still doesn't allow users to delete their own accounts). Building this type of functionality in Drupal is easy.

There was only one objection to DriesK's patch and I think it should be rerolled for 7.x-dev. Otherwise, the only way I see this issue ever going forward is if a contrib module that handles all the multiple use cases sun listed is developed separately with the purpose of adding to core.

bsherwood’s picture

Title: Let a user remove her own account » Let a user deactivate or remove their own account

I realize that this is a difficult concept to code with many different approaches. Do we deactivate, delete, rename content/files/etc... from the site and if so how can we allow site admins to control what actually gets deleted or kept?

I would say the simplest solution would be to delete the account and change all users content over to a dummy "guest" account. From a non-developer's standpoint this would be the simplest approach:

1. Locate all nodes/comments from "X user"
2. Change "X user" to "Guest"
3. Block Account, Log off user, delete sessions and other cleanup
4. Remove account

This way we can at least get something on the table and can be expanded further down the road. Sometimes just starting it can help fuel the fire. Maybe even down the road there could be a whole API to handle this. But it seems to me that there is a lot of what should happen when a user wants to remove themselves from the site. I just think we can start with one option and expand it from there.

r0g’s picture

subscribe

Wolfflow’s picture

subscribe

Pasqualle’s picture

-1
I hate legal issues :)

John Doe’s picture

I could miss something in this thread, but as I see, there there are still no solution (or even bloody hack) for Drupal 6.x.
Maybe someone can propose more or less working solution until this feature will be implemented?

sethcohn’s picture

Happy (belated) 7th anniversary to this bug. Is this the oldest unclosed bug for Drupal? (aka subscribe)

Pasqualle’s picture

no, the oldest is: http://drupal.org/node/3 (but it is a top secret, you can rule all the Drupal sites with that secret)
Every Christmas the best Drupal developer gets that secret, so he/she can repair all your broken sites.. Did you ever wonder why your site just started magically working again?

alexanderpas’s picture

subscribing :D

might be working on this later.

sun’s picture

Assigned: simon rawson » sun
Priority: Normal » Critical
Status: Needs work » Needs review
FileSize
21.49 KB
Failed: Failed to install HEAD. View

Happy New Year! It's time for a change. ;)

1 year 1 month and 26 days after chx's announcement to the development list - let's get this done!

New permissions:

  • 'remove own account': Allows users to remove or disable their own accounts.
  • 'select account removal method': Allows users to select the method for removing their account (and optionally contents).

Account removal options (# refers to issue comments; all upper-case refers to defines introduced by this patch):

  • #14: Remove user account and anonymize all contents. (assigned to uid 0; default)
    USER_REMOVAL_ANONYMIZE
  • #14/18: Remove user account and remove [or zero out] all contents. (zero out only required for threaded contents and handled internally)
    USER_REMOVAL_DELETE
  • #45: Disable [block] user account and keep all contents.
    USER_REMOVAL_BLOCK
  • #45: Disable [block] user account and unpublish all contents.
    USER_REMOVAL_BLOCK_UNPUBLISH

User removal confirmation form allows

  • users with 'select account removal method' or 'administer users' permission to override the default user removal behavior.
  • users with 'administer users' permission to override the "Notify user when account is deleted." setting by ticking a checkbox.

Each account removal request has to be confirmed via email. The exception to this is when aforementioned checkbox on the user removal confirmation form was checked by an administrator, in which case no confirmation and no account deletion notification mail is sent.

The removal of all contents of a user can only be handled solidly when this removal type is provided by core. That is, because contrib modules need a standard here they can rely on. node_user_delete() has been altered as a example implementation for now. As mentioned in the (only) @todo comment, hook_user_delete() is misleading for the USER_REMOVAL_BLOCK* methods, so we probably want to rename it to hook_user_remove() or similar.

Based on the (obviously outdated) comments in this issue, at least the following modules/contents need to be removed:

- node: node, node_revisions, history [done]
- comment: comments, node_comment_statistics
- files
- profile
- poll: poll_vote
- statistics: accesslog

Ensuring that a deleted user name can no longer be used by another user is outside the scope of this issue and can also be happily left for contrib.

Also included is a module update for user.module to set the default behavior for user removals to USER_REMOVAL_ANONYMIZE (the way user deletions worked so far).

Please note that the new default mail template for the account removal confirmation mail does not follow the coding-style of the other templates. I will fix the other templates in #333658: Code clean-up.

What I need to know now is:

a) whether the overall approach and design is ok?

b) whether there is a better method of completely storing $form_state['values'] for the time the account removal mail is sent and the user comes back to confirm?

c) whether the names of the defines could be improved?

sun’s picture

Removed the 'skip sending mails' checkbox when an admin removes his own account. This was handled in the corresponding submit function before, but the checkbox actually did not (!) have any effect.

This time with screenshots. :)

sun’s picture

FileSize
28.07 KB
Failed: Failed to install HEAD. View

- Moved defines into user.module.

- user_delete() takes account removal $method as third argument now.

- Fixed missing status message after disabling a user.

- Added support for mass-user removals.

- Fixed user_remove(); previous code was plain wrong, sorry.

- Fixed and added user registration and deletion tests. Almost working, but I have difficulties in generating the same user account removal link like in the mail that would be sent to the user and invoke the user removal with those parameters afterwards. Interestingly, this part of the user registration process is not (yet?) covered by tests, too - intentionally?

sun’s picture

Status: Needs review » Needs work

(channeling chx) Example implementation node_user_delete() needs work. We have to properly invoke the Node API/node_delete(), which in turn will require a batch.

dnewkerk’s picture

Status: Needs work » Needs review

I wanted to add a suggestion that the admin be able to decide whether users are offered the permissions/ability to remove the content they have created, while still allowing them to disable/delete their account, and if deleted, only set their content to anonymous but not delete it.

So if set this way by the admin, the user's UI would show only:
- Disable user account and keep all contents.
- Remove user account and anonymize all contents.

On a site with an active discussion forum for instance, a user deleting years worth of the forum topics they initiated could lead to the loss of countless useful discussions, solutions, ideas, etc which were made in the comments/replies to the initial topic (e.g. imagine if that happened here at drupal.org - one of the primary learning resources here is reading the past solutions of others in forum discussions... if someone decided they no longer prefer Drupal and then wiped out all of those solutions, it would be a loss to everyone else). On my forum at least, the policy is that we do not delete a user's old content by request unless required to do so by law (though we do respond to requests to remove personal info like email addresses and full names from old content if the user points out the URLs of the posts in question). If it were just about the user's own topics and their own replies that would be deleted, it wouldn't be as much of an issue; but the loss of other people's content (comments/discussions) as a side-effect is the issue for my site (and possibly other sites).

Anyhow, just my 2 cents. Thanks for your great work on this sun! :)

dnewkerk’s picture

Status: Needs review » Needs work

Oops your post came through while I was writing mine, so the status changed back. Setting it to the status you changed to in your last post.

bsherwood’s picture

I agree with Keyz on this point as well, Regular users should not be able to delete other users content (comments). I think making all the content become a "guest" account should be the default. This method should acceptable from a legal perspective, since all user information would be removed.

Another point to make is, do we actually change the user name of the deleted user? A lot of users in forum discussions will refer to other users in their comments and if a lot of users delete their account, the site will just be filled with "guest" posts.

Personally, I don't have a major problem with this, but I am sure some forum admins would like to use this method (i.e Username (user removed)) as opposed to just using "guest" or "anonymous".

BTW, great work sun for getting the ball rolling.

sun’s picture

@Keyz/specmav: This is exactly why I chose to implement the options by using the Form API and user permissions. This way, you can allow only certain roles to remove their own accounts; you can assign a default removal method for all users, grant certain user roles the permission to select their account removal method, and form_alter() the heck out of the form. You could even go a step further and implement a new removal method in a contributed or custom module or do whatever you would like to do for your particular use-case -- the design should (hopefully) allow it. However, core will provide the default account removal methods only. So please let us focus on the default methods for core (and make sure that contrib/custom modules are able to customize them).

Now, I am rather concerned about the following points:

a) The user account removal process is almost identical to the user registration process. When a user decides to cancel/remove the own account, an account removal request e-mail is sent to the user that contains a link to confirm the account removal. This happens not only for security, but also to be consistent with the user registration process. Since I was not able to find a better method to store the selected account removal method (i.e. $form_state['values']) for the time until the user comes back to confirm, the method id is simply "stored" in the link that is output in the mail. Is this too hacky/dirty/insecure, should we rather store that information in users.data ($user) or somewhere else, or is the current approach acceptable?

b) The introduced defines use the USER_REMOVAL_* namespace. Likewise, the UI and email uses the term "account removal". If this could be improved, we should start to think from a UX perspective and adapt functions/defines accordingly afterwards. Anything with "deletion" would be misleading/wrong, since the account removal will either block or delete a user account. Would something with "cancel" be more appropriate? This point should be discussed by the UX team, please. (ideally, off-issue, e.g. in IRC)

c) slightly depending on b) user_delete() and hook_user_delete() most likely need to be renamed, since - again - the user is either blocked or deleted. As of now, renaming them to hook_user_remove() or hook_user_cancel() would be more appropriate. However, I will defer this question until there is an agreement on b).

d) I plan to enable the "Skip emails" checkbox for administrative user account removals by default, because I guess that the common case for administrative users is to delete malicious user accounts or spammers, who should not receive any account removal request and account blocked/deleted notification mails. That said, the current (single) checkbox disables the sending of *both* mails. Furthermore, the account blocked/deleted notification mail is only sent to the user if the corresponding "Notify user when account is blocked/deleted." option is enabled in the user settings. I wonder whether there should be two checkboxes to independently skip the account removal process (confirmation via mail) and account blocked/deleted notification mail?

e) slightly depending on a) The test for the user account removal process currently fails, but the process works when testing it manually. It must have something to do with the testing environment and I'll try to solve this independently from a), since we *really* should have a test for the user registration process (using the one-time login e-mail link) as well.

f) Admittedly, I am not yet familiar with the batch API. I think the proper way would be to start a global batch before module_invoke_all('user_delete') is invoked, so all modules can add expensive operations to this batch then.

g) Mass-operations on admin/user/user contain an option to "block selected users", which is redundant now. Should this option be kept or removed? I would suggest to just replace the operation name with "remove" or "cancel", i.e. depending on b).

yktdan’s picture

We just ran into an issue with e-commerce (UberCart) where people might anonymously buy something but they wind up registered so that there is a place to hold their address, etc. until the shipping process is completed. If they are allowed to delete themselves before the shipping process is completed they are likely to wind up in the state of their CC being charged, but us having no place to ship it to. Ideally, after shipping we would convert their records to anon so the info is not lost. Even after shipping it would be good to retain findability for a bit as the goods may be damaged on receipt (I don't know how long to stay in this limbo status) and we have to interact with them somewhat later. The E-Commerce store code may do this all somewhat differently and it probably depends somewhat on configuration issues.

At the very least, it looks like any e-commerce package MUST hook the delete process.

sun’s picture

@yktdan: Limiting/granting access to remove the own account can be done via role assignments and I'm pretty sure that E-Commerce/Ubercart already have extensive support for (un)assigning user roles.

Ok. Enough use-cases. Please stay on topic, review the patch, and help to flesh out above mentioned design issues. Otherwise, I'll be forced to get someone to unpublish follow-ups that are unrelated/offtopic.

keith.smith’s picture

FileSize
2.67 KB
28.03 KB
Failed: Failed to install HEAD. View

I've tweaked a very few strings in the attached version of the patch, notably removing a few awkward "own"'s that were in a few permission descriptions.

(A diff between this patch and the previous one is included for reference.)

sun’s picture

FileSize
28.21 KB
Failed: Failed to install HEAD. View

- Moved "Account removal mail" fieldset above "Account blocked mail" and "Account deleted mail" fieldset.

- Merged string changes of #115.

- Fixed user removal tests after string changes.

- Fixed no notification sent for USER_REMOVAL_BLOCK* methods.

I need feedback on #112.

sun’s picture

The real cause for the failing tests is #353618: drupalLogin does not update passed-in user object.

sun’s picture

FileSize
30.64 KB
Failed: Failed to install HEAD. View

All user tests pass. #353618 will be included in this patch until it has been committed.

- Fixed session and global $user object not properly destroyed if current user is removed (forked from user_logout()).

- Removed user registration tests from test case (already tested elsewhere).

- Improved tests by attempting bogus and expired account removal.

Recent discussions in IRC revealed that "cancel" would be a better term than "remove".

Bojhan’s picture

Subscribing.

catch’s picture

subscribing

sun’s picture

Title: Let a user deactivate or remove their own account » Let users cancel their accounts
Status: Needs work » Needs review
FileSize
42.57 KB
Failed: Failed to install HEAD. View

After in-depth discussion with a few members of the UX team, we decided that "Cancel (your) account" is the proper way to call this. Likewise, we have "cancel account methods", user_delete() becomes user_cancel(), and hook_user_delete() becomes hook_user_cancel().

@UX team: Mind to review the new strings?

This means that a), b), c), and e) are basically done now - leaving us with:

d) Implement 2 checkboxes (instead of one) to skip confirmation mail and notification mail (if enabled) separately. Enable them by default.

f) Implement a batch for module_invoke_all('user_cancel'), so all modules can add expensive operations to this batch.

g) Mass-operations on admin/user/user contain an option to "block selected users", which is redundant now. Should this option be kept or removed? I would suggest to just replace the operation name with "cancel".

h) Discussing the removal of contents and upcoming changes to Comment module with catch revealed that we cannot delete all contents, and have to zero out all nodes/comments instead. This is, because Comment will most probably delete replies to deleted comments in a thread in the future. The killer example is: Consider ax deciding to cancel his account. ;) Strong opponents to this decision should ping catch and me in IRC.

i) Enhance tests to verify content removals (zero outs), unpublishing contents, blocking a user, and that users without "cancel account" permission are unable to cancel their accounts.

j) This patch also changes all triggers and actions for the user 'delete' event to become the 'cancel' event. I am very, very unsure whether this change is correct, and how to update existing actions if it is. Any advice is appreciated.

k) Move discussion about proper UX for placing the "Cancel account" button into separate UX-focused issue. Any takers?

I get 31 failing tests when running all tests on Windows/Apache/PHP5.2.6/latest HEAD - most of them unrelated - can anyone confirm or is it just me or due to this patch?

sun’s picture

FileSize
50.98 KB
Failed: Failed to install HEAD. View

d) Separated checkboxes for confirmation/notification. After discussion in IRC, the meaning of the checkbox(es) has been reversed.

i) Added tests for all cancel account methods, including test for cancelling account for user without required permissions. However, node creation/unpublishing/deletion tests fail for any reason.

This basically leaves us with:

f) Implement a batch for module_invoke_all('user_cancel'), so all modules can add expensive operations to this batch.

g) By all means, I have absolutely no idea how user_user_operations() happens to invoke the user_multiple_delete_confirm() function for the 'delete' operation without defining a callback. Neither does user_user_operations(), nor does user_admin_account_submit() implement a special case for leaving out the callback.

h) Discussing the removal of contents and upcoming changes to Comment module with catch revealed that we cannot delete all contents, and have to zero out all nodes/comments instead. This is, because Comment will most probably delete replies to deleted comments in a thread in the future. The killer example is: Consider ax deciding to cancel his account. ;) Strong opponents to this decision should ping catch and me in IRC.

i) Enhance tests to verify content removals (zero outs), unpublishing contents, blocking a user, and that users without "cancel account" permission are unable to cancel their accounts.

j) This patch also changes all triggers and actions for the user 'delete' event to become the 'cancel' event. I am very, very unsure whether this change is correct, and how to update existing actions if it is. Any advice is appreciated.

k) Move discussion about proper UX for placing the "Cancel account" button into separate UX-focused issue. Any takers?

Dries’s picture

I'd be happy to commit this functionality. I haven't reviewed the code yet, but I think that we should provide a bit more information to the administrator/user.

Specifically, I think it is useful to educate them a bit more about the pros and cons of each method. For example, if you choose to delete all the content of a user, that might result in other people's content, such as comments, being deleted as well. Furthermore, it can result in broken links and references.

If you choose to "anonymize" the content, it is worth pointing out (from a legal point of view) that it might not be 100% anonymous. For example, a lot of the content could have references that allow one to uniquely identify the original user (e.g. the person linked to his own homepage or used a signature in his/her comments).

All of this is obvious for us experts, but if you are new to Drupal, or if you are simply an end-user of the Drupal website, you don't have the expertise required to make these subtle trade-offs. Either we need to give the end-user less control, or we need to do a better job explaining what can and could not happen.

Here is a pretty dramatic scenario:

  1. A user posts a lot of content that has some content that uniquely identifies himself.
  2. The user decides to cancel his account and the data gets anonymized.
  3. The user finds out that some of his anonymized content can still identify him -- all his signatures are left in place.
  4. The user decides to mail the site owner and asks him to delete his old content. He explains the site owner that the instructions were inaccurate (or that he was simply confused).
  5. The site owner can't delete the content because it has been anonymized and he no longer knows what belonged to the user.
  6. Crap hits the fan ...
Dries’s picture

h) Discussing the removal of contents and upcoming changes to Comment module with catch revealed that we cannot delete all contents, and have to zero out all nodes/comments instead. This is, because Comment will most probably delete replies to deleted comments in a thread in the future. The killer example is: Consider ax deciding to cancel his account. ;) Strong opponents to this decision should ping catch and me in IRC.

Zero-ing out the content is not an option, IMO. I think we should simply delete the sub-treads of the affected comments. If certain comments quote the original comment, or use the name of the original comment author, the zero'ing becomes moot. I think we should simply delete all affected comment threads and document this in the UI.

Dries’s picture

I'd like to point out that I've yet to see a site give an end-user (not an administrator) such a wide variety of options when canceling his/her account. I don't mind this, I'm just pointing it out. At first glance, I would be OK with giving end-users (not administrators) less control. If someone has seen sites that give the end-user such amount of control, please include a screenshot or link.

Thanks for rocking this issue, sun!

sun’s picture

FileSize
5.4 KB
54.35 KB
Failed: Failed to install HEAD. View

Thanks for your first feedback, Dries!

Re #125: The point is that laws for certain countries, services, or audiences may enforce a certain privacy policy. I guess that the option to let users choose their account cancellation method on their own is an uncommon case. However, since we need to allow administrators to define the default method, and also want to be able to override the default method when having to deal with malicious users and spammers, we need to provide the cancel account methods anyway. But, to make this clear: users can only select the method for cancelling their own accounts if the "select account cancelling method" permission was granted. As mentioned in #112, the additional benefit of this is that contrib modules are able to customize or even enhance the methods this way.

New patch:

We had a further discussion on the "Account blocked" and "Account deleted" notification mails in IRC today. The result is that we replace the "Account deleted" notification with an "Account canceled" notification and that we want to keep the entire block/unblock functionality as is. From a UX perspective, those actions should be renamed to "ban" (in a separate issue), as this is the commonly used term. A user that cancels the account just receives the "Account canceled" notification. Whether the account is banned/blocked or deleted afterwards, depends on the configured default cancel method, or whether the user was allowed to choose the method. What counts is that the user canceled her account on its own and is no longer able to login. This makes the entire process for cancelling an account much cleaner. A log of the IRC discussion is attached (with permission).

f) Updated all implementations of hook_user_cancel().

g) Mass-operation "Delete selected users" has been replaced by "Cancel selected user accounts". Note that user.module's own mass-operations for users *badly* need some love (in a separate issue). The current code for invoking mass-operations is nothing else than a dirty hack (not introduced here).

i) All user tests pass. Lessons learned: Always pass the $reset argument for node_load() and node_load_multiple() in tests to make sure your tests are not using outdated/cached data. That is, because node_save() does a node_load() immediately, which in turn caches the data of the saved node.

j) I reverted all changes for triggers/actions, because a "cancel" event would be ambigious and could either mean "deleted" or "banned" (the latter trigger does not exist yet). Instead, trigger_user_cancel() invokes actions for the "delete" trigger if the cancel method is USER_CANCEL_ANONYMIZE or USER_CANCEL_DELETE. Support for a new "banned" trigger is thereby left for a separate issue. If we want to, we can additionally introduce a new, rather 'dumb' trigger for the "cancel" event.

This leaves us with:

b) UX team: Review of all introduced and changed strings.

f) Implement a batch for module_invoke_all('user_cancel'), so all modules can add expensive operations to this batch.

h) Discuss anonymizing contents as well as deleting or zero-ing out contents again. In IRC, please.

j) Do we want to add a trigger for "cancel" in this patch or defer that to a follow-up patch (if at all)?

l) Add descriptions for cancel methods, so regular users (if allowed to choose) and user administrators get to know about the implications of each option.

m) user_delete() is not invoked directly throughout Drupal core. Do we need to add a separate API function for deleting users only, or should all modules use user_cancel() and therefore have to specify a cancel method? Most probably, this can be left for a separate issue, too.

z) Create separate issues for
- "UX: Cancel account UI/button placement"
- #92: "Do not allow to re-register deleted usernames" (?)
- "UX: Rename 'block' to 'ban'"
- Fix mass-operations in user.module
- Add triggers/actions for banning/unbanning users
- Improve USER_CANCEL_ANONYMIZE method
- Add user_delete() API function (?)

This actually means that we are pretty, pretty close to completion. I will work on l) and f) now, and would like to get feedback on the other points in the meanwhile.

webchick’s picture

Holy cow, sun! :) What an awesome surprise!

Initial thoughts after some poking around:

1) As a native English speaker, "contents" sounds odd to me in sentences like "Disable user account and keep all contents." I would replace with just "content."

2) I agree with Dries that offering all of those options to end users is way overkill in all but maybe 2% of sites out there. Offering this option to all *administrative* users, however, is nice because most sites have two user cancellation scenarios: the tinfoil hat user who wants to erase all traces of their existence, and the smarmy viagra peddler for whom all traces of existence you want to erase.

I think that a simple thing we could do to help guide people in the right direction on this is to adjust that permission name/description slightly. Instead of "Select method for canceling own account" let's do something like "Select account cancellation method" with a description of "Allows administrative users to view a confirmation screen and choose what happens when a specific account is cancelled." That 2% of sites who wants to bombard their end-users with all of those options can just assign this permission to "authenticated user."

3) From the UI side of things, this confirmation screen has two problems:
a) It's going to create a Wall of Text (tm), especially when the descriptions come into play. Again, most sites are going to choose from their tinfoil vs. viagra method, and offering more options just bombards them.
b) You are one mis-click away from happy, reversible content switching to OMG THERE IS NO GOING BACK.

I don't really have any suggestions to offer here, except maybe bolding the word "Remove" wherever it appears.

4) I think to alleviate Dries's concern about "Anonymizing" not actually anonymizing anything (though, thankfully, the signature would get removed thanks to the fact that it's now a user property as of D6), the easy solution for this ever-expanding issue is simply renaming "Remove user account and anonymize all contents." to "Remove user account and assign all content to system user." since that's exactly what this option is doing. Later, we could perhaps expand this out into a "Reassign user" feature where you could arbitrarily name the user that the content of cancelled accounts gets moved to (vBulletin apparently has a "Removed Users" account for this purpose) if you wanted to separate it from Anonymous.

Any attempts to decide on how to better "anonymize" users' content than author assigning are likely to a) drag this issue out another 4 years and b) are better handled in contrib anyway, since there are likely conflicting legal requirements in different regions.

5) Speaking of legal stuff, I would like to highly encourage the removal of the sentence "Please note that laws for certain countries, services, and audiences may require one of the listed options." It is not in our purview to provide legal advice, no matter how vague it is. Because of this, this sentence is vague to the point of unhelpfulness, and thus should probably just be removed.

Dave Reid’s picture

Awesome work so far sun! Equally awesome feedback from Dries and webchick that I just can't top. :)

sun’s picture

FileSize
8.99 KB
17.51 KB
22.95 KB
17.95 KB
56.19 KB
Failed: Failed to install HEAD. View

Merged idea of #119683: User administrators should not be able to delete their own accounts into this patch: Mass-operation for cancelling accounts prevents cancelling own user account now.

@webchick:

1) done.

2) The "select method for cancelling account" permission applies to regular users only. User administrators will, of course, always see the options, as long as they do not attempt to cancel their own account, in which case they can still select the method, but have to confirm via e-mail like any other user.

3) We, erm, _intensively_ discussed this in IRC... It basically boils down to: Contrib modules as well as your custom modules are able to alter the form and the default setting. They can also add new methods, which may or may not, do extra stuff and afterwards invoke one of the core methods. Supplying extra goodness is really a matter of taste here. Aside from that, I could imagine some cool ways to automate the method selection, but we'll leave those ideas to mature in contrib (and possibly be included in D8/9/10). As far as the strings are concerned, I'm awaiting feedback by the UX team, see b).

4) We also had, uhm, a very sensible discussion about the anonymization of contents. Aside from the fact that Drupal's default behavior since 3.x (and until this patch lands on January, 8th 2009) is against any copyright law around the world, we decided to defer the discussion and implementing a proper method of anonymizing contents (or re-assigning contents to another user) to another issue, since there are plenty of possibilities and each one comes with its own issues. Since it was the default method until now, we will just keep it - temporarily or even until Drupal XVI.

5) I'm unsure about removing this pointer. In the end, webchick agreed to keep it, but only since I pointed out that most of the Drupal users and site builders as well as consultants are not aware of the legal issue and implications at all. We need to build a bridge between novice Drupal users and extra-large organizations in this regard. Keeping it may lead to additional support requests, which cannot be answered (legal advice), but removing it leaves Drupal users in the dark. I think that it would be ok to sensitize users about potential privacy/legal issues that could result of their actions (configuration), but in the end, we cannot do more than point them to the question. I would highly appreciate further opinions on this question. In IRC, please.

Also:

l) Added descriptions for cancel methods as per Dries' request in #123, so regular users (if allowed to choose) and user administrators get to know about the implications of each option. However, this looks plain b0rked and we do not have such radio buttons with description in core yet (besides input formats, and although that would be a worth improvement, if done properly). UX team, please come to help!

This leaves us with: (sorry to repeat, but this is the only way of keeping focus in this issue currently)

f) Implement a batch for module_invoke_all('user_cancel'), so all modules can add expensive operations to this batch.

h) Discuss anonymizing contents as well as deleting or zero-ing out contents again. In IRC, please.

j) Do we want to add a trigger for "cancel" in this patch or defer that to a follow-up patch (if at all)?

m) user_delete() is not invoked directly throughout Drupal core. Do we need to add an API function for deleting users only, or should all modules use user_cancel() and therefore have to specify a cancel method?

z) Create separate issues for
- "UX: Cancel account UI/button placement"
- #92: "Do not allow to re-register deleted usernames"
- "UX: Rename 'block' to 'ban'"
- Fix mass-operations in user.module
- Add triggers/actions for banning/unbanning users
- Improve USER_CANCEL_ANONYMIZE method
- Allow #options in FAPI #type radios to accept #title and #description

This time, including some screenshots to get some (even more!) attention by the UX team! 8)

sun’s picture

FileSize
57.73 KB
Failed: Failed to install HEAD. View

Thanks to chx (again!), who provided me with some pointers to other functions, f) is almost done. Even tests are passing. ;)

However, we have to introduce a node_mass_delete() function, 1:1 following node_mass_update() to get the USER_CANCEL_DELETE method done. Since this will just be a simple fork of node_mass_update() and _node_mass_update_batch_process(), I'd consider and accept scope creep here. Posting an intermediate patch for now.

In the meantime, dmitrig01 replied in IRC that he'd like to see j) and m). However, those also sound like good candidates for separate issues - any other opinions?

Since h) zero-ing out contents was basically voted-out by Dries' reply in #124 and we are not going to re-invent anonymizing contents in this issue, we are almost done (after implementing node_mass_delete()).

This means that, after node_mass_delete() has been implemented (next patch), we are basically only awaiting a major string review by the UX team. However, string optimizations can also be deferred to other issues...

That said:

I'm seriously trying hard to get this done. So, if you have something to add or criticize, please try to ping me (tha_sun) in IRC or outline the issues you have with this patch in a follow-up. I'm looking forward to in-depth reviews.

sun’s picture

Just noticed http://api.drupal.org/api/search/7/delete?page=1 and specifically http://api.drupal.org/api/function/node_multiple_delete_confirm_submit/7, which is directly invoking http://api.drupal.org/api/function/node_delete/7 for mass-deletion -- this means that a helper function for node deletes leveraging batch API would have other uses throughout core -- in turn, defer to a separate issue?

Dries’s picture

I _still_ haven't reviewed this patch's code yet but ... let's make sure to update CHANGELOG.txt.

sun’s picture

Missing screenshot for UX team to review.

sun’s picture

FileSize
63.86 KB
Failed: Failed to install HEAD. View

Finally, I had the chance to discuss the UI and introduced strings with yoroy, Bojhan, and webchick on IRC. We decided to remove the extended descriptions again and clarify the account cancellation methods instead (radio buttons). Also, if a user is not allowed to choose the method, the confirmation form does not mention what exactly happens to the user account and only describes what happens to the user's contents, for example:

"All your content will be assigned to the /Anonymous/ user. This action cannot be undone."

Aside from other string improvements, we also decided to remove the legal notice from the administrative settings.

Done:

f) Implemented a batch for module_invoke_all('user_cancel'). We cannot rewrite half of Drupal core in this issue, so the cancel account methods use mass-update functions where existent. However, in cases where no such batch-compatible functions exist (node_mass_delete() or comment_mass_*()), the code uses the same methods as similar functions in core.

h) Opinions on anonymizing, deleting, and zero-ing out contents differ a lot. It seems we cannot find a "proper" solution in this issue. The decision is to defer the optimization of the current methods into another issue - or let some better methods evolve in contrib during the 7.x release cycle.

j) Likewise, adding a trigger for "cancel" is deferred to a separate issue. I cannot think of a use-case for this, and we would have to decide whether actions for the 'cancel' trigger should run before or after the 'delete' trigger (and the 'block' trigger when that got added).

m) Likewise, I cannot think of a use-case for user_delete(), without specifying a proper cancel method that indicates what should happen with the user's contents. If a module wants to be dirty, it can always DELETE FROM users WHERE uid = 123. A user_delete() API function wouldn't do more really.

n) Massively enhanced the Cancel account tests to test proper anonymization/deletion of nodes, nodes revisions, and comments. Also added tests for mass-operations, ensuring that the administrative user account is not cancelled during a mass-operation without confirmation. All User tests pass.

o) Updated CHANGELOG.txt.

p) Updated user.api.php. And NO, I'm not going to fix user.api.php in this issue. The remaining hooks will be covered in #333658: Code clean-up, if no one beats me to it.

-----

This means we are mostly done. The UX team seems to be happy. Several people scanned through the last patches and did not come up with any issues. I'll have another look at the user_cancel_methods() implementation (FAPI #process may come to our rescue), but would like to get a core maintainer review in the meantime.

If I would have known that this issue would require me working five days full-time on it, I wouldn't know whether I would have started with it at all...

For later reference:

z) Create separate issues for
- "UX: Cancel account UI/button placement"
- #92: "Do not allow to re-register deleted usernames" (?)
- "UX: Rename 'block' to 'ban'"
- Fix mass-operations in user.module
- Add triggers/actions for banning/unbanning users
- Improve USER_CANCEL_ANONYMIZE method (?)
- Allow #options in FAPI #type radios to accept #title and #description
- Add a common 'cancel' trigger for users
- Re-add a user_delete() API function (?)
- Add node_mass_delete() or make node_mass_update() more flexible
- Add comment_mass_update() and comment_mass_delete()

yoroy’s picture

To clarify the copy changes:
- The descriptions for each removal method were not adding any new information, we made the actual option texts more explicit instead, in simpler wording.
- We removed the legal note because if we keep it, there are quite a few places that would need similar warnings (e.g. node deletion, account creation). Alas, Drupal is not a lawyer.

sun’s picture

FileSize
31.16 KB
Failed: Failed to install HEAD. View
62.82 KB
Failed: Failed to install HEAD. View

keithsmith gave the last patch a thorough review and we were able to find much better strings; mainly for the administrative account cancellation confirmation and notification checkboxes.

Also, I completely revamped user_cancel_methods(), since the previous use of extract() was ugly. Much cleaner now.

Additionally, I refactored the way how the optional cancel account method selection and administrative checkboxes are added to the forms. The code is using #access now, so contrib modules can alter those form elements more easily.

Furthermore, batch API did not properly redirect the user to the front page, if the current user was cancelling the own account. Works now.

Wow, I must say, I'm pleased with this patch now. :) So far, I'd like to thank everyone involved with it (and especially for still being friendly while me going on everybody's nerves). ;)

To get some traction into the patch reviewing process, I'm attaching two patches this time. The second one being a convenience patch, only containing the relevant changes for this issue. The full patch contains all required changes throughout core and also user tests.

sun’s picture

FileSize
62.88 KB
Failed: Failed to install HEAD. View

Core maintainer summary.

  • The primary purpose of this patch is to allow users to cancel their own account and introduce different methods for doing so.
  • By default, the administrator defines the default account cancellation method for all users on admin/user/settings.
  • A user is only able to cancel the own account if the "cancel account" permission was granted.
  • Each user who attempts to cancel the own account has to confirm the account cancellation via e-mail. This also applies to administrators who may or may not accidentally requested to cancel their accounts.
  • If the user was granted the "select cancel account method" permission, she can choose the method on her own. The primary purpose of this is to allow contrib modules to customize, enhance, or replace the available methods.
  • Users with the "administer users" permission can always select the method for cancelling user accounts. They can also override whether the e-mail confirmation is required and toggle whether the user receives a notification after the account has been canceled. However, they cannot disable the e-mail confirmation for their own account.
  • The account cancellation methods are ordered from "least destructive" to "complete destructive":
    1. Disable the account and keep all content.
    2. Disable the account and unpublish all content.
    3. Delete the account and make all content belong to the Anonymous user.
    4. Delete the account and all content.

    The first option is the same as blocking (banning) a user. We need this method, since laws for certain sites, services, or audiences require to keep all information about canceled accounts.

That's it. This should cover and solve all of the issues that have been mentioned in this thread since 2001, and also adds the flexibility to alter core's default behaviors in contributed or custom modules.

Now, let me also try to summarize a few technical issues and decisions:

  • The e-mail confirmation for cancelling the own account is consistent to our user registration process. A few people thought and mentioned that it would be more secure if we would ask for the user's password, since the e-mail address could be manipulated if the user happened to not log out on a public terminal. However, since we do not ask for the current password when trying to change the own password, this is equally unsecure. If it should happen that we improve the process for changing the own password, I would agree that we can ask for the current password instead. Until then, we can go with the e-mail confirmation.
  • Discussing the account cancellation methods was a pain, since everyone came up with yet-another-idea of doing one of the methods properly, while still leaving unresolved issues for each one. From a legal standpoint, the anonymization of contents is probably the most offensive method. It was also proposed to add a "re-assign" method, where the administrator can define the target user account, which is found in some other forum systems. chx mentioned that we may come up with a better anonymization method that would keep the canceled account intact, but flag it as anonymized and make it unusable. However, I do not want to discuss this topic further in this issue, because each method would require its own issue to be discussed properly. The anonymization of contents was the default method since Drupal 3.x, so we do not have to change it in this issue. However, the introduction of the "delete all content" method is a major part of this issue, so this cannot be dropped. Personally, I would like to see different approaches in contrib during the next release cycle(s) of Drupal core, and think about improvements to the core methods afterwards.
  • The Disable the account and keep all content. method is the same as blocking/banning as user. As mentioned above, this is one of the mission critical methods that have been the cause for why this issue was languishing around for 8 years; which means that some sites are required to keep all information or delete all information about a user. Primarily for UX reasons and community functionality, we decided to keep the "ban" functionality separated, although it is - again - logically the same as the first method (but not technically).

All other topics/issues are not tied to the core functionality of this issue and have been deferred to separate issues. None of them should hold up this patch from landing, and regarding some of them, I am even unsure whether they will ever be created. Here is the full list:

- UX: Cancel account UI/button placement
- #92: Do not allow to re-register deleted usernames (?)
- UX: Rename "block" to "ban"
- Fix mass-operations in user.module
- Add triggers/actions for banning/unbanning users
- Improve USER_CANCEL_ANONYMIZE method (?)
- Allow #options in FAPI #type radios to accept #title and #description
- Add a common 'cancel' trigger for users (?)
- Re-add a user_delete() API function (?)
- Add node_mass_delete() or make node_mass_update() more flexible
- Add comment_mass_update() and comment_mass_delete()

Fixed two very minor things not worth to mention. Aside from that, I would not mind if this was RTBC or even committed before January 8th. ;)

sun’s picture

FileSize
63.06 KB
Failed: Failed to install HEAD. View

Fixed all strings that contained "cancel account method" or "cancel account confirmation" to always use "account cancellation" (mostly in PHP comments). Thanks, catch! :)

Also, since I may not expressed this clearly: The existing functionality of blocking/unblocking users is not touched at all.

Dave Reid’s picture

Version: 6.8 » 7.x-dev
Category: bug » task
Priority: Critical » Normal

This is a patch against the 7.x development version (CVS HEAD) of Drupal, where only new features are accepted. Please don't change the status or version of this issue.

sun’s picture

Assigned: Unassigned » sun
Priority: Normal » Critical

@RockyMontana: Your comment was unpublished. We are seriously trying to get this done.

Reverting to critical, and putting back under my hood.

RockyMontana’s picture

And I was seriously trying to help.
But hey, sorry for that...

webchick’s picture

@RockyMontana: Thanks for trying to test the patch out and provide feedback. This handbook page http://drupal.org/node/28245 has more information on getting a test site up and running to test against HEAD / Drupal 7. Please drop by #drupal on irc.freenode.net if you need help getting setup for patch review! :)

Dries’s picture

I spend about 15-20 minutes looking at this patch and I did not spot any red flags. Two weird things stood out:

1) t('[confirm cancellation] Asks for confirmation.') -- I think the square brackets is a poor pattern to introduce and breaks consistency with other tests.

2) The profile module does always deletes the profile, even when the user's content is unpublished. If that is on purpose, it warrants some better documentation, but it does look like a bug to me. It is certainly not the expected behavior.

Otherwise looks good.

Dries’s picture

Issue tags: +Favorite-of-Dries
sun’s picture

1) Those square brackets were not introduced here; all tests in user.test use them. However, I have changed all assertion messages in the UserCancelTestCase now. Fixed.

2) Very good catch! I don't know how I was able overlook this mistake in the past iterations. Fixed.

Please note that due to some committed typos in #353480: Trigger tests for remove $op from hook_comment()., the tests for this issue are failing.

sun’s picture

FileSize
63.15 KB
Failed: Failed to install HEAD. View

Uhm, new patch got lost.

sun’s picture

FileSize
63.5 KB
Failed: Failed to install HEAD. View

- Renamed all instances of $user to $account in user.test per webchick's request. Also improved PHP comments.

webchick’s picture

My review after the first hour (sun has been fixing as I've been spouting stuff):

I looked at the tests first, since that always helps me understand how code works:

+    // Create a user.
+    $user = $this->drupalCreateUser(array('cancel account'));
+    $this->drupalLogin($user);
+    $timestamp = time();
+    // Update user object.
+    $old_user = $user;
+    $user = user_load($user->uid);
+    $user->pass_raw = $old_user->pass_raw;

1) I got confused with that $old_user stuff. A comment clarification could help, but even better might be just archiving the password variable rather than $old_user, since that seems to be all you're interested in.
2) Seems like we could move this to setUp() since it's basically the same in every test function. Except you can't do that, because you need different passwords in each test case, so nevermind. :P
3) This code is very, very awkward. I realize that #353618: drupalLogin does not update passed-in user object was essentially marked won't fix, but we should think about how to improve this in the future (separate patch).

There is some broken English here and there in these tests:

+   * Disable user account and keep all contents.

should be "content."

t('Does not allow to select account cancellation method.')

Should be: "Does not allow user to select ..."

+    // Confirm to cancel account.

Confirm account cancellation.

Those three or slight variations of them are in most of the tests.

Also, there are a bunch of "attempt invalid cancellation request" tests, which is great to see! However, they seem to be spread out over several testX functions. It'd be nice to centrally locate them inside a testX function of their own.

---

I have two "nails-on-chalkboard" issues with this patch, which I'd like to see given another few hours' discussion (so we meet our 1/8 deadline ;)):

#1: I think it's a big mistake to not "zero out" content when we're talking about nodes and comments with replies by other users. I would be *seriously* enraged off if *my* content on a site was deleted because some *other* user decided to delete their account. In fact, so much so that it would probably cause me to leave a given site and never come back.

I've seen other websites handle this by simply replacing the contents of any posts with "[This content was posted by a former user.]", attributed to the "Former user" account (in our case, "Anonymous user"). This seems perfectly fine and acceptable from a privacy standpoint. In the worst-case scenario, if someone else quoted a deleted user, and the deleted user raises a stink, then an admin can go in and selectively edit *that* reply to remove the offending quote, rather than potentially losing *years* worth of content (imagine the blow to our community if Steven Wittens ever came back and deleted his account...)

Dries, any chance you'd reconsider your stance on this?

#2: On the note of that "select account cancelling method" permission. I can't see any sites, other than possibly EFF.org, enabling this permission.

1. We should match user expectations. When I go to a website, I expect to be told what happens when I remove my account, not to get to choose.
2. Being given the choice naturally raises a whole bunch of questions. Why would I choose one option over another? These support requests are going to fall on the 100,000+ Drupal.org webmasters out there.
3. In general, it is desirable from a usability standpoint to *limit* the choices given to users rather than throw more things at them.
4. Any kind of thing like this that 2% of websites are ever going to use falls squarely into the "contrib" bucket, traditionally. As mentioned earlier, modules can hook_form_alter() this form to their content.

Coupled with Dries's note at #125, I would advocate that this permission be removed.

However, if we keep it:

+     'select account cancelling method' => array(
+       'title' => t('Select method for cancelling own account'),
+       'description' => t('Select the method for cancelling own user account.'),
+     ),

We need a better description here. I'd also probably call it "select account cancellation method".

OK, now onto functionality! :P

sun’s picture

FileSize
64.34 KB
Failed: Failed to install HEAD. View

Fixed all of webchick's issues in #149 (not #1, and for #2, only renamed permission).

webchick’s picture

On IRC, I threw a litany of minor things to sun which he fixed or is in the process of fixing. This is now good to go from a code standpoint, in my eyes.

#3: The only other user-facing thing I would point out is that both the confirmation page and the e-mail mention only the title of the option selected, and not the underlying consequences of choosing these options. I realize "make descriptions on radio buttons" is a separate patch (as it should be), but IMO there is value to storing a list of consequences along with each of these options and displaying it:

a) At the top of the confirmation page.
b) In the e-mail as a variable like !cancel_description.

user_cancel_methods() seems like the ideal place to store both of these things.

Apart from this (and the fact I didn't get a chance to test the mail stuff -- tomorrow), this patch looks good to go. I'd love one more round of feedback on #1, #2, and now #3, and then I'm ready to commit. Although, since it's Dries's favourite, maybe he wants to. ;)

David_Rothstein’s picture

Status: Needs review » Needs work

This is an impressive effort! I reviewed this patch (mostly with an eye towards security issues), and although it looks pretty good I found some issues:

1. The patch has a rather big security hole, in that users without the "select account cancellation method" permission can still choose whichever account cancellation method they want to. To reproduce this, you can set up a site with the defaults and then have a regular user try to cancel their account. When they do so, they receive a link like this in their email:

http://localhost/drupal/user/3/cancel/confirm/1231307894/0/0/69edd9bfa84d790318b3892d72669e48

which is designed to let them cancel their account in the most harmless way possible (the only option that is supposed to be available to them). However, all they have to do is edit this URL as follows:

http://localhost/drupal/user/3/cancel/confirm/1231307894/3/0/69edd9bfa84d790318b3892d72669e48

And then boom, when they visit that URL hundreds of nodes and comments on the site might be deleted...

The solution is to do some validation of the account cancellation method in user_cancel_confirm(). Actually, I think the best way might be to use the chosen cancellation method as part of the hash computation (since that way a particular hash is hardcoded to a particular cancellation method and there is no way to confuse them either intentionally or by mistake)?

On a minor but related note, it does not seem to me like the $notify parameter is needed as part of the URL, since it does not seem to be used in user_cancel_confirm().

2. I totally agree with @webchick that giving the users the ability to delete other users' content during account deletion is a really bad idea. A further argument against it is that the rationale for this method was that you might want to allow the user to remove all traces of themselves from your site (included any quoted material). However, there is absolutely no guarantee that all quotes or other traces are contained in comment threads that spin off something the user wrote, right? They could equally well be quoted all over the site in nodes that they never touched. Legal requirement or not, there is absolutely no automated way to guarantee that all traces of a user will be removed unless you delete the entire database :) So it doesn't seem worth having this extraordinarily dangerous method in there if it can't be expected to work correctly anyway.

3. The ability to either delete or "zero out" all one's content with a single button click is a really powerful and dangerous permission. I would suggest that some kind of very dire warning be added to the User settings page that explains to the site admin what this means and why (in most cases) it is dangerous to choose. The "select account cancellation method" permission is equally dangerous, so it might be good to add something similar to the standard "this permission has security implications" warning on the Permissions page for this one (on the other hand, the regular delete permissions do not have this warning, so maybe it's OK as is).

The following are UI comments so they may belong in a followup issue but I'm recording them here anyway while I noticed them:

4. It seems to me that the confirmation form for cancelling an account is in the wrong place. You get the confirmation form after you click the "Cancel account" button, but clicking that button doesn't do anything bad; it just sends you an email. I think the correct place for a confirm form is after you click the link in your email, because that is when you are about to do the irreversible thing. (Or possibly the confirm form should be in both places, but in that case the first one should not say "This action cannot be undone.")

5. The buttons for the confirmation form in user_cancel_confirm_form() look like this:

+    t('Cancel account'), t('Cancel'));

It's not quite as bad in the UI as it looks in the text, since one is a button and one is a link, but... yikes :) Maybe the "Cancel" link could be changed to something like "Go back" or "Leave account unchanged" for this particular confirm form?

Finally, the following comment is definitely for a followup issue, but I'm recording it here so it's part of the list:

6. Is there also a legal requirement to allow "anonymous" users to delete their personal data? By which I mean the fact that Drupal core stores personal info (name and email address) of anonymous users who leave comments on the site and fill out the relevant fields. I don't know the relevant laws, but I'm guessing the laws don't distinguish between actual Drupal user accounts and "non-accounts", if both contain personal info. Again, that's definitely not for now, just a potential followup issue :)

sun’s picture

Status: Needs work » Needs review
FileSize
65.15 KB
Failed: Failed to install HEAD. View

Separated invalid account cancellation attempts into an own test.

Re: #149 - #1: Zero-ing out content

Deleting all content including replies by other users is, as of now, the only method where administrators can be 99.9% sure that all information related to a user has really been deleted. This is crucial for many site operators, use-cases and organizations. Also, this method is helpful when moderators have to deal with spammer accounts. I would not want to have to manually delete hundreds of posts by a bot. So removing this method is not an option IMHO. That said, zero-ing out contents would have the same issues as the current anonymization method. If at all, we should discuss adding a new USER_CANCEL_ZERO method in a separate issue.

Re: #149 - #2: "select account cancellation method" permission

I am not opposed to removing this permission from core in general. I can see and understand the usability issues. But after all, it is a separate permission, which is disabled by default, but gives freedom to users who want to cancel their account when the permission is granted. Personally, I like the idea of being able to decide what happens with *my* account and *my* contents on a site when the time has come that I decide to cancel my account. If the site owner/operator is not forced (by law) to use a certain account cancellation method, I would consider it a perfectly valid option to let the users choose on their own. But, anyway, if this should be all that holds up this issue, then I'll happily remove that permission.

Also, I've taken the time to think thoroughly about #151 - #3. There are several issues with that:

  • As of now, user mail tokens only output links and stuff, but not complete sentences/content. Given a !cancel_description token, the replacement value would not be evident for the administrator.
  • If we would change the structure of the returned array, we have to prepare and parse this value to make it suitable for #options, even in (the majority) of places, where the description is not used.
  • Lastly, user mail tokens are prepared centrally in user_mail_tokens() and notification mails are sent via _user_mail_notify(), retrieving default mail templates from _user_mail_text(). All of those functions are designed to always use default token values. We would need to make the !cancel_description token dependent on the selected account cancellation method, which would require a new token generation function like the introduced user_cancel_url() function.

That said, I've splashed a

  // Allow modules to customize account cancellation methods.
  drupal_alter('user_cancel_methods', $methods);

into user_cancel_methods() now to simplify the customization of the default methods. This way, removing, altering, and adding methods on all forms is very easy.

Everyone: Feedback on #2 and #3, please!

sun’s picture

FileSize
65.26 KB
Failed: Failed to install HEAD. View

@David_Rothstein:

  1. Thanks for being the first who mentions this issue! I agree that it is not ideal, but I considered this upfront. Several points:

    1a) The link is contained in a mail we send to the user. If we do not trust the contents in such mails, then we need to rethink our entire user registration process, which uses the same way to provide a one-time login link.

    1b) There is no separate and temporary storage for user data in Drupal. If we would store the expected values in users.data, then we would have to clean-up the users.data column over time, or this data would needlessly be loaded in user_load() in the future. Storing the values in a variable is not an option either. The only other option I could think of would be using the user session, but the session could be configured to expire very early.

    1c) If an administrator cancels an account, he is able to select the account cancellation method, but still require an e-mail confirmation from the user (which is a perfectly valid use-case; even more possible if we remove the "select account cancellation method" permission). So even if the user is not permitted to choose the method, the method can be different than the default method. Since we have no storage for temporary user account data, storing the value in the cancel account link seemed the only option to me.

    1d) Likewise, there is no temporary storage for the notification parameter.

    1e) If Drupal would ask for the current password to change a user's password, then I would agree that completely switching the account cancellation process to require the current password (instead of sending a confirmation e-mail) would be the proper way. However, that's a completely different issue.

    I'm open to better suggestions.

  2. Again, removing the "Delete all content" method is not an option. There are sites - other than the usual community sites - and use-cases for Drupal that need this option or are forced to use this method (by law). Not to mention the administrator's desire to delete a spammer's account along with all of the spam content.
  3. Added the common %warning placeholder text to the permission description.
  4. Hm. I basically agree with the "This action cannot be undone." part, but if we remove that, then we have to be sure that it is mentioned in the account cancellation e-mail. The UX team already complained (slightly) about the double-confirmation, so I am very unsure whether they will support a third confirmation form after clicking on the e-mail link. If possible, I would like to defer this into the overall UX: Cancel account UI/flow issue.
  5. The UX team does not like the current confirmation forms in Drupal at all. After *days* of discussion about all the UI and UX issues, I'd prefer to defer this to the overall UX: Cancel account issue as well.
  6. Yeah, definitely a follow-up issue (if at all).
catch’s picture

Status: Needs review » Needs work

I discussed the 'delete all content' method with Sun in irc, and I'm still firmly opposed to it - thanks to webchick and David for putting some additional arguments forward here and sorry for not posting more in-depth feedback on the issue until now.

There's a massive difference between an administrator getting a 'delete all content associated with this user' button for spammers, and a regular user getting it. Additionally, while the site that needs to meet stringent legal requirements will need to look into this method properly, the 99.99% of Drupal sites which don't need it are exposed to potential misconfiguration disasters if a webmaster switches it on by mistake. As for quoting, I've both run and used sites with built in quote functionality - and a very large number of quotes are copy and pastes from the browser rather than threaded replies to comments (i.e. responding to more than one person at the same time) - so there's absolutely no benefit to deleting content rather than zero-ing out (other than the spammer example which is a completely separate use case anyway, and would rely on the administer users permission anyway). Additionally if we were really going to do this, we'd also have to clear the page cache too.

If a site really cared about anonymising content, they could maybe process all the text of all the content belonging to the user (and their name etc.) then selectively delete/zero/replace all other content elsewhere on the site which contains text matching the deleted content (to whatever extent). I don't see another way of getting this 99.9% guarantee of clearing everything out, and that's infinitely more complex.
Either way if a site has a legal requirement to use a method like this, then they should take on the extra burden of dealing with it - i.e. installing a contrib module. Since delete isn't going to actually deal with this extreme case, I'd still go for zeroing out in core which has limited damage, for all the reasons previously given.

Sun mentioned the 'delete own foo content' permission and that this allows you to delete the comments of other users as well. However, this permission clearly has the 'delete' word in it, whereas 'cancel account' does imply 'go down and take everyone else with me'. It also doesn't give you access to mass operations, whereas this fires off mass operations which the user won't necessarily even be aware of.

This brings us back to the e-mail notification issue - if we have 'delete content' in core, then we give a disgruntled user the opportunity to delete everyone else's replies to their content even if the administrator has not chosen that method as an option. For example, say a prominent politician posted something on a website, resulting in many thousands of critical replies to their initial comment/post, then they cancelled their account triggering the delete method. Opening up administrators to this either via misconfiguration or the e-mail loophole is IMO a lot worse than not supporting original use case. In fact it's essentially privilege escalation as it stands.

Otherwise apart from some of the previous feedback which it looks like Sun has largely dealt with, this looks good.

sun’s picture

Status: Needs work » Needs review
FileSize
66.19 KB
Failed: Failed to install HEAD. View

- Fixed the (debatable, but potential) security issue by storing account cancellation method and notification settings in the user object. After further discussion in IRC, we decided that the probability for a user to request an account cancellation and NOT confirming this request is very small.

- Fixed regular user account form submission stores "cancel" property in {users}.data.

- Fixed $timestamp defined in a misleading way in tests. (Yes, we want to use time() here, since $timestamp is actually generated in user_cancel_url() after submitting the account cancellation confirmation form.)

I am about to remove the USER_CANCEL_DELETE method from core. Not replacing it with a zero-out method. Both methods, including a USER_CANCEL_BETTER_ANONYMIZE method, will be left for contrib. If no one steps up *now* and lion-heartedly disagrees, I'll go ahead.

By the way, the list for separate issues has increased in the meanwhile:

- "UX: Cancel account UI/button placement"
- #92: "Do not allow to re-register deleted usernames" (?)
- "UX: Rename 'block' to 'ban'"
- Fix mass-operations in user.module
- Add triggers/actions for banning/unbanning users
- Improve USER_CANCEL_ANONYMIZE method (?)
- Allow #options in FAPI #type radios to accept #title and #description
- Add a common 'cancel' trigger for users
- Re-add a user_delete() API function (?)
- Add node_mass_delete() or make node_mass_update() more flexible
- Add comment_mass_update() and comment_mass_delete()
- Security: Ask for current password to change password
- block.module stores an empty array in users.data
- user_profile_form_submit() and user_edit_submit(), and user_profile_form_validate() and user_edit_validate() are 100% equal!
- Introduce users_data table to replace {users}.data with a more intelligent storage for user options

sun’s picture

FileSize
66.56 KB
Failed: Failed to install HEAD. View

Instead of dropping USER_CANCEL_DELETE, a long discussion in IRC resulted in the decision that this method should never be available for regular users, but only for user administrators. Effectively, user_cancel_methods() now returns FAPI elements that can have an #access property (like any other form element). So the methods should be resolved now.

This leaves us with one open question:

When a user is not allowed to choose the account cancellation method, we are explaining what happens to the user's content in the confirmation form ((outdated) screenshot). However, this description is currently defined and set in the confirmation form itself. So, if a module adds another method, it has to form_alter() its way into the confirmation form to add the appropriate description. Now, the idea arose to simply abuse the #description property of the account cancellation method radio buttons (current code), so it does not get displayed as description for the radio button, but is used for the confirmation form description instead (of course, that would be properly documented in the PHPDoc). This would allow modules to add/remove/alter the methods by simply doing some drupal_alter() magic, without having to implement further form_alter() logic for the confirmation form.

Opinions?

sun’s picture

FileSize
69.01 KB
Failed: 8349 passes, 1 fail, 2 exceptions View

Implemented the proposed #description solution after further discussion in IRC. Search for hook_user_cancel_methods_alter() and user_cancel_methods() in this patch to see a detailed explanation.

sun’s picture

FileSize
69.36 KB
Failed: 8349 passes, 1 fail, 0 exceptions View

Thanks to keithsmith and catch, vastly improved the new PHPDoc of both functions. Since the hook is the interesting part for developers, user_cancel_methods() just refers to the hook documentation. Also ensured that any method with an #access property cannot be configured as default method.

Sorry, I'm too tired for a more detailed description now.

Actually, I think this patch is very, very, very RTBC now. (At least, I was not involved in another patch that went through so many reviews and stuff. "All eyes on 8.")

sun’s picture

FileSize
69.38 KB
Failed: 8349 passes, 1 fail, 0 exceptions View

Very minor fix.

Dries’s picture

I agree that we don't want users to delete other people's content. I'm willing to reconsider my opinion and go for "zeroing out" content. However, I think it would be equally destructive if, say, Steven would zero out all his content so it is not an option I'd ever allow the user to pick him or herself.

Both options are like handing out hand grenades to your community members: "if you don't commit this patch, the 25 of us will delete our accounts and destroy 15% of the content on drupal.org". It certainly would add a new dimension to patch reviews -- one I'd prefer to avoid to be honest. ;-)

Status: Needs review » Needs work

The last submitted patch failed testing.

sun’s picture

Status: Needs work » Needs review
FileSize
70 KB
Passed: 8377 passes, 0 fails, 0 exceptions View

To clarify: We implemented an additional #access property for account cancellation methods. The USER_CANCEL_DELETE ("Delete the account and all content.") method is only available for users with the "administer users" permission and is not exposed to regular users (by default — a contrib module could change this behavior). Also, any account cancellation method that has an #access property defined can no longer be configured as the default method in the user settings. This means, the "most destructive" account cancellation method that can be configured as the default method for regular users is the anonymization method, i.e. the same method that was the default in core since Drupal 3.x.

With regard to zero-ing out contents: Please, this is just a new method that may be provided by core or contrib, but please, can we please defer this to a separate issue, please? The primary purpose and focus of this issue should be to implement and provide the "core" functionality to make it possible for users to cancel their own accounts at all. This patch underwent a long way of endless discussions, trying to find a common denominator of 2,315,683 different opinions, and now implements a very flexible way for doing so. Any optimizations and improvements to existing methods as well as additions of new methods can be done in separate issues. Contrib modules are able to implement experimental or better methods; instead of the default methods or enhancing them. The debatable question of zero-ing out contents should NOT hold up this patch from landing, please.

...sorry for this rant. I spent approximately 50% of the total time for this patch with discussions about account cancellation methods and user experience. We should not forget that this issue existed for almost 8 years, because everyone saw a different "proper" way of doing it. Instead of striving for perfection (which certainly can come later), this patch implements a Drupalish way to implement the basic functionality to let users cancel their accounts, which has the required flexibility to be further enhanced in core or in contrib.

Fixed the DBLog test. All tests should pass now.

webchick’s picture

Status: Needs review » Needs work

On #163, I have but two issues:

#1: The descriptions for the various options are pretty weak still. I pastebined some text to sun in #drupal and we re-worked it so they are more descriptive of the actual consequences.

#2: user_cancel_methods() doesn't quite smell right:
a) We don't use #properties unless we're talking about forms. (ref: hook_node_info(), hook_perm(), hook_link(), hook_menu(), etc. etc.)
b) I realize we're "kinda" talking about forms, because later you add these properties into actual form elements. But this has the side-effect of creating two different ways to alter this form, which would make tracking down misbehaviour with it very difficult, as well as be confusing to new developers.
c) It's odd to me that the "registry" of these methods and the "moosh these into a form" are in the same function. I would separate those out for more flexibility.

sun’s picture

Status: Needs work » Needs review
FileSize
70.41 KB
Passed: 8378 passes, 0 fails, 0 exceptions View

Fixed reference to previous $method should be broken in user_cancel_methods(), as requested by chx. [ - obsolete now due to further changes - ]

Improved confirmation form descriptions, as requested by webchick.

Replaced core method ids (constants) with strings, as requested by Crell and Eaton.

Changed method definition to use properties without #-signs, as requested by Crell, Eaton, and webchick. (I was rather opposed to this change, but it's changed now.)

sun’s picture

Sorry, I missed to point out that I created the first, critical offshoot issue from this list; the second being the next critical one:

- #355905: node_delete_multiple() has to use batch API
- #355926: Add comment_mass_update()
- "UX: Cancel account UI/button placement"
- "UX: Rename 'block' to 'ban'"
- Security: Ask for current password to change password
- block.module stores an empty array in users.data
- user_profile_form_submit() and user_edit_submit(), and user_profile_form_validate() and user_edit_validate() are 100% equal!
- Fix mass-operations in user.module
- Introduce users_data table to replace {users}.data with a more intelligent storage for user options
- #92: "Do not allow to re-register deleted usernames" (?)
- Add triggers/actions for banning/unbanning users
- Add a common 'cancel' trigger for users (?)
- Improve content anonymization method (?)
- Add content zero-out method (?)
- Allow #options in FAPI #type radios to accept #title and #description
- Re-add a user_delete() API function (?)

sun’s picture

FileSize
70.44 KB
Passed: 8378 passes, 0 fails, 0 exceptions View

Fixed remaining constant in user.install.

sun’s picture

Status: Needs review » Reviewed & tested by the community
FileSize
70.44 KB
Unable to apply patch drupal.user-cancel-8-RTBC.patch View

Renamed 'user_cancel_anonymize' to 'user_cancel_reassign', as requested by webchick and Crell.

Now assigning the magic status.

David_Rothstein’s picture

I was just finishing a long read-through and review of an earlier version of the patch when the newest one was posted. So I didn't do more than glance at the very newest changes... but someone has to set this RTBC, and the patch has indeed been reviewed to death!

Clearly there are some important followup issues, but they're all documented here (although I could probably add a few to the list later, based on my recent read-through) and none of them are immediate showstoppers. The main point is that the underlying architecture seems really solid, and the patch is getting so long that it's difficult to review it if it keeps getting tweaked. The followups will be much easier to deal with in separate issues.

So, I'm going out on a limb and setting this RTBC... pending the opinion of the testbot, of course :)

David_Rothstein’s picture

Oh, apparently I was beaten to the punch, and it was set to RTBC while I was writing that :)

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Ok! :)

After putting more than 8 hours of patch review time into this patch, coupled with the 8 days sun and most of #drupal spent on this, and EIGHT YEARS LATER, I felt confident committing the patch at #168. :) Hope that's okay, Dries. One wishlist item down! :)

Great work, everyone, and sun you get the award for most tenacity or most insanity or possibly both. THANK YOU.

See you in the follow-up issues! :)

sun’s picture

Status: Fixed » Needs work

Yeeeeeeeeha! :)

Actually CNW though, because of upgrade docs.

cburschka’s picture

After putting more than 8 hours of patch review time into this patch, coupled with the 8 days sun and most of #drupal spent on this, and EIGHT YEARS LATER, I felt confident committing the patch at #168.

A nice numerical theme in issue #8... and even though it didn't make it for '08, at least today is January 8. :D

sun’s picture

Status: Needs work » Fixed
int’s picture

this bug was reported by user nº 8
:)

barinder’s picture

the issue fixed yesterday i.e. on 8th and issue reported by uid => 8 , issue reported on 17 ( 1+7 = 8) , issue fixed after 8 years..... node id => 8

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for two weeks with no activity.

am22’s picture

I'm sorry, but all this thread is quite confusing. Is there any working patch for Drupal 6.8/ 6.9 ?

Pasqualle’s picture

@am22: there will be no patch for D6 as this is a new feature. You have to find a contrib module for this functionality.
http://drupal.org/project/user_delete
http://drupal.org/project/user_cancellation
they are not ready for D6 yet, but there is a patch in the issue queue for the second module..

am22’s picture

Thank you, Pasqualle. I found a patch for user_delete only, Here is the link for everybody else who is looking for it:
http://drupal.org/node/333460

dnewkerk’s picture

I started a discussion for the D6 plan for User Delete vs User Cancellation module here: http://groups.drupal.org/node/18492

KSR’s picture

I haven't used for a long while, so i decided to cancel this account.

lapadapblah’s picture

Hey, here's a new one!

After recently creating a Drupal account, I've decided I don't want it after all. So I went into my account settings and discovered that there's no way for me to delete it. So I went to support and discovered that there's this here topic that was started ten years ago. In case anyone's forgotten, here's the title of this thread: Let users cancel their accounts.

Well, ten years after, this standard functionality is still non-existent. Amazing. I can only imagine it wouldn't take but a few minutes for one of the core Drupal team members to set this up. Perhaps another ten years from now?

In the meantime, where does that leave the rest of us who want to move on, no offense to this wonderful creation.

Thanks!

StevenPatz’s picture

Quit posting.

Dave Reid’s picture

Locking this thread.

christefano’s picture

Sorry to reopen this but it's important that people realize that deleting one's own account on Drupal.org will only be possible when Drupal.org itself is upgraded to Drupal 7.

pal4life’s picture

Issue summary: View changes

Mandatory reading for Drupal newbies to understand

  • Drupal culture
  • The democratic side of Drupal
  • A classic test for PMP exam
  • My fav, Drupal's example of "With great power comes great responsibility"

:)