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 download of images to users with edit_metadata permisssion where that user is not the image uploader #3873

Conversation

phillipbarron
Copy link
Contributor

@phillipbarron phillipbarron commented Sep 28, 2022

What does this change?

Download of images is restricted to users who have the edit_metadata permission. This is passed to the client in the image_response object to allow filtering.
Where a mix of downloadable and non-downloadable images are selected, a warning icon and accompanying tooltip indicate that a limited set of the selected imaged are downloadable. Those images with no user permission are filtered from the resulting zip archive.

Screenshot 2022-09-30 at 09 22 09

Config is now passed as an object rather that individual config items.

ES_JAVA_OPTS limiting memory usage to 1GB have been added to the docker environment configuration for Elastic search to address instability with the container during dev.

How should a reviewer test this change?

How can success be measured?

Who should look at this?

Tested? Documented?

  • locally by committer
  • locally by Guardian reviewer
  • on the Guardian's TEST environment
  • relevant documentation added or amended (if needed)

phillipbarron and others added 2 commits September 27, 2022 13:04
…the addition of new restrictDownload parameter.

Limiting downloads to the same conditions as can_crop. Where restrictDownload enabled, download will not be available for images where valid evaluate to false. Where multiple images are selected and some are available but other are not, this is highlighted in the UI through a warning icon and tooltip.
Added css to style position of download warning icon
Added userCanEdit property to image response which is mapped to the userCanEdit permission. This value is used to determine the set of downloadable images.

Co-authored-by: Ara Cho <ara.cho@guardian.co.uk>
@phillipbarron phillipbarron force-pushed the restrict-download-of-non-uploader-to-edit-metadata-permission branch from 9b54280 to e16df7b Compare September 28, 2022 14:30
Remove redundant var from controller
Using button for download rather than link for consistency
@phillipbarron phillipbarron changed the title Restrict download of non uploader to edit metadata permission Restrict download of images to users with edit_metadata permisssion where that user is not the image uploader Sep 30, 2022
@phillipbarron phillipbarron marked this pull request as ready for review September 30, 2022 08:29
@phillipbarron phillipbarron requested a review from a team as a code owner September 30, 2022 08:29
Added 1 GB memory limit to EElastic Docker compose to address instability
Updated tooltip to refer to one or many images
Removed redundant or condition for rendering low-res download button

// const isTooBig = blob.size > maxBlobSize;
const isTooBig = false;
// const isTooBig = blob.size > maxBlobSize;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can we remove this?

Copy link
Member

@andrew-nowak andrew-nowak left a comment

Choose a reason for hiding this comment

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

code looks good with one suggestion

kahuna/public/js/components/gr-downloader/gr-downloader.js Outdated Show resolved Hide resolved
@prout-bot
Copy link

Seen on cropper, collections, image-loader, metadata-editor, thrall, leases (merged by @phillipbarron 15 minutes and 31 seconds ago) Please check your changes!

@prout-bot
Copy link

Seen on usage, kahuna, media-api, auth (merged by @phillipbarron 16 minutes and 21 seconds ago) Please check your changes!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants