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