Page MenuHomePhabricator

No OneTemporary

diff --git a/bin/controllers/app.php b/bin/controllers/app.php
index 905cb0a..04caf2a 100644
--- a/bin/controllers/app.php
+++ b/bin/controllers/app.php
@@ -43,6 +43,7 @@ class AppController extends BaseController
$app->name = $_POST['name'];
#TODO: Replace with the proper app secret generation
$app->appSecret = preg_replace('/[^a-z\d]/i', '', base64_encode(random_bytes(35)));
+ $app->twofactor = false;
if ($_POST['icon'] instanceof Upload) {
$app->icon = $_POST['icon']->validate()->store()->uri();
diff --git a/bin/controllers/auth.php b/bin/controllers/auth.php
index b4ba735..0c7c9d1 100644
--- a/bin/controllers/auth.php
+++ b/bin/controllers/auth.php
@@ -1,7 +1,11 @@
<?php
use app\AuthLock;
+use client\LocationModel;
+use client\ScopeModel;
use connection\AuthModel;
+use exceptions\suspension\LoginDisabledException;
+use magic3w\http\url\reflection\URLReflection;
use signature\Signature;
use spitfire\core\Environment;
use spitfire\core\http\URL;
@@ -39,7 +43,8 @@ class AuthController extends BaseController
* @validate GET#client (string required)
* @validate GET#state (string required)
* @validate GET#redirect (string required)
- * @validate GET#challenge (string required)
+ * @validate GET#code_challenge (string required)
+ * @validate GET#code_challenge_method (string required)
*
* @param type $tokenid
* @return void
@@ -48,6 +53,30 @@ class AuthController extends BaseController
*/
public function oauth() {
+ $code_challenge = $_GET['code_challenge'];
+ $code_challenge_method = $_GET['code_challenge_method']?? 'plain';
+ $audience = $_GET['audience']?
+ db()->table(AuthAppModel::class)->get('appID', $_GET['audience'])->first(true) :
+ db()->table(AuthAppModel::class)->get('_id', SysSettingModel::getValue('app.self'))->first(true);
+
+ /*
+ * In order to ensure that the client can be given appropriate access, the
+ * server needs to make sure that the application requests the appropriate
+ * scopes for this application.
+ *
+ * An application must never request access to a scope that doesn't exist,
+ * granting access to undefined scopes may lead to dangerous behavior.
+ *
+ * Scopes are defined by the audience that receives the token, to make sure
+ * you request the right scopes, refer to the documentation of the application
+ * you wish to read data from.
+ */
+ $scopes = collect(explode(' ', $_GET['scope']))
+ ->filter()
+ ->each(function ($e) use ($audience) {
+ return db()->table(ScopeModel::class)->get('identifier', sprintf('%s.%s', $audience->appID, $e))->first(true);
+ });
+
/*
* 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
@@ -102,15 +131,35 @@ class AuthController extends BaseController
*/
$grant = false;
- #TODO: verify the challenge is not malformed
- #TODO: Verify the scopes exist
+ /*
+ * Our implementation does not accept anything but S256, plain code_challenges
+ * are going to be rejected. These could be intercepted easily.
+ */
+ if ($code_challenge_method != 'S256') {
+ throw new PublicException('Invalid code_challenge_method', 400);
+ }
/*
* Extract the redirect, and make sure that it points to a URL that the client
* is authorized to send the user to.
*/
- $redirect = $_GET['redirect'];
- #TODO: Check the redirect is pointing to the application we intent to authenticate
+ $redirect = URLReflection::fromURL($_GET['redirect']);
+
+ /*
+ * In order to validate the redirect we make sure that the protocol, hostname
+ * and paths for the redirect match.
+ */
+ $valid = db()->table(LocationModel::class)->get('client', $client)->all()->reduce(function ($valid, LocationModel $e) use ($redirect) {
+ if ($e->protocol !== $redirect->getProtocol()) { return $valid; }
+ if ($e->hostname !== $redirect->getServer()) { return $valid; }
+ if (!Strings::startsWith($redirect->getPath(), $e->path)) { return $valid; }
+
+ return true;
+ }, false);
+
+ if (!$valid) {
+ throw new PublicException(sprintf('Redirect to %s is invalid', __($redirect)), 401);
+ }
if (false) {
@@ -123,9 +172,6 @@ class AuthController extends BaseController
*/
elseif ($this->request->isPost()) {
- #TODO: Verify that the resource owner is not being tricked into allowing
- # the client access using XSRF
-
#TODO: Check if permission allows this user to authenticate codes for this
# application. Important for elevated privileges apps
@@ -153,17 +199,16 @@ class AuthController extends BaseController
$challenge->client = $client;
$challenge->user = $this->user;
$challenge->state = $_GET['state'];
- $challenge->challenge = $_GET['challenge'];
+ $challenge->challenge = sprintf('%s:%s', $code_challenge_method, $code_challenge);
$challenge->scope = $_GET['scope'];
- $challenge->redirect = $_GET['redirect'];
+ $challenge->redirect = (string)$redirect;
$challenge->created = time();
$challenge->expires = time() + 180;
$challenge->session = $this->session;
$challenge->store();
- #TODO: Use url-reflection to create the URL instead of jsut appending the params
-
- $this->response->setBody('Redirect')->getHeaders()->redirect($challenge->redirect . '?' . http_build_query(['code' => $challenge->code, 'state' => $challenge->state]));
+ return $this->response->setBody('Redirect')
+ ->getHeaders()->redirect((clone $redirect)->setQueryString(['code' => $challenge->code, 'state' => $challenge->state]));
}
/*
@@ -177,11 +222,10 @@ class AuthController extends BaseController
* If the user has not been able to allow or deny the request, the server
* should request their permission.
*/
-
$this->view->set('client', $client);
- $this->view->set('cancel', $_GET['redirect'] . '?' . http_build_query(['error' => 'denied', 'description' => 'Authentication request was denied']));
- #TODO: If a target was defined, we would like to know that
-
+ $this->view->set('audience', $audience);
+ $this->view->set('redirect', (string)$redirect);
+ $this->view->set('cancel', (string)(clone $redirect)->setQueryString(['error' => 'denied', 'description' => 'Authentication request was denied']));
}
/**
diff --git a/bin/controllers/mfa/EmailController.php b/bin/controllers/mfa/EmailController.php
index 8f8ef19..ed50563 100644
--- a/bin/controllers/mfa/EmailController.php
+++ b/bin/controllers/mfa/EmailController.php
@@ -117,8 +117,7 @@ class EmailController extends BaseController
throw new PublicException('Login required to remove email addresses', 401);
}
- #TODO: Replace that OIDC Session placeholder
- $strength = db()->table('authentication\challenge')->get('session', '[OIDC Session goes here]')->where('cleared', '>', time() - 1200)->all();
+ $strength = $this->level;
$expected = $this->user->mfa? 2 : 1;
if ($strength->count() < $expected) {
diff --git a/bin/controllers/mfa/PhoneController.php b/bin/controllers/mfa/PhoneController.php
index ae5f996..5ce00cd 100644
--- a/bin/controllers/mfa/PhoneController.php
+++ b/bin/controllers/mfa/PhoneController.php
@@ -125,8 +125,7 @@ class PhoneController extends BaseController
throw new PublicException('Login required to remove phone numbers', 401);
}
- #TODO: Replace that OIDC Session placeholder
- $strength = db()->table('authentication\challenge')->get('session', '[OIDC Session goes here]')->where('cleared', '>', time() - 1200)->all();
+ $strength = $this->level;
$expected = $this->user->mfa? 2 : 1;
if ($strength->count() < $expected) {
diff --git a/bin/controllers/mfa/TOTPController.php b/bin/controllers/mfa/TOTPController.php
index a21ab71..754ad1a 100644
--- a/bin/controllers/mfa/TOTPController.php
+++ b/bin/controllers/mfa/TOTPController.php
@@ -194,8 +194,7 @@ class TOTPController extends BaseController
throw new PublicException('Login required to remove phone numbers', 401);
}
- #TODO: Replace that OIDC Session placeholder
- $strength = db()->table('authentication\challenge')->get('session', '[OIDC Session goes here]')->where('cleared', '>', time() - 1200)->all();
+ $strength = $this->level;
$expected = $this->user->mfa? 2 : 1;
/*
diff --git a/bin/controllers/token.php b/bin/controllers/token.php
index 6fb51df..ea5f48b 100644
--- a/bin/controllers/token.php
+++ b/bin/controllers/token.php
@@ -34,7 +34,18 @@ class TokenController extends BaseController
$type = $_POST['grant_type']?? 'code';
$appid = isset($_POST['client'])? $_POST['client'] : $_GET['client'];
$secret = $_POST['secret']?? null;
- $expires = Environment::get('phpas.token.expiration')?: 14400;
+
+ /*
+ * We define the algorithms that the server understands. Since the oauth
+ * spec defines a different name for sha256, we will make our server translates
+ * the hashing algo back.
+ *
+ * We currently do not service anything but sha256.
+ */
+ $hash_algos = [
+ 's256' => 'sha256',
+ 'sha256' => 'sha256'
+ ];
/*
* Check if an app with the provided ID does indeed exist.
@@ -88,10 +99,18 @@ class TokenController extends BaseController
}
/*
- * Check the code verifier
+ * Check the code verifier. We need to make sure that the algorhithm
+ * exists, so we don't pass invalid data into the hasing function.
*/
list($algo, $hash) = explode(':', $code->challenge);
-
+
+ if (isset($hash_algos[strtolower($algo)])) {
+ $algo = $hash_algos[strtolower($algo)];
+ }
+ else {
+ throw new PublicException('Invalid hashing algorhithm specified.', 400);
+ }
+
if (hash($algo, $_POST['verifier']) !== $hash) {
throw new PublicException('Hash failed', 403);
}
@@ -101,9 +120,26 @@ class TokenController extends BaseController
*/
$code->expires = time();
$code->store();
-
- $token = TokenModel::create($code->session, $app, null, $code->user, $expires);
- $refresh = RefreshModel::create($app, null, $code->user, time() + 86400 * 365 * 5);
+
+ #TODO: This code could be extracted into an helper that could be pulled
+ #in via service providers to reduce the amount of code duplication.
+ /*
+ * Instance a token that can be sent to the client to provide them access
+ * to the resources of the owner.
+ */
+ $token = db()->table(TokenModel::class)->newRecord();
+ $token->session = $code->session;
+ $token->owner = $code->user;
+ $token->audience = $code->audience;
+ $token->client = $app;
+ $token->store();
+
+ $refresh = db()->table(RefreshModel::class)->newRecord();
+ $refresh->session = $code->session;
+ $refresh->owner = $code->user;
+ $refresh->audience = $code->audience;
+ $refresh->client = $app;
+ $refresh->store();
}
elseif ($type === 'refresh_token') {
/**
@@ -118,8 +154,25 @@ class TokenController extends BaseController
throw new PublicException('Tried refreshing a token owned by a different client', 403);
}
- $token = TokenModel::create($provided->session, $app, null, $provided->owner, $expires);
- $refresh = RefreshModel::create($app, null, $provided->owner, time() + 86400 * 365 * 5);
+ #TODO: This code could be extracted into an helper that could be pulled
+ #in via service providers to reduce the amount of code duplication.
+ /*
+ * Instance a token that can be sent to the client to provide them access
+ * to the resources of the owner.
+ */
+ $token = db()->table(TokenModel::class)->newRecord();
+ $token->session = $provided->session;
+ $token->owner = $provided->owner;
+ $token->audience = $provided->audience;
+ $token->client = $provided->client;
+ $token->store();
+
+ $refresh = db()->table(RefreshModel::class)->newRecord();
+ $refresh->session = $provided->session;
+ $refresh->owner = $provided->owner;
+ $refresh->audience = $provided->audience;
+ $refresh->client = $provided->client;
+ $refresh->store();
}
else {
throw new PublicException('Invalid grant_type selected', 400);
diff --git a/bin/models/access/refresh.php b/bin/models/access/refresh.php
index 5f6cd21..8b15e9a 100644
--- a/bin/models/access/refresh.php
+++ b/bin/models/access/refresh.php
@@ -42,13 +42,14 @@ class RefreshModel extends Model
const TOKEN_PREFIX = 'r_';
const TOKEN_LENGTH = 50;
+ const TOKEN_TTL = 63072000;
public function definitions(Schema $schema) {
$schema->token = new StringField(self::TOKEN_LENGTH);
$schema->owner = new Reference('user');
$schema->client = new Reference('authapp');
- $schema->host = new Reference('authapp');
+ $schema->audience = new Reference('authapp');
$schema->scopes = new StringField(255);
@@ -62,31 +63,6 @@ class RefreshModel extends Model
}
- /**
- * There's only one endpoint generating tokens, and the function's signature is
- * getting very unwieldy. So we're moving best this to the controller.
- *
- * @deprecated since version 0.2-dev
- * @param SessionModel $session
- * @param type $client
- * @param type $server
- * @param type $owner
- * @param type $expires
- * @return type
- * @throws PrivateException
- */
- public static function create($client, $server = null, $owner = null, $expires = 63072000) {
- $record = db()->table('access\refresh')->newRecord();
- $record->created = time();
- $record->owner = $owner;
- $record->host = $server?: null;
- $record->client = $client;
- $record->expires = time() + $expires;
- $record->ttl = $expires;
- $record->store();
- return $record;
- }
-
public function onbeforesave(): void {
parent::onbeforesave();
@@ -98,6 +74,22 @@ class RefreshModel extends Model
do { $this->token = substr(self::TOKEN_PREFIX . bin2hex(random_bytes(25)), 0, self::TOKEN_LENGTH); }
while (db()->table('access\token')->get('token', $this->token)->first());
}
+
+ /*
+ * If the token has no creation date we assume that it has never been stored
+ * before and record the creation time.
+ */
+ if (!$this->created) {
+ $this->created = time();
+ }
+
+ /*
+ * Set the expiration time to a timestamp in the future (by default 30 minutes)
+ * if the expiration was not explicitly set before.
+ */
+ if (!$this->expires) {
+ $this->expires = time() + self::TOKEN_TTL;
+ }
}
-}
\ No newline at end of file
+}
diff --git a/bin/models/access/token.php b/bin/models/access/token.php
index 1faf7cb..c77461b 100644
--- a/bin/models/access/token.php
+++ b/bin/models/access/token.php
@@ -53,13 +53,14 @@ class TokenModel extends Model
const TOKEN_PREFIX = 't_';
const TOKEN_LENGTH = 50;
+ const TOKEN_TTL = 1800;
public function definitions(Schema $schema) {
$schema->token = new StringField(self::TOKEN_LENGTH);
$schema->owner = new Reference('user');
$schema->client = new Reference('authapp');
- $schema->host = new Reference('authapp');
+ $schema->audience = new Reference('authapp');
$schema->scopes = new StringField(255);
@@ -73,32 +74,6 @@ class TokenModel extends Model
}
- /**
- * There's only one endpoint generating tokens, and the function's signature is
- * getting very unwieldy. So we're moving best this to the controller.
- *
- * @deprecated since version 0.2-dev
- * @param SessionModel $session
- * @param type $client
- * @param type $server
- * @param type $owner
- * @param type $expires
- * @return type
- * @throws PrivateException
- */
- public static function create(SessionModel $session, $client, $server = null, $owner = null, $expires = 14400) {
- $record = db()->table('access\token')->newRecord();
- $record->created = time();
- $record->owner = $owner;
- $record->host = $server?: $client;
- $record->client = $client;
- $record->session = $session;
- $record->expires = time() + $expires;
- $record->ttl = $expires;
- $record->store();
- return $record;
- }
-
public function onbeforesave(): void {
parent::onbeforesave();
@@ -110,6 +85,22 @@ class TokenModel extends Model
do { $this->token = substr(self::TOKEN_PREFIX . bin2hex(random_bytes(25)), 0, self::TOKEN_LENGTH); }
while (db()->table('access\token')->get('token', $this->token)->first());
}
+
+ /*
+ * If the token has no creation date we assume that it has never been stored
+ * before and record the creation time.
+ */
+ if (!$this->created) {
+ $this->created = time();
+ }
+
+ /*
+ * Set the expiration time to a timestamp in the future (by default 30 minutes)
+ * if the expiration was not explicitly set before.
+ */
+ if (!$this->expires) {
+ $this->expires = time() + self::TOKEN_TTL;
+ }
}
-}
\ No newline at end of file
+}
diff --git a/bin/templates/auth/oauth.php b/bin/templates/auth/oauth.php
index d510b7b..d4ce067 100644
--- a/bin/templates/auth/oauth.php
+++ b/bin/templates/auth/oauth.php
@@ -1,35 +1,71 @@
-<div class="spacer" style="height: 30px"></div>
+<div class="spacer small"></div>
-<div class="row5">
- <div class="span1">
+<div class="row l5">
+ <div class="span l1">
</div>
- <div class="span3">
+ <div class="span l3">
<form method="POST" action="">
- <h1>Access <?= $client->name ?>?</h1>
-
- <div class="material">
- <p style="text-align: center">
- <img src="<?= url('image', 'app', $client->_id, 128) ?>" width="128" style="border-radius: 3px; border: solid 1px #777;">
- <img src="<?= \spitfire\core\http\URL::asset('img/right-arrow.png') ?>" style="margin: 4px 20px;">
- <img src="<?= url('image', 'user', $authUser->_id, 128) ?>" width="128" style="border-radius: 3px; border: solid 1px #777;">
- </p>
+ <div class="align-center text:green-600">
+ <img src="<?= url('image', 'app', $client->_id, 128) ?>" width="128" style="border-radius: 50%; border: solid 1px #777; vertical-align: middle;">
+ <div style="display: inline-block; width: 50px; border-top: solid 1px #CCC; vertical-align: middle;"></div>
+ <svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 20 20" fill="currentColor" style="height: 40px; vertical-align: middle;">
+ <path fill-rule="evenodd" d="M10 18a8 8 0 100-16 8 8 0 000 16zm3.707-9.293a1 1 0 00-1.414-1.414L9 10.586 7.707 9.293a1 1 0 00-1.414 1.414l2 2a1 1 0 001.414 0l4-4z" clip-rule="evenodd" />
+ </svg>
+ <div style="display: inline-block; width: 50px; border-top: solid 1px #CCC; vertical-align: middle;"></div>
+ <img src="<?= url('image', 'app', $audience->_id, 128) ?>" width="128" style="border-radius: 50%; border: solid 1px #777; vertical-align: middle;">
+ </div>
- <p>
- If you wish to proceed and log into the application using your account,
- click "Continue". If you do not trust the application, just cancel the
- log in process - your account details won't be shared
- </p>
+ <div class="align-center">
+ <div class="spacer large"></div>
+ <h1 class="text:grey-700" style="font-size: 1.8rem">Authorize <?= $client->name ?></h1>
+ <div class="spacer small"></div>
+ </div>
+
+ <div class="box box-soft">
+ <div class="padded">
+ <div class="spacer medium"></div>
+
+ <div class="row s7">
+ <div class="span s1">
+ <img src="<?= url('image', 'user', $client->owner->_id, 128) ?>" width="128" style="border-radius: 50%; vertical-align: middle;">
+ </div>
+ <div class="span s6 text:grey-700">
+ <div class="spacer minuscule"></div>
+ <div>
+ <strong class="text:grey-800"><?= $client->name ?></strong>
+ by <strong class="text:grey-800"><?= $client->owner->usernames->getQuery()->first()->name ?></strong>
+ </div>
+ <div>
+ is requesting access your data on <strong class="text:grey-800"><?= $audience->name ?></strong>
+ </div>
+ </div>
+ </div>
- <p style="text-align: center">
- <a href="<?= $cancel ?>">Cancel</a>
- <span style="display: inline-block; width: 20px"></span>
- <input type="hidden" name="grant" value="grant">
- <!-- TODO: Add XSRF -->
- <input type="submit" value="Grant access to your account" class="button">
- </p>
- </div>
+ <p class="">
+ </p>
+
+ <div class="spacer large"></div>
+
+ <div style="text-align: center">
+ <input type="hidden" name="grant" value="grant">
+ <input type="submit" value="Grant access to your account" class="button full-width" style="width: 100%">
+ <div class="spacer small"></div>
+ <a href="<?= $cancel ?>">Cancel</a>
+ </div>
+
+
+ <div class="spacer medium"></div>
+ </div>
+ </div>
+ </form>
+
+ <div class="spacer large"></div>
+
+ <p class="text:grey-600 align-center" style="font-size: .8rem">
+ Authorizing redirects to <strong class="text:grey-700"><?= $redirect ?></strong>
+ </p>
</div>
</div>
diff --git a/composer.lock b/composer.lock
index b4da9f2..dbb03e3 100644
--- a/composer.lock
+++ b/composer.lock
@@ -44,8 +44,13 @@
"source": {
"type": "git",
"url": "https://phabricator.magic3w.com/source/permission-PHP-SDK.git",
- "reference": "872b399182d83caafdc7c142ac089551356974e1"
+ "reference": "b457f030ac81e8b613b8275f23efc3475f440471"
},
+ "require": {
+ "magic3w/url-reflection": "dev-master",
+ "spitfire/request": "dev-master"
+ },
+ "default-branch": true,
"type": "library",
"autoload": {
"psr-4": {
@@ -63,7 +68,7 @@
}
],
"description": "Allows your application to communicate with a permission server through a simple API",
- "time": "2020-08-26T15:10:23+00:00"
+ "time": "2020-12-17T09:06:38+00:00"
},
{
"name": "magic3w/url-reflection",
@@ -71,7 +76,7 @@
"source": {
"type": "git",
"url": "https://phabricator.magic3w.com/source/url-reflection.git",
- "reference": "6d7529af43c2cb1ed264e9b88d257ac4066b641c"
+ "reference": "341d1ede12d28159101b77c8ae12b544dba15a8c"
},
"require-dev": {
"phpunit/phpunit": "^9.3"
@@ -94,7 +99,7 @@
}
],
"description": "Allows applications to have a URL parsed and retrieve information about it's components",
- "time": "2020-11-04T12:01:53+00:00"
+ "time": "2021-02-11T13:17:34+00:00"
},
{
"name": "spitfire/defer",
@@ -130,9 +135,10 @@
"source": {
"type": "git",
"url": "https://phabricator.magic3w.com/source/spitfire-request.git",
- "reference": "2567db6a18f287cf33ffc93de1c8aaf7b7c7c3a6"
+ "reference": "cc2a34540bf114d97d269b9c61f37ee983dcc9a5"
},
"require": {
+ "ext-curl": "*",
"magic3w/url-reflection": "dev-master"
},
"require-dev": {
@@ -156,7 +162,7 @@
}
],
"description": "Spitfire request mechanism",
- "time": "2020-12-28T14:18:45+00:00"
+ "time": "2020-12-28T15:14:13+00:00"
},
{
"name": "squizlabs/php_codesniffer",
@@ -218,16 +224,16 @@
"packages-dev": [
{
"name": "phpstan/phpstan",
- "version": "0.12.64",
+ "version": "0.12.79",
"source": {
"type": "git",
"url": "https://github.com/phpstan/phpstan.git",
- "reference": "23eb1cb7ae125f45f1d0e48051bcf67a9a9b08aa"
+ "reference": "dd7769915648b704b9bd12925994457f1c2c8442"
},
"dist": {
"type": "zip",
- "url": "https://api.github.com/repos/phpstan/phpstan/zipball/23eb1cb7ae125f45f1d0e48051bcf67a9a9b08aa",
- "reference": "23eb1cb7ae125f45f1d0e48051bcf67a9a9b08aa",
+ "url": "https://api.github.com/repos/phpstan/phpstan/zipball/dd7769915648b704b9bd12925994457f1c2c8442",
+ "reference": "dd7769915648b704b9bd12925994457f1c2c8442",
"shasum": ""
},
"require": {
@@ -258,7 +264,7 @@
"description": "PHPStan - PHP Static Analysis Tool",
"support": {
"issues": "https://github.com/phpstan/phpstan/issues",
- "source": "https://github.com/phpstan/phpstan/tree/0.12.64"
+ "source": "https://github.com/phpstan/phpstan/tree/0.12.79"
},
"funding": [
{
@@ -274,7 +280,7 @@
"type": "tidelift"
}
],
- "time": "2020-12-21T11:59:02+00:00"
+ "time": "2021-02-25T16:44:57+00:00"
}
],
"aliases": [],

File Metadata

Mime Type
text/x-diff
Expires
Wed, Apr 14, 1:38 PM (3 w, 6 d ago)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
7071
Default Alt Text
(24 KB)

Event Timeline