From a68b7a71157e42f31c50e276027cf8b2297f783f Mon Sep 17 00:00:00 2001 From: Mikkel Ricky Date: Wed, 1 May 2024 14:59:47 +0200 Subject: [PATCH 1/6] Added support for os2web_key --- .github/workflows/pr.yaml | 58 ++----- .gitignore | 2 - .markdownlint.jsonc | 13 ++ .markdownlintrc | 9 - CHANGELOG.md | 9 +- README.md | 63 ++++--- composer.json | 65 +++---- drush.services.yml | 6 + os2forms_fasit.info.yml | 4 +- os2forms_fasit.install | 15 ++ os2forms_fasit.services.yml | 9 +- package.json | 13 -- phpstan.neon | 11 ++ scripts/code-analysis | 48 ++++++ src/Drush/Commands/FasitTestCommands.php | 38 +++++ src/Form/SettingsForm.php | 208 ++++++++--------------- src/Helper/CertificateLocatorHelper.php | 86 ---------- src/Helper/FasitHelper.php | 107 +++++++----- src/Helper/Settings.php | 93 ++++------ 19 files changed, 400 insertions(+), 457 deletions(-) create mode 100644 .markdownlint.jsonc delete mode 100644 .markdownlintrc create mode 100644 drush.services.yml create mode 100644 os2forms_fasit.install delete mode 100644 package.json create mode 100644 phpstan.neon create mode 100755 scripts/code-analysis create mode 100644 src/Drush/Commands/FasitTestCommands.php delete mode 100644 src/Helper/CertificateLocatorHelper.php diff --git a/.github/workflows/pr.yaml b/.github/workflows/pr.yaml index 1b14b6d..a181a0d 100644 --- a/.github/workflows/pr.yaml +++ b/.github/workflows/pr.yaml @@ -49,22 +49,9 @@ jobs: composer validate --strict composer.json # Check that dependencies resolve. composer update --${{ matrix.dependency-version }} --prefer-dist --no-interaction - - markdown-coding-standards: - name: Yarn - Check Coding Standards (Node ${{ matrix.node }}) - runs-on: ubuntu-latest - strategy: - matrix: - node: [ '20' ] - steps: - - uses: actions/checkout@v2 - - name: Setup node - uses: actions/setup-node@v2 - with: - node-version: ${{ matrix.node }} - - run: | - yarn install - yarn coding-standards-check + - name: Check that composer file is normalized + run: | + composer normalize --dry-run php-check-coding-standards: name: PHP - Check Coding Standards @@ -125,34 +112,17 @@ jobs: path: ${{ steps.composer-cache.outputs.dir }} key: ${{ runner.os }}-composer-${{ hashFiles('**/composer.lock') }} restore-keys: ${{ runner.os }}-composer- - - name: drupal-check + - name: Code analysis run: | - # We need a Drupal project to run drupal-check (cf. https://github.com/mglaman/drupal-check#usage) - # Install Drupal - composer --no-interaction create-project drupal/recommended-project:^9 --stability=dev drupal - # Copy our module source code into the Drupal module folder. - mkdir -p drupal/web/modules/contrib/os2forms_fasit - cp -r os2forms_fasit.* composer.json src drupal/web/modules/contrib/os2forms_fasit - # Add our module as a composer repository. - composer --no-interaction --working-dir=drupal config repositories.os2forms/os2forms_fasit path web/modules/contrib/os2forms_fasit - # Restore Drupal composer repository. - composer --no-interaction --working-dir=drupal config repositories.drupal composer https://packages.drupal.org/8 - - composer --no-interaction --working-dir=drupal config --no-plugins allow-plugins.cweagans/composer-patches true - composer --no-interaction --working-dir=drupal config --no-plugins allow-plugins.zaporylie/composer-drupal-optimizations true - composer --no-interaction --working-dir=drupal config --no-plugins allow-plugins.simplesamlphp/composer-module-installer true - # @see https://getcomposer.org/doc/03-cli.md#modifying-extra-values - composer --no-interaction --working-dir=drupal config --no-plugins --json extra.enable-patching true + ./scripts/code-analysis - # Require our module. - composer --no-interaction --working-dir=drupal require 'os2forms/os2forms_fasit:*' + coding-standards-markdown: + name: Markdown coding standards + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@master - # Check code - composer --no-interaction --working-dir=drupal require --dev drupal/core-dev - cd drupal/web/modules/contrib/os2forms_fasit - # Remove our non-dev dependencies to prevent duplicated Drupal installation - # PHP Fatal error: Cannot redeclare drupal_get_filename() (previously declared in /home/runner/work/os2forms_fasit/os2forms_fasit/drupal/web/modules/contrib/os2forms_fasit/vendor/drupal/core/includes/bootstrap.inc:190) in /home/runner/work/os2forms_fasit/os2forms_fasit/drupal/web/core/includes/bootstrap.inc on line 190 - # Use sed to remove the "require" property in composer.json - sed -i '/^\s*"require":/,/^\s*}/d' composer.json - composer --no-interaction install - composer code-analysis + - name: Coding standards + run: | + docker run --rm --volume $PWD:/md peterdavehello/markdownlint markdownlint --ignore vendor --ignore LICENSE.md '**/*.md' diff --git a/.gitignore b/.gitignore index ef96e04..3a9875b 100644 --- a/.gitignore +++ b/.gitignore @@ -1,4 +1,2 @@ /vendor/ composer.lock -/node_modules/ -yarn.lock diff --git a/.markdownlint.jsonc b/.markdownlint.jsonc new file mode 100644 index 0000000..a28c580 --- /dev/null +++ b/.markdownlint.jsonc @@ -0,0 +1,13 @@ +{ + "default": true, + // https://github.com/DavidAnson/markdownlint/blob/main/doc/md013.md + "line-length": { + "line_length": 120, + "code_blocks": false, + "tables": false + }, + // https://github.com/DavidAnson/markdownlint/blob/main/doc/md024.md + "no-duplicate-heading": { + "siblings_only": true + } +} diff --git a/.markdownlintrc b/.markdownlintrc deleted file mode 100644 index 9124ec0..0000000 --- a/.markdownlintrc +++ /dev/null @@ -1,9 +0,0 @@ -{ - "default": true, - "MD013": { - "code_blocks": false - }, - "no-duplicate-heading": { - "siblings_only": true - } -} diff --git a/CHANGELOG.md b/CHANGELOG.md index 9cceb59..69cfd6e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,17 +7,20 @@ about writing changes to this log. ## [Unreleased] +* [PR-7](https://github.com/itk-dev/os2forms_fasit/pull/7) + Added support for os2web_key + ## [1.0.2] 2024-04-22 -- Fixed `Content-Type` header. +* Fixed `Content-Type` header. ## [1.0.1] 2024-04-04 -- Fixed queue install configuration. +* Fixed queue install configuration. ## [1.0.0] -- Initial module base. +* Initial module base. [Unreleased]: https://github.com/itk-dev/os2forms_fasit/compare/1.0.1...HEAD [1.0.1]: https://github.com/itk-dev/os2forms_fasit/compare/1.0.0...1.0.1 diff --git a/README.md b/README.md index 5f0c10e..dd9b117 100644 --- a/README.md +++ b/README.md @@ -1,53 +1,48 @@ # OS2Forms Fasit -Adds [Fasit Schultz](https://schultz.dk/loesninger/schultz-fasit/) -handler for archiving purposes. +Adds [Fasit Schultz](https://schultz.dk/loesninger/schultz-fasit/) handler for archiving purposes. ## Installation -```sh +```shell composer require os2forms/os2forms_fasit vendor/bin/drush pm:enable os2forms_fasit ``` ## Settings -Configure Fasit API `base url` and a way of getting -certificate on `/admin/os2forms_fasit/settings`. +Go to `/admin/os2forms_fasit/settings` and configure the module. ### Certificate -The certificate must be in `pem` or `cer` format and -must be whitelisted by Fasit Schultz. -For this the certificate thumbprint, -in lowercase and without colons, is needed. +The certificate must be in `pem` or `cer` format and must be whitelisted by Fasit Schultz. For this the certificate +thumbprint, in lowercase and without colons, is needed. + To get the thumbprint in the correct format from the command line run -```sh +```shell openssl x509 -in SOME_CERTIFICATE.pem -noout -fingerprint | cut -d= -f2 | sed 's/://g' | tr '[:upper:]' '[:lower:]' ``` -Example output +Example output: -```sh +```shell 6acb261f393172d87fa3997cec86569759a8528a ``` ## Queue -Archiving is done via an -[Advanced Queue](https://www.drupal.org/project/advancedqueue) -called `fasit_queue`. +Archiving is done via an [Advanced Queue](https://www.drupal.org/project/advancedqueue) called `fasit_queue`. The queue should be processed with `drush`: -```sh +```shell drush advancedqueue:queue:process fasit_queue ``` List the queue (and all other queues) with -```sh +```shell drush advancedqueue:queue:list ``` @@ -64,21 +59,33 @@ Consider running the queue via a cronjob. ## Coding standards -Check coding standards: +Our coding are checked by GitHub Actions (cf. [.github/workflows/pr.yml](.github/workflows/pr.yml)). Use the commands +below to run the checks locally. -```sh -// PHP CS Fixer -docker run --rm --interactive --tty --volume ${PWD}:/app itkdev/php8.1-fpm:latest composer install -docker run --rm --interactive --tty --volume ${PWD}:/app itkdev/php8.1-fpm:latest composer coding-standards-check +### PHP -// Markdownlint -docker run --rm --interactive --tty --volume ${PWD}:/app node:20 yarn --cwd /app install -docker run --rm --interactive --tty --volume ${PWD}:/app node:20 yarn --cwd /app coding-standards-check +```shell +docker run --rm --volume ${PWD}:/app --workdir /app itkdev/php8.1-fpm composer install +# Fix (some) coding standards issues +docker run --rm --volume ${PWD}:/app --workdir /app itkdev/php8.1-fpm composer coding-standards-apply +# Check that code adheres to the coding standards +docker run --rm --volume ${PWD}:/app --workdir /app itkdev/php8.1-fpm composer coding-standards-check ``` -Apply coding standards: +### Markdown + +```shell +docker run --rm --volume $PWD:/md peterdavehello/markdownlint markdownlint --ignore vendor --ignore LICENSE.md '**/*.md' --fix +docker run --rm --volume $PWD:/md peterdavehello/markdownlint markdownlint --ignore vendor --ignore LICENSE.md '**/*.md' +``` + +## Code analysis + +We use [PHPStan](https://phpstan.org/) for static code analysis. + +Running statis code analysis on a standalone Drupal module is a bit tricky, so we use a helper script to run the +analysis: ```shell -docker run --rm --interactive --tty --volume ${PWD}:/app itkdev/php8.1-fpm:latest composer coding-standards-apply -docker run --rm --interactive --tty --volume ${PWD}:/app node:20 yarn --cwd /app coding-standards-apply +docker run --rm --volume ${PWD}:/app --workdir /app itkdev/php8.1-fpm ./scripts/code-analysis ``` diff --git a/composer.json b/composer.json index a6f4575..0eccd5b 100644 --- a/composer.json +++ b/composer.json @@ -1,61 +1,66 @@ { "name": "os2forms/os2forms_fasit", "description": "OS2Forms Fasit integration", - "type": "drupal-module", "license": "MIT", + "type": "drupal-module", "authors": [ { "name": "Jeppe Kuhlmann Andersen", "email": "jekua@aarhus.dk" } ], - "minimum-stability": "dev", - "prefer-stable": true, - "repositories": [ - { - "type": "composer", - "url": "https://packages.drupal.org/8" - } - ], "require": { "php": "^8.1", "ext-dom": "*", "drupal/advancedqueue": "^1.0", "drupal/webform": "^6.1", "os2forms/os2forms": "^3.13", + "os2web/os2web_key": "dev-os2web_key", "symfony/options-resolver": "^5.4 || ^6.0" }, "require-dev": { "dealerdirect/phpcodesniffer-composer-installer": "^1.0", "drupal/coder": "^8.3", - "mglaman/drupal-check": "^1.4" + "ergebnis/composer-normalize": "^2.42", + "mglaman/phpstan-drupal": "^1.2", + "phpstan/extension-installer": "^1.3", + "phpstan/phpstan-deprecation-rules": "^1.1" + }, + "repositories": [ + { + "type": "vcs", + "url": "https://github.com/itk-dev/os2web_key" + }, + { + "type": "composer", + "url": "https://packages.drupal.org/8" + } + ], + "minimum-stability": "dev", + "prefer-stable": true, + "config": { + "allow-plugins": { + "cweagans/composer-patches": true, + "dealerdirect/phpcodesniffer-composer-installer": true, + "ergebnis/composer-normalize": true, + "phpstan/extension-installer": true, + "simplesamlphp/composer-module-installer": true, + "zaporylie/composer-drupal-optimizations": true + }, + "sort-packages": true }, "scripts": { - "code-analysis/drupal-check": [ - "drupal-check --deprecations --analysis --exclude-dir='vendor' *.* src" - ], - "code-analysis": [ - "@code-analysis/drupal-check" + "coding-standards-apply": [ + "@coding-standards-apply/phpcs" ], - "coding-standards-check/phpcs": [ - "vendor/bin/phpcs --standard=phpcs.xml.dist" + "coding-standards-apply/phpcs": [ + "phpcbf --standard=phpcs.xml.dist" ], "coding-standards-check": [ "@coding-standards-check/phpcs" ], - "coding-standards-apply/phpcs": [ - "vendor/bin/phpcbf --standard=phpcs.xml.dist" - ], - "coding-standards-apply": [ - "@coding-standards-apply/phpcs" + "coding-standards-check/phpcs": [ + "phpcs --standard=phpcs.xml.dist" ] - }, - "config": { - "allow-plugins": { - "dealerdirect/phpcodesniffer-composer-installer": true, - "zaporylie/composer-drupal-optimizations": true, - "cweagans/composer-patches": true, - "simplesamlphp/composer-module-installer": true - } } } diff --git a/drush.services.yml b/drush.services.yml new file mode 100644 index 0000000..7b8cbec --- /dev/null +++ b/drush.services.yml @@ -0,0 +1,6 @@ +services: + Drupal\os2forms_fasit\Drush\Commands\FasitTestCommands: + arguments: + - '@Drupal\os2forms_fasit\Helper\FasitHelper' + tags: + - { name: drush.command } diff --git a/os2forms_fasit.info.yml b/os2forms_fasit.info.yml index 0142493..209f649 100644 --- a/os2forms_fasit.info.yml +++ b/os2forms_fasit.info.yml @@ -4,7 +4,9 @@ description: 'Fasit integration' package: OS2Forms core_version_requirement: ^9 || ^10 dependencies: - - drupal:webform - drupal:advancedqueue + - drupal:webform - os2forms:os2forms_attachment + - os2web_key:os2web_key + configure: os2forms_fasit.admin.settings diff --git a/os2forms_fasit.install b/os2forms_fasit.install new file mode 100644 index 0000000..b671b66 --- /dev/null +++ b/os2forms_fasit.install @@ -0,0 +1,15 @@ +install([ + 'key', + ], TRUE); +} diff --git a/os2forms_fasit.services.yml b/os2forms_fasit.services.yml index 23e41d4..87e84c4 100644 --- a/os2forms_fasit.services.yml +++ b/os2forms_fasit.services.yml @@ -1,15 +1,12 @@ services: Drupal\os2forms_fasit\Helper\Settings: arguments: - - "@keyvalue" - - Drupal\os2forms_fasit\Helper\CertificateLocatorHelper: - arguments: - - "@Drupal\\os2forms_fasit\\Helper\\Settings" + - "@config.factory" + - "@key.repository" Drupal\os2forms_fasit\Helper\FasitHelper: arguments: - '@http_client' - '@entity_type.manager' - "@Drupal\\os2forms_fasit\\Helper\\Settings" - - "@Drupal\\os2forms_fasit\\Helper\\CertificateLocatorHelper" + - "@file_system" diff --git a/package.json b/package.json deleted file mode 100644 index 229a645..0000000 --- a/package.json +++ /dev/null @@ -1,13 +0,0 @@ -{ - "devDependencies": { - "markdownlint-cli": "^0.33" - }, - "license": "MIT", - "private": true, - "scripts": { - "coding-standards-check/markdownlint": "yarn markdownlint --ignore LICENSE.md --ignore vendor --ignore node_modules **/*.md", - "coding-standards-check": "yarn coding-standards-check/markdownlint", - "coding-standards-apply/markdownlint": "yarn markdownlint --fix --ignore LICENSE.md --ignore vendor --ignore node_modules **/*.md", - "coding-standards-apply": "yarn coding-standards-apply/markdownlint" - } -} diff --git a/phpstan.neon b/phpstan.neon new file mode 100644 index 0000000..8b1d88e --- /dev/null +++ b/phpstan.neon @@ -0,0 +1,11 @@ +parameters: + level: 6 + paths: + - src + + ignoreErrors: + - '#Unsafe usage of new static\(\).#' + +# Local Variables: +# mode: yaml +# End: diff --git a/scripts/code-analysis b/scripts/code-analysis new file mode 100755 index 0000000..2ef69d0 --- /dev/null +++ b/scripts/code-analysis @@ -0,0 +1,48 @@ +#!/usr/bin/env bash +script_dir=$(pwd) +module_name=$(basename "$script_dir") +drupal_dir=vendor/drupal-module-code-analysis +# Relative to $drupal_dir +module_path=web/modules/contrib/$module_name + +cd "$script_dir" || exit + +drupal_composer() { + composer --working-dir="$drupal_dir" --no-interaction "$@" +} + +# Create new Drupal 9 project +if [ ! -f "$drupal_dir/composer.json" ]; then + composer --no-interaction create-project drupal/recommended-project:^9 "$drupal_dir" +fi +# Copy our code into the modules folder + +# Clean up +rm -fr "${drupal_dir:?}/$module_path" + +# https://stackoverflow.com/a/15373763 +# rsync --archive --compress . --filter=':- .gitignore' --exclude "$drupal_dir" --exclude .git "$drupal_dir/$module_path" + +# The rsync command in not available in itkdev/php8.1-fpm + +git config --global --add safe.directory /app +# Copy all module files not ignored by git into module path +# (cf. https://stackoverflow.com/a/77197460) +for f in $(git ls-files --cached --others --exclude-standard); do + mkdir -p "$drupal_dir/$module_path/$(dirname "$f")" + cp "$f" "$drupal_dir/$module_path/$f" +done + +drupal_composer config minimum-stability dev + +# Allow ALL plugins +# https://getcomposer.org/doc/06-config.md#allow-plugins +drupal_composer config --no-plugins allow-plugins true + +drupal_composer require wikimedia/composer-merge-plugin +drupal_composer config extra.merge-plugin.include "$module_path/composer.json" +# https://www.drupal.org/project/drupal/issues/3220043#comment-14845434 +drupal_composer require --dev symfony/phpunit-bridge + +# Run PHPStan +(cd "$drupal_dir/$module_path" && ../../../../vendor/bin/phpstan) diff --git a/src/Drush/Commands/FasitTestCommands.php b/src/Drush/Commands/FasitTestCommands.php new file mode 100644 index 0000000..4ac7153 --- /dev/null +++ b/src/Drush/Commands/FasitTestCommands.php @@ -0,0 +1,38 @@ +helper->pingApi(); + $this->io()->success('Successfully connected to Fasit API'); + } + catch (\Throwable $t) { + $this->io()->error($t->getMessage()); + } + + } + +} diff --git a/src/Form/SettingsForm.php b/src/Form/SettingsForm.php index d0fe119..32aaba2 100644 --- a/src/Form/SettingsForm.php +++ b/src/Form/SettingsForm.php @@ -2,41 +2,61 @@ namespace Drupal\os2forms_fasit\Form; -use Drupal\Core\Form\FormBase; +use Drupal\Core\Config\ConfigFactoryInterface; +use Drupal\Core\Form\ConfigFormBase; use Drupal\Core\Form\FormStateInterface; use Drupal\Core\StringTranslation\StringTranslationTrait; -use Drupal\os2forms_fasit\Helper\CertificateLocatorHelper; -use Drupal\os2forms_fasit\Helper\Settings; +use Drupal\os2forms_fasit\Helper\FasitHelper; use Symfony\Component\DependencyInjection\ContainerInterface; -use Symfony\Component\OptionsResolver\Exception\ExceptionInterface as OptionsResolverException; /** - * Organisation settings form. + * Fasit settings form. */ -final class SettingsForm extends FormBase { +final class SettingsForm extends ConfigFormBase { use StringTranslationTrait; + public const CONFIG_NAME = 'os2forms_fasit.settings'; + public const FASIT_API_BASE_URL = 'fasit_api_base_url'; public const FASIT_API_TENANT = 'fasit_api_tenant'; public const FASIT_API_VERSION = 'fasit_api_version'; - public const CERTIFICATE = 'certificate'; + public const KEY = 'key'; + + public const ACTION_PING_API = 'action_ping_api'; /** - * Constructor. + * {@inheritdoc} */ - public function __construct(private readonly Settings $settings, private readonly CertificateLocatorHelper $certificateLocatorHelper) { + public function __construct( + ConfigFactoryInterface $config_factory, + private readonly FasitHelper $helper, + ) { + parent::__construct($config_factory); } /** * {@inheritdoc} + * + * @phpstan-return self */ - public static function create(ContainerInterface $container): SettingsForm { + public static function create(ContainerInterface $container): self { return new static( - $container->get(Settings::class), - $container->get(CertificateLocatorHelper::class) + $container->get('config.factory'), + $container->get(FasitHelper::class) ); } + /** + * {@inheritdoc} + * + * @phpstan-return array + */ + protected function getEditableConfigNames() { + return [ + self::CONFIG_NAME, + ]; + } + /** * {@inheritdoc} */ @@ -51,116 +71,56 @@ public function getFormId() { * @phpstan-return array */ public function buildForm(array $form, FormStateInterface $form_state): array { + $form = parent::buildForm($form, $form_state); + $config = $this->config(self::CONFIG_NAME); - $fasitApiBaseUrl = $this->settings->getFasitApiBaseUrl(); $form[self::FASIT_API_BASE_URL] = [ '#type' => 'textfield', '#title' => $this->t('Fasit API base url'), '#required' => TRUE, - '#default_value' => !empty($fasitApiBaseUrl) ? $fasitApiBaseUrl : NULL, + '#default_value' => $config->get(self::FASIT_API_BASE_URL), '#description' => $this->t('Specifies which base url to use. This is disclosed by Schultz'), ]; - $fasitApiTenant = $this->settings->getFasitApiTenant(); $form[self::FASIT_API_TENANT] = [ '#type' => 'textfield', '#title' => $this->t('Fasit API tenant'), '#required' => TRUE, - '#default_value' => !empty($fasitApiTenant) ? $fasitApiTenant : NULL, + '#default_value' => $config->get(self::FASIT_API_TENANT), '#description' => $this->t('Specifies which tenant to use. This is disclosed by Schultz'), ]; - $fasitApiVersion = $this->settings->getFasitApiVersion(); $form[self::FASIT_API_VERSION] = [ '#type' => 'textfield', '#title' => $this->t('Fasit API version'), '#required' => TRUE, - '#default_value' => !empty($fasitApiVersion) ? $fasitApiVersion : NULL, + '#default_value' => $config->get(self::FASIT_API_VERSION), '#description' => $this->t('Specifies which api version to use. Should probably be v2'), ]; - $certificate = $this->settings->getCertificate(); - - $form[self::CERTIFICATE] = [ - '#type' => 'fieldset', - '#title' => $this->t('Certificate'), - '#tree' => TRUE, - - CertificateLocatorHelper::LOCATOR_TYPE => [ - '#type' => 'select', - '#title' => $this->t('Certificate locator type'), - '#options' => [ - CertificateLocatorHelper::LOCATOR_TYPE_AZURE_KEY_VAULT => $this->t('Azure key vault'), - CertificateLocatorHelper::LOCATOR_TYPE_FILE_SYSTEM => $this->t('File system'), - ], - '#default_value' => $certificate[CertificateLocatorHelper::LOCATOR_TYPE] ?? NULL, + $form[self::KEY] = [ + '#type' => 'key_select', + '#key_filters' => [ + 'type' => 'os2web_key_certificate', ], + '#title' => $this->t('Key'), + '#default_value' => $config->get(self::KEY), ]; - $form[self::CERTIFICATE][CertificateLocatorHelper::LOCATOR_TYPE_AZURE_KEY_VAULT] = [ - '#type' => 'fieldset', - '#title' => $this->t('Azure key vault'), - '#states' => [ - 'visible' => [':input[name="certificate[locator_type]"]' => ['value' => CertificateLocatorHelper::LOCATOR_TYPE_AZURE_KEY_VAULT]], - ], - ]; - - $settings = [ - 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[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, - '#states' => [ - 'required' => [':input[name="certificate[locator_type]"]' => ['value' => CertificateLocatorHelper::LOCATOR_TYPE_AZURE_KEY_VAULT]], - ], - ]; - } + $form['actions']['ping_api'] = [ + '#type' => 'container', - $form[self::CERTIFICATE][CertificateLocatorHelper::LOCATOR_TYPE_FILE_SYSTEM] = [ - '#type' => 'fieldset', - '#title' => $this->t('File system'), - '#states' => [ - 'visible' => [':input[name="certificate[locator_type]"]' => ['value' => CertificateLocatorHelper::LOCATOR_TYPE_FILE_SYSTEM]], + self::ACTION_PING_API => [ + '#type' => 'submit', + '#name' => self::ACTION_PING_API, + '#value' => $this->t('Ping API'), ], - 'path' => [ - '#type' => 'textfield', - '#title' => $this->t('Path'), - '#default_value' => $certificate[CertificateLocatorHelper::LOCATOR_TYPE_FILE_SYSTEM]['path'] ?? NULL, - '#states' => [ - 'required' => [':input[name="certificate[locator_type]"]' => ['value' => CertificateLocatorHelper::LOCATOR_TYPE_FILE_SYSTEM]], - ], + 'message' => [ + '#markup' => $this->t('Note: Pinging the API will used saved config.'), ], ]; - $form[self::CERTIFICATE][CertificateLocatorHelper::LOCATOR_PASSPHRASE] = [ - '#type' => 'textfield', - '#title' => $this->t('Passphrase'), - '#default_value' => $certificate[CertificateLocatorHelper::LOCATOR_PASSPHRASE] ?? NULL, - ]; - - $form['actions']['#type'] = 'actions'; - - $form['actions']['submit'] = [ - '#type' => 'submit', - '#value' => $this->t('Save settings'), - ]; - - $form['actions']['testCertificate'] = [ - '#type' => 'submit', - '#name' => 'testCertificate', - '#value' => $this->t('Test certificate'), - ]; - return $form; } @@ -169,20 +129,12 @@ public function buildForm(array $form, FormStateInterface $form_state): array { * * @phpstan-param array $form */ - public function validateForm(array &$form, FormStateInterface $formState): void { - $triggeringElement = $formState->getTriggeringElement(); - if ('testCertificate' === ($triggeringElement['#name'] ?? NULL)) { + public function validateForm(array &$form, FormStateInterface $form_state): void { + if (self::ACTION_PING_API === ($form_state->getTriggeringElement()['#name'] ?? NULL)) { return; } - $values = $formState->getValues(); - - 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])); - } - } + parent::validateForm($form, $form_state); } /** @@ -190,44 +142,30 @@ public function validateForm(array &$form, FormStateInterface $formState): void * * @phpstan-param array $form */ - public function submitForm(array &$form, FormStateInterface $formState): void { - $triggeringElement = $formState->getTriggeringElement(); - if ('testCertificate' === ($triggeringElement['#name'] ?? NULL)) { - $this->testCertificate(); + public function submitForm(array &$form, FormStateInterface $form_state): void { + if (self::ACTION_PING_API === ($form_state->getTriggeringElement()['#name'] ?? NULL)) { + try { + $this->helper->pingApi(); + $this->messenger()->addStatus($this->t('Pinged API successfully.')); + } + catch (\Throwable $t) { + $this->messenger()->addError($this->t('Pinging API failed: @message', ['@message' => $t->getMessage()])); + } return; } - try { - $settings[self::CERTIFICATE] = $formState->getValue(self::CERTIFICATE); - $settings[self::FASIT_API_BASE_URL] = $formState->getValue(self::FASIT_API_BASE_URL); - $settings[self::FASIT_API_TENANT] = $formState->getValue(self::FASIT_API_TENANT); - $settings[self::FASIT_API_VERSION] = $formState->getValue(self::FASIT_API_VERSION); - - $this->settings->setSettings($settings); - $this->messenger()->addStatus($this->t('Settings saved')); + $config = $this->config(self::CONFIG_NAME); + foreach ([ + self::FASIT_API_BASE_URL, + self::FASIT_API_TENANT, + self::FASIT_API_VERSION, + self::KEY, + ] as $key) { + $config->set($key, $form_state->getValue($key)); } - catch (OptionsResolverException $exception) { - $this->messenger()->addError($this->t('Settings not saved (@message)', ['@message' => $exception->getMessage()])); - - return; - } - - $this->messenger()->addStatus($this->t('Settings saved')); - } + $config->save(); - /** - * Test certificate. - */ - private function testCertificate(): void { - try { - $certificateLocator = $this->certificateLocatorHelper->getCertificateLocator(); - $certificateLocator->getCertificates(); - $this->messenger()->addStatus($this->t('Certificate successfully tested')); - } - catch (\Throwable $throwable) { - $message = $this->t('Error testing certificate: %message', ['%message' => $throwable->getMessage()]); - $this->messenger()->addError($message); - } + parent::submitForm($form, $form_state); } } diff --git a/src/Helper/CertificateLocatorHelper.php b/src/Helper/CertificateLocatorHelper.php deleted file mode 100644 index 1c39a75..0000000 --- a/src/Helper/CertificateLocatorHelper.php +++ /dev/null @@ -1,86 +0,0 @@ -settings->getCertificate(); - - $locatorType = $certificateSettings[self::LOCATOR_TYPE]; - $options = $certificateSettings[$locatorType]; - $options += [ - self::LOCATOR_PASSPHRASE => $certificateSettings[self::LOCATOR_PASSPHRASE] ?: '', - ]; - - if (self::LOCATOR_TYPE_AZURE_KEY_VAULT === $locatorType) { - $httpClient = new GuzzleAdapter(new Client()); - $requestFactory = new RequestFactory(); - - $vaultToken = new VaultToken($httpClient, $requestFactory); - - $token = $vaultToken->getToken( - $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[self::LOCATOR_AZURE_KEY_VAULT_NAME], - $token->getAccessToken() - ); - - return new AzureKeyVaultCertificateLocator( - $vault, - $options[self::LOCATOR_AZURE_KEY_VAULT_SECRET], - $options[self::LOCATOR_AZURE_KEY_VAULT_VERSION], - $options[self::LOCATOR_PASSPHRASE], - ); - } - elseif (self::LOCATOR_TYPE_FILE_SYSTEM === $locatorType) { - $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[self::LOCATOR_PASSPHRASE]); - } - - throw new CertificateLocatorException(sprintf('Invalid certificate locator type: %s', $locatorType)); - } - -} diff --git a/src/Helper/FasitHelper.php b/src/Helper/FasitHelper.php index 2003e45..a969b22 100644 --- a/src/Helper/FasitHelper.php +++ b/src/Helper/FasitHelper.php @@ -4,6 +4,7 @@ use Drupal\Core\Entity\EntityInterface; use Drupal\Core\Entity\EntityTypeManagerInterface; +use Drupal\Core\File\FileSystemInterface; use Drupal\os2forms_attachment\Element\AttachmentElement; use Drupal\os2forms_fasit\Exception\FasitResponseException; use Drupal\os2forms_fasit\Exception\FasitXMLGenerationException; @@ -14,6 +15,7 @@ use GuzzleHttp\ClientInterface; use GuzzleHttp\Exception\GuzzleException; use GuzzleHttp\Psr7\Utils; +use Psr\Http\Message\ResponseInterface; use Symfony\Component\HttpFoundation\Response; /** @@ -32,7 +34,15 @@ class FasitHelper { 'managed_file', ]; - public function __construct(private readonly ClientInterface $client, private readonly EntityTypeManagerInterface $entityTypeManager, private readonly Settings $settings, private readonly CertificateLocatorHelper $certificateLocator) { + /** + * Constructor. + */ + public function __construct( + private readonly ClientInterface $client, + private readonly EntityTypeManagerInterface $entityTypeManager, + private readonly Settings $settings, + private readonly FileSystemInterface $fileSystem, + ) { } /** @@ -183,27 +193,19 @@ private function uploadDocument(array $uploads, string $submissionId, array $han } } - [$certificateOptions, $tempCertFilename] = $this->getCertificateOptionsAndTempCertFilename(); - $options = [ 'headers' => [ 'Content-Type' => 'application/xml', ], 'body' => $doc->saveXML(), - 'cert' => $certificateOptions, ]; // Attempt upload. try { - $response = $this->client->request('POST', $endpoint, $options); + $response = $this->post($endpoint, $options); } catch (GuzzleException $e) { throw new FasitResponseException($e->getMessage(), $e->getCode()); - } finally { - // Remove the certificate from disk. - if (file_exists($tempCertFilename)) { - unlink($tempCertFilename); - } } if (Response::HTTP_OK !== $response->getStatusCode()) { @@ -230,26 +232,6 @@ private function checkHandlerConfiguration(array $handlerConfiguration, string $ } } - /** - * Gets certificate options and temp certificate filename. - * - * @throws \Drupal\os2forms_fasit\Exception\CertificateLocatorException - * Certificate locator exception. - * - * @phpstan-return array - */ - private function getCertificateOptionsAndTempCertFilename(): array { - $certificateLocator = $this->certificateLocator->getCertificateLocator(); - $localCertFilename = tempnam(sys_get_temp_dir(), 'cert'); - file_put_contents($localCertFilename, $certificateLocator->getCertificate()); - $certificateOptions = - $certificateLocator->hasPassphrase() ? - [$localCertFilename, $certificateLocator->getPassphrase()] - : $localCertFilename; - - return [$certificateOptions, $localCertFilename]; - } - /** * Uploads attachment to Fasit. * @@ -311,8 +293,6 @@ private function uploadFile(string $originalFilename, string $tempFilename): arr self::FASIT_API_METHOD_UPLOAD ); - [$certificateOptions, $tempCertFilename] = $this->getCertificateOptionsAndTempCertFilename(); - // Attempt upload. try { $options = [ @@ -322,18 +302,13 @@ private function uploadFile(string $originalFilename, string $tempFilename): arr 'X-Title' => pathinfo($originalFilename, PATHINFO_FILENAME), ], 'body' => Utils::tryFopen($tempFilename, 'r'), - 'cert' => $certificateOptions, ]; - $response = $this->client->request('POST', $endpoint, $options); + $response = $this->post($endpoint, $options); } catch (GuzzleException $e) { throw new FasitResponseException($e->getMessage(), $e->getCode()); } finally { - // Remove the certificate from disk. - if (file_exists($tempCertFilename)) { - unlink($tempCertFilename); - } // Remove the attachment from disk. if (file_exists($tempFilename)) { unlink($tempFilename); @@ -469,4 +444,60 @@ private function getSubmission(string $submissionId): EntityInterface { return $storage->load($submissionId); } + /** + * Send POST request to Fasit API. + * + * @param string $endpoint + * The API endpoint. + * @param array $options + * The request options. + * + * @return \Psr\Http\Message\ResponseInterface + * The response. + * + * @throws \GuzzleHttp\Exception\GuzzleException + * A Guzzle exception. + */ + private function post(string $endpoint, array $options): ResponseInterface { + try { + $certificate = $this->settings->getCertificate(); + $certPath = $this->fileSystem->tempnam($this->fileSystem->getTempDirectory(), 'os2forms_fasit_cert'); + // `tempnam` has created a file, so we must replace when saving. + $this->fileSystem->saveData($certificate, $certPath, FileSystemInterface::EXISTS_REPLACE); + $options['cert'] = $certPath; + + return $this->client->request('POST', $endpoint, $options); + } finally { + // Remove the certificate from disk. + if (isset($certPath) && file_exists($certPath)) { + unlink($certPath); + } + } + } + + /** + * Ping the Fasit API and expect a 400 Bad Request response. + * + * @throws \Throwable + */ + public function pingApi(): void { + $endpoint = sprintf('%s/%s/%s/documents/%s', + $this->settings->getFasitApiBaseUrl(), + $this->settings->getFasitApiTenant(), + $this->settings->getFasitApiVersion(), + self::FASIT_API_METHOD_UPLOAD + ); + + try { + $this->post($endpoint, []); + } + catch (\Throwable $t) { + // Throw if it's not a 400 Bad Request exception. + if (!($t instanceof GuzzleException) + || Response::HTTP_BAD_REQUEST !== $t->getCode()) { + throw $t; + } + } + } + } diff --git a/src/Helper/Settings.php b/src/Helper/Settings.php index de065fc..e18feb9 100644 --- a/src/Helper/Settings.php +++ b/src/Helper/Settings.php @@ -2,66 +2,69 @@ namespace Drupal\os2forms_fasit\Helper; -use Drupal\Core\KeyValueStore\KeyValueFactoryInterface; -use Drupal\Core\KeyValueStore\KeyValueStoreInterface; -use Drupal\os2forms_fasit\Exception\InvalidSettingException; +use Drupal\Core\Config\ConfigFactoryInterface; +use Drupal\Core\Config\ImmutableConfig; +use Drupal\key\KeyRepositoryInterface; use Drupal\os2forms_fasit\Form\SettingsForm; -use Symfony\Component\OptionsResolver\OptionsResolver; /** * General settings for os2forms_fasit. */ final class Settings { /** - * The store. + * The config. * - * @var \Drupal\Core\KeyValueStore\KeyValueStoreInterface + * @var \Drupal\Core\Config\ImmutableConfig */ - private KeyValueStoreInterface $store; + private ImmutableConfig $config; /** - * The key value collection name. - * - * @var string + * The constructor. */ - private $collection = 'os2forms_fasit'; + public function __construct( + ConfigFactoryInterface $configFactory, + private readonly KeyRepositoryInterface $keyRepository, + ) { + $this->config = $configFactory->get(SettingsForm::CONFIG_NAME); + } /** - * The constructor. + * Get fasit api base url. */ - public function __construct(KeyValueFactoryInterface $keyValueFactory) { - $this->store = $keyValueFactory->get($this->collection); + public function getFasitApiBaseUrl(): ?string { + return $this->get(SettingsForm::FASIT_API_BASE_URL); } /** * Get fasit api base url. */ - public function getFasitApiBaseUrl(): string { - return $this->get(SettingsForm::FASIT_API_BASE_URL, ''); + public function getFasitApiTenant(): ?string { + return $this->get(SettingsForm::FASIT_API_TENANT); } /** * Get fasit api base url. */ - public function getFasitApiTenant(): string { - return $this->get(SettingsForm::FASIT_API_TENANT, ''); + public function getFasitApiVersion(): ?string { + return $this->get(SettingsForm::FASIT_API_VERSION); } /** - * Get fasit api base url. + * Get key. */ - public function getFasitApiVersion(): string { - return $this->get(SettingsForm::FASIT_API_VERSION, ''); + public function getKey(): ?string { + return $this->get(SettingsForm::KEY); } /** * Get certificate. - * - * @phpstan-return array */ - public function getCertificate(): array { - $value = $this->get(SettingsForm::CERTIFICATE); - return is_array($value) ? $value : []; + public function getCertificate(): ?string { + $key = $this->keyRepository->getKey( + $this->getKey(), + ); + + return $key?->getKeyValue(); } /** @@ -75,42 +78,8 @@ public function getCertificate(): array { * @return mixed * The setting value. */ - private function get(string $key, $default = NULL) { - $resolver = $this->getSettingsResolver(); - if (!$resolver->isDefined($key)) { - throw new InvalidSettingException(sprintf('Setting %s is not defined', $key)); - } - - return $this->store->get($key, $default); - } - - /** - * Set settings. - * - * @throws \Symfony\Component\OptionsResolver\Exception\ExceptionInterface - * - * @phpstan-param array $settings - */ - public function setSettings(array $settings): self { - $settings = $this->getSettingsResolver()->resolve($settings); - foreach ($settings as $key => $value) { - $this->store->set($key, $value); - } - - return $this; - } - - /** - * Get settings resolver. - */ - private function getSettingsResolver(): OptionsResolver { - return (new OptionsResolver()) - ->setDefaults([ - SettingsForm::FASIT_API_BASE_URL => '', - SettingsForm::FASIT_API_TENANT => '', - SettingsForm::FASIT_API_VERSION => '', - SettingsForm::CERTIFICATE => [], - ]); + private function get(string $key, $default = NULL): mixed { + return $this->config->get($key) ?? $default; } } From bcc0482e539ef51d3496aacf569a8eaa5def8228 Mon Sep 17 00:00:00 2001 From: Mikkel Ricky Date: Mon, 13 May 2024 09:15:06 +0200 Subject: [PATCH 2/6] Required os2web_key 1.0 --- composer.json | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/composer.json b/composer.json index 0eccd5b..acd2aee 100644 --- a/composer.json +++ b/composer.json @@ -15,7 +15,7 @@ "drupal/advancedqueue": "^1.0", "drupal/webform": "^6.1", "os2forms/os2forms": "^3.13", - "os2web/os2web_key": "dev-os2web_key", + "os2web/os2web_key": "^1.0", "symfony/options-resolver": "^5.4 || ^6.0" }, "require-dev": { @@ -27,10 +27,6 @@ "phpstan/phpstan-deprecation-rules": "^1.1" }, "repositories": [ - { - "type": "vcs", - "url": "https://github.com/itk-dev/os2web_key" - }, { "type": "composer", "url": "https://packages.drupal.org/8" From c33ffcefd21bd4523f1440df5e084a083a192f30 Mon Sep 17 00:00:00 2001 From: Mikkel Ricky Date: Thu, 16 May 2024 13:58:54 +0200 Subject: [PATCH 3/6] Made key required --- src/Form/SettingsForm.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Form/SettingsForm.php b/src/Form/SettingsForm.php index 32aaba2..ad21f59 100644 --- a/src/Form/SettingsForm.php +++ b/src/Form/SettingsForm.php @@ -104,6 +104,7 @@ public function buildForm(array $form, FormStateInterface $form_state): array { 'type' => 'os2web_key_certificate', ], '#title' => $this->t('Key'), + '#required' => TRUE, '#default_value' => $config->get(self::KEY), ]; From e57a82738c34eaf7b49b66943b25007e441b2ea3 Mon Sep 17 00:00:00 2001 From: Mikkel Ricky Date: Fri, 17 May 2024 11:07:00 +0200 Subject: [PATCH 4/6] Fixed typo --- src/Form/SettingsForm.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Form/SettingsForm.php b/src/Form/SettingsForm.php index ad21f59..d9ebecb 100644 --- a/src/Form/SettingsForm.php +++ b/src/Form/SettingsForm.php @@ -118,7 +118,7 @@ public function buildForm(array $form, FormStateInterface $form_state): array { ], 'message' => [ - '#markup' => $this->t('Note: Pinging the API will used saved config.'), + '#markup' => $this->t('Note: Pinging the API will use saved config.'), ], ]; From 265b93819a2e0cae9a754888ae9e0463543ecdd3 Mon Sep 17 00:00:00 2001 From: jekuaitk Date: Fri, 25 Oct 2024 10:45:19 +0200 Subject: [PATCH 5/6] Cleanups for D10 --- composer.json | 3 ++- scripts/code-analysis | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/composer.json b/composer.json index a9ff645..6f37115 100644 --- a/composer.json +++ b/composer.json @@ -41,7 +41,8 @@ "ergebnis/composer-normalize": true, "phpstan/extension-installer": true, "simplesamlphp/composer-module-installer": true, - "zaporylie/composer-drupal-optimizations": true + "zaporylie/composer-drupal-optimizations": true, + "mglaman/composer-drupal-lenient": true }, "sort-packages": true }, diff --git a/scripts/code-analysis b/scripts/code-analysis index 2ef69d0..85d77f2 100755 --- a/scripts/code-analysis +++ b/scripts/code-analysis @@ -13,7 +13,7 @@ drupal_composer() { # Create new Drupal 9 project if [ ! -f "$drupal_dir/composer.json" ]; then - composer --no-interaction create-project drupal/recommended-project:^9 "$drupal_dir" + composer --no-interaction create-project drupal/recommended-project:^10 "$drupal_dir" fi # Copy our code into the modules folder From 6244833b56cb70474510dd4d411e17833af7bdc3 Mon Sep 17 00:00:00 2001 From: jekuaitk Date: Fri, 25 Oct 2024 10:50:53 +0200 Subject: [PATCH 6/6] Normalize composer.json --- composer.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/composer.json b/composer.json index 6f37115..930e751 100644 --- a/composer.json +++ b/composer.json @@ -39,10 +39,10 @@ "cweagans/composer-patches": true, "dealerdirect/phpcodesniffer-composer-installer": true, "ergebnis/composer-normalize": true, + "mglaman/composer-drupal-lenient": true, "phpstan/extension-installer": true, "simplesamlphp/composer-module-installer": true, - "zaporylie/composer-drupal-optimizations": true, - "mglaman/composer-drupal-lenient": true + "zaporylie/composer-drupal-optimizations": true }, "sort-packages": true },