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

AdapterChain does not reset the chained adapters #74

Open
visto9259 opened this issue Apr 16, 2024 · 5 comments
Open

AdapterChain does not reset the chained adapters #74

visto9259 opened this issue Apr 16, 2024 · 5 comments
Labels
bug Something isn't working V3 To be fixed in version 3 V4 To be implemented in version 4

Comments

@visto9259
Copy link
Contributor

visto9259 commented Apr 16, 2024

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 ZF-Commons/ZfcUser#689

@visto9259 visto9259 added the bug Something isn't working label Apr 16, 2024
@lon9man
Copy link

lon9man commented Apr 17, 2024

holly cow))
i created this issue 2 years ago ZF-Commons/ZfcUser#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
Copy link
Contributor Author

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
Copy link

lon9man commented Apr 17, 2024

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
Copy link
Contributor Author

visto9259 commented Apr 17, 2024

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.

@visto9259 visto9259 added V4 To be implemented in version 4 V3 To be fixed in version 3 labels Apr 17, 2024
@lon9man
Copy link

lon9man commented Apr 18, 2024

@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)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working V3 To be fixed in version 3 V4 To be implemented in version 4
Projects
Status: 🆕 New
Development

No branches or pull requests

2 participants