From 0d4cc49a4ca9f167db8016a980b2bd25a669455f Mon Sep 17 00:00:00 2001 From: Mikkel Ricky Date: Wed, 1 May 2024 14:46:43 +0200 Subject: [PATCH] Cleaned up settings form --- README.md | 3 +- .../src/Form/SettingsForm.php | 66 ++++++++++--------- .../src/Helper/CertificateLocatorHelper.php | 40 ++++++----- .../src/Helper/MemoryCertificateLocator.php | 23 ++++--- .../src/Helper/Settings.php | 17 ++--- 5 files changed, 81 insertions(+), 68 deletions(-) diff --git a/README.md b/README.md index 12e4ad05..c2f09623 100644 --- a/README.md +++ b/README.md @@ -122,10 +122,9 @@ run the checks locally. ```sh docker run --rm --volume ${PWD}:/app --workdir /app itkdev/php8.1-fpm composer install -docker run --rm --volume ${PWD}:/app --workdir /app itkdev/php8.1-fpm composer coding-standards-check - # Fix (some) coding standards issues. docker run --rm --volume ${PWD}:/app --workdir /app itkdev/php8.1-fpm composer coding-standards-apply +docker run --rm --volume ${PWD}:/app --workdir /app itkdev/php8.1-fpm composer coding-standards-check ``` ### Markdown diff --git a/modules/os2forms_digital_post/src/Form/SettingsForm.php b/modules/os2forms_digital_post/src/Form/SettingsForm.php index 3780b439..9f4b93d6 100644 --- a/modules/os2forms_digital_post/src/Form/SettingsForm.php +++ b/modules/os2forms_digital_post/src/Form/SettingsForm.php @@ -19,6 +19,11 @@ final class SettingsForm extends FormBase { use StringTranslationTrait; + public const TEST_MODE = 'test_mode'; + public const SENDER = 'sender'; + public const CERTIFICATE = 'certificate'; + public const PROCESSING = 'processing'; + /** * The queue storage. * @@ -63,15 +68,15 @@ public function getFormId() { * @phpstan-param array $form * @phpstan-return array */ - public function buildForm(array $form, FormStateInterface $form_state) { - $form['test_mode'] = [ + public function buildForm(array $form, FormStateInterface $form_state): array { + $form[self::TEST_MODE] = [ '#type' => 'checkbox', '#title' => $this->t('Test mode'), '#default_value' => $this->settings->getTestMode(), ]; $sender = $this->settings->getSender(); - $form['sender'] = [ + $form[self::SENDER] = [ '#type' => 'fieldset', '#title' => $this->t('Sender'), '#tree' => TRUE, @@ -102,12 +107,12 @@ public function buildForm(array $form, FormStateInterface $form_state) { ]; $certificate = $this->settings->getCertificate(); - $form['certificate'] = [ + $form[self::CERTIFICATE] = [ '#type' => 'fieldset', '#title' => $this->t('Certificate'), '#tree' => TRUE, - 'locator_type' => [ + CertificateLocatorHelper::LOCATOR_TYPE => [ '#type' => 'select', '#title' => $this->t('Certificate locator type'), '#options' => [ @@ -115,11 +120,11 @@ public function buildForm(array $form, FormStateInterface $form_state) { CertificateLocatorHelper::LOCATOR_TYPE_AZURE_KEY_VAULT => $this->t('Azure key vault'), CertificateLocatorHelper::LOCATOR_TYPE_FILE_SYSTEM => $this->t('File system'), ], - '#default_value' => $certificate['locator_type'] ?? NULL, + '#default_value' => $certificate[CertificateLocatorHelper::LOCATOR_TYPE] ?? NULL, ], ]; - $form['certificate'][CertificateLocatorHelper::LOCATOR_TYPE_KEY] = [ + $form[self::CERTIFICATE][CertificateLocatorHelper::LOCATOR_TYPE_KEY] = [ '#type' => 'container', '#states' => [ 'visible' => [':input[name="certificate[locator_type]"]' => ['value' => CertificateLocatorHelper::LOCATOR_TYPE_KEY]], @@ -138,7 +143,7 @@ public function buildForm(array $form, FormStateInterface $form_state) { ], ]; - $form['certificate'][CertificateLocatorHelper::LOCATOR_TYPE_AZURE_KEY_VAULT] = [ + $form[self::CERTIFICATE][CertificateLocatorHelper::LOCATOR_TYPE_AZURE_KEY_VAULT] = [ '#type' => 'fieldset', '#title' => $this->t('Azure key vault'), '#states' => [ @@ -147,16 +152,16 @@ public function buildForm(array $form, FormStateInterface $form_state) { ]; $settings = [ - 'tenant_id' => ['title' => $this->t('Tenant id')], - 'application_id' => ['title' => $this->t('Application id')], - 'client_secret' => ['title' => $this->t('Client secret')], - 'name' => ['title' => $this->t('Name')], - 'secret' => ['title' => $this->t('Secret')], - 'version' => ['title' => $this->t('Version')], + CertificateLocatorHelper::LOCATOR_AZURE_KEY_VAULT_TENANT_ID => ['title' => $this->t('Tenant id')], + CertificateLocatorHelper::LOCATOR_AZURE_KEY_VAULT_APPLICATION_ID => ['title' => $this->t('Application id')], + CertificateLocatorHelper::LOCATOR_AZURE_KEY_VAULT_CLIENT_SECRET => ['title' => $this->t('Client secret')], + CertificateLocatorHelper::LOCATOR_AZURE_KEY_VAULT_NAME => ['title' => $this->t('Name')], + CertificateLocatorHelper::LOCATOR_AZURE_KEY_VAULT_SECRET => ['title' => $this->t('Secret')], + CertificateLocatorHelper::LOCATOR_AZURE_KEY_VAULT_VERSION => ['title' => $this->t('Version')], ]; foreach ($settings as $key => $info) { - $form['certificate'][CertificateLocatorHelper::LOCATOR_TYPE_AZURE_KEY_VAULT][$key] = [ + $form[self::CERTIFICATE][CertificateLocatorHelper::LOCATOR_TYPE_AZURE_KEY_VAULT][$key] = [ '#type' => 'textfield', '#title' => $info['title'], '#default_value' => $certificate[CertificateLocatorHelper::LOCATOR_TYPE_AZURE_KEY_VAULT][$key] ?? NULL, @@ -166,7 +171,7 @@ public function buildForm(array $form, FormStateInterface $form_state) { ]; } - $form['certificate'][CertificateLocatorHelper::LOCATOR_TYPE_FILE_SYSTEM] = [ + $form[self::CERTIFICATE][CertificateLocatorHelper::LOCATOR_TYPE_FILE_SYSTEM] = [ '#type' => 'fieldset', '#title' => $this->t('File system'), '#states' => [ @@ -183,29 +188,29 @@ public function buildForm(array $form, FormStateInterface $form_state) { ], ]; - $form['certificate']['passphrase'] = [ + $form[self::CERTIFICATE][CertificateLocatorHelper::LOCATOR_PASSPHRASE] = [ '#type' => 'textfield', '#title' => $this->t('Passphrase'), - '#default_value' => $certificate['passphrase'] ?? NULL, - + '#default_value' => $certificate[CertificateLocatorHelper::LOCATOR_PASSPHRASE] ?? NULL, '#states' => [ - 'visible' => [':input[name="certificate[locator_type]"]' => [ + 'visible' => [ + ':input[name="certificate[locator_type]"]' => [ ['value' => CertificateLocatorHelper::LOCATOR_TYPE_AZURE_KEY_VAULT], ['value' => CertificateLocatorHelper::LOCATOR_TYPE_FILE_SYSTEM], - ], + ], ], ], ]; $processing = $this->settings->getProcessing(); - $form['processing'] = [ + $form[self::PROCESSING] = [ '#type' => 'fieldset', '#title' => $this->t('Processing'), '#tree' => TRUE, ]; $defaultValue = $processing['queue'] ?? 'os2forms_digital_post'; - $form['processing']['queue'] = [ + $form[self::PROCESSING]['queue'] = [ '#type' => 'select', '#title' => $this->t('Queue'), '#options' => array_map( @@ -247,8 +252,8 @@ public function validateForm(array &$form, FormStateInterface $formState): void } $values = $formState->getValues(); - if (CertificateLocatorHelper::LOCATOR_TYPE_FILE_SYSTEM === $values['certificate']['locator_type']) { - $path = $values['certificate'][CertificateLocatorHelper::LOCATOR_TYPE_FILE_SYSTEM]['path'] ?? NULL; + if (CertificateLocatorHelper::LOCATOR_TYPE_FILE_SYSTEM === $values[self::CERTIFICATE][CertificateLocatorHelper::LOCATOR_TYPE]) { + $path = $values[self::CERTIFICATE][CertificateLocatorHelper::LOCATOR_TYPE_FILE_SYSTEM]['path'] ?? NULL; if (!file_exists($path)) { $formState->setErrorByName('certificate][file_system][path', $this->t('Invalid certificate path: %path', ['%path' => $path])); } @@ -268,10 +273,11 @@ public function submitForm(array &$form, FormStateInterface $formState): void { } try { - $settings['test_mode'] = (bool) $formState->getValue('test_mode'); - $settings['sender'] = $formState->getValue('sender'); - $settings['certificate'] = $formState->getValue('certificate'); - $settings['processing'] = $formState->getValue('processing'); + $settings[self::TEST_MODE] = (bool) $formState->getValue(self::TEST_MODE); + $settings[self::SENDER] = $formState->getValue(self::SENDER); + $settings[self::CERTIFICATE] = $formState->getValue(self::CERTIFICATE); + $settings[self::PROCESSING] = $formState->getValue(self::PROCESSING); + $this->settings->setSettings($settings); $this->messenger()->addStatus($this->t('Settings saved')); } @@ -287,7 +293,7 @@ private function testCertificate(): void { try { $certificateLocator = $this->certificateLocatorHelper->getCertificateLocator(); $certificateLocator->getCertificates(); - $this->messenger()->addStatus($this->t('Certificate succesfully tested')); + $this->messenger()->addStatus($this->t('Certificate successfully tested')); } catch (\Throwable $throwable) { $message = $this->t('Error testing certificate: %message', ['%message' => $throwable->getMessage()]); diff --git a/modules/os2forms_digital_post/src/Helper/CertificateLocatorHelper.php b/modules/os2forms_digital_post/src/Helper/CertificateLocatorHelper.php index 9458f3d3..79953091 100644 --- a/modules/os2forms_digital_post/src/Helper/CertificateLocatorHelper.php +++ b/modules/os2forms_digital_post/src/Helper/CertificateLocatorHelper.php @@ -17,16 +17,25 @@ * Certificate locator helper. */ class CertificateLocatorHelper { + public const LOCATOR_TYPE = 'locator_type'; public const LOCATOR_TYPE_AZURE_KEY_VAULT = 'azure_key_vault'; public const LOCATOR_TYPE_FILE_SYSTEM = 'file_system'; public const LOCATOR_TYPE_KEY = 'key'; + public const LOCATOR_PASSPHRASE = 'passphrase'; + public const LOCATOR_AZURE_KEY_VAULT_TENANT_ID = 'tenant_id'; + public const LOCATOR_AZURE_KEY_VAULT_APPLICATION_ID = 'application_id'; + public const LOCATOR_AZURE_KEY_VAULT_CLIENT_SECRET = 'client_secret'; + public const LOCATOR_AZURE_KEY_VAULT_NAME = 'name'; + public const LOCATOR_AZURE_KEY_VAULT_SECRET = 'secret'; + public const LOCATOR_AZURE_KEY_VAULT_VERSION = 'version'; + public const LOCATOR_FILE_SYSTEM_PATH = 'path'; /** - * {@inheritdoc} + * Constructor. */ public function __construct( private readonly Settings $settings, - private readonly KeyRepositoryInterface $keyRepository + private readonly KeyRepositoryInterface $keyRepository, ) { } @@ -36,10 +45,10 @@ public function __construct( public function getCertificateLocator(): CertificateLocatorInterface { $certificateSettings = $this->settings->getCertificate(); - $locatorType = $certificateSettings['locator_type']; + $locatorType = $certificateSettings[self::LOCATOR_TYPE]; $options = $certificateSettings[$locatorType]; $options += [ - 'passphrase' => $certificateSettings['passphrase'] ?: '', + self::LOCATOR_PASSPHRASE => $certificateSettings[self::LOCATOR_PASSPHRASE] ?: '', ]; switch ($locatorType) { @@ -60,32 +69,31 @@ public function getCertificateLocator(): CertificateLocatorInterface { $vaultToken = new VaultToken($httpClient, $requestFactory); $token = $vaultToken->getToken( - $options['tenant_id'], - $options['application_id'], - $options['client_secret'], + $options[self::LOCATOR_AZURE_KEY_VAULT_TENANT_ID], + $options[self::LOCATOR_AZURE_KEY_VAULT_APPLICATION_ID], + $options[self::LOCATOR_AZURE_KEY_VAULT_CLIENT_SECRET], ); $vault = new VaultSecret( $httpClient, $requestFactory, - $options['name'], + $options[self::LOCATOR_AZURE_KEY_VAULT_NAME], $token->getAccessToken() ); return new AzureKeyVaultCertificateLocator( $vault, - $options['secret'], - $options['version'], - $options['passphrase'], + $options[self::LOCATOR_AZURE_KEY_VAULT_SECRET], + $options[self::LOCATOR_AZURE_KEY_VAULT_VERSION], + $options[self::LOCATOR_PASSPHRASE], ); case self::LOCATOR_TYPE_FILE_SYSTEM: - $certificatepath = realpath($options['path']) ?: null; - if (null === $certificatepath) { - throw new CertificateLocatorException(sprintf('Invalid certificate path %s', $options['path'])); + $certificatepath = realpath($options[self::LOCATOR_FILE_SYSTEM_PATH]) ?: NULL; + if (NULL === $certificatepath) { + throw new CertificateLocatorException(sprintf('Invalid certificate path %s', $options[self::LOCATOR_FILE_SYSTEM_PATH])); } - - return new FilesystemCertificateLocator($certificatepath, $options['passphrase']); + return new FilesystemCertificateLocator($certificatepath, $options[self::LOCATOR_PASSPHRASE]); default: throw new CertificateLocatorException(sprintf('Invalid certificate locator type: %s', $locatorType)); diff --git a/modules/os2forms_digital_post/src/Helper/MemoryCertificateLocator.php b/modules/os2forms_digital_post/src/Helper/MemoryCertificateLocator.php index ac63996b..2e8effe2 100644 --- a/modules/os2forms_digital_post/src/Helper/MemoryCertificateLocator.php +++ b/modules/os2forms_digital_post/src/Helper/MemoryCertificateLocator.php @@ -5,21 +5,22 @@ use ItkDev\Serviceplatformen\Certificate\AbstractCertificateLocator; use ItkDev\Serviceplatformen\Certificate\Exception\CertificateLocatorException; -class MemoryCertificateLocator extends AbstractCertificateLocator -{ +/** + * Memory certificate locator. + */ +class MemoryCertificateLocator extends AbstractCertificateLocator { + public function __construct( // The passwordless certificate. - private readonly string $certificate - ) - { + private readonly string $certificate, + ) { parent::__construct(''); } /** * {@inheritdoc} */ - public function getCertificates(): array - { + public function getCertificates(): array { $certificates = []; if (!openssl_pkcs12_read($this->certificate, $certificates, $this->passphrase)) { throw new CertificateLocatorException('Could not read certificate.'); @@ -31,17 +32,15 @@ public function getCertificates(): array /** * {@inheritdoc} */ - public function getCertificate(): string - { + public function getCertificate(): string { return $this->certificate; } /** * {@inheritdoc} */ - public function getAbsolutePathToCertificate(): string - { - throw new CertificateLocatorException(__METHOD__.' should not be used.'); + public function getAbsolutePathToCertificate(): string { + throw new CertificateLocatorException(__METHOD__ . ' should not be used.'); } } diff --git a/modules/os2forms_digital_post/src/Helper/Settings.php b/modules/os2forms_digital_post/src/Helper/Settings.php index e64be738..33e6040c 100644 --- a/modules/os2forms_digital_post/src/Helper/Settings.php +++ b/modules/os2forms_digital_post/src/Helper/Settings.php @@ -5,6 +5,7 @@ use Drupal\Core\KeyValueStore\KeyValueFactoryInterface; use Drupal\Core\KeyValueStore\KeyValueStoreInterface; use Drupal\os2forms_digital_post\Exception\InvalidSettingException; +use Drupal\os2forms_digital_post\Form\SettingsForm; use Symfony\Component\OptionsResolver\OptionsResolver; /** @@ -40,7 +41,7 @@ public function __construct(KeyValueFactoryInterface $keyValueFactory) { * Get test mode. */ public function getTestMode(): bool { - return (bool) $this->get('test_mode', TRUE); + return (bool) $this->get(SettingsForm::TEST_MODE, TRUE); } /** @@ -49,7 +50,7 @@ public function getTestMode(): bool { * @phpstan-return array */ public function getSender(): array { - $value = $this->get('sender'); + $value = $this->get(SettingsForm::SENDER); return is_array($value) ? $value : []; } @@ -59,7 +60,7 @@ public function getSender(): array { * @phpstan-return array */ public function getCertificate(): array { - $value = $this->get('certificate'); + $value = $this->get(SettingsForm::CERTIFICATE); return is_array($value) ? $value : []; } @@ -69,7 +70,7 @@ public function getCertificate(): array { * @phpstan-return array */ public function getProcessing(): array { - $value = $this->get('processing'); + $value = $this->get(SettingsForm::PROCESSING); return is_array($value) ? $value : []; } @@ -115,10 +116,10 @@ public function setSettings(array $settings): self { private function getSettingsResolver(): OptionsResolver { return (new OptionsResolver()) ->setDefaults([ - 'test_mode' => TRUE, - 'sender' => [], - 'certificate' => [], - 'processing' => [], + SettingsForm::TEST_MODE => TRUE, + SettingsForm::SENDER => [], + SettingsForm::CERTIFICATE => [], + SettingsForm::PROCESSING => [], ]); }