mezzio / mezzio-authentication-oauth2

OAuth2 (server) authentication middleware for Mezzio and PSR-7 applications.
https://docs.mezzio.dev/mezzio-authentication-oauth2/
BSD 3-Clause "New" or "Revised" License
25 stars 17 forks source link

Defining Event Listener Providers throws error PHP >= 8.0 #63

Closed cardcc closed 2 months ago

cardcc commented 1 year ago

Bug Report

Q A
mezzio/mezzio-authentication-oauth2 2.5.0

I think this issue is still present in mezzio/mezzio-authentication-oauth2 in 2.8.0

Aditional info: | league/event | 2.2.0 | league/oauth2-server | 8.4.0

Summary

By defining a Event Listener Provider without a priority level, the error "ArgumentCountError array_merge() does not accept unknown named parameters" is thrown for PHP >= 8.0

Current behavior

According to Intro mezzio Oauth2 in order to define a Event Listener one should use:

return [
    'event_listeners' => [
        // using a container service
        [
            \League\OAuth2\Server\RequestEvent::CLIENT_AUTHENTICATION_FAILED,
            \My\Event\Listener\Service::class,
        ],

which are then handled in the "AuthorizationServerFactory" as:

private function addListeners(
        AuthorizationServer $authServer,
        ContainerInterface $container
    ): void {
        $listeners = $this->getListenersConfig($container);

        foreach ($listeners as $idx => $listenerConfig) {
            $event    = $listenerConfig[0];
            $listener = $listenerConfig[1];
            $priority = $listenerConfig[2] ?? null;

where if no priority is defined (as in the example displayed in help page), priority defaults to: $priority = null

Then Emitter.php file from “league/event” (wich can be found here: Emitter.php) adds the listeners as

public function addListener($event, $listener, $priority = self::P_NORMAL)
    {
        $listener = $this->ensureListener($listener);
        $this->listeners[$event][$priority][] = $listener;
        $this->clearSortedListeners($event);

        return $this;
    }

where $priority would stay as null.

The problem is that in PHP >= 8.0, when a listener is triggered with “Emitter.php” file uses the function:

protected function getSortedListeners($event)
    {
        if (! $this->hasListeners($event)) {
            return [];
        }

        $listeners = $this->listeners[$event];
        krsort($listeners);

        return call_user_func_array('array_merge', $listeners);
    }

wich in turn throws an error: "ArgumentCountError array_merge() does not accept unknown named parameters"

due to PHP 8.0 named parameters feature.

How to reproduce

Define a event listener,

return [
    'event_listeners' => [
        // using a container service
        [
            \League\OAuth2\Server\RequestEvent::CLIENT_AUTHENTICATION_FAILED,
            \My\Event\Listener\UserAuthenticationFailedListener,
        ],

Create a simple event listener

namespace My\Event\Listener;

use League\Event\AbstractListener;
use League\Event\EventInterface;
use League\OAuth2\Server\Exception\OAuthServerException;

class UserAuthenticationFailedListener extends AbstractListener
{
    public function handle(EventInterface $event)
    {
        echo "do something right there"; exit;
    }

}

If client authentication fails and the event "\League\OAuth2\Server\RequestEvent::CLIENT_AUTHENTICATION_FAILED" is detected the error "ArgumentCountError array_merge() does not accept unknown named parameters" will be thrown

Expected behavior

Not throw the error and implement the code in the listener.

It is possible to disable the error just by defining a priority for each listener, as:

'event_listeners' => [                                            
            [
                \League\OAuth2\Server\RequestEvent::USER_AUTHENTICATION_FAILED,
                \My\Event\Listener\UserAuthenticationFailedListener,
                \League\Event\EmitterInterface::P_HIGH
            ]
        ],       

Another possible solution would be something like:

if ( $priority ) {
$authServer->getEmitter()->addListener($event, $listener, $priority);
} else {
$authServer->getEmitter()->addListener($event, $listener );
}
akrabat commented 2 months ago

Fixed by https://github.com/mezzio/mezzio-authentication-oauth2/pull/72