Document file upload random uuid security
This documents the current state of file uploads regarding random UUID and security. Fixes #3569 (closed)
Thanks to @DouweM for the language.
Merge request reports
Activity
Super clear. Very good @dblessing
Yes, thanks Drew :)
Aaaaand conflicts.
Edited by Achilleas Pipinellis@axil No conflicts here. Are you able to merge?
mentioned in commit 66706570
No problem. Thanks @axil
- doc/security/user_file_uploads.md 0 → 100644
1 # User File Uploads 2 3 Images attached to issues, merge requests or comments do not require authentication 4 to be viewed if someone knows the direct URL. This direct URL contains a random 5 32-character ID that prevents unauthorized people from guessing the URL to an 6 image containing sensitive information. We don't enable authentication because 7 these images need to be visible in the body of notification emails, which are 8 often read from email clients that are not authenticated with GitLab, like 9 Outlook, Apple Mail, or the Mail app on your mobile device. 10 11 Note that non-image attachments do require authentication to be viewed. @dblessing Why do non-image attachments work differently?
@sytses I'm guessing the original intent was to enable images to be displayed inline in email notifications. Other attachments will just generate a link in the email so we can prompt users for authentication when they follow the link.
@dblessing Thanks!
@dzaporozhets Does the above ring a bell?
@dblessing correct.
@sytses
When Douwe and me implemented attachment authentication a while ago, we accidentially broke all images in emails because we required authorization.
We decided that for images the 32-character ID is secure enough.See this MR for more infos: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/373
@sytses I think its ok for images. Otherwise we have a problem with displaying images in emails. I think its normal practice to secure images by hash. And I do not expect image to store some super sensitive information. Especially now when we have it documented.
Edited by username-removed-444
mentioned in commit ad5949b6
