Skip to content

Commit c1215f5

Browse files
Merge pull request #31658 from nextcloud/bugfix/noid/limit-token-names
Limit the length of app password names
2 parents 0fa17f8 + 343476f commit c1215f5

File tree

7 files changed

+81
-73
lines changed

7 files changed

+81
-73
lines changed

apps/settings/lib/Controller/AuthSettingsController.php

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,10 @@ public function create($name) {
145145
return $this->getServiceNotAvailableResponse();
146146
}
147147

148+
if (mb_strlen($name) > 128) {
149+
$name = mb_substr($name, 0, 120) . '';
150+
}
151+
148152
$token = $this->generateRandomDeviceToken();
149153
$deviceToken = $this->tokenProvider->generateToken($token, $this->uid, $loginName, $password, $name, IToken::PERMANENT_TOKEN);
150154
$tokenData = $deviceToken->jsonSerialize();
@@ -241,6 +245,10 @@ public function update($id, array $scope, string $name) {
241245
$this->publishActivity($scope['filesystem'] ? Provider::APP_TOKEN_FILESYSTEM_GRANTED : Provider::APP_TOKEN_FILESYSTEM_REVOKED, $token->getId(), ['name' => $currentName]);
242246
}
243247

248+
if (mb_strlen($name) > 128) {
249+
$name = mb_substr($name, 0, 120) . '';
250+
}
251+
244252
if ($token instanceof INamedToken && $name !== $currentName) {
245253
$token->setName($name);
246254
$this->publishActivity(Provider::APP_TOKEN_RENAMED, $token->getId(), ['name' => $currentName, 'newName' => $name]);

core/Controller/AppPasswordController.php

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,9 @@ public function getAppPassword(): DataResponse {
9999
}
100100

101101
$userAgent = $this->request->getHeader('USER_AGENT');
102+
if (mb_strlen($userAgent) > 128) {
103+
$userAgent = mb_substr($userAgent, 0, 120) . '';
104+
}
102105

103106
$token = $this->random->generate(72, ISecureRandom::CHAR_UPPER.ISecureRandom::CHAR_LOWER.ISecureRandom::CHAR_DIGITS);
104107

core/Controller/ClientFlowLoginController.php

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -322,6 +322,10 @@ public function generateAppPassword($stateToken,
322322
$clientName = $client->getName();
323323
}
324324

325+
if (mb_strlen($clientName) > 128) {
326+
$clientName = mb_substr($clientName, 0, 120) . '';
327+
}
328+
325329
$token = $this->random->generate(72, ISecureRandom::CHAR_UPPER.ISecureRandom::CHAR_LOWER.ISecureRandom::CHAR_DIGITS);
326330
$uid = $this->userSession->getUser()->getUID();
327331
$generatedToken = $this->tokenProvider->generateToken(

lib/private/Authentication/Token/Manager.php

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,10 @@ public function generateToken(string $token,
6161
string $name,
6262
int $type = IToken::TEMPORARY_TOKEN,
6363
int $remember = IToken::DO_NOT_REMEMBER): IToken {
64+
if (mb_strlen($name) > 128) {
65+
throw new InvalidTokenException('The given name is too long');
66+
}
67+
6468
try {
6569
return $this->publicKeyTokenProvider->generateToken(
6670
$token,

lib/private/Authentication/Token/PublicKeyTokenProvider.php

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,10 @@ public function generateToken(string $token,
8484
string $name,
8585
int $type = IToken::TEMPORARY_TOKEN,
8686
int $remember = IToken::DO_NOT_REMEMBER): IToken {
87+
if (mb_strlen($name) > 128) {
88+
throw new InvalidTokenException('The given name is too long');
89+
}
90+
8791
$dbToken = $this->newToken($token, $uid, $loginName, $password, $name, $type, $remember);
8892
$this->mapper->insert($dbToken);
8993

tests/lib/Authentication/Token/PublicKeyTokenProviderTest.php

Lines changed: 36 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -80,10 +80,7 @@ public function testGenerateToken() {
8080
$uid = 'user';
8181
$user = 'User';
8282
$password = 'passme';
83-
$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'
84-
. 'User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.12) Gecko/20101026 Firefox/3.6.12'
85-
. 'User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.12) Gecko/20101026 Firefox/3.6.12'
86-
. 'User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.12) Gecko/20101026 Firefox/3.6.12';
83+
$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';
8784
$type = IToken::PERMANENT_TOKEN;
8885

8986
$actual = $this->tokenProvider->generateToken($token, $uid, $user, $password, $name, $type, IToken::DO_NOT_REMEMBER);
@@ -96,6 +93,22 @@ public function testGenerateToken() {
9693
$this->assertSame($password, $this->tokenProvider->getPassword($actual, $token));
9794
}
9895

96+
public function testGenerateTokenInvalidName() {
97+
$this->expectException(\OC\Authentication\Exceptions\InvalidTokenException::class);
98+
99+
$token = 'token';
100+
$uid = 'user';
101+
$user = 'User';
102+
$password = 'passme';
103+
$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'
104+
. 'User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.12) Gecko/20101026 Firefox/3.6.12'
105+
. 'User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.12) Gecko/20101026 Firefox/3.6.12'
106+
. 'User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.12) Gecko/20101026 Firefox/3.6.12';
107+
$type = IToken::PERMANENT_TOKEN;
108+
109+
$actual = $this->tokenProvider->generateToken($token, $uid, $user, $password, $name, $type, IToken::DO_NOT_REMEMBER);
110+
}
111+
99112
public function testUpdateToken() {
100113
$tk = new PublicKeyToken();
101114
$this->mapper->expects($this->once())
@@ -137,10 +150,7 @@ public function testGetPassword() {
137150
$uid = 'user';
138151
$user = 'User';
139152
$password = 'passme';
140-
$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'
141-
. 'User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.12) Gecko/20101026 Firefox/3.6.12'
142-
. 'User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.12) Gecko/20101026 Firefox/3.6.12'
143-
. 'User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.12) Gecko/20101026 Firefox/3.6.12';
153+
$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';
144154
$type = IToken::PERMANENT_TOKEN;
145155

146156
$actual = $this->tokenProvider->generateToken($token, $uid, $user, $password, $name, $type, IToken::DO_NOT_REMEMBER);
@@ -167,10 +177,7 @@ public function testGetPasswordInvalidToken() {
167177
$uid = 'user';
168178
$user = 'User';
169179
$password = 'passme';
170-
$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'
171-
. 'User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.12) Gecko/20101026 Firefox/3.6.12'
172-
. 'User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.12) Gecko/20101026 Firefox/3.6.12'
173-
. 'User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.12) Gecko/20101026 Firefox/3.6.12';
180+
$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';
174181
$type = IToken::PERMANENT_TOKEN;
175182

176183
$actual = $this->tokenProvider->generateToken($token, $uid, $user, $password, $name, $type, IToken::DO_NOT_REMEMBER);
@@ -183,10 +190,7 @@ public function testSetPassword() {
183190
$uid = 'user';
184191
$user = 'User';
185192
$password = 'passme';
186-
$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'
187-
. 'User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.12) Gecko/20101026 Firefox/3.6.12'
188-
. 'User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.12) Gecko/20101026 Firefox/3.6.12'
189-
. 'User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.12) Gecko/20101026 Firefox/3.6.12';
193+
$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';
190194
$type = IToken::PERMANENT_TOKEN;
191195

192196
$actual = $this->tokenProvider->generateToken($token, $uid, $user, $password, $name, $type, IToken::DO_NOT_REMEMBER);
@@ -246,12 +250,12 @@ public function testInvalidateOldTokens() {
246250
['session_lifetime', $defaultSessionLifetime, 150],
247251
['remember_login_cookie_lifetime', $defaultRememberMeLifetime, 300],
248252
]);
249-
$this->mapper->expects($this->at(0))
250-
->method('invalidateOld')
251-
->with($this->time - 150);
252-
$this->mapper->expects($this->at(1))
253+
$this->mapper->expects($this->exactly(2))
253254
->method('invalidateOld')
254-
->with($this->time - 300);
255+
->withConsecutive(
256+
[$this->time - 150],
257+
[$this->time - 300]
258+
);
255259

256260
$this->tokenProvider->invalidateOldTokens();
257261
}
@@ -261,21 +265,18 @@ public function testRenewSessionTokenWithoutPassword() {
261265
$uid = 'user';
262266
$user = 'User';
263267
$password = null;
264-
$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'
265-
. 'User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.12) Gecko/20101026 Firefox/3.6.12'
266-
. 'User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.12) Gecko/20101026 Firefox/3.6.12'
267-
. 'User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.12) Gecko/20101026 Firefox/3.6.12';
268+
$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';
268269
$type = IToken::PERMANENT_TOKEN;
269270

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

272273
$this->mapper
273-
->expects($this->at(0))
274+
->expects($this->once())
274275
->method('getToken')
275276
->with(hash('sha512', 'oldId' . '1f4h9s'))
276277
->willReturn($oldToken);
277278
$this->mapper
278-
->expects($this->at(1))
279+
->expects($this->once())
279280
->method('insert')
280281
->with($this->callback(function (PublicKeyToken $token) use ($user, $uid, $name) {
281282
return $token->getUID() === $uid &&
@@ -286,7 +287,7 @@ public function testRenewSessionTokenWithoutPassword() {
286287
$token->getPassword() === null;
287288
}));
288289
$this->mapper
289-
->expects($this->at(2))
290+
->expects($this->once())
290291
->method('delete')
291292
->with($this->callback(function ($token) use ($oldToken) {
292293
return $token === $oldToken;
@@ -300,21 +301,18 @@ public function testRenewSessionTokenWithPassword() {
300301
$uid = 'user';
301302
$user = 'User';
302303
$password = 'password';
303-
$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'
304-
. 'User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.12) Gecko/20101026 Firefox/3.6.12'
305-
. 'User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.12) Gecko/20101026 Firefox/3.6.12'
306-
. 'User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.12) Gecko/20101026 Firefox/3.6.12';
304+
$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';
307305
$type = IToken::PERMANENT_TOKEN;
308306

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

311309
$this->mapper
312-
->expects($this->at(0))
310+
->expects($this->once())
313311
->method('getToken')
314312
->with(hash('sha512', 'oldId' . '1f4h9s'))
315313
->willReturn($oldToken);
316314
$this->mapper
317-
->expects($this->at(1))
315+
->expects($this->once())
318316
->method('insert')
319317
->with($this->callback(function (PublicKeyToken $token) use ($user, $uid, $name) {
320318
return $token->getUID() === $uid &&
@@ -326,7 +324,7 @@ public function testRenewSessionTokenWithPassword() {
326324
$this->tokenProvider->getPassword($token, 'newId') === 'password';
327325
}));
328326
$this->mapper
329-
->expects($this->at(2))
327+
->expects($this->once())
330328
->method('delete')
331329
->with($this->callback(function ($token) use ($oldToken) {
332330
return $token === $oldToken;
@@ -370,10 +368,7 @@ public function testGetExpiredToken() {
370368
$uid = 'user';
371369
$user = 'User';
372370
$password = 'passme';
373-
$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'
374-
. 'User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.12) Gecko/20101026 Firefox/3.6.12'
375-
. 'User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.12) Gecko/20101026 Firefox/3.6.12'
376-
. 'User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.12) Gecko/20101026 Firefox/3.6.12';
371+
$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';
377372
$type = IToken::PERMANENT_TOKEN;
378373

379374
$actual = $this->tokenProvider->generateToken($token, $uid, $user, $password, $name, $type, IToken::DO_NOT_REMEMBER);
@@ -438,10 +433,7 @@ public function testRotate() {
438433
$uid = 'user';
439434
$user = 'User';
440435
$password = 'password';
441-
$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'
442-
. 'User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.12) Gecko/20101026 Firefox/3.6.12'
443-
. 'User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.12) Gecko/20101026 Firefox/3.6.12'
444-
. 'User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.12) Gecko/20101026 Firefox/3.6.12';
436+
$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';
445437
$type = IToken::PERMANENT_TOKEN;
446438

447439
$actual = $this->tokenProvider->generateToken($token, $uid, $user, $password, $name, $type, IToken::DO_NOT_REMEMBER);
@@ -456,10 +448,7 @@ public function testRotateNoPassword() {
456448
$uid = 'user';
457449
$user = 'User';
458450
$password = null;
459-
$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'
460-
. 'User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.12) Gecko/20101026 Firefox/3.6.12'
461-
. 'User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.12) Gecko/20101026 Firefox/3.6.12'
462-
. 'User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.12) Gecko/20101026 Firefox/3.6.12';
451+
$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';
463452
$type = IToken::PERMANENT_TOKEN;
464453

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

tests/lib/Authentication/TwoFactorAuth/ManagerTest.php

Lines changed: 22 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -376,13 +376,13 @@ public function testVerifyChallenge() {
376376
->method('get')
377377
->with('two_factor_remember_login')
378378
->willReturn(false);
379-
$this->session->expects($this->at(1))
379+
$this->session->expects($this->exactly(2))
380380
->method('remove')
381-
->with('two_factor_auth_uid');
382-
$this->session->expects($this->at(2))
383-
->method('remove')
384-
->with('two_factor_remember_login');
385-
$this->session->expects($this->at(3))
381+
->withConsecutive(
382+
['two_factor_auth_uid'],
383+
['two_factor_remember_login']
384+
);
385+
$this->session->expects($this->once())
386386
->method('set')
387387
->with(Manager::SESSION_UID_DONE, 'jos');
388388
$this->session->method('getId')
@@ -494,17 +494,13 @@ public function testVerifyInvalidChallenge() {
494494

495495
public function testNeedsSecondFactor() {
496496
$user = $this->createMock(IUser::class);
497-
$this->session->expects($this->at(0))
498-
->method('exists')
499-
->with('app_password')
500-
->willReturn(false);
501-
$this->session->expects($this->at(1))
497+
$this->session->expects($this->exactly(3))
502498
->method('exists')
503-
->with('two_factor_auth_uid')
504-
->willReturn(false);
505-
$this->session->expects($this->at(2))
506-
->method('exists')
507-
->with(Manager::SESSION_UID_DONE)
499+
->withConsecutive(
500+
['app_password'],
501+
['two_factor_auth_uid'],
502+
[Manager::SESSION_UID_DONE],
503+
)
508504
->willReturn(false);
509505

510506
$this->session->method('getId')
@@ -575,12 +571,12 @@ public function testPrepareTwoFactorLogin() {
575571
$this->user->method('getUID')
576572
->willReturn('ferdinand');
577573

578-
$this->session->expects($this->at(0))
579-
->method('set')
580-
->with('two_factor_auth_uid', 'ferdinand');
581-
$this->session->expects($this->at(1))
574+
$this->session->expects($this->exactly(2))
582575
->method('set')
583-
->with('two_factor_remember_login', true);
576+
->withConsecutive(
577+
['two_factor_auth_uid', 'ferdinand'],
578+
['two_factor_remember_login', true]
579+
);
584580

585581
$this->session->method('getId')
586582
->willReturn('mysessionid');
@@ -605,12 +601,12 @@ public function testPrepareTwoFactorLoginDontRemember() {
605601
$this->user->method('getUID')
606602
->willReturn('ferdinand');
607603

608-
$this->session->expects($this->at(0))
609-
->method('set')
610-
->with('two_factor_auth_uid', 'ferdinand');
611-
$this->session->expects($this->at(1))
604+
$this->session->expects($this->exactly(2))
612605
->method('set')
613-
->with('two_factor_remember_login', false);
606+
->withConsecutive(
607+
['two_factor_auth_uid', 'ferdinand'],
608+
['two_factor_remember_login', false]
609+
);
614610

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

0 commit comments

Comments
 (0)