Skip to content
This repository has been archived by the owner on Jun 16, 2024. It is now read-only.

Various cleanup #27

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 16 additions & 5 deletions Module.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,9 @@ class Module implements
AutoloaderProviderInterface,
ConfigProviderInterface
{
public function getConfig()
{
return include __DIR__ . '/config/module.config.php';
}

/**
* {@inheritDoc}
*/
public function getAutoloaderConfig()
{
return array(
Expand All @@ -32,6 +30,19 @@ public function getAutoloaderConfig()
);
}

/**
* {@inheritDoc}
*/
public function getConfig()
{
return include __DIR__ . '/config/module.config.php';
}

/**
* Get the Liquibase path
*
* @return string
*/
public function getLiquibasePath()
{
return __DIR__ . '/liquibase/changelog.xml';
Expand Down
15 changes: 15 additions & 0 deletions config/baconuser.global.php.dist
Original file line number Diff line number Diff line change
@@ -1,6 +1,21 @@
<?php
return array(
'bacon_user' => array(
// User settings
'user' => array(
// Enable or disable the username field for users
'enable_username' => true,

// Enable or disable the user state
'enable_user_state' => true,

// Default user state (the state must be in allowed_login_states array)
'default_user_state' => 1,

// The allowed login states
'allowed_login_states' => array(null, 1)
),

// Password settings
'password' => array(
// This is a Zend\ServiceManager compatible configuration containing
Expand Down
7 changes: 1 addition & 6 deletions src/BaconUser/Password/Factory/BcryptFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,9 @@ class BcryptFactory implements FactoryInterface
*/
public function createService(ServiceLocatorInterface $serviceLocator)
{
$config = $serviceLocator->getServiceLocator()->get('BaconUser\Config');
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This produces an error for me in PHPUnit:
Fatal error: Call to undefined method Zend\ServiceManager\ServiceManager::getServiceLocator() in /Password/Factory/BcryptFactory.php on line 31

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$serviceLocator is not a plugin manager then... It should! So lileky the object that ask this bcrypt factory uses the global service manager instead of using the proper plugin manageR.

$bcrypt = new Bcrypt();

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed this check because this is not needed. "bcrypt" is inside a plugin manager so the service locator will always be a plugin manager. People that want to add a new BcryptHandler will add it in the handler plugin manager so this is no problem.

if ($serviceLocator instanceof AbstractPluginManager) {
$config = $serviceLocator->getServiceLocator()->get('BaconUser\Config');
} else {
$config = $serviceLocator->get('BaconUser\Config');
}

if (isset($config['password']['bcrypt'])) {
$config = $config['password']['bcrypt'];

Expand Down
12 changes: 6 additions & 6 deletions src/BaconUser/Password/HandlerAggregate.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
use BaconUser\Password\Options\PasswordHandlerAggregateOptionsInterface;

/**
* Aggregate password handler for using multiple hashing methods parallely.
* Aggregate password handler for using multiple hashing methods in parallel.
*/
class HandlerAggregate implements HandlerInterface
{
Expand Down Expand Up @@ -87,7 +87,7 @@ public function shouldRehash($hash)
$handler = $this->getHandlerByHash($hash);

if ($handler === null) {
// Hash is not uspported by any method, migration recommended.
// Hash is not supported by any method, migration recommended.
return true;
}

Expand All @@ -107,7 +107,7 @@ public function shouldRehash($hash)
}

/**
* @return HashManager
* @return HandlerManager
*/
public function getHandlerManager()
{
Expand Down Expand Up @@ -156,11 +156,11 @@ public function setOptions(PasswordHandlerAggregateOptionsInterface $options)
*/
protected function getHandlerByName($hashingMethod)
{
if (!isset($this->hashCache[$hashingMethod])) {
$this->hashCache[$hashingMethod] = $this->getHandlerManager()->get($hashingMethod);
if (!isset($this->handlerCache[$hashingMethod])) {
$this->handlerCache[$hashingMethod] = $this->getHandlerManager()->get($hashingMethod);
}

return $this->hashCache[$hashingMethod];
return $this->handlerCache[$hashingMethod];
}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/BaconUser/Password/HandlerInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ public function compare($password, $hash);
*
* This is used primarily by the authentication adapter, which will verify
* whether it should re-hash the plain-text password. This is useful if you
* are migrating to a more secure algorythm, or when the parameters for your
* are migrating to a more secure algorithm, or when the parameters for your
* current hash changed.
*
* @param string $hash
Expand Down
2 changes: 1 addition & 1 deletion src/BaconUser/Password/HandlerManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ class HandlerManager extends AbstractPluginManager
* @see AbstractPluginManager::validatePlugin()
* @param mixed $plugin
* @return void
* @throws RuntimeException
* @throws Exception\RuntimeException
*/
public function validatePlugin($plugin)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ public function getHashingMethods()

/**
* @param array $hashingMethods
* @return HandlerAggregate
* @return \BaconUser\Password\HandlerAggregate
*/
public function setHashingMethods(array $hashingMethods)
{
Expand All @@ -63,7 +63,7 @@ public function getDefaultHashingMethod()

/**
* @param string $defaultHashingMethod
* @return HandlerAggregate
* @return \BaconUser\Password\HandlerAggregate
*/
public function setDefaultHashingMethod($defaultHashingMethod)
{
Expand All @@ -81,7 +81,7 @@ public function getMigrateToDefaultHashingMethod()

/**
* @param bool $migrateToDefaultHashingMethod
* @return HandlerAggregate
* @return \BaconUser\Password\HandlerAggregate
*/
public function setMigrateToDefaultHashingMethod($migrateToDefaultHashingMethod)
{
Expand Down
16 changes: 6 additions & 10 deletions tests/BaconUserTest/Password/Factory/BcryptFactoryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,6 @@
*/
class BcryptFactoryTest extends TestCase
{
public function testFactoryFromServiceLocator()
{
$locator = new ServiceManager();
$locator->setService('BaconUser\Config', array());

$factory = new BcryptFactory();
$this->assertInstanceOf('BaconUser\Password\Bcrypt', $factory->createService($locator));
}

public function testFactoryFromPluginManager()
{
$locator = new ServiceManager();
Expand All @@ -46,7 +37,12 @@ public function testFactoryWithCostSet()
$locator = new ServiceManager();
$locator->setService('BaconUser\Config', array('password' => array('bcrypt' => array('cost' => 4))));

$pluginManager = $this->getMock('Zend\ServiceManager\AbstractPluginManager');
$pluginManager->expects($this->once())
->method('getServiceLocator')
->will($this->returnValue($locator));

$factory = new BcryptFactory();
$this->assertInstanceOf('BaconUser\Password\Bcrypt', $factory->createService($locator));
$this->assertInstanceOf('BaconUser\Password\Bcrypt', $factory->createService($pluginManager));
}
}