Problem/Motivation

The route for the Flipbook PDF style settings form uses the generic "access content" permission. Since anonymous users commonly have this permission, the configuration form at /admin/config/choosepdfstyle can be accessed by anonymous users.

Steps to reproduce

1. Enable the flipbook module.
2. Ensure the anonymous role has "access content" permission.
3. Log out or open an incognito/private browser window.
4. Visit /admin/config/choosepdfstyle.

Expected

Anonymous users can access the "FlipBook Pdf Style Settings" form.

Actual

Anonymous users should receive access denied. This settings form should require an administrative permission.

Proposed resolution

In flipbook.routing.yml

flipbook.chooseform:
    path: '/admin/config/choosepdfstyle'
    defaults:
      _form: 'Drupal\flipbook\Form\ChoosePdfStyleForm'
      _title: 'FlipBook Pdf Style Settings'
    requirements:
      _permission: 'access content'

Change the permission to:

_permission: 'administer flipbook entity'

Issue fork flipbook-3607001

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

dkmishra created an issue. See original summary.

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

mukeshaddweb opened merge request !13

mukeshaddweb’s picture

mukeshaddweb commented
Status: Active » Needs review

  • 79e9cd35 committed on 11.0.1.x 
    Merge d12-compat: Drupal 12 support + D11 bug fixes (#3607001, #3606988...

  • 97a61c57 committed on 11.0.1.x 
    Issue #3607001: Restrict Choose PDF style form to 'administer flipbook...
dkmishra’s picture

dkmishra commented

Thanks for getting this fixed so quickly — I pulled the latest 11.0.1.x and confirmed the permission restriction is working correctly now.
I did want to flag the process here for future reference: mukeshaddweb had already opened MR !13 with essentially the same fix, but it looks like the change was committed directly (see 97a61c57) rather than merging the MR. A couple of things that affects:

Contribution credit — since the fix went in as a direct commit rather than through the MR, neither the original diagnosis/proposed resolution nor mukeshaddweb's patch appear to be credited. Would it be possible to add credit for both of us on this issue, since the analysis and fix originated here?
Issue status — the issue can probably now be moved to "Fixed" since the commit is in and verified, and MR !13 can be closed/referenced.

For future issues, merging via the MR (even with edits) keeps the CI/testing trail intact and makes sure credit tracking works as intended. Appreciate the quick turnaround on the actual fix either way — just wanted to raise the process point for next time.