Bug #104348
closed
Harden "fetchPossibleUsers" of the class "AbstractUserAuthentication"
0%
Description
In the method "fetchPossibleUsers" of the class "AbstractUserAuthentication", the method "getUser" is called. This method return either an array or false.
In case of return of an empty array, the following condition is met (if (is_array($row))) and a PHP warning is thrown.
The condition must be hardened and check the the variable $row is an array and not an empty one.
Another possibility is to throw an exception if return is an array, but "uid" and "username" column are not in there.
Updated by Georg Ringer 4 months ago
- Status changed from New to Needs Feedback
thanks for creating the issue. when does the method getUser returns an empty array? how can this be reproduced? asking because it is better to change the wrong usage of empty array to return false instead of checking for array's content
Updated by Eric Chavaillaz 4 months ago
With a custom auth and a custom method getUser. For me before, the method return always an array and never false.
I fix it now, but I think the core has to be more hard in this case.
Updated by Georg Ringer 4 months ago
thanks for the quick response. so it is your getUser method returning the empty array? just asking to understand the issue.
hardening is always good but that would mean instead of false just throw an exception and react on that, however that would be a breaking change.
Updated by Eric Chavaillaz 4 months ago
Yes, the PHP warning become with PHP 8. Before it was ok. And I never remark that the method getUser must return false in case of no user is finded.
Updated by Georg Ringer 4 months ago
- Status changed from Needs Feedback to Closed
as discussed it doesn't make much sense to check at one place in the class and whole service implementation. if this is once redone, it must be moved to an object oriented way to avoid exceptions through notices.
the method states return array<string, mixed>|false User array or FALSE but I agree that code quality can be better here