Problem/Motivation

If a user has "Administer advertisements" permissions, only the advertisement entities should be manageable, not the advertisement types. Currently /admin/structure/ad-content is accessible then.

Steps to reproduce

See above

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Issue fork ad-3581479

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

anybody created an issue. See original summary.

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Status: Active » Needs review

Implemented this blindly, please try and please add a simple test on the path admin/structure/ad-content to be only accessible with this permission

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

lrwebks opened merge request !51

lrwebks’s picture

lrwebks commented
Assigned: lrwebks » Unassigned

LGTM and works in both tests and manual testing. PHPUnit failure is unrelated, so I'd say after my changes are reviewed, this is ready to merge!

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Assigned: Unassigned » lrwebks
Status: Needs review » Needs work

Code LGTM but test doesn't LGTM. Regular users ALSO should not be able to administer ad types, but we need three types of users here:

  • adminUser who also may administer ad types
  • adManagerUser who may only manage ads but not types
  • user regular user
lrwebks’s picture

lrwebks commented
Assigned: lrwebks » Unassigned
Status: Needs work » Needs review
anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented

Perfect, please add the further permission tests to be safe as discussed.

lrwebks’s picture

lrwebks commented
Assigned: Unassigned » lrwebks
Status: Needs review » Needs work
lrwebks’s picture

lrwebks commented

While currently creating the general permission tests, I noticed that we have a similar problematic situation with the administer ads permission also allowing access to manage the ad placements. I will add a special permission for administering them too.

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented

Perfect!

lrwebks’s picture

lrwebks commented
Assigned: lrwebks » Unassigned
Status: Needs work » Needs review

  • anybody committed bd85d629 on 11.x authored by lrwebks 
    feat: #3581479 Administer advertisements permission should not grant...
anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Status: Needs review » Reviewed & tested by the community
anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Status: Reviewed & tested by the community » Fixed

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

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