Skip to:

Opened 8 years ago

Closed 8 years ago

#4915 closed defect (bug) (fixed)

In 1.7, a user cannot delete his account anymore.

Reported by: imath Owned by:
Milestone: 1.7 Priority: normal
Severity: major Version: 1.7
Component: Members Keywords: needs-patch dev-feedback



If a member (regular user) goes in its setting page checked "i understand..." and then submit the form to delete his account, then nothing happens, the page redirect to itself with no message. The problem seems to be located in the function bp_core_delete_account when checking bp_current_user_can( 'delete_users' ) in bp-members-functions.php.

Attachments (2)

4415.patch (2.3 KB) - added by boonebgorges 8 years ago.
4915.patch (989 bytes) - added by boonebgorges 8 years ago.

Download all attachments as: .zip

Change History (8)

#1 @r-a-y
8 years ago

  • Keywords needs-patch dev-feedback added

It probably has to do with these changesets: r6855, r6861

Looks like account deletion rights have been raised to super admins or users with the 'delete_users' role.

If account deletion is enabled, one should be able to delete their own account.

Moving to 1.7.

#2 @r-a-y
8 years ago

  • Milestone changed from Awaiting Review to 1.7

#3 @boonebgorges
8 years ago

Confirmed. This is a regression introduced by r6855.

The way that the permissions check worked before r6855 may have been overlax (I'm not privy to the details of the security issue that led to the check), but the fix prevents non-admins from deleting their own accounts.

Because I don't know exactly what the previous changeset was meant to fix (people deleting others' accounts? admins accidentally deleting users' accounts?) I'm not totally sure how to patch this. I'll attempt a patch that looks right to me, though I'd like feedback from johnjamesjacoby.

I wrote some automated tests for this issue. If anyone would like to take a minute and make sure I've covered all the relevant test cases (I have 5 at the moment), that'd be helpful.

#4 @boonebgorges
8 years ago

4915.patch introduces a change that makes my tests pass (thus fixing the immediate problem) but it is not beautiful. I suppose the "right" thing to do is map the delete_users cap onto non-admins when the user they're trying to delete is themselves, but that's a lot of machinery to add at this point.

8 years ago

8 years ago

#5 @boonebgorges
8 years ago

(Never mind 4415.patch, that's obviously the wrong file.)

#6 @johnjamesjacoby
8 years ago

  • Resolution set to fixed
  • Status changed from new to closed

(In [6891]) In bp_core_delete_account(), move capability checks into a logged-in user comparison block, to ensure the logged-in user is not trying to delete someone else's account. Props boonegorges. Fixes #4915.

Note: See TracTickets for help on using tickets.