From 1c2969a4be208e648984651490d65aa24c59957f Mon Sep 17 00:00:00 2001 From: ErickSkrauch Date: Wed, 15 Jan 2025 14:13:08 +0100 Subject: [PATCH] Implemented desktop application type --- .../oauth/controllers/ClientsController.php | 16 ++++-- .../oauth/models/BaseOauthClientType.php | 4 +- .../oauth/models/DesktopApplicationType.php | 24 +++++++++ .../oauth/models/OauthClientFormFactory.php | 13 +++-- ...icationType.php => WebApplicationType.php} | 9 ++-- api/tests/_pages/OauthRoute.php | 54 ------------------- .../dev/applications/CreateClientCest.php | 42 ++++++++++----- .../dev/applications/DeleteClientCest.php | 11 +--- .../dev/applications/GetClientsCest.php | 19 +++---- .../dev/applications/ResetClientCest.php | 13 ++--- .../dev/applications/UpdateClientCest.php | 15 ++---- .../oauth/models/BaseOauthClientTypeTest.php | 2 +- .../oauth/models/MinecraftServerTypeTest.php | 4 +- .../models/OauthClientFormFactoryTest.php | 31 ++++++++--- .../oauth/models/OauthClientFormTest.php | 12 ++--- ...ypeTest.php => WebApplicationTypeTest.php} | 8 +-- .../OAuth2/AuthorizationServerFactory.php | 1 - .../OAuth2/Entities/ClientEntity.php | 18 ++++--- .../Grants/ValidateRedirectUriTrait.php | 30 +++++++++-- .../OAuth2/Repositories/ClientRepository.php | 11 ++-- common/models/OauthClient.php | 15 +++--- phpstan-baseline.neon | 45 ---------------- 22 files changed, 183 insertions(+), 214 deletions(-) create mode 100644 api/modules/oauth/models/DesktopApplicationType.php rename api/modules/oauth/models/{ApplicationType.php => WebApplicationType.php} (80%) delete mode 100644 api/tests/_pages/OauthRoute.php rename api/tests/unit/modules/oauth/models/{ApplicationTypeTest.php => WebApplicationTypeTest.php} (81%) diff --git a/api/modules/oauth/controllers/ClientsController.php b/api/modules/oauth/controllers/ClientsController.php index f556d5f..cd165f7 100644 --- a/api/modules/oauth/controllers/ClientsController.php +++ b/api/modules/oauth/controllers/ClientsController.php @@ -94,7 +94,7 @@ class ClientsController extends Controller { $client = new OauthClient(); $client->account_id = $account->id; - $client->type = $type; + $client->type = $type; // @phpstan-ignore assign.propertyType (this value will be validated in the createForm()) $requestModel = $this->createForm($client); $requestModel->load(Yii::$app->request->post()); $form = new OauthClientForm($client); @@ -163,7 +163,7 @@ class ClientsController extends Controller { /** @var \common\models\OauthSession[] $oauthSessions */ $oauthSessions = $account->getOauthSessions() ->innerJoinWith(['client c' => function(ActiveQuery $query): void { - $query->andOnCondition(['c.type' => OauthClient::TYPE_APPLICATION]); + $query->andOnCondition(['c.type' => OauthClient::TYPE_WEB_APPLICATION]); }]) ->andWhere([ 'OR', @@ -206,10 +206,12 @@ class ClientsController extends Controller { return ['success' => true]; } + /** + * @return array + */ private function formatClient(OauthClient $client): array { $result = [ 'clientId' => $client->id, - 'clientSecret' => $client->secret, 'type' => $client->type, 'name' => $client->name, 'websiteUrl' => $client->website_url, @@ -217,12 +219,18 @@ class ClientsController extends Controller { ]; switch ($client->type) { - case OauthClient::TYPE_APPLICATION: + case OauthClient::TYPE_WEB_APPLICATION: + $result['clientSecret'] = $client->secret; $result['description'] = $client->description; $result['redirectUri'] = $client->redirect_uri; $result['countUsers'] = (int)$client->getSessions()->count(); break; + case OauthClient::TYPE_DESKTOP_APPLICATION: + $result['description'] = $client->description; + $result['countUsers'] = (int)$client->getSessions()->count(); + break; case OauthClient::TYPE_MINECRAFT_SERVER: + $result['clientSecret'] = $client->secret; $result['minecraftServerIp'] = $client->minecraft_server_ip; break; } diff --git a/api/modules/oauth/models/BaseOauthClientType.php b/api/modules/oauth/models/BaseOauthClientType.php index 72a97ec..8445b59 100644 --- a/api/modules/oauth/models/BaseOauthClientType.php +++ b/api/modules/oauth/models/BaseOauthClientType.php @@ -9,9 +9,9 @@ use common\models\OauthClient; abstract class BaseOauthClientType extends ApiForm implements OauthClientTypeForm { - public $name; + public mixed $name = null; - public $websiteUrl; + public mixed $websiteUrl = null; public function rules(): array { return [ diff --git a/api/modules/oauth/models/DesktopApplicationType.php b/api/modules/oauth/models/DesktopApplicationType.php new file mode 100644 index 0000000..427690f --- /dev/null +++ b/api/modules/oauth/models/DesktopApplicationType.php @@ -0,0 +1,24 @@ +description = $this->description; + } + +} diff --git a/api/modules/oauth/models/OauthClientFormFactory.php b/api/modules/oauth/models/OauthClientFormFactory.php index 7b3eb92..459bb61 100644 --- a/api/modules/oauth/models/OauthClientFormFactory.php +++ b/api/modules/oauth/models/OauthClientFormFactory.php @@ -6,27 +6,30 @@ namespace api\modules\oauth\models; use api\modules\oauth\exceptions\UnsupportedOauthClientType; use common\models\OauthClient; -class OauthClientFormFactory { +final class OauthClientFormFactory { /** - * @param OauthClient $client - * - * @return OauthClientTypeForm * @throws UnsupportedOauthClientType */ public static function create(OauthClient $client): OauthClientTypeForm { return match ($client->type) { - OauthClient::TYPE_APPLICATION => new ApplicationType([ + OauthClient::TYPE_WEB_APPLICATION => new WebApplicationType([ 'name' => $client->name, 'websiteUrl' => $client->website_url, 'description' => $client->description, 'redirectUri' => $client->redirect_uri, ]), + OauthClient::TYPE_DESKTOP_APPLICATION => new DesktopApplicationType([ + 'name' => $client->name, + 'description' => $client->description, + 'websiteUrl' => $client->website_url, + ]), OauthClient::TYPE_MINECRAFT_SERVER => new MinecraftServerType([ 'name' => $client->name, 'websiteUrl' => $client->website_url, 'minecraftServerIp' => $client->minecraft_server_ip, ]), + // @phpstan-ignore match.unreachable (Not quite correct code, but the value comes from the user and might be not expected) default => throw new UnsupportedOauthClientType($client->type), }; } diff --git a/api/modules/oauth/models/ApplicationType.php b/api/modules/oauth/models/WebApplicationType.php similarity index 80% rename from api/modules/oauth/models/ApplicationType.php rename to api/modules/oauth/models/WebApplicationType.php index e53fd6a..9767a86 100644 --- a/api/modules/oauth/models/ApplicationType.php +++ b/api/modules/oauth/models/WebApplicationType.php @@ -3,21 +3,20 @@ declare(strict_types=1); namespace api\modules\oauth\models; -use Closure; use common\helpers\Error as E; use common\models\OauthClient; use yii\helpers\ArrayHelper; -final class ApplicationType extends BaseOauthClientType { +final class WebApplicationType extends BaseOauthClientType { - public $description; + public mixed $description = null; - public $redirectUri; + public mixed $redirectUri = null; public function rules(): array { return ArrayHelper::merge(parent::rules(), [ ['redirectUri', 'required', 'message' => E::REDIRECT_URI_REQUIRED], - ['redirectUri', Closure::fromCallable([$this, 'validateUrl'])], + ['redirectUri', $this->validateUrl(...)], ['description', 'string'], ]); } diff --git a/api/tests/_pages/OauthRoute.php b/api/tests/_pages/OauthRoute.php deleted file mode 100644 index 564d7d6..0000000 --- a/api/tests/_pages/OauthRoute.php +++ /dev/null @@ -1,54 +0,0 @@ -getActor()->sendPOST('/api/v1/oauth2/' . $type, $postParams); - } - - /** - * @deprecated - */ - public function updateClient(string $clientId, array $params): void { - $this->getActor()->sendPUT('/api/v1/oauth2/' . $clientId, $params); - } - - /** - * @deprecated - */ - public function deleteClient(string $clientId): void { - $this->getActor()->sendDELETE('/api/v1/oauth2/' . $clientId); - } - - /** - * @deprecated - */ - public function resetClient(string $clientId, bool $regenerateSecret = false): void { - $this->getActor()->sendPOST("/api/v1/oauth2/{$clientId}/reset" . ($regenerateSecret ? '?regenerateSecret' : '')); - } - - /** - * @deprecated - */ - public function getClient(string $clientId): void { - $this->getActor()->sendGET("/api/v1/oauth2/{$clientId}"); - } - - /** - * @deprecated - */ - public function getPerAccount(int $accountId): void { - $this->getActor()->sendGET("/api/v1/accounts/{$accountId}/oauth2/clients"); - } - -} diff --git a/api/tests/functional/dev/applications/CreateClientCest.php b/api/tests/functional/dev/applications/CreateClientCest.php index c6d1268..2b071fe 100644 --- a/api/tests/functional/dev/applications/CreateClientCest.php +++ b/api/tests/functional/dev/applications/CreateClientCest.php @@ -3,20 +3,13 @@ declare(strict_types=1); namespace api\tests\functional\dev\applications; -use api\tests\_pages\OauthRoute; use api\tests\FunctionalTester; final class CreateClientCest { - private OauthRoute $route; - - public function _before(FunctionalTester $I): void { - $this->route = new OauthRoute($I); - } - - public function testCreateApplication(FunctionalTester $I): void { + public function testCreateWebApplication(FunctionalTester $I): void { $I->amAuthenticated('admin'); - $this->route->createClient('application', [ + $I->sendPOST('/api/v1/oauth2/application', [ 'name' => 'My admin application', 'description' => 'Application description.', 'redirectUri' => 'http://some-site.com/oauth/ely', @@ -39,9 +32,33 @@ final class CreateClientCest { $I->canSeeResponseJsonMatchesJsonPath('$.data.createdAt'); } + public function testCreateDesktopApplication(FunctionalTester $I): void { + $I->amAuthenticated('admin'); + $I->sendPOST('/api/v1/oauth2/desktop-application', [ + 'name' => 'Mega Launcher', + 'description' => "Launcher's description.", + 'websiteUrl' => 'http://mega-launcher.com', + ]); + $I->canSeeResponseCodeIs(200); + $I->canSeeResponseIsJson(); + $I->canSeeResponseContainsJson([ + 'success' => true, + 'data' => [ + 'clientId' => 'mega-launcher', + 'type' => 'desktop-application', + 'name' => 'Mega Launcher', + 'description' => "Launcher's description.", + 'websiteUrl' => 'http://mega-launcher.com', + 'countUsers' => 0, + ], + ]); + $I->cantSeeResponseJsonMatchesJsonPath('$.data.clientSecret'); + $I->canSeeResponseJsonMatchesJsonPath('$.data.createdAt'); + } + public function testCreateMinecraftServer(FunctionalTester $I): void { $I->amAuthenticated('admin'); - $this->route->createClient('minecraft-server', [ + $I->sendPOST('/api/v1/oauth2/minecraft-server', [ 'name' => 'My amazing server', 'websiteUrl' => 'http://some-site.com', 'minecraftServerIp' => 'hypixel.com:25565', @@ -64,7 +81,7 @@ final class CreateClientCest { public function testCreateApplicationWithTheSameNameAsDeletedApp(FunctionalTester $I): void { $I->wantTo('create application with the same name as the recently deleted application'); $I->amAuthenticated('admin'); - $this->route->createClient('application', [ + $I->sendPOST('/api/v1/oauth2/application', [ 'name' => 'Deleted OAuth Client', 'description' => '', 'redirectUri' => 'http://some-site.com/oauth/ely', @@ -82,8 +99,7 @@ final class CreateClientCest { public function testCreateApplicationWithWrongParams(FunctionalTester $I): void { $I->amAuthenticated('admin'); - - $this->route->createClient('application', []); + $I->sendPOST('/api/v1/oauth2/application', []); $I->canSeeResponseCodeIs(200); $I->canSeeResponseContainsJson([ 'success' => false, diff --git a/api/tests/functional/dev/applications/DeleteClientCest.php b/api/tests/functional/dev/applications/DeleteClientCest.php index 0924afa..4040d08 100644 --- a/api/tests/functional/dev/applications/DeleteClientCest.php +++ b/api/tests/functional/dev/applications/DeleteClientCest.php @@ -3,20 +3,13 @@ declare(strict_types=1); namespace api\tests\functional\dev\applications; -use api\tests\_pages\OauthRoute; use api\tests\FunctionalTester; -class DeleteClientCest { - - private OauthRoute $route; - - public function _before(FunctionalTester $I): void { - $this->route = new OauthRoute($I); - } +final class DeleteClientCest { public function testDelete(FunctionalTester $I): void { $I->amAuthenticated('TwoOauthClients'); - $this->route->deleteClient('first-test-oauth-client'); + $I->sendDELETE('/api/v1/oauth2/first-test-oauth-client'); $I->canSeeResponseCodeIs(200); $I->canSeeResponseIsJson(); $I->canSeeResponseContainsJson([ diff --git a/api/tests/functional/dev/applications/GetClientsCest.php b/api/tests/functional/dev/applications/GetClientsCest.php index c2ca7f9..b5c32d1 100644 --- a/api/tests/functional/dev/applications/GetClientsCest.php +++ b/api/tests/functional/dev/applications/GetClientsCest.php @@ -3,20 +3,13 @@ declare(strict_types=1); namespace api\tests\functional\dev\applications; -use api\tests\_pages\OauthRoute; use api\tests\FunctionalTester; -class GetClientsCest { - - private OauthRoute $route; - - public function _before(FunctionalTester $I): void { - $this->route = new OauthRoute($I); - } +final class GetClientsCest { public function testGet(FunctionalTester $I): void { $I->amAuthenticated('admin'); - $this->route->getClient('admin-oauth-client'); + $I->sendGET('/api/v1/oauth2/admin-oauth-client'); $I->canSeeResponseCodeIs(200); $I->canSeeResponseIsJson(); $I->canSeeResponseContainsJson([ @@ -33,7 +26,7 @@ class GetClientsCest { public function testGetNotOwn(FunctionalTester $I): void { $I->amAuthenticated('admin'); - $this->route->getClient('another-test-oauth-client'); + $I->sendGET('/api/v1/oauth2/another-test-oauth-client'); $I->canSeeResponseCodeIs(403); $I->canSeeResponseIsJson(); $I->canSeeResponseContainsJson([ @@ -44,8 +37,8 @@ class GetClientsCest { } public function testGetAllPerAccountList(FunctionalTester $I): void { - $I->amAuthenticated('TwoOauthClients'); - $this->route->getPerAccount(14); + $accountId = $I->amAuthenticated('TwoOauthClients'); + $I->sendGET("/api/v1/accounts/{$accountId}/oauth2/clients"); $I->canSeeResponseCodeIs(200); $I->canSeeResponseIsJson(); $I->canSeeResponseContainsJson([ @@ -74,7 +67,7 @@ class GetClientsCest { public function testGetAllPerNotOwnAccount(FunctionalTester $I): void { $I->amAuthenticated('TwoOauthClients'); - $this->route->getPerAccount(1); + $I->sendGET('/api/v1/accounts/1/oauth2/clients'); $I->canSeeResponseCodeIs(403); $I->canSeeResponseIsJson(); $I->canSeeResponseContainsJson([ diff --git a/api/tests/functional/dev/applications/ResetClientCest.php b/api/tests/functional/dev/applications/ResetClientCest.php index b7741d6..094f82e 100644 --- a/api/tests/functional/dev/applications/ResetClientCest.php +++ b/api/tests/functional/dev/applications/ResetClientCest.php @@ -3,20 +3,13 @@ declare(strict_types=1); namespace api\tests\functional\dev\applications; -use api\tests\_pages\OauthRoute; use api\tests\FunctionalTester; -class ResetClientCest { - - private OauthRoute $route; - - public function _before(FunctionalTester $I): void { - $this->route = new OauthRoute($I); - } +final class ResetClientCest { public function testReset(FunctionalTester $I): void { $I->amAuthenticated('TwoOauthClients'); - $this->route->resetClient('first-test-oauth-client'); + $I->sendPOST('/api/v1/oauth2/first-test-oauth-client/reset'); $I->canSeeResponseCodeIs(200); $I->canSeeResponseIsJson(); $I->canSeeResponseContainsJson([ @@ -36,7 +29,7 @@ class ResetClientCest { public function testResetWithSecretChanging(FunctionalTester $I): void { $I->amAuthenticated('TwoOauthClients'); - $this->route->resetClient('first-test-oauth-client', true); + $I->sendPOST('/api/v1/oauth2/first-test-oauth-client/reset?regenerateSecret'); $I->canSeeResponseCodeIs(200); $I->canSeeResponseIsJson(); $I->canSeeResponseContainsJson([ diff --git a/api/tests/functional/dev/applications/UpdateClientCest.php b/api/tests/functional/dev/applications/UpdateClientCest.php index a259b55..7e63149 100644 --- a/api/tests/functional/dev/applications/UpdateClientCest.php +++ b/api/tests/functional/dev/applications/UpdateClientCest.php @@ -3,20 +3,13 @@ declare(strict_types=1); namespace api\tests\functional\dev\applications; -use api\tests\_pages\OauthRoute; use api\tests\FunctionalTester; -class UpdateClientCest { +final class UpdateClientCest { - private OauthRoute $route; - - public function _before(FunctionalTester $I): void { - $this->route = new OauthRoute($I); - } - - public function testUpdateApplication(FunctionalTester $I): void { + public function testUpdateWebApplication(FunctionalTester $I): void { $I->amAuthenticated('TwoOauthClients'); - $this->route->updateClient('first-test-oauth-client', [ + $I->sendPUT('/api/v1/oauth2/first-test-oauth-client', [ 'name' => 'Updated name', 'description' => 'Updated description.', 'redirectUri' => 'http://new-site.com/oauth/ely', @@ -41,7 +34,7 @@ class UpdateClientCest { public function testUpdateMinecraftServer(FunctionalTester $I): void { $I->amAuthenticated('TwoOauthClients'); - $this->route->updateClient('another-test-oauth-client', [ + $I->sendPUT('/api/v1/oauth2/another-test-oauth-client', [ 'name' => 'Updated server name', 'websiteUrl' => 'http://new-site.com', 'minecraftServerIp' => 'hypixel.com:25565', diff --git a/api/tests/unit/modules/oauth/models/BaseOauthClientTypeTest.php b/api/tests/unit/modules/oauth/models/BaseOauthClientTypeTest.php index 1293e59..cb57bc6 100644 --- a/api/tests/unit/modules/oauth/models/BaseOauthClientTypeTest.php +++ b/api/tests/unit/modules/oauth/models/BaseOauthClientTypeTest.php @@ -7,7 +7,7 @@ use api\modules\oauth\models\BaseOauthClientType; use api\tests\unit\TestCase; use common\models\OauthClient; -class BaseOauthClientTypeTest extends TestCase { +final class BaseOauthClientTypeTest extends TestCase { public function testApplyTyClient(): void { $client = new OauthClient(); diff --git a/api/tests/unit/modules/oauth/models/MinecraftServerTypeTest.php b/api/tests/unit/modules/oauth/models/MinecraftServerTypeTest.php index 0d69718..7e44c05 100644 --- a/api/tests/unit/modules/oauth/models/MinecraftServerTypeTest.php +++ b/api/tests/unit/modules/oauth/models/MinecraftServerTypeTest.php @@ -1,11 +1,13 @@ type = OauthClient::TYPE_APPLICATION; + $client->type = OauthClient::TYPE_WEB_APPLICATION; $client->name = 'Application name'; $client->description = 'Application description.'; $client->website_url = 'http://example.com'; $client->redirect_uri = 'http://example.com/oauth/ely'; - /** @var ApplicationType $requestForm */ + /** @var WebApplicationType $requestForm */ $requestForm = OauthClientFormFactory::create($client); - $this->assertInstanceOf(ApplicationType::class, $requestForm); + $this->assertInstanceOf(WebApplicationType::class, $requestForm); $this->assertSame('Application name', $requestForm->name); $this->assertSame('Application description.', $requestForm->description); $this->assertSame('http://example.com', $requestForm->websiteUrl); $this->assertSame('http://example.com/oauth/ely', $requestForm->redirectUri); + } + public function testCreateDesktopApplication(): void { + $client = new OauthClient(); + $client->type = OauthClient::TYPE_DESKTOP_APPLICATION; + $client->name = 'Application name'; + $client->description = 'Application description.'; + $client->website_url = 'http://example.com'; + /** @var \api\modules\oauth\models\DesktopApplicationType $requestForm */ + $requestForm = OauthClientFormFactory::create($client); + $this->assertInstanceOf(DesktopApplicationType::class, $requestForm); + $this->assertSame('Application name', $requestForm->name); + $this->assertSame('Application description.', $requestForm->description); + $this->assertSame('http://example.com', $requestForm->websiteUrl); + } + + public function testCreateMinecraftServer(): void { $client = new OauthClient(); $client->type = OauthClient::TYPE_MINECRAFT_SERVER; $client->name = 'Server name'; @@ -44,7 +61,7 @@ class OauthClientFormFactoryTest extends TestCase { $this->expectException(UnsupportedOauthClientType::class); $client = new OauthClient(); - $client->type = 'unknown-type'; + $client->type = 'unknown-type'; // @phpstan-ignore assign.propertyType (its alright for tests) OauthClientFormFactory::create($client); } diff --git a/api/tests/unit/modules/oauth/models/OauthClientFormTest.php b/api/tests/unit/modules/oauth/models/OauthClientFormTest.php index 814dcb5..50dcb39 100644 --- a/api/tests/unit/modules/oauth/models/OauthClientFormTest.php +++ b/api/tests/unit/modules/oauth/models/OauthClientFormTest.php @@ -9,13 +9,13 @@ use api\tests\unit\TestCase; use common\models\OauthClient; use common\tasks\ClearOauthSessions; -class OauthClientFormTest extends TestCase { +final class OauthClientFormTest extends TestCase { public function testSave(): void { $client = $this->createPartialMock(OauthClient::class, ['save']); $client->method('save')->willReturn(true); $client->account_id = 1; - $client->type = OauthClient::TYPE_APPLICATION; + $client->type = OauthClient::TYPE_WEB_APPLICATION; $client->name = 'Test application'; $form = $this->createPartialMock(OauthClientForm::class, ['getClient', 'isClientExists']); @@ -39,7 +39,7 @@ class OauthClientFormTest extends TestCase { $client->id = 'application-id'; $client->secret = 'application_secret'; $client->account_id = 1; - $client->type = OauthClient::TYPE_APPLICATION; + $client->type = OauthClient::TYPE_WEB_APPLICATION; $client->name = 'Application name'; $client->description = 'Application description'; $client->redirect_uri = 'http://example.com/oauth/ely'; @@ -81,7 +81,7 @@ class OauthClientFormTest extends TestCase { $client = $this->createPartialMock(OauthClient::class, ['save']); $client->method('save')->willReturn(true); $client->id = 'mocked-id'; - $client->type = OauthClient::TYPE_APPLICATION; + $client->type = OauthClient::TYPE_WEB_APPLICATION; $form = new OauthClientForm($client); $this->assertTrue($form->delete()); @@ -98,7 +98,7 @@ class OauthClientFormTest extends TestCase { $client->method('save')->willReturn(true); $client->id = 'mocked-id'; $client->secret = 'initial_secret'; - $client->type = OauthClient::TYPE_APPLICATION; + $client->type = OauthClient::TYPE_WEB_APPLICATION; $form = new OauthClientForm($client); $this->assertTrue($form->reset()); @@ -115,7 +115,7 @@ class OauthClientFormTest extends TestCase { $client->method('save')->willReturn(true); $client->id = 'mocked-id'; $client->secret = 'initial_secret'; - $client->type = OauthClient::TYPE_APPLICATION; + $client->type = OauthClient::TYPE_WEB_APPLICATION; $form = new OauthClientForm($client); $this->assertTrue($form->reset(true)); diff --git a/api/tests/unit/modules/oauth/models/ApplicationTypeTest.php b/api/tests/unit/modules/oauth/models/WebApplicationTypeTest.php similarity index 81% rename from api/tests/unit/modules/oauth/models/ApplicationTypeTest.php rename to api/tests/unit/modules/oauth/models/WebApplicationTypeTest.php index 6d0c8b0..c3bae1e 100644 --- a/api/tests/unit/modules/oauth/models/ApplicationTypeTest.php +++ b/api/tests/unit/modules/oauth/models/WebApplicationTypeTest.php @@ -1,14 +1,16 @@ name = 'Application name'; $model->websiteUrl = 'http://example.com'; $model->redirectUri = 'http://example.com/oauth/ely'; diff --git a/common/components/OAuth2/AuthorizationServerFactory.php b/common/components/OAuth2/AuthorizationServerFactory.php index 06750e9..e0fa71e 100644 --- a/common/components/OAuth2/AuthorizationServerFactory.php +++ b/common/components/OAuth2/AuthorizationServerFactory.php @@ -31,7 +31,6 @@ final class AuthorizationServerFactory { ); /** @noinspection PhpUnhandledExceptionInspection */ $authCodeGrant = new Grants\AuthCodeGrant($authCodesRepo, $refreshTokensRepo, new DateInterval('PT10M')); - $authCodeGrant->disableRequireCodeChallengeForPublicClients(); $authServer->enableGrantType($authCodeGrant, $accessTokenTTL); $authCodeGrant->setScopeRepository($publicScopesRepo); // Change repository after enabling diff --git a/common/components/OAuth2/Entities/ClientEntity.php b/common/components/OAuth2/Entities/ClientEntity.php index 8a53a8b..48be814 100644 --- a/common/components/OAuth2/Entities/ClientEntity.php +++ b/common/components/OAuth2/Entities/ClientEntity.php @@ -20,7 +20,8 @@ final class ClientEntity implements ClientEntityInterface { string $id, string $name, string|array $redirectUri, - private readonly bool $isTrusted, + private readonly bool $isTrusted = false, + public readonly ?OauthClient $model = null, ) { $this->identifier = $id; $this->name = $name; @@ -29,15 +30,20 @@ final class ClientEntity implements ClientEntityInterface { public static function fromModel(OauthClient $model): self { return new self( - $model->id, // @phpstan-ignore argument.type - $model->name, - $model->redirect_uri ?: '', - (bool)$model->is_trusted, + id: $model->id, // @phpstan-ignore argument.type + name: $model->name, + redirectUri: $model->redirect_uri ?: '', + isTrusted: (bool)$model->is_trusted, + model: $model, ); } public function isConfidential(): bool { - return true; + return match ($this->model->type) { + OauthClient::TYPE_WEB_APPLICATION => true, + OauthClient::TYPE_DESKTOP_APPLICATION => false, + OauthClient::TYPE_MINECRAFT_SERVER => true, + }; } public function isTrusted(): bool { diff --git a/common/components/OAuth2/Grants/ValidateRedirectUriTrait.php b/common/components/OAuth2/Grants/ValidateRedirectUriTrait.php index d9e7dd3..2047ffa 100644 --- a/common/components/OAuth2/Grants/ValidateRedirectUriTrait.php +++ b/common/components/OAuth2/Grants/ValidateRedirectUriTrait.php @@ -3,6 +3,8 @@ declare(strict_types=1); namespace common\components\OAuth2\Grants; +use common\components\OAuth2\Entities\ClientEntity; +use common\models\OauthClient; use League\OAuth2\Server\Entities\ClientEntityInterface; use League\OAuth2\Server\EventEmitting\EventEmitter; use League\OAuth2\Server\Exception\OAuthServerException; @@ -14,15 +16,35 @@ trait ValidateRedirectUriTrait { abstract public function getEmitter(): EventEmitter; + /** + * Override the original method since we need a custom validation logic based on the client type. + * @inheritDoc + */ protected function validateRedirectUri( string $redirectUri, ClientEntityInterface $client, ServerRequestInterface $request, ): void { - $allowedRedirectUris = (array)$client->getRedirectUri(); - foreach ($allowedRedirectUris as $allowedRedirectUri) { - if (StringHelper::startsWith($redirectUri, $allowedRedirectUri)) { - return; + if ($client instanceof ClientEntity && $client->model?->type === OauthClient::TYPE_DESKTOP_APPLICATION) { + $uri = parse_url($redirectUri); + if ($uri) { + // Allow any custom scheme, that is not http + if ($uri['scheme'] !== 'http' && $uri['scheme'] !== 'https') { + return; + } + + // If it's a http, than should allow only redirection to the local machine + if (in_array($uri['host'], ['localhost', '127.0.0.1', '[::1]'])) { + return; + } + } + } else { + // The original implementation checks url too strictly (port and path must exactly match). + // It's nice to have, but we made it this way earlier and so we must keep the same behavior as long as possible + foreach ((array)$client->getRedirectUri() as $allowedRedirectUri) { + if (StringHelper::startsWith($redirectUri, $allowedRedirectUri)) { + return; + } } } diff --git a/common/components/OAuth2/Repositories/ClientRepository.php b/common/components/OAuth2/Repositories/ClientRepository.php index e791c62..0709225 100644 --- a/common/components/OAuth2/Repositories/ClientRepository.php +++ b/common/components/OAuth2/Repositories/ClientRepository.php @@ -25,11 +25,11 @@ final class ClientRepository implements ClientRepositoryInterface { return false; } - if ($client->type !== OauthClient::TYPE_APPLICATION) { + if (!in_array($client->type, [OauthClient::TYPE_WEB_APPLICATION, OauthClient::TYPE_DESKTOP_APPLICATION], true)) { return false; } - if (!empty($clientSecret) && $clientSecret !== $client->secret) { + if ($client->type === OauthClient::TYPE_WEB_APPLICATION && !empty($clientSecret) && $clientSecret !== $client->secret) { return false; } @@ -37,12 +37,7 @@ final class ClientRepository implements ClientRepositoryInterface { } private function findModel(string $id): ?OauthClient { - $client = OauthClient::findOne(['id' => $id]); - if ($client === null || $client->type !== OauthClient::TYPE_APPLICATION) { - return null; - } - - return $client; + return OauthClient::findOne(['id' => $id]); } } diff --git a/common/models/OauthClient.php b/common/models/OauthClient.php index 260272f..15ac08b 100644 --- a/common/models/OauthClient.php +++ b/common/models/OauthClient.php @@ -12,7 +12,7 @@ use yii\db\ActiveRecord; * Fields: * @property string $id * @property string $secret - * @property string $type + * @property self::TYPE_* $type * @property string $name * @property string $description * @property string|null $redirect_uri @@ -29,14 +29,14 @@ use yii\db\ActiveRecord; */ class OauthClient extends ActiveRecord { - public const TYPE_APPLICATION = 'application'; - public const TYPE_MINECRAFT_SERVER = 'minecraft-server'; - public const TYPE_MINECRAFT_GAME_LAUNCHER = 'minecraft-game-launcher'; + public const string TYPE_WEB_APPLICATION = 'application'; + public const string TYPE_DESKTOP_APPLICATION = 'desktop-application'; + public const string TYPE_MINECRAFT_SERVER = 'minecraft-server'; /** * Abstract oauth_client, used to */ - public const UNAUTHORIZED_MINECRAFT_GAME_LAUNCHER = 'unauthorized_minecraft_game_launcher'; + public const string UNAUTHORIZED_MINECRAFT_GAME_LAUNCHER = 'unauthorized_minecraft_game_launcher'; public static function tableName(): string { return 'oauth_clients'; @@ -55,10 +55,13 @@ class OauthClient extends ActiveRecord { $this->secret = Yii::$app->security->generateRandomString(64); } - public function getAccount(): ActiveQuery { + public function getAccount(): AccountQuery { return $this->hasOne(Account::class, ['id' => 'account_id']); } + /** + * @return \yii\db\ActiveQuery<\common\models\OauthSession> + */ public function getSessions(): ActiveQuery { return $this->hasMany(OauthSession::class, ['client_id' => 'id']); } diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 2842a92..da8516c 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -475,21 +475,6 @@ parameters: count: 1 path: api/modules/oauth/controllers/AuthorizationController.php - - - message: "#^Method api\\\\modules\\\\oauth\\\\controllers\\\\ClientsController\\:\\:formatClient\\(\\) return type has no value type specified in iterable type array\\.$#" - count: 1 - path: api/modules/oauth/controllers/ClientsController.php - - - - message: "#^Property api\\\\modules\\\\oauth\\\\models\\\\ApplicationType\\:\\:\\$description has no type specified\\.$#" - count: 1 - path: api/modules/oauth/models/ApplicationType.php - - - - message: "#^Property api\\\\modules\\\\oauth\\\\models\\\\ApplicationType\\:\\:\\$redirectUri has no type specified\\.$#" - count: 1 - path: api/modules/oauth/models/ApplicationType.php - - message: "#^Method api\\\\modules\\\\oauth\\\\models\\\\BaseOauthClientType\\:\\:getValidationErrors\\(\\) return type has no value type specified in iterable type array\\.$#" count: 1 @@ -500,16 +485,6 @@ parameters: count: 1 path: api/modules/oauth/models/BaseOauthClientType.php - - - message: "#^Property api\\\\modules\\\\oauth\\\\models\\\\BaseOauthClientType\\:\\:\\$name has no type specified\\.$#" - count: 1 - path: api/modules/oauth/models/BaseOauthClientType.php - - - - message: "#^Property api\\\\modules\\\\oauth\\\\models\\\\BaseOauthClientType\\:\\:\\$websiteUrl has no type specified\\.$#" - count: 1 - path: api/modules/oauth/models/BaseOauthClientType.php - - message: "#^Method api\\\\modules\\\\oauth\\\\models\\\\IdentityInfo\\:\\:info\\(\\) return type has no value type specified in iterable type array\\.$#" count: 1 @@ -720,16 +695,6 @@ parameters: count: 1 path: api/tests/_pages/MojangApiRoute.php - - - message: "#^Method api\\\\tests\\\\_pages\\\\OauthRoute\\:\\:createClient\\(\\) has parameter \\$postParams with no value type specified in iterable type array\\.$#" - count: 1 - path: api/tests/_pages/OauthRoute.php - - - - message: "#^Method api\\\\tests\\\\_pages\\\\OauthRoute\\:\\:updateClient\\(\\) has parameter \\$params with no value type specified in iterable type array\\.$#" - count: 1 - path: api/tests/_pages/OauthRoute.php - - message: "#^Method api\\\\tests\\\\_pages\\\\SessionServerRoute\\:\\:hasJoined\\(\\) has parameter \\$params with no value type specified in iterable type array\\.$#" count: 1 @@ -1350,16 +1315,6 @@ parameters: count: 1 path: common/models/EmailActivation.php - - - message: "#^Method common\\\\models\\\\OauthClient\\:\\:getAccount\\(\\) return type with generic class yii\\\\db\\\\ActiveQuery does not specify its types\\: T$#" - count: 1 - path: common/models/OauthClient.php - - - - message: "#^Method common\\\\models\\\\OauthClient\\:\\:getSessions\\(\\) return type with generic class yii\\\\db\\\\ActiveQuery does not specify its types\\: T$#" - count: 1 - path: common/models/OauthClient.php - - message: "#^Class common\\\\models\\\\OauthClientQuery extends generic class yii\\\\db\\\\ActiveQuery but does not specify its types\\: T$#" count: 1