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

fix remember me login #1347

Merged
merged 6 commits into from
Nov 2, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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: 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 @@ -1035,6 +1035,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 @@ -290,6 +290,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
41 changes: 37 additions & 4 deletions lib/private/Authentication/Token/DefaultTokenProvider.php
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
<?php
/**
* @copyright Copyright (c) 2016, ownCloud, Inc.
* @copyright Copyright (c) 2016, Christoph Wurst <[email protected]>
*
* @author Christoph Wurst <[email protected]>
*
Expand Down Expand Up @@ -56,7 +57,11 @@ class DefaultTokenProvider implements IProvider {
* @param ILogger $logger
* @param ITimeFactory $time
*/
public function __construct(DefaultTokenMapper $mapper, ICrypto $crypto, IConfig $config, ILogger $logger, ITimeFactory $time) {
public function __construct(DefaultTokenMapper $mapper,
ICrypto $crypto,
IConfig $config,
ILogger $logger,
ITimeFactory $time) {
$this->mapper = $mapper;
$this->crypto = $crypto;
$this->config = $config;
Expand All @@ -73,9 +78,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 +91,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 All @@ -96,6 +103,7 @@ public function generateToken($token, $uid, $loginName, $password, $name, $type
* Save the updated token
*
* @param IToken $token
* @throws InvalidTokenException
*/
public function updateToken(IToken $token) {
if (!($token instanceof DefaultToken)) {
Expand Down Expand Up @@ -151,6 +159,28 @@ public function getToken($tokenId) {
}
}

/**
* @param string $oldSessionId
* @param string $sessionId
* @throws InvalidTokenException
*/
public function renewSessionToken($oldSessionId, $sessionId) {
Copy link
Member

Choose a reason for hiding this comment

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

Function needs tests. Will write myself later.

$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 +237,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
13 changes: 12 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,15 @@ public function generateToken($token, $uid, $loginName, $password, $name, $type
*/
public function getToken($tokenId) ;

/**
* Duplicate an existing session token
*
* @param string $oldSessionId
Copy link
Member

Choose a reason for hiding this comment

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

Add at least a single line comment what the function is supposed to do? :)

Copy link
Member Author

Choose a reason for hiding this comment

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

done

* @param string $sessionId
* @throws InvalidTokenException
*/
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 REMEMBER_LOGIN = 'two_factor_remember_login';

/** @var AppManager */
private $appManager;
Expand Down Expand Up @@ -114,6 +115,7 @@ public function getBackupProvider(IUser $user) {
* @param IUser $user
* @param bool $includeBackupApp
* @return IProvider[]
* @throws Exception
*/
public function getProviders(IUser $user, $includeBackupApp = false) {
$allApps = $this->appManager->getEnabledAppsForUser($user);
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::REMEMBER_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::REMEMBER_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::REMEMBER_LOGIN, $rememberMe);
}

}
4 changes: 2 additions & 2 deletions lib/private/Server.php
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ public function __construct($webRoot, \OC\Config $config) {
$defaultTokenProvider = null;
}

$userSession = new \OC\User\Session($manager, $session, $timeFactory, $defaultTokenProvider, $c->getConfig());
$userSession = new \OC\User\Session($manager, $session, $timeFactory, $defaultTokenProvider, $c->getConfig(), $c->getSecureRandom());
$userSession->listen('\OC\User', 'preCreateUser', function ($uid, $password) {
\OC_Hook::emit('OC_User', 'pre_createUser', array('run' => true, 'uid' => $uid, 'password' => $password));
});
Expand Down Expand Up @@ -283,7 +283,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