Skip to content

Commit

Permalink
Simplify second factor auth API
Browse files Browse the repository at this point in the history
We now only pass SecondFactor object to plugins, not individual
parameters.

Signed-off-by: Michal Čihař <michal@cihar.com>
  • Loading branch information
nijel committed Nov 1, 2017
1 parent 540b78d commit b5e5f4e
Show file tree
Hide file tree
Showing 5 changed files with 41 additions and 72 deletions.
23 changes: 12 additions & 11 deletions libraries/classes/Plugins/SecondFactor/Application.php
Expand Up @@ -8,6 +8,7 @@
namespace PhpMyAdmin\Plugins\SecondFactor;

use PhpMyAdmin\Message;
use PhpMyAdmin\SecondFactor;
use PhpMyAdmin\Template;
use PhpMyAdmin\Plugins\SecondFactorPlugin;
use PragmaRX\Google2FA\Google2FA;
Expand All @@ -31,14 +32,16 @@ class Application extends SecondFactorPlugin
/**
* Creates object
*
* @param string $user User name
* @param array $config Second factor configuration
* @param SecondFactor $second SecondFactor instance
*/
public function __construct($user, $config)
public function __construct(SecondFactor $second)
{
parent::__construct($user, $config);
parent::__construct($second);
$this->_google2fa = new Google2FA();
$this->_google2fa->setWindow(8);
if (!isset($this->_second->config['settings']['secret'])) {
$this->_second->config['settings']['secret'] = '';
}
}

/**
Expand All @@ -53,8 +56,6 @@ public function __get($property)
switch ($property) {
case 'google2fa':
return $this->_google2fa;
case 'config':
return $this->_config;
}
}

Expand All @@ -66,12 +67,12 @@ public function __get($property)
public function check()
{
$this->_provided = false;
if (!isset($_POST['2fa_code']) || !isset($this->_config['secret'])) {
if (!isset($_POST['2fa_code'])) {
return false;
}
$this->_provided = true;
return $this->_google2fa->verifyKey(
$this->_config['secret'], $_POST['2fa_code']
$this->_second->config['settings']['secret'], $_POST['2fa_code']
);
}

Expand Down Expand Up @@ -104,8 +105,8 @@ public function setup()
}
$inlineUrl = $this->_google2fa->getQRCodeInline(
'phpMyAdmin',
$this->_user,
$this->_config['secret']
$this->_second->user,
$this->_second->config['settings']['secret']
);
return Template::get('login/second/application_configure')->render([
'image' => $inlineUrl,
Expand All @@ -122,7 +123,7 @@ public function configure()
if (! isset($_SESSION['2fa_application_key'])) {
$_SESSION['2fa_application_key'] = $this->_google2fa->generateSecretKey();
}
$this->_config['secret'] = $_SESSION['2fa_application_key'];
$this->_second->config['settings']['secret'] = $_SESSION['2fa_application_key'];

$result = $this->check();
if ($result) {
Expand Down
31 changes: 8 additions & 23 deletions libraries/classes/Plugins/SecondFactor/Key.php
Expand Up @@ -10,6 +10,7 @@
use PhpMyAdmin\Core;
use PhpMyAdmin\Message;
use PhpMyAdmin\Response;
use PhpMyAdmin\SecondFactor;
use PhpMyAdmin\Template;
use PhpMyAdmin\Plugins\SecondFactorPlugin;
use Samyoul\U2F\U2FServer\U2FServer;
Expand All @@ -31,29 +32,13 @@ class Key extends SecondFactorPlugin
/**
* Creates object
*
* @param string $user User name
* @param array $config Second factor configuration
* @param SecondFactor $second SecondFactor instance
*/
public function __construct($user, $config)
public function __construct(SecondFactor $second)
{
if (!isset($config['registrations'])) {
$config['registrations'] = [];
}
parent::__construct($user, $config);
}

/**
* Get any property of this class
*
* @param string $property name of the property
*
* @return mixed|void if property exist, value of the relevant property
*/
public function __get($property)
{
switch ($property) {
case 'config':
return $this->_config;
parent::__construct($second);
if (!isset($this->_second->config['settings']['registrations'])) {
$this->_second->config['settings']['registrations'] = [];
}
}

Expand All @@ -65,7 +50,7 @@ public function __get($property)
public function getRegistrations()
{
$result = [];
foreach ($this->_config['registrations'] as $data) {
foreach ($this->_second->config['settings']['registrations'] as $data) {
$reg = new \StdClass;
$reg->keyHandle = $data['keyHandle'];
$reg->publicKey = $data['publicKey'];
Expand Down Expand Up @@ -207,7 +192,7 @@ public function configure()
$registration = U2FServer::register(
$_SESSION['registrationRequest'], $response
);
$this->_config['registrations'][] = [
$this->_second->config['settings']['registrations'][] = [
'keyHandle' => $registration->getKeyHandle(),
'publicKey' => $registration->getPublicKey(),
'certificate' => $registration->getCertificate(),
Expand Down
28 changes: 7 additions & 21 deletions libraries/classes/Plugins/SecondFactorPlugin.php
Expand Up @@ -7,6 +7,8 @@
*/
namespace PhpMyAdmin\Plugins;

use PhpMyAdmin\SecondFactor;

/**
* Second factor authentication plugin class
*
Expand All @@ -22,24 +24,18 @@ class SecondFactorPlugin
public static $id = '';

/**
* @var string
* @var SecondFactor
*/
protected $_user;
/**
* @var array
*/
protected $_config;
protected $_second;

/**
* Creates object
*
* @param string $user User name
* @param array $config Second factor configuration
* @param SecondFactor $second SecondFactor instance
*/
public function __construct($user, $config)
public function __construct(SecondFactor $second)
{
$this->_user = $user;
$this->_config = $config;
$this->_second = $second;
}

/**
Expand Down Expand Up @@ -82,16 +78,6 @@ public function configure()
return true;
}

/**
* Return current configuration
*
* @return array
*/
public function getConfig()
{
return $this->_config;
}

/**
* Get user visible name
*
Expand Down
29 changes: 13 additions & 16 deletions libraries/classes/SecondFactor.php
Expand Up @@ -17,12 +17,12 @@ class SecondFactor
/**
* @var string
*/
protected $_user;
public $user;

/**
* @var array
*/
protected $_config;
public $config;

/**
* @var boolean
Expand All @@ -46,10 +46,10 @@ class SecondFactor
*/
public function __construct($user)
{
$this->_user = $user;
$this->user = $user;
$this->_available = $this->getAvailable();
$this->_config = $this->readConfig();
$this->_writable = ($this->_config['type'] == 'db');
$this->config = $this->readConfig();
$this->_writable = ($this->config['type'] == 'db');
$this->_backend = $this->getBackend();
}

Expand Down Expand Up @@ -91,8 +91,6 @@ public function __get($property)
return $this->_available;
case 'writable':
return $this->_writable;
case 'config':
return $this->_config;
}
}

Expand Down Expand Up @@ -139,8 +137,8 @@ public function getBackendClass($name)
*/
public function getBackend()
{
$name = $this->getBackendClass($this->_config['backend']);
return new $name($this->_user, $this->_config['settings']);
$name = $this->getBackendClass($this->config['backend']);
return new $name($this);
}

/**
Expand Down Expand Up @@ -193,29 +191,28 @@ public function setup()
*/
public function configure($name)
{
$config = [
$this->config = [
'backend' => $name
];
if ($name === '') {
$cls = $this->getBackendClass($name);
$this->_backend = new $cls($this->_user, []);
$config['settings'] = [];
$this->config['settings'] = [];
$this->_backend = new $cls($this);
} else {
if (! in_array($name, $this->_available)) {
return false;
}
$cls = $this->getBackendClass($name);
$this->_backend = new $cls($this->_user, []);
$this->config['settings'] = [];
$this->_backend = new $cls($this);
if (! $this->_backend->configure()) {
return false;
}
$config['settings'] = $this->_backend->getConfig();
}
$result = UserPreferences::persistOption('2fa', $config, null);
$result = UserPreferences::persistOption('2fa', $this->config, null);
if ($result !== true) {
$result->display();
}
$this->_config = $config['settings'];
return true;
}
}
2 changes: 1 addition & 1 deletion test/classes/SecondFactorTest.php
Expand Up @@ -104,7 +104,7 @@ public function testApplication()
/* Generate valid code */
$google2fa = $object->backend->google2fa;
$_POST['2fa_code'] = $google2fa->oathHotp(
$object->backend->config['secret'],
$object->config['settings']['secret'],
$google2fa->getTimestamp()
);
$this->assertTrue($object->configure('application'));
Expand Down

0 comments on commit b5e5f4e

Please sign in to comment.