Cleaned up settings form

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

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<string, mixed> $form
    * @phpstan-return array<string, mixed>
    */
-  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,24 +107,24 @@ 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' => [
           CertificateLocatorHelper::LOCATOR_TYPE_KEY => $this->t('Key'),
           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()]);

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));

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.');
   }
 
 }

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<string, mixed>
    */
   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<string, mixed>
    */
   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<string, mixed>
    */
   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 => [],
       ]);
   }