Page MenuHomePhorge
Files F16740456

D4932.diff
No OneTemporary
Actions

D4932.diff
View Options

diff --git a/src/app/Auth/SecondFactor.php b/src/app/Auth/SecondFactor.php
--- a/src/app/Auth/SecondFactor.php
+++ b/src/app/Auth/SecondFactor.php
@@ -3,12 +3,13 @@
namespace App\Auth;
use Illuminate\Support\Facades\Auth;
-use Kolab2FA\Storage\Base;
+use Kolab2FA\Driver\Base as DriverBase;
+use Kolab2FA\Storage\Base as StorageBase;
/**
* A class to maintain 2-factor authentication
*/
-class SecondFactor extends Base
+class SecondFactor extends StorageBase
{
protected $user;
protected $cache = [];
@@ -25,6 +26,7 @@
public function __construct($user)
{
$this->user = $user;
+ $this->username = $this->user->email;
parent::__construct();
}
@@ -130,7 +132,7 @@
*
* @param string $factor Factor identifier (<method>:<id>)
*
- * @return \Kolab2FA\Driver\Base
+ * @return DriverBase
*/
protected function getDriver(string $factor)
{
@@ -138,13 +140,7 @@
$config = \config('2fa.' . $method, []);
- $driver = \Kolab2FA\Driver\Base::factory($factor, $config);
-
- // configure driver
- $driver->storage = $this;
- $driver->username = $this->user->email;
-
- return $driver;
+ return DriverBase::factory($this, $factor, $config);
}
/**
@@ -161,6 +157,7 @@
'label' => 'Mobile app (TOTP)',
'created' => 1584573552,
'secret' => 'UAF477LDHZNWVLNA',
+ 'digest' => 'sha1',
'active' => true,
],
// 'dummy:dummy' => [
@@ -189,6 +186,8 @@
public static function code(string $email): string
{
$sf = new self(\App\User::where('email', $email)->first());
+
+ /** @var \Kolab2FA\Driver\TOTP $driver */
$driver = $sf->getDriver('totp:8132a46b1f741f88de25f47e');
return (string) $driver->get_code();
@@ -304,6 +303,7 @@
}
$prefs = array_merge($old_prefs, $prefs);
+ $prefs = array_filter($prefs, fn($v) => !is_null($v));
$this->dbh()->table('users')
->where('username', strtolower($this->user->email))
diff --git a/src/include/Kolab2FA/Driver/Base.php b/src/include/Kolab2FA/Driver/Base.php
--- a/src/include/Kolab2FA/Driver/Base.php
+++ b/src/include/Kolab2FA/Driver/Base.php
@@ -23,58 +23,61 @@
namespace Kolab2FA\Driver;
+/**
+ * Kolab 2-Factor-Authentication Driver base class
+ */
abstract class Base
{
public $method;
public $id;
public $storage;
- public $username;
- protected $config = array();
- protected $props = array();
- protected $user_props = array();
+ protected $config = [];
+ protected $config_keys = [];
+ protected $props = [];
+ protected $user_props = [];
protected $pending_changes = false;
protected $temporary = false;
- protected $allowed_props = array('username');
+ protected $allowed_props = ['username'];
- public $user_settings = array(
- 'active' => array(
+ public $user_settings = [
+ 'active' => [
'type' => 'boolean',
'editable' => false,
'hidden' => false,
'default' => false,
- ),
- 'label' => array(
+ ],
+ 'label' => [
'type' => 'text',
'editable' => true,
'label' => 'label',
'generator' => 'default_label',
- ),
- 'created' => array(
+ ],
+ 'created' => [
'type' => 'datetime',
'editable' => false,
'hidden' => false,
'label' => 'created',
'generator' => 'time',
- ),
- );
+ ],
+ ];
/**
* Static factory method
*/
- public static function factory($id, $config)
+ public static function factory($storage, $id, $config)
{
- list($method) = explode(':', $id);
+ [$method] = explode(':', $id);
- $classmap = array(
+ $classmap = [
'totp' => '\\Kolab2FA\\Driver\\TOTP',
'hotp' => '\\Kolab2FA\\Driver\\HOTP',
'yubikey' => '\\Kolab2FA\\Driver\\Yubikey',
- );
+ ];
$cls = $classmap[strtolower($method)];
if ($cls && class_exists($cls)) {
- return new $cls($config, $id);
+ return new $cls($storage, $config, $id);
}
throw new Exception("Unknown 2FA driver '$method'");
@@ -83,61 +86,77 @@
/**
* Default constructor
*/
- public function __construct($config = null, $id = null)
+ public function __construct($storage, $config = null, $id = null)
{
- $this->init($config);
+ if (!is_array($config)) {
+ $config = [];
+ }
+
+ $this->storage = $storage;
+ $this->props['username'] = (string) $storage->username;
if (!empty($id) && $id != $this->method) {
$this->id = $id;
- }
- else { // generate random ID
+ if ($this->storage) {
+ $this->user_props = (array) $this->storage->read($this->id);
+ foreach ($this->config_keys as $key) {
+ if (isset($this->user_props[$key])) {
+ $config[$key] = $this->user_props[$key];
+ }
+ }
+ }
+ } else { // generate random ID
$this->id = $this->method . ':' . bin2hex(openssl_random_pseudo_bytes(12));
$this->temporary = true;
}
+
+ $this->init($config);
}
/**
* Initialize the driver with the given config options
*/
- public function init($config)
+ protected function init($config)
{
if (is_array($config)) {
$this->config = array_merge($this->config, $config);
}
-
- if (!empty($config['storage'])) {
- $this->storage = \Kolab2FA\Storage\Base::factory($config['storage'], $config['storage_config']);
- }
}
/**
* Verify the submitted authentication code
*
- * @param string $code The 2nd authentication factor to verify
- * @param int $timestamp Timestamp of authentication process (window start)
- * @return boolean True if valid, false otherwise
+ * @param string $code The 2nd authentication factor to verify
+ * @param int $timestamp Timestamp of authentication process (window start)
+ *
+ * @return bool True if valid, false otherwise
*/
- abstract function verify($code, $timestamp = null);
+ abstract public function verify($code, $timestamp = null);
+
+ /**
+ * Implement this method if the driver can be provisioned via QR code
+ */
+ /* abstract function get_provisioning_uri(); */
/**
* Getter for user-visible properties
*/
public function props($force = false)
{
- $data = array();
+ $data = [];
foreach ($this->user_settings as $key => $p) {
if (!empty($p['private'])) {
continue;
}
- $data[$key] = array(
+ $data[$key] = [
'type' => $p['type'],
- 'editable' => $p['editable'],
- 'hidden' => $p['hidden'],
- 'label' => $p['label'],
+ 'editable' => $p['editable'] ?? false,
+ 'hidden' => $p['hidden'] ?? false,
+ 'label' => $p['label'] ?? '',
'value' => $this->get($key, $force),
- );
+ ];
// format value into text
switch ($p['type']) {
@@ -152,6 +171,7 @@
break;
}
+ // no break
default:
$data[$key]['text'] = $data[$key]['value'];
}
@@ -160,28 +180,24 @@
return $data;
}
- /**
- * Implement this method if the driver can be prpvisioned via QR code
- */
- /* abstract function get_provisioning_uri(); */
-
/**
* Generate a random secret string
*/
public function generate_secret($length = 16)
{
// Base32 characters
- $chars = array(
+ $chars = [
'A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', // 7
'I', 'J', 'K', 'L', 'M', 'N', 'O', 'P', // 15
'Q', 'R', 'S', 'T', 'U', 'V', 'W', 'X', // 23
'Y', 'Z', '2', '3', '4', '5', '6', '7', // 31
- );
+ ];
$secret = '';
for ($i = 0; $i < $length; $i++) {
$secret .= $chars[array_rand($chars)];
}
+
return $secret;
}
@@ -197,15 +213,6 @@
return strtoupper($this->method);
}
- /**
- * Get current code (for testing)
- */
- public function get_code()
- {
- // to be overriden by a driver
- return '';
- }
-
/**
* Getter for read-only access to driver properties
*/
@@ -216,10 +223,10 @@
$value = $this->get_user_prop($key);
// generate property value
- if (!isset($value) && $force && $this->user_settings[$key]['generator']) {
+ if (!isset($value) && $force && !empty($this->user_settings[$key]['generator'])) {
$func = $this->user_settings[$key]['generator'];
if (is_string($func) && !is_callable($func)) {
- $func = array($this, $func);
+ $func = [$this, $func];
}
if (is_callable($func)) {
$value = call_user_func($func);
@@ -228,9 +235,12 @@
$this->set_user_prop($key, $value);
}
}
- }
- else {
- $value = $this->props[$key];
+ } else {
+ $value = $this->get_user_prop($key);
+
+ if ($value === null) {
+ $value = $this->props[$key] ?? null;
+ }
}
return $value;
@@ -251,9 +261,8 @@
$setter = 'set_' . $key;
if (method_exists($this, $setter)) {
- call_user_func(array($this, $setter), $value);
- }
- else if (in_array($key, $this->allowed_props)) {
+ call_user_func([$this, $setter], $value);
+ } elseif (in_array($key, $this->allowed_props)) {
$this->props[$key] = $value;
}
@@ -266,7 +275,17 @@
public function commit()
{
if (!empty($this->user_props) && $this->storage && $this->pending_changes) {
- if ($this->storage->write($this->id, $this->user_props)) {
+ $props = $this->user_props;
+
+ // Remamber the driver config too. It will be used to verify the code.
+ // The configured one may be different than the one used on code creation.
+ foreach ($this->config_keys as $key) {
+ if (isset($this->config[$key])) {
+ $props[$key] = $this->config[$key];
+ }
+ }
+
+ if ($this->storage->write($this->id, $props)) {
$this->pending_changes = false;
$this->temporary = false;
}
@@ -276,29 +295,23 @@
}
/**
- * Dedicated setter for the username property
+ * Clear data stored for this driver
*/
- public function set_username($username)
+ public function clear()
{
- $this->props['username'] = $username;
-
if ($this->storage) {
- $this->storage->set_username($username);
+ return $this->storage->remove($this->id);
}
- return true;
+ return false;
}
/**
- * Clear data stored for this driver
+ * Checks that a string contains a semicolon
*/
- public function clear()
+ protected function hasSemicolon($value)
{
- if ($this->storage) {
- return $this->storage->remove($this->id);
- }
-
- return false;
+ return preg_match('/(:|%3A)/i', (string) $value) > 0;
}
/**
@@ -318,37 +331,8 @@
*/
protected function set_user_prop($key, $value)
{
- $this->pending_changes |= ($this->user_props[$key] !== $value);
+ $this->pending_changes |= (($this->user_props[$key] ?? null) !== $value);
$this->user_props[$key] = $value;
return true;
}
-
- /**
- * Magic getter for read-only access to driver properties
- */
- public function __get($key)
- {
- // this is a per-user property: get from persistent storage
- if (isset($this->user_settings[$key])) {
- return $this->get_user_prop($key);
- }
-
- return $this->props[$key];
- }
-
- /**
- * Magic setter for restricted access to driver properties
- */
- public function __set($key, $value)
- {
- $this->set($key, $value, false);
- }
-
- /**
- * Magic check if driver property is defined
- */
- public function __isset($key)
- {
- return isset($this->props[$key]);
- }
}
diff --git a/src/include/Kolab2FA/Driver/Exception.php b/src/include/Kolab2FA/Driver/Exception.php
--- a/src/include/Kolab2FA/Driver/Exception.php
+++ b/src/include/Kolab2FA/Driver/Exception.php
@@ -4,5 +4,4 @@
class Exception extends \Exception
{
-
}
diff --git a/src/include/Kolab2FA/Driver/HOTP.php b/src/include/Kolab2FA/Driver/HOTP.php
--- a/src/include/Kolab2FA/Driver/HOTP.php
+++ b/src/include/Kolab2FA/Driver/HOTP.php
@@ -27,12 +27,13 @@
{
public $method = 'hotp';
- protected $config = array(
+ protected $config = [
'digits' => 6,
'window' => 4,
'digest' => 'sha1',
- );
+ ];
+ protected $config_keys = ['digits', 'digest'];
protected $backend;
/**
@@ -42,27 +43,39 @@
{
parent::init($config);
- $this->user_settings += array(
- 'secret' => array(
+ $this->user_settings += [
+ 'secret' => [
'type' => 'text',
'private' => true,
'label' => 'secret',
'generator' => 'generate_secret',
- ),
- 'counter' => array(
+ ],
+ 'counter' => [
'type' => 'integer',
'editable' => false,
'hidden' => true,
'generator' => 'random_counter',
- ),
- );
+ ],
+ ];
+
+ if (!in_array($this->config['digest'], ['md5', 'sha1', 'sha256', 'sha512'])) {
+ throw new \Exception("'{$this->config['digest']}' digest is not supported.");
+ }
+
+ if (!is_numeric($this->config['digits']) || $this->config['digits'] < 1) {
+ throw new \Exception('Digits must be at least 1.');
+ }
+
+ if ($this->hasSemicolon($this->config['issuer'])) {
+ throw new \Exception('Issuer must not contain a semi-colon.');
+ }
// copy config options
$this->backend = \OTPHP\HOTP::create(
- null,
- 0,
- $this->config['digest'],
- $this->config['digits']
+ null, // secret
+ 0, // counter
+ $this->config['digest'], // digest
+ $this->config['digits'] // digits
);
$this->backend->setIssuer($this->config['issuer']);
@@ -76,41 +89,33 @@
{
// get my secret from the user storage
$secret = $this->get('secret');
- $counter = (int) $this->get('counter');
if (!strlen($secret)) {
- // LOG: "no secret set for user $this->username"
- // rcube::console("VERIFY HOTP: no secret set for user $this->username");
return false;
}
try {
- $this->backend->setLabel($this->username);
+ $this->backend->setLabel($this->get('username'));
$this->backend->setSecret($secret);
- $this->backend->setParameter('counter', $counter);
- $pass = $this->backend->verify($code, $counter, $this->config['window']);
+ $pass = $this->backend->verify($code, $this->get('counter'), (int) $this->config['window']);
// store incremented counter value
$this->set('counter', $this->backend->getCounter());
$this->commit();
- }
- catch (\Exception $e) {
- // LOG: exception
- // rcube::console("VERIFY HOTP: $this->id, " . strval($e));
+ } catch (\Exception $e) {
$pass = false;
}
- // rcube::console('VERIFY HOTP', $this->username, $secret, $counter, $code, $pass);
return $pass;
}
/**
- *
+ * Get the provisioning URI.
*/
public function get_provisioning_uri()
{
- if (!$this->secret) {
+ if (!$this->get('secret')) {
// generate new secret and store it
$this->set('secret', $this->get('secret', true));
$this->set('counter', $this->get('counter', true));
@@ -120,9 +125,9 @@
// TODO: deny call if already active?
- $this->backend->setLabel($this->username);
- $this->backend->setSecret($this->secret);
- $this->backend->setParameter('counter', (int) $this->get('counter'));
+ $this->backend->setLabel($this->get('username'));
+ $this->backend->setSecret($this->get('secret'));
+ $this->backend->setCounter(intval($this->get('counter')));
return $this->backend->getProvisioningUri();
}
diff --git a/src/include/Kolab2FA/Driver/TOTP.php b/src/include/Kolab2FA/Driver/TOTP.php
--- a/src/include/Kolab2FA/Driver/TOTP.php
+++ b/src/include/Kolab2FA/Driver/TOTP.php
@@ -27,12 +27,13 @@
{
public $method = 'totp';
- protected $config = array(
+ protected $config = [
'digits' => 6,
'interval' => 30,
'digest' => 'sha1',
- );
+ ];
+ protected $config_keys = ['digits', 'digest'];
protected $backend;
/**
@@ -42,21 +43,37 @@
{
parent::init($config);
- $this->user_settings += array(
- 'secret' => array(
+ $this->user_settings += [
+ 'secret' => [
'type' => 'text',
'private' => true,
'label' => 'secret',
'generator' => 'generate_secret',
- ),
- );
+ ],
+ ];
+
+ if (!in_array($this->config['digest'], ['md5', 'sha1', 'sha256', 'sha512'])) {
+ throw new \Exception("'{$this->config['digest']}' digest is not supported.");
+ }
+
+ if (!is_numeric($this->config['digits']) || $this->config['digits'] < 1) {
+ throw new \Exception('Digits must be at least 1.');
+ }
+
+ if (!is_numeric($this->config['interval']) || $this->config['interval'] < 1) {
+ throw new \Exception('Interval must be at least 1.');
+ }
+
+ if ($this->hasSemicolon($this->config['issuer'])) {
+ throw new \Exception('Issuer must not contain a semi-colon.');
+ }
// copy config options
$this->backend = \OTPHP\TOTP::create(
- null,
- $this->config['interval'],
- $this->config['digest'],
- $this->config['digits']
+ null, //secret
+ $this->config['interval'], // period
+ $this->config['digest'], // digest
+ $this->config['digits'] // digits
);
$this->backend->setIssuer($this->config['issuer']);
@@ -72,16 +89,15 @@
$secret = $this->get('secret');
if (!strlen($secret)) {
- // LOG: "no secret set for user $this->username"
- // rcube::console("VERIFY TOTP: no secret set for user $this->username");
return false;
}
- $this->backend->setLabel($this->username);
- $this->backend->setParameter('secret', $secret);
+ $this->backend->setLabel($this->get('username'));
+ $this->backend->setSecret($secret);
- // Pass a window to indicate the maximum timeslip between client (device) and server.
- $pass = $this->backend->verify((string) $code, $timestamp, 150);
+ // Pass a window to indicate the maximum timeslip between client (mobile
+ // device) and server.
+ $pass = $this->backend->verify($code, (int) $timestamp, 150);
// try all codes from $timestamp till now
if (!$pass && $timestamp) {
@@ -92,47 +108,44 @@
}
}
- // rcube::console('VERIFY TOTP', $this->username, $secret, $code, $timestamp, $pass);
return $pass;
}
/**
- * Get current code (for testing)
+ * Get the provisioning URI.
*/
- public function get_code()
+ public function get_provisioning_uri()
{
- // get my secret from the user storage
- $secret = $this->get('secret');
-
- if (!strlen($secret)) {
- return;
+ if (!$this->get('secret')) {
+ // generate new secret and store it
+ $this->set('secret', $this->get('secret', true));
+ $this->set('created', $this->get('created', true));
+ $this->commit();
}
- $this->backend->setLabel($this->username);
- $this->backend->setParameter('secret', $secret);
+ // TODO: deny call if already active?
- return $this->backend->at(time());
+ $this->backend->setLabel($this->get('username'));
+ $this->backend->setSecret($this->get('secret'));
+
+ return $this->backend->getProvisioningUri();
}
/**
- *
+ * Get current code (for testing)
*/
- public function get_provisioning_uri()
+ public function get_code()
{
- // rcube::console('PROV', $this->secret);
- if (!$this->secret) {
- // generate new secret and store it
- $this->set('secret', $this->get('secret', true));
- $this->set('created', $this->get('created', true));
- // rcube::console('PROV2', $this->secret);
- $this->commit();
- }
+ // get my secret from the user storage
+ $secret = $this->get('secret');
- // TODO: deny call if already active?
+ if (!strlen($secret)) {
+ return;
+ }
- $this->backend->setLabel($this->username);
+ $this->backend->setLabel($this->get('username'));
$this->backend->setParameter('secret', $secret);
- return $this->backend->getProvisioningUri();
+ return $this->backend->at(time());
}
}
diff --git a/src/include/Kolab2FA/Driver/Yubikey.php b/src/include/Kolab2FA/Driver/Yubikey.php
--- a/src/include/Kolab2FA/Driver/Yubikey.php
+++ b/src/include/Kolab2FA/Driver/Yubikey.php
@@ -36,13 +36,13 @@
{
parent::init($config);
- $this->user_settings += array(
- 'yubikeyid' => array(
+ $this->user_settings += [
+ 'yubikeyid' => [
'type' => 'text',
'editable' => true,
'label' => 'secret',
- ),
- );
+ ],
+ ];
// initialize validator
$this->backend = new \Yubikey\Validate($this->config['apikey'], $this->config['clientid']);
@@ -67,7 +67,6 @@
$pass = false;
if (!strlen($keyid)) {
- // LOG: "no key registered for user $this->username"
return false;
}
@@ -76,20 +75,18 @@
try {
$response = $this->backend->check($code);
$pass = $response->success() === true;
- }
- catch (\Exception $e) {
+ } catch (\Exception $e) {
// TODO: log exception
}
}
- // rcube::console('VERIFY Yubikey', $this->username, $keyid, $code, $pass);
return $pass;
}
/**
* @override
*/
- public function set($key, $value)
+ public function set($key, $value, $persistent = true)
{
if ($key == 'yubikeyid' && strlen($value) > 12) {
// verify the submitted code
@@ -99,16 +96,19 @@
// TODO: report error
return false;
}
- }
- catch (\Exception $e) {
+ } catch (\Exception $e) {
return false;
}
// truncate the submitted yubikey code to 12 characters
$value = substr($value, 0, 12);
}
+ // invalid or no yubikey token provided
+ elseif ($key == 'yubikeyid') {
+ return false;
+ }
- return parent::set($key, $value);
+ return parent::set($key, $value, $persistent);
}
/**

File Metadata

Mime Type
text/plain
Expires
Wed, Nov 6, 10:08 AM (20 h, 25 m)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
10186533
Default Alt Text
D4932.diff (24 KB)

Event Timeline