Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add new public key token provider (tokens survive password change) #9485

Merged
merged 13 commits into from
Jun 19, 2018
54 changes: 54 additions & 0 deletions core/Migrations/Version14000Date20180518120534.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
<?php
declare(strict_types=1);
/**
* @copyright Copyright (c) 2018 Roeland Jago Douma <roeland@famdouma.nl>
*
* @author Roeland Jago Douma <roeland@famdouma.nl>
*
* @license GNU AGPL version 3 or any later version
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as
* published by the Free Software Foundation, either version 3 of the
* License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*
*/

namespace OC\Core\Migrations;

use OCP\DB\ISchemaWrapper;
use OCP\Migration\SimpleMigrationStep;
use OCP\Migration\IOutput;

class Version14000Date20180518120534 extends SimpleMigrationStep {

public function changeSchema(IOutput $output, \Closure $schemaClosure, array $options) {
/** @var ISchemaWrapper $schema */
$schema = $schemaClosure();

$table = $schema->getTable('authtoken');
$table->addColumn('private_key', 'text', [
'notnull' => false,
]);
$table->addColumn('public_key', 'text', [
'notnull' => false,
]);
$table->addColumn('version', 'smallint', [
'notnull' => true,
'default' => 1,
'unsigned' => true,
]);
$table->addIndex(['uid'], 'authtoken_uid_index');
$table->addIndex(['version'], 'authtoken_version_index');

return $schema;
}
}
5 changes: 5 additions & 0 deletions lib/composer/composer/autoload_classmap.php
Original file line number Diff line number Diff line change
Expand Up @@ -421,6 +421,10 @@
'OC\\Authentication\\Token\\ExpiredTokenException' => $baseDir . '/lib/private/Authentication/Exceptions/ExpiredTokenException.php',
'OC\\Authentication\\Token\\IProvider' => $baseDir . '/lib/private/Authentication/Token/IProvider.php',
'OC\\Authentication\\Token\\IToken' => $baseDir . '/lib/private/Authentication/Token/IToken.php',
'OC\\Authentication\\Token\\Manager' => $baseDir . '/lib/private/Authentication/Token/Manager.php',
'OC\\Authentication\\Token\\PublicKeyToken' => $baseDir . '/lib/private/Authentication/Token/PublicKeyToken.php',
'OC\\Authentication\\Token\\PublicKeyTokenMapper' => $baseDir . '/lib/private/Authentication/Token/PublicKeyTokenMapper.php',
'OC\\Authentication\\Token\\PublicKeyTokenProvider' => $baseDir . '/lib/private/Authentication/Token/PublicKeyTokenProvider.php',
'OC\\Authentication\\TwoFactorAuth\\Manager' => $baseDir . '/lib/private/Authentication/TwoFactorAuth/Manager.php',
'OC\\Avatar' => $baseDir . '/lib/private/Avatar.php',
'OC\\AvatarManager' => $baseDir . '/lib/private/AvatarManager.php',
Expand Down Expand Up @@ -570,6 +574,7 @@
'OC\\Core\\Migrations\\Version14000Date20180129121024' => $baseDir . '/core/Migrations/Version14000Date20180129121024.php',
'OC\\Core\\Migrations\\Version14000Date20180404140050' => $baseDir . '/core/Migrations/Version14000Date20180404140050.php',
'OC\\Core\\Migrations\\Version14000Date20180516101403' => $baseDir . '/core/Migrations/Version14000Date20180516101403.php',
'OC\\Core\\Migrations\\Version14000Date20180518120534' => $baseDir . '/core/Migrations/Version14000Date20180518120534.php',
'OC\\DB\\Adapter' => $baseDir . '/lib/private/DB/Adapter.php',
'OC\\DB\\AdapterMySQL' => $baseDir . '/lib/private/DB/AdapterMySQL.php',
'OC\\DB\\AdapterOCI8' => $baseDir . '/lib/private/DB/AdapterOCI8.php',
Expand Down
5 changes: 5 additions & 0 deletions lib/composer/composer/autoload_static.php
Original file line number Diff line number Diff line change
Expand Up @@ -451,6 +451,10 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c
'OC\\Authentication\\Token\\ExpiredTokenException' => __DIR__ . '/../../..' . '/lib/private/Authentication/Exceptions/ExpiredTokenException.php',
'OC\\Authentication\\Token\\IProvider' => __DIR__ . '/../../..' . '/lib/private/Authentication/Token/IProvider.php',
'OC\\Authentication\\Token\\IToken' => __DIR__ . '/../../..' . '/lib/private/Authentication/Token/IToken.php',
'OC\\Authentication\\Token\\Manager' => __DIR__ . '/../../..' . '/lib/private/Authentication/Token/Manager.php',
'OC\\Authentication\\Token\\PublicKeyToken' => __DIR__ . '/../../..' . '/lib/private/Authentication/Token/PublicKeyToken.php',
'OC\\Authentication\\Token\\PublicKeyTokenMapper' => __DIR__ . '/../../..' . '/lib/private/Authentication/Token/PublicKeyTokenMapper.php',
'OC\\Authentication\\Token\\PublicKeyTokenProvider' => __DIR__ . '/../../..' . '/lib/private/Authentication/Token/PublicKeyTokenProvider.php',
'OC\\Authentication\\TwoFactorAuth\\Manager' => __DIR__ . '/../../..' . '/lib/private/Authentication/TwoFactorAuth/Manager.php',
'OC\\Avatar' => __DIR__ . '/../../..' . '/lib/private/Avatar.php',
'OC\\AvatarManager' => __DIR__ . '/../../..' . '/lib/private/AvatarManager.php',
Expand Down Expand Up @@ -600,6 +604,7 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c
'OC\\Core\\Migrations\\Version14000Date20180129121024' => __DIR__ . '/../../..' . '/core/Migrations/Version14000Date20180129121024.php',
'OC\\Core\\Migrations\\Version14000Date20180404140050' => __DIR__ . '/../../..' . '/core/Migrations/Version14000Date20180404140050.php',
'OC\\Core\\Migrations\\Version14000Date20180516101403' => __DIR__ . '/../../..' . '/core/Migrations/Version14000Date20180516101403.php',
'OC\\Core\\Migrations\\Version14000Date20180518120534' => __DIR__ . '/../../..' . '/core/Migrations/Version14000Date20180518120534.php',
'OC\\DB\\Adapter' => __DIR__ . '/../../..' . '/lib/private/DB/Adapter.php',
'OC\\DB\\AdapterMySQL' => __DIR__ . '/../../..' . '/lib/private/DB/AdapterMySQL.php',
'OC\\DB\\AdapterOCI8' => __DIR__ . '/../../..' . '/lib/private/DB/AdapterOCI8.php',
Expand Down
7 changes: 7 additions & 0 deletions lib/private/Authentication/Token/DefaultToken.php
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,12 @@
* @method void setRemember(int $remember)
* @method void setLastActivity(int $lastactivity)
* @method int getLastActivity()
* @method void setVersion(int $version)
*/
class DefaultToken extends Entity implements IToken {

const VERSION = 1;

/** @var string user UID */
protected $uid;

Expand Down Expand Up @@ -73,6 +76,9 @@ class DefaultToken extends Entity implements IToken {
/** @var int */
protected $expires;

/** @var int */
protected $version;

public function __construct() {
$this->addType('uid', 'string');
$this->addType('loginName', 'string');
Expand All @@ -85,6 +91,7 @@ public function __construct() {
$this->addType('lastCheck', 'int');
$this->addType('scope', 'string');
$this->addType('expires', 'int');
$this->addType('version', 'int');
}

public function getId(): int {
Expand Down
33 changes: 17 additions & 16 deletions lib/private/Authentication/Token/DefaultTokenMapper.php
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@
use OCP\AppFramework\Db\QBMapper;
use OCP\DB\QueryBuilder\IQueryBuilder;
use OCP\IDBConnection;
use OCP\IUser;

class DefaultTokenMapper extends QBMapper {

Expand All @@ -50,8 +49,8 @@ public function invalidate(string $token) {
/* @var $qb IQueryBuilder */
$qb = $this->db->getQueryBuilder();
$qb->delete('authtoken')
->where($qb->expr()->eq('token', $qb->createParameter('token')))
->setParameter('token', $token)
->where($qb->expr()->eq('token', $qb->createNamedParameter($token, IQueryBuilder::PARAM_STR)))
->andWhere($qb->expr()->eq('version', $qb->createNamedParameter(DefaultToken::VERSION, IQueryBuilder::PARAM_INT)))
->execute();
}

Expand All @@ -66,6 +65,7 @@ public function invalidateOld(int $olderThan, int $remember = IToken::DO_NOT_REM
->where($qb->expr()->lt('last_activity', $qb->createNamedParameter($olderThan, IQueryBuilder::PARAM_INT)))
->andWhere($qb->expr()->eq('type', $qb->createNamedParameter(IToken::TEMPORARY_TOKEN, IQueryBuilder::PARAM_INT)))
->andWhere($qb->expr()->eq('remember', $qb->createNamedParameter($remember, IQueryBuilder::PARAM_INT)))
->andWhere($qb->expr()->eq('version', $qb->createNamedParameter(DefaultToken::VERSION, IQueryBuilder::PARAM_INT)))
->execute();
}

Expand All @@ -79,9 +79,10 @@ public function invalidateOld(int $olderThan, int $remember = IToken::DO_NOT_REM
public function getToken(string $token): DefaultToken {
/* @var $qb IQueryBuilder */
$qb = $this->db->getQueryBuilder();
$result = $qb->select('*')
$result = $qb->select('id', 'uid', 'login_name', 'password', 'name', 'token', 'type', 'remember', 'last_activity', 'last_check', 'scope', 'expires', 'version')
->from('authtoken')
->where($qb->expr()->eq('token', $qb->createNamedParameter($token)))
->andWhere($qb->expr()->eq('version', $qb->createNamedParameter(DefaultToken::VERSION, IQueryBuilder::PARAM_INT)))
->execute();

$data = $result->fetch();
Expand All @@ -102,9 +103,10 @@ public function getToken(string $token): DefaultToken {
public function getTokenById(int $id): DefaultToken {
/* @var $qb IQueryBuilder */
$qb = $this->db->getQueryBuilder();
$result = $qb->select('*')
$result = $qb->select('id', 'uid', 'login_name', 'password', 'name', 'token', 'type', 'remember', 'last_activity', 'last_check', 'scope', 'expires', 'version')
->from('authtoken')
->where($qb->expr()->eq('id', $qb->createNamedParameter($id)))
->andWhere($qb->expr()->eq('version', $qb->createNamedParameter(DefaultToken::VERSION, IQueryBuilder::PARAM_INT)))
->execute();

$data = $result->fetch();
Expand All @@ -121,15 +123,16 @@ public function getTokenById(int $id): DefaultToken {
* The provider may limit the number of result rows in case of an abuse
* where a high number of (session) tokens is generated
*
* @param IUser $user
* @param string $uid
* @return DefaultToken[]
*/
public function getTokenByUser(IUser $user): array {
public function getTokenByUser(string $uid): array {
/* @var $qb IQueryBuilder */
$qb = $this->db->getQueryBuilder();
$qb->select('*')
$qb->select('id', 'uid', 'login_name', 'password', 'name', 'token', 'type', 'remember', 'last_activity', 'last_check', 'scope', 'expires', 'version')
->from('authtoken')
->where($qb->expr()->eq('uid', $qb->createNamedParameter($user->getUID())))
->where($qb->expr()->eq('uid', $qb->createNamedParameter($uid)))
->andWhere($qb->expr()->eq('version', $qb->createNamedParameter(DefaultToken::VERSION, IQueryBuilder::PARAM_INT)))
->setMaxResults(1000);
$result = $qb->execute();
$data = $result->fetchAll();
Expand All @@ -142,16 +145,13 @@ public function getTokenByUser(IUser $user): array {
return $entities;
}

/**
* @param IUser $user
* @param int $id
*/
public function deleteById(IUser $user, int $id) {
public function deleteById(string $uid, int $id) {
/* @var $qb IQueryBuilder */
$qb = $this->db->getQueryBuilder();
$qb->delete('authtoken')
->where($qb->expr()->eq('id', $qb->createNamedParameter($id)))
->andWhere($qb->expr()->eq('uid', $qb->createNamedParameter($user->getUID())));
->andWhere($qb->expr()->eq('uid', $qb->createNamedParameter($uid)))
->andWhere($qb->expr()->eq('version', $qb->createNamedParameter(DefaultToken::VERSION, IQueryBuilder::PARAM_INT)));
$qb->execute();
}

Expand All @@ -163,7 +163,8 @@ public function deleteById(IUser $user, int $id) {
public function deleteByName(string $name) {
$qb = $this->db->getQueryBuilder();
$qb->delete('authtoken')
->where($qb->expr()->eq('name', $qb->createNamedParameter($name), IQueryBuilder::PARAM_STR));
->where($qb->expr()->eq('name', $qb->createNamedParameter($name), IQueryBuilder::PARAM_STR))
->andWhere($qb->expr()->eq('version', $qb->createNamedParameter(DefaultToken::VERSION, IQueryBuilder::PARAM_INT)));
$qb->execute();
}

Expand Down
27 changes: 6 additions & 21 deletions lib/private/Authentication/Token/DefaultTokenProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@
use OCP\AppFramework\Utility\ITimeFactory;
use OCP\IConfig;
use OCP\ILogger;
use OCP\IUser;
use OCP\Security\ICrypto;

class DefaultTokenProvider implements IProvider {
Expand Down Expand Up @@ -105,6 +104,7 @@ public function generateToken(string $token,
$dbToken->setRemember($remember);
$dbToken->setLastActivity($this->time->getTime());
$dbToken->setLastCheck($this->time->getTime());
$dbToken->setVersion(DefaultToken::VERSION);

$this->mapper->insert($dbToken);

Expand Down Expand Up @@ -143,17 +143,8 @@ public function updateTokenActivity(IToken $token) {
}
}

/**
* Get all tokens of a user
*
* The provider may limit the number of result rows in case of an abuse
* where a high number of (session) tokens is generated
*
* @param IUser $user
* @return IToken[]
*/
public function getTokenByUser(IUser $user): array {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also remove \OCP\IUser use statement

return $this->mapper->getTokenByUser($user);
public function getTokenByUser(string $uid): array {
return $this->mapper->getTokenByUser($uid);
}

/**
Expand Down Expand Up @@ -265,14 +256,8 @@ public function invalidateToken(string $token) {
$this->mapper->invalidate($this->hashToken($token));
}

/**
* Invalidate (delete) the given token
*
* @param IUser $user
* @param int $id
*/
public function invalidateTokenById(IUser $user, int $id) {
$this->mapper->deleteById($user, $id);
public function invalidateTokenById(string $uid, int $id) {
$this->mapper->deleteById($uid, $id);
}

/**
Expand Down Expand Up @@ -313,7 +298,7 @@ public function rotate(IToken $token, string $oldTokenId, string $newTokenId): I
* @param string $token
* @return string
*/
private function hashToken(string $token) {
private function hashToken(string $token): string {
$secret = $this->config->getSystemValue('secret');
return hash('sha512', $token . $secret);
}
Expand Down
9 changes: 4 additions & 5 deletions lib/private/Authentication/Token/IProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@

use OC\Authentication\Exceptions\InvalidTokenException;
use OC\Authentication\Exceptions\PasswordlessTokenException;
use OCP\IUser;

interface IProvider {

Expand Down Expand Up @@ -92,10 +91,10 @@ public function invalidateToken(string $token);
/**
* Invalidate (delete) the given token
*
* @param IUser $user
* @param string $uid
* @param int $id
*/
public function invalidateTokenById(IUser $user, int $id);
public function invalidateTokenById(string $uid, int $id);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I usually prefer to work with IUser objects on higher-level services over stringly-typed UIDs. What's the reasoning for choosing strings over rich objects?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. Because we want to update all other tokens when we cdhange a password. I would need to inject the user manager to get the user object. Because the Tokens only have the uid.


/**
* Invalidate (delete) old session tokens
Expand All @@ -122,10 +121,10 @@ public function updateTokenActivity(IToken $token);
* The provider may limit the number of result rows in case of an abuse
* where a high number of (session) tokens is generated
*
* @param IUser $user
* @param string $uid
* @return IToken[]
*/
public function getTokenByUser(IUser $user): array;
public function getTokenByUser(string $uid): array;

/**
* Get the (unencrypted) password of the given token
Expand Down
Loading