[regression] Roles should depend on objects that are building the granted permissions

See https://groups.drupal.org/node/484788 for more information on the rc target tag.

So I don't think we're going to get round to #27 now. We do have a problem with cleaning up roles when things are deleted that provide dynamic permissions. We have exactly the same problems for the node type based permissions.

Oh and we still don't cleanup permissions on uninstall of a module either.

This issue is pretty much the same as #607238: Permissions are assumed to be unique among modules, but uniqueness is not enforced

Re-rolled the patch after a couple of years. I think we should do this instead of #2026983: [regression] Grants for permissions defined by a module are not revoked / removed from Role config entities when that module is uninstalled.

No interdiff because it's been 2 years and by the looks of it #26 and quite a few unrelated changes in it so an interdiff is just going to confuse matters and be unnecessarily big.

We need to implement and test for:

• content_translation
• rest

We need to add test coverage for:

• content_moderation
• field_ui
• vocabulary

core/modules/taxonomy/src/TaxonomyPermissions.php has changed in 8.5.x so we're going to need to do this there. And I think it is likely this will be 8.5.x only because it will need an upgrade path.

Whoops.

This does not yet resolve the test failures.

Ah the fun of module install :) So the fails are caused by calling $format->url() whilst creating configuration during a module install and the route provider can't kind the new routes of previous installed modules. Fortunately we already have a fix for this - #2913912: URL generator may have a stale route provider during module installation.

I have closed #2026983: [regression] Grants for permissions defined by a module are not revoked / removed from Role config entities when that module is uninstalled in favour of this issue because the approach to fixing the issue is more correct. However that means that this issue should be critical for the reasons that #2026983: [regression] Grants for permissions defined by a module are not revoked / removed from Role config entities when that module is uninstalled was. This is also a regression from Drupal 7. I'm crediting the people who worked on that issue.

Added blockers to issue summary.

Here's an interdiff for #42

Here's the patch with the fix from #2913912: URL generator may have a stale route provider during module installation to see if anything else is broken.

Thanks for the review @claudiu.cristea

1. I'm not sure why this if actually exists - if a permission does not exist as this point then shouldn't it be removed?
2. I've never come across this standard and don't feel particularly strongly about that. These are comments you wrote btw :) this test is from your patch in #26
3. Good point. fixed.

I outlined the need for an upgrade path and tests in #37 and #39.
The static permission thing - I'm not sure we should even support that - what would the use case be? These only really can depend on their provider.

Here are some test failure fixes, not all yet.

I don't think assertions are returning something in PHPUnit. Probably this if() is left here from the WebTestBase version of the test?

Nice catch!

We use to use the standard "Check that ..." in comments explaining the assertions.

I don't think we have anything like a standard about that :)

Some more fixes. The implementation of \Drupal\user\Entity\Role::onDependencyRemoval has been rewritten.

Oh wow - so a lot of the API first tests added are depending on permissions that don't actually exist!

#61: Oh, hah! So evidently it's possible to grant non-existent permissions and have them be respected by the permission checking logic … otherwise those API-First tests could never have passed!

This helps API-First Drupal become more reliable (in core, but also in the contrib JSON API and GraphQL modules), so tagging API-First Initiative.

#61: Oh, hah! So evidently it's possible to grant non-existent permissions and have them be respected by the permission checking logic … otherwise those API-First tests could never have passed!

Its indeed possible. I'm wondering whether we should add validation of some kind on the grantPermissio
level. This would still lead to a fast runtime, but we have a form of validation at least.

@dawehner I think we should explore whether or not we should do that in a follow-up. I guess there is a question about whether or not this change in behaviour is allowed even in a minor. The problem being that granting non-existent perms is a critical as well because I think it is a data integrity issue.

Here's a patch that might be green. We have test coverage for:

• content_moderation
• content_translation
• field_ui

Missing test coverage for

• rest
• node
• filter
• taxonomy

There is some implied test coverage for node and filter in UserRoleDeleteTest::testDependenciesRemoval() but I think we should have explicit test coverage that the dependencies are added correctly in each module that provides dynamic permissions that depend on things other than the module.

In the issue summary it used to say

Fix at least one use case. Let's fix filter_format in this issue.

I don't think this is a good way to go because once we add dependencies to roles than we need to try to keep them correct all the time. Doing this in steps just means we have lots and lots of upgrade headaches. Rather than one. We still need to write the upgrade path and tests for it.

The interdiff attached is pseudo interdiff because #2913912: URL generator may have a stale route provider during module installation landed so I had to rebase.

The only other blocker, #2892821: Core modules check node module's "access content" permission for displaying things that have nothing to do with nodes, landed!

Expanded test coverage to explicitly ensure that the access content case is handled correctly. I'm not sure if it really is that very useful though…

@alexpott: feel free to remove this small bit of extra test coverage for checking the #2892821: Core modules check node module's "access content" permission for displaying things that have nothing to do with nodes special case explicitly, if you feel it doesn't add much (and feel free to uncredit me).

The interdiff on #68 is not correct. Here's the correct one. I think having explicit testing about not removing the access content permission is fine. It proves that other non-related permissions are maintained through module uninstall.

:O how the hell did that #68 interdiff end up looking like THAT!? I haven't seen any changes like that in as long as I can remember. I'd swear d.o mixed up files here… :O

The #69 interdiff is the one I remember seeing an hour ago, thanks for uploding that.

And: great!

Patch adds test coverage for

• rest
• node
• filter
• taxonomy

And reverts the change to ModuleUninstallTest because access content is now provided by System there are no config updates when uninstalling Node.

@Berdir talked to me about this issue on IRC and he brought up a great point. During Drupal install there's the ability for install profiles to provide alternate configuration to be used in place of the configuration provided by the module. People are using this to provide alternate user.role.anonymous and user.role.authenticated config entities. These entities have permissions that do not exist when user is installed. Therefore we have a big problem because the current implementation of this patch is likely to remove those permissions. Ideas of how to fix this are welcome!

Thanks for writing that down here.

I mostly just wanted to make sure that we are aware of that problem, install profiles config/overrides are excluded from BC, we've broken that before e.g. for views and so on. But if we do, we should at least make sure we are aware of the impact and to document in the change record what should be done.

The only viable alternative that currently exists is to move that to hook_install() of the install profile and grant the permissions there once all modules are enabled.

That said, this is a general problem that doesn't just affect this, it's especially common in form/view displays, views and other complex config with a lot of dependencies.. e.g when you want to add another custom field that some module in your install profile adds to admin/content. A possible generic solution would be a config/post_override folder, which is config that would only be replaced once the installation is finished.. Or config_installer based install profiles, although I could imagine problems there as well, e.g. when the auth/anon user roles are installed only very late due to dependencies and other modules somehow rely on them in code.

Here's a patch which shows the nature of the problem @Berdir is talking about... it should fail.

So one idea would be to not override configuration entities that are provided by profiles. And just update the configuration when the profile is installed. This seems to work. Perhaps we could just do all the profile updates when the profile is being installed and not do the early swap out.

Hmmm... actually that won't work because profiles can provide overrides for modules that are optional... ie. be installed after Drupal has be installed.

Patch attached tries to get the best of both worlds - also ConfigInstallProfileUnmetDependenciesTest is still failing because in this instance the dependencies can now be met!

So let's split off the installer stuff into a new blocker to make it easier to review / think about in isolation... created #2922417: Profile provided configuration entities can have unmeetable dependencies

Patch includes an upgrade path and a test and the current patch from #2922417: Profile provided configuration entities can have unmeetable dependencies

So I think once the blocker lands we are good to go here.

I don't understand the nature of the problem in #75.

These entities have permissions that do not exist when user is installed.

Why isn't the following enough to fix that?

+++ b/core/profiles/testing_config_overrides/config/install/user.role.authenticated.yml
@@ -0,0 +1,11 @@
+dependencies:
+  module:
+    - tour

Oh, never mind. I'll comment in #2922417: Profile provided configuration entities can have unmeetable dependencies.

@effulgentsia because you can't install any module before installing user.module, that won't end well :) Also, config dependency order doesnot override module order, it just makes it fail if the dependencies aren't already installed. But maybe your "never mind" meant you got that already :)

On second thought, I don't think this is a significant impact for API-First. I'm disagreeing with myself in #64 :) Untagging.

Can we get a change record here too - because obviously contrib etc will need to add their support for it.

I really like the approach here - nice work

@effulgentsia, @xjm, @plach, @catch and I discussed this on a triage call and agree this is critical, primarily because the presence of the permission on the role grants access, even if the permission doesn't exist.

1. +++ b/core/modules/rest/tests/src/Functional/EntityResource/EntityFormDisplay/EntityFormDisplayResourceTestBase.php
   @@ -11,7 +11,7 @@
   -  public static $modules = ['node'];
   +  public static $modules = ['node', 'field_ui'];
   

   I assume this is because we're using permissions with dependencies in tests - where previously those modules weren't enabled?

2. +++ b/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php
   @@ -450,12 +450,17 @@ public function testGet() {
   +      // Filter out definitions that do not have a plugin.
   +      // @see \Drupal\rest\Plugin\rest\resource\EntityResource::addLinkHeaders()
   +      $link_relations = array_values(array_filter(array_keys($this->entity->getEntityType()->getLinkTemplates()), function ($relation_name) use ($link_relation_type_manager) {
   +        return $link_relation_type_manager->hasDefinition($relation_name);
   +      }));
          $expected_link_relation_headers = array_map(function ($relation_name) use ($link_relation_type_manager) {
            $link_relation_type = $link_relation_type_manager->createInstance($relation_name);
            return $link_relation_type->isRegistered()
              ? $link_relation_type->getRegisteredName()
              : $link_relation_type->getExtensionUri();
   -      }, array_keys($this->entity->getEntityType()->getLinkTemplates()));
   +      }, $link_relations);
   

   unrelated?

3. +++ b/core/modules/user/src/Entity/Role.php
   @@ -186,4 +186,68 @@ public function preSave(EntityStorageInterface $storage) {
   +      // @todo Why is this necessary? Surely for a permission to be on role it
   +      // has to exist.
   

   Should we have a follow up for this?

4. +++ b/core/modules/user/src/Entity/Role.php
   @@ -186,4 +186,68 @@ public function preSave(EntityStorageInterface $storage) {
   +        $name = in_array($type, ['config', 'content']) ? $dependency->getConfigDependencyName() : $dependency;
   ...
   +      if (in_array($permission_definitions[$permission]['provider'], $dependencies['module'], TRUE)) {
   

   can we use strict checking here because they're strings

5. +++ b/core/modules/user/src/PermissionHandlerInterface.php
   @@ -34,7 +34,20 @@
   +   *     The module providing this permission doesn't have to be added as
   +   *     dependency, because is automatically parsed, stored and retrieved from
   

   nit 'as a dependency'

NW for #85.

Can we get a change record here too - because obviously contrib etc will need to add their support for it.

Added one

I assume this is because we're using permissions with dependencies in tests - where previously those modules weren't enabled?

Yes, in the exactly you've given, it's the "administer node form display" permission.

unrelated?

Seems so, I reverted it

Should we have a follow up for this?

See #3055557: Role migration results in migration permissions that do not exist on the target site

I hope these are the remaining failures.

The plumbing has made it in to the ConfigInstaller. Now we can focus on user role dependencies and not fixing the config system. Rerolled.

+++ b/core/modules/user/user.post_update.php
@@ -20,3 +20,17 @@ function user_post_update_enforce_order_of_permissions() {
+  $existing_permissions = array_keys(\Drupal::service('user.permissions')->getPermissions());
+  $entity_save = function (Role $role) use ($existing_permissions) {
+    $permissions = array_intersect($role->getPermissions(), $existing_permissions);
+    $role
+      ->set('permissions', $permissions)
+      ->save();
+  };
+  array_map($entity_save, Role::loadMultiple());

This should use the ConfigEntityUpdater class.

+++ b/core/tests/Drupal/FunctionalTests/Rest/EntityViewDisplayResourceTestBase.php
@@ -11,7 +11,7 @@ abstract class EntityViewDisplayResourceTestBase extends EntityResourceTestBase
-  public static $modules = ['node'];
+  public static $modules = ['node', 'field_ui'];

These changes look odd - can't recall why they are necessary need to have a look at the issue history.

Some test fixes.

Looks like Views UI is not defining some link relation types. And this exposes this because Views entities use an admin permission that's provided by Views UI so the tests are not currently exposing this.

I think the test failures in #97 are exposing some fundamentally odd things about entity link templates: They use link relation types as their keys and REST is automatically trying to expose them.

What does an edit-display-form link relation type even mean to a REST client? In 99% of cases, that serialization format is going to be JSON/XML, not HTML, and neither of these media types define form semantics like HTML does.

IMHO, we shouldn't be adding Link headers to REST responses for every link template. Especially if a matching link relation type is not defined for it. All that does is create a noisy and poor DX for anyone using REST. Worse, our extension link relation types are not even valid.

The attached removes the assumption that every link template has an associated link relation type. I think we should do something to this effect and then define some follow-ups to disassociate them everywhere else.

n.b. the interdiff is to #97, not #99

Thanks for looking at this @gabesullice - so the whole link relation thing happens because we have to install Views UI for the rest tests. We have to install Views UI because it provides the permission 'administer views' which these tests use. This permission is defined on the View entity provided by the Views module - so that's a weird circular dependency. Once we install the Views UI module that exposes a load of undefined link relationships. It's a completely separate bug that is being surfaced here and not really part of this issue. On HEAD ff you install views ui and rest and try and get views via rest you'll have exactly the same problem. I'll open a separate issue about this.

Opened #3088282: Missing Views link relationships when Views UI installed to address this link relation stuff.

@alexpott, did you mean to add the link relation types in this issue? It sounded like you were instead going to address the permission placement instead. The interdiff just shows 3 new link relations.

@gabesullice well the permission placement is one solution. I'm not sure it's right though. If we put it in Views then people will have an "Administer views" permission with no UI which is odd.

If users have JSON:API + JSON:API Extras in contrib, it's possible, although extremely unlikely, that users could be editing Views display config without a UI.

The link relation type stuff has been put into #3088282: Missing Views link relationships when Views UI installed which is now RTBC and kinda blocks this one. Here are the remaining fixes to make the patch green. The patch includes #3088282: Missing Views link relationships when Views UI installed.

+++ b/core/modules/media_library/tests/src/FunctionalJavascript/MediaLibraryTest.php
@@ -282,7 +282,7 @@ public function testWidgetWithoutMediaTypes() {
-    $field_ui_uninstalled_message = 'There are no allowed media types configured for this field. Edit the field settings to select the allowed media types.';
+    $field_ui_uninstalled_message = 'There are no allowed media types configured for this field. Please contact the site administrator.';

This change shows how important this change is. In HEAD atm if you uninstall the field UI module you still get messages implying you can edit field settings whereas in reality there is no way to edit them.

We can fallback to \Drupal\Tests\rest\Functional\EntityResource\EntityResourceTestBase::getExpectedCacheContexts() now. No need to override.

Now that #3088282: Missing Views link relationships when Views UI installed has landed rerolled the patch.

Needed to re-apply #111 to the correct test.

1. +++ b/core/modules/user/tests/src/Functional/Update/UserUpdateRoleDependenciesTest.php
   @@ -0,0 +1,47 @@
   +      __DIR__ . '/../../../../../system/tests/fixtures/update/drupal-8-rc1.bare.standard.php.gz',
   

   Need to convert to using the 8.8 file.

2. +++ b/core/modules/user/user.post_update.php
   @@ -20,3 +20,17 @@ function user_post_update_enforce_order_of_permissions() {
   +  $existing_permissions = array_keys(\Drupal::service('user.permissions')->getPermissions());
   +  $entity_save = function (Role $role) use ($existing_permissions) {
   +    $permissions = array_intersect($role->getPermissions(), $existing_permissions);
   +    $role
   +      ->set('permissions', $permissions)
   +      ->save();
   +  };
   +  array_map($entity_save, Role::loadMultiple());
   

   Need to convert to using the ConfigEntityUpdater

Addressing #116

(Caveat: I'm on 8.8.5, so I'm not going to change to RTBC, but...)

The functionality worked PERFECTLY for me when I tested it with a new vocabulary (full steps to reproduce/test in my duplicate issue from a couple months ago, in case it helps anyone else with testing).

Two questions:

1. The patch didn't clean up existing orphaned permissions (i.e. permissions for creating/editing/deleting terms in vocabularies that were removed from our site ages ago). Have y'all already discussed whether this (really wonderful!!!!!!) change should be cleaning up orphaned permissions that existed on a site before applying the patch? If not, I urge you to do so -- though I'd certainly be ok with it being addressed in a follow-up issue, instead of holding up this one.
2. Would a backport to 8.8.x be permitted? I'm happy to help, but wanted to see if it would be accepted, or if this issue doesn't qualify.

Thank you @alexpott and everybody who's worked on this issue!!

Also, didn't notice this before: I get a slew of "notice" log messages when performing certain actions that I think trigger the check for permission configs -- here's an example message, when I was deleting my test vocabulary:
Notice: Undefined index: use text format full_html in Drupal\user\Entity\Role->onDependencyRemoval() (line 243 of /path/to/core/modules/user/src/Entity/Role.php)

>> Text format "full_html" no longer exists on this site, so "use text format full_html" is one of the many orphaned permissions we have. This example log message happened when I was deleting my test vocab (to test the patch) -- there were 6 such log messages at the time, idk how it decided which orphaned permissions to complain about. (I say that because ⤵️ )
>> I happened to uninstall a module while trying out this patch, and when I did that, the logs were ***FLOODED*** with a "notice" log message like the example above, for every orphaned permission -- we're talking several pages of log messages.

Anyway, FYI!

Rerolled patch from #117 for latest 8.9.x

Here's a patch for 9.1.x since this will only get commit to 9.x at this point and probably only 9.1.x since it has a post update.

Re #119 / @alisonjo315 this patch does clean up roles with permissions that do not exist. You need to run update.php / updatedb

1. +++ b/core/modules/user/user.post_update.php
   @@ -13,3 +16,15 @@ function user_removed_post_updates() {
   +/**
   + * Calculate role dependencies and remove non-existent permissions.
   + */
   +function user_post_update_update_roles(&$sandbox = NULL) {
   +  $existing_permissions = array_keys(\Drupal::service('user.permissions')->getPermissions());
   +  \Drupal::classResolver(ConfigEntityUpdater::class)->update($sandbox, 'user_role', function (Role $role) use ($existing_permissions) {
   +    $permissions = array_intersect($role->getPermissions(), $existing_permissions);
   +    $role->set('permissions', $permissions);
   +    return TRUE;
   +  });
   +}
   

   This function will remove non-existing permissions.

2. +++ b/core/modules/user/tests/src/Functional/Update/UserUpdateRoleDependenciesTest.php
   @@ -0,0 +1,47 @@
   +  public function testRolePermissions() {
   +    // Edit the role to have a non-existent permission.
   +    $raw_config = $this->config('user.role.authenticated');
   +    $permissions = $raw_config->get('permissions');
   +    $permissions[] = 'does_not_exist';
   +    $raw_config
   +      ->set('permissions', $permissions)
   +      ->save();
   +
   +    $authenticated = Role::load('authenticated');
   +    $this->assertTrue($authenticated->hasPermission('does_not_exist'), 'Authenticated role has a permission that does not exist');
   +    $this->assertEquals([], $authenticated->getDependencies());
   +
   +    $this->runUpdates();
   +    $authenticated = Role::load('authenticated');
   +    $this->assertFalse($authenticated->hasPermission('does_not_exist'), 'Authenticated role does not have a permission that does not exist');
   +    $this->assertEquals(['config' => ['filter.format.basic_html'], 'module' => ['comment', 'contact', 'filter', 'shortcut', 'system']], $authenticated->getDependencies());
   +  }
   

   This tests this functionality.

+++ b/core/modules/user/src/Entity/Role.php
@@ -193,4 +193,68 @@ public function preSave(EntityStorageInterface $storage) {
+      // @todo Why is this necessary? Surely for a permission to be on role it
+      // has to exist.
+      if (isset($permissions[$permission])) {

We need to address this - one thought is that we could support adding non existent permissions in D9 but deprecate it and then remove support in D10. This might help contrib / custom tests deal with this change in a graceful way.

Add link to steps to reproduce to issue summary -- feel free to remove, esp since it's on a D8 site, just adding in case they're helpful, but I won't be offended if it's determined they don't belong here.

FWIW, #121 works great for me on 8.9.12!

@alexpott Thank you for the comment - sorry I disappeared - the update hook totally worked, as you said, idk what happened to me back in May, but it all definitely works, so, hooray! I 💜 my permission config entities now....

Re-rolled for 9.2.x. It'd be great to get this to rtbc and committed.

Added release note.

Fixing the broken tests.

Removing the @todo and seeing what is broken....

Resolving some of the fails in #132 which shows the importance of resolving most of #3055557: Role migration results in migration permissions that do not exist on the target site in this issue. I think in a future patch will need to trigger a deprecation when saving with a permission that does not exist as that will help contrib / custom deal with this and prevent core from regressing again.

Discussed the migrate fails with @heddn. Given the complexity of the migrate part of this I think it is better to punt that to #3055557: Role migration results in migration permissions that do not exist on the target site a re-purpose that to working out what to do about migrations.

Discussed where to put the role clean-up on filter disable with @catch. After considering putting in the Filter entity, on the filter module we plumped for the user module because this means tests like EditorFilterIntegrationTest don't have to change and it is Roles changing when a filter is disabled and Roles are owned by the user module and that's what the hook is for.

Fixing the last fail.

Improved the issue summary.

+++ b/core/modules/field/tests/src/Functional/Rest/FieldConfigResourceTestBase.php
@@ -12,7 +12,7 @@ abstract class FieldConfigResourceTestBase extends EntityResourceTestBase {
-  protected static $modules = ['field', 'node'];
+  protected static $modules = ['field', 'field_ui', 'node'];

+++ b/core/modules/field/tests/src/Functional/Rest/FieldStorageConfigResourceTestBase.php
@@ -10,7 +10,7 @@ abstract class FieldStorageConfigResourceTestBase extends EntityResourceTestBase
-  protected static $modules = ['node'];
+  protected static $modules = ['field_ui', 'node'];

+++ b/core/modules/jsonapi/tests/src/Functional/ActionTest.php
@@ -16,7 +16,7 @@ class ActionTest extends ResourceTestBase {
-  protected static $modules = ['user'];
+  protected static $modules = ['action'];

+++ b/core/modules/jsonapi/tests/src/Functional/BaseFieldOverrideTest.php
@@ -16,7 +16,7 @@ class BaseFieldOverrideTest extends ResourceTestBase {
-  protected static $modules = ['field', 'node'];
+  protected static $modules = ['field', 'node', 'field_ui'];

+++ b/core/modules/jsonapi/tests/src/Functional/EntityFormDisplayTest.php
@@ -16,7 +16,7 @@ class EntityFormDisplayTest extends ResourceTestBase {
-  protected static $modules = ['node'];
+  protected static $modules = ['node', 'field_ui'];

+++ b/core/modules/jsonapi/tests/src/Functional/EntityViewDisplayTest.php
@@ -16,7 +16,7 @@ class EntityViewDisplayTest extends ResourceTestBase {
-  protected static $modules = ['node'];
+  protected static $modules = ['node', 'field_ui'];

+++ b/core/modules/jsonapi/tests/src/Functional/FieldConfigTest.php
@@ -19,7 +19,7 @@ class FieldConfigTest extends ResourceTestBase {
-  protected static $modules = ['field', 'node'];
+  protected static $modules = ['field', 'node', 'field_ui'];

+++ b/core/modules/jsonapi/tests/src/Functional/FieldStorageConfigTest.php
@@ -15,7 +15,7 @@ class FieldStorageConfigTest extends ResourceTestBase {
-  protected static $modules = ['node'];
+  protected static $modules = ['node', 'field_ui'];

+++ b/core/modules/jsonapi/tests/src/Functional/PathAliasTest.php
@@ -16,7 +16,7 @@ class PathAliasTest extends ResourceTestBase {
-  protected static $modules = ['user'];
+  protected static $modules = ['path'];

+++ b/core/modules/jsonapi/tests/src/Functional/ViewTest.php
@@ -15,7 +15,7 @@ class ViewTest extends ResourceTestBase {
-  protected static $modules = ['views'];
+  protected static $modules = ['views', 'views_ui'];

+++ b/core/modules/path_alias/tests/src/Functional/Rest/PathAliasResourceTestBase.php
@@ -14,7 +14,7 @@ abstract class PathAliasResourceTestBase extends EntityResourceTestBase {
-  protected static $modules = ['path_alias'];
+  protected static $modules = ['path', 'path_alias'];

+++ b/core/modules/system/tests/src/Functional/Rest/ActionResourceTestBase.php
@@ -11,7 +11,7 @@ abstract class ActionResourceTestBase extends EntityResourceTestBase {
-  protected static $modules = ['user'];
+  protected static $modules = ['action', 'user'];

+++ b/core/tests/Drupal/FunctionalTests/Rest/BaseFieldOverrideResourceTestBase.php
@@ -11,7 +11,7 @@ abstract class BaseFieldOverrideResourceTestBase extends EntityResourceTestBase
-  protected static $modules = ['field', 'node'];
+  protected static $modules = ['field', 'field_ui', 'node'];

+++ b/core/tests/Drupal/FunctionalTests/Rest/EntityFormDisplayResourceTestBase.php
@@ -11,7 +11,7 @@ abstract class EntityFormDisplayResourceTestBase extends EntityResourceTestBase
-  protected static $modules = ['node'];
+  protected static $modules = ['node', 'field_ui'];

+++ b/core/tests/Drupal/FunctionalTests/Rest/EntityViewDisplayResourceTestBase.php
@@ -11,7 +11,7 @@ abstract class EntityViewDisplayResourceTestBase extends EntityResourceTestBase
-  protected static $modules = ['node'];
+  protected static $modules = ['node', 'field_ui'];

These tests depend on the 'administer node fields' permission and that is provided by the field ui module. Perhaps we should consider moving that to the field module? There's a similar thing going on with Actions and Views UI. They both provide permissions which are used by entities provided by other modules.

The other permissions are:

• 'administer actions'
• 'administer views'

I think given JSON:API in core we should consider moving these permissions to the module that provides the entity that depends on these permissions. So...

• 'administer views' => views.permissions.yml
• 'administer actions' => system.permissions.yml
• 'administer url aliases' => path_alias.permissions.yml
• 'create url aliases' => path_alias.permissions.yml
• \Drupal\field_ui\FieldUiPermissions => \Drupal\field\FieldPermissions ???

I'm not sure where 'administer display modes' should be. I think looking at the usage field_ui.permissions.yml is fine - it's where it current is. EntityViewDisplay and EntityFormDisplay both use permissions created by \Drupal\field_ui\FieldUiPermissions which I think should be moved.