From 00e99af5863e40e89c012f3ce642802c891def4e Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Wed, 26 Sep 2018 13:10:17 +0200 Subject: [PATCH] Mark token as invalid if the password doesn't match Signed-off-by: Roeland Jago Douma --- .../Authentication/Token/DefaultTokenProvider.php | 10 ++++++++++ lib/private/Authentication/Token/IProvider.php | 8 ++++++++ lib/private/Authentication/Token/Manager.php | 5 +++++ lib/private/Authentication/Token/PublicKeyToken.php | 6 ++++++ .../Authentication/Token/PublicKeyTokenProvider.php | 11 +++++++++++ lib/private/User/Session.php | 13 ++++++++++--- tests/lib/User/SessionTest.php | 6 ++---- 7 files changed, 52 insertions(+), 7 deletions(-) diff --git a/lib/private/Authentication/Token/DefaultTokenProvider.php b/lib/private/Authentication/Token/DefaultTokenProvider.php index ad45303fa7..19aba58b05 100644 --- a/lib/private/Authentication/Token/DefaultTokenProvider.php +++ b/lib/private/Authentication/Token/DefaultTokenProvider.php @@ -338,4 +338,14 @@ class DefaultTokenProvider implements IProvider { } } + public function markPasswordInvalid(IToken $token, string $tokenId) { + if (!($token instanceof DefaultToken)) { + throw new InvalidTokenException(); + } + + //No need to mark as invalid. We just invalide default tokens + $this->invalidateToken($tokenId); + } + + } diff --git a/lib/private/Authentication/Token/IProvider.php b/lib/private/Authentication/Token/IProvider.php index ab46bd1212..d1b067868b 100644 --- a/lib/private/Authentication/Token/IProvider.php +++ b/lib/private/Authentication/Token/IProvider.php @@ -156,4 +156,12 @@ interface IProvider { * @return IToken */ public function rotate(IToken $token, string $oldTokenId, string $newTokenId): IToken; + + /** + * Marks a token as having an invalid password. + * + * @param IToken $token + * @param string $tokenId + */ + public function markPasswordInvalid(IToken $token, string $tokenId); } diff --git a/lib/private/Authentication/Token/Manager.php b/lib/private/Authentication/Token/Manager.php index 254a159894..711d211039 100644 --- a/lib/private/Authentication/Token/Manager.php +++ b/lib/private/Authentication/Token/Manager.php @@ -227,4 +227,9 @@ class Manager implements IProvider { } throw new InvalidTokenException(); } + + + public function markPasswordInvalid(IToken $token, string $tokenId) { + $this->getProvider($token)->markPasswordInvalid($token, $tokenId); + } } diff --git a/lib/private/Authentication/Token/PublicKeyToken.php b/lib/private/Authentication/Token/PublicKeyToken.php index 0e793ce8c7..9896915ca3 100644 --- a/lib/private/Authentication/Token/PublicKeyToken.php +++ b/lib/private/Authentication/Token/PublicKeyToken.php @@ -43,6 +43,8 @@ use OCP\AppFramework\Db\Entity; * @method string getPublicKey() * @method void setPublicKey(string $key) * @method void setVersion(int $version) + * @method bool getPasswordInvalid() + * @method void setPasswordInvalid(bool $invalid); */ class PublicKeyToken extends Entity implements IToken { @@ -90,6 +92,9 @@ class PublicKeyToken extends Entity implements IToken { /** @var int */ protected $version; + /** @var bool */ + protected $passwordInvalid; + public function __construct() { $this->addType('uid', 'string'); $this->addType('loginName', 'string'); @@ -105,6 +110,7 @@ class PublicKeyToken extends Entity implements IToken { $this->addType('publicKey', 'string'); $this->addType('privateKey', 'string'); $this->addType('version', 'int'); + $this->addType('passwordInvalid', 'bool'); } public function getId(): int { diff --git a/lib/private/Authentication/Token/PublicKeyTokenProvider.php b/lib/private/Authentication/Token/PublicKeyTokenProvider.php index 7e98ee939c..9afdb5a8ff 100644 --- a/lib/private/Authentication/Token/PublicKeyTokenProvider.php +++ b/lib/private/Authentication/Token/PublicKeyTokenProvider.php @@ -317,4 +317,15 @@ class PublicKeyTokenProvider implements IProvider { return $dbToken; } + + public function markPasswordInvalid(IToken $token, string $tokenId) { + if (!($token instanceof PublicKeyToken)) { + throw new InvalidTokenException(); + } + + $token->setPasswordInvalid(true); + $this->mapper->update($token); + } + + } diff --git a/lib/private/User/Session.php b/lib/private/User/Session.php index 5593e178ca..8ac42eac4e 100644 --- a/lib/private/User/Session.php +++ b/lib/private/User/Session.php @@ -694,12 +694,19 @@ class Session implements IUserSession, Emitter { return true; } - if ($this->manager->checkPassword($dbToken->getLoginName(), $pwd) === false - || (!is_null($this->activeUser) && !$this->activeUser->isEnabled())) { + // Invalidate token if the user is no longer active + if (!is_null($this->activeUser) && !$this->activeUser->isEnabled()) { $this->tokenProvider->invalidateToken($token); - // Password has changed or user was disabled -> log user out return false; } + + // If the token password is no longer valid mark it as such + if ($this->manager->checkPassword($dbToken->getLoginName(), $pwd) === false) { + $this->tokenProvider->markPasswordInvalid($dbToken, $token); + // User is logged out + return false; + } + $dbToken->setLastCheck($now); return true; } diff --git a/tests/lib/User/SessionTest.php b/tests/lib/User/SessionTest.php index 24677b57dd..81ceade9e0 100644 --- a/tests/lib/User/SessionTest.php +++ b/tests/lib/User/SessionTest.php @@ -1017,10 +1017,8 @@ class SessionTest extends \Test\TestCase { ->method('getPassword') ->with($token, 'APP-PASSWORD') ->will($this->returnValue('123456')); - $userManager->expects($this->once()) - ->method('checkPassword') - ->with('susan', '123456') - ->will($this->returnValue(true)); + $userManager->expects($this->never()) + ->method('checkPassword'); $user->expects($this->once()) ->method('isEnabled') ->will($this->returnValue(false));