Merge pull request #31677 from nextcloud/backport/31658/stable22

[stable22] Limit the length of app password names
2026-03-20 09:32:35 -04:00 · 2022-04-14 14:15:08 +02:00 · 2022-04-14 14:15:08 +02:00 · a96a2ec55c
commit a96a2ec55c
parent ff16b5870c 9b386f2b43
7 changed files with 107 additions and 75 deletions
--- a/apps/settings/lib/Controller/AuthSettingsController.php
+++ b/apps/settings/lib/Controller/AuthSettingsController.php
@ -145,6 +145,10 @@ class AuthSettingsController extends Controller {
 			return $this->getServiceNotAvailableResponse();
 		}

+		if (mb_strlen($name) > 128) {
+			$name = mb_substr($name, 0, 120) . '…';
+		}
+
 		$token = $this->generateRandomDeviceToken();
 		$deviceToken = $this->tokenProvider->generateToken($token, $this->uid, $loginName, $password, $name, IToken::PERMANENT_TOKEN);
 		$tokenData = $deviceToken->jsonSerialize();
@ -241,6 +245,10 @@ class AuthSettingsController extends Controller {
 			$this->publishActivity($scope['filesystem'] ? Provider::APP_TOKEN_FILESYSTEM_GRANTED : Provider::APP_TOKEN_FILESYSTEM_REVOKED, $token->getId(), ['name' => $currentName]);
 		}

+		if (mb_strlen($name) > 128) {
+			$name = mb_substr($name, 0, 120) . '…';
+		}
+
 		if ($token instanceof INamedToken && $name !== $currentName) {
 			$token->setName($name);
 			$this->publishActivity(Provider::APP_TOKEN_RENAMED, $token->getId(), ['name' => $currentName, 'newName' => $name]);
--- a/lib/private/Authentication/Token/IProvider.php
+++ b/lib/private/Authentication/Token/IProvider.php
@ -44,7 +44,7 @@ interface IProvider {
 	 * @param string $uid
 	 * @param string $loginName
 	 * @param string|null $password
-	 * @param string $name
+	 * @param string $name Name will be trimmed to 120 chars when longer
 	 * @param int $type token type
 	 * @param int $remember whether the session token should be used for remember-me
 	 * @return IToken
--- a/lib/private/Authentication/Token/Manager.php
+++ b/lib/private/Authentication/Token/Manager.php
@ -53,7 +53,7 @@ class Manager implements IProvider {
 	 * @param string $uid
 	 * @param string $loginName
 	 * @param string|null $password
-	 * @param string $name
+	 * @param string $name Name will be trimmed to 120 chars when longer
 	 * @param int $type token type
 	 * @param int $remember whether the session token should be used for remember-me
 	 * @return IToken
@ -65,6 +65,10 @@ class Manager implements IProvider {
 								  string $name,
 								  int $type = IToken::TEMPORARY_TOKEN,
 								  int $remember = IToken::DO_NOT_REMEMBER): IToken {
+		if (mb_strlen($name) > 128) {
+			$name = mb_substr($name, 0, 120) . '…';
+		}
+
 		try {
 			return $this->publicKeyTokenProvider->generateToken(
 				$token,
--- a/lib/private/Authentication/Token/PublicKeyTokenProvider.php
+++ b/lib/private/Authentication/Token/PublicKeyTokenProvider.php
@ -84,6 +84,10 @@ class PublicKeyTokenProvider implements IProvider {
 								  string $name,
 								  int $type = IToken::TEMPORARY_TOKEN,
 								  int $remember = IToken::DO_NOT_REMEMBER): IToken {
+		if (mb_strlen($name) > 128) {
+			throw new InvalidTokenException('The given name is too long');
+		}
+
 		$dbToken = $this->newToken($token, $uid, $loginName, $password, $name, $type, $remember);
 		$this->mapper->insert($dbToken);

--- a/tests/lib/Authentication/Token/ManagerTest.php
+++ b/tests/lib/Authentication/Token/ManagerTest.php
@ -127,6 +127,37 @@ class ManagerTest extends TestCase {
 		$this->assertSame($token, $actual);
 	}

+	public function testGenerateTokenTooLongName() {
+		$token = $this->createMock(IToken::class);
+		$token->method('getName')
+			->willReturn(str_repeat('a', 120) . '…');
+
+
+		$this->publicKeyTokenProvider->expects($this->once())
+			->method('generateToken')
+			->with(
+				'token',
+				'uid',
+				'loginName',
+				'password',
+				str_repeat('a', 120) . '…',
+				IToken::TEMPORARY_TOKEN,
+				IToken::REMEMBER
+			)->willReturn($token);
+
+		$actual = $this->manager->generateToken(
+			'token',
+			'uid',
+			'loginName',
+			'password',
+			str_repeat('a', 200),
+			IToken::TEMPORARY_TOKEN,
+			IToken::REMEMBER
+		);
+
+		$this->assertSame(121, mb_strlen($actual->getName()));
+	}
+
 	public function tokenData(): array {
 		return [
 			[new DefaultToken()],
--- a/tests/lib/Authentication/Token/PublicKeyTokenProviderTest.php
+++ b/tests/lib/Authentication/Token/PublicKeyTokenProviderTest.php
@ -82,10 +82,7 @@ class PublicKeyTokenProviderTest extends TestCase {
 		$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'
-			. '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'
-			. 'User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.12) Gecko/20101026 Firefox/3.6.12';
+		$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;

 		$actual = $this->tokenProvider->generateToken($token, $uid, $user, $password, $name, $type, IToken::DO_NOT_REMEMBER);
@ -98,6 +95,22 @@ class PublicKeyTokenProviderTest extends TestCase {
 		$this->assertSame($password, $this->tokenProvider->getPassword($actual, $token));
 	}

+	public function testGenerateTokenInvalidName() {
+		$this->expectException(\OC\Authentication\Exceptions\InvalidTokenException::class);
+
+		$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'
+			. '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'
+			. '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;
+
+		$actual = $this->tokenProvider->generateToken($token, $uid, $user, $password, $name, $type, IToken::DO_NOT_REMEMBER);
+	}
+
 	public function testUpdateToken() {
 		$tk = new PublicKeyToken();
 		$this->mapper->expects($this->once())
@ -139,10 +152,7 @@ class PublicKeyTokenProviderTest extends TestCase {
 		$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'
-			. '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'
-			. 'User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.12) Gecko/20101026 Firefox/3.6.12';
+		$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;

 		$actual = $this->tokenProvider->generateToken($token, $uid, $user, $password, $name, $type, IToken::DO_NOT_REMEMBER);
@ -169,10 +179,7 @@ class PublicKeyTokenProviderTest extends TestCase {
 		$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'
-			. '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'
-			. 'User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.12) Gecko/20101026 Firefox/3.6.12';
+		$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;

 		$actual = $this->tokenProvider->generateToken($token, $uid, $user, $password, $name, $type, IToken::DO_NOT_REMEMBER);
@ -185,10 +192,7 @@ class PublicKeyTokenProviderTest extends TestCase {
 		$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'
-			. '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'
-			. 'User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.12) Gecko/20101026 Firefox/3.6.12';
+		$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;

 		$actual = $this->tokenProvider->generateToken($token, $uid, $user, $password, $name, $type, IToken::DO_NOT_REMEMBER);
@ -248,12 +252,12 @@ class PublicKeyTokenProviderTest extends TestCase {
 				['session_lifetime', $defaultSessionLifetime, 150],
 				['remember_login_cookie_lifetime', $defaultRememberMeLifetime, 300],
 			]);
-		$this->mapper->expects($this->at(0))
+		$this->mapper->expects($this->exactly(2))
 			->method('invalidateOld')
-			->with($this->time - 150);
-		$this->mapper->expects($this->at(1))
-			->method('invalidateOld')
-			->with($this->time - 300);
+			->withConsecutive(
+				[$this->time - 150],
+				[$this->time - 300]
+			);

 		$this->tokenProvider->invalidateOldTokens();
 	}
@ -263,21 +267,18 @@ class PublicKeyTokenProviderTest extends TestCase {
 		$uid = 'user';
 		$user = 'User';
 		$password = null;
-		$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'
-			. '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'
-			. 'User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.12) Gecko/20101026 Firefox/3.6.12';
+		$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;

 		$oldToken = $this->tokenProvider->generateToken($token, $uid, $user, $password, $name, $type, IToken::DO_NOT_REMEMBER);

 		$this->mapper
-			->expects($this->at(0))
+			->expects($this->once())
 			->method('getToken')
 			->with(hash('sha512', 'oldId' . '1f4h9s'))
 			->willReturn($oldToken);
 		$this->mapper
-			->expects($this->at(1))
+			->expects($this->once())
 			->method('insert')
 			->with($this->callback(function (PublicKeyToken $token) use ($user, $uid, $name) {
 				return $token->getUID() === $uid &&
@ -288,7 +289,7 @@ class PublicKeyTokenProviderTest extends TestCase {
 					$token->getPassword() === null;
 			}));
 		$this->mapper
-			->expects($this->at(2))
+			->expects($this->once())
 			->method('delete')
 			->with($this->callback(function ($token) use ($oldToken) {
 				return $token === $oldToken;
@ -302,21 +303,18 @@ class PublicKeyTokenProviderTest extends TestCase {
 		$uid = 'user';
 		$user = 'User';
 		$password = 'password';
-		$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'
-			. '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'
-			. 'User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.12) Gecko/20101026 Firefox/3.6.12';
+		$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;

 		$oldToken = $this->tokenProvider->generateToken($token, $uid, $user, $password, $name, $type, IToken::DO_NOT_REMEMBER);

 		$this->mapper
-			->expects($this->at(0))
+			->expects($this->once())
 			->method('getToken')
 			->with(hash('sha512', 'oldId' . '1f4h9s'))
 			->willReturn($oldToken);
 		$this->mapper
-			->expects($this->at(1))
+			->expects($this->once())
 			->method('insert')
 			->with($this->callback(function (PublicKeyToken $token) use ($user, $uid, $name) {
 				return $token->getUID() === $uid &&
@ -328,7 +326,7 @@ class PublicKeyTokenProviderTest extends TestCase {
 					$this->tokenProvider->getPassword($token, 'newId') === 'password';
 			}));
 		$this->mapper
-			->expects($this->at(2))
+			->expects($this->once())
 			->method('delete')
 			->with($this->callback(function ($token) use ($oldToken) {
 				return $token === $oldToken;
@ -372,10 +370,7 @@ class PublicKeyTokenProviderTest extends TestCase {
 		$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'
-			. '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'
-			. 'User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.12) Gecko/20101026 Firefox/3.6.12';
+		$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;

 		$actual = $this->tokenProvider->generateToken($token, $uid, $user, $password, $name, $type, IToken::DO_NOT_REMEMBER);
@ -440,10 +435,7 @@ class PublicKeyTokenProviderTest extends TestCase {
 		$uid = 'user';
 		$user = 'User';
 		$password = 'password';
-		$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'
-			. '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'
-			. 'User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.12) Gecko/20101026 Firefox/3.6.12';
+		$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;

 		$actual = $this->tokenProvider->generateToken($token, $uid, $user, $password, $name, $type, IToken::DO_NOT_REMEMBER);
@ -458,10 +450,7 @@ class PublicKeyTokenProviderTest extends TestCase {
 		$uid = 'user';
 		$user = 'User';
 		$password = null;
-		$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'
-			. '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'
-			. 'User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.12) Gecko/20101026 Firefox/3.6.12';
+		$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;

 		$actual = $this->tokenProvider->generateToken($token, $uid, $user, $password, $name, $type, IToken::DO_NOT_REMEMBER);
--- a/tests/lib/Authentication/TwoFactorAuth/ManagerTest.php
+++ b/tests/lib/Authentication/TwoFactorAuth/ManagerTest.php
@ -376,13 +376,13 @@ class ManagerTest extends TestCase {
 			->method('get')
 			->with('two_factor_remember_login')
 			->willReturn(false);
-		$this->session->expects($this->at(1))
+		$this->session->expects($this->exactly(2))
 			->method('remove')
-			->with('two_factor_auth_uid');
-		$this->session->expects($this->at(2))
-			->method('remove')
-			->with('two_factor_remember_login');
-		$this->session->expects($this->at(3))
+			->withConsecutive(
+				['two_factor_auth_uid'],
+				['two_factor_remember_login']
+			);
+		$this->session->expects($this->once())
 			->method('set')
 			->with(Manager::SESSION_UID_DONE, 'jos');
 		$this->session->method('getId')
@ -494,17 +494,13 @@ class ManagerTest extends TestCase {

 	public function testNeedsSecondFactor() {
 		$user = $this->createMock(IUser::class);
-		$this->session->expects($this->at(0))
+		$this->session->expects($this->exactly(3))
 			->method('exists')
-			->with('app_password')
-			->willReturn(false);
-		$this->session->expects($this->at(1))
-			->method('exists')
-			->with('two_factor_auth_uid')
-			->willReturn(false);
-		$this->session->expects($this->at(2))
-			->method('exists')
-			->with(Manager::SESSION_UID_DONE)
+			->withConsecutive(
+				['app_password'],
+				['two_factor_auth_uid'],
+				[Manager::SESSION_UID_DONE],
+			)
 			->willReturn(false);

 		$this->session->method('getId')
@ -575,12 +571,12 @@ class ManagerTest extends TestCase {
 		$this->user->method('getUID')
 			->willReturn('ferdinand');

-		$this->session->expects($this->at(0))
+		$this->session->expects($this->exactly(2))
 			->method('set')
-			->with('two_factor_auth_uid', 'ferdinand');
-		$this->session->expects($this->at(1))
-			->method('set')
-			->with('two_factor_remember_login', true);
+			->withConsecutive(
+				['two_factor_auth_uid', 'ferdinand'],
+				['two_factor_remember_login', true]
+			);

 		$this->session->method('getId')
 			->willReturn('mysessionid');
@ -605,12 +601,12 @@ class ManagerTest extends TestCase {
 		$this->user->method('getUID')
 			->willReturn('ferdinand');

-		$this->session->expects($this->at(0))
+		$this->session->expects($this->exactly(2))
 			->method('set')
-			->with('two_factor_auth_uid', 'ferdinand');
-		$this->session->expects($this->at(1))
-			->method('set')
-			->with('two_factor_remember_login', false);
+			->withConsecutive(
+				['two_factor_auth_uid', 'ferdinand'],
+				['two_factor_remember_login', false]
+			);

 		$this->session->method('getId')
 			->willReturn('mysessionid');