Merge pull request #20127 from nextcloud/bugfix/noid/check-user-on-remote-wipe

Check the user on remote wipe

Author: rullzer

apps/settings/lib/Controller/AuthSettingsController.php

@@ -289,7 +289,13 @@ private function findTokenByIdAndUser(int $id): IToken {
 	 * @throws \OC\Authentication\Exceptions\ExpiredTokenException
 	 */
 	public function wipe(int $id): JSONResponse {
-		if (!$this->remoteWipe->markTokenForWipe($id)) {
+		try {
+			$token = $this->findTokenByIdAndUser($id);
+		} catch (InvalidTokenException $e) {
+			return new JSONResponse([], Http::STATUS_NOT_FOUND);
+		}
+
+		if (!$this->remoteWipe->markTokenForWipe($token)) {
 			return new JSONResponse([], Http::STATUS_BAD_REQUEST);
 		}
 

apps/settings/tests/Controller/AuthSettingsControllerTest.php

@@ -36,6 +36,7 @@
 use OC\Authentication\Token\DefaultToken;
 use OC\Authentication\Token\IProvider;
 use OC\Authentication\Token\IToken;
+use OC\Authentication\Token\IWipeableToken;
 use OC\Authentication\Token\RemoteWipe;
 use OCA\Settings\Controller\AuthSettingsController;
 use OCP\Activity\IEvent;
@@ -428,9 +429,15 @@ private function mockGetTokenById(int $tokenId, $token): void {
 	}
 
 	public function testRemoteWipeNotSuccessful(): void {
+		$token = $this->createMock(IToken::class);
+		$token->expects($this->once())
+			->method('getUID')
+			->willReturn($this->uid);
+		$this->mockGetTokenById(123, $token);
+
 		$this->remoteWipe->expects($this->once())
 			->method('markTokenForWipe')
-			->with(123)
+			->with($token)
 			->willReturn(false);
 
 		$response = $this->controller->wipe(123);
@@ -439,10 +446,32 @@ public function testRemoteWipeNotSuccessful(): void {
 		$this->assertEquals($expected, $response);
 	}
 
+	public function testRemoteWipeWrongUser(): void {
+		$token = $this->createMock(IToken::class);
+		$token->expects($this->once())
+			->method('getUID')
+			->willReturn('definetly-not-' . $this->uid);
+		$this->mockGetTokenById(123, $token);
+
+		$this->remoteWipe->expects($this->never())
+			->method('markTokenForWipe');
+
+		$response = $this->controller->wipe(123);
+
+		$expected = new JSONResponse([], Http::STATUS_NOT_FOUND);
+		$this->assertEquals($expected, $response);
+	}
+
 	public function testRemoteWipeSuccessful(): void {
+		$token = $this->createMock(IWipeableToken::class);
+		$token->expects($this->once())
+			->method('getUID')
+			->willReturn($this->uid);
+		$this->mockGetTokenById(123, $token);
+
 		$this->remoteWipe->expects($this->once())
 			->method('markTokenForWipe')
-			->with(123)
+			->with($token)
 			->willReturn(true);
 
 		$response = $this->controller->wipe(123);

lib/private/Authentication/Token/RemoteWipe.php

@@ -57,18 +57,14 @@ public function __construct(IProvider $tokenProvider,
 	}
 
 	/**
-	 * @param int $id
-	 *
+	 * @param IToken $token
 	 * @return bool
 	 *
 	 * @throws InvalidTokenException
 	 * @throws WipeTokenException
-	 * @throws ExpiredTokenException
 	 */
-	public function markTokenForWipe(int $id): bool {
-		$token = $this->tokenProvider->getTokenById($id);
-
-		if (!($token instanceof IWipeableToken)) {
+	public function markTokenForWipe(IToken $token): bool {
+		if (!$token instanceof IWipeableToken) {
 			return false;
 		}
 

tests/lib/Authentication/Token/RemoteWipeTest.php

@@ -67,30 +67,20 @@ protected function setUp(): void {
 
 	public function testMarkNonWipableTokenForWipe(): void {
 		$token = $this->createMock(IToken::class);
-		$this->tokenProvider->expects($this->once())
-			->method('getTokenById')
-			->with(123)
-			->willReturn($token);
-
-		$result = $this->remoteWipe->markTokenForWipe(123);
-
+		$result = $this->remoteWipe->markTokenForWipe($token);
 		$this->assertFalse($result);
 	}
 
 	public function testMarkTokenForWipe(): void {
 		$token = $this->createMock(IWipeableToken::class);
-		$this->tokenProvider->expects($this->once())
-			->method('getTokenById')
-			->with(123)
-			->willReturn($token);
 		$token->expects($this->once())
 			->method('wipe');
+
 		$this->tokenProvider->expects($this->once())
 			->method('updateToken')
 			->with($token);
 
-		$result = $this->remoteWipe->markTokenForWipe(123);
-
+		$result = $this->remoteWipe->markTokenForWipe($token);
 		$this->assertTrue($result);
 	}