Skip to content

Commit

Permalink
bring back remember-me
Browse files Browse the repository at this point in the history
* try to reuse the old session token for remember me login
* decrypt/encrypt token password and set the session id accordingly
* create remember-me cookies only if checkbox is checked and 2fa solved
* adjust db token cleanup to store remembered tokens longer
* adjust unit tests

Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>
  • Loading branch information
ChristophWurst committed Oct 25, 2016
1 parent 01a85a9 commit 21a1aa1
Show file tree
Hide file tree
Showing 20 changed files with 424 additions and 188 deletions.
11 changes: 8 additions & 3 deletions core/Controller/LoginController.php
Original file line number Diff line number Diff line change
Expand Up @@ -196,9 +196,10 @@ private function generateRedirect($redirectUrl) {
* @param string $user
* @param string $password
* @param string $redirect_url
* @param boolean $remember_login
* @return RedirectResponse
*/
public function tryLogin($user, $password, $redirect_url) {
public function tryLogin($user, $password, $redirect_url, $remember_login = false) {
$currentDelay = $this->throttler->getDelay($this->request->getRemoteAddress());
$this->throttler->sleepDelay($this->request->getRemoteAddress());

Expand Down Expand Up @@ -236,13 +237,13 @@ public function tryLogin($user, $password, $redirect_url) {
// TODO: remove password checks from above and let the user session handle failures
// requires https://github.com/owncloud/core/pull/24616
$this->userSession->login($user, $password);
$this->userSession->createSessionToken($this->request, $loginResult->getUID(), $user, $password);
$this->userSession->createSessionToken($this->request, $loginResult->getUID(), $user, $password, $remember_login);

// User has successfully logged in, now remove the password reset link, when it is available
$this->config->deleteUserValue($loginResult->getUID(), 'core', 'lostpassword');

if ($this->twoFactorManager->isTwoFactorAuthenticated($loginResult)) {
$this->twoFactorManager->prepareTwoFactorLogin($loginResult);
$this->twoFactorManager->prepareTwoFactorLogin($loginResult, $remember_login);

$providers = $this->twoFactorManager->getProviders($loginResult);
if (count($providers) === 1) {
Expand All @@ -265,6 +266,10 @@ public function tryLogin($user, $password, $redirect_url) {
return new RedirectResponse($this->urlGenerator->linkToRoute($url, $urlParams));
}

if ($remember_login) {
$this->userSession->createRememberMeToken($loginResult);
}

return $this->generateRedirect($redirect_url);
}

Expand Down
9 changes: 5 additions & 4 deletions core/Controller/TwoFactorChallengeController.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
namespace OC\Core\Controller;

use OC\Authentication\TwoFactorAuth\Manager;
use OC\User\Session;
use OC_User;
use OC_Util;
use OCP\AppFramework\Controller;
Expand All @@ -32,14 +33,14 @@
use OCP\IRequest;
use OCP\ISession;
use OCP\IURLGenerator;
use OCP\IUserSession;
use OCP\IUserSession as UserSession;

class TwoFactorChallengeController extends Controller {

/** @var Manager */
private $twoFactorManager;

/** @var IUserSession */
/** @var UserSession */
private $userSession;

/** @var ISession */
Expand All @@ -53,10 +54,10 @@ class TwoFactorChallengeController extends Controller {
* @param IRequest $request
* @param Manager $twoFactorManager
* @param IUserSession $userSession
* @param ISession $session
* @param UserSession $session
* @param IURLGenerator $urlGenerator
*/
public function __construct($appName, IRequest $request, Manager $twoFactorManager, IUserSession $userSession,
public function __construct($appName, IRequest $request, Manager $twoFactorManager, UserSession $userSession,
ISession $session, IURLGenerator $urlGenerator) {
parent::__construct($appName, $request);
$this->twoFactorManager = $twoFactorManager;
Expand Down
9 changes: 9 additions & 0 deletions db_structure.xml
Original file line number Diff line number Diff line change
Expand Up @@ -1125,6 +1125,15 @@
<length>2</length>
</field>

<field>
<name>remember</name>
<type>integer</type>
<default>0</default>
<notnull>true</notnull>
<unsigned>true</unsigned>
<length>1</length>
</field>

<field>
<name>last_activity</name>
<type>integer</type>
Expand Down
6 changes: 6 additions & 0 deletions lib/base.php
Original file line number Diff line number Diff line change
Expand Up @@ -1040,6 +1040,12 @@ static function handleLogin(OCP\IRequest $request) {
if ($userSession->tryTokenLogin($request)) {
return true;
}
if (isset($_COOKIE['nc_username'])
&& isset($_COOKIE['nc_token'])
&& isset($_COOKIE['nc_session_id'])
&& $userSession->loginWithCookie($_COOKIE['nc_username'], $_COOKIE['nc_token'], $_COOKIE['nc_session_id'])) {
return true;
}
if ($userSession->tryBasicAuthLogin($request, \OC::$server->getBruteForceThrottler())) {
return true;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,7 @@ public function __construct($appName, $urlParams = array()){
$this->registerService('OCP\\IUserSession', function($c) {
return $this->getServer()->getUserSession();
});
$this->registerAlias(\OC\User\Session::class, \OCP\IUserSession::class);

$this->registerService('OCP\\ISession', function($c) {
return $this->getServer()->getSession();
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 @@ -35,6 +35,8 @@
* @method string getToken()
* @method void setType(string $type)
* @method int getType()
* @method void setRemember(int $remember)
* @method int getRemember()
* @method void setLastActivity(int $lastActivity)
* @method int getLastActivity()
*/
Expand Down Expand Up @@ -70,6 +72,11 @@ class DefaultToken extends Entity implements IToken {
*/
protected $type;

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

/**
* @var int
*/
Expand Down
17 changes: 9 additions & 8 deletions lib/private/Authentication/Token/DefaultTokenMapper.php
Original file line number Diff line number Diff line change
Expand Up @@ -40,24 +40,25 @@ public function __construct(IDBConnection $db) {
* @param string $token
*/
public function invalidate($token) {
/* @var $qb IQueryBuilder */
$qb = $this->db->getQueryBuilder();
$qb->delete('authtoken')
->andWhere($qb->expr()->eq('token', $qb->createParameter('token')))
->where($qb->expr()->eq('token', $qb->createParameter('token')))
->setParameter('token', $token)
->execute();
}

/**
* @param int $olderThan
* @param int $remember
*/
public function invalidateOld($olderThan) {
public function invalidateOld($olderThan, $remember = IToken::DO_NOT_REMEMBER) {
/* @var $qb IQueryBuilder */
$qb = $this->db->getQueryBuilder();
$qb->delete('authtoken')
->where($qb->expr()->lt('last_activity', $qb->createParameter('last_activity')))
->andWhere($qb->expr()->eq('type', $qb->createParameter('type')))
->setParameter('last_activity', $olderThan, IQueryBuilder::PARAM_INT)
->setParameter('type', IToken::TEMPORARY_TOKEN, IQueryBuilder::PARAM_INT)
->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)))
->execute();
}

Expand All @@ -71,7 +72,7 @@ public function invalidateOld($olderThan) {
public function getToken($token) {
/* @var $qb IQueryBuilder */
$qb = $this->db->getQueryBuilder();
$result = $qb->select('id', 'uid', 'login_name', 'password', 'name', 'type', 'token', 'last_activity', 'last_check')
$result = $qb->select('id', 'uid', 'login_name', 'password', 'name', 'type', 'remember', 'token', 'last_activity', 'last_check')
->from('authtoken')
->where($qb->expr()->eq('token', $qb->createParameter('token')))
->setParameter('token', $token)
Expand All @@ -97,7 +98,7 @@ public function getToken($token) {
public function getTokenByUser(IUser $user) {
/* @var $qb IQueryBuilder */
$qb = $this->db->getQueryBuilder();
$qb->select('id', 'uid', 'login_name', 'password', 'name', 'type', 'token', 'last_activity', 'last_check')
$qb->select('id', 'uid', 'login_name', 'password', 'name', 'type', 'remember', 'token', 'last_activity', 'last_check')
->from('authtoken')
->where($qb->expr()->eq('uid', $qb->createNamedParameter($user->getUID())))
->setMaxResults(1000);
Expand Down
32 changes: 29 additions & 3 deletions lib/private/Authentication/Token/DefaultTokenProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,10 @@ public function __construct(DefaultTokenMapper $mapper, ICrypto $crypto, IConfig
* @param string|null $password
* @param string $name
* @param int $type token type
* @param int $remember whether the session token should be used for remember-me
* @return IToken
*/
public function generateToken($token, $uid, $loginName, $password, $name, $type = IToken::TEMPORARY_TOKEN) {
public function generateToken($token, $uid, $loginName, $password, $name, $type = IToken::TEMPORARY_TOKEN, $remember = IToken::DO_NOT_REMEMBER) {
$dbToken = new DefaultToken();
$dbToken->setUid($uid);
$dbToken->setLoginName($loginName);
Expand All @@ -85,6 +86,7 @@ public function generateToken($token, $uid, $loginName, $password, $name, $type
$dbToken->setName($name);
$dbToken->setToken($this->hashToken($token));
$dbToken->setType($type);
$dbToken->setRemember($remember);
$dbToken->setLastActivity($this->time->getTime());

$this->mapper->insert($dbToken);
Expand Down Expand Up @@ -151,6 +153,27 @@ public function getToken($tokenId) {
}
}

/**
* @param string $oldSessionId
* @param string $sessionId
*/
public function renewSessionToken($oldSessionId, $sessionId) {
$token = $this->getToken($oldSessionId);

$newToken = new DefaultToken();
$newToken->setUid($token->getUID());
$newToken->setLoginName($token->getLoginName());
if (!is_null($token->getPassword())) {
$password = $this->decryptPassword($token->getPassword(), $oldSessionId);
$newToken->setPassword($this->encryptPassword($password, $sessionId));
}
$newToken->setName($token->getName());
$newToken->setToken($this->hashToken($sessionId));
$newToken->setType(IToken::TEMPORARY_TOKEN);
$newToken->setLastActivity($this->time->getTime());
$this->mapper->insert($newToken);
}

/**
* @param IToken $savedToken
* @param string $tokenId session token
Expand Down Expand Up @@ -207,8 +230,11 @@ public function invalidateTokenById(IUser $user, $id) {
*/
public function invalidateOldTokens() {
$olderThan = $this->time->getTime() - (int) $this->config->getSystemValue('session_lifetime', 60 * 60 * 24);
$this->logger->info('Invalidating tokens older than ' . date('c', $olderThan));
$this->mapper->invalidateOld($olderThan);
$this->logger->info('Invalidating session tokens older than ' . date('c', $olderThan));
$this->mapper->invalidateOld($olderThan, IToken::DO_NOT_REMEMBER);
$rememberThreshold = $this->time->getTime() - (int) $this->config->getSystemValue('remember_login_cookie_lifetime', 60 * 60 * 24 * 15);
$this->logger->info('Invalidating remembered session tokens older than ' . date('c', $rememberThreshold));
$this->mapper->invalidateOld($rememberThreshold, IToken::REMEMBER);
}

/**
Expand Down
10 changes: 9 additions & 1 deletion lib/private/Authentication/Token/IProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@

interface IProvider {


/**
* Create and persist a new token
*
Expand All @@ -37,9 +38,10 @@ interface IProvider {
* @param string|null $password
* @param string $name
* @param int $type token type
* @param int $remember whether the session token should be used for remember-me
* @return IToken
*/
public function generateToken($token, $uid, $loginName, $password, $name, $type = IToken::TEMPORARY_TOKEN);
public function generateToken($token, $uid, $loginName, $password, $name, $type = IToken::TEMPORARY_TOKEN, $remember = IToken::DO_NOT_REMEMBER);

/**
* Get a token by token id
Expand All @@ -50,6 +52,12 @@ public function generateToken($token, $uid, $loginName, $password, $name, $type
*/
public function getToken($tokenId) ;

/**
* @param string $oldSessionId
* @param string $sessionId
*/
public function renewSessionToken($oldSessionId, $sessionId);

/**
* Invalidate (delete) the given session token
*
Expand Down
2 changes: 2 additions & 0 deletions lib/private/Authentication/Token/IToken.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ interface IToken extends JsonSerializable {

const TEMPORARY_TOKEN = 0;
const PERMANENT_TOKEN = 1;
const DO_NOT_REMEMBER = 0;
const REMEMBER = 1;

/**
* Get the token ID
Expand Down
19 changes: 14 additions & 5 deletions lib/private/Authentication/TwoFactorAuth/Manager.php
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ class Manager {
const SESSION_UID_KEY = 'two_factor_auth_uid';
const BACKUP_CODES_APP_ID = 'twofactor_backupcodes';
const BACKUP_CODES_PROVIDER_ID = 'backup_codes';
const REMEBER_LOGIN = 'two_factor_remember_login';

/** @var AppManager */
private $appManager;
Expand All @@ -51,6 +52,7 @@ class Manager {
* @param AppManager $appManager
* @param ISession $session
* @param IConfig $config
* @param Session $userSession
*/
public function __construct(AppManager $appManager, ISession $session, IConfig $config) {
$this->appManager = $appManager;
Expand Down Expand Up @@ -171,11 +173,16 @@ public function verifyChallenge($providerId, IUser $user, $challenge) {
return false;
}

$result = $provider->verifyChallenge($user, $challenge);
if ($result) {
$passed = $provider->verifyChallenge($user, $challenge);
if ($passed) {
if ($this->session->get(self::REMEBER_LOGIN) === true) {
// TODO: resolve cyclic dependency and use DI
\OC::$server->getUserSession()->createRememberMeToken($user);
}
$this->session->remove(self::SESSION_UID_KEY);
$this->session->remove(self::REMEBER_LOGIN);
}
return $result;
return $passed;
}

/**
Expand All @@ -202,12 +209,14 @@ public function needsSecondFactor(IUser $user = null) {
}

/**
* Prepare the 2FA login (set session value)
* Prepare the 2FA login
*
* @param IUser $user
* @param boolean $rememberMe
*/
public function prepareTwoFactorLogin(IUser $user) {
public function prepareTwoFactorLogin(IUser $user, $rememberMe) {
$this->session->set(self::SESSION_UID_KEY, $user->getUID());
$this->session->set(self::REMEBER_LOGIN, $rememberMe);
}

}
2 changes: 1 addition & 1 deletion lib/private/Server.php
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,7 @@ public function __construct($webRoot, \OC\Config $config) {
return $userSession;
});

$this->registerService('\OC\Authentication\TwoFactorAuth\Manager', function (Server $c) {
$this->registerService(\OC\Authentication\TwoFactorAuth\Manager::class, function (Server $c) {
return new \OC\Authentication\TwoFactorAuth\Manager($c->getAppManager(), $c->getSession(), $c->getConfig());
});

Expand Down
Loading

0 comments on commit 21a1aa1

Please sign in to comment.