search

ulsdevteam / pkp-betterPassword

Plugin to implement password requirements for PKP OJS/OMP
GNU General Public License v2.0
7 stars 6 forks source link

Error: Call to a member function getId() on null #36

Closed dagosalas closed 1 year ago

dagosalas commented 1 year ago

I have implemented the plugin in several journals with OJS 3.3.0.11 , and I have started noticing that some users are not allowed to login. Apache sends this: [Fri Jan 13 09:38:59.278080 2023] [php7:error] [pid 19318] [client 148.234.16.165:56472] PHP Fatal error: Uncaught Error: Call to a member function getId() on null in /web/htdocs/revistas/innovaciones/lib/pkp/classes/notification/NotificationDAO.inc.php:216\nStack trace:\n#0 /web/htdocs/revistas/innovaciones/plugins/generic/betterPassword/features/ForceExpiration.inc.php(107): NotificationDAO->deleteObject(NULL)\n#1 /web/htdocs/revistas/innovaciones/plugins/generic/betterPassword/features/ForceExpiration.inc.php(65): ForceExpiration->_handleNotification(Object(User))\n#2 /web/htdocs/revistas/innovaciones/lib/pkp/classes/plugins/HookRegistry.inc.php(107): ForceExpiration->{closure}('LoadHandler', Array)\n#3 /web/htdocs/revistas/innovaciones/lib/pkp/classes/core/PKPPageRouter.inc.php(202): HookRegistry::call('LoadHandler', Array)\n#4 /web/htdocs/revistas/innovaciones/lib/pkp/classes/core/Dispatcher.inc.php(144): PKPPageRouter->route(Object(Request))\n#5 /web/htdocs/revistas/innovaciones/lib/pkp/classes/core/PKPApplication.inc.php(362): Dispatcher->dispatch(Object(Request))\n#6 /web/htdocs/revistas/innovaciones/index.php(68 in /web/htdocs/revistas/innovaciones/lib/pkp/classes/notification/NotificationDAO.inc.php on line 216, referer: https://revistainnovaciones.uanl.mx/index.php/revin/login The only way they can get in is (at this time) by leaving the password expiration option empty. It does allow the user to login but sends this error to the LOG: [Fri Jan 13 09:46:44.212946 2023] [php7:warn] [pid 19566] [client 148.234.16.165:56551] PHP Warning: Invalid argument supplied for foreach() in /web/htdocs/revistas/innovaciones/plugins/generic/betterPassword/BetterPasswordSettingsForm.inc.php on line 126, referer: https://revistainnovaciones.uanl.mx/index.php/revin/management/settings/website

ctgraham commented 1 year ago

Thank you for the detailed report. It appears that the notification which was stored in the user object may have already been deleted when the code attempts to delete it here: https://github.com/ulsdevteam/pkp-betterPassword/blob/19ef1fca9ca97db694b8253b839bbdf033a627f6/features/ForceExpiration.inc.php#L106-L107

I suspect that the intent is to limit the number of notifications present for a user, but the current implementation can be improved to confirm the notification's existence before calling the delete.

We also will want to migrate away from the use of Trivial Notifications, which are not WCAG compliant, in favor of Task notifications.

@wopsononock is currently working on this for another plugin, and I will tag him here.

IgorBaptist4 commented 1 year ago

I had the same problem and just made a simple change to check if the notification exists before try to delete it

$notification = $notificationDao->getById($lastNotification->id, $user->getId());

if($notification){
    $notificationDao->deleteObject($notification);
}
wopsononock commented 1 year ago

Thanks @dagosalas and @IgorBaptist4. https://github.com/ulsdevteam/pkp-betterPassword/releases/tag/v1.2.0-2 should take care of this issue.