#3059984: Add new “Content Editor” role to Standard Profile added permissions to a role provided by the standard profile for modules that are not get installed. The system has no knowledge of these permissions at this point. If you go to admin/people/permissions none of the permissions are listed because they cannot possibly by returned by \Drupal::service('user.permissions')->getPermissions().
This is an issue because it blocks the commit of #2571235: [regression] Roles should depend on objects that are building the granted permissions - which is a security critical because roles are not cleaned up when a module is uninstalled meaning that if a module is re-installed users might suddenly get unexpected permissions.
$role = \Drupal\user\Entity\Role::load('content_editor');
array_diff($role->getPermissions(), array_keys(\Drupal::service('user.permissions')->getPermissions()));
This results in the following list of permissions:
=> [
5 => "access media overview",
6 => "access news feeds",
7 => "access printer-friendly version",
10 => "add content to books",
13 => "create book content",
14 => "create content translations",
15 => "create media",
16 => "create new books",
21 => "delete book revisions",
22 => "delete content translations",
23 => "delete media",
25 => "delete own book content",
29 => "edit own book content",
34 => "update content translations",
35 => "update media",
36 => "use editorial transition archive",
37 => "use editorial transition archived_draft",
38 => "use editorial transition archived_published",
39 => "use editorial transition create_new_draft",
40 => "use editorial transition publish",
41 => "view all media revisions",
43 => "view any unpublished content",
44 => "view latest version",
46 => "view own unpublished media",
47 => "view post access counter",
]
None
None
None
| Comment | File | Size | Author |
|---|---|---|---|
| #9 | 3221258-9.patch | 1.77 KB | alexpott |
| #5 | 3221258-5.patch | 2.53 KB | alexpott |
| #5 | 3-5-interdiff.txt | 911 bytes | alexpott |
| #3 | 3221258-3.patch | 3.42 KB | alexpott |
Comments
Comment #2
alexpottComment #3
alexpottComment #4
aaronmchaleMy first thought would be, how many of the permissions in question relate to modules that will be installed in the standard profile in the future (e.g. media, content moderation, etc)?
There were extensive discussions around the content editor role during UX Calls, and if I remember correctly, we did discuss the idea of adding permissions to the role as those modules become included in the standard profile.
I think the easy approach is just to remove the effected permissions, and then create separate issues for the relevant modules that will eventually be installed in the standard profile to update the content editor role as part of the inclusion process.
The thing that we discussed during the UX Calls is that this role (and the Content Manager role) is really just meant to be a sensible starting point, not really a be-all-end-all of that type of role. We noted that we wouldn't want to then later update existing sites because they may have customised this role beyond what our default is meant to represent, and maybe deleted it all together. With that in mind I think using hooks to add the permissions on install of specific modules is probably a bit overkill for what the intention of these roles are supposed to be, and possibly could create more problems for existing sites.
Thanks,
-Aaron
Comment #5
alexpottWhoops some cruft got in from another issue.
@AaronMcHale thanks for the comment - I like the idea of getting the modules installed as part of standard - I've filed #3221259: Add permissions for optional modules to content editor role as they become enabled for what to do next.
Comment #6
aaronmchaleThanks @alexpott
I'll re-post my previous comment in that issue as well as it's probably quite relevant there.
Thanks,
-Aaron
Comment #7
benjifisherThe patch needs a reroll after #3221206: Fix indentation in user.role.content_editor.yml. Sorry, I should have caught the indentation problem when I reviewed #3059984: Add new “Content Editor” role to Standard Profile.
I was aware of the conflict, but I thought that we could fix it as part of whichever issue was fixed last. That turns out to mean #2571235: [regression] Roles should depend on objects that are building the granted permissions. I am happy to review the fix here instead.
If there is support for adding modules such as Content Moderation to the Standard profile, then that will be a good solution. I hope we can manage it before 9.3.0 is released. Another alternative is to add a new profile to Drupal core. I suggested this at one of the usability meetings, and I think the main objection was that it would create a maintenance burden.
Comment #8
alexpottThis should be critical because it currently blocking one. I wished I'd reverted #3059984: Add new “Content Editor” role to Standard Profile instead of attempting to keep it in.
Comment #9
alexpottRebased and resolved the conflicts.
Comment #10
benjifisherI checked the removed permissions. They are from the following modules, none of which are enabled in the Standard profile:
aggregatorbookcontent_moderationcontent_translationmediastatisticsI also tested as follows:
content_editorrole.The same permissions were removed.
Comment #12
catchCommitted 4fef3bd and pushed to 9.3.x. Thanks!