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

Restrict media privacy until a referencing page is published #46

Merged
merged 68 commits into from
Jan 20, 2025

Conversation

ababic
Copy link
Contributor

@ababic ababic commented Nov 29, 2024

What is the context of this PR?

This is a follow-up to #42 that focusses on addressing solely public/private status of media, without attempting to tie objects to pages to aid with permission policy customisation.

It's fully functional and has lots of tests (which are worth checking out to get a feel for the intended behaviour).

A brief summary:

  • All media should be 'private' by default (regardless of upload route)
  • Media is only made public when used within the content field of a page, and the revision featuring the media is published.
  • If the referencing page is unpublished, any media referenced solely by that page is made private again (anything referenced by other 'live' pages remains public)

When a media item's privacy changes:

  • Where the storage backend supports it (S3), attempts are made to set file-level permissions to reflect the new privacy value.
  • Where permissions for all a media item's file are updated successfully, the file_permissions_last_set timestamp is updated for the media item, and the object's file_permissions_are_outdated() method will return False.
  • If any errors occurred during the update process, file_permissions_last_set is not updated, and the file_permissions_are_outdated() method will return True.
  • Where front-end cache invalidation is configured, purge requests will also be triggered for all relevant 'serve' or direct file URLs

How to review

Testing media privacy for a draft page

  1. In Wagtail, create a new Information Page
  2. Enter "Media upload test" for the title and "Test" for the summary
  3. Add an "Image" block to the content, and use the chooser modal to upload a new image
  4. Add a "Documents" block to the content, use the sub-block options to add a document, and use the chooser modal for that item to upload a new document
  5. Open the preview pane for the page and inspect the HTML where the blocks are rendered.
  6. The src value of the image rendition should look something like /images/{secure key}/2/original/{image filename}, which means it's being served by the 'serve' view.
  7. The href value of the document link should look something like /documents/
  8. Open the document link URL and image src URL in new tabs (they should still work).
  9. Sign out of Wagtail.
  10. Refresh the the tabs opened in step 9 - You should now get a "403 Forbidden" response for both.
  11. In a new tab, Sign back in to Wagtail.
  12. Refresh the tabs once more, and they should work again

Testing media privacy after a page has been published

  1. Publish the "Media upload test" page created in the above steps
  2. In the green success message on the next page, click the "View live" button to view the live/public-facing version of the page.
  3. Inspect the HTML for the image. The src value should have changed to something like: media/images/{image filename}.fill-446x390.format-webp-.original.png.
  4. Access Wagtail again edit the "Media upload test" page once more.
  5. Open the preview pane and inspect the HTML for the image there too. The src value should look the same as it did on the live version.
  6. At the bottom of the page, click the arrow next to the "Save draft" button to view the additional options, and click "Unpublish page".
  7. On the confirmation screen, click "Yes, unpublish it" to confirm the action.
  8. Edit the page again in Wagtail and look again at the preview pane.
  9. Inspect the HTML for the image once more. The src value should now again look like: /images/{secure key}/2/original/{image filename}.

Testing image privacy once a draft page has been published

  1. In Wagtail, edit the draft "Image upload test" page again, and publish it.
  2. In the green success message on the next page, click the "View live" button to view the live/public-facing version of the page.
  3. Inspect the HTML for the image. The src value should look something like: media/images/{image filename}.fill-446x390.format-webp-.original.png.
  4. Access Wagtail again edit the "Image upload test" page once more.
  5. Open the preview pane and inspect the HTML for the image there too. The src value should look the same as it did on the live version.
  6. At the bottom of the page, click the arrow next to the "Save draft" button to view the additional options, and click "Unpublish page".
  7. On the confirmation screen, click "Yes, unpublish it" to confirm the action.
  8. Edit the page again in Wagtail and look again at the preview pane.
  9. Inspect the HTML for the image once more. The src value should now again look like: /images/{secure key}/2/original/{image filename}.

Follow-up Actions

  • Update documentation
  • Ensure PRIVATE_MEDIA_BULK_UPDATE_MAX_WORKERS is set appropriately for all target environments
  • Ensure python manage.py retry_file_permission_set_attempts is set to run on a cron every 10 minutes in all target environments

@ababic ababic requested a review from a team as a code owner November 29, 2024 13:52
@ababic ababic marked this pull request as draft November 29, 2024 13:52
@ababic ababic force-pushed the feature/private-media branch 21 times, most recently from 3ba9bfb to b856066 Compare December 2, 2024 17:11
@ababic ababic marked this pull request as ready for review December 2, 2024 17:16
@ababic ababic force-pushed the feature/private-media branch from b856066 to 9a52eb5 Compare December 3, 2024 07:14
@ababic ababic force-pushed the feature/private-media branch from 74dae78 to b581823 Compare December 13, 2024 13:15
@ababic ababic force-pushed the feature/private-media branch from 19376c0 to 5198943 Compare December 13, 2024 13:23
@ababic ababic marked this pull request as draft December 13, 2024 14:02
@ababic ababic force-pushed the feature/private-media branch from 021a9dd to faefcac Compare December 13, 2024 15:37
…eMixin.get_privacy_controlled_serve_urls() to return an empty iterable
* main:
  Wire in the quote block (#59)
  Introduce Megalinter (#40)
  Add CI job to add coverage to GH Action summary (#58)

# Conflicts:
#	docs/index.md
#	poetry.lock
@ababic ababic marked this pull request as ready for review December 13, 2024 15:52
ababic and others added 4 commits December 13, 2024 15:52
* main:
  Remove the analysis app (#71)
  Fix to related content on the release page (#72)
  Fix for  equations and embeds rendering on Methodology page(#74)
  Update readme with extra steps needed for pre-commit (#67)
  Remove the analysis models (#70)
  Add an articles app for statistical articles (#69)
  Methodology page (#57)
  Ensure the default DB is **always** used for writes (#68)
  Styling for the release page plus some refactoring (#55)
  Bundles: exclude Release Calendar pages with a date in the past (#66)
  Disable markup from megalinter (#64)
  Ignore vscode workspace settings files (#65)
* main:
  Update the embed block to be a custom video embed that uses the design system video embed template (#62)
  Django Migration Linter Integration (#43)
  Tidy up the info/warning panel and fix title/no title output (#73)
  Introduce Functional Tests using Playwright and Behave #31

# Conflicts:
#	poetry.lock
#	pyproject.toml
…ns as well as instances of botocore.ClientError
Copy link
Contributor

@MebinAbraham MebinAbraham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work @ababic 🎉

@ababic ababic force-pushed the feature/private-media branch from e47a6f3 to ce5bb63 Compare January 16, 2025 13:10
@ababic ababic merged commit 59d7a1e into main Jan 20, 2025
9 checks passed
@ababic ababic deleted the feature/private-media branch January 20, 2025 16:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants