Fix updating and deleting authtokens

[kinolaev]

Problem: updating and deleting expired authtokens don't works because PublicKeyTokenProvider->getTokenById throws ExpiredTokenException and client receives response 404.
Solution: we need to catch ExpiredTokenException and get token from this exception (as already implemented in OauthApiController)

[rullzer]

Nice catch

[rullzer]

/backport to stable17

[rullzer]

/backport to stable16

[kesselb]

Nice catch! I would prefer that update and destroy catch the exception. findTokenByIdAndUser should act like getTokenById with the additon that the token ownership is validated. However it's a internal method so not super important.

[kesselb]

Index: apps/settings/tests/Controller/AuthSettingsControllerTest.php
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- apps/settings/tests/Controller/AuthSettingsControllerTest.php	(revision 9bf55c33e15e6c2a576604e35e86b344fa6f534f)
+++ apps/settings/tests/Controller/AuthSettingsControllerTest.php	(date 1570182212152)
@@ -22,6 +22,7 @@
namespace Test\Settings\Controller;
use OC\AppFramework\Http;
+use OC\Authentication\Exceptions\ExpiredTokenException;
use OC\Authentication\Exceptions\InvalidTokenException;
use OC\Authentication\Token\DefaultToken;
use OC\Authentication\Token\IProvider;
@@ -188,6 +189,28 @@
$this->assertEquals([], $this->controller->destroy($tokenId));
}
+	public function testDestroyExpired() {
+ $tokenId = 124;
+
+ /** @var DefaultToken $token */
+ $token = $this->createMock(DefaultToken::class);
+
+ $token->expects($this->exactly(2))
+ ->method('getId')
+ ->willReturn($tokenId);
+
+ $this->tokenProvider->expects($this->once())
+ ->method('getTokenById')
+ ->with($this->equalTo($tokenId))
+ ->willThrowException(new ExpiredTokenException($token));
+
+ $this->tokenProvider->expects($this->once())
+ ->method('invalidateTokenById')
+ ->with($this->uid, $tokenId);
+
+ $this->assertSame([], $this->controller->destroy($tokenId));
+	}
+
public function testDestroyWrongUser() {
$tokenId = 124;
$token = $this->createMock(DefaultToken::class);
@@ -354,6 +377,27 @@
$this->assertSame(\OCP\AppFramework\Http::STATUS_NOT_FOUND, $response->getStatus());
}
+	public function testUpdateTokenExpired() {
+ $tokenId = 42;
+
+ /** @var DefaultToken $token */
+ $token = $this->createMock(DefaultToken::class);
+
+ $token->expects($this->exactly(2))
+ ->method('getId')
+ ->willReturn($tokenId);
+
+ $this->tokenProvider->expects($this->once())
+ ->method('getTokenById')
+ ->with($this->equalTo($tokenId))
+ ->willThrowException(new ExpiredTokenException($token));
+
+ $this->tokenProvider->expects($this->once())
+ ->method('updateToken');
+
+ $this->assertSame([], $this->controller->update($tokenId, ['filesystem' => true], 'App password'));
+	}
+
private function mockActivityManager(): void {
$this->activityManager->expects($this->once())
->method('generateEvent')

Added some tests for this pr. Feel free to push them.

[kinolaev]

Hello @kesselb, I added tests, thank you!

[welcome]

Thanks for your first pull request and welcome to the community! Feel free to keep them coming! If you are looking for issues to tackle then have a look at this selection: https://github.com/nextcloud/server/issues?q=is%3Aopen+is%3Aissue+label%3A%22good+first+issue%22
Most developers hang out on IRC. So join #nextcloud-dev on Freenode for a chat!

[backportbot-nextcloud]

backport to stable17 in #17415

[skjnldsv]

/backport to stable16

[backportbot-nextcloud]

backport to stable16 in #17416