Page MenuHomePhabricator

D577.id1897.diff
No OneTemporary

D577.id1897.diff

diff --git a/bin/classes/BaseController.php b/bin/classes/BaseController.php
--- a/bin/classes/BaseController.php
+++ b/bin/classes/BaseController.php
@@ -13,8 +13,8 @@
* @var UserModel|null
*/
protected $user = null;
- protected $token = null;
protected $isAdmin = false;
+ protected $session;
/**
*
@@ -41,7 +41,6 @@
#The user could also check the token
$s = Session::getInstance();
$u = $s->getUser();
- $t = isset($_GET['token'])? db()->table('token')->get('token', $_GET['token'])->fetch() : null;
try {
#Check if the user is an administrator
@@ -51,33 +50,43 @@
$admingroupid = null;
}
- if ($u || $t) {
+ #Find the application for the SSO Server
+ //$self = db()->table(AuthAppModel::class)->get('_id', SysSettingModel::getValue('app.self'))->first();
+ $self = db()->table('authapp')->get('_id', SysSettingModel::getValue('app.self'))->first();
+
+ try {
#Export the user to the controllers that may need it.
- $user = $u? db()->table('user')->get('_id', $u)->fetch() : $t->user;
+ $sess = db()->table('session')->get('_id', $u)->fetch(true);
+ $user = $sess->user;
$this->user = $user;
- $this->token = $t;
#Retrieve the user's authentication level
$this->level = db()->table('authentication\challenge')
- ->get('session', db()->table('session')->get('_id', $s->sessionId())->first())
+ ->get('session', $sess)
->where('cleared', '!=', null)
->where('expires', '>', time())
->all();
$isAdmin = !!db()->table('user\group')->get('group__id', $admingroupid)->addRestriction('user', $user)->fetch();
}
+ catch (\spitfire\exceptions\PrivateException $ex) {
+ #There was a problem loading the session
+ $this->level = collect();
+ $this->user = null;
+ $isAdmin = false;
+ }
$this->signature = new Helper(db());
- if (isset($_GET['signature']) && is_string($_GET['signature'])) {
- list($signature, $src, $target) = $this->signature->verify();
-
- if ($target) {
- throw new PublicException('_GET[signature] must not have remotes', 401);
- }
-
- $this->authapp = $src;
+ /*
+ * Check if the request is being sent by an application that wishes to
+ * directly interact with the SSO Server.
+ */
+ $t = isset($_GET['token'])? db()->table('token')->get('token', $_GET['token'])->fetch() : null;
+
+ if ($t && $self && $t->owner === null && $t->audience->_id === $self->_id) {
+ $this->authapp = $t->client;
$this->level = collect();
}
@@ -91,6 +100,7 @@
}
$this->isAdmin = $isAdmin?? false;
+ $this->session = $sess?? null;
$this->view->set('level', $this->level);
$this->view->set('authUser', $this->user);
diff --git a/bin/classes/MySQLSessionHandler.php b/bin/classes/MySQLSessionHandler.php
deleted file mode 100644
--- a/bin/classes/MySQLSessionHandler.php
+++ /dev/null
@@ -1,81 +0,0 @@
-<?php
-
-use spitfire\exceptions\FileNotFoundException;
-use spitfire\exceptions\PrivateException;
-use spitfire\io\session\Session;
-
-class MySQLSessionHandler extends spitfire\io\session\SessionHandler
-{
-
- /**
- *
- * @var spitfire\storage\database\Table
- */
- private $table;
-
- private $handle;
-
- public function __construct($table, $timeout = null) {
- $this->table = $table;
- parent::__construct($timeout);
- }
-
- public function close() {
- return true;
- }
-
- public function destroy($id) {
- $record = $this->table->get('_id', $id)->first();
- $record && $record->delete();
- return true;
- }
-
- public function gc($maxlifetime) {
- if ($this->getTimeout()) { $maxlifetime = $this->getTimeout(); }
-
- $records = $this->table->get('expires', time() - $maxlifetime, '<')->all();
- $records->each(function ($e) { $e->delete(); });
-
- return true;
- }
-
- public function getHandle() {
- if ($this->handle) { return $this->handle; }
- if (!Session::sessionId()) { return false; }
-
-
- #Initialize the session itself
- $id = Session::sessionId(false);
-
- $this->handle = $this->table->get('_id', $id)->first();
-
- if (!$this->handle) {
- $this->handle = $this->table->newRecord();
- $this->handle->_id = $id;
- $this->handle->expires = time() + 90 * 86400;
- $this->handle->store();
- }
-
- return $this->handle;
- }
-
- public function open($savePath, $sessionName) {
- return true;
- }
-
- public function read($__garbage) {
- //The system can only read the first 8MB of the session.
- //We do hardcode to improve the performance since PHP will stop at EOF
- $_ret = $this->getHandle()->payload;
- return (string)$_ret;
- }
-
- public function write($__garbage, $data) {
- $this->getHandle()->payload = $data;
- $this->getHandle()->expires = time() + 90 * 86400;
- $this->getHandle()->store();
-
- return true;
- }
-
-}
diff --git a/bin/controllers/auth.php b/bin/controllers/auth.php
--- a/bin/controllers/auth.php
+++ b/bin/controllers/auth.php
@@ -48,12 +48,6 @@
*/
public function oauth() {
- /*
- * We're going to need the session to provide it's ID to the code challenge
- * model.
- */
- $session = Session::getInstance();
-
/*
* The response type used to be code or token for applications implementing
* oAuth2 whenever the server and/or client does not support PKCE. Since our
@@ -65,7 +59,11 @@
throw new PublicException('This server does only accept a response_type of code. Please refer to the manual', 400);
}
- if (!$session->getUser()) {
+ /*
+ * When generating an oAuth session we do require the user to be fully
+ * authenticated.
+ */
+ if (!$this->user) {
$this->response->setBody('Redirecting...');
return $this->response->getHeaders()->redirect(url('user', 'login', Array('returnto' => (string) URL::current())));
}
@@ -160,7 +158,7 @@
$challenge->redirect = $_GET['redirect'];
$challenge->created = time();
$challenge->expires = time() + 180;
- $challenge->session = db()->table('session')->get('_id', $session->sessionId())->first();
+ $challenge->session = $this->session;
$challenge->store();
#TODO: Use url-reflection to create the URL instead of jsut appending the params
@@ -379,7 +377,7 @@
{
- if (!$this->user) {
+ if (!$this->session) {
$this->response->setBody('Redirecting')->getHeaders()->redirect(url('user', 'login', ['returnto' => strval(URL::current())]));
return;
}
@@ -389,10 +387,6 @@
* Depending on the expected threshold, the application will require a different
* combination of providers:
*
- * For level 0, the application will check whether the user has a properly
- * authenticated session, if this is not the case, the user will have to
- * provide a primary authentication provider.
- *
* For level 1, the application will require the user to either confirm their
* password, if the password wasn't confirmed in a given amount of time,
* or use any other primary provider.
@@ -406,9 +400,8 @@
$secondary = Environment::get('phpauth.mfa.providers.secondary')? explode(',', Environment::get('phpauth.mfa.providers.secondary')) : ['phone', 'rfc6238', 'backup', 'webauthn'];
$levels = [
- 0 => [],
1 => $primary,
- 1 => $secondary,
+ 2 => $secondary,
3 => array_merge($primary, $secondary),
4 => array_merge($primary, $secondary),
5 => array_merge($primary, $secondary)
@@ -425,18 +418,10 @@
*/
$providers = db()->table('authentication\provider')
->get('expires', null)
- ->where('user', $this->user)
+ ->where('user', $this->session->candidate)
+ ->setOrder('preferred', 'DESC')
->all();
- /*
- * Check whether the sessin has been locked to this user and we're certain
- * that they've logged in (even though their verification may have expired)
- */
- if (!$this->user) {
- #TODO: The user is not authenticated at all, this means we need to verify
- #their password or some other provider for sure.
- }
-
foreach ($levels as $level => $required)
{
/*
@@ -452,6 +437,10 @@
return array_search($e->type, $required);
});
+ if ($accepted->isEmpty()) {
+ throw new PrivateException('Authentication provider for level ' . $level . ' is unavailable');
+ }
+
/*
* Check if any of the providers the user has passed recently was a
* primary provider
@@ -469,7 +458,8 @@
$providers = $providers->filter(function ($e) use ($success) { return $success->_id != $e->_id; });
}
else {
- return $this->response->setBody('Redirect')->getHeaders()->redirect(url(['mfa', $accepted[0]->type], 'challenge', $accepted[0]->_id));
+ $provider = $accepted->rewind();
+ return $this->response->setBody('Redirect')->getHeaders()->redirect(url('mfa', $provider->type, 'challenge', $provider->_id, ['returnto' => (string)URL::current()]));
}
}
diff --git a/bin/controllers/mfa/PasswordController.php b/bin/controllers/mfa/PasswordController.php
--- a/bin/controllers/mfa/PasswordController.php
+++ b/bin/controllers/mfa/PasswordController.php
@@ -2,9 +2,11 @@
use authentication\ProviderModel;
use BaseController;
+use spitfire\core\http\URL;
use spitfire\exceptions\HTTPMethodException;
use spitfire\exceptions\PrivateException;
use spitfire\validation\ValidationException;
+use Strings;
use function db;
use function url;
@@ -50,10 +52,16 @@
* a stolen session cannot hijack an account.
*/
if (!$this->user) {
- #TODO: Redirect to login
+ $this->response->setBody('Redirect')->getHeaders()->redirect(url('user', 'login', ['returnto' => (string) URL::current()]));
}
- #TODO: Require the user to be strongly authenticated to perform this action
+ /*
+ * If the user has multi factor authentication enabled, we check that they
+ * are indeed strongly authenticated before continuing.
+ */
+ if ($this->level->count() < ($this->user->mfa? 2 : 1)) {
+ $this->response->setBody('Redirect')->getHeaders()->redirect(url('auth', 'threshold', ($this->user->mfa? 2 : 1), ['returnto' => (string)URL::current()]));
+ }
/*
* Fetch the authentication provider for the password. The user can only
@@ -118,8 +126,15 @@
public function challenge()
{
- #TODO: This needs to work with session candidates
- $user = $this->user;
+ if (!$this->session) {
+ $this->response->setBody('Redirecting')->getHeaders()->redirect(url('user', 'login', ['returnto' => strval(URL2::current())]));
+ return;
+ }
+
+ if (isset($_GET['returnto']) && Strings::startsWith($_GET['returnto'], '/')) { $returnto = $_GET['returnto']; }
+ else { $returnto = (string)url('twofactor'); }
+
+ $user = $this->session->candidate;
/*
* Fetch the authentication provider for the password. The user can only
@@ -129,7 +144,7 @@
* could be given if the administrator created an account for a user on
* a server that has no sign-up method)
*/
- $provider = db()->table('authentication\provider')->get('user', $user)->where('type', ProviderModel::TYPE_PASSWORD)->first();
+ $provider = db()->table('authentication\provider')->get('user', $user)->where('expires', null)->where('type', ProviderModel::TYPE_PASSWORD)->first();
/*
* If there is no password hashing mechanism, we should abort ASAP. Since
@@ -163,10 +178,9 @@
}
$challenge = db()->table('authentication\challenge')->newRecord();
- $challenge->user = $user;
- $challenge->type = ProviderModel::TYPE_PASSWORD;
- #TODO: The challenge needs to be locked to the session authenticating the user
- //$challenge->session = ;
+ $challenge->provider = $provider;
+ $challenge->session = $this->session;
+ $challenge->expires = time() + 1200;
$challenge->cleared = time();
$challenge->store();
@@ -174,7 +188,7 @@
* Once the password has been properly set, redirect the user to a success
* page.
*/
- $this->response->setBody('Redirect...')->getHeaders()->redirect(url('twofactor'));
+ $this->response->setBody('Redirect...')->getHeaders()->redirect($returnto);
}
catch (HTTPMethodException $ex) {
/*Show the form*/
diff --git a/bin/controllers/mfa/TOTPController.php b/bin/controllers/mfa/TOTPController.php
--- a/bin/controllers/mfa/TOTPController.php
+++ b/bin/controllers/mfa/TOTPController.php
@@ -165,9 +165,10 @@
if ( intval($_POST['challenge']) !== intval($totp)) { throw new ValidationException('Code failed', 0, []); }
$challenge = db()->table('authentication\challenge')->newRecord();
- #TODO: $challenge->session = ;
+ $challenge->session = $this->session;
$challenge->provider = $provider;
$challenge->cleared = time();
+ $challenge->expires = time() + 1200;
$challenge->store();
#TODO: Add flag for whether the provider is active
diff --git a/bin/controllers/user.php b/bin/controllers/user.php
--- a/bin/controllers/user.php
+++ b/bin/controllers/user.php
@@ -133,6 +133,17 @@
else {
$returnto = (string)url();
}
+
+ if ($this->session && $this->level->count() < 1) {
+ return $this->response->setBody('Redirect')->getHeaders()->redirect(url('auth', 'threshold', 1, ['returnto' => strval(\spitfire\core\http\URL::current())]));
+ }
+
+ if ($this->session && $this->level->count() > 0) {
+ $this->session->user = $this->session->candidate;
+ $this->session->store();
+
+ return $this->response->setBody('Redirect')->getHeaders()->redirect($returnto);
+ }
if ($_SERVER['REQUEST_METHOD'] === 'POST') {
@@ -153,12 +164,10 @@
if ($user && $user->disabled !== null) {
throw new PublicException('This account has been disabled permanently.', 401);
}
- elseif ($user && $user->checkPassword($_POST['password'])) {
- $session = Session::getInstance();
- $session->lock($user->_id);
-
- $dbsession = db()->table('session')->get('_id', $session->sessionId(false))->first(true);
- $dbsession->user = $user;
+ elseif ($user) {
+ $dbsession = db()->table('session')->newRecord();
+ $dbsession->user = null;
+ $dbsession->candidate = $user;
$dbsession->device = DeviceModel::makeFromRequest();
$dbsession->ip = isset($_SERVER['HTTP_X_FORWARDED_FOR'])? $_SERVER['HTTP_X_FORWARDED_FOR']: $_SERVER['REMOTE_ADDR'];
@@ -172,9 +181,14 @@
}
$dbsession->store();
- async()->defer(time() + 86400 * 90, new IncinerateSessionTask($dbsession->_id));
- return $this->response->getHeaders()->redirect($returnto);
+ $session = Session::getInstance();
+ $session->lock($dbsession->_id);
+
+ //async()->defer(time() + 86400 * 90, new IncinerateSessionTask($dbsession->_id));
+
+ return $this->response->setBody('Redirect')->getHeaders()->redirect(url('auth', 'threshold', 1, ['returnto' => strval(\spitfire\core\http\URL::current())]));
+
} else {
$this->view->set('message', 'Username or password did not match');
}
@@ -185,11 +199,12 @@
public function logout() {
$s = Session::getInstance();
- $s->destroy();
$dbsession = db()->table('session')->get('_id', $s->sessionId())->first();
$token = isset($_GET['token'])? db()->table('access\token')->get('_id', $_GET['token'])->first() : null;
- $rtt = new URLReflection($_GET['returnto']?? null);
+ $rtt = URLReflection::fromURL($_GET['returnto']?? null);
+
+ $s->destroy();
if ($dbsession) {
$dbsession->expires = time();
diff --git a/bin/models/session.php b/bin/models/session.php
--- a/bin/models/session.php
+++ b/bin/models/session.php
@@ -33,6 +33,7 @@
* tokens that have not been granted as offline will be terminated.
*
* @property UserModel $user Session owner
+ * @property UserModel $claim When a user is logging in, they can claim to be a certain person. This is not a valid user authentication.
* @property LocationModel $location The location from where the session was authorized
* @property DeviceModel $device The device from which the session was authorized
*
@@ -44,6 +45,9 @@
class SessionModel extends Model
{
+ protected const TOKEN_PREFIX = 's_';
+ const TOKEN_LENGTH = 50;
+
/**
*
* @param Schema $schema
@@ -52,7 +56,8 @@
public function definitions(Schema $schema) {
#The session ID will be retrieved from the Session
unset($schema->_id);
- $schema->_id = new StringField(70);
+ $schema->_id = new StringField(self::TOKEN_LENGTH);
+ $schema->candidate= new Reference(UserModel::class);
$schema->user = new Reference(UserModel::class);
$schema->location = new Reference(LocationModel::class);
$schema->device = new Reference(DeviceModel::class);
@@ -77,8 +82,6 @@
$schema->created = new IntegerField(true);
$schema->expires = new IntegerField(true);
- $schema->payload = new TextField();
-
$schema->index($schema->_id)->setPrimary(true);
$schema->index($schema->expires);
}
@@ -86,6 +89,11 @@
public function onbeforesave(): void {
parent::onbeforesave();
+ if (!$this->_id) {
+ do { $this->_id = substr(self::TOKEN_PREFIX . bin2hex(random_bytes(25)), 0, self::TOKEN_LENGTH); }
+ while (db()->table('session')->get('_id', $this->_id)->first());
+ }
+
if (!$this->created) { $this->created = time(); }
$this->expires = time() + 86400 * 90;
}
diff --git a/bin/settings/routes.php b/bin/settings/routes.php
--- a/bin/settings/routes.php
+++ b/bin/settings/routes.php
@@ -18,6 +18,7 @@
spitfire\core\router\Router::getInstance()->request('/mfa/backup-code/:action?', ['controller' => ['mfa', 'BackUpCode'], 'action' => ':action']);
spitfire\core\router\Router::getInstance()->request('/mfa/password/:action?', ['controller' => ['mfa', 'Password'], 'action' => ':action']);
spitfire\core\router\Router::getInstance()->request('/mfa/totp/:action?', ['controller' => ['mfa', 'TOTP'], 'action' => ':action']);
+spitfire\core\router\Router::getInstance()->request('/mfa/rfc6238/:action?', ['controller' => ['mfa', 'TOTP'], 'action' => ':action']);
spitfire\core\router\Router::getInstance()->request('/mfa/phone/:action?', ['controller' => ['mfa', 'Phone'], 'action' => ':action']);
spitfire\core\router\Router::getInstance()->request('/mfa/email/:action?', ['controller' => ['mfa', 'Email'], 'action' => ':action']);
spitfire\core\router\Router::getInstance()->request('/session/:action?', ['controller' => ['Session'], 'action' => ':action']);
\ No newline at end of file
diff --git a/bin/templates/layout.php b/bin/templates/layout.php
--- a/bin/templates/layout.php
+++ b/bin/templates/layout.php
@@ -56,7 +56,7 @@
</div>
</div>
<?php else: ?>
- <a class="menu-item" href="<?= url('account', 'login') ?>">Login</a>
+ <a class="menu-item" href="<?= url('user', 'login') ?>">Login</a>
<?php endif; ?>
</div>
<div class="center align-center">
diff --git a/bin/templates/user/login.php b/bin/templates/user/login.php
--- a/bin/templates/user/login.php
+++ b/bin/templates/user/login.php
@@ -36,7 +36,7 @@
<div class="spacer small"></div>
- <div class="frm-ctrl-grp">
+ <!--<div class="frm-ctrl-grp">
<div class="frm-ctrl-outer">
<input class="frm-ctrl" placeholder="" type="password" name="password" id="password" autocomplete="current-password" required aria-describedby="password-constraints" minlength="8" style="height: 3.275rem">
<label for="password">Password</label>
@@ -58,7 +58,7 @@
</button>
</span>
</div>
- </div>
+ </div>-->
<div class="spacer small"></div>

File Metadata

Mime Type
text/plain
Expires
Apr 11 2021, 9:19 AM (9 w, 2 d ago)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
5848
Default Alt Text
D577.id1897.diff (19 KB)

Event Timeline