From c6ae53096c36e6a475467eaeb6df00ac8d38e4b2 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 1 Dec 2021 22:17:19 +0100 Subject: [PATCH] More test fixing Signed-off-by: Joas Schilling --- .../lib/Controller/SettingsController.php | 14 -- .../Controller/SettingsControllerTest.php | 9 - .../Controller/AuthSettingsControllerTest.php | 18 +- .../Personal/Security/AuthtokensTest.php | 8 +- .../lib/Authentication/Token/ManagerTest.php | 208 +----------------- .../Token/PublicKeyTokenProviderTest.php | 33 +-- 6 files changed, 19 insertions(+), 271 deletions(-) diff --git a/apps/oauth2/lib/Controller/SettingsController.php b/apps/oauth2/lib/Controller/SettingsController.php index 28500128629..046e6d77041 100644 --- a/apps/oauth2/lib/Controller/SettingsController.php +++ b/apps/oauth2/lib/Controller/SettingsController.php @@ -30,7 +30,6 @@ declare(strict_types=1); */ namespace OCA\OAuth2\Controller; -use OC\Authentication\Token\DefaultTokenMapper; use OCA\OAuth2\Db\AccessTokenMapper; use OCA\OAuth2\Db\Client; use OCA\OAuth2\Db\ClientMapper; @@ -48,34 +47,22 @@ class SettingsController extends Controller { private $secureRandom; /** @var AccessTokenMapper */ private $accessTokenMapper; - /** @var DefaultTokenMapper */ - private $defaultTokenMapper; /** @var IL10N */ private $l; public const validChars = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789'; - /** - * @param string $appName - * @param IRequest $request - * @param ClientMapper $clientMapper - * @param ISecureRandom $secureRandom - * @param AccessTokenMapper $accessTokenMapper - * @param DefaultTokenMapper $defaultTokenMapper - */ public function __construct(string $appName, IRequest $request, ClientMapper $clientMapper, ISecureRandom $secureRandom, AccessTokenMapper $accessTokenMapper, - DefaultTokenMapper $defaultTokenMapper, IL10N $l ) { parent::__construct($appName, $request); $this->secureRandom = $secureRandom; $this->clientMapper = $clientMapper; $this->accessTokenMapper = $accessTokenMapper; - $this->defaultTokenMapper = $defaultTokenMapper; $this->l = $l; } @@ -106,7 +93,6 @@ class SettingsController extends Controller { public function deleteClient(int $id): JSONResponse { $client = $this->clientMapper->getByUid($id); $this->accessTokenMapper->deleteByClientId($id); - $this->defaultTokenMapper->deleteByName($client->getName()); $this->clientMapper->delete($client); return new JSONResponse([]); } diff --git a/apps/oauth2/tests/Controller/SettingsControllerTest.php b/apps/oauth2/tests/Controller/SettingsControllerTest.php index 40785e280a0..4954d379f9d 100644 --- a/apps/oauth2/tests/Controller/SettingsControllerTest.php +++ b/apps/oauth2/tests/Controller/SettingsControllerTest.php @@ -26,7 +26,6 @@ */ namespace OCA\OAuth2\Tests\Controller; -use OC\Authentication\Token\DefaultTokenMapper; use OCA\OAuth2\Controller\SettingsController; use OCA\OAuth2\Db\AccessTokenMapper; use OCA\OAuth2\Db\Client; @@ -47,8 +46,6 @@ class SettingsControllerTest extends TestCase { private $secureRandom; /** @var AccessTokenMapper|\PHPUnit\Framework\MockObject\MockObject */ private $accessTokenMapper; - /** @var DefaultTokenMapper|\PHPUnit\Framework\MockObject\MockObject */ - private $defaultTokenMapper; /** @var SettingsController */ private $settingsController; @@ -59,7 +56,6 @@ class SettingsControllerTest extends TestCase { $this->clientMapper = $this->createMock(ClientMapper::class); $this->secureRandom = $this->createMock(ISecureRandom::class); $this->accessTokenMapper = $this->createMock(AccessTokenMapper::class); - $this->defaultTokenMapper = $this->createMock(DefaultTokenMapper::class); $l = $this->createMock(IL10N::class); $l->method('t') ->willReturnArgument(0); @@ -70,7 +66,6 @@ class SettingsControllerTest extends TestCase { $this->clientMapper, $this->secureRandom, $this->accessTokenMapper, - $this->defaultTokenMapper, $l ); } @@ -136,10 +131,6 @@ class SettingsControllerTest extends TestCase { ->expects($this->once()) ->method('deleteByClientId') ->with(123); - $this->defaultTokenMapper - ->expects($this->once()) - ->method('deleteByName') - ->with('My Client Name'); $this->clientMapper ->method('delete') ->with($client); diff --git a/apps/settings/tests/Controller/AuthSettingsControllerTest.php b/apps/settings/tests/Controller/AuthSettingsControllerTest.php index 477d0fcc8f5..b744b942e09 100644 --- a/apps/settings/tests/Controller/AuthSettingsControllerTest.php +++ b/apps/settings/tests/Controller/AuthSettingsControllerTest.php @@ -32,10 +32,10 @@ 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; use OC\Authentication\Token\IToken; use OC\Authentication\Token\IWipeableToken; +use OC\Authentication\Token\PublicKeyToken; use OC\Authentication\Token\RemoteWipe; use OCA\Settings\Controller\AuthSettingsController; use OCP\Activity\IEvent; @@ -178,7 +178,7 @@ class AuthSettingsControllerTest extends TestCase { public function testDestroy() { $tokenId = 124; - $token = $this->createMock(DefaultToken::class); + $token = $this->createMock(PublicKeyToken::class); $this->mockGetTokenById($tokenId, $token); $this->mockActivityManager(); @@ -200,7 +200,7 @@ class AuthSettingsControllerTest extends TestCase { public function testDestroyExpired() { $tokenId = 124; - $token = $this->createMock(DefaultToken::class); + $token = $this->createMock(PublicKeyToken::class); $token->expects($this->exactly(2)) ->method('getId') @@ -224,7 +224,7 @@ class AuthSettingsControllerTest extends TestCase { public function testDestroyWrongUser() { $tokenId = 124; - $token = $this->createMock(DefaultToken::class); + $token = $this->createMock(PublicKeyToken::class); $this->mockGetTokenById($tokenId, $token); @@ -252,7 +252,7 @@ class AuthSettingsControllerTest extends TestCase { */ public function testUpdateRename(string $name, string $newName): void { $tokenId = 42; - $token = $this->createMock(DefaultToken::class); + $token = $this->createMock(PublicKeyToken::class); $this->mockGetTokenById($tokenId, $token); $this->mockActivityManager(); @@ -295,7 +295,7 @@ class AuthSettingsControllerTest extends TestCase { */ public function testUpdateFilesystemScope(bool $filesystem, bool $newFilesystem): void { $tokenId = 42; - $token = $this->createMock(DefaultToken::class); + $token = $this->createMock(PublicKeyToken::class); $this->mockGetTokenById($tokenId, $token); $this->mockActivityManager(); @@ -325,7 +325,7 @@ class AuthSettingsControllerTest extends TestCase { public function testUpdateNoChange(): void { $tokenId = 42; - $token = $this->createMock(DefaultToken::class); + $token = $this->createMock(PublicKeyToken::class); $this->mockGetTokenById($tokenId, $token); @@ -356,7 +356,7 @@ class AuthSettingsControllerTest extends TestCase { public function testUpdateExpired() { $tokenId = 42; - $token = $this->createMock(DefaultToken::class); + $token = $this->createMock(PublicKeyToken::class); $token->expects($this->once()) ->method('getUID') @@ -376,7 +376,7 @@ class AuthSettingsControllerTest extends TestCase { public function testUpdateTokenWrongUser() { $tokenId = 42; - $token = $this->createMock(DefaultToken::class); + $token = $this->createMock(PublicKeyToken::class); $this->mockGetTokenById($tokenId, $token); diff --git a/apps/settings/tests/Settings/Personal/Security/AuthtokensTest.php b/apps/settings/tests/Settings/Personal/Security/AuthtokensTest.php index 3ee39c3624c..8fae0a44d8f 100644 --- a/apps/settings/tests/Settings/Personal/Security/AuthtokensTest.php +++ b/apps/settings/tests/Settings/Personal/Security/AuthtokensTest.php @@ -25,8 +25,8 @@ declare(strict_types=1); */ namespace OCA\Settings\Tests\Settings\Personal\Security; -use OC\Authentication\Token\DefaultToken; use OC\Authentication\Token\IProvider as IAuthTokenProvider; +use OC\Authentication\Token\PublicKeyToken; use OCA\Settings\Settings\Personal\Security\Authtokens; use OCP\AppFramework\Http\TemplateResponse; use OCP\AppFramework\Services\IInitialState; @@ -74,15 +74,15 @@ class AuthtokensTest extends TestCase { } public function testGetForm() { - $token1 = new DefaultToken(); + $token1 = new PublicKeyToken(); $token1->setId(100); - $token2 = new DefaultToken(); + $token2 = new PublicKeyToken(); $token2->setId(200); $tokens = [ $token1, $token2, ]; - $sessionToken = new DefaultToken(); + $sessionToken = new PublicKeyToken(); $sessionToken->setId(100); $this->authTokenProvider->expects($this->once()) diff --git a/tests/lib/Authentication/Token/ManagerTest.php b/tests/lib/Authentication/Token/ManagerTest.php index fb92b3e5018..0a6578ced00 100644 --- a/tests/lib/Authentication/Token/ManagerTest.php +++ b/tests/lib/Authentication/Token/ManagerTest.php @@ -29,8 +29,6 @@ namespace Test\Authentication\Token; use Doctrine\DBAL\Exception\UniqueConstraintViolationException; use OC\Authentication\Exceptions\InvalidTokenException; use OC\Authentication\Exceptions\PasswordlessTokenException; -use OC\Authentication\Token\DefaultToken; -use OC\Authentication\Token\DefaultTokenProvider; use OC\Authentication\Token\IToken; use OC\Authentication\Token\Manager; use OC\Authentication\Token\PublicKeyToken; @@ -42,8 +40,6 @@ class ManagerTest extends TestCase { /** @var PublicKeyTokenProvider|MockObject */ private $publicKeyTokenProvider; - /** @var DefaultTokenProvider|MockObject */ - private $defaultTokenProvider; /** @var Manager */ private $manager; @@ -51,17 +47,12 @@ class ManagerTest extends TestCase { parent::setUp(); $this->publicKeyTokenProvider = $this->createMock(PublicKeyTokenProvider::class); - $this->defaultTokenProvider = $this->createMock(DefaultTokenProvider::class); $this->manager = new Manager( - $this->defaultTokenProvider, $this->publicKeyTokenProvider ); } public function testGenerateToken() { - $this->defaultTokenProvider->expects($this->never()) - ->method('generateToken'); - $token = new PublicKeyToken(); $this->publicKeyTokenProvider->expects($this->once()) @@ -92,8 +83,6 @@ class ManagerTest extends TestCase { public function testGenerateConflictingToken() { /** @var MockObject|UniqueConstraintViolationException $exception */ $exception = $this->createMock(UniqueConstraintViolationException::class); - $this->defaultTokenProvider->expects($this->never()) - ->method('generateToken'); $token = new PublicKeyToken(); $token->setUid('uid'); @@ -129,18 +118,12 @@ class ManagerTest extends TestCase { public function tokenData(): array { return [ - [new DefaultToken()], [new PublicKeyToken()], [$this->createMock(IToken::class)], ]; } protected function setNoCall(IToken $token) { - if (!($token instanceof DefaultToken)) { - $this->defaultTokenProvider->expects($this->never()) - ->method($this->anything()); - } - if (!($token instanceof PublicKeyToken)) { $this->publicKeyTokenProvider->expects($this->never()) ->method($this->anything()); @@ -148,13 +131,6 @@ class ManagerTest extends TestCase { } protected function setCall(IToken $token, string $function, $return = null) { - if ($token instanceof DefaultToken) { - $this->defaultTokenProvider->expects($this->once()) - ->method($function) - ->with($token) - ->willReturn($return); - } - if ($token instanceof PublicKeyToken) { $this->publicKeyTokenProvider->expects($this->once()) ->method($function) @@ -164,7 +140,7 @@ class ManagerTest extends TestCase { } protected function setException(IToken $token) { - if (!($token instanceof DefaultToken) && !($token instanceof PublicKeyToken)) { + if (!($token instanceof PublicKeyToken)) { $this->expectException(InvalidTokenException::class); } } @@ -216,10 +192,6 @@ class ManagerTest extends TestCase { } public function testInvalidateTokens() { - $this->defaultTokenProvider->expects($this->once()) - ->method('invalidateToken') - ->with('token'); - $this->publicKeyTokenProvider->expects($this->once()) ->method('invalidateToken') ->with('token'); @@ -228,10 +200,6 @@ class ManagerTest extends TestCase { } public function testInvalidateTokenById() { - $this->defaultTokenProvider->expects($this->once()) - ->method('invalidateTokenById') - ->with('uid', 42); - $this->publicKeyTokenProvider->expects($this->once()) ->method('invalidateTokenById') ->with('uid', 42); @@ -240,9 +208,6 @@ class ManagerTest extends TestCase { } public function testInvalidateOldTokens() { - $this->defaultTokenProvider->expects($this->once()) - ->method('invalidateOldTokens'); - $this->publicKeyTokenProvider->expects($this->once()) ->method('invalidateOldTokens'); @@ -250,28 +215,19 @@ class ManagerTest extends TestCase { } public function testGetTokenByUser() { - $t1 = new DefaultToken(); - $t2 = new DefaultToken(); - $t3 = new PublicKeyToken(); - $t4 = new PublicKeyToken(); - - $this->defaultTokenProvider - ->method('getTokenByUser') - ->willReturn([$t1, $t2]); + $t1 = new PublicKeyToken(); + $t2 = new PublicKeyToken(); $this->publicKeyTokenProvider ->method('getTokenByUser') - ->willReturn([$t3, $t4]); + ->willReturn([$t1, $t2]); $result = $this->manager->getTokenByUser('uid'); - $this->assertEquals([$t1, $t2, $t3, $t4], $result); + $this->assertEquals([$t1, $t2], $result); } public function testRenewSessionTokenPublicKey() { - $this->defaultTokenProvider->expects($this->never()) - ->method($this->anything()); - $this->publicKeyTokenProvider->expects($this->once()) ->method('renewSessionToken') ->with('oldId', 'newId'); @@ -279,30 +235,12 @@ class ManagerTest extends TestCase { $this->manager->renewSessionToken('oldId', 'newId'); } - public function testRenewSessionTokenDefault() { - $this->publicKeyTokenProvider->expects($this->once()) - ->method('renewSessionToken') - ->with('oldId', 'newId') - ->willThrowException(new InvalidTokenException()); - - $this->defaultTokenProvider->expects($this->once()) - ->method('renewSessionToken') - ->with('oldId', 'newId'); - - $this->manager->renewSessionToken('oldId', 'newId'); - } - public function testRenewSessionInvalid() { $this->publicKeyTokenProvider->expects($this->once()) ->method('renewSessionToken') ->with('oldId', 'newId') ->willThrowException(new InvalidTokenException()); - $this->defaultTokenProvider->expects($this->once()) - ->method('renewSessionToken') - ->with('oldId', 'newId') - ->willThrowException(new InvalidTokenException()); - $this->expectException(InvalidTokenException::class); $this->manager->renewSessionToken('oldId', 'newId'); } @@ -315,26 +253,6 @@ class ManagerTest extends TestCase { ->with(42) ->willReturn($token); - $this->defaultTokenProvider->expects($this->never()) - ->method($this->anything()); - - - $this->assertSame($token, $this->manager->getTokenById(42)); - } - - public function testGetTokenByIdDefault() { - $token = $this->createMock(IToken::class); - - $this->publicKeyTokenProvider->expects($this->once()) - ->method('getTokenById') - ->with(42) - ->willThrowException(new InvalidTokenException()); - - $this->defaultTokenProvider->expects($this->once()) - ->method('getTokenById') - ->with(42) - ->willReturn($token); - $this->assertSame($token, $this->manager->getTokenById(42)); } @@ -344,11 +262,6 @@ class ManagerTest extends TestCase { ->with(42) ->willThrowException(new InvalidTokenException()); - $this->defaultTokenProvider->expects($this->once()) - ->method('getTokenById') - ->with(42) - ->willThrowException(new InvalidTokenException()); - $this->expectException(InvalidTokenException::class); $this->manager->getTokenById(42); } @@ -356,9 +269,6 @@ class ManagerTest extends TestCase { public function testGetTokenPublicKey() { $token = new PublicKeyToken(); - $this->defaultTokenProvider->expects($this->never()) - ->method($this->anything()); - $this->publicKeyTokenProvider ->method('getToken') ->with('tokenId') @@ -368,11 +278,6 @@ class ManagerTest extends TestCase { } public function testGetTokenInvalid() { - $this->defaultTokenProvider - ->method('getToken') - ->with('tokenId') - ->willThrowException(new InvalidTokenException()); - $this->publicKeyTokenProvider ->method('getToken') ->with('tokenId') @@ -382,58 +287,6 @@ class ManagerTest extends TestCase { $this->manager->getToken('tokenId'); } - public function testGetTokenConvertPassword() { - $oldToken = new DefaultToken(); - $newToken = new PublicKeyToken(); - - $this->publicKeyTokenProvider - ->method('getToken') - ->with('tokenId') - ->willThrowException(new InvalidTokenException()); - - $this->defaultTokenProvider - ->method('getToken') - ->willReturn($oldToken); - - $this->defaultTokenProvider - ->method('getPassword') - ->with($oldToken, 'tokenId') - ->willReturn('password'); - - $this->publicKeyTokenProvider - ->method('convertToken') - ->with($oldToken, 'tokenId', 'password') - ->willReturn($newToken); - - $this->assertSame($newToken, $this->manager->getToken('tokenId')); - } - - public function testGetTokenConvertNoPassword() { - $oldToken = new DefaultToken(); - $newToken = new PublicKeyToken(); - - $this->publicKeyTokenProvider - ->method('getToken') - ->with('tokenId') - ->willThrowException(new InvalidTokenException()); - - $this->defaultTokenProvider - ->method('getToken') - ->willReturn($oldToken); - - $this->defaultTokenProvider - ->method('getPassword') - ->with($oldToken, 'tokenId') - ->willThrowException(new PasswordlessTokenException()); - - $this->publicKeyTokenProvider - ->method('convertToken') - ->with($oldToken, 'tokenId', null) - ->willReturn($newToken); - - $this->assertSame($newToken, $this->manager->getToken('tokenId')); - } - public function testRotateInvalid() { $this->expectException(InvalidTokenException::class); $this->manager->rotate($this->createMock(IToken::class), 'oldId', 'newId'); @@ -450,60 +303,12 @@ class ManagerTest extends TestCase { $this->assertSame($token, $this->manager->rotate($token, 'oldId', 'newId')); } - public function testRotateConvertPassword() { - $oldToken = new DefaultToken(); - $newToken = new PublicKeyToken(); - - $this->defaultTokenProvider - ->method('getPassword') - ->with($oldToken, 'oldId') - ->willReturn('password'); - - $this->publicKeyTokenProvider - ->method('convertToken') - ->with($oldToken, 'newId', 'password') - ->willReturn($newToken); - - $this->assertSame($newToken, $this->manager->rotate($oldToken, 'oldId', 'newId')); - } - - public function testRotateConvertNoPassword() { - $oldToken = new DefaultToken(); - $newToken = new PublicKeyToken(); - - $this->defaultTokenProvider - ->method('getPassword') - ->with($oldToken, 'oldId') - ->willThrowException(new PasswordlessTokenException()); - - $this->publicKeyTokenProvider - ->method('convertToken') - ->with($oldToken, 'newId', null) - ->willReturn($newToken); - - $this->assertSame($newToken, $this->manager->rotate($oldToken, 'oldId', 'newId')); - } - - public function testMarkPasswordInvalidDefault() { - $token = $this->createMock(DefaultToken::class); - - $this->defaultTokenProvider->expects($this->once()) - ->method('markPasswordInvalid') - ->with($token, 'tokenId'); - $this->publicKeyTokenProvider->expects($this->never()) - ->method($this->anything()); - - $this->manager->markPasswordInvalid($token, 'tokenId'); - } - public function testMarkPasswordInvalidPublicKey() { $token = $this->createMock(PublicKeyToken::class); $this->publicKeyTokenProvider->expects($this->once()) ->method('markPasswordInvalid') ->with($token, 'tokenId'); - $this->defaultTokenProvider->expects($this->never()) - ->method($this->anything()); $this->manager->markPasswordInvalid($token, 'tokenId'); } @@ -515,9 +320,6 @@ class ManagerTest extends TestCase { } public function testUpdatePasswords() { - $this->defaultTokenProvider->expects($this->once()) - ->method('updatePasswords') - ->with('uid', 'pass'); $this->publicKeyTokenProvider->expects($this->once()) ->method('updatePasswords') ->with('uid', 'pass'); diff --git a/tests/lib/Authentication/Token/PublicKeyTokenProviderTest.php b/tests/lib/Authentication/Token/PublicKeyTokenProviderTest.php index 486660f17c6..6d18c22ca07 100644 --- a/tests/lib/Authentication/Token/PublicKeyTokenProviderTest.php +++ b/tests/lib/Authentication/Token/PublicKeyTokenProviderTest.php @@ -476,39 +476,8 @@ class PublicKeyTokenProviderTest extends TestCase { $this->assertNull($new->getPassword()); } - public function testConvertToken() { - $defaultToken = new DefaultToken(); - $defaultToken->setId(42); - $defaultToken->setPassword('oldPass'); - $defaultToken->setExpires(1337); - $defaultToken->setToken('oldToken'); - $defaultToken->setUid('uid'); - $defaultToken->setLoginName('loginName'); - $defaultToken->setLastActivity(999); - $defaultToken->setName('name'); - $defaultToken->setRemember(IToken::REMEMBER); - $defaultToken->setType(IToken::PERMANENT_TOKEN); - - $this->mapper->expects($this->once()) - ->method('update') - ->willReturnArgument(0); - - $newToken = $this->tokenProvider->convertToken($defaultToken, 'newToken', 'newPassword'); - - $this->assertSame(42, $newToken->getId()); - $this->assertSame('newPassword', $this->tokenProvider->getPassword($newToken, 'newToken')); - $this->assertSame(1337, $newToken->getExpires()); - $this->assertSame('uid', $newToken->getUID()); - $this->assertSame('loginName', $newToken->getLoginName()); - $this->assertSame(1313131, $newToken->getLastActivity()); - $this->assertSame(1313131, $newToken->getLastCheck()); - $this->assertSame('name', $newToken->getName()); - $this->assertSame(IToken::REMEMBER, $newToken->getRemember()); - $this->assertSame(IToken::PERMANENT_TOKEN, $newToken->getType()); - } - public function testMarkPasswordInvalidInvalidToken() { - $token = $this->createMock(DefaultToken::class); + $token = $this->createMock(IToken::class); $this->expectException(InvalidTokenException::class); -- 2.39.5