Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FIX: Change secure media to encompass attachments as well #9271

Merged
merged 9 commits into from Mar 25, 2020

Conversation

@martin-brennan
Copy link
Contributor

martin-brennan commented Mar 25, 2020

If the “secure media” site setting is enabled then ALL files uploaded to Discourse (images, video, audio, pdf, txt, zip etc. etc.) will follow the secure media rules. The “prevent anons from downloading files” setting will no longer have any bearing on upload security. Basically, the feature will more appropriately be called “secure uploads” instead of “secure media”.

This is being done because there are communities out there that would like all attachments and media to be secure based on category rules but still allow anonymous users to download attachments in public places, which is not possible in the current arrangement.

* when the prevent_anon_from_downloading_files setting was
  changed we were enqueueing a job that ran through ALL
  uploads and did update_secure_status. this is unnecessary
  because prevent_anon has no bearing on secure status.
* the only job that needs to be run related to uploads is
  the uploads:secure_upload_analyse_and_update job, which
  must be run manually when secure media is changed
* also get rid of part of s3 job that was marking uploads
  with a private acl if prevent_anon_from_downloading_files was
  enabled, because this does not happen anywhere else in the app
* also do the same if the prevent anons setting is _not_ enabled
  and the user is anon, because we still don't want anons to get
  their mitts on uploads where we dont have to post to check if
  the post is public or not
@SamSaffron

This comment has been minimized.

Copy link
Member

SamSaffron commented Mar 25, 2020

huge fan of the simplification here, is this ready to merge?

@martin-brennan martin-brennan merged commit 097851c into master Mar 25, 2020
7 checks passed
7 checks passed
PLUGINS-BACKEND
Details
CORE-BACKEND
Details
PLUGINS-FRONTEND
Details
CORE-FRONTEND
Details
PLUGINS-LINT
Details
CORE-LINT
Details
license/cla Contributor License Agreement is signed.
Details
@martin-brennan martin-brennan deleted the issue/make-secure-media-encompass-attachments branch Mar 25, 2020
@discoursebot

This comment has been minimized.

Copy link

discoursebot commented Mar 26, 2020

This pull request has been mentioned on Discourse Meta. There might be relevant details there:

https://meta.discourse.org/t/secure-media-uploads/140017/51

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.