From f97bdf9542e4f716c28904a309ed9f15d5f90d35 Mon Sep 17 00:00:00 2001 From: Ransom Roberson Date: Fri, 12 Aug 2022 09:56:24 -0400 Subject: [PATCH 01/11] Initial Craft 4 refactor (untested) --- CHANGELOG.md | 10 ++- README.md | 6 +- composer.json | 9 ++- src/Plugin.php | 84 ++++++++++------------ src/console/controllers/AppsController.php | 4 +- src/controllers/AppsController.php | 4 +- src/controllers/AuthorizeController.php | 10 +-- src/models/App.php | 8 +-- src/models/Token.php | 4 +- src/services/Credentials.php | 6 +- src/variables/OAuthVariable.php | 2 +- 11 files changed, 71 insertions(+), 76 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4102fdd..c7b6db9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,8 +1,14 @@ # OAuth 2.0 Client Changelog -All notable changes to this project will be documented in this file. +## 4.0.0-beta.1 - Unreleased -The format is based on [Keep a Changelog](http://keepachangelog.com/) and this project adheres to [Semantic Versioning](http://semver.org/). +### Changed +- Now requires Craft 4 & PHP 8.0.2 + +### Removed +- Removed route `oauthclient/authorize/refresh/` use `oauth/authorize/refresh/` instead +- Removed route `oauthclient/authorize/` use `oauth/authorize/` instead +- Removed `Plugin::$plugin` static property - use `Plugin::getInstance()` instead ## 2.1.9 - 2021-03-30 ### Added diff --git a/README.md b/README.md index 96db73b..54580d7 100644 --- a/README.md +++ b/README.md @@ -1,4 +1,4 @@ -OAuth 2.0 Client plugin for Craft CMS 3 +OAuth 2.0 Client plugin for Craft CMS 4 === This plugin provides developers with an easy centralized approach to managing and storing OAuth 2.0 @@ -26,7 +26,7 @@ act as an authentication provider for users to login to the CMS. ## Requirements -This plugin should work on Craft CMS 3.1.34.3 or later +This plugin should work on Craft CMS 4.0.0 or later ## Installation @@ -222,7 +222,7 @@ use venveo\oauthclient\Plugin; // Get the plugin instance. Note: make sure you do this after the application has been inited, such as in a route or // event. -$plugin = Plugin::$plugin; +$plugin = Plugin::getInstance(); // Let's grab a valid token - we could pass the current user ID in here to limit it $tokens = $plugin->credentials->getValidTokensForAppAndUser('google'); // Get the app from the apps service diff --git a/composer.json b/composer.json index 9ca2a71..5fee260 100644 --- a/composer.json +++ b/composer.json @@ -2,7 +2,7 @@ "name": "venveo/craft-oauthclient", "description": "Simple OAuth 2.0 client", "type": "craft-plugin", - "version": "2.1.9", + "version": "4.0.0-beta.1", "keywords": [ "craft", "cms", @@ -22,7 +22,8 @@ } ], "require": { - "craftcms/cms": "^3.1.34.3", + "php": "^8.0.2", + "craftcms/cms": "^4.0", "league/oauth2-client": "^2.2.1", "league/oauth2-google": "^2.2 || ^3.0", "league/oauth2-facebook": "^2.0", @@ -34,11 +35,9 @@ } }, "extra": { - "hasCpSettings": true, - "hasCpSection": false, "name": "OAuth 2.0 Client", "handle": "oauthclient", - "changelogUrl": "https://raw.githubusercontent.com/venveo/craft-oauthclient/master/CHANGELOG.md", + "changelogUrl": "https://raw.githubusercontent.com/venveo/craft-oauthclient/main/CHANGELOG.md", "class": "venveo\\oauthclient\\Plugin" } } diff --git a/src/Plugin.php b/src/Plugin.php index 9560b09..b6ecf71 100644 --- a/src/Plugin.php +++ b/src/Plugin.php @@ -13,11 +13,13 @@ use craft\events\RegisterUrlRulesEvent; use craft\events\RegisterUserPermissionsEvent; use craft\helpers\UrlHelper; -use craft\log\FileTarget; +use craft\log\MonologTarget; use craft\services\ProjectConfig; use craft\services\UserPermissions; use craft\web\twig\variables\CraftVariable; use craft\web\UrlManager; +use Monolog\Formatter\LineFormatter; +use Psr\Log\LogLevel; use venveo\oauthclient\services\Apps as AppsService; use venveo\oauthclient\services\Credentials as CredentialsService; use venveo\oauthclient\services\Providers; @@ -42,29 +44,14 @@ */ class Plugin extends BasePlugin { - const HANDLE = 'oauthclient'; - - // Static Properties - // ========================================================================= - public static $PROJECT_CONFIG_KEY = 'oauthClient'; + public const HANDLE = 'oauthclient'; + public static string $PROJECT_CONFIG_KEY = 'oauthClient'; - /** - * @var Plugin - */ - public static $plugin; + public string $schemaVersion = '2.1.3'; + public bool $hasCpSettings = true; - // Public Properties - // ========================================================================= - - /** - * @var string - */ - public $schemaVersion = '2.1.3'; - public $hasCpSettings = true; - // Public Methods - // ========================================================================= /** * @inheritdoc @@ -72,14 +59,12 @@ class Plugin extends BasePlugin public function init() { parent::init(); - self::$plugin = $this; if (Craft::$app->request->getIsConsoleRequest()) { $this->controllerNamespace = 'venveo\oauthclient\console\controllers'; } $this->_registerLogger(); - $this->_setComponents(); $this->_registerCpRoutes(); $this->_registerVariables(); $this->_registerProjectConfig(); @@ -94,29 +79,36 @@ public function init() */ private function _registerLogger() { - Craft::getLogger()->dispatcher->targets[] = new FileTarget([ - 'logFile' => Craft::getAlias('@storage/logs/oauthclient.log'), - 'categories' => ['venveo\oauthclient\*'], + Craft::getLogger()->dispatcher->targets[] = new MonologTarget([ + 'name' => 'oauthclient', + 'categories' => ['venveo\oauthcleint\*'], + 'level' => LogLevel::INFO, + 'logContext' => false, + 'allowLineBreaks' => false, + 'formatter' => new LineFormatter( + format: "[%datetime%] %message%\n", + dateFormat: 'Y-m-d H:i:s', + ), ]); } - // Private Methods - // ========================================================================= /** - * Set our service components + * @inheritdoc */ - private function _setComponents() + public static function config(): array { - $this->setComponents([ - 'apps' => AppsService::class, - 'providers' => ProvidersService::class, - 'tokens' => TokensService::class, - 'credentials' => CredentialsService::class, - ]); - + return [ + 'components' => [ + 'apps' => ['class' => AppsService::class], + 'providers' => ['class' => ProvidersService::class], + 'tokens' => ['class' => TokensService::class], + 'credentials' => ['class' => CredentialsService::class] + ], + ]; } + /** * Adds the event handler for registering CP routes */ @@ -130,11 +122,6 @@ private function _registerCpRoutes() 'oauthclient/apps/' => 'oauthclient/apps/edit', 'oauthclient/apps/delete' => 'oauthclient/apps/delete', - // TODO: Remove these in next version in favor of `oauth` route - 'oauthclient/authorize/refresh/' => 'oauthclient/authorize/refresh', - 'oauthclient/authorize/' => 'oauthclient/authorize/authorize-app', - // These are duplicates of potentially non-admin-facing actions. Craft automatically checks routes for - // the plugin handle, so we needed new routes without the handle. 'oauth/authorize/refresh/' => 'oauthclient/authorize/refresh', 'oauth/authorize/' => 'oauthclient/authorize/authorize-app', ]); @@ -197,16 +184,19 @@ private function _handleProjectConfigRebuild(RebuildConfigEvent $e) private function _registerPermissions() { Event::on(UserPermissions::class, UserPermissions::EVENT_REGISTER_PERMISSIONS, function (RegisterUserPermissionsEvent $event) { - $apps = Plugin::$plugin->apps->getAllApps(); + $apps = $this->apps->getAllApps(); $loginPermissions = []; foreach ($apps as $app) { $suffix = ':' . $app->uid; $loginPermissions['oauthclient-login' . $suffix] = ['label' => self::t('Login to “{name}” ({handle}) app', ['name' => $app->name, 'handle' => $app->handle])]; } - $event->permissions[self::t('OAuth Client')] = [ - 'oauthclient-login' => [ - 'label' => self::t('Login to Apps'), 'nested' => $loginPermissions - ] + $event->permissions[] = [ + 'heading' => self::t('OAuth Client'), + 'permissions' => [ + 'oauthclient-login' => [ + 'label' => self::t('Login to Apps'), 'nested' => $loginPermissions + ] + ], ]; }); } @@ -222,7 +212,7 @@ public static function t($message, $params = [], $language = null) /** * @inheritdoc */ - public function getSettingsResponse() + public function getSettingsResponse(): mixed { return Craft::$app->getResponse()->redirect(UrlHelper::cpUrl('oauthclient/apps')); } diff --git a/src/console/controllers/AppsController.php b/src/console/controllers/AppsController.php index 74ab9b0..a310284 100644 --- a/src/console/controllers/AppsController.php +++ b/src/console/controllers/AppsController.php @@ -21,8 +21,8 @@ class AppsController extends Controller */ public function actionRefreshTokens($appHandle) { - $credentialService = Plugin::$plugin->credentials; - $appService = Plugin::$plugin->apps; + $credentialService = Plugin::getInstance()->credentials; + $appService = Plugin::getInstance()->apps; if (!$app = $appService->getAppByHandle($appHandle)) { $this->stderr('No app found with that handle' . PHP_EOL); return 1; diff --git a/src/controllers/AppsController.php b/src/controllers/AppsController.php index cafaf55..369dd7a 100644 --- a/src/controllers/AppsController.php +++ b/src/controllers/AppsController.php @@ -107,13 +107,13 @@ public function actionDelete() $request = Craft::$app->getRequest(); $id = $request->getRequiredBodyParam('id'); - $app = Plugin::$plugin->apps->getAppById($id); + $app = Plugin::getInstance()->apps->getAppById($id); if (!$app) { throw new NotFoundHttpException('App does not exist'); } - Plugin::$plugin->apps->deleteApp($app); + Plugin::getInstance()->apps->deleteApp($app); return $this->asJson(['success' => true]); } diff --git a/src/controllers/AuthorizeController.php b/src/controllers/AuthorizeController.php index c2a6082..8188df1 100644 --- a/src/controllers/AuthorizeController.php +++ b/src/controllers/AuthorizeController.php @@ -100,7 +100,7 @@ public function actionAuthorizeApp($handle): Response Craft::$app->session->set(self::REDIRECT_URL_SESSION_KEY, $returnUrl); } - $app = Plugin::$plugin->apps->getAppByHandle($event->appHandle); + $app = Plugin::getInstance()->apps->getAppByHandle($event->appHandle); if (!$app instanceof AppModel) { Craft::$app->response->setStatusCode(404, 'App handle does not exist'); return Craft::$app->response; @@ -123,7 +123,7 @@ public function actionAuthorizeApp($handle): Response $state = $this->getRandomState(); Craft::$app->session->set(self::STATE_SESSION_KEY, $state); Craft::$app->session->set(self::CONTEXT_SESSION_KEY, $event->context); - $url = Plugin::$plugin->apps->getAuthorizationUrlForApp($app, $state, $event->context); + $url = Plugin::getInstance()->apps->getAuthorizationUrlForApp($app, $state, $event->context); return Craft::$app->response->redirect($url); } @@ -146,7 +146,7 @@ public function actionAuthorizeApp($handle): Response $token->userId = Craft::$app->user->getId(); try { - $saved = Plugin::$plugin->tokens->saveToken($token); + $saved = Plugin::getInstance()->tokens->saveToken($token); if ($saved) { $event->token = $token; Craft::$app->session->setFlash(Craft::t('oauthclient', 'Connected via ' . $app->name)); @@ -185,7 +185,7 @@ protected function getRandomState($length = 32) */ public function actionRefresh($id) { - $token = Plugin::$plugin->tokens->getTokenById($id); + $token = Plugin::getInstance()->tokens->getTokenById($id); if (!$token) { return Craft::$app->response->setStatusCode(404, 'Token not found'); } @@ -193,7 +193,7 @@ public function actionRefresh($id) $app = $token->getApp(); $this->requirePermission('oauthclient-login:' . $app->uid); - $refreshed = Plugin::$plugin->credentials->refreshToken($token); + $refreshed = Plugin::getInstance()->credentials->refreshToken($token); if ($refreshed) { $app = $token->getApp(); return Craft::$app->response->redirect($app->getCpEditUrl()); diff --git a/src/models/App.php b/src/models/App.php index 879fb79..48ff8ed 100644 --- a/src/models/App.php +++ b/src/models/App.php @@ -148,7 +148,7 @@ public function getProviderInstance() 'app' => $this, 'type' => $this->provider ]; - $this->providerInstance = Plugin::$plugin->providers->createProvider($config); + $this->providerInstance = Plugin::getInstance()->providers->createProvider($config); return $this->providerInstance; } @@ -159,7 +159,7 @@ public function getProviderInstance() */ public function getAllTokens(): array { - return Plugin::$plugin->tokens->getAllTokensForApp($this->id); + return Plugin::getInstance()->tokens->getAllTokensForApp($this->id); } /** @@ -219,13 +219,13 @@ public function getValidTokensForUser($user = null) // No user, but let's return an empty array so we don't break anything upstream return []; } - return Plugin::$plugin->credentials->getValidTokensForAppAndUser($this, $userId); + return Plugin::getInstance()->credentials->getValidTokensForAppAndUser($this, $userId); } /** * @inheritdoc */ - public function rules() + public function rules(): array { return [ [['handle', 'name', 'clientId', 'clientSecret', 'provider'], 'required'], diff --git a/src/models/Token.php b/src/models/Token.php index b6b3195..83729ee 100644 --- a/src/models/Token.php +++ b/src/models/Token.php @@ -65,7 +65,7 @@ public function getApp() return $this->app; } - $this->app = Plugin::$plugin->apps->getAppById($this->appId); + $this->app = Plugin::getInstance()->apps->getAppById($this->appId); return $this->app; } @@ -82,7 +82,7 @@ public function getRefreshURL() /** * @inheritdoc */ - public function rules() + public function rules(): array { return [ [['accessToken', 'appId'], 'required'], diff --git a/src/services/Credentials.php b/src/services/Credentials.php index 58dd4d9..dfc5ae0 100644 --- a/src/services/Credentials.php +++ b/src/services/Credentials.php @@ -49,7 +49,7 @@ public function getValidTokensForAppAndUser($appHandle, $user = null): array if ($appHandle instanceof AppModel) { $app = $appHandle; } else { - $app = Plugin::$plugin->apps->getAppByHandle($appHandle); + $app = Plugin::getInstance()->apps->getAppByHandle($appHandle); } if (!$app instanceof AppModel) { @@ -76,7 +76,7 @@ public function getValidTokensForAppAndUser($appHandle, $user = null): array $tokenModels = []; foreach ($tokenRecords as $tokenRecord) { - $tokenModel = Plugin::$plugin->tokens->createToken($tokenRecord); + $tokenModel = Plugin::getInstance()->tokens->createToken($tokenRecord); // We need to prune this token at some point if ($tokenModel->hasExpired() && empty($tokenModel->refreshToken)) { continue; @@ -112,7 +112,7 @@ public function refreshToken(TokenModel $tokenModel): bool try { $app = $tokenModel->getApp(); $app->getProviderInstance()->refreshToken($tokenModel); - $saved = Plugin::$plugin->tokens->saveToken($tokenModel); + $saved = Plugin::getInstance()->tokens->saveToken($tokenModel); if ($saved) { if ($this->hasEventHandlers(self::EVENT_AFTER_REFRESH_TOKEN)) { $this->trigger(self::EVENT_AFTER_REFRESH_TOKEN, new TokenEvent([ diff --git a/src/variables/OAuthVariable.php b/src/variables/OAuthVariable.php index d65cec8..d6bb2e7 100644 --- a/src/variables/OAuthVariable.php +++ b/src/variables/OAuthVariable.php @@ -20,6 +20,6 @@ class OAuthVariable */ public function getAppByHandle($handle) { - return Plugin::$plugin->apps->getAppByHandle($handle); + return Plugin::getInstance()->apps->getAppByHandle($handle); } } \ No newline at end of file From 87ffa069da714e4144de856fb4ccfb831aa609d1 Mon Sep 17 00:00:00 2001 From: Ransom Roberson Date: Fri, 12 Aug 2022 15:49:56 -0400 Subject: [PATCH 02/11] Update editable table field for Craft 4 --- src/templates/apps/_edit.twig | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/templates/apps/_edit.twig b/src/templates/apps/_edit.twig index 98b3304..9363c61 100644 --- a/src/templates/apps/_edit.twig +++ b/src/templates/apps/_edit.twig @@ -107,6 +107,9 @@ code: true, } }, + allowAdd: true, + allowReorder: true, + allowDelete: true, rows: app.getScopes(true), errors: app.getErrors('scopes'), }) }} From 978c89f6418171efbc371b99b3486c7ed02b775f Mon Sep 17 00:00:00 2001 From: Ransom Roberson Date: Fri, 12 Aug 2022 17:54:10 -0400 Subject: [PATCH 03/11] Code cleanup --- CHANGELOG.md | 1 + src/base/Provider.php | 6 +- src/base/ProviderInterface.php | 8 +-- src/console/controllers/AppsController.php | 11 ++-- src/models/App.php | 53 +++++++++--------- src/models/Token.php | 64 +++++++--------------- src/records/App.php | 6 -- src/records/Token.php | 17 +----- src/services/Apps.php | 38 +++++++------ src/services/Credentials.php | 12 ++-- src/services/Providers.php | 8 +-- src/services/Tokens.php | 10 +++- src/variables/OAuthVariable.php | 2 +- 13 files changed, 99 insertions(+), 137 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c7b6db9..ba95844 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ ### Changed - Now requires Craft 4 & PHP 8.0.2 +- `Provider::getConfiguredProvider()` now returns `ProviderInterface` instead of `AbstractProvider` ### Removed - Removed route `oauthclient/authorize/refresh/` use `oauth/authorize/refresh/` instead diff --git a/src/base/Provider.php b/src/base/Provider.php index e32102e..7d735a5 100644 --- a/src/base/Provider.php +++ b/src/base/Provider.php @@ -22,7 +22,7 @@ */ abstract class Provider extends Component implements ProviderInterface { - const EVENT_CREATE_TOKEN_MODEL_FROM_RESPONSE = 'EVENT_CREATE_TOKEN_MODEL_FROM_RESPONSE'; + public const EVENT_CREATE_TOKEN_MODEL_FROM_RESPONSE = 'EVENT_CREATE_TOKEN_MODEL_FROM_RESPONSE'; protected $configuredProvider; private $app; @@ -53,7 +53,7 @@ public function getDefaultAuthorizationUrlOptions(): array * @inheritDoc * @throws ReflectionException */ - public function getConfiguredProvider() + public function getConfiguredProvider(): ProviderInterface { if ($this->configuredProvider instanceof AbstractProvider) { return $this->configuredProvider; @@ -113,7 +113,7 @@ public function setApp(AppModel $app) /** * Gets a unique state parameter. We're gonna use the CSRF token by default - * @return string|null + * @return string */ public function getState(): string { diff --git a/src/base/ProviderInterface.php b/src/base/ProviderInterface.php index b5fc861..4bfc30e 100644 --- a/src/base/ProviderInterface.php +++ b/src/base/ProviderInterface.php @@ -28,9 +28,9 @@ public static function getProviderClass(): string; /** * Gets a concrete League provider instance - * @return AbstractProvider + * @return ProviderInterface */ - public function getConfiguredProvider(); + public function getConfiguredProvider(): ProviderInterface; /** * Get the URL used to authorize the token @@ -41,12 +41,12 @@ public function getAuthorizeURL($options): string; /** * Get the session security state - * @return string|null + * @return string */ public function getState(): string; /** - * Gets an access token, returing a League access token + * Gets an access token, returning a League access token * @param $grant * @param array $options * @return AccessTokenInterface diff --git a/src/console/controllers/AppsController.php b/src/console/controllers/AppsController.php index a310284..0c0ce72 100644 --- a/src/console/controllers/AppsController.php +++ b/src/console/controllers/AppsController.php @@ -8,8 +8,8 @@ namespace venveo\oauthclient\console\controllers; use craft\console\Controller; -use venveo\oauthclient\models\Token; use venveo\oauthclient\Plugin; +use yii\console\ExitCode; class AppsController extends Controller { @@ -19,24 +19,23 @@ class AppsController extends Controller * @param $appHandle * @return int */ - public function actionRefreshTokens($appHandle) + public function actionRefreshTokens($appHandle): int { $credentialService = Plugin::getInstance()->credentials; $appService = Plugin::getInstance()->apps; if (!$app = $appService->getAppByHandle($appHandle)) { $this->stderr('No app found with that handle' . PHP_EOL); - return 1; + return ExitCode::UNSPECIFIED_ERROR; } $tokens = $app->getAllTokens(); $total = count($tokens); if (!$total) { $this->stdout('No tokens exist for that app' . PHP_EOL); - return 0; + return ExitCode::OK; } $progress = 0; $hadErrors = false; - /** @var Token $token */ foreach ($tokens as $token) { ++$progress; $prefix = "($progress/$total)"; @@ -49,6 +48,6 @@ public function actionRefreshTokens($appHandle) } } - return $hadErrors ? 1 : 0; + return $hadErrors ? ExitCode::UNSPECIFIED_ERROR : ExitCode::OK; } } \ No newline at end of file diff --git a/src/models/App.php b/src/models/App.php index 48ff8ed..74e286a 100644 --- a/src/models/App.php +++ b/src/models/App.php @@ -7,7 +7,6 @@ use craft\elements\User; use craft\helpers\Template; use craft\helpers\UrlHelper; -use craft\services\Security; use craft\validators\UniqueValidator; use Exception; use Twig\Error\LoaderError; @@ -20,7 +19,6 @@ use venveo\oauthclient\records\Token as TokenRecord; use yii\base\InvalidConfigException; use yii\db\ActiveQuery; -use yii\web\HttpException; /** * Class App @@ -34,22 +32,20 @@ */ class App extends Model { - public $uid; - public $id; - public $dateCreated; - public $dateUpdated; + public ?string $uid = null; + public ?int $id = null; + public ?\DateTime $dateCreated = null; + public ?\DateTime $dateUpdated = null; public $scopes; - public $userId; - public $provider; - public $name; - public $handle; - public $clientId; - public $clientSecret; - public $urlAuthorize; + public ?int $userId = null; + public ?string $provider = null; + public ?string $name = null; + public ?string $handle = null; + public ?int $clientId = null; + public ?string $clientSecret = null; + public ?string $urlAuthorize = null; - public $isNew; - - private $providerInstance = null; + private ?Provider $providerInstance = null; /** * Returns the name of this payment method. @@ -68,7 +64,7 @@ public function __toString() */ public function getClientId(): string { - return Craft::parseEnv($this->clientId); + return \craft\helpers\App::parseEnv($this->clientId); } /** @@ -78,7 +74,7 @@ public function getClientId(): string */ public function getClientSecret(): string { - return Craft::parseEnv($this->clientSecret); + return \craft\helpers\App::parseEnv($this->clientSecret); } /** @@ -88,7 +84,7 @@ public function getClientSecret(): string */ public function getUrlAuthorize(): string { - return Craft::parseEnv($this->urlAuthorize); + return \craft\helpers\App::parseEnv($this->urlAuthorize); } /** @@ -97,10 +93,10 @@ public function getUrlAuthorize(): string * @param bool $forTable If true, we'll format the output for Craft's table field * @return array */ - public function getScopes($forTable = false): array + public function getScopes(bool $forTable = false): array { if ($forTable) { - return array_map(function ($scope) { + return array_map(static function ($scope) { return ['scope' => $scope]; }, explode(',', $this->scopes)); } @@ -120,11 +116,13 @@ public function getCpEditUrl(): string /** * Get the URL callback URL * - * @param null|string $context A context that will be passed to the controller to help tag events for handling. + * @param string|null $context A context that will be passed to the controller to help tag events for handling. * @param null $returnUrl * @return string + * @throws InvalidConfigException + * @throws \yii\base\Exception */ - public function getRedirectUrl($context = null, $returnUrl = null): string + public function getRedirectUrl(string $context = null, $returnUrl = null): string { return UrlHelper::cpUrl('oauth/authorize/' . $this->handle, [ 'context' => $context, @@ -138,7 +136,7 @@ public function getRedirectUrl($context = null, $returnUrl = null): string * @return Provider|null * @throws InvalidConfigException */ - public function getProviderInstance() + public function getProviderInstance(): ?Provider { if ($this->providerInstance instanceof Provider) { return $this->providerInstance; @@ -202,11 +200,12 @@ public function renderConnector($context = null) * Get all tokens valid tokens for the supplied user. If no user is supplied, the current user will * be used. * - * @param null|int|User $user + * @param int|User|null $user * @return Token[] * @throws Exception + * @throws \Throwable */ - public function getValidTokensForUser($user = null) + public function getValidTokensForUser(int|User $user = null): array { $userId = null; if ($user instanceof User) { @@ -216,7 +215,7 @@ public function getValidTokensForUser($user = null) } elseif ($currentUser = Craft::$app->user->getIdentity()) { $userId = $currentUser->id; } else { - // No user, but let's return an empty array so we don't break anything upstream + // No user, but let's return an empty array, so we don't break anything upstream return []; } return Plugin::getInstance()->credentials->getValidTokensForAppAndUser($this, $userId); diff --git a/src/models/Token.php b/src/models/Token.php index 83729ee..2ba460b 100644 --- a/src/models/Token.php +++ b/src/models/Token.php @@ -9,7 +9,6 @@ use craft\validators\DateTimeValidator; use DateTime; use League\OAuth2\Client\Token\AccessTokenInterface; -use venveo\oauthclient\models\App as AppModel; use venveo\oauthclient\Plugin; /** @@ -20,27 +19,24 @@ * @property mixed $token * @property mixed $values * @property mixed $expires + * @property-read null|App $app * @property string $cpEditUrl */ class Token extends Model implements AccessTokenInterface { - public $id; - public $dateCreated; - public $dateUpdated; + public ?int $id = null; + public ?\DateTime $dateCreated = null; + public ?\DateTime $dateUpdated = null; - public $userId; - public $appId; - public $expiryDate; - public $refreshToken; - public $accessToken; - public $uid; + public ?int $userId = null; + public ?int $appId = null; + public ?\DateTime $expiryDate = null; + public ?string $refreshToken = null; + public ?string $accessToken = null; + public ?string $uid = null; - private $tokenValues; - - /** @var AppModel */ - private $app; - /** @var User */ - private $user; + private ?array $tokenValues = null; + private ?User $user = null; /** * Gets the user the token belongs to @@ -61,19 +57,14 @@ public function getUser() */ public function getApp() { - if ($this->app instanceof AppModel) { - return $this->app; - } - - $this->app = Plugin::getInstance()->apps->getAppById($this->appId); - return $this->app; + return Plugin::getInstance()->apps->getAppById($this->appId); } /** * The internal URL in the CP to refresh this token * @return string */ - public function getRefreshURL() + public function getRefreshURL(): string { return UrlHelper::cpUrl('oauth/authorize/refresh/' . $this->id); } @@ -98,7 +89,7 @@ public function rules(): array /** * @inheritDoc */ - public function getToken() + public function getToken(): ?string { return $this->accessToken; } @@ -106,7 +97,7 @@ public function getToken() /** * @inheritDoc */ - public function getRefreshToken() + public function getRefreshToken(): ?string { return $this->refreshToken; } @@ -114,7 +105,7 @@ public function getRefreshToken() /** * @inheritDoc */ - public function getExpires() + public function getExpires(): DateTime|int|null { return $this->expiryDate; } @@ -122,7 +113,7 @@ public function getExpires() /** * @inheritDoc */ - public function hasExpired() + public function hasExpired(): bool { if (!$this->expiryDate) { return false; @@ -137,7 +128,7 @@ public function hasExpired() /** * @inheritDoc */ - public function getValues() + public function getValues(): array { return $this->tokenValues; } @@ -145,7 +136,7 @@ public function getValues() /** * @inheritDoc */ - public function jsonSerialize() + public function jsonSerialize(): array { return $this->toArray(); } @@ -155,19 +146,6 @@ public function jsonSerialize() */ public function __toString() { - return $this->accessToken; - } - - - /** - * @inheritdoc - */ - public function datetimeAttributes(): array - { - $attributes = parent::datetimeAttributes(); - - $attributes[] = 'expiryDate'; - - return $attributes; + return $this?->accessToken ?? ''; } } diff --git a/src/records/App.php b/src/records/App.php index ba1b455..0cf2990 100644 --- a/src/records/App.php +++ b/src/records/App.php @@ -36,9 +36,6 @@ */ class App extends ActiveRecord { - // Public Static Methods - // ========================================================================= - /** * @inheritdoc */ @@ -47,9 +44,6 @@ public static function tableName() return '{{%oauthclient_apps}}'; } - // Public Methods - // ========================================================================= - /** * Returns the OAuth Tokens’s user. * diff --git a/src/records/Token.php b/src/records/Token.php index 20bf90b..528ce51 100644 --- a/src/records/Token.php +++ b/src/records/Token.php @@ -35,13 +35,10 @@ */ class Token extends ActiveRecord { - // Public Static Methods - // ========================================================================= - /** * @inheritdoc */ - public static function tableName() + public static function tableName(): string { return '{{%oauthclient_tokens}}'; } @@ -65,16 +62,4 @@ public function getApp(): ActiveQueryInterface { return $this->hasOne(AppRecord::class, ['id' => 'appId']); } - - /** - * @inheritdoc - */ - public function datetimeAttributes(): array - { - $attributes = parent::datetimeAttributes(); - - $attributes[] = 'expiryDate'; - - return $attributes; - } } diff --git a/src/services/Apps.php b/src/services/Apps.php index 395ea6f..fcc1314 100644 --- a/src/services/Apps.php +++ b/src/services/Apps.php @@ -34,18 +34,18 @@ class Apps extends Component { - const EVENT_BEFORE_APP_SAVED = 'EVENT_BEFORE_APP_SAVED'; - const EVENT_AFTER_APP_SAVED = 'EVENT_AFTER_APP_SAVED'; + public const EVENT_BEFORE_APP_SAVED = 'EVENT_BEFORE_APP_SAVED'; + public const EVENT_AFTER_APP_SAVED = 'EVENT_AFTER_APP_SAVED'; - const EVENT_BEFORE_APP_DELETED = 'EVENT_BEFORE_APP_DELETED'; - const EVENT_AFTER_APP_DELETED = 'EVENT_AFTER_APP_DELETED'; + public const EVENT_BEFORE_APP_DELETED = 'EVENT_BEFORE_APP_DELETED'; + public const EVENT_AFTER_APP_DELETED = 'EVENT_AFTER_APP_DELETED'; - const EVENT_GET_URL_OPTIONS = 'EVENT_GET_URL_OPTIONS'; + public const EVENT_GET_URL_OPTIONS = 'EVENT_GET_URL_OPTIONS'; - private $_APPS_BY_HANDLE = []; - private $_APPS_BY_ID = []; - private $_APPS_BY_UID = []; - private $_ALL_APPS_FETCHED = false; + private array $_APPS_BY_HANDLE = []; + private array $_APPS_BY_ID = []; + private array $_APPS_BY_UID = []; + private bool $_ALL_APPS_FETCHED = false; /** * Returns all apps @@ -115,7 +115,7 @@ public function createApp($config): AppModel * @param $id * @return AppModel|null */ - public function getAppById($id) + public function getAppById($id): ?AppModel { if (isset($this->_APPS_BY_ID[$id])) { return $this->_APPS_BY_ID[$id]; @@ -138,7 +138,7 @@ public function getAppById($id) * @param $handle * @return AppModel|null */ - public function getAppByHandle($handle) + public function getAppByHandle($handle): ?AppModel { if (isset($this->_APPS_BY_HANDLE[$handle])) { return $this->_APPS_BY_HANDLE[$handle]; @@ -167,7 +167,7 @@ public function getAppByHandle($handle) * @throws ReflectionException * @throws InvalidConfigException */ - public function getAuthorizationUrlForApp(AppModel $app, $state, $context = null) + public function getAuthorizationUrlForApp(AppModel $app, $state, $context = null): ?string { $options = ['state' => $state]; @@ -195,7 +195,7 @@ public function getAuthorizationUrlForApp(AppModel $app, $state, $context = null * * @param AppModel $app */ - public function deleteApp(AppModel $app) + public function deleteApp(AppModel $app): void { if ($this->hasEventHandlers(self::EVENT_BEFORE_APP_DELETED)) { $this->trigger(self::EVENT_BEFORE_APP_DELETED, new AppEvent([ @@ -222,8 +222,10 @@ public function saveApp(AppModel $app, bool $runValidation = true): bool // Ensure the product type has a UID if ($isNew) { $app->uid = StringHelper::UUID(); - } else if (!$app->uid) { - $app->uid = Db::uidById('{{%oauthclient_apps}}', $app->id); + } else { + if (!$app->uid) { + $app->uid = Db::uidById('{{%oauthclient_apps}}', $app->id); + } } if ($this->hasEventHandlers(self::EVENT_BEFORE_APP_SAVED)) { @@ -261,7 +263,7 @@ public function saveApp(AppModel $app, bool $runValidation = true): bool * @param ConfigEvent $event * @throws Exception */ - public function handleUpdatedApp(ConfigEvent $event) + public function handleUpdatedApp(ConfigEvent $event): void { $uid = $event->tokenMatches[0]; @@ -318,7 +320,7 @@ public function handleUpdatedApp(ConfigEvent $event) * @param $uid * @return AppModel|null */ - public function getAppByUid($uid) + public function getAppByUid($uid): ?AppModel { if (isset($this->_APPS_BY_UID[$uid])) { return $this->_APPS_BY_UID[$uid]; @@ -341,7 +343,7 @@ public function getAppByUid($uid) * @param ConfigEvent $event * @throws Exception */ - public function handleRemovedApp(ConfigEvent $event) + public function handleRemovedApp(ConfigEvent $event): void { $uid = $event->tokenMatches[0]; diff --git a/src/services/Credentials.php b/src/services/Credentials.php index dfc5ae0..4f584bd 100644 --- a/src/services/Credentials.php +++ b/src/services/Credentials.php @@ -31,19 +31,19 @@ */ class Credentials extends Component { - const EVENT_BEFORE_REFRESH_TOKEN = 'EVENT_BEFORE_REFRESH_TOKEN'; - const EVENT_AFTER_REFRESH_TOKEN = 'EVENT_BEFORE_REFRESH_TOKEN'; - const EVENT_TOKEN_REFRESH_FAILED = 'EVENT_TOKEN_REFRESH_FAILED'; + public const EVENT_BEFORE_REFRESH_TOKEN = 'EVENT_BEFORE_REFRESH_TOKEN'; + public const EVENT_AFTER_REFRESH_TOKEN = 'EVENT_BEFORE_REFRESH_TOKEN'; + public const EVENT_TOKEN_REFRESH_FAILED = 'EVENT_TOKEN_REFRESH_FAILED'; /** * Gets valid tokens given an application and optionally, a Craft user ID * This method will attempt to refresh expired tokens for an app - * @param $appHandle AppModel|string - * @param $user User|int + * @param $appHandle string|AppModel + * @param $user int|User|null * @return TokenModel[] * @throws \Exception */ - public function getValidTokensForAppAndUser($appHandle, $user = null): array + public function getValidTokensForAppAndUser(string|AppModel $appHandle, int|User|null $user = null): array { // Allow models or IDs against my better judgement if ($appHandle instanceof AppModel) { diff --git a/src/services/Providers.php b/src/services/Providers.php index f6c6481..46062ba 100644 --- a/src/services/Providers.php +++ b/src/services/Providers.php @@ -32,10 +32,10 @@ */ class Providers extends Component { - const EVENT_REGISTER_PROVIDER_TYPES = 'EVENT_REGISTER_GATEWAY_TYPES'; + public const EVENT_REGISTER_PROVIDER_TYPES = 'EVENT_REGISTER_GATEWAY_TYPES'; /** - * Get all of the registered provider types. This is a great place to register your custom providers! + * Get all the registered provider types. This is a great place to register your custom providers! * @return array */ public function getAllProviderTypes(): array @@ -59,11 +59,11 @@ public function getAllProviderTypes(): array /** * Creates a Provider with a given config * - * @param mixed $config The providers’s class name, or its config, with a `type` value and optionally a `settings` value + * @param array|string $config The provider's class name, or its config, with a `type` value and optionally a `settings` value * @return Provider The provider * @throws InvalidConfigException */ - public function createProvider($config): Provider + public function createProvider(array|string $config): Provider { if (is_string($config)) { $config = ['type' => $config]; diff --git a/src/services/Tokens.php b/src/services/Tokens.php index 372fbd5..6f0f63d 100644 --- a/src/services/Tokens.php +++ b/src/services/Tokens.php @@ -30,8 +30,8 @@ class Tokens extends Component { - const EVENT_BEFORE_TOKEN_SAVED = 'EVENT_BEFORE_TOKEN_SAVED'; - const EVENT_AFTER_TOKEN_SAVED = 'EVENT_AFTER_TOKEN_SAVED'; + public const EVENT_BEFORE_TOKEN_SAVED = 'EVENT_BEFORE_TOKEN_SAVED'; + public const EVENT_AFTER_TOKEN_SAVED = 'EVENT_AFTER_TOKEN_SAVED'; /** * Returns all tokens @@ -84,7 +84,7 @@ public function createToken($config): TokenModel * @param $id * @return TokenModel|null */ - public function getTokenById($id) + public function getTokenById($id): ?TokenModel { $result = $this->_createTokenQuery() ->where(['id' => $id]) @@ -93,6 +93,10 @@ public function getTokenById($id) return $result ? $this->createToken($result) : null; } + /** + * @param $appId + * @return array + */ public function getAllTokensForApp($appId): array { $rows = $this->_createTokenQuery() diff --git a/src/variables/OAuthVariable.php b/src/variables/OAuthVariable.php index d6bb2e7..82a1780 100644 --- a/src/variables/OAuthVariable.php +++ b/src/variables/OAuthVariable.php @@ -18,7 +18,7 @@ class OAuthVariable * @param $handle * @return App|null */ - public function getAppByHandle($handle) + public function getAppByHandle($handle): ?App { return Plugin::getInstance()->apps->getAppByHandle($handle); } From b2d8dc49cf775d975b8908daf29fa94e0c7d8104 Mon Sep 17 00:00:00 2001 From: Ransom Roberson Date: Fri, 12 Aug 2022 18:07:55 -0400 Subject: [PATCH 04/11] Fix error caused by removing isNew property from App model --- src/controllers/AppsController.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/controllers/AppsController.php b/src/controllers/AppsController.php index 369dd7a..5d20be6 100644 --- a/src/controllers/AppsController.php +++ b/src/controllers/AppsController.php @@ -128,6 +128,7 @@ public function actionSave() if (!Craft::$app->getConfig()->getGeneral()->allowAdminChanges) { throw new ForbiddenHttpException('Administrative changes are disallowed in this environment'); } + $isNew = true; $this->requireAdmin(); $this->requirePostRequest(); @@ -150,6 +151,9 @@ public function actionSave() /** @var AppModel $app */ $app = $appService->createApp($config); + if ($app->id) { + $isNew = false; + } $session = Craft::$app->session; @@ -164,6 +168,6 @@ public function actionSave() } $session->setNotice(Craft::t('oauthclient', 'App saved')); - return $this->redirect(UrlHelper::cpUrl('oauthclient/apps/' . $app->handle . ($app->isNew ? '#info-tab' : ''))); + return $this->redirect(UrlHelper::cpUrl('oauthclient/apps/' . $app->handle . ($isNew ? '#info-tab' : ''))); } } From 4c21ff5e45ae25b0dbc79932500675c631cc23d8 Mon Sep 17 00:00:00 2001 From: Ransom Roberson Date: Fri, 12 Aug 2022 18:08:02 -0400 Subject: [PATCH 05/11] More docs & return types --- src/controllers/AppsController.php | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/src/controllers/AppsController.php b/src/controllers/AppsController.php index 5d20be6..d2629fa 100644 --- a/src/controllers/AppsController.php +++ b/src/controllers/AppsController.php @@ -29,7 +29,11 @@ class AppsController extends Controller { - public function actionIndex() + /** + * @return \yii\web\Response + * @throws ForbiddenHttpException + */ + public function actionIndex(): \yii\web\Response { $this->requireAdmin(); $apps = Plugin::getInstance()->apps->getAllApps(); @@ -39,14 +43,13 @@ public function actionIndex() ]); } - /** - * @param AppModel|null $app - * @return Response - * @throws HttpException - * @throws InvalidConfigException + * @param $handle + * @param $app + * @return \yii\web\Response + * @throws ForbiddenHttpException */ - public function actionEdit($handle = null, $app = null) + public function actionEdit($handle = null, $app = null): \yii\web\Response { if (!Craft::$app->getConfig()->getGeneral()->allowAdminChanges) { throw new ForbiddenHttpException('Administrative changes are disallowed in this environment'); @@ -93,11 +96,11 @@ public function actionEdit($handle = null, $app = null) /** * Attempt to delete an app by its ID * @return \yii\web\Response - * @throws \yii\db\Exception * @throws BadRequestHttpException * @throws ForbiddenHttpException + * @throws NotFoundHttpException */ - public function actionDelete() + public function actionDelete(): \yii\web\Response { if (!Craft::$app->getConfig()->getGeneral()->allowAdminChanges) { throw new ForbiddenHttpException('Administrative changes are disallowed in this environment'); @@ -119,11 +122,11 @@ public function actionDelete() } /** - * @return Response|null - * @throws HttpException - * @throws Exception + * @return \yii\web\Response|null + * @throws BadRequestHttpException + * @throws ForbiddenHttpException */ - public function actionSave() + public function actionSave(): ?\yii\web\Response { if (!Craft::$app->getConfig()->getGeneral()->allowAdminChanges) { throw new ForbiddenHttpException('Administrative changes are disallowed in this environment'); @@ -149,7 +152,6 @@ public function actionSave() 'scopes' => $scopes ]; - /** @var AppModel $app */ $app = $appService->createApp($config); if ($app->id) { $isNew = false; From 35dceea18386178f5eaa8605ddb19cde586e85fa Mon Sep 17 00:00:00 2001 From: Ransom Roberson Date: Fri, 12 Aug 2022 22:42:20 -0400 Subject: [PATCH 06/11] Fix some type issues --- CHANGELOG.md | 1 - src/base/Provider.php | 2 +- src/base/ProviderInterface.php | 4 ++-- src/controllers/AppsController.php | 7 +++---- src/models/App.php | 4 ++-- 5 files changed, 8 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ba95844..c7b6db9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,6 @@ ### Changed - Now requires Craft 4 & PHP 8.0.2 -- `Provider::getConfiguredProvider()` now returns `ProviderInterface` instead of `AbstractProvider` ### Removed - Removed route `oauthclient/authorize/refresh/` use `oauth/authorize/refresh/` instead diff --git a/src/base/Provider.php b/src/base/Provider.php index 7d735a5..799d32c 100644 --- a/src/base/Provider.php +++ b/src/base/Provider.php @@ -53,7 +53,7 @@ public function getDefaultAuthorizationUrlOptions(): array * @inheritDoc * @throws ReflectionException */ - public function getConfiguredProvider(): ProviderInterface + public function getConfiguredProvider(): AbstractProvider { if ($this->configuredProvider instanceof AbstractProvider) { return $this->configuredProvider; diff --git a/src/base/ProviderInterface.php b/src/base/ProviderInterface.php index 4bfc30e..871381e 100644 --- a/src/base/ProviderInterface.php +++ b/src/base/ProviderInterface.php @@ -28,9 +28,9 @@ public static function getProviderClass(): string; /** * Gets a concrete League provider instance - * @return ProviderInterface + * @return AbstractProvider */ - public function getConfiguredProvider(): ProviderInterface; + public function getConfiguredProvider(): AbstractProvider; /** * Get the URL used to authorize the token diff --git a/src/controllers/AppsController.php b/src/controllers/AppsController.php index d2629fa..cd8ed3a 100644 --- a/src/controllers/AppsController.php +++ b/src/controllers/AppsController.php @@ -161,14 +161,13 @@ public function actionSave(): ?\yii\web\Response // Save it if (!Plugin::getInstance()->apps->saveApp($app)) { - $session->setError(Craft::t('oauthclient', 'Failed to save app')); - // Send the volume back to the template - Craft::$app->getUrlManager()->setRouteParams([ + // TODO: Switch to asModelFailure + return $this->asFailure('Failed to save app', [], [ 'app' => $app ]); - return null; } + // TODO: Switch to asModuleSuccess $session->setNotice(Craft::t('oauthclient', 'App saved')); return $this->redirect(UrlHelper::cpUrl('oauthclient/apps/' . $app->handle . ($isNew ? '#info-tab' : ''))); } diff --git a/src/models/App.php b/src/models/App.php index 74e286a..28becf7 100644 --- a/src/models/App.php +++ b/src/models/App.php @@ -41,7 +41,7 @@ class App extends Model public ?string $provider = null; public ?string $name = null; public ?string $handle = null; - public ?int $clientId = null; + public ?string $clientId = null; public ?string $clientSecret = null; public ?string $urlAuthorize = null; @@ -84,7 +84,7 @@ public function getClientSecret(): string */ public function getUrlAuthorize(): string { - return \craft\helpers\App::parseEnv($this->urlAuthorize); + return \craft\helpers\App::parseEnv($this->urlAuthorize) ?: ''; } /** From 4d93d72c89bc7635a47fc29e6b35daf0cde96468 Mon Sep 17 00:00:00 2001 From: Ransom Roberson Date: Sat, 13 Aug 2022 13:38:36 -0400 Subject: [PATCH 07/11] Better return types --- src/models/App.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/models/App.php b/src/models/App.php index 28becf7..deac4e5 100644 --- a/src/models/App.php +++ b/src/models/App.php @@ -80,18 +80,18 @@ public function getClientSecret(): string /** * Returns the custom authorization URL. * - * @return string + * @return string|null */ - public function getUrlAuthorize(): string + public function getUrlAuthorize(): ?string { - return \craft\helpers\App::parseEnv($this->urlAuthorize) ?: ''; + return \craft\helpers\App::parseEnv($this->urlAuthorize); } /** * Get the scopes for the app * * @param bool $forTable If true, we'll format the output for Craft's table field - * @return array + * @return array */ public function getScopes(bool $forTable = false): array { From 7ff4f7408aaa8d1739cf26669592a111a885edae Mon Sep 17 00:00:00 2001 From: Ransom Roberson Date: Sat, 13 Aug 2022 13:41:16 -0400 Subject: [PATCH 08/11] Tidier --- src/models/App.php | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/src/models/App.php b/src/models/App.php index deac4e5..721c78f 100644 --- a/src/models/App.php +++ b/src/models/App.php @@ -8,6 +8,7 @@ use craft\helpers\Template; use craft\helpers\UrlHelper; use craft\validators\UniqueValidator; +use craft\web\View; use Exception; use Twig\Error\LoaderError; use Twig\Error\RuntimeError; @@ -173,26 +174,22 @@ public function getTokenRecordQuery(): ActiveQuery /** * Renders some basic UI to allow a user to connect to the app * + * @param string|null $context * @return Markup * @throws LoaderError * @throws RuntimeError * @throws SyntaxError + * @throws \Throwable * @throws \yii\base\Exception */ - public function renderConnector($context = null) + public function renderConnector(string|null $context = null): Markup { - $view = Craft::$app->getView(); - $oldTemplateMode = $view->getTemplateMode(); - if ($oldTemplateMode !== $view::TEMPLATE_MODE_CP) { - $view->setTemplateMode($view::TEMPLATE_MODE_CP); - } $tokens = $this->getValidTokensForUser(); $template = Craft::$app->view->renderTemplate('oauthclient/_connector/connector', [ 'context' => $context, 'app' => $this, 'token' => count($tokens) ? $tokens[0] : null - ]); - $view->setTemplateMode($oldTemplateMode); + ], View::TEMPLATE_MODE_CP); return Template::raw($template); } From e1070bb75780c750e64418cd88aa1864cef8ca0d Mon Sep 17 00:00:00 2001 From: Ransom Roberson Date: Sat, 13 Aug 2022 13:53:31 -0400 Subject: [PATCH 09/11] Update Google oauthclient minimum version --- composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer.json b/composer.json index 5fee260..bacaad1 100644 --- a/composer.json +++ b/composer.json @@ -25,7 +25,7 @@ "php": "^8.0.2", "craftcms/cms": "^4.0", "league/oauth2-client": "^2.2.1", - "league/oauth2-google": "^2.2 || ^3.0", + "league/oauth2-google": "^4.0", "league/oauth2-facebook": "^2.0", "league/oauth2-github": "^2.0 || ^3.0" }, From f870d126a2565917a281f651876dffc91a993562 Mon Sep 17 00:00:00 2001 From: Ransom Roberson Date: Sat, 13 Aug 2022 13:59:28 -0400 Subject: [PATCH 10/11] Fix error when saving an app with no scopes (Fixes #37) --- CHANGELOG.md | 3 +++ src/controllers/AppsController.php | 4 +++- src/models/App.php | 2 +- 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c7b6db9..9fcaa2f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,9 @@ ### Changed - Now requires Craft 4 & PHP 8.0.2 +## Fixed +- Fixed saving an app with no scopes producing an error + ### Removed - Removed route `oauthclient/authorize/refresh/` use `oauth/authorize/refresh/` instead - Removed route `oauthclient/authorize/` use `oauth/authorize/` instead diff --git a/src/controllers/AppsController.php b/src/controllers/AppsController.php index cd8ed3a..20861c1 100644 --- a/src/controllers/AppsController.php +++ b/src/controllers/AppsController.php @@ -139,7 +139,9 @@ public function actionSave(): ?\yii\web\Response $appService = Plugin::getInstance()->apps; $scopes = $request->getBodyParam('scopes'); - $scopes = implode(',', ArrayHelper::getColumn($scopes, 'scope')); + if (is_array($scopes)) { + $scopes = implode(',', ArrayHelper::getColumn($scopes, 'scope')); + } $config = [ 'id' => $request->getBodyParam('id'), diff --git a/src/models/App.php b/src/models/App.php index 721c78f..2e2d78c 100644 --- a/src/models/App.php +++ b/src/models/App.php @@ -37,7 +37,7 @@ class App extends Model public ?int $id = null; public ?\DateTime $dateCreated = null; public ?\DateTime $dateUpdated = null; - public $scopes; + public string $scopes = ''; public ?int $userId = null; public ?string $provider = null; public ?string $name = null; From 0d0ce0462c3feff3668dc67d05230f04d950d7bb Mon Sep 17 00:00:00 2001 From: Ransom Roberson Date: Mon, 15 Aug 2022 11:10:03 -0400 Subject: [PATCH 11/11] Remove userId from apps --- CHANGELOG.md | 2 + src/Plugin.php | 16 ++--- src/migrations/Install.php | 64 +++---------------- .../m220815_121624_adjust_token_storage.php | 52 +++++++++++++++ src/models/App.php | 1 - src/records/App.php | 10 --- src/services/Apps.php | 2 - 7 files changed, 70 insertions(+), 77 deletions(-) create mode 100644 src/migrations/m220815_121624_adjust_token_storage.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 9fcaa2f..2542a9a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ ### Changed - Now requires Craft 4 & PHP 8.0.2 +- Apps no longer keep track of who created them ## Fixed - Fixed saving an app with no scopes producing an error @@ -12,6 +13,7 @@ - Removed route `oauthclient/authorize/refresh/` use `oauth/authorize/refresh/` instead - Removed route `oauthclient/authorize/` use `oauth/authorize/` instead - Removed `Plugin::$plugin` static property - use `Plugin::getInstance()` instead +- Removed `userId` property from Apps ## 2.1.9 - 2021-03-30 ### Added diff --git a/src/Plugin.php b/src/Plugin.php index b6ecf71..ac5c0cc 100644 --- a/src/Plugin.php +++ b/src/Plugin.php @@ -48,7 +48,7 @@ class Plugin extends BasePlugin public static string $PROJECT_CONFIG_KEY = 'oauthClient'; - public string $schemaVersion = '2.1.3'; + public string $schemaVersion = '4.0.0'; public bool $hasCpSettings = true; @@ -56,7 +56,7 @@ class Plugin extends BasePlugin /** * @inheritdoc */ - public function init() + public function init(): void { parent::init(); @@ -77,7 +77,7 @@ public function init() /** * Register a custom logger */ - private function _registerLogger() + private function _registerLogger(): void { Craft::getLogger()->dispatcher->targets[] = new MonologTarget([ 'name' => 'oauthclient', @@ -112,7 +112,7 @@ public static function config(): array /** * Adds the event handler for registering CP routes */ - private function _registerCpRoutes() + private function _registerCpRoutes(): void { Event::on(UrlManager::class, UrlManager::EVENT_REGISTER_CP_URL_RULES, function (RegisterUrlRulesEvent $event) { $event->rules = array_merge($event->rules, [ @@ -131,7 +131,7 @@ private function _registerCpRoutes() /** * Set our Twig variable */ - private function _registerVariables() + private function _registerVariables(): void { Event::on( CraftVariable::class, @@ -146,7 +146,7 @@ function (Event $event) { /** * Register project config handlers */ - private function _registerProjectConfig() + private function _registerProjectConfig(): void { Craft::$app->projectConfig ->onAdd(self::$PROJECT_CONFIG_KEY . '.apps.{uid}', [$this->apps, 'handleUpdatedApp']) @@ -162,7 +162,7 @@ private function _registerProjectConfig() * Handle project config rebuilding * @param RebuildConfigEvent $e */ - private function _handleProjectConfigRebuild(RebuildConfigEvent $e) + private function _handleProjectConfigRebuild(RebuildConfigEvent $e): void { $appData = []; $apps = $this->apps->getAllApps(); @@ -204,7 +204,7 @@ private function _registerPermissions() /** * @see Craft::t() */ - public static function t($message, $params = [], $language = null) + public static function t($message, $params = [], $language = null): string { return Craft::t(self::HANDLE, $message, $params, $language); } diff --git a/src/migrations/Install.php b/src/migrations/Install.php index ea00143..63574ff 100644 --- a/src/migrations/Install.php +++ b/src/migrations/Install.php @@ -20,27 +20,15 @@ */ class Install extends Migration { - // Public Properties - // ========================================================================= - - /** - * @var string The database driver to use - */ - public $driver; - - // Public Methods - // ========================================================================= /** * @inheritdoc */ public function safeUp() { - $this->driver = Craft::$app->getConfig()->getDb()->driver; if ($this->createTables()) { $this->createIndexes(); $this->addForeignKeys(); - // Refresh the db schema caches Craft::$app->db->schema->refresh(); } @@ -50,10 +38,8 @@ public function safeUp() /** * @return bool */ - protected function createTables() + protected function createTables(): bool { - $tablesCreated = false; - $tableSchema = Craft::$app->db->schema->getTableSchema('{{%oauthclient_tokens}}'); if ($tableSchema === null) { $this->createTable( @@ -81,7 +67,6 @@ protected function createTables() 'dateCreated' => $this->dateTime()->notNull(), 'dateUpdated' => $this->dateTime()->notNull(), 'uid' => $this->uid(), - 'userId' => $this->integer(), 'name' => $this->string(255)->notNull(), 'handle' => $this->string(255)->notNull(), 'provider' => $this->string(255)->notNull(), @@ -96,41 +81,20 @@ protected function createTables() return true; } - // Protected Methods - // ========================================================================= - /** * @return void */ - protected function createIndexes() + protected function createIndexes(): void { - $this->createIndex( - $this->db->getIndexName( - '{{%oauthclient_apps}}', - 'handle', - true - ), - '{{%oauthclient_apps}}', - 'handle', - true - ); - - $this->createIndex( - $this->db->getIndexName( - '{{%oauthclient_apps}}', - 'provider', - false - ), - '{{%oauthclient_apps}}', - 'provider', - false - ); + $this->createIndex(null, '{{%oauthclient_apps}}', 'handle', true); + $this->createIndex(null, '{{%oauthclient_apps}}', 'provider', false); + $this->createIndex(null, '{{%oauthclient_tokens}}', ['userId', 'appId'], true); } /** * @return void */ - protected function addForeignKeys() + protected function addForeignKeys(): void { $this->addForeignKey( $this->db->getForeignKeyName('{{%oauthclient_tokens}}', 'appId'), @@ -151,33 +115,21 @@ protected function addForeignKeys() 'CASCADE', 'CASCADE' ); - - $this->addForeignKey( - $this->db->getForeignKeyName('{{%oauthclient_apps}}', 'userId'), - '{{%oauthclient_apps}}', - 'userId', - '{{%users}}', - 'id', - 'CASCADE', - 'CASCADE' - ); } /** * @inheritdoc */ - public function safeDown() + public function safeDown(): bool { - $this->driver = Craft::$app->getConfig()->getDb()->driver; $this->removeTables(); - return true; } /** * @return void */ - protected function removeTables() + protected function removeTables(): void { $this->dropTableIfExists('{{%oauthclient_tokens}}'); $this->dropTableIfExists('{{%oauthclient_apps}}'); diff --git a/src/migrations/m220815_121624_adjust_token_storage.php b/src/migrations/m220815_121624_adjust_token_storage.php new file mode 100644 index 0000000..053d5df --- /dev/null +++ b/src/migrations/m220815_121624_adjust_token_storage.php @@ -0,0 +1,52 @@ +dropOldestTokens(); + $this->dropForeignKeyIfExists('{{%oauthclient_apps}}', 'userId'); + $this->dropColumn('{{%oauthclient_apps}}', 'userId'); + $this->createIndex(null, '{{%oauthclient_tokens}}', ['userId', 'appId'], true); + return true; + } + + protected function dropOldestTokens(): void + { + $query = (new Query())->select('id') + ->from(['t1' => '{{%oauthclient_tokens}}']) + ->innerJoin( + [ + 't2' => (new Query())->select(['MAX(id) as lastId', 'userId', 'appId']) + ->from('{{%oauthclient_tokens}}') + ->groupBy(['userId', 'appId']) + ->having('COUNT(*) > 1') + ], + '[[t2.userId]] = [[t1.userId]] AND [[t1.appId]] = [[t2.appId]]' + ) + ->where('[[t1.id]] < [[t2.lastId]]'); + $ids = $query->column($this->db); + + $this->delete('{{%oauthclient_tokens}}', ['IN', 'id', $ids]); + } + + /** + * @inheritdoc + */ + public function safeDown(): bool + { + echo "m220815_121624_adjust_token_storage cannot be reverted.\n"; + return false; + } +} diff --git a/src/models/App.php b/src/models/App.php index 2e2d78c..cdff54e 100644 --- a/src/models/App.php +++ b/src/models/App.php @@ -38,7 +38,6 @@ class App extends Model public ?\DateTime $dateCreated = null; public ?\DateTime $dateUpdated = null; public string $scopes = ''; - public ?int $userId = null; public ?string $provider = null; public ?string $name = null; public ?string $handle = null; diff --git a/src/records/App.php b/src/records/App.php index 0cf2990..706a2ad 100644 --- a/src/records/App.php +++ b/src/records/App.php @@ -23,7 +23,6 @@ * @property DateTime $dateCreated * @property DateTime $dateUpdated * @property string $uid - * @property int $userId * @property string $name * @property string $provider * @property string $handle @@ -44,15 +43,6 @@ public static function tableName() return '{{%oauthclient_apps}}'; } - /** - * Returns the OAuth Tokens’s user. - * - * @return ActiveQueryInterface The relational query object. - */ - public function getUser(): ActiveQueryInterface - { - return $this->hasOne(User::class, ['id' => 'userId']); - } /** * @return ActiveQueryInterface diff --git a/src/services/Apps.php b/src/services/Apps.php index fcc1314..a2b0f01 100644 --- a/src/services/Apps.php +++ b/src/services/Apps.php @@ -86,7 +86,6 @@ private function _createAppQuery(): Query 'id', 'provider', 'name', - 'userId', 'dateCreated', 'dateUpdated', 'clientId', @@ -107,7 +106,6 @@ private function _createAppQuery(): Query public function createApp($config): AppModel { $app = new AppModel($config); - $app->userId = $app->userId ?? Craft::$app->user->getId(); return $app; }