Dynamic permission can't save in role

[hyzx86]

bug in pull: #12510

1. Edit a role
2. Select some dynamic permissions , like 'ExecuteApi_RecentBlogPosts'
3. Click save .
4. Reopen the role , ExecuteApi_RecentBlogPosts is not checked

cuz the code only can load the static permissions , the program will break on line 235

OrchardCore/src/OrchardCore.Modules/OrchardCore.Roles/Controllers/AdminController.cs

Lines 219 to 252 in 083d5d7

	var permissionNames = _permissionProviders.SelectMany(x => x.GetDefaultStereotypes())
	.SelectMany(y => y.Permissions ?? Enumerable.Empty<Permission>())
	.Select(x => x.Name)
	.ToList();
	// Save
	var rolePermissions = new List<RoleClaim>();
	foreach (var key in Request.Form.Keys)
	{
	var permissionName = key;
	if (key.StartsWith("Checkbox.", StringComparison.Ordinal))
	{
	permissionName = key.Substring("Checkbox.".Length);
	}
	if (!permissionNames.Contains(permissionName, StringComparer.OrdinalIgnoreCase))
	{
	// The request contains an invalid permission, let's ignore it
	continue;
	}
	if (!rolesDocument.PermissionGroups[role.RoleName].Contains(permissionName, StringComparer.OrdinalIgnoreCase))
	{
	// Save the permission in the permissions history so it never gets auto assigned to this role.
	rolesDocument.PermissionGroups[role.RoleName].Add(permissionName);
	updateRolesDocument = true;
	}
	if (String.Equals(Request.Form[key], "true", StringComparison.OrdinalIgnoreCase))
	{
	rolePermissions.Add(new RoleClaim { ClaimType = Permission.ClaimType, ClaimValue = permissionName });
	}
	}

We should load all permissions this way:

OrchardCore/src/OrchardCore.Modules/OrchardCore.Roles/Controllers/AdminController.cs

Lines 184 to 185 in 083d5d7

	var installedPermissions = await GetInstalledPermissionsAsync();
	var allPermissions = installedPermissions.SelectMany(x => x.Value);

[hyzx86]

chrome_SF0p462j0X.mp4

[gvkries]

Hehe, I just wanted to open the same bug. Actually there is a second even bigger issue with this PR. The way the RoleUpdater is now implemented causes it to always overwrite any admin settings, whenever the site starts. E.g. the 'Anonymous' role is now always getting the 'ViewContent' permission on startup, which breaks the whole content type permission system. Every content type is then allowed implicitly for everyone.

[hyzx86]

It seems that I should turn off PR first 🤣,
I haven't looked at the code carefully, I just let it work
Embarrassed, I found that the role permissions could not be changed until I released the program yesterday

[jtkech]

@hyzx86

I think you were in the right path, yes using GetInstalledPermissionsAsync() in the controller edit post seems to fix the issue because it uses GetPermissionsAsync() that also gets dynamic permissions.

For info I could also make it working by just using

        var permissionNames = new List<string>();
        foreach (var permissionProvider in _permissionProviders)
        {
            var permissions = await permissionProvider.GetPermissionsAsync();
            permissionNames.AddRange(permissions.Select(permission => permission.Name));
        }

Maybe you could re-open your PR ;)

As a reminder, things to be checked.

• Check if okay to use _rolesDocumentManager directly in RoleUpdater and AdminController to update the permissions history, knowing that the RoleStore (called by the aspnet RoleManager) will update the same document.

• Check if necessary to call the RoleUpdater logic on each feature enabling, before was on feature installed (called once on the first enabling).

• Check if necessary to call the RoleUpdater logic on each tenant activation. Hmm when a feature state change the shell will be rebuilt and then activated, so maybe at least only on tenant activation but not on any feature change, will see.

[hyzx86]

@jtkech reopened,
But I haven't looked at the problem mentioned by @gvkries carefully

[jtkech]

Okay no problem will look at the PR tomorrow

In the meantime can you try my above suggestion, if you have time

[hyzx86]

I am planning to build a module to obtain the corresponding site settings by specifying a type name
CustomSettings

The first time I tested with 'ViewContent'. Then I changed it to: DeleteCcontent to test again.
I don't think the latter should be given to any user without a role

But it didn't turn out, as I expected, that it actually had access

I have a big screenshot here, which shows my problems

[MikeAlhayek]

@hyzx86 are you working on a PR for this?

[hyzx86]

Not really. I just fixed the problem of edit role
#13025

The permission check is not work. I don't know what the problem is

[gvkries]

I've created a separate issue #13035 for the RoleUpdater bug. I think your PR fixes the bug described here, the other one is related but a little bit harder to solve correctly.