Skip to content

Commit

Permalink
chore: Refactor basic auth
Browse files Browse the repository at this point in the history
  • Loading branch information
johnnynotsolucky committed Nov 18, 2024
1 parent ca59556 commit 609c07d
Show file tree
Hide file tree
Showing 8 changed files with 27 additions and 149 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
18 changes: 0 additions & 18 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
31 changes: 0 additions & 31 deletions src/Plugin.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand All @@ -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',
],
],
];
}
);
}
Expand Down
38 changes: 9 additions & 29 deletions src/controllers/OrdersController.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -36,42 +35,24 @@ 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.
* If this is not found or is any other string, this will throw a 400 exception.
*
* @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'),
};
}

/**
Expand All @@ -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);
}

/**
Expand Down
8 changes: 1 addition & 7 deletions src/controllers/SettingsController.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,14 @@ public function actionIndex(): Response
$plugin = Plugin::getInstance();
return $this->renderTemplate('shipstationconnect/settings/index', [
'settings' => $plugin?->settings,
'isUsingCraftAuth' => $plugin?->isAuthHandledByCraft(),
]);
}

/**
* @throws MissingComponentException
* @throws BadRequestHttpException
*/
public function actionSave(): Response
public function actionSave(): void
{
$this->requirePostRequest();
/** @var Application $app */
Expand All @@ -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();
}
}
10 changes: 10 additions & 0 deletions src/models/Settings.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
17 changes: 5 additions & 12 deletions src/templates/settings/index.twig
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@
{% endblock %}

{% set content %}
<input type="hidden" name="action" value="shipstationconnect/settings/save">
<input type="hidden" name="action">
{{ actionInput('shipstationconnect/settings/save') }}
{{ redirectInput('shipstationconnect/settings') }}
<div>
<h3>Custom Store</h3>
Expand Down Expand Up @@ -40,7 +41,7 @@
</div>
{% 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({
Expand Down Expand Up @@ -82,12 +83,6 @@
</div>
</div>
{% endif %}
{% if isUsingCraftAuth %}
<p class="error">
This site is using Craft's enableBasicHttpAuth feature. A dedicated user
should be added to access ShipStation Connect.
</p>
{% endif %}

{{ forms.autosuggestField({
label: 'Username'|t('shipstationconnect'),
Expand All @@ -98,7 +93,6 @@
errors: settings.getErrors('shipstationUsername'),
class: 'ltr',
suggestEnvVars: true,
disabled: isUsingCraftAuth,
}) }}

{{ forms.autosuggestField({
Expand All @@ -111,7 +105,6 @@
errors: settings.getErrors('shipstationPassword'),
class: 'ltr',
suggestEnvVars: true,
disabled: isUsingCraftAuth,
}) }}

{{ forms.textField({
Expand Down Expand Up @@ -158,7 +151,7 @@
</ul>
</div>

{% 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]',
Expand All @@ -177,7 +170,7 @@

<h2>Shipping Info Matrix Field</h2>

{% 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 %}
<div class="error">
Expand Down
52 changes: 0 additions & 52 deletions src/web/twig/filters/IsFieldTypeFilter.php

This file was deleted.

0 comments on commit 609c07d

Please sign in to comment.