From 1c261675ad3da9804bd9a8c88326103eb2f56bd3 Mon Sep 17 00:00:00 2001 From: Christoph Wurst Date: Wed, 3 Jul 2019 09:44:37 +0200 Subject: [PATCH] Refactor: move remote wipe token logic to RW service Signed-off-by: Christoph Wurst --- .../Authentication/Token/IWipeableToken.php | 7 ++- .../Authentication/Token/RemoteWipe.php | 14 ++++++ .../Controller/AuthSettingsController.php | 14 +++--- .../Controller/AuthSettingsControllerTest.php | 45 ++++++++++++++++--- .../Authentication/Token/RemoteWipeTest.php | 30 +++++++++++++ 5 files changed, 97 insertions(+), 13 deletions(-) diff --git a/lib/private/Authentication/Token/IWipeableToken.php b/lib/private/Authentication/Token/IWipeableToken.php index 8d4d3a6078..f0777bf4a2 100644 --- a/lib/private/Authentication/Token/IWipeableToken.php +++ b/lib/private/Authentication/Token/IWipeableToken.php @@ -24,6 +24,11 @@ declare(strict_types=1); namespace OC\Authentication\Token; -interface IWipeableToken { +interface IWipeableToken extends IToken { + + /** + * Mark the token for remote wipe + */ public function wipe(): void; + } diff --git a/lib/private/Authentication/Token/RemoteWipe.php b/lib/private/Authentication/Token/RemoteWipe.php index 5534ff1cba..38f1f439e8 100644 --- a/lib/private/Authentication/Token/RemoteWipe.php +++ b/lib/private/Authentication/Token/RemoteWipe.php @@ -35,6 +35,7 @@ use OCP\Activity\IManager as IActivityManager; use OCP\AppFramework\Utility\ITimeFactory; use OCP\EventDispatcher\IEventDispatcher; use OCP\ILogger; +use OCP\IUser; use OCP\Notification\IManager as INotificationManager; use Symfony\Component\EventDispatcher\EventDispatcher; @@ -57,6 +58,19 @@ class RemoteWipe { $this->logger = $logger; } + public function markTokenForWipe(int $id): bool { + $token = $this->tokenProvider->getTokenById($id); + + if (!($token instanceof IWipeableToken)) { + return false; + } + + $token->wipe(); + $this->tokenProvider->updateToken($token); + + return true; + } + /** * @param string $token * diff --git a/settings/Controller/AuthSettingsController.php b/settings/Controller/AuthSettingsController.php index be497315ed..da9414dcb1 100644 --- a/settings/Controller/AuthSettingsController.php +++ b/settings/Controller/AuthSettingsController.php @@ -35,6 +35,7 @@ use OC\Authentication\Token\INamedToken; use OC\Authentication\Token\IProvider; use OC\Authentication\Token\IToken; use OC\Authentication\Token\IWipeableToken; +use OC\Authentication\Token\RemoteWipe; use OC\Settings\Activity\Provider; use OCP\Activity\IManager; use OCP\AppFramework\Controller; @@ -63,6 +64,9 @@ class AuthSettingsController extends Controller { /** @var IManager */ private $activityManager; + /** @var RemoteWipe */ + private $remoteWipe; + /** @var ILogger */ private $logger; @@ -74,6 +78,7 @@ class AuthSettingsController extends Controller { * @param ISecureRandom $random * @param string|null $userId * @param IManager $activityManager + * @param RemoteWipe $remoteWipe * @param ILogger $logger */ public function __construct(string $appName, @@ -83,6 +88,7 @@ class AuthSettingsController extends Controller { ISecureRandom $random, ?string $userId, IManager $activityManager, + RemoteWipe $remoteWipe, ILogger $logger) { parent::__construct($appName, $request); $this->tokenProvider = $tokenProvider; @@ -90,6 +96,7 @@ class AuthSettingsController extends Controller { $this->session = $session; $this->random = $random; $this->activityManager = $activityManager; + $this->remoteWipe = $remoteWipe; $this->logger = $logger; } @@ -262,15 +269,10 @@ class AuthSettingsController extends Controller { * @throws \OC\Authentication\Exceptions\ExpiredTokenException */ public function wipe(int $id): JSONResponse { - $token = $this->tokenProvider->getTokenById($id); - - if (!($token instanceof IWipeableToken)) { + if (!$this->remoteWipe->markTokenForWipe($id)) { return new JSONResponse([], Http::STATUS_BAD_REQUEST); } - $token->wipe(); - $this->tokenProvider->updateToken($token); - return new JSONResponse([]); } } diff --git a/tests/Settings/Controller/AuthSettingsControllerTest.php b/tests/Settings/Controller/AuthSettingsControllerTest.php index 198b3a72c3..d335abc98a 100644 --- a/tests/Settings/Controller/AuthSettingsControllerTest.php +++ b/tests/Settings/Controller/AuthSettingsControllerTest.php @@ -26,6 +26,7 @@ use OC\Authentication\Exceptions\InvalidTokenException; use OC\Authentication\Token\DefaultToken; use OC\Authentication\Token\IProvider; use OC\Authentication\Token\IToken; +use OC\Authentication\Token\RemoteWipe; use OC\Settings\Controller\AuthSettingsController; use OCP\Activity\IEvent; use OCP\Activity\IManager; @@ -35,22 +36,25 @@ use OCP\IRequest; use OCP\ISession; use OCP\Security\ISecureRandom; use OCP\Session\Exceptions\SessionNotAvailableException; +use PHPUnit\Framework\MockObject\MockObject; use Test\TestCase; class AuthSettingsControllerTest extends TestCase { /** @var AuthSettingsController */ private $controller; - /** @var IRequest|\PHPUnit_Framework_MockObject_MockObject */ + /** @var IRequest|MockObject */ private $request; - /** @var IProvider|\PHPUnit_Framework_MockObject_MockObject */ + /** @var IProvider|MockObject */ private $tokenProvider; - /** @var ISession|\PHPUnit_Framework_MockObject_MockObject */ + /** @var ISession|MockObject */ private $session; - /** @var ISecureRandom|\PHPUnit_Framework_MockObject_MockObject */ + /** @var ISecureRandom|MockObject */ private $secureRandom; - /** @var IManager|\PHPUnit_Framework_MockObject_MockObject */ + /** @var IManager|MockObject */ private $activityManager; + /** @var RemoteWipe|MockObject */ + private $remoteWipe; private $uid = 'jane'; protected function setUp() { @@ -61,7 +65,8 @@ class AuthSettingsControllerTest extends TestCase { $this->session = $this->createMock(ISession::class); $this->secureRandom = $this->createMock(ISecureRandom::class); $this->activityManager = $this->createMock(IManager::class); - /** @var ILogger|\PHPUnit_Framework_MockObject_MockObject $logger */ + $this->remoteWipe = $this->createMock(RemoteWipe::class); + /** @var ILogger|MockObject $logger */ $logger = $this->createMock(ILogger::class); $this->controller = new AuthSettingsController( @@ -72,6 +77,7 @@ class AuthSettingsControllerTest extends TestCase { $this->secureRandom, $this->uid, $this->activityManager, + $this->remoteWipe, $logger ); } @@ -201,6 +207,7 @@ class AuthSettingsControllerTest extends TestCase { /** * @dataProvider dataRenameToken + * * @param string $name * @param string $newName */ @@ -243,6 +250,7 @@ class AuthSettingsControllerTest extends TestCase { /** * @dataProvider dataUpdateFilesystemScope + * * @param bool $filesystem * @param bool $newFilesystem */ @@ -359,4 +367,29 @@ class AuthSettingsControllerTest extends TestCase { ->with($this->equalTo($tokenId)) ->willReturn($token); } + + public function testRemoteWipeNotSuccessful(): void { + $this->remoteWipe->expects($this->once()) + ->method('markTokenForWipe') + ->with(123) + ->willReturn(false); + + $response = $this->controller->wipe(123); + + $expected = new JSONResponse([], Http::STATUS_BAD_REQUEST); + $this->assertEquals($expected, $response); + } + + public function testRemoteWipeSuccessful(): void { + $this->remoteWipe->expects($this->once()) + ->method('markTokenForWipe') + ->with(123) + ->willReturn(true); + + $response = $this->controller->wipe(123); + + $expected = new JSONResponse([]); + $this->assertEquals($expected, $response); + } + } diff --git a/tests/lib/Authentication/Token/RemoteWipeTest.php b/tests/lib/Authentication/Token/RemoteWipeTest.php index e0b3e9fcae..d5d63b2fb4 100644 --- a/tests/lib/Authentication/Token/RemoteWipeTest.php +++ b/tests/lib/Authentication/Token/RemoteWipeTest.php @@ -29,6 +29,7 @@ use OC\Authentication\Exceptions\WipeTokenException; use OC\Authentication\Token\IProvider as ITokenProvider; use OC\Authentication\Token\IProvider; use OC\Authentication\Token\IToken; +use OC\Authentication\Token\IWipeableToken; use OC\Authentication\Token\RemoteWipe; use OCP\EventDispatcher\IEventDispatcher; use OCP\ILogger; @@ -63,6 +64,35 @@ class RemoteWipeTest extends TestCase { ); } + 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); + + $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); + + $this->assertTrue($result); + } + public function testStartWipingNotAWipeToken() { $token = $this->createMock(IToken::class); $this->tokenProvider->expects($this->once())