From 609c07db1520e578203e4fccf8984c7a4b1cb2e7 Mon Sep 17 00:00:00 2001 From: Tyrone Tudehope Date: Mon, 18 Nov 2024 12:13:19 +0200 Subject: [PATCH] chore: Refactor basic auth --- CHANGELOG.md | 2 + README.md | 18 -------- src/Plugin.php | 31 ------------- src/controllers/OrdersController.php | 38 ++++------------ src/controllers/SettingsController.php | 8 +--- src/models/Settings.php | 10 +++++ src/templates/settings/index.twig | 17 +++---- src/web/twig/filters/IsFieldTypeFilter.php | 52 ---------------------- 8 files changed, 27 insertions(+), 149 deletions(-) delete mode 100644 src/web/twig/filters/IsFieldTypeFilter.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 2a1e7f3..813b213 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,8 @@ - Added `failOnValidation` config item and toggle in settings UI. - Removed the `Xml::ORDER_FIELD_EVENT` event. `Xml::ORDER_EVENT` should be used instead. - Removed `OrderFieldEvent`. All details about an order can be updated in a single event now. +- Removed support for Craft's basic authentication through a dedicated Craft user. +- Removed unnecessary Twig filters, `is_matrix`, `is_dropdown` and `is_asset`. ## 2.1.0 - 2024-05-24 diff --git a/README.md b/README.md index 1cf5ee8..89e5fe1 100644 --- a/README.md +++ b/README.md @@ -57,24 +57,6 @@ credentials. As of version 1.2.4, these values can be set with environment variables. ![Username/Password variables](screenshots/username-password-env-values.png) -#### Using Craft's Basic Authentication - -As of Craft 3.5.0 basic authentication headers can be used to authenticate users -in Craft by setting the -[enableBasicHttpAuth](https://github.com/craftcms/cms/commit/0bb12973635f8cd3cfa11e97b94306dc643c054b) -config setting to `true`. - -If basic authentication is enabled for your site, -ShipStation Connect will assume that requests to it have already been -authenticated and continue processing. Using this feature removes the -requirement to set a username/password in the settings and instead it is -recommended to create a dedicated user with the -`shipstationconnect-processOrders` permission for accessing ShipStation Connect. - -When `enableBasicHttpAuth` is `false`, the plugin will read the auth header and -validate against the username/password configured in ShipStation Connect's -settings. - #### Debugging Apache Authentication Errors > The remote server returned an error diff --git a/src/Plugin.php b/src/Plugin.php index 4e4fcb1..03af1a1 100644 --- a/src/Plugin.php +++ b/src/Plugin.php @@ -5,12 +5,9 @@ use Craft; use craft\base\Model; use craft\events\RegisterUrlRulesEvent; -use craft\events\RegisterUserPermissionsEvent; -use craft\services\UserPermissions; use craft\web\UrlManager; use fostercommerce\shipstationconnect\models\Settings; use fostercommerce\shipstationconnect\services\Xml; -use fostercommerce\shipstationconnect\web\twig\filters\IsFieldTypeFilter; use yii\base\Event; /** @@ -33,39 +30,11 @@ public function init(): void 'xml' => Xml::class, ]); - Craft::$app->view->registerTwigExtension(new IsFieldTypeFilter()); - Event::on( UrlManager::class, UrlManager::EVENT_REGISTER_CP_URL_RULES, function (RegisterUrlRulesEvent $event): void { $event->rules['shipstationconnect/settings'] = 'shipstationconnect/settings/index'; - $event->rules['shipstationconnect/settings/save'] = 'shipstationconnect/settings/save'; - } - ); - - - Event::on( - UrlManager::class, - UrlManager::EVENT_REGISTER_SITE_URL_RULES, - function (RegisterUrlRulesEvent $event): void { - $event->rules['export'] = 'shipstationconnect/orders/export'; - } - ); - - - Event::on( - UserPermissions::class, - UserPermissions::EVENT_REGISTER_PERMISSIONS, - function (RegisterUserPermissionsEvent $event): void { - $event->permissions[] = [ - 'heading' => 'ShipStation Connect', - 'permissions' => [ - 'shipstationconnect-processOrders' => [ - 'label' => 'Process Orders', - ], - ], - ]; } ); } diff --git a/src/controllers/OrdersController.php b/src/controllers/OrdersController.php index 59453ea..4d25875 100644 --- a/src/controllers/OrdersController.php +++ b/src/controllers/OrdersController.php @@ -6,7 +6,6 @@ use craft\commerce\elements\db\OrderQuery; use craft\commerce\elements\Order; use craft\commerce\Plugin as CommercePlugin; -use craft\db\Query; use craft\elements\Entry; use craft\fields\Matrix; use craft\helpers\App; @@ -36,20 +35,6 @@ class OrdersController extends Controller protected array|int|bool $allowAnonymous = true; - public function init(): void - { - // Allow anonymous access only when this plugin is handling basic - // authentication, otherwise require auth so that Craft doesn't let - // unauthenticated requests go through. - $isUsingCraftAuth = Plugin::getInstance()?->isAuthHandledByCraft(); - $this->allowAnonymous = ! $isUsingCraftAuth; - if ($isUsingCraftAuth) { - $this->requirePermission('shipstationconnect-processOrders'); - } - - parent::init(); - } - /** * ShipStation will hit this action for processing orders, both POSTing and GETting. * ShipStation will send a GET param 'action' of either shipnotify or export. @@ -57,21 +42,17 @@ public function init(): void * * @throws HttpException */ - public function actionProcess(?string $store = null, ?string $action = null): YiiResponse + public function actionProcess(?string $store = null, ?string $action = null): ?YiiResponse { if (! $this->authenticate()) { throw new HttpException(401, 'Invalid ShipStation username or password.'); } - switch ($action) { - case 'export': - $this->getOrders($store); - // no break - case 'shipnotify': - return $this->postShipment(); - default: - throw new HttpException(400, 'No action set. Set the ?action= parameter as `export` or `shipnotify`.'); - } + return match ($action) { + 'export' => $this->getOrders($store), + 'shipnotify' => $this->postShipment(), + default => throw new HttpException(400, 'Invalid action'), + }; } /** @@ -82,16 +63,15 @@ public function actionProcess(?string $store = null, ?string $action = null): Yi protected function authenticate(): bool { $plugin = Plugin::getInstance(); - if ($plugin?->isAuthHandledByCraft()) { - return true; - } + /** @var string $expectedUsername */ $expectedUsername = App::parseEnv($plugin?->settings->shipstationUsername); + /** @var string $expectedPassword */ $expectedPassword = App::parseEnv($plugin?->settings->shipstationPassword); [$username, $password] = $this->getApp()->getRequest()->getAuthCredentials(); - return $expectedUsername === $username && $expectedPassword === $password; + return hash_equals($expectedUsername, $username) && hash_equals($expectedPassword, $password); } /** diff --git a/src/controllers/SettingsController.php b/src/controllers/SettingsController.php index 7bbdfd6..0451e3a 100644 --- a/src/controllers/SettingsController.php +++ b/src/controllers/SettingsController.php @@ -19,7 +19,6 @@ public function actionIndex(): Response $plugin = Plugin::getInstance(); return $this->renderTemplate('shipstationconnect/settings/index', [ 'settings' => $plugin?->settings, - 'isUsingCraftAuth' => $plugin?->isAuthHandledByCraft(), ]); } @@ -27,7 +26,7 @@ public function actionIndex(): Response * @throws MissingComponentException * @throws BadRequestHttpException */ - public function actionSave(): Response + public function actionSave(): void { $this->requirePostRequest(); /** @var Application $app */ @@ -44,13 +43,8 @@ public function actionSave(): Response if (! $settings->validate() || ! Craft::$app->getPlugins()->savePluginSettings($plugin, $settings->toArray())) { Craft::$app->getSession()->setError(Craft::t('shipstationconnect', 'Couldn’t save settings.')); - return $this->renderTemplate('shipstationconnect/settings/index', [ - 'settings' => $settings, - ]); } Craft::$app->getSession()->setNotice(Craft::t('shipstationconnect', 'Settings saved.')); - - return $this->redirectToPostedUrl(); } } diff --git a/src/models/Settings.php b/src/models/Settings.php index cebea4c..3689ea7 100644 --- a/src/models/Settings.php +++ b/src/models/Settings.php @@ -40,4 +40,14 @@ class Settings extends Model public bool $billingSameAsShipping = false; public string $phoneNumberFieldHandle = ''; + + /** + * Utility to help filter out fields we don't want to list on the settings page. + * + * @param class-string $className + */ + public static function isA(string $className, mixed $target): bool + { + return $target instanceof $className; + } } diff --git a/src/templates/settings/index.twig b/src/templates/settings/index.twig index 1ad0638..68fadcb 100644 --- a/src/templates/settings/index.twig +++ b/src/templates/settings/index.twig @@ -12,7 +12,8 @@ {% endblock %} {% set content %} - + + {{ actionInput('shipstationconnect/settings/save') }} {{ redirectInput('shipstationconnect/settings') }}

Custom Store

@@ -40,7 +41,7 @@
{% set fields = craft.app.fields.allFields %} {% set entryTypes = craft.app.entries.allEntryTypes %} - {% set storeFields = fields|filter(f => is_dropdown(f))|map(f => { label: f.name, value: f.handle }) %} + {% set storeFields = fields|filter(f => settings.isA('craft\\fields\\Dropdown', f))|map(f => { label: f.name, value: f.handle }) %} {% set storeFields = [{ label: 'Default', value: '' }]|merge(storeFields) %} {{ forms.selectField({ @@ -82,12 +83,6 @@ {% endif %} - {% if isUsingCraftAuth %} -

- This site is using Craft's enableBasicHttpAuth feature. A dedicated user - should be added to access ShipStation Connect. -

- {% endif %} {{ forms.autosuggestField({ label: 'Username'|t('shipstationconnect'), @@ -98,7 +93,6 @@ errors: settings.getErrors('shipstationUsername'), class: 'ltr', suggestEnvVars: true, - disabled: isUsingCraftAuth, }) }} {{ forms.autosuggestField({ @@ -111,7 +105,6 @@ errors: settings.getErrors('shipstationPassword'), class: 'ltr', suggestEnvVars: true, - disabled: isUsingCraftAuth, }) }} {{ forms.textField({ @@ -158,7 +151,7 @@ - {% set assetFields = [{label: 'None', value: null}]|merge(fields|filter(f => is_asset(f))|map(f => { label: f.name, value: f.handle })) %} + {% set assetFields = [{label: 'None', value: null}]|merge(fields|filter(f => settings.isA('craft\\fields\\Assets', f))|map(f => { label: f.name, value: f.handle })) %} {{ forms.selectField({ label: "Product Images Field Handle"|t('shipstationconnect'), name: 'settings[productImagesHandle]', @@ -177,7 +170,7 @@

Shipping Info Matrix Field

- {% set matrixFields = fields|filter(f => is_matrix(f))|map(f => { label: f.name, value: f.handle }) %} + {% set matrixFields = fields|filter(f => settings.isA('craft\\fields\\Matrix', f))|map(f => { label: f.name, value: f.handle }) %} {% if matrixFields|length == 0 or settings.matrixFieldHandle is null or settings.matrixFieldHandle|length == 0 %}
diff --git a/src/web/twig/filters/IsFieldTypeFilter.php b/src/web/twig/filters/IsFieldTypeFilter.php deleted file mode 100644 index 6049b29..0000000 --- a/src/web/twig/filters/IsFieldTypeFilter.php +++ /dev/null @@ -1,52 +0,0 @@ -isMatrixField(...)), - new TwigFilter('is_dropdown', $this->isDropdownField(...)), - new TwigFilter('is_asset', $this->isAssetField(...)), - ]; - } - - public function getFunctions(): array - { - return [ - new TwigFunction('is_matrix', $this->isMatrixField(...)), - new TwigFunction('is_dropdown', $this->isDropdownField(...)), - new TwigFunction('is_asset', $this->isAssetField(...)), - ]; - } - - public function isMatrixField(Field $field): bool - { - return $field instanceof Matrix; - } - - public function isDropdownField(Field $field): bool - { - return $field instanceof Dropdown; - } - - public function isAssetField(Field $field): bool - { - return $field instanceof Assets; - } -}