Secure Media Uploads breaks Category Logos

Bug
1

The “Secure Media” + “Enable S3 Uploads” options seems to break Category Logos.

I recently discovered that uploaded documents could be viewed/downloaded via direct url link by users who didn’t have access to the private post where the document was uploaded. I understood that S3 Uploads had to first be enabled to use the Secure Media feature, so I enabled that first and manually copied the uploads from the Discourse server to the S3 bucket. I then followed this post to enable Secure Media:

After following that procedure, I had to do a discourse remap to change the /uploads/default/ to //bucketname-etc-etc/. After the remap, I did a rake posts:rebake and everything looked good for the most part. All other uploads (documents and images in posts, site branding images, etc.) still displayed and linked correctly after enabling Secure Media and running the appropriate rake and remap tasks. I did have to reupload user avatars and group logos, but once those were reuploaded, they all displayed correctly and had the new short urls and secure-media-uploads urls as expected.

I did see that the Category Logo Images no longer displayed. While checking with my browser’s developer mode, I noticed that when reuploading a Category Logo Image, the correct secure-media-uploads url was created, displaying the image correctly. However, once I would click “Save Category”, the category page would refresh and incorrectly try to map directly to the AWS S3 unsigned url for the image, preventing the image from being displayed:

image
image1596×513 56 KB

Every page in Discourse that displays a Category Logo Image tries to map the image directly to the AWS S3 unsigned url instead of the secure-media-uploads url.

As a workaround, I’ve had to tediously visit each Category Logo Image object in AWS S3 and change the permissions to Public Read.

I’ve confirmed that this behavior exists in Chrome, Firefox, and Microsoft Edge browsers. I’ve tried posts:rebake and uploads:secure_upload_analyse_and_update rake tasks, but those don’t seem to do anything with the Category Logo. Is there perhaps another task that will fix these incorrectly mapped Category Logo Image urls? Or is this in fact by design, requiring all S3 upload objects to be public read, except those which are secure (private posts)?

3 Likes
2

Thanks for raising this, I wasn’t aware that Category Logos existed :sweat: I’ll take a look at this this week and try get a fix for you here.

6 Likes
4

Awesome! Thanks a lot @martin!

3 Likes
5

I think the main issue here is that the secure_upload_analyse_and_update is a little heavy-handed when it comes to determining what should and shouldn’t be secure, because the UploadSecurity class (https://github.com/discourse/discourse/blob/master/lib/upload_security.rb) checks where these public type of uploads (e.g. avatars, category logos etc.) should be secure or not at upload time, and this type is not present when checking again in the rake task.

I have a task tracked internally to improve this by checking all the places an upload can live at the time of this security check but it is a way off, and is part of a larger plan to have upload references stored in a neater way.

Anyway, I have a PR building now to fix this issue by making category logos and backgrounds public types when considering whether they should be secure or not. Once this is merged and you have updated you will just have to re-upload the category images and things should be fine.

https://github.com/discourse/discourse/pull/10513

4 Likes