LM-Commons / LmcUser

A generic user registration and authentication module for Laminas. Supports Laminas\Db and Doctrine2. (Formerly ZfcUser)
BSD 3-Clause "New" or "Revised" License
14 stars 16 forks source link

AdapterChain does not reset the chained adapters #74

Closed visto9259 closed 1 week ago

visto9259 commented 2 months ago

The resetAdapters method of the AdapterChain authentication adapter does not reset the chained adapters.

The method assumes that the chained adapters are attached to the shared event manager but this is not the case. Chained adapters are attached to the event manager.

The code:

$sharedManager = $this->getEventManager()->getSharedManager();

        if ($sharedManager) {
            $listeners = $sharedManager->getListeners(['authenticate'], 'authenticate');
            foreach ($listeners as $listener) {
                if (is_array($listener) && $listener[0] instanceof ChainableAdapter) {
                    $listener[0]->getStorage()->clear();
                }
            }
        }

There could be cases where the session container where the chained adapters are storing data may not have been clear when starting the login sequence.

Suggested solution is to add a new event 'reset' and trigger the event (similar to logout). Chained adapters would need to implement reset event listeners.

By the way, chained adapters should implement the ListenerAggregateInterface such that there is one call to the attach()method and let the adapter set its listeners as needed.

Same as https://github.com/ZF-Commons/ZfcUser/issues/689

lon9man commented 2 months ago

holly cow)) i created this issue 2 years ago https://github.com/ZF-Commons/ZfcUser/issues/689, but without luck to be fixed.

i made next, not clear, but works: AdapterChainServiceFactory.php

<?php

namespace User\ZfcUser\Authentication\Adapter;

use Psr\Container\ContainerInterface;
use Laminas\ServiceManager\Factory\FactoryInterface;

/**
 * !!IMPORTANT!! overridden vendor classes to fix issue https://github.com/ZF-Commons/ZfcUser/issues/689
 * affected files:
 *      User\ZfcUser\Authentication\Adapter\AdapterChainServiceFactory.php
 *      User\ZfcUser\Authentication\Adapter\AdapterChain.php
 */
class AdapterChainServiceFactory extends \LmcUser\Authentication\Adapter\AdapterChainServiceFactory implements FactoryInterface
{
    # refactored method, exists in vendor
    # changes:
    # 1. use overridden AdapterChain-class
    # 2. set adapters into AdapterChain() to use them in resetAdapters-method
    public function __invoke( ContainerInterface $serviceLocator, $requestedName, array $options = null )
    {
        $chain = new AdapterChain();
        $chain->setEventManager( $serviceLocator->get( 'EventManager' ) );

        $options  = $this->getOptions( $serviceLocator );
        $adapters = [];

        // iterate and attach multiple adapters and events if offered
        foreach ( $options->getAuthAdapters() as $priority => $adapterName ) {
            $adapter    = $serviceLocator->get( $adapterName );
            $adapters[] = $adapter;

            if ( is_callable( array( $adapter, 'authenticate' ) ) ) {
                $chain->getEventManager()->attach( 'authenticate', array( $adapter, 'authenticate' ), $priority );
            }

            if ( is_callable( array( $adapter, 'logout' ) ) ) {
                $chain->getEventManager()->attach( 'logout', array( $adapter, 'logout' ), $priority );
            }
        }

        $chain->setAdapters( $adapters );

        return $chain;
    }
}

AdapterChain.php

<?php

namespace User\ZfcUser\Authentication\Adapter;

use LmcUser\Authentication\Adapter\AbstractAdapter;

/**
 * !!IMPORTANT!! overridden vendor classes to fix issue https://github.com/ZF-Commons/ZfcUser/issues/689
 * affected files:
 *      User\ZfcUser\Authentication\Adapter\AdapterChainServiceFactory.php
 *      User\ZfcUser\Authentication\Adapter\AdapterChain.php
 */
class AdapterChain extends \LmcUser\Authentication\Adapter\AdapterChain
{
    protected $adapters = [];

    # added method, absent in vendor
    public function getAdapters()
    {
        return $this->adapters;
    }

    # added method, absent in vendor
    public function setAdapters( $adapters = [] )
    {
        $this->adapters = $adapters;
    }

    # refactored method, exists in vendor
    public function resetAdapters()
    {
        # vendor code commented, because listeners was attached using EventManager (not SharedEventManager)
        /*
        $sharedManager = $this->getEventManager()->getSharedManager();

        if ( $sharedManager ) {
            $listeners = $sharedManager->getListeners( [ 'authenticate' ], 'authenticate' );
            foreach ( $listeners as $listener ) {
                if ( is_array( $listener ) && $listener[0] instanceof ChainableAdapter ) {
                    $listener[0]->getStorage()->clear();
                }
            }
        }
        */

        # since ZF3 it is absent EventManager->getListeners (which was available in ZF2), so
        # in overridden User\ZfcUser\Authentication\Adapter\AdapterChainServiceFactory.php we set all registered adapters
        # and here we use them to clear storage
        /** @var AbstractAdapter $adapter */
        foreach ( $this->getAdapters() as $adapter ) {
            $adapter->getStorage()->clear();
        }

        return $this;
    }
}
visto9259 commented 2 months ago

Thanks @lon9man,

No one, I think, is monitoring ZfcUser. I am not monitoring it. But at the same time, we should go through the list of open issues/PR and import any relevant items into this repository.

The ZF-Commons team should mark the repos as archived.

lon9man commented 2 months ago

When I made issue i migrated to Zend 3, so Laminas was unknown for me, especially this package. Strictly today I checked this code in sources and decided to visit github and check newer versions with hope that this place was fixed) and have seen your post with familiar things)

visto9259 commented 2 months ago

Are you using LmcUser now?

Typically, when I consider using a repo, I always looked for signs of activity. If the last commit was a couple of years ago, it's a sign that no one is maintaining it.

lon9man commented 2 months ago

@visto9259, yes, i am using it now. after migration to laminas also migrated ZfcUser to LmcUser.

I always looked for signs of activity

i also do this. but for documentation purpose created issue too. if it was not created - then i am unable to quote it here)