Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Chain authentication storage #2718

Closed
wants to merge 7 commits into from

4 participants

Roel van Duijnhoven Matthew Weier O'Phinney Kathryn Reeve Evan Coury
Roel van Duijnhoven

This component can be usefull if you want to store the identity in multiple places. For example once in the database and once in $_SESSION. If either one of the storages is non-empty then the chain should also be non-empty.

On top of that the order in which the storages are accessed is important. A use case that is often used: the identity is stored in the database and can be obtained using a cookie on the client. However one wants to retrieve this data from the database only on the first request of a browser session and therefore the identity is stored in $_SESSION.

Such configuration might look like:

$chain = new ChainStorage;
$chain->add(new SessionStorage, 2);
$chain->add(new DatabaseStorage, 1);

I developed the component for a website I am developing, but though it might be of use to other people.

Matthew Weier O'Phinney

The example is a bit flawed -- session is always queried first anyways when you use AuthenticationService::(has|get)Identity(). However, I've seen developers wanting to chain oauth, http, and db before (this was an issue with ZfcUser) -- I think @EvanDotPro may have created something like your authentication chain in that module as well), and as such, this is definitely a useful addition.

Evan Coury EvanDotPro was assigned
Matthew Weier O'Phinney

Assigned @EvanDotPro to evaluate.

Matthew Weier O'Phinney

Needs tests, for sure, @roelvanduijnhoven !

Roel van Duijnhoven

Allright! As soon as I receive positive feedback that this component could make it into master and it is not a duplicate of something already present in ZfcUser I start writing tests.

Kathryn Reeve

Write tests anyway. Don't leave the tests as an after thought...

Roel van Duijnhoven

I originally wrote this component for the use case I presented in the PRs introduction. @weierophinney was right however that the example was flawed. In my own project I now use the Session storage instead.

Still it could be usefull to other people. Therefore I just published a new commit introducing tests for this storage. These test should show more clearly my original intention.

Matthew Weier O'Phinney

@roelvanduihnhoven This looks reasonable, particularly with the new test cases. I'm going to leave some detailed feedback on code momentarily -- however, my bigger question is: do you feel you can incorporate that feedback and get some docs on this storage adapter in place by end of this week? If so, I'll keep it scheduled for 2.1; otherwise, I'll clear the milestone, and push it to 2.2.

Thanks!

Also, please rebase this off current master so that it can merge cleanly. :)

library/Zend/Authentication/Storage/Chain.php
((29 lines not shown))
+ */
+ protected $storages;
+
+ /**
+ * Initializes the priority queue that contains storages.
+ */
+ public function __construct()
+ {
+ $this->storages = new PriorityQueue();
+ }
+
+ /**
+ * @param StorageInterface $storage
+ * @param integer $priority
+ */
+ public function add( StorageInterface $storage, $priority = 1 )
Matthew Weier O'Phinney Owner

CS: remove spaces inside the opening and closing parentheses; this is true throughout the code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
library/Zend/Authentication/Storage/Chain.php
((64 lines not shown))
+ // than the current one.
+ foreach( $this->storages as $storage )
+ {
+ if( $storage->isEmpty() )
+ {
+ $storagesWithHigherPriority[] = $storage;
+ }
+ else
+ {
+ $storageValue = $storage->read();
+ foreach( $storagesWithHigherPriority as $higherPriorityStorage )
+ $higherPriorityStorage->write($storageValue);
+
+ return false;
+ }
+ }
Matthew Weier O'Phinney Owner

I'd rewrite the above to avoid the else condition; additionally, always use blocks for conditionals and loops, and the opening brace should be on the same line.

foreach ($this->storages as $storage) {
    if ($storage->isEmpty()) {
        $storagesWithHigherPriority[] = $storage;
        continue;
    }
    $storageValue = $storage->read();
    foreach ($storagesWithHigherPriority as $higherPriorityStorage) {
        $higherPriorityStorage->write($storageValue);
    }
    return false;
}

Also, the plural of "storage" is... "storage". I'd rename storagesWithHigherPriority to storageWithHigherPriority, and rename the storages member property to storage or storageChain.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
library/Zend/Authentication/Storage/Chain.php
((88 lines not shown))
+ * @see StorageInterface::read()
+ */
+ public function read()
+ {
+ return $this->storages->top()->read();
+ }
+
+ /**
+ * Write the new $contents to all storages in the chain.
+ *
+ * @see StorageInterface::write()
+ */
+ public function write( $contents )
+ {
+ foreach( $this->storages as $storage )
+ $storage->write($contents);
Matthew Weier O'Phinney Owner

Put this in a block, please.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Matthew Weier O'Phinney

Run the code through php-cs-fixer, as this will fix a number of the issues I reported, or, at the minimum, alert you to their existence; ping somebody in #zftalk.dev on Freenode IRC if you need assistance.

Roel van Duijnhoven

The comments have all been processed @weierophinney and I have rebased against the latest master.

I could write some documentation about this storage adapter, no problem. Proposal would be to write a subsection under the identity persistence section on this page this page. Does that sound allright?

Matthew Weier O'Phinney

@roelvanduijnhoven Re: documentation -- yes, exactly there.

I'll merge momentarily!

Matthew Weier O'Phinney weierophinney referenced this pull request from a commit
Matthew Weier O'Phinney weierophinney [#2718] CS fixes
- Remove leading/trailing space inside method argument lists.
99d6505
Matthew Weier O'Phinney

Merged to develop branch, for release with 2.1.0.

Roel van Duijnhoven roelvanduijnhoven referenced this pull request in zendframework/zf2-documentation
Merged

Documentation for Chain Storage of Authentication module #597

Maks3w Maks3w referenced this pull request from a commit in Maks3w/zf2
Matthew Weier O'Phinney weierophinney Merge branch 'feature/2718' into develop
Close #2718
611d447
Deleted user Unknown referenced this pull request from a commit
Matthew Weier O'Phinney weierophinney [#2718] CS fixes
- Remove leading/trailing space inside method argument lists.
5c9914e
Deleted user Unknown referenced this pull request from a commit
Matthew Weier O'Phinney weierophinney Merge branch 'feature/2718' into develop
Close #2718
4a195bf
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
116 library/Zend/Authentication/Storage/Chain.php
View
@@ -0,0 +1,116 @@
+<?php
+/**
+ * Zend Framework (http://framework.zend.com/)
+ *
+ * @link http://github.com/zendframework/zf2 for the canonical source repository
+ * @copyright Copyright (c) 2005-2012 Zend Technologies USA Inc. (http://www.zend.com)
+ * @license http://framework.zend.com/license/new-bsd New BSD License
+ * @package Zend_Authentication
+ */
+
+namespace Zend\Authentication\Storage;
+
+use Zend\Stdlib\PriorityQueue;
+use Zend\Authentication\Storage\StorageInterface;
+
+/**
+ * @category Zend
+ * @package Zend_Authentication
+ * @subpackage Storage
+ */
+class Chain implements StorageInterface
+{
+ /**
+ * Contains all storage that this authentication method uses. A storage
+ * placed in the priority queue with a higher priority is always used
+ * before using a storage with a lower priority.
+ *
+ * @var PriorityQueue
+ */
+ protected $storageChain;
+
+ /**
+ * Initializes the priority queue.
+ */
+ public function __construct()
+ {
+ $this->storageChain = new PriorityQueue();
+ }
+
+ /**
+ * @param StorageInterface $storage
+ * @param integer $priority
+ */
+ public function add( StorageInterface $storage, $priority = 1 )
+ {
+ $this->storageChain->insert($storage, $priority);
+ }
+
+ /**
+ * Loop over the queue of storage until a storage is found that is non-empty. If such
+ * storage is not found, then this chain storage itself is empty.
+ *
+ * In case a non-empty storage is found then this chain storage is also non-empty. Report
+ * that, but also make sure that all storage with higher priorty that are empty
+ * are filled.
+ *
+ * @see StorageInterface::isEmpty()
+ */
+ public function isEmpty()
+ {
+ $storageWithHigherPriority = array();
+
+ // Loop invariant: $storageWithHigherPriority contains all storage with higher priorty
+ // than the current one.
+ foreach ($this->storageChain as $storage) {
+ if ( $storage->isEmpty() ) {
+ $storageWithHigherPriority[] = $storage;
+ continue;
+ }
+
+ $storageValue = $storage->read();
+ foreach ($storageWithHigherPriority as $higherPriorityStorage) {
+ $higherPriorityStorage->write($storageValue);
+ }
+
+ return false;
+ }
+
+ return true;
+ }
+
+ /**
+ * If the chain is non-empty then the storage with the top priority is guaranteed to be
+ * filled. Return its value.
+ *
+ * @see StorageInterface::read()
+ */
+ public function read()
+ {
+ return $this->storageChain->top()->read();
+ }
+
+ /**
+ * Write the new $contents to all storage in the chain.
+ *
+ * @see StorageInterface::write()
+ */
+ public function write( $contents )
+ {
+ foreach ($this->storageChain as $storage) {
+ $storage->write($contents);
+ }
+ }
+
+ /**
+ * Clear all storage in the chain.
+ *
+ * @see StorageInterface::clear()
+ */
+ public function clear()
+ {
+ foreach ($this->storageChain as $storage) {
+ $storage->clear();
+ }
+ }
+}
150 tests/ZendTest/Authentication/Storage/ChainTest.php
View
@@ -0,0 +1,150 @@
+<?php
+/**
+ * Zend Framework (http://framework.zend.com/)
+ *
+ * @link http://github.com/zendframework/zf2 for the canonical source repository
+ * @copyright Copyright (c) 2005-2013 Zend Technologies USA Inc. (http://www.zend.com)
+ * @license http://framework.zend.com/license/new-bsd New BSD License
+ * @package Zend_Uri
+ */
+
+namespace ZendTest\Authentication\Storage;
+
+use Zend\Authentication\Storage\Chain,
+ Zend\Authentication\Storage\StorageInterface,
+ Zend\Authentication\Storage\NonPersistent;
+
+use PHPUnit_Framework_TestCase as TestCase;
+
+/**
+ * @category Zend
+ * @package Zend_Auth
+ * @subpackage UnitTests
+ * @group Zend_Auth
+ */
+class ChainTest extends TestCase
+{
+ const ID = 1337;
+
+ /**
+ * Ensure chain without storage behavious as empty storage.
+ */
+ public function testEmptyChain()
+ {
+ $chain = new Chain;
+
+ $this->assertTrue($chain->isEmpty());
+ }
+
+ /**
+ * Ensure chain with single empty storage behavious as expected.
+ */
+ public function testSingularChainEmpty()
+ {
+ $chain = new Chain;
+ $chain->add($this->storageFactory());
+
+ $this->assertTrue($chain->isEmpty());
+ }
+
+ /**
+ * Ensure chain with single non-empty storage behavious as expected.
+ */
+ public function testSingularChainNonEmpty()
+ {
+ $chain = new Chain;
+ $chain->add($this->storageFactory(self::ID));
+
+ $this->assertFalse($chain->isEmpty());
+ $this->assertEquals(self::ID, $chain->read());
+ }
+
+ /**
+ * Ensure the priority of storage engines is correctly used.
+ */
+ public function testChainPriority()
+ {
+ $storageA = $this->storageFactory();
+ $storageB = $this->storageFactory(self::ID);
+
+ $chain = new Chain;
+ $chain->add($storageA); // Defaults to 1
+ $chain->add($storageB, 10);
+ $chain->isEmpty();
+
+ // Storage B has higher priority AND is non-empty. Thus
+ // storage A should been used at all and remain empty.
+ $this->assertTrue($storageA->isEmpty());
+ }
+
+ /**
+ * Ensure that a chain with empty storages is considered empty and
+ * won't populated any of its underlying storages.
+ */
+ public function testEmptyChainIsEmpty()
+ {
+ $emptyStorageA = $this->storageFactory();
+ $emptyStorageB = $this->storageFactory();
+
+ $chain = new Chain;
+ $chain->add($emptyStorageA);
+ $chain->add($emptyStorageB);
+
+ $this->assertTrue($chain->isEmpty());
+
+ // Storage A and B remain empty
+ $this->assertTrue($emptyStorageA->isEmpty());
+ $this->assertTrue($emptyStorageB->isEmpty());
+ }
+
+ /**
+ * Ensure that chain will yield non-empty if one of its underlying storage
+ * engines is non-empty.
+ *
+ * Make sure that storage engines with higher priority then the first non-empty
+ * storage engine get populated with that same content.
+ */
+ public function testSuccessfullReadWillPopulateStoragesWithHigherPriority()
+ {
+ $emptyStorageA = $this->storageFactory();
+ $emptyStorageB = $this->storageFactory();
+ $storageC = $this->storageFactory(self::ID);
+ $emptyStorageD = $this->storageFactory();
+
+ $chain = new Chain;
+ $chain->add($emptyStorageA);
+ $chain->add($emptyStorageB);
+ $chain->add($storageC);
+ $chain->add($emptyStorageD);
+
+ // Chain is non empty
+ $this->assertFalse($chain->isEmpty());
+ $this->assertEquals(self::ID, $chain->read());
+
+ // Storage A and B are filled
+ $this->assertFalse($emptyStorageA->isEmpty());
+ $this->assertEquals(self::ID, $emptyStorageA->read());
+ $this->assertFalse($emptyStorageA->isEmpty());
+ $this->assertEquals(self::ID, $emptyStorageB->read());
+
+ // Storage C and D remain identical
+ $this->assertFalse($storageC->isEmpty());
+ $this->assertEquals(self::ID, $storageC->read());
+ $this->assertTrue($emptyStorageD->isEmpty());
+ }
+
+ /**
+ * @param mixed $identity
+ * @return StorageInterface
+ */
+ protected function storageFactory( $identity = null )
+ {
+ $storage = new NonPersistent();
+
+ if ($identity !== null) {
+ $storage->write($identity);
+ }
+
+ return $storage;
+ }
+}
Something went wrong with that request. Please try again.