Why run UpdatePostUploadsSecureStatus even when secure uploads is disabled?

saulshanabrook · February 18, 2023, 4:45pm

Thanks for your work on allowing different S3 backends! I am using minio and it is working great…

The only slight issue I have is that the UpdatePostUploadsSecureStatus job is failing because Minio does not support ACLs:

github.com/minio/minio

Support Object ACLs

opened 02:26PM - 06 Sep 19 UTC

closed 03:58AM - 07 Sep 19 UTC

rfornea

community wont fix triage

I've read through a few github issues and the minio docs, and it seems you don't… support setting object ACLs (nor intend to). You claim to be an S3 compatible server, but if you aren't supporting this feature, that's not really true IMO. Regardless of whether or not AWS recommends using ACLs, they nevertheless still support them and it is a good enough solution for plenty of people's usage. Not supporting object ACLs prevents Minio from being a drop-in replacement for S3. Someone who wanted to switch to Minio would have to start adding prefixes to everything which could break their existing naming scheme for objects. This is a problem if they wanted to be able to support objects stored in S3 and in Minio, and if they already had pre-existing data in S3 which used Object ACLs. This is the situation I personally am in. I really think you should reconsider this.

Is there a way I can disable this job from running?

Falco · February 18, 2023, 4:51pm

Have you enabled SiteSetting.secure_uploads ?

saulshanabrook · February 19, 2023, 1:08pm

Nope, I have not.

EDIT: I did import some emails which had images embedded I’m them. I am reading the Secure uploads in emails docs, is that saying that all uploads from emails will be “secure”?

Falco · February 19, 2023, 1:35pm

Why do we schedule this job for every upload on every post even when the setting is disabled @martin ?

martin · February 19, 2023, 11:38pm

Good question – this is called in two places. PostCreator:

github.com

discourse/discourse/blob/3c57db5c633297f076c9e2ed09beff41e4133573/lib/post_creator.rb#L405-L407


      
          def update_uploads_secure_status
            @post.update_uploads_secure_status(source: "post creator") if SiteSetting.secure_uploads?
          end

And PostRevisor:

github.com

discourse/discourse/blob/3c57db5c633297f076c9e2ed09beff41e4133573/lib/post_revisor.rb#L250-L252


      
          # This must be done before post_process_post, because that uses
          # post upload security status to cook URLs.
          @post.update_uploads_secure_status(source: "post revisor")

However as you can see I neglected to do the SiteSetting.secure_uploads? check in the latter…I will make a PR to fix this and just move the check to post.update_uploads_secure_status.

Edit: PR is here, hopefully should merge today FIX: Do not enqueue UpdatePostUploadsSecureStatus unnecessarily by martin-brennan · Pull Request #20366 · discourse/discourse · GitHub

Topic		Replies	Views
Can not edit topics with picture with S3 backend bug	11	974	July 12, 2021
Minio: A header you provided implies S3 functionality that is not implemented support	20	8074	April 5, 2021
Secure Uploads announcements secure-uploads	41	14563	July 25, 2023
Error in rebuilding using minio as object store support	9	2372	October 4, 2022
Has anyone succeed in use minio for upload files? support	7	1069	November 12, 2019

Why run UpdatePostUploadsSecureStatus even when secure uploads is disabled?

Related Topics