Steps to reproduce

  1. Navigate to Media settings ('/admin/config/media/media-settings')
  2. Enable Standalone media URL
  3. Now, view any media from the front end while ensuring that you are logged out.
  4. As a result, the "View" and "Revision" tabs are visible to logged-out or anonymous users.

Issue fork drupal-3411837

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

uditrawat created an issue. See original summary.

cilefen’s picture

cilefen commented
catch’s picture

catch
he/him
Primary language English
commented

This was discussed within the security team and decided this could be fixed publicly.

catch’s picture

catch
he/him
Primary language English
commented
Issue tags: +Patch release target

longwave made their first commit to this issue’s fork.

longwave opened merge request !6013

longwave credited greggles.

longwave credited larowlan.

longwave credited marcoscano.

longwave credited smustgrave.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Active » Needs review
longwave’s picture

longwave
he/him
Primary language English
Location UK
commented

Uploaded the patch by @larowlan to an MR.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented

Saving issue credits based on the triage done over on security.drupal.org.

catch’s picture

catch
he/him
Primary language English
commented
Priority: Major » Critical

One question on the MR.

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work

seems to be failing a number of rest tests. Should they be updated or the solution relooked at?

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Assigned: Unassigned » larowlan

working on fails

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Assigned: larowlan » Unassigned
Status: Needs work » Needs review

Addressed failure, which was also asserting that revision info was available to non admin users 🙃

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Title: Media revision UI is accessible to anonymous users. » Media revision listing is accessible to anonymous users.

Clarifying title, because whilst they can view the list and published revisions, they can't interact with the revision UI (e.g. revert revisions, delete revisions etc)

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Issue tags: +Needs subsystem maintainer review, +Security improvements

I think this should be reviewed by >=1 media system maintainer?

catch’s picture

catch
he/him
Primary language English
commented
Status: Needs review » Reviewed & tested by the community

I was concerned that we were opening up a new information disclosure bug by removing the entity access checks with the original fix here, turns out we were, but @larowlan added extra test coverage and restored some of that logic (in a place that actually works correctly).

For me this is RTBC now, I've also pinged the media system maintainers for a review in slack per #19.

marcoscano’s picture

marcoscano
Primary language Portuguese, Brazil
Location Barcelona, Spain
commented

👍 looks good to me, thanks!

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed
Issue tags: -Patch release target, -Needs subsystem maintainer review

Committed and pushed 8f3f374984 to 11.x and 8a4467d55e to 10.2.x. Thanks!

  • alexpott committed 8a4467d5 on 10.2.x 
    Issue #3411837 by larowlan, longwave, catch, uditrawat, marcoscano,...

  • alexpott committed 8f3f3749 on 11.x 
    Issue #3411837 by larowlan, longwave, catch, uditrawat, marcoscano,...
longwave’s picture

longwave
he/him
Primary language English
Location UK
commented

Nice work, was about to commit this but @alexpott already had!

larowlan closed merge request !6013

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.