diff options
Diffstat (limited to 'apps/oauth2/tests/Controller')
3 files changed, 240 insertions, 90 deletions
diff --git a/apps/oauth2/tests/Controller/LoginRedirectorControllerTest.php b/apps/oauth2/tests/Controller/LoginRedirectorControllerTest.php index eb8e47a102f..04ac0bfbd28 100644 --- a/apps/oauth2/tests/Controller/LoginRedirectorControllerTest.php +++ b/apps/oauth2/tests/Controller/LoginRedirectorControllerTest.php @@ -1,38 +1,42 @@ <?php + /** * SPDX-FileCopyrightText: 2017 Nextcloud GmbH and Nextcloud contributors * SPDX-License-Identifier: AGPL-3.0-or-later */ namespace OCA\OAuth2\Tests\Controller; +use OC\Core\Controller\ClientFlowLoginController; use OCA\OAuth2\Controller\LoginRedirectorController; use OCA\OAuth2\Db\Client; use OCA\OAuth2\Db\ClientMapper; use OCA\OAuth2\Exceptions\ClientNotFoundException; use OCP\AppFramework\Http\RedirectResponse; use OCP\AppFramework\Http\TemplateResponse; +use OCP\IAppConfig; +use OCP\IConfig; use OCP\IL10N; use OCP\IRequest; use OCP\ISession; use OCP\IURLGenerator; +use OCP\Security\ISecureRandom; +use PHPUnit\Framework\MockObject\MockObject; use Test\TestCase; /** * @group DB */ class LoginRedirectorControllerTest extends TestCase { - /** @var IRequest|\PHPUnit\Framework\MockObject\MockObject */ - private $request; - /** @var IURLGenerator|\PHPUnit\Framework\MockObject\MockObject */ - private $urlGenerator; - /** @var ClientMapper|\PHPUnit\Framework\MockObject\MockObject */ - private $clientMapper; - /** @var ISession|\PHPUnit\Framework\MockObject\MockObject */ - private $session; - /** @var LoginRedirectorController */ - private $loginRedirectorController; - /** @var IL10N */ - private $l; + private IRequest&MockObject $request; + private IURLGenerator&MockObject $urlGenerator; + private ClientMapper&MockObject $clientMapper; + private ISession&MockObject $session; + private IL10N&MockObject $l; + private ISecureRandom&MockObject $random; + private IAppConfig&MockObject $appConfig; + private IConfig&MockObject $config; + + private LoginRedirectorController $loginRedirectorController; protected function setUp(): void { parent::setUp(); @@ -42,6 +46,9 @@ class LoginRedirectorControllerTest extends TestCase { $this->clientMapper = $this->createMock(ClientMapper::class); $this->session = $this->createMock(ISession::class); $this->l = $this->createMock(IL10N::class); + $this->random = $this->createMock(ISecureRandom::class); + $this->appConfig = $this->createMock(IAppConfig::class); + $this->config = $this->createMock(IConfig::class); $this->loginRedirectorController = new LoginRedirectorController( 'oauth2', @@ -49,11 +56,14 @@ class LoginRedirectorControllerTest extends TestCase { $this->urlGenerator, $this->clientMapper, $this->session, - $this->l + $this->l, + $this->random, + $this->appConfig, + $this->config, ); } - public function testAuthorize() { + public function testAuthorize(): void { $client = new Client(); $client->setClientIdentifier('MyClientIdentifier'); $this->clientMapper @@ -72,15 +82,74 @@ class LoginRedirectorControllerTest extends TestCase { 'core.ClientFlowLogin.showAuthPickerPage', [ 'clientIdentifier' => 'MyClientIdentifier', + 'providedRedirectUri' => '', ] ) ->willReturn('https://example.com/?clientIdentifier=foo'); + $this->config + ->expects($this->once()) + ->method('getSystemValueBool') + ->with('oauth2.enable_oc_clients', false) + ->willReturn(false); $expected = new RedirectResponse('https://example.com/?clientIdentifier=foo'); $this->assertEquals($expected, $this->loginRedirectorController->authorize('MyClientId', 'MyState', 'code')); } - public function testAuthorizeWrongResponseType() { + public function testAuthorizeSkipPicker(): void { + $client = new Client(); + $client->setName('MyClientName'); + $client->setClientIdentifier('MyClientIdentifier'); + $this->clientMapper + ->expects($this->once()) + ->method('getByIdentifier') + ->with('MyClientId') + ->willReturn($client); + $this->session + ->expects(static::exactly(2)) + ->method('set') + ->willReturnCallback(function (string $key, string $value): void { + switch ([$key, $value]) { + case ['oauth.state', 'MyState']: + case [ClientFlowLoginController::STATE_NAME, 'MyStateToken']: + /* Expected */ + break; + default: + throw new LogicException(); + } + }); + $this->appConfig + ->expects(static::once()) + ->method('getValueArray') + ->with('oauth2', 'skipAuthPickerApplications', []) + ->willReturn(['MyClientName']); + $this->random + ->expects(static::once()) + ->method('generate') + ->willReturn('MyStateToken'); + $this->urlGenerator + ->expects($this->once()) + ->method('linkToRouteAbsolute') + ->with( + 'core.ClientFlowLogin.grantPage', + [ + 'stateToken' => 'MyStateToken', + 'clientIdentifier' => 'MyClientIdentifier', + 'providedRedirectUri' => '', + ] + ) + ->willReturn('https://example.com/?clientIdentifier=foo'); + $this->config + ->expects($this->once()) + ->method('getSystemValueBool') + ->with('oauth2.enable_oc_clients', false) + ->willReturn(false); + + $expected = new RedirectResponse('https://example.com/?clientIdentifier=foo'); + $this->assertEquals($expected, $this->loginRedirectorController->authorize('MyClientId', 'MyState', 'code')); + } + + public function testAuthorizeWrongResponseType(): void { $client = new Client(); $client->setClientIdentifier('MyClientIdentifier'); $client->setRedirectUri('http://foo.bar'); @@ -98,7 +167,75 @@ class LoginRedirectorControllerTest extends TestCase { $this->assertEquals($expected, $this->loginRedirectorController->authorize('MyClientId', 'MyState', 'wrongcode')); } - public function testClientNotFound() { + public function testAuthorizeWithLegacyOcClient(): void { + $client = new Client(); + $client->setClientIdentifier('MyClientIdentifier'); + $client->setRedirectUri('http://localhost:*'); + $this->clientMapper + ->expects($this->once()) + ->method('getByIdentifier') + ->with('MyClientId') + ->willReturn($client); + $this->session + ->expects($this->once()) + ->method('set') + ->with('oauth.state', 'MyState'); + $this->urlGenerator + ->expects($this->once()) + ->method('linkToRouteAbsolute') + ->with( + 'core.ClientFlowLogin.showAuthPickerPage', + [ + 'clientIdentifier' => 'MyClientIdentifier', + 'providedRedirectUri' => 'http://localhost:30000', + ] + ) + ->willReturn('https://example.com/?clientIdentifier=foo&providedRedirectUri=http://localhost:30000'); + $this->config + ->expects($this->once()) + ->method('getSystemValueBool') + ->with('oauth2.enable_oc_clients', false) + ->willReturn(true); + + $expected = new RedirectResponse('https://example.com/?clientIdentifier=foo&providedRedirectUri=http://localhost:30000'); + $this->assertEquals($expected, $this->loginRedirectorController->authorize('MyClientId', 'MyState', 'code', 'http://localhost:30000')); + } + + public function testAuthorizeNotForwardingUntrustedURIs(): void { + $client = new Client(); + $client->setClientIdentifier('MyClientIdentifier'); + $this->clientMapper + ->expects($this->once()) + ->method('getByIdentifier') + ->with('MyClientId') + ->willReturn($client); + $this->session + ->expects($this->once()) + ->method('set') + ->with('oauth.state', 'MyState'); + $this->urlGenerator + ->expects($this->once()) + ->method('linkToRouteAbsolute') + ->with( + 'core.ClientFlowLogin.showAuthPickerPage', + [ + 'clientIdentifier' => 'MyClientIdentifier', + 'providedRedirectUri' => '', + ] + ) + ->willReturn('https://example.com/?clientIdentifier=foo'); + $this->config + ->expects($this->once()) + ->method('getSystemValueBool') + ->with('oauth2.enable_oc_clients', false) + ->willReturn(false); + + $expected = new RedirectResponse('https://example.com/?clientIdentifier=foo'); + $this->assertEquals($expected, $this->loginRedirectorController->authorize('MyClientId', 'MyState', 'code', 'http://untrusted-uri.com')); + } + + + public function testClientNotFound(): void { $clientNotFound = new ClientNotFoundException('could not find client test123', 0); $this->clientMapper ->expects($this->once()) diff --git a/apps/oauth2/tests/Controller/OauthApiControllerTest.php b/apps/oauth2/tests/Controller/OauthApiControllerTest.php index 369b5f38e0b..53dd8549196 100644 --- a/apps/oauth2/tests/Controller/OauthApiControllerTest.php +++ b/apps/oauth2/tests/Controller/OauthApiControllerTest.php @@ -1,4 +1,5 @@ <?php + /** * SPDX-FileCopyrightText: 2017 Nextcloud GmbH and Nextcloud contributors * SPDX-License-Identifier: AGPL-3.0-or-later @@ -84,7 +85,7 @@ class OauthApiControllerTest extends TestCase { ); } - public function testGetTokenInvalidGrantType() { + public function testGetTokenInvalidGrantType(): void { $expected = new JSONResponse([ 'error' => 'invalid_grant', ], Http::STATUS_BAD_REQUEST); @@ -93,7 +94,7 @@ class OauthApiControllerTest extends TestCase { $this->assertEquals($expected, $this->oauthApiController->getToken('foo', null, null, null, null)); } - public function testGetTokenInvalidCode() { + public function testGetTokenInvalidCode(): void { $expected = new JSONResponse([ 'error' => 'invalid_request', ], Http::STATUS_BAD_REQUEST); @@ -106,7 +107,7 @@ class OauthApiControllerTest extends TestCase { $this->assertEquals($expected, $this->oauthApiController->getToken('authorization_code', 'invalidcode', null, null, null)); } - public function testGetTokenExpiredCode() { + public function testGetTokenExpiredCode(): void { $codeCreatedAt = 100; $expiredSince = 123; @@ -131,7 +132,7 @@ class OauthApiControllerTest extends TestCase { $this->assertEquals($expected, $this->oauthApiController->getToken('authorization_code', 'validcode', null, null, null)); } - public function testGetTokenWithCodeForActiveToken() { + public function testGetTokenWithCodeForActiveToken(): void { // if a token has already delivered oauth tokens, // it should not be possible to get a new oauth token from a valid authorization code $codeCreatedAt = 100; @@ -158,7 +159,7 @@ class OauthApiControllerTest extends TestCase { $this->assertEquals($expected, $this->oauthApiController->getToken('authorization_code', 'validcode', null, null, null)); } - public function testGetTokenClientDoesNotExist() { + public function testGetTokenClientDoesNotExist(): void { // In this test, the token's authorization code is valid and has not expired // and we check what happens when the associated Oauth client does not exist $codeCreatedAt = 100; @@ -189,7 +190,7 @@ class OauthApiControllerTest extends TestCase { $this->assertEquals($expected, $this->oauthApiController->getToken('authorization_code', 'validcode', null, null, null)); } - public function testRefreshTokenInvalidRefreshToken() { + public function testRefreshTokenInvalidRefreshToken(): void { $expected = new JSONResponse([ 'error' => 'invalid_request', ], Http::STATUS_BAD_REQUEST); @@ -202,7 +203,7 @@ class OauthApiControllerTest extends TestCase { $this->assertEquals($expected, $this->oauthApiController->getToken('refresh_token', null, 'invalidrefresh', null, null)); } - public function testRefreshTokenClientDoesNotExist() { + public function testRefreshTokenClientDoesNotExist(): void { $expected = new JSONResponse([ 'error' => 'invalid_request', ], Http::STATUS_BAD_REQUEST); @@ -222,7 +223,7 @@ class OauthApiControllerTest extends TestCase { $this->assertEquals($expected, $this->oauthApiController->getToken('refresh_token', null, 'validrefresh', null, null)); } - public function invalidClientProvider() { + public static function invalidClientProvider() { return [ ['invalidClientId', 'invalidClientSecret'], ['clientId', 'invalidClientSecret'], @@ -231,12 +232,12 @@ class OauthApiControllerTest extends TestCase { } /** - * @dataProvider invalidClientProvider * * @param string $clientId * @param string $clientSecret */ - public function testRefreshTokenInvalidClient($clientId, $clientSecret) { + #[\PHPUnit\Framework\Attributes\DataProvider('invalidClientProvider')] + public function testRefreshTokenInvalidClient($clientId, $clientSecret): void { $expected = new JSONResponse([ 'error' => 'invalid_client', ], Http::STATUS_BAD_REQUEST); @@ -249,9 +250,20 @@ class OauthApiControllerTest extends TestCase { ->with('validrefresh') ->willReturn($accessToken); + $this->crypto + ->method('calculateHMAC') + ->with($this->callback(function (string $text) { + return $text === 'clientSecret' || $text === 'invalidClientSecret'; + })) + ->willReturnCallback(function (string $text) { + return $text === 'clientSecret' + ? 'hashedClientSecret' + : 'hashedInvalidClientSecret'; + }); + $client = new Client(); $client->setClientIdentifier('clientId'); - $client->setSecret('clientSecret'); + $client->setSecret(bin2hex('hashedClientSecret')); $this->clientMapper->method('getByUid') ->with(42) ->willReturn($client); @@ -259,7 +271,7 @@ class OauthApiControllerTest extends TestCase { $this->assertEquals($expected, $this->oauthApiController->getToken('refresh_token', null, 'validrefresh', $clientId, $clientSecret)); } - public function testRefreshTokenInvalidAppToken() { + public function testRefreshTokenInvalidAppToken(): void { $expected = new JSONResponse([ 'error' => 'invalid_request', ], Http::STATUS_BAD_REQUEST); @@ -276,21 +288,20 @@ class OauthApiControllerTest extends TestCase { $client = new Client(); $client->setClientIdentifier('clientId'); - $client->setSecret('encryptedClientSecret'); + $client->setSecret(bin2hex('hashedClientSecret')); $this->clientMapper->method('getByUid') ->with(42) ->willReturn($client); $this->crypto ->method('decrypt') - ->with($this->callback(function (string $text) { - return $text === 'encryptedClientSecret' || $text === 'encryptedToken'; - })) - ->willReturnCallback(function (string $text) { - return $text === 'encryptedClientSecret' - ? 'clientSecret' - : ($text === 'encryptedToken' ? 'decryptedToken' : ''); - }); + ->with('encryptedToken') + ->willReturn('decryptedToken'); + + $this->crypto + ->method('calculateHMAC') + ->with('clientSecret') + ->willReturn('hashedClientSecret'); $this->tokenProvider->method('getTokenById') ->with(1337) @@ -303,7 +314,7 @@ class OauthApiControllerTest extends TestCase { $this->assertEquals($expected, $this->oauthApiController->getToken('refresh_token', null, 'validrefresh', 'clientId', 'clientSecret')); } - public function testRefreshTokenValidAppToken() { + public function testRefreshTokenValidAppToken(): void { $accessToken = new AccessToken(); $accessToken->setClientId(42); $accessToken->setTokenId(1337); @@ -315,21 +326,20 @@ class OauthApiControllerTest extends TestCase { $client = new Client(); $client->setClientIdentifier('clientId'); - $client->setSecret('encryptedClientSecret'); + $client->setSecret(bin2hex('hashedClientSecret')); $this->clientMapper->method('getByUid') ->with(42) ->willReturn($client); $this->crypto ->method('decrypt') - ->with($this->callback(function (string $text) { - return $text === 'encryptedClientSecret' || $text === 'encryptedToken'; - })) - ->willReturnCallback(function (string $text) { - return $text === 'encryptedClientSecret' - ? 'clientSecret' - : ($text === 'encryptedToken' ? 'decryptedToken' : ''); - }); + ->with('encryptedToken') + ->willReturn('decryptedToken'); + + $this->crypto + ->method('calculateHMAC') + ->with('clientSecret') + ->willReturn('hashedClientSecret'); $appToken = new PublicKeyToken(); $appToken->setUid('userId'); @@ -343,7 +353,7 @@ class OauthApiControllerTest extends TestCase { $this->secureRandom->method('generate') ->willReturnCallback(function ($len) { - return 'random'.$len; + return 'random' . $len; }); $this->tokenProvider->expects($this->once()) @@ -373,8 +383,8 @@ class OauthApiControllerTest extends TestCase { ->method('update') ->with( $this->callback(function (AccessToken $token) { - return $token->getHashedCode() === hash('sha512', 'random128') && - $token->getEncryptedToken() === 'newEncryptedToken'; + return $token->getHashedCode() === hash('sha512', 'random128') + && $token->getEncryptedToken() === 'newEncryptedToken'; }) ); @@ -400,7 +410,7 @@ class OauthApiControllerTest extends TestCase { $this->assertEquals($expected, $this->oauthApiController->getToken('refresh_token', null, 'validrefresh', 'clientId', 'clientSecret')); } - public function testRefreshTokenValidAppTokenBasicAuth() { + public function testRefreshTokenValidAppTokenBasicAuth(): void { $accessToken = new AccessToken(); $accessToken->setClientId(42); $accessToken->setTokenId(1337); @@ -412,21 +422,20 @@ class OauthApiControllerTest extends TestCase { $client = new Client(); $client->setClientIdentifier('clientId'); - $client->setSecret('encryptedClientSecret'); + $client->setSecret(bin2hex('hashedClientSecret')); $this->clientMapper->method('getByUid') ->with(42) ->willReturn($client); $this->crypto ->method('decrypt') - ->with($this->callback(function (string $text) { - return $text === 'encryptedClientSecret' || $text === 'encryptedToken'; - })) - ->willReturnCallback(function (string $text) { - return $text === 'encryptedClientSecret' - ? 'clientSecret' - : ($text === 'encryptedToken' ? 'decryptedToken' : ''); - }); + ->with('encryptedToken') + ->willReturn('decryptedToken'); + + $this->crypto + ->method('calculateHMAC') + ->with('clientSecret') + ->willReturn('hashedClientSecret'); $appToken = new PublicKeyToken(); $appToken->setUid('userId'); @@ -440,7 +449,7 @@ class OauthApiControllerTest extends TestCase { $this->secureRandom->method('generate') ->willReturnCallback(function ($len) { - return 'random'.$len; + return 'random' . $len; }); $this->tokenProvider->expects($this->once()) @@ -470,8 +479,8 @@ class OauthApiControllerTest extends TestCase { ->method('update') ->with( $this->callback(function (AccessToken $token) { - return $token->getHashedCode() === hash('sha512', 'random128') && - $token->getEncryptedToken() === 'newEncryptedToken'; + return $token->getHashedCode() === hash('sha512', 'random128') + && $token->getEncryptedToken() === 'newEncryptedToken'; }) ); @@ -500,7 +509,7 @@ class OauthApiControllerTest extends TestCase { $this->assertEquals($expected, $this->oauthApiController->getToken('refresh_token', null, 'validrefresh', null, null)); } - public function testRefreshTokenExpiredAppToken() { + public function testRefreshTokenExpiredAppToken(): void { $accessToken = new AccessToken(); $accessToken->setClientId(42); $accessToken->setTokenId(1337); @@ -512,21 +521,20 @@ class OauthApiControllerTest extends TestCase { $client = new Client(); $client->setClientIdentifier('clientId'); - $client->setSecret('encryptedClientSecret'); + $client->setSecret(bin2hex('hashedClientSecret')); $this->clientMapper->method('getByUid') ->with(42) ->willReturn($client); $this->crypto ->method('decrypt') - ->with($this->callback(function (string $text) { - return $text === 'encryptedClientSecret' || $text === 'encryptedToken'; - })) - ->willReturnCallback(function (string $text) { - return $text === 'encryptedClientSecret' - ? 'clientSecret' - : ($text === 'encryptedToken' ? 'decryptedToken' : ''); - }); + ->with('encryptedToken') + ->willReturn('decryptedToken'); + + $this->crypto + ->method('calculateHMAC') + ->with('clientSecret') + ->willReturn('hashedClientSecret'); $appToken = new PublicKeyToken(); $appToken->setUid('userId'); @@ -540,7 +548,7 @@ class OauthApiControllerTest extends TestCase { $this->secureRandom->method('generate') ->willReturnCallback(function ($len) { - return 'random'.$len; + return 'random' . $len; }); $this->tokenProvider->expects($this->once()) @@ -570,8 +578,8 @@ class OauthApiControllerTest extends TestCase { ->method('update') ->with( $this->callback(function (AccessToken $token) { - return $token->getHashedCode() === hash('sha512', 'random128') && - $token->getEncryptedToken() === 'newEncryptedToken'; + return $token->getHashedCode() === hash('sha512', 'random128') + && $token->getEncryptedToken() === 'newEncryptedToken'; }) ); diff --git a/apps/oauth2/tests/Controller/SettingsControllerTest.php b/apps/oauth2/tests/Controller/SettingsControllerTest.php index f73703f1929..030a220e3d7 100644 --- a/apps/oauth2/tests/Controller/SettingsControllerTest.php +++ b/apps/oauth2/tests/Controller/SettingsControllerTest.php @@ -1,4 +1,5 @@ <?php + /** * SPDX-FileCopyrightText: 2017 Nextcloud GmbH and Nextcloud contributors * SPDX-License-Identifier: AGPL-3.0-or-later @@ -18,6 +19,7 @@ use OCP\IUser; use OCP\IUserManager; use OCP\Security\ICrypto; use OCP\Security\ISecureRandom; +use OCP\Server; use Test\TestCase; /** @@ -70,7 +72,7 @@ class SettingsControllerTest extends TestCase { } - public function testAddClient() { + public function testAddClient(): void { $this->secureRandom ->expects($this->exactly(2)) ->method('generate') @@ -81,23 +83,23 @@ class SettingsControllerTest extends TestCase { $this->crypto ->expects($this->once()) - ->method('encrypt') - ->willReturn('MyEncryptedSecret'); + ->method('calculateHMAC') + ->willReturn('MyHashedSecret'); $client = new Client(); $client->setName('My Client Name'); $client->setRedirectUri('https://example.com/'); - $client->setSecret('MySecret'); + $client->setSecret(bin2hex('MyHashedSecret')); $client->setClientIdentifier('MyClientIdentifier'); $this->clientMapper ->expects($this->once()) ->method('insert') ->with($this->callback(function (Client $c) { - return $c->getName() === 'My Client Name' && - $c->getRedirectUri() === 'https://example.com/' && - $c->getSecret() === 'MyEncryptedSecret' && - $c->getClientIdentifier() === 'MyClientIdentifier'; + return $c->getName() === 'My Client Name' + && $c->getRedirectUri() === 'https://example.com/' + && $c->getSecret() === bin2hex('MyHashedSecret') + && $c->getClientIdentifier() === 'MyClientIdentifier'; }))->willReturnCallback(function (Client $c) { $c->setId(42); return $c; @@ -117,16 +119,19 @@ class SettingsControllerTest extends TestCase { ], $data); } - public function testDeleteClient() { + public function testDeleteClient(): void { - $userManager = \OC::$server->getUserManager(); + $userManager = Server::get(IUserManager::class); // count other users in the db before adding our own $count = 0; - $function = function (IUser $user) use (&$count) { - $count++; + $function = function (IUser $user) use (&$count): void { + if ($user->getLastLogin() > 0) { + $count++; + } }; $userManager->callForAllUsers($function); $user1 = $userManager->createUser('test101', 'test101'); + $user1->updateLastLoginTimestamp(); $tokenProviderMock = $this->getMockBuilder(IAuthTokenProvider::class)->getMock(); // expect one call per user and ensure the correct client name @@ -139,7 +144,7 @@ class SettingsControllerTest extends TestCase { $client->setId(123); $client->setName('My Client Name'); $client->setRedirectUri('https://example.com/'); - $client->setSecret('MySecret'); + $client->setSecret(bin2hex('MyHashedSecret')); $client->setClientIdentifier('MyClientIdentifier'); $this->clientMapper @@ -174,7 +179,7 @@ class SettingsControllerTest extends TestCase { $user1->delete(); } - public function testInvalidRedirectUri() { + public function testInvalidRedirectUri(): void { $result = $this->settingsController->addClient('test', 'invalidurl'); $this->assertEquals(Http::STATUS_BAD_REQUEST, $result->getStatus()); |