Skip to content

Add improvement suggestions and remove test code #2

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.

Sign up for GitHub

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

Open
FrankFlorian wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Add improvement suggestions and remove test code #2

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b1274f7
add improvement suggestions
FrankFlorian Mar 5, 2018
a4881d1
remove tests
FrankFlorian Mar 5, 2018
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
73 changes: 11 additions & 62 deletions lib/Horde/Auth/Cascading.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,65 +31,37 @@ class Horde_Auth_Cascading extends Horde_Auth_Base
* @param array $params Required parameters:
* <pre>
* 'drivers' - array hash of (Horde_Auth_Base) The list of backend drivers.
* 'order' - a list of drivers indexes to define a default order.
* </pre>
*
* 'capabilities' - defines capabilities this driver
* exposes and how to map them to the backends.
* Defaults to "order" for all drivers which support it.
* exposes and how to map them to the backends.
* Defaults to "order" for all drivers which support it.
* </pre>
*
* @throws InvalidArgumentException
*/
public function __construct(array $params = array())
{
$this->_test = $params;
foreach (array('drivers', 'order') as $val) {
if (!isset($params[$val])) {
throw new InvalidArgumentException('Missing ' . $val . ' parameter.');
}
if (!isset($params['drivers'])) {
throw new InvalidArgumentException('Missing ' . $params['drivers'] . ' parameter.');
}
$capabilities = array();
// Autodetect capabilities and build a default execution order
foreach ($this->_capabilities as $capabilityKey => $capability) {
foreach ($params['order'] as $driverKey) {
foreach ($params['drivers'] as $driverKey => $driver) {
if ($params['drivers'][$driverKey]->hasCapability($capabilityKey)) {
if (empty($capabilities[$capabilityKey])) {
/* TODO: resetpassword capability is debatable -
We actually call update on the backend drivers to
get the same password in all backends
Thus, automatically assign resetpassword capability
to drivers which have 'update'.
The user may override this manually.
Drivers which actually only provide resetpassword are
supported, but more than one brings unpredictable
results
*/
$capabilities[$capabilityKey] = array();
}
array_push($capabilities[$capabilityKey], $driverKey);
}
}
}

if (!empty($params['capabilities'])) {
// override default capabilities with provided capabilities
$capabilities = array_merge($capabilities, $params['capabilities']);

}
$params['capabilities'] = $capabilities;
// TODO: unset order, we don't use it after initialization
// Do base initialisation
unset($params['order']);
parent::__construct($params);
}

protected $_test = array(); //test array

public function getParams() //test function to see all arrays
{
return($this->_test);
}

/**
* Find out if a set of login credentials are valid.
* Valid means valid in any backend
Expand Down Expand Up @@ -133,8 +105,6 @@ public function hasCapability($capability)
*/
public function transparent()
{
// TODO: Check each configured driver in $this->_params['capabilities']['transparent']. Stop and return true if successful. Throw Exception if no driver available

if (!$this->hasCapability('transparent'))
{
throw new Horde_Auth_Exception('Unsupported.');
Expand Down Expand Up @@ -185,7 +155,7 @@ public function updateUser($oldID, $newID, $credentials)
// TODO try to add the user to all backends in $this->_params['capabilities']['update'] - throw exception if no driver available
if (!$this->hasCapability('update')) {
throw new Horde_Auth_Exception('Unsupported.');
}
}
foreach ($this->_params['capabilities']['update'] as $driverKey) {
try{
$this->_params['drivers'][$driverKey]->updateUser($oldID, $newID, $credentials);
Expand All @@ -205,11 +175,6 @@ public function updateUser($oldID, $newID, $credentials)
*/
public function resetPassword($userId)
{
// Implement this later:
// Check the list of auth drivers for drivers which has resetpassword but not update capability (configured).
// If exists, remove these drivers from list and run resetPassword on these driver. Use the first returned password for all drivers which have update (see below)
// Else, do as below

if (!$this->hasCapability('resetpassword')) {
throw new Horde_Auth_Exception('Unsupported.');
}
Expand All @@ -218,28 +183,17 @@ public function resetPassword($userId)
foreach ($this->_params['capabilities']['resetpassword'] as $resetKey) {
if (in_array($resetKey, $this->_params['capabilities']['update'])) {
array_push($resetButUpdate, $resetKey);
}
else {
} else {
try {
$newPassword = $this->_params['drivers'][$resetKey]->resetPassword($userId);
} catch (Horde_Auth_Exception $e) {
}
}
}
// Implement this first:
// Generate a random password ONCE
// Try to update the user password to all backends - throw exception if no driver available
// Return the new random password

// inspired by https://stackoverflow.com/questions/4356289/php-random-string-generator

if (!empty($resetButUpdate)) {
if (newPassword == '') {
$characters = '0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ';
$charactersLength = strlen($characters);
for ($i = 0; $i < 8; $i++) {
$newPassword .= $characters[rand(0, $charactersLength - 1)];
}
if ($newPassword == '') {
$newPassword = Horde_Auth::genRandomPassword();
}
$credentials = array('password' => $newPassword);
foreach ($resetButUpdate as $driverKey) {
Expand All @@ -261,7 +215,6 @@ public function resetPassword($userId)
*/
public function removeUser($userId)
{
// TODO try to remove the user from all backends in $this->_params['capabilities']['remove'] - throw exception if no driver available
if (!$this->hasCapability('remove')) {
throw new Horde_Auth_Exception('Unsupported.');
}
Expand All @@ -284,9 +237,6 @@ public function removeUser($userId)
*/
public function listUsers($sort = false)
{
//Todo list all users from all backends and merge them with array_unique
// Merge the results - don't list any user twice

if (!$this->hasCapability('list')) {
throw new Horde_Auth_Exception('Unsupported.');
}
Expand All @@ -310,8 +260,7 @@ public function listUsers($sort = false)
*/
public function exists($userId)
{
// rotate through all backends which have list capabddlity or exists capability - return true if any backend has this user, otherwise return false.
if (!$this->hasCapability('list') and !$this->hasCapability('exists')) {
if (!$this->hasCapability('list') && !$this->hasCapability('exists')) {
throw new Horde_Auth_Exception('Unsupported.');
}
if ($this->hasCapability('exists')) {
Expand Down
18 changes: 12 additions & 6 deletions test/Horde/Auth/Unit/CascadingMockTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ function setUp() {
$this->cascading = new Horde_Auth_Cascading(
array(
'drivers' => array('admins' => $d1, 'db' => $d2, 'cryptdb' => $d3),
'order' => array('admins', 'db', 'cryptdb'),
)
);
}
Expand All @@ -52,7 +51,7 @@ public function testHasCapability()
public function testTransparent()
{
//throw exception if no backend provides transparent
$this->setExpectedException(Horde_Auth_Exception::class);
$this->setExpectedException('Horde_Auth_Exception');
$this->cascading->transparent();
}

Expand Down Expand Up @@ -107,15 +106,22 @@ public function testUpdateUser()

public function testUpdateUserFailDoesNotExist()
{
// Try renaming unknown user
// $this->setExpectedException(Horde_Auth_Exception::class);
// Try renaming unknown user in unknown user
// Execption should be catched
$this->cascading->updateUser('unknownuser', 'newname', array('password' => 'foo'));
$this->setUp();
$this->assertFalse($this->cascading->exists('unknownuser'));
$this->assertFalse($this->cascading->exists('newname'));
// Try renaming unknown user in known user to change user1´ss password
// Execption should be catched
$this->cascading->updateUser('unknownuser', 'user1', array('password' => 'foo'));
$this->assertFalse($this->cascading->exists('unknownuser'));
$this->assertFalse($this->cascading->authenticate('user1', array('password' => 'foo')));
$this->assertTrue($this->cascading->authenticate('user1', array('password' => 'pw1')));
}

public function testResetPassword()
{
$this->setExpectedException(Horde_Auth_Exception::class);
$this->setExpectedException('Horde_Auth_Exception');
$newPassword = $this->cascading->resetPassword('user1');
//old Password should not work
$this->assertFalse($this->cascading->authenticate('user1' , array('password' => 'pw1')));
Expand Down