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 (:) * - * @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); } /**