Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

A lot of exceptions after login in backend #3625

Closed
empiricompany opened this issue Nov 1, 2023 · 12 comments · Fixed by #3642 or #3647
Closed

A lot of exceptions after login in backend #3625

empiricompany opened this issue Nov 1, 2023 · 12 comments · Fixed by #3642 or #3647

Comments

@empiricompany
Copy link
Contributor

Reference PR:

The bug was introduced by this PR in loadRules(): an exception was added instead of this.
https://github.com/OpenMage/magento-lts/pull/2339/files#diff-1048571d4baf38d246708d5953af464948b5591c2c8d648036394d6101ba8022

Possible solution

Rollback and remove Mage::logException($e); in app/code/core/Mage/Admin/Model/Resource/Acl.php::loadRules()
I also recommend including the commented TODO code because it would be a feature to implement (the deletion of old resources),
that was this:

//$m = $e->getMessage();
//if ( eregi("^Resource '(.*)' not found", $m) ) {
    // Deleting non existent resource rule from rules table
    //$cond = $this->_write->quoteInto('resource_id = ?', $resource);
    //$this->_write->delete(Mage::getSingleton('core/resource')->getTableName('admin/rule'), $cond);
//} else {
    //TODO: We need to log such exceptions to somewhere like a system/errors.log
//}

in Mage_Admin_Model_Resource_Acl::loadRules() was added an exception

Preconditions (*)

  1. Install OpenMage 20.2.0 - PHP 7.4 - MySQL 5.7

  2. Install an extension that adds some resources to the admin_rule table (seems like the issue only occurs with deny permission resources).
    mageplaza_admin_rule

  3. Disable the previously installed extension from app/etc/modules/Vendor_Extension.xml.

Steps to reproduce (*)

  1. Login to the admin area.

Expected result (*)

  1. No exceptions should be logged for resources with deny permission.

Actual result (*)

  1. All resources with deny permission referring to the disabled extension have been logged with exceptions:
2023-11-01T08:44:04+00:00 ERR (3):
Zend_Acl_Exception: Resource 'admin/mageplaza_betterblog/new/action' not found in /var/www/html/htdocs/vendor/shardj/zf1-future/library/Zend/Acl.php:365
Stack trace:
#0 /var/www/html/htdocs/vendor/shardj/zf1-future/library/Zend/Acl.php(641): Zend_Acl->get()
#1 /var/www/html/htdocs/vendor/shardj/zf1-future/library/Zend/Acl.php(519): Zend_Acl->setRule()
#2 /var/www/html/htdocs/app/code/core/Mage/Admin/Model/Resource/Acl.php(132): Zend_Acl->deny()
#3 /var/www/html/htdocs/app/code/core/Mage/Admin/Model/Resource/Acl.php(70): Mage_Admin_Model_Resource_Acl->loadRules()
#4 /var/www/html/htdocs/app/code/core/Mage/Admin/Model/Session.php(162): Mage_Admin_Model_Resource_Acl->loadAcl()
#5 /var/www/html/htdocs/app/code/core/Mage/Admin/Model/Observer.php(62): Mage_Admin_Model_Session->login()
#6 /var/www/html/htdocs/app/code/core/Mage/Core/Model/App.php(1448): Mage_Admin_Model_Observer->actionPreDispatchAdmin()
#7 /var/www/html/htdocs/app/code/core/Mage/Core/Model/App.php(1426): Mage_Core_Model_App->_callObserverMethod()
#8 /var/www/html/htdocs/app/Mage.php(509): Mage_Core_Model_App->dispatchEvent()
#9 /var/www/html/htdocs/app/code/core/Mage/Core/Controller/Varien/Action.php(527): Mage::dispatchEvent()
#10 /var/www/html/htdocs/app/code/core/Mage/Adminhtml/Controller/Action.php(173): Mage_Core_Controller_Varien_Action->preDispatch()
#11 /var/www/html/htdocs/app/code/core/Mage/Core/Controller/Varien/Action.php(410): Mage_Adminhtml_Controller_Action->preDispatch()
#12 /var/www/html/htdocs/app/code/core/Mage/Core/Controller/Varien/Router/Standard.php(255): Mage_Core_Controller_Varien_Action->dispatch()
#13 /var/www/html/htdocs/app/code/core/Mage/Core/Controller/Varien/Front.php(181): Mage_Core_Controller_Varien_Router_Standard->match()
#14 /var/www/html/htdocs/app/code/core/Mage/Core/Model/App.php(358): Mage_Core_Controller_Varien_Front->dispatch()
#15 /var/www/html/htdocs/app/Mage.php(743): Mage_Core_Model_App->run()
#16 /var/www/html/htdocs/index.php(65): Mage::run()
#17 {main}

It seems that this issue also occurs for old deprecated resources (my installation was updated from Magento 1.9.4.5) such as:

2023-11-01T08:44:04+00:00 ERR (3):
Zend_Acl_Exception: Resource 'admin/system/api/consumer/edit' not found in /var/www/html/htdocs/vendor/shardj/zf1-future/library/Zend/Acl.php:365
Stack trace:
#0 /var/www/html/htdocs/vendor/shardj/zf1-future/library/Zend/Acl.php(641): Zend_Acl->get()
#1 /var/www/html/htdocs/vendor/shardj/zf1-future/library/Zend/Acl.php(519): Zend_Acl->setRule()
#2 /var/www/html/htdocs/app/code/core/Mage/Admin/Model/Resource/Acl.php(132): Zend_Acl->deny()
#3 /var/www/html/htdocs/app/code/core/Mage/Admin/Model/Resource/Acl.php(70): Mage_Admin_Model_Resource_Acl->loadRules()
#4 /var/www/html/htdocs/app/code/core/Mage/Admin/Model/Session.php(162): Mage_Admin_Model_Resource_Acl->loadAcl()
#5 /var/www/html/htdocs/app/code/core/Mage/Admin/Model/Observer.php(62): Mage_Admin_Model_Session->login()
#6 /var/www/html/htdocs/app/code/core/Mage/Core/Model/App.php(1448): Mage_Admin_Model_Observer->actionPreDispatchAdmin()
#7 /var/www/html/htdocs/app/code/core/Mage/Core/Model/App.php(1426): Mage_Core_Model_App->_callObserverMethod()
#8 /var/www/html/htdocs/app/Mage.php(509): Mage_Core_Model_App->dispatchEvent()
#9 /var/www/html/htdocs/app/code/core/Mage/Core/Controller/Varien/Action.php(527): Mage::dispatchEvent()
#10 /var/www/html/htdocs/app/code/core/Mage/Adminhtml/Controller/Action.php(173): Mage_Core_Controller_Varien_Action->preDispatch()
#11 /var/www/html/htdocs/app/code/core/Mage/Core/Controller/Varien/Action.php(410): Mage_Adminhtml_Controller_Action->preDispatch()
#12 /var/www/html/htdocs/app/code/core/Mage/Core/Controller/Varien/Router/Standard.php(255): Mage_Core_Controller_Varien_Action->dispatch()
#13 /var/www/html/htdocs/app/code/core/Mage/Core/Controller/Varien/Front.php(181): Mage_Core_Controller_Varien_Router_Standard->match()
#14 /var/www/html/htdocs/app/code/core/Mage/Core/Model/App.php(358): Mage_Core_Controller_Varien_Front->dispatch()
#15 /var/www/html/htdocs/app/Mage.php(743): Mage_Core_Model_App->run()
#16 /var/www/html/htdocs/index.php(65): Mage::run()
#17 {main}

@m-overlund
Copy link
Contributor

can confirm - having the same issues.

@ADDISON74
Copy link
Contributor

@sreichel - Could you please check? This report refers to a PR created by you and we honestly appreciate your feedback.

@empiricompany
Copy link
Contributor Author

empiricompany commented Nov 1, 2023

Here too, exceptions have been introduced that were not there before.

@ADDISON74
Copy link
Contributor

If it turns out to be a wrong change then I propose to revert PR #2339.

@sreichel
Copy link
Contributor

sreichel commented Nov 1, 2023

@sreichel - Could you please check? This report refers to a PR created by you and we honestly appreciate your feedback.

Imho it's okay to log it. Just need to add documentation how to remove that orphan entries.

#2339 (comment)

@empiricompany
Copy link
Contributor Author

@sreichel - Could you please check? This report refers to a PR created by you and we honestly appreciate your feedback.

Imho it's okay to log it. Just need to add documentation how to remove that orphan entries.

#2339 (comment)

It's okay if there is a method to clean the database; otherwise, it just unnecessarily fills up the logs.
I propose removing all the added logException calls from the PR.

@Sdfendor
Copy link
Contributor

Sdfendor commented Nov 2, 2023

@sreichel - Could you please check? This report refers to a PR created by you and we honestly appreciate your feedback.

Imho it's okay to log it. Just need to add documentation how to remove that orphan entries.
#2339 (comment)

It's okay if there is a method to clean the database; otherwise, it just unnecessarily fills up the logs. I propose removing all the added logException calls from the PR.

I (the team I'm working in) actually really like this change. By that we found a lot of orphans in our DB we could clean up. A script would be nice though. What's the purpose of such exceptions, if higher up they just get ignored (the previous state)? The database fills (slowly) with garbage data and nobody notices it.

@empiricompany
Copy link
Contributor Author

@sreichel - Could you please check? This report refers to a PR created by you and we honestly appreciate your feedback.

Imho it's okay to log it. Just need to add documentation how to remove that orphan entries.
#2339 (comment)

It's okay if there is a method to clean the database; otherwise, it just unnecessarily fills up the logs. I propose removing all the added logException calls from the PR.

I (the team I'm working in) actually really like this change. By that we found a lot of orphans in our DB we could clean up. A script would be nice though. What's the purpose of such exceptions, if higher up they just get ignored (the previous state)? The database fills (slowly) with garbage data and nobody notices it.

Yes, it would be necessary to implement a cleanup of old entries, but until we have it, it is necessary to remove the exception, otherwise, the exception log will grow.

@sreichel
Copy link
Contributor

sreichel commented Nov 4, 2023

Yes, it would be necessary to implement a cleanup of old entries, but until we have it, it is necessary to remove the exception, otherwise, the exception log will grow.

The problem i see are the disabled extensions - how to detect them?

Just an idea ... if an unused acl is found ...

  • whitelist them in core_config_data (or somewhere else)
  • check whitelist before delete entries
  • log exeption / add an admin notification only once

Any better ideas?

@empiricompany
Copy link
Contributor Author

Yes, it would be necessary to implement a cleanup of old entries, but until we have it, it is necessary to remove the exception, otherwise, the exception log will grow.

The problem i see are the disabled extensions - how to detect them?

Just an idea ... if an unused acl is found ...

* whitelist them in core_config_data (or somewhere else)

* check whitelist before delete entries

* log exeption / add an admin notification only once

Any better ideas?

The priority is to remove the logException ASAP.

The main issue is that I can temporarily disable an extension without having to necessarily perform a cleanup of ACL and other orphaned data in the database, including tables like core_resource and core_config_data. As a result, the removal and cleanup of this data should be done explicitly and cannot be automated.

@kiatng
Copy link
Contributor

kiatng commented Nov 4, 2023

An idea is to add a menu "Manage Orphaned Resources" to render a grid with collection:

$collection = Mage::getResourceModel('admin/rules_collection')
    ->addFieldToFilter('resource_id', ['nin' => Mage::getModel('admin/roles')->getResourcesList2D()]);

And add a mass-action to delete the selected items.

@loekvangool
Copy link
Contributor

loekvangool commented Nov 8, 2023

It also sounds like a typical job for n98-magerun to remove orphaned ACL entries, just throwing it out there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants