Role permission settings are overwritten if a tenant is started

[gvkries]

Describe the bug

Whenever a tenant is activated, the RoleUpdater reapplies any default permissions to the roles. This overwrites any adjustments made from the admin dashboard. This breaks all permissions assigned to specific content types, because e.g. the anonymous role is always reset.

To Reproduce

Example steps to reproduce the behavior:

1. Go to security -> roles and edit the 'Anonymous' role.
2. Remove 'View' the permission from 'Content' and save.
3. Restart the web app.
4. The removed permission is added back.

Expected behavior

Permission settings on roles are persisted between site restarts.

This bug was introduced by PR #12510.
See also bug #13024.

[jtkech]

@gvkries

I'm working on it, also on the issue about dynamic permissions in the AdminController.

Here do you meant in the Contents (OrchardCore.Contents) section? Then View all content permission or View own content? If I uncheck the first one (the only one that is checked in the Allow column) and re-edit, both are unchecked in both columns, and they don't come back on a new startup.

Also, if it's another permission, try to re-edit the role before a new startup to see what was persisted, this to make the distinction between the RoleUpdater and the AdminController.

[gvkries]

Here do you meant in the Contents (OrchardCore.Contents) section? Then View all content permission or View own content? If I uncheck the first one (the only one that is checked in the Allow column) and re-edit, both are unchecked in both columns, and they don't come back on a new startup.

Yes, that is correct. I meant the Contents (OrchardCore.Contents) section with View all content permission.
I've had temporary fixes in my branch to separate the issues.

I don't see the error anymore on my end too. But the RoleUpdater still logs this at the debug level:
OrchardCore.Roles.Services.RoleUpdater: Debug: Default role 'Anonymous' granted permission 'ViewContent'

I'm confused...

[jtkech]

Okay thanks, I will take care of this, anyway I may change the RoleUpdater approach and will ask you to test it again when it will be ready ;)

[gvkries]

Thank you. But I still want to know why its behaving differently than yesterday ;-)

[gvkries]

Okay, it happens again if I use an older DB snapshot from a date before adding the commit with this change. So the problem may be somehow related to a migration and it is fixed when the permissions are saved once again. It would be less critical in this case.

[jtkech]

Okay thanks for the info

[gvkries]

Beside this being breaking for existing installations I think the only (minor) issue that remains after more research is a false log entry in RoleUpdater, Line 129:

foreach (var permissionName in additionalPermissionNames)
{
    if (_logger.IsEnabled(LogLevel.Debug))
    {
        _logger.LogDebug("Default role '{Role}' granted permission '{Permission}'", group.RoleName, permissionName);
    }

    if (rolesDocument.PermissionGroups[group.RoleName].Contains(permissionName, StringComparer.OrdinalIgnoreCase))
    {
        // The permission was previously assigned to the role, we can't assign it again.
        continue;
    }

     await _roleManager.AddClaimAsync(role, new Claim(Permission.ClaimType, permissionName));
}

The line should only be logged if the claim is actually being added.

[jtkech]

Yes I saw, was already fixed locally, anyway I will check different things, I think this week end, for example if we can prevent the logic to be executed on each shell activation, if possible.