diff options
author | blizzz <blizzz@arthur-schiwon.de> | 2022-08-04 12:16:46 +0300 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-08-04 12:16:46 +0300 |
commit | d7c442deaa7da8fad3a76206b59d07f400b89ce3 (patch) | |
tree | 19669e3f4eb5e508d975badd3b4a5376dc637b73 | |
parent | 4d84690a410b0172139185d4b284adce3e8c47ea (diff) | |
parent | bc29ff5567beead508ea551b424c53c91a40d000 (diff) |
Merge pull request #33443 from nextcloud/backport/33407/stable23
[stable23] Handle one time and large passwords
4 files changed, 119 insertions, 8 deletions
diff --git a/apps/settings/lib/Controller/ChangePasswordController.php b/apps/settings/lib/Controller/ChangePasswordController.php index 8dd1e6ba028..f595368563f 100644 --- a/apps/settings/lib/Controller/ChangePasswordController.php +++ b/apps/settings/lib/Controller/ChangePasswordController.php @@ -107,7 +107,7 @@ class ChangePasswordController extends Controller { } try { - if ($newpassword === null || $user->setPassword($newpassword) === false) { + if ($newpassword === null || strlen($newpassword) > 469 || $user->setPassword($newpassword) === false) { return new JSONResponse([ 'status' => 'error' ]); @@ -155,6 +155,16 @@ class ChangePasswordController extends Controller { ]); } + if (strlen($password) > 469) { + return new JSONResponse([ + 'status' => 'error', + 'data' => [ + 'message' => $this->l->t('Unable to change password. Password too long.'), + ], + ]); + } + + $currentUser = $this->userSession->getUser(); $targetUser = $this->userManager->get($username); if ($currentUser === null || $targetUser === null || diff --git a/config/config.sample.php b/config/config.sample.php index cd96f002bc9..8c916b197b6 100644 --- a/config/config.sample.php +++ b/config/config.sample.php @@ -319,6 +319,21 @@ $CONFIG = [ 'auth.webauthn.enabled' => true, /** + * Whether encrypted password should be stored in the database + * + * The passwords are only decrypted using the login token stored uniquely in the + * clients and allow to connect to external storages, autoconfigure mail account in + * the mail app and periodically check if the password it still valid. + * + * This might be desirable to disable this functionality when using one time + * passwords or when having a password policy enforcing long passwords (> 300 + * characters). + * + * By default the passwords are stored encrypted in the database. + */ +'auth.storeCryptedPassword' => true, + +/** * By default the login form is always available. There are cases (SSO) where an * admin wants to avoid users entering their credentials to the system if the SSO * app is unavailable. diff --git a/lib/private/Authentication/Token/PublicKeyTokenProvider.php b/lib/private/Authentication/Token/PublicKeyTokenProvider.php index c74cf960cb1..a6c672f70c6 100644 --- a/lib/private/Authentication/Token/PublicKeyTokenProvider.php +++ b/lib/private/Authentication/Token/PublicKeyTokenProvider.php @@ -370,7 +370,7 @@ class PublicKeyTokenProvider implements IProvider { $config = array_merge([ 'digest_alg' => 'sha512', - 'private_key_bits' => 2048, + 'private_key_bits' => $password !== null && strlen($password) > 250 ? 4096 : 2048, ], $this->config->getSystemValue('openssl', [])); // Generate new key @@ -392,7 +392,10 @@ class PublicKeyTokenProvider implements IProvider { $dbToken->setPublicKey($publicKey); $dbToken->setPrivateKey($this->encrypt($privateKey, $token)); - if (!is_null($password)) { + if (!is_null($password) && $this->config->getSystemValueBool('auth.storeCryptedPassword', true)) { + if (strlen($password) > 469) { + throw new \RuntimeException('Trying to save a password with more than 469 characters is not supported. If you want to use big passwords, disable the auth.storeCryptedPassword option in config.php'); + } $dbToken->setPassword($this->encryptPassword($password, $publicKey)); } @@ -422,7 +425,7 @@ class PublicKeyTokenProvider implements IProvider { $this->cache->clear(); // prevent setting an empty pw as result of pw-less-login - if ($password === '') { + if ($password === '' || !$this->config->getSystemValueBool('auth.storeCryptedPassword', true)) { return; } diff --git a/tests/lib/Authentication/Token/PublicKeyTokenProviderTest.php b/tests/lib/Authentication/Token/PublicKeyTokenProviderTest.php index a587944a930..768931562ad 100644 --- a/tests/lib/Authentication/Token/PublicKeyTokenProviderTest.php +++ b/tests/lib/Authentication/Token/PublicKeyTokenProviderTest.php @@ -25,6 +25,7 @@ namespace Test\Authentication\Token; use OC\Authentication\Exceptions\ExpiredTokenException; use OC\Authentication\Exceptions\InvalidTokenException; +use OC\Authentication\Exceptions\PasswordlessTokenException; use OC\Authentication\Token\DefaultToken; use OC\Authentication\Token\IToken; use OC\Authentication\Token\PublicKeyToken; @@ -85,6 +86,10 @@ class PublicKeyTokenProviderTest extends TestCase { $name = 'User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.12) Gecko/20101026 Firefox/3.6.12'; $type = IToken::PERMANENT_TOKEN; + $this->config->method('getSystemValueBool') + ->willReturnMap([ + ['auth.storeCryptedPassword', true, true], + ]); $actual = $this->tokenProvider->generateToken($token, $uid, $user, $password, $name, $type, IToken::DO_NOT_REMEMBER); $this->assertInstanceOf(PublicKeyToken::class, $actual); @@ -95,6 +100,48 @@ class PublicKeyTokenProviderTest extends TestCase { $this->assertSame($password, $this->tokenProvider->getPassword($actual, $token)); } + public function testGenerateTokenNoPassword(): void { + $token = 'token'; + $uid = 'user'; + $user = 'User'; + $password = 'passme'; + $name = 'User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.12) Gecko/20101026 Firefox/3.6.12'; + $type = IToken::PERMANENT_TOKEN; + $this->config->method('getSystemValueBool') + ->willReturnMap([ + ['auth.storeCryptedPassword', true, false], + ]); + $this->expectException(PasswordlessTokenException::class); + + $actual = $this->tokenProvider->generateToken($token, $uid, $user, $password, $name, $type, IToken::DO_NOT_REMEMBER); + + $this->assertInstanceOf(PublicKeyToken::class, $actual); + $this->assertSame($uid, $actual->getUID()); + $this->assertSame($user, $actual->getLoginName()); + $this->assertSame($name, $actual->getName()); + $this->assertSame(IToken::DO_NOT_REMEMBER, $actual->getRemember()); + $this->tokenProvider->getPassword($actual, $token); + } + + public function testGenerateTokenLongPassword() { + $token = 'token'; + $uid = 'user'; + $user = 'User'; + $password = ''; + for ($i = 0; $i < 500; $i++) { + $password .= 'e'; + } + $name = 'User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.12) Gecko/20101026 Firefox/3.6.12'; + $type = IToken::PERMANENT_TOKEN; + $this->config->method('getSystemValueBool') + ->willReturnMap([ + ['auth.storeCryptedPassword', true, true], + ]); + $this->expectException(\RuntimeException::class); + + $actual = $this->tokenProvider->generateToken($token, $uid, $user, $password, $name, $type, IToken::DO_NOT_REMEMBER); + } + public function testGenerateTokenInvalidName() { $token = 'token'; $uid = 'user'; @@ -105,6 +152,10 @@ class PublicKeyTokenProviderTest extends TestCase { . 'User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.12) Gecko/20101026 Firefox/3.6.12' . 'User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.12) Gecko/20101026 Firefox/3.6.12'; $type = IToken::PERMANENT_TOKEN; + $this->config->method('getSystemValueBool') + ->willReturnMap([ + ['auth.storeCryptedPassword', true, true], + ]); $actual = $this->tokenProvider->generateToken($token, $uid, $user, $password, $name, $type, IToken::DO_NOT_REMEMBER); @@ -122,6 +173,10 @@ class PublicKeyTokenProviderTest extends TestCase { ->method('updateActivity') ->with($tk, $this->time); $tk->setLastActivity($this->time - 200); + $this->config->method('getSystemValueBool') + ->willReturnMap([ + ['auth.storeCryptedPassword', true, true], + ]); $this->tokenProvider->updateTokenActivity($tk); @@ -159,6 +214,10 @@ class PublicKeyTokenProviderTest extends TestCase { $password = 'passme'; $name = 'User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.12) Gecko/20101026 Firefox/3.6.12'; $type = IToken::PERMANENT_TOKEN; + $this->config->method('getSystemValueBool') + ->willReturnMap([ + ['auth.storeCryptedPassword', true, true], + ]); $actual = $this->tokenProvider->generateToken($token, $uid, $user, $password, $name, $type, IToken::DO_NOT_REMEMBER); @@ -187,6 +246,10 @@ class PublicKeyTokenProviderTest extends TestCase { $name = 'User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.12) Gecko/20101026 Firefox/3.6.12'; $type = IToken::PERMANENT_TOKEN; + $this->config->method('getSystemValueBool') + ->willReturnMap([ + ['auth.storeCryptedPassword', true, true], + ]); $actual = $this->tokenProvider->generateToken($token, $uid, $user, $password, $name, $type, IToken::DO_NOT_REMEMBER); $this->tokenProvider->getPassword($actual, 'wrongtoken'); @@ -199,6 +262,10 @@ class PublicKeyTokenProviderTest extends TestCase { $password = 'passme'; $name = 'User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.12) Gecko/20101026 Firefox/3.6.12'; $type = IToken::PERMANENT_TOKEN; + $this->config->method('getSystemValueBool') + ->willReturnMap([ + ['auth.storeCryptedPassword', true, true], + ]); $actual = $this->tokenProvider->generateToken($token, $uid, $user, $password, $name, $type, IToken::DO_NOT_REMEMBER); @@ -303,7 +370,7 @@ class PublicKeyTokenProviderTest extends TestCase { $this->tokenProvider->renewSessionToken('oldId', 'newId'); } - public function testRenewSessionTokenWithPassword() { + public function testRenewSessionTokenWithPassword(): void { $token = 'oldId'; $uid = 'user'; $user = 'User'; @@ -311,6 +378,10 @@ class PublicKeyTokenProviderTest extends TestCase { $name = 'User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.12) Gecko/20101026 Firefox/3.6.12'; $type = IToken::PERMANENT_TOKEN; + $this->config->method('getSystemValueBool') + ->willReturnMap([ + ['auth.storeCryptedPassword', true, true], + ]); $oldToken = $this->tokenProvider->generateToken($token, $uid, $user, $password, $name, $type, IToken::DO_NOT_REMEMBER); $this->mapper @@ -321,7 +392,7 @@ class PublicKeyTokenProviderTest extends TestCase { $this->mapper ->expects($this->once()) ->method('insert') - ->with($this->callback(function (PublicKeyToken $token) use ($user, $uid, $name) { + ->with($this->callback(function (PublicKeyToken $token) use ($user, $uid, $name): bool { return $token->getUID() === $uid && $token->getLoginName() === $user && $token->getName() === $name && @@ -333,14 +404,14 @@ class PublicKeyTokenProviderTest extends TestCase { $this->mapper ->expects($this->once()) ->method('delete') - ->with($this->callback(function ($token) use ($oldToken) { + ->with($this->callback(function ($token) use ($oldToken): bool { return $token === $oldToken; })); $this->tokenProvider->renewSessionToken('oldId', 'newId'); } - public function testGetToken() { + public function testGetToken(): void { $token = new PublicKeyToken(); $this->config->method('getSystemValue') @@ -443,6 +514,10 @@ class PublicKeyTokenProviderTest extends TestCase { $name = 'User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.12) Gecko/20101026 Firefox/3.6.12'; $type = IToken::PERMANENT_TOKEN; + $this->config->method('getSystemValueBool') + ->willReturnMap([ + ['auth.storeCryptedPassword', true, true], + ]); $actual = $this->tokenProvider->generateToken($token, $uid, $user, $password, $name, $type, IToken::DO_NOT_REMEMBER); $new = $this->tokenProvider->rotate($actual, 'oldtoken', 'newtoken'); @@ -487,6 +562,10 @@ class PublicKeyTokenProviderTest extends TestCase { ->method('update') ->willReturnArgument(0); + $this->config->method('getSystemValueBool') + ->willReturnMap([ + ['auth.storeCryptedPassword', true, true], + ]); $newToken = $this->tokenProvider->convertToken($defaultToken, 'newToken', 'newPassword'); $this->assertSame(42, $newToken->getId()); @@ -540,6 +619,10 @@ class PublicKeyTokenProviderTest extends TestCase { 'random2', IToken::PERMANENT_TOKEN, IToken::REMEMBER); + $this->config->method('getSystemValueBool') + ->willReturnMap([ + ['auth.storeCryptedPassword', true, true], + ]); $this->mapper->method('hasExpiredTokens') ->with($uid) |