People: profile picture should be editable by their author (not by "No one")
Open, HighPublic
Actions

Assigned To

Authored By

	albertoleoncio
	May 20 2023, 14:23

Description

Steps:

Visit Profile → Manage → Edit Profile Picture
Upload a Picture
Turn back to Profile → Manage
Visit the filename of that new Picture (but also any old picture) (pictures are mentioned from the right sidebar)

What happens:

File is not editable: «No one can take this action»
File cannot be deleted: «No one can take this action»
(File can only be detached)

What should happen instead:

File should be Editable by the Author (to update its quality for example)
File should be Deletable by the Author (to fix a huge mistake for example)

So, in short, profile images are shown here forever, and cannot be deleted by their users:

Screenshot_20240505_151934.png (706×1 px, 172 KB)

What happens

*************************** 42. row ***************************
              id: 43
            phid: PHID-FILE-wzbdm6d76sa3bqpvgf23
            name: Screenshot_20240204_101538.png
        mimeType: image/png
        byteSize: 389182
   storageEngine: blob
   storageFormat: raw
   storageHandle: 31
     dateCreated: 1714856499
    dateModified: 1714856499
      authorPHID: PHID-USER-mutepdoozum2ey2pm7of
       secretKey: jkdb3d3xoo5xyvmo2dhd
     contentHash: fe2a9511cb1ab4ac2eedff6e32aff01fd96778e680005f5741e8b9c2758f81ad
        metadata: {"storage":[],"width":1920,"height":1080,"canCDN":1}
             ttl: NULL
isExplicitUpload: 0
         mailKey: fqgie335f3qu2jc3t2es
      viewPolicy: users
       isPartial: 0
      builtinKey: NULL
       isDeleted: 0
*************************** 43. row ***************************
              id: 44
            phid: PHID-FILE-anvsulbhvqsxguk65tid
            name: profile
        mimeType: image/png
        byteSize: 59341
   storageEngine: blob
   storageFormat: raw
   storageHandle: 32
     dateCreated: 1714856499
    dateModified: 1714856499
      authorPHID: NULL                                                                ← ← ← ← ←
       secretKey: dzvuzmanku2aayk3x36v
     contentHash: 9de9a20b98478c40fa94b52c2aea3e4db5736994bb88766ce3021f0caa4b49d6
        metadata: {"storage":[],"width":400,"height":400,"canCDN":1,"profile":true}
             ttl: NULL
isExplicitUpload: 0
         mailKey: du26qw5527bwxnttww4k
      viewPolicy: users
       isPartial: 0
      builtinKey: NULL
       isDeleted: 0

SELECT * FROM file_transformedfile WHERE originalPHID = 'PHID-FILE-wzbdm6d76sa3bqpvgf23';
Empty set (0,001 sec)

Workaround

Having shell access in the Phorge server and run the destroy workflow from the phorge/ directory:

./bin/remove destroy F47

P.S. For historical details about this beautiful workflow, see here:

https://we.phorge.it/rP985eb26c7e32cdc19536769def6ab037645100fb

Revisions and Commits

rP Phorge
	Needs Review	D25622 Transformed Pictures: make them non-orphan

Related Objects
Search...

		Status	Assigned	Task
		Open	valerio.bozzolan	T15407 People: profile picture should be editable by their author (not by "No one")
		Open	valerio.bozzolan	T15814 Files: reduce number of orphan transformed files

Event Timeline

albertoleoncio created this task.May 20 2023, 14:23

Herald added subscribers: Cigaryno, Matthew, chris and 2 others. · View Herald TranscriptMay 20 2023, 14:23

valerio.bozzolan awarded a token.May 20 2023, 14:23

Thanks for this Task

I have not a clear big picture of this situation but probably most of the business logic that could be debugged to understand that is here:

https://we.phorge.it/source/phorge/browse/master/src/applications/people/controller/PhabricatorPeopleProfilePictureController.php

The text says "The user who uploaded a file can always view and edit it.". I checked the DB, and the author field for the relevant file is null.
That implies that this upload code is bypassing some security checks...

(Raising to "HIGH", until we figure out if there's a security concern).

waldyrious subscribed.Nov 14 2023, 14:42

valerio.bozzolan lowered the priority of this task from High to Normal.Nov 20 2023, 16:58

valerio.bozzolan added a project: User-valerio.bozzolan.

Indeed reported from a Wikimedia user about Wikimedia Phabricator.

Even if I'm unaware of any downstream task.

I do not (yet?) understand the use case of this task.

Why should thumbnails be manually editable instead of automatically generated when they are just small versions of some other files?

Visit the filename of that new Picture (but also any old picture) (pictures are mentioned from the right sidebar)

I cannot follow. Which page are we on? It does not seem to be Profile → Manage. What are clear steps to reproduce?

File should be Editable by the Author (to update its quality for example)

Why if you could just upload a new image with improved quality? (And why care about some thumbnail of your file in this case?)

File should be Deletable by the Author (to fix a huge mistake for example)

Files are manually deletable by the author. Thumbnails of such files should not be manually deleteable - code should handle that, in my opinion.

Edit: Uh wait, maybe "user avatar" or "user profile picture" was meant instead of "thumbnail"?

aklapper removed a project: Affects-Wikimedia.Thu, May 2, 16:36

valerio.bozzolan renamed this task from People: uploaded thumbnails should be editable by their author (not by "No one") to People: profile picture should be editable by their author (not by "No one").Thu, May 2, 17:34

(Oh sorry avivey, I have not noticed your priority action - I agree)

After a small database inspection, it seems that uploading the picture causes:

✅ the original file, uploaded with the correct authorPHID
the transform, orphan, and without authorPHID

valerio.bozzolan updated the task description. (Show Details)Sun, May 5, 13:20

valerio.bozzolan added a revision: D25622: Transformed Pictures: make them non-orphan.Mon, May 6, 16:10

People: profile picture should be editable by their author (not by "No one")Open, HighPublicActions