Improved orphaned resources detection in backend, fixed #4007

[kiatng]

…le`.

Description (*)

This PR fixed the following:

1. Show the notice on orphaned resources based on admin user ACL system/acl/orphaned_resources
2. Fixed bug on adding incorrect resources to table admin_rule

Related Pull Requests

PR #3647

Fixed Issues (if relevant)

1. Fixes OpenMage/magento-lts#4007

Manual testing scenarios (*)

First, replicate the conditions in issue #4007 by adding <action>some/action</action> node in any adminhtml.xml file. and refresh the cache.

1. Check the notice is not displayed when Orphaned Role Resources is unchecked in admin > System > Permissions > Roles > click to edit a role
2. Logout and login as user with the role from step 1. No notice should be shown.
3. Logout and login as full admin user. Notice on orphaned resources is shown.
4. Click on the link and delete the orphaned resources.
5. Check that no invalid entries are added to table admin_rule by navigating to admin > System > Permissions > Roles > click to edit a role > Save (there is no need to edit the role). Before this PR, the <action> would be added. With this PR, they are not added.

Comments

The cause of issue #4007 is due to incorrect elements in adminhtml.xml in 3rd-party extensions. In the past, before PR #3647, the invalid resources added to table admin_rule remain as is. After PR #3647, it exposes a bug in the core that was hidden since the beginning. This PR fixed the bug but is also tolerant of incorrect elements in the adminhtml.xml file.

[kiatng]

Here's the source of the bug:

This is a recursive function iterating the XML tree. On node <action> , $resource->getName() is string "action", if (!in_array($resource->getName(), ['title', 'sort_order', 'children', 'disabled'])) { is true, it is treated as a legitimate resource even though it is not.

Since all resources have children, as they need at least a title, so checking for non-empty children can validate real resources.

[m-overlund]

The notification is still only shown once (upon login). but now I'm able to see and delete the orphaned roles.

[fballiano]

it works for me, I applied the PR, logged in, I see the notice, click and the grid has values that I can remove. logout-login, no alert is shown anymore ;-)

[fballiano]

@kiatng I've no way of testing if, merging this PR, the original situation would still work (I mean before the first round of removing orphaned ACLs) are you confident it still works in any case?

[kiatng]

This PR is an improvement over the last one. But it's always better if more users can test it. There are quite a few people affected, should we wait for a couple more confirmations?

[fballiano]

let's wait a few days and see if somebody jumps in, in case we don't have any more feedback we'll merge it anyway, it is what it is

[fballiano]

I'll merge it as it is now, we've waited a bit, we've 1 green + 2 grays + the PR is by a maintainer so I think it's good to go