Moodle

No easy way to remove pictures of deleted users

Details

  • Type: Bug Bug
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: 1.9.3
  • Fix Version/s: 1.8.8, 1.9.4
  • Component/s: Administration
  • Labels:
    None

Description

If a user account has been deleted, their picture remains attached to forum posts. However, if the admin decides that they don't like the picture (say because it is offensive) there is now no way to delete or edit it.

Why do profiles of deleted users simply not remain editable to admins? There could even be a simple (and useful) undelete function!

Issue Links

This issue has a non-specific relationship to:
MDL-24149 Display pictures of deleted users [patch] Minor Open

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Howard Miller added a comment -

It's worse than that. If you have a link to the picture via pix.php then it's viewable by anyone. Surely, pix.php should check if the picture associated with a user is "deleted" when accessed by guest users?

Show
Howard Miller added a comment - It's worse than that. If you have a link to the picture via pix.php then it's viewable by anyone. Surely, pix.php should check if the picture associated with a user is "deleted" when accessed by guest users?
Hide
Permalink
Eloy Lafuente (stronk7) added a comment -

Assigning to Petr. I agree with Howard about pictures of deleted users not being accessible anymore:

1) at least from guests.
2) perhaps from everybody.

Not sure if that implies deleting the picture or just detecting it and sending the "default" image for deleted users.

Ciao

Show
Eloy Lafuente (stronk7) added a comment - Assigning to Petr. I agree with Howard about pictures of deleted users not being accessible anymore: 1) at least from guests. 2) perhaps from everybody. Not sure if that implies deleting the picture or just detecting it and sending the "default" image for deleted users. Ciao
Hide
Permalink
Anthony Borrow added a comment -

I wonder if instead of actually deleting the picture instead we just do a check and if it is a deleted picture then use the generic smiley face or perhaps even a grayed out smiley face to indicate that it is a deleted user. It could be as simple as using grayscale images (which I'll attach) or overlaying 'Deleted User' or something over the image. I think our general approach of not actually removing data is good as it does leave the door open should we want at some point to implement a restore/undelete user option via the UI. Peace - Anthony

Show
Anthony Borrow added a comment - I wonder if instead of actually deleting the picture instead we just do a check and if it is a deleted picture then use the generic smiley face or perhaps even a grayed out smiley face to indicate that it is a deleted user. It could be as simple as using grayscale images (which I'll attach) or overlaying 'Deleted User' or something over the image. I think our general approach of not actually removing data is good as it does leave the door open should we want at some point to implement a restore/undelete user option via the UI. Peace - Anthony
Hide
Permalink
Anthony Borrow added a comment -

ATW was asking about voting on this issue. I'm guessing that because it is a security issue that normal users cannot vote on it. Peace - Anthony

Show
Anthony Borrow added a comment - ATW was asking about voting on this issue. I'm guessing that because it is a security issue that normal users cannot vote on it. Peace - Anthony
Hide
Permalink
Anthony Borrow added a comment -

here are the grayscaled f1 and f2 files

Show
Anthony Borrow added a comment - here are the grayscaled f1 and f2 files
Hide
Permalink
Anthony Borrow added a comment -

Just a quick follow up, my idea would be to make a small modification to the print_user_picture function in /lib/weblib.php to check if the user is deleted and if so to use the deleted user image files.

Show
Anthony Borrow added a comment - Just a quick follow up, my idea would be to make a small modification to the print_user_picture function in /lib/weblib.php to check if the user is deleted and if so to use the deleted user image files.
Hide
Permalink
Petr Škoda (skodak) added a comment -

I have added a test into user/pix.php to validate that user is not deleted, if yes default image is used.
Changing print_user_picture might not be enough because the images are also stored in emails

There is no "undelete user" feature planned (either manual or automatic) and I would strongly oppose it, if you really want it you can use db editor or some custom script.
Instead we should have easy to use "user suspend" and proper user delete+purge - we must clean up the old data because it is becoming a real performance problem.

I am planning to rework the profile image handling in 2.0 - I suppose it will use the same trick as SCORM to work around the caching problems, I agree the gray images is a good idea (+1 for HEAD).

Show
Petr Škoda (skodak) added a comment - I have added a test into user/pix.php to validate that user is not deleted, if yes default image is used. Changing print_user_picture might not be enough because the images are also stored in emails There is no "undelete user" feature planned (either manual or automatic) and I would strongly oppose it, if you really want it you can use db editor or some custom script. Instead we should have easy to use "user suspend" and proper user delete+purge - we must clean up the old data because it is becoming a real performance problem. I am planning to rework the profile image handling in 2.0 - I suppose it will use the same trick as SCORM to work around the caching problems, I agree the gray images is a good idea (+1 for HEAD).
Hide
Permalink
Howard Miller added a comment -

I agree with you, Petr. However, the underlying problem is that delete was never actually delete at all. It creates an expectation that is not true. Simply renaming it to suspend would have helped.

Show
Howard Miller added a comment - I agree with you, Petr. However, the underlying problem is that delete was never actually delete at all. It creates an expectation that is not true. Simply renaming it to suspend would have helped.
Hide
Permalink
Petr Škoda (skodak) added a comment -

well, there is a big difference between delete and suspend
1/ delete should delete stuff that is not needed like assignment submissions (no more user access expected) + purge should remove all traces of users including posts, etc.
2/ suspend should just prevent logins while keeping ALL user data and preventing creation of account with the same email/username

I believe that we need enrolment suspend and proper delete too. It is very similar to this problem imho.

Show
Petr Škoda (skodak) added a comment - well, there is a big difference between delete and suspend 1/ delete should delete stuff that is not needed like assignment submissions (no more user access expected) + purge should remove all traces of users including posts, etc. 2/ suspend should just prevent logins while keeping ALL user data and preventing creation of account with the same email/username I believe that we need enrolment suspend and proper delete too. It is very similar to this problem imho.
Hide
Permalink
Helen Foster added a comment -

Thanks for reporting this issue Howard, thanks for everyone's comments, and thanks for fixing it Petr

Show
Helen Foster added a comment - Thanks for reporting this issue Howard, thanks for everyone's comments, and thanks for fixing it Petr
Hide
Permalink
Martin Dougiamas added a comment -

I'm removing security status to open this up.

Show
Martin Dougiamas added a comment - I'm removing security status to open this up.

Dates

  • Created:
    Updated:
    Resolved:
Atlassian JIRA (v4.4.5#665-sha1:422aead) | Report a problem
Powered by a free Atlassian JIRA open source license for Moodle. Try JIRA - bug tracking software for your team.