add support for deleting previous avatar in set_avatar() with SSO OIDC logins#14626
add support for deleting previous avatar in set_avatar() with SSO OIDC logins#14626ashfame wants to merge 4 commits intomatrix-org:developfrom
Conversation
ashfame
changed the title
reivilibre
left a comment
reivilibre
left a comment
There was a problem hiding this comment.
unfortunately I think this is a bit flawed as it stands.
It would be useful to understand your motivation for doing this to give suggestions on how to remedy this properly, though it's not necessarily required that we delete old avatars — the 'normal' process of uploading an avatar certainly doesn't and this means that a person's old messages still have a visible avatar.
|
|
||
| # delete previous avatar, if one existed | ||
| if delete_previous_avatar and existing_media_id is not None: | ||
| await self._media_repo.delete_local_media_ids(existing_media_id) |
There was a problem hiding this comment.
note that a user can typically set their avatar_url to whatever they like, including another user's piece of media or another user's avatar. Allowing them to delete a piece of media this way is therefore dangerous.
(It also means their previous messages will be left without an avatar, which might be unwanted depending on the circumstances.)
I'm not sure what the best way to approach this is; I suppose it will depend on the motivation here.
| ) | ||
|
|
||
| # delete previous avatar, if one existed | ||
| if delete_previous_avatar and existing_media_id is not None: |
There was a problem hiding this comment.
(existing_media_id seems to be the 'local' part of the media ID as far as I can tell, but we haven't checked whether it is media for our server name.)
|
@reivilibre Thanks for taking a look! I didn't realize the avatars can be set to media not owned by the user in general. My motivation was to just cleanup SSO avatars' media files that are no longer used. But I see that's harder to do, without a way of knowing if that media file is not used elsewhere. Would you say we close this then, considering usual avatars uploaded by the users are also never cleaned up? And there isn't a desire to do so i.e. this isn't seen as an attack vector for exhausting disk space. |
2 participants
Coming from #13917
This PR changes the SSO OIDC avatar related functionality to delete the previous avatar's media file when a new avatar is set for the user.