From b8f95f641898e22be8bcd25b0ad4f9b9fdeb02ee Mon Sep 17 00:00:00 2001 From: "t.munk (workhorse)" Date: Mon, 5 Oct 2026 11:44:30 +0000 Subject: [PATCH 1/7] feat(security)!: encrypt, mask and admin-restrict configJson secrets [*] Closes #8. - All operations of ApiConfiguration, reads included, require ROLE_ADMIN. - Type schemas mark secret keys with "writeOnly": true. - Marked keys are encrypted at rest (CredentialEncryption, onFlush listener), masked as "********" in responses and validator logs, and decrypted only in ApiClientFactory. - On PUT/PATCH an omitted key or the mask keeps the stored secret, null removes it. - New command app:api-configuration:encrypt-secrets (idempotent) for existing rows. - Fix app:api:test-connection calling non-existent getCredentials(). BREAKING CHANGE: reads require ROLE_ADMIN; secrets are masked in API responses; getConfigJson() returns enc:v1: ciphertext for marked keys; a usable CREDENTIALS_ENCRYPTION_KEY is required once a secret is stored; ApiClientFactory, ApiConfigurationValidator and CreateApiConfigurationCommand take an additional ConfigSecrets argument. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 31 +++ README.md | 64 ++++- composer.json | 2 +- src/ApiClient/ApiClientFactory.php | 9 +- src/Command/CreateApiConfigurationCommand.php | 6 +- .../EncryptApiConfigurationSecretsCommand.php | 79 +++++++ src/Command/TestApiConnectionCommand.php | 9 +- src/Doctrine/EncryptConfigSecretsListener.php | 59 +++++ src/Entity/ApiConfiguration.php | 6 +- src/Security/ConfigSecrets.php | 219 ++++++++++++++++++ src/Security/SecretEncryptionException.php | 14 ++ src/Security/SecretSchemaResolver.php | 212 +++++++++++++++++ .../ApiConfigurationSecretsNormalizer.php | 90 +++++++ src/Validator/ApiConfigurationValidator.php | 14 +- tests/Doctrine/EncryptConfigSecretsTest.php | 123 ++++++++++ tests/Entity/ApiConfigurationSecurityTest.php | 38 +++ tests/Fixtures/SecretsFixture.php | 66 ++++++ tests/Fixtures/demo.schema.json | 34 +++ tests/Security/ConfigSecretsTest.php | 150 ++++++++++++ tests/Security/SecretSchemaResolverTest.php | 80 +++++++ .../ApiConfigurationSecretsNormalizerTest.php | 125 ++++++++++ 21 files changed, 1410 insertions(+), 20 deletions(-) create mode 100644 CHANGELOG.md create mode 100644 src/Command/EncryptApiConfigurationSecretsCommand.php create mode 100644 src/Doctrine/EncryptConfigSecretsListener.php create mode 100644 src/Security/ConfigSecrets.php create mode 100644 src/Security/SecretEncryptionException.php create mode 100644 src/Security/SecretSchemaResolver.php create mode 100644 src/Serializer/ApiConfigurationSecretsNormalizer.php create mode 100644 tests/Doctrine/EncryptConfigSecretsTest.php create mode 100644 tests/Entity/ApiConfigurationSecurityTest.php create mode 100644 tests/Fixtures/SecretsFixture.php create mode 100644 tests/Fixtures/demo.schema.json create mode 100644 tests/Security/ConfigSecretsTest.php create mode 100644 tests/Security/SecretSchemaResolverTest.php create mode 100644 tests/Serializer/ApiConfigurationSecretsNormalizerTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 0000000..9e38ed1 --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,31 @@ + + +# Changelog + +## Unreleased + +Secure storage and transport of the credentials in `configJson` ([Issue #8](https://github.com/dmstr/api-configuration-bundle/issues/8)). Release as a new minor version: the changes below break consumers in the ways listed under "Breaking changes". + +### Breaking changes + +- **Reads require `ROLE_ADMIN`.** `GET /api/admin/api_configurations`, `GET /api/admin/api_configurations/{id}` and `GET /api/admin/api_configurations/{id}/health` answer `403` for non-admin users (before: `ROLE_USER`). There is no option to restore the old behaviour. +- **Secrets are masked in responses.** Keys marked `writeOnly` in the type schema come back as `********`, also for administrators. Clients that read credentials over the API no longer get them. +- **Secrets are encrypted at rest.** `ApiConfiguration::getConfigJson()` returns `enc:v1:…` for marked keys after a flush. Code that reads secrets from the entity directly (instead of through `ApiClientFactory`) has to decrypt with `ConfigSecrets::decrypt()`, for example an `AuthorizableExtensionInterface` that reads a client secret in `buildAuthorizeUrl()`. +- **A usable encryption key is required** as soon as a configuration holds a marked secret: `dmstr_api_platform_utils.credential_encryption.key: '%env(base64:CREDENTIALS_ENCRYPTION_KEY)%'`. Without it, persisting or using the secret throws `SecretEncryptionException`. +- **Constructor signatures**: `ApiClientFactory`, `ApiConfigurationValidator` and `CreateApiConfigurationCommand` take an additional `ConfigSecrets` argument (only relevant when they are instantiated manually instead of autowired). + +### Added + +- Secret keys are declared in the type schema with `"writeOnly": true`; the existing type schemas in the consuming bundles and applications need the marker on their `password`, `token`, `client_secret` and equivalent keys. +- Encryption at rest of the marked keys with `CredentialEncryption` (Doctrine `onFlush` listener). +- Masking of the marked keys in API responses and in the validator's logs. +- Update rule: an omitted secret or the mask keeps the stored value, `null` removes it. +- Console command `app:api-configuration:encrypt-secrets` (`--dry-run`) to encrypt the secrets of existing rows, idempotent. + +### Fixed + +- `app:api:test-connection` called the non-existent `ApiConfiguration::getCredentials()` and failed with a PHP error; it now builds the client from the entity. + +## 0.4.0 and earlier + +See git history. diff --git a/README.md b/README.md index 6504208..6b1dff2 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@ Manage external API connections as Doctrine entities. ## Features (planned) -- `ApiConfiguration` entity — type, credentials (encrypted), endpoint config +- `ApiConfiguration` entity — type, credentials (encrypted at rest, masked on read), endpoint config - Custom operations: `health`, `authorize`, `test-connection` - `ApiExtensionRegistry` — discoverable adapter pattern via tag - `ApiExtensionInterface` / `AuthorizableExtensionInterface` — adapters @@ -14,6 +14,68 @@ Manage external API connections as Doctrine entities. - OAuth callback controller for `authorize` flow - CLI mirrors: `api-configuration:create`, `:health`, `:test-connection` +## Security + +### Access + +Every operation of the `ApiConfiguration` resource under `/api/admin/api_configurations` — reads and the `health` and `authorize` sub-resources included — requires `ROLE_ADMIN`, because `configJson` holds the credentials for the upstream systems. + +### Secrets in `configJson` + +A type marks its secret keys in its own `schema.json` with the standard JSON Schema annotation `writeOnly`: + +```json +{ + "properties": { + "username": { "type": "string" }, + "password": { "type": "string", "writeOnly": true }, + "oauth": { + "type": "object", + "properties": { + "client_secret": { "type": "string", "writeOnly": true } + } + } + } +} +``` + +Marked keys are found in nested objects, local `$ref`s, `allOf`/`anyOf`/`oneOf`, `if`/`then`/`else`, array `items` and `additionalProperties`. The schema of a type is looked up by schema provider name, then by extension name, then by the `type` const of the registered schemas; for an unknown type the marked keys of all schemas apply, so a missing mapping over-masks instead of leaking. + +For marked keys the bundle guarantees: + +- **Encrypted at rest.** Values are stored as `enc:v1:` (libsodium secretbox via `Dmstr\ApiPlatformUtils\Service\CredentialEncryption`), whoever writes the entity — API, console or application code (Doctrine `onFlush` listener). +- **Masked on read.** API responses contain `********` instead of the value, also for administrators. Logs of the configuration validator are masked the same way. +- **Kept on update.** In `PUT` and `PATCH`, an omitted key or the mask `********` keeps the stored value, `null` removes the key, any other value replaces it. Nothing is carried over when `type` changes. +- **Decrypted only for the client.** `ApiClientFactory::create()` and `createFromEntity()` decrypt before calling `ApiExtensionInterface::createClient()`. Code that reads secrets from `ApiConfiguration::getConfigJson()` itself (for example an `AuthorizableExtensionInterface` reading a client secret) must decrypt with `Dmstr\ApiConfiguration\Security\ConfigSecrets::decrypt()`. + +### Encryption key + +The key comes from `dmstr/api-platform-utils-bundle`. `CredentialEncryption` expects the 32 raw key bytes, the generated key is base64, so decode it in the configuration: + +```yaml +# config/packages/dmstr_api_platform_utils.yaml +dmstr_api_platform_utils: + credential_encryption: + key: '%env(base64:CREDENTIALS_ENCRYPTION_KEY)%' +``` + +```bash +bin/console dmstr:generate-encryption-key # prints CREDENTIALS_ENCRYPTION_KEY=... +``` + +Without a usable key (missing, wrong length, not base64), storing or using a secret fails with `Dmstr\ApiConfiguration\Security\SecretEncryptionException`; nothing is stored in clear. Configurations without secrets keep working. Changing the key makes existing secrets unreadable. + +### Existing rows + +Rows stored before encryption at rest hold their secrets in clear. Encrypt them once after the upgrade; the command is idempotent and leaves encrypted values alone, so it can also run on every deployment: + +```bash +bin/console app:api-configuration:encrypt-secrets --dry-run +bin/console app:api-configuration:encrypt-secrets +``` + +Rows that are updated through Doctrine are encrypted on the next flush anyway. + ## License MIT © diemeisterei GmbH diff --git a/composer.json b/composer.json index 324371c..457d7f4 100644 --- a/composer.json +++ b/composer.json @@ -14,7 +14,7 @@ "symfony/framework-bundle": "^7.0", "api-platform/symfony": "^4.0", "doctrine/orm": "^3.0", - "doctrine/doctrine-bundle": "^2.0", + "doctrine/doctrine-bundle": "^2.8", "dmstr/api-platform-utils-bundle": "*", "dmstr/openapi-json-schema-bundle": "*", "opis/json-schema": "^2.3" diff --git a/src/ApiClient/ApiClientFactory.php b/src/ApiClient/ApiClientFactory.php index 39b9e2b..f5c907f 100644 --- a/src/ApiClient/ApiClientFactory.php +++ b/src/ApiClient/ApiClientFactory.php @@ -5,6 +5,7 @@ namespace Dmstr\ApiConfiguration\ApiClient; +use Dmstr\ApiConfiguration\Security\ConfigSecrets; use Dmstr\ApiConfiguration\Service\ApiExtensionRegistry; /** @@ -13,7 +14,8 @@ class ApiClientFactory { public function __construct( - private readonly ApiExtensionRegistry $registry + private readonly ApiExtensionRegistry $registry, + private readonly ConfigSecrets $secrets, ) { } @@ -21,9 +23,10 @@ public function __construct( * Create an API client based on configuration * * @param string $apiName API name (basecamp2, github, gitlab, jira) - * @param array $config Configuration array + * @param array $config Configuration array; encrypted secrets are decrypted here * @return ApiClientInterface * @throws \InvalidArgumentException If API name is not supported or config is invalid + * @throws \Dmstr\ApiConfiguration\Security\SecretEncryptionException If a secret cannot be decrypted */ public function create(string $apiName, array $config): ApiClientInterface { @@ -39,7 +42,7 @@ public function create(string $apiName, array $config): ApiClientInterface ); } - return $extension->createClient($config); + return $extension->createClient($this->secrets->decrypt($config)); } /** diff --git a/src/Command/CreateApiConfigurationCommand.php b/src/Command/CreateApiConfigurationCommand.php index c100dbe..2528a5a 100644 --- a/src/Command/CreateApiConfigurationCommand.php +++ b/src/Command/CreateApiConfigurationCommand.php @@ -6,6 +6,7 @@ namespace Dmstr\ApiConfiguration\Command; use Dmstr\ApiConfiguration\Entity\ApiConfiguration; +use Dmstr\ApiConfiguration\Security\ConfigSecrets; use Doctrine\ORM\EntityManagerInterface; use Symfony\Component\Console\Attribute\AsCommand; use Symfony\Component\Console\Command\Command; @@ -24,7 +25,8 @@ class CreateApiConfigurationCommand extends Command { public function __construct( private readonly EntityManagerInterface $entityManager, - private readonly ValidatorInterface $validator + private readonly ValidatorInterface $validator, + private readonly ConfigSecrets $secrets, ) { parent::__construct(); } @@ -103,7 +105,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int if ($output->isVerbose()) { $io->section('Config JSON'); - $io->writeln(json_encode($config->getConfigJson(), JSON_PRETTY_PRINT)); + $io->writeln(json_encode($this->secrets->mask($config->getConfigJson()), JSON_PRETTY_PRINT)); } return Command::SUCCESS; diff --git a/src/Command/EncryptApiConfigurationSecretsCommand.php b/src/Command/EncryptApiConfigurationSecretsCommand.php new file mode 100644 index 0000000..d9c4c91 --- /dev/null +++ b/src/Command/EncryptApiConfigurationSecretsCommand.php @@ -0,0 +1,79 @@ +addOption('dry-run', null, InputOption::VALUE_NONE, 'Only list the configurations that would be encrypted'); + } + + protected function execute(InputInterface $input, OutputInterface $output): int + { + $io = new SymfonyStyle($input, $output); + $dryRun = (bool) $input->getOption('dry-run'); + + $rows = []; + foreach ($this->entityManager->getRepository(ApiConfiguration::class)->findAll() as $config) { + $stored = $config->getConfigJson(); + $encrypted = $this->secrets->encrypt($stored); + if ($encrypted === $stored) { + continue; + } + + $rows[] = [$config->getId()->toRfc4122(), $config->getName(), $config->getType()]; + if (!$dryRun) { + $config->setConfigJson($encrypted); + } + } + + if ($rows === []) { + $io->success('All secrets are encrypted, nothing to do.'); + + return Command::SUCCESS; + } + + $io->table(['ID', 'Name', 'Type'], $rows); + + if ($dryRun) { + $io->note(sprintf('Dry run: %d configuration(s) would be encrypted.', count($rows))); + + return Command::SUCCESS; + } + + $this->entityManager->flush(); + $io->success(sprintf('Encrypted the secrets of %d configuration(s).', count($rows))); + + return Command::SUCCESS; + } +} diff --git a/src/Command/TestApiConnectionCommand.php b/src/Command/TestApiConnectionCommand.php index 803e275..6ed046f 100644 --- a/src/Command/TestApiConnectionCommand.php +++ b/src/Command/TestApiConnectionCommand.php @@ -1,4 +1,5 @@ warning('This configuration is disabled.'); } - $credentials = $config->getCredentials(); - if ($credentials === null || $credentials === []) { - $io->error('No credentials configured for this API.'); - return Command::FAILURE; - } - $io->section('Authenticating...'); try { - $client = $this->clientFactory->create($config->getType(), $credentials); + $client = $this->clientFactory->createFromEntity($config); if ($client->authenticate()) { $io->success('Authentication successful!'); diff --git a/src/Doctrine/EncryptConfigSecretsListener.php b/src/Doctrine/EncryptConfigSecretsListener.php new file mode 100644 index 0000000..7f10892 --- /dev/null +++ b/src/Doctrine/EncryptConfigSecretsListener.php @@ -0,0 +1,59 @@ +getObjectManager(); + $unitOfWork = $entityManager->getUnitOfWork(); + + $entities = [ + ...$unitOfWork->getScheduledEntityInsertions(), + ...$unitOfWork->getScheduledEntityUpdates(), + ]; + + foreach ($entities as $entity) { + if (!$entity instanceof ApiConfiguration) { + continue; + } + + $config = $entity->getConfigJson(); + $encrypted = $this->secrets->encrypt($config); + if ($encrypted === $config) { + continue; + } + + $entity->setConfigJson($encrypted); + $unitOfWork->recomputeSingleEntityChangeSet( + $entityManager->getClassMetadata(ApiConfiguration::class), + $entity, + ); + } + } +} diff --git a/src/Entity/ApiConfiguration.php b/src/Entity/ApiConfiguration.php index 2ffae42..a47738c 100644 --- a/src/Entity/ApiConfiguration.php +++ b/src/Entity/ApiConfiguration.php @@ -1,4 +1,5 @@ ` + * ({@see CredentialEncryption}, key `CREDENTIALS_ENCRYPTION_KEY`). + * - Read: secret values are replaced by {@see self::MASK}. + * - Update: an omitted secret or the mask keeps the stored value, `null` + * removes it, any other value replaces it. + * + * Which keys are secret is declared by the type schema, see + * {@see SecretSchemaResolver}. + */ +final class ConfigSecrets +{ + public const string MASK = '********'; + public const string PREFIX = 'enc:v1:'; + + /** + * @param \Closure(): CredentialEncryption $encryption resolved lazily, so a + * missing key only fails where a secret is actually encrypted or + * decrypted + */ + public function __construct( + private readonly SecretSchemaResolver $resolver, + #[AutowireServiceClosure(CredentialEncryption::class)] + private readonly \Closure $encryption, + ) { + } + + public static function isEncrypted(mixed $value): bool + { + return is_string($value) && str_starts_with($value, self::PREFIX); + } + + /** + * The `type` of a configuration, as used to look up its schema. + */ + public static function typeOf(array $config): string + { + return is_string($config['type'] ?? null) ? $config['type'] : ''; + } + + /** + * Encrypts all secret values that are not encrypted yet (idempotent). + * + * @throws SecretEncryptionException if a secret is present but no usable key is configured + */ + public function encrypt(array $config): array + { + foreach ($this->resolver->getSecretPaths(self::typeOf($config)) as $path) { + $config = $this->mapPath($config, $path, function (mixed $value): mixed { + if ($value === null || $value === '' || self::isEncrypted($value)) { + return $value; + } + + return self::PREFIX . $this->cipher()->encrypt(['v' => $value]); + }); + } + + return $config; + } + + /** + * Decrypts every encrypted value, wherever it is. Only for building a + * client, never for output. + * + * @throws SecretEncryptionException if a value cannot be decrypted + */ + public function decrypt(array $config): array + { + array_walk_recursive($config, function (mixed &$value): void { + if (!self::isEncrypted($value)) { + return; + } + + try { + $value = $this->cipher()->decrypt(substr($value, strlen(self::PREFIX)))['v'] ?? null; + } catch (SecretEncryptionException $e) { + throw $e; + } catch (\Throwable $e) { + throw new SecretEncryptionException( + 'Cannot decrypt an ApiConfiguration secret: wrong CREDENTIALS_ENCRYPTION_KEY or corrupted value.', + 0, + $e, + ); + } + }); + + return $config; + } + + /** + * Replaces secret values by {@see self::MASK}, for API output and logs. + * Encrypted values are masked even where the schema no longer marks them. + */ + public function mask(array $config): array + { + foreach ($this->resolver->getSecretPaths(self::typeOf($config)) as $path) { + $config = $this->mapPath( + $config, + $path, + static fn (mixed $value): mixed => $value === null || $value === '' ? $value : self::MASK, + ); + } + + array_walk_recursive($config, static function (mixed &$value): void { + if (self::isEncrypted($value)) { + $value = self::MASK; + } + }); + + return $config; + } + + /** + * Applies the update rules for secrets: an omitted secret or the mask + * keeps the stored value, `null` removes the key, any other value + * replaces it. Nothing is carried over when the type changes. + */ + public function keepStored(array $incoming, array $stored): array + { + $type = self::typeOf($incoming); + if ($type !== self::typeOf($stored)) { + return $incoming; + } + + foreach ($this->resolver->getSecretPaths($type) as $path) { + $incoming = $this->keepPath($incoming, $stored, $path); + } + + return $incoming; + } + + /** + * @param list $path + */ + private function keepPath(array $incoming, array $stored, array $path): array + { + $segment = array_shift($path); + $isWildcard = $segment === SecretSchemaResolver::WILDCARD; + + foreach ($isWildcard ? array_keys($incoming) : [$segment] as $key) { + if ($path !== []) { + if (is_array($incoming[$key] ?? null) && is_array($stored[$key] ?? null)) { + $incoming[$key] = $this->keepPath($incoming[$key], $stored[$key], $path); + } + continue; + } + + // Only fixed keys can be removed or omitted; for items of a list or + // map only the mask counts, so their keys stay intact + if (!$isWildcard && array_key_exists($key, $incoming) && $incoming[$key] === null) { + unset($incoming[$key]); + continue; + } + + $sendsMask = ($incoming[$key] ?? null) === self::MASK; + $omitted = !$isWildcard && !array_key_exists($key, $incoming); + + if (($sendsMask || $omitted) && array_key_exists($key, $stored)) { + $incoming[$key] = $stored[$key]; + } + } + + return $incoming; + } + + /** + * Applies $fn to every existing value at $path. + * + * @param list $path + */ + private function mapPath(array $data, array $path, \Closure $fn): array + { + $segment = array_shift($path); + $keys = $segment === SecretSchemaResolver::WILDCARD ? array_keys($data) : [$segment]; + + foreach ($keys as $key) { + if (!array_key_exists($key, $data)) { + continue; + } + + if ($path === []) { + $data[$key] = $fn($data[$key]); + } elseif (is_array($data[$key])) { + $data[$key] = $this->mapPath($data[$key], $path, $fn); + } + } + + return $data; + } + + private function cipher(): CredentialEncryption + { + try { + return ($this->encryption)(); + } catch (\Throwable $e) { + throw new SecretEncryptionException( + 'ApiConfiguration secrets cannot be encrypted or decrypted: the credential encryption service is not usable. ' + . 'Generate a key with "bin/console dmstr:generate-encryption-key", set CREDENTIALS_ENCRYPTION_KEY and configure ' + . 'dmstr_api_platform_utils.credential_encryption.key: "%env(base64:CREDENTIALS_ENCRYPTION_KEY)%". Reason: ' + . $e->getMessage(), + 0, + $e, + ); + } + } +} diff --git a/src/Security/SecretEncryptionException.php b/src/Security/SecretEncryptionException.php new file mode 100644 index 0000000..f5aa582 --- /dev/null +++ b/src/Security/SecretEncryptionException.php @@ -0,0 +1,14 @@ +>> */ + private array $cache = []; + + public function __construct( + private readonly SchemaRegistry $schemaRegistry, + private readonly ApiExtensionRegistry $extensionRegistry, + ) { + } + + /** + * Secret paths of the given type. + * + * The schema is looked up by provider name, then by extension name, then + * by the `type` const/enum of every registered schema. An unknown type + * gets the secret paths of all registered schemas, so that a missing + * mapping over-masks instead of leaking. + * + * @return list> + */ + public function getSecretPaths(string $type): array + { + return $this->cache[$type] ??= $this->resolve($type); + } + + /** + * Collects the paths of all properties marked as secret in a schema. + * + * @return list> + */ + public static function collectSecretPaths(array $schema): array + { + $paths = []; + self::walk($schema, [], $schema, [], $paths); + + return array_values($paths); + } + + /** + * @return list> + */ + private function resolve(string $type): array + { + $schemas = []; + + if ($type !== '' && ($provider = $this->schemaRegistry->getProvider($type)) !== null) { + $schemas[] = $this->load($provider->getSchemaPath()); + } elseif ($type !== '' && ($extension = $this->extensionRegistry->get($type)) !== null) { + $schemas[] = $this->load($extension->getSchemaPath()); + } else { + $all = array_map( + fn ($provider): array => $this->load($provider->getSchemaPath()), + array_values($this->schemaRegistry->getProviders()), + ); + $matching = array_filter($all, static fn (array $schema): bool => self::declaresType($schema, $type)); + $schemas = $matching !== [] ? $matching : $all; + } + + $paths = []; + foreach ($schemas as $schema) { + foreach (self::collectSecretPaths($schema) as $path) { + $paths[self::key($path)] = $path; + } + } + + return array_values($paths); + } + + private function load(string $schemaPath): array + { + $content = is_file($schemaPath) ? file_get_contents($schemaPath) : false; + if ($content === false) { + throw new \RuntimeException(sprintf('Cannot read API configuration schema "%s"', $schemaPath)); + } + + $schema = json_decode($content, true); + if (!is_array($schema)) { + throw new \RuntimeException(sprintf('Invalid JSON in API configuration schema "%s"', $schemaPath)); + } + + return $schema; + } + + private static function declaresType(array $schema, string $type): bool + { + $typeSchema = $schema['properties']['type'] ?? null; + if (!is_array($typeSchema)) { + return false; + } + + return ($typeSchema['const'] ?? null) === $type + || in_array($type, (array) ($typeSchema['enum'] ?? []), true); + } + + /** + * @param list $path + * @param array $seenRefs + * @param array> $paths + */ + private static function walk(mixed $node, array $path, array $root, array $seenRefs, array &$paths): void + { + if (!is_array($node)) { + return; + } + + if (isset($node['$ref']) && is_string($node['$ref']) && !isset($seenRefs[$node['$ref']])) { + $seenRefs[$node['$ref']] = true; + self::walk(self::resolveRef($root, $node['$ref']), $path, $root, $seenRefs, $paths); + } + + if ($path !== [] && ($node[self::MARKER] ?? false) === true) { + $paths[self::key($path)] = $path; + + return; + } + + foreach ((array) ($node['properties'] ?? []) as $name => $property) { + self::walk($property, [...$path, (string) $name], $root, $seenRefs, $paths); + } + + foreach ((array) ($node['patternProperties'] ?? []) as $property) { + self::walk($property, [...$path, self::WILDCARD], $root, $seenRefs, $paths); + } + + foreach (['additionalProperties', 'items', 'additionalItems'] as $keyword) { + $sub = $node[$keyword] ?? null; + if (is_array($sub) && array_is_list($sub)) { + // tuple form of `items` + foreach ($sub as $item) { + self::walk($item, [...$path, self::WILDCARD], $root, $seenRefs, $paths); + } + } else { + self::walk($sub, [...$path, self::WILDCARD], $root, $seenRefs, $paths); + } + } + + foreach ((array) ($node['prefixItems'] ?? []) as $item) { + self::walk($item, [...$path, self::WILDCARD], $root, $seenRefs, $paths); + } + + // Combinators and conditionals describe the same level + foreach (['allOf', 'anyOf', 'oneOf'] as $keyword) { + foreach ((array) ($node[$keyword] ?? []) as $sub) { + self::walk($sub, $path, $root, $seenRefs, $paths); + } + } + foreach (['if', 'then', 'else'] as $keyword) { + self::walk($node[$keyword] ?? null, $path, $root, $seenRefs, $paths); + } + foreach (['dependentSchemas', 'dependencies'] as $keyword) { + foreach ((array) ($node[$keyword] ?? []) as $sub) { + self::walk($sub, $path, $root, $seenRefs, $paths); + } + } + } + + /** + * Resolves a local JSON pointer (`#/definitions/auth`); external + * references are not followed. + */ + private static function resolveRef(array $root, string $ref): mixed + { + if (!str_starts_with($ref, '#')) { + return null; + } + + $node = $root; + foreach (array_filter(explode('/', substr($ref, 1)), static fn (string $s): bool => $s !== '') as $segment) { + $segment = str_replace(['~1', '~0'], ['/', '~'], rawurldecode($segment)); + if (!is_array($node) || !array_key_exists($segment, $node)) { + return null; + } + $node = $node[$segment]; + } + + return $node; + } + + /** + * @param list $path + */ + private static function key(array $path): string + { + return implode("\0", $path); + } +} diff --git a/src/Serializer/ApiConfigurationSecretsNormalizer.php b/src/Serializer/ApiConfigurationSecretsNormalizer.php new file mode 100644 index 0000000..8d1acbe --- /dev/null +++ b/src/Serializer/ApiConfigurationSecretsNormalizer.php @@ -0,0 +1,90 @@ +normalizer->normalize($data, $format, $context); + + if (is_array($normalized) && is_array($normalized[self::CONFIG_PROPERTY] ?? null)) { + $normalized[self::CONFIG_PROPERTY] = $this->secrets->mask($normalized[self::CONFIG_PROPERTY]); + } + + return $normalized; + } + + public function supportsNormalization(mixed $data, ?string $format = null, array $context = []): bool + { + return $data instanceof ApiConfiguration && !isset($context[self::ALREADY_CALLED]); + } + + public function denormalize(mixed $data, string $type, ?string $format = null, array $context = []): mixed + { + // PATCH (and non-standard PUT) populate the loaded entity; a standard + // PUT builds a new object, the stored one is in `previous_data`. + $previous = $context[AbstractNormalizer::OBJECT_TO_POPULATE] + ?? $this->requestStack->getCurrentRequest()?->attributes->get('previous_data'); + // Copy before denormalizing, which overwrites the populated object + $stored = $previous instanceof ApiConfiguration ? $previous->getConfigJson() : null; + + $context[self::ALREADY_CALLED] = true; + $object = $this->denormalizer->denormalize($data, $type, $format, $context); + + if ($stored !== null && $object instanceof ApiConfiguration) { + $object->setConfigJson($this->secrets->keepStored($object->getConfigJson(), $stored)); + } + + return $object; + } + + public function supportsDenormalization(mixed $data, string $type, ?string $format = null, array $context = []): bool + { + return $type === ApiConfiguration::class && !isset($context[self::ALREADY_CALLED]); + } + + public function getSupportedTypes(?string $format): array + { + // Not cacheable: support depends on the ALREADY_CALLED context flag + return [ApiConfiguration::class => false]; + } +} diff --git a/src/Validator/ApiConfigurationValidator.php b/src/Validator/ApiConfigurationValidator.php index d2b3380..b502734 100644 --- a/src/Validator/ApiConfigurationValidator.php +++ b/src/Validator/ApiConfigurationValidator.php @@ -5,6 +5,7 @@ namespace Dmstr\ApiConfiguration\Validator; +use Dmstr\ApiConfiguration\Security\ConfigSecrets; use Dmstr\OpenApiJsonSchema\Service\SchemaRegistry; use Opis\JsonSchema\Validator; use Psr\Log\LoggerInterface; @@ -21,7 +22,8 @@ class ApiConfigurationValidator extends ConstraintValidator { public function __construct( private readonly SchemaRegistry $schemaRegistry, - private readonly LoggerInterface $logger + private readonly LoggerInterface $logger, + private readonly ConfigSecrets $secrets, ) { } @@ -41,13 +43,17 @@ public function validate(mixed $value, Constraint $constraint): void // Get unified schema from SchemaRegistry $unifiedSchema = $this->schemaRegistry->getUnifiedSchema(); + // Secrets kept from the stored configuration are encrypted; validate + // their clear values. Logs only ever see the masked configuration. + $loggable = $this->secrets->mask($value); + // Convert to stdClass for validation - $data = json_decode(json_encode($value)); + $data = json_decode(json_encode($this->secrets->decrypt($value))); $schema = json_decode(json_encode($unifiedSchema)); // Log what we're validating $this->logger->debug('Validating API configuration', [ - 'data' => $value, + 'data' => $loggable, 'has_anyOf' => isset($unifiedSchema['anyOf']), 'anyOf_count' => isset($unifiedSchema['anyOf']) ? count($unifiedSchema['anyOf']) : 0, 'data_keys' => array_keys($value) @@ -88,7 +94,7 @@ public function validate(mixed $value, Constraint $constraint): void 'error_keyword' => $error->keyword(), 'error_args' => $error->args(), 'sub_errors' => $subErrors, - 'data' => $value + 'data' => $loggable ]); $errorMessage = $this->formatError($error); diff --git a/tests/Doctrine/EncryptConfigSecretsTest.php b/tests/Doctrine/EncryptConfigSecretsTest.php new file mode 100644 index 0000000..810d032 --- /dev/null +++ b/tests/Doctrine/EncryptConfigSecretsTest.php @@ -0,0 +1,123 @@ +enableNativeLazyObjects(true); + } + $connection = DriverManager::getConnection(['driver' => 'pdo_sqlite', 'memory' => true], $config); + $this->entityManager = new EntityManager($connection, $config); + (new SchemaTool($this->entityManager))->createSchema([ + $this->entityManager->getClassMetadata(ApiConfiguration::class), + ]); + + $this->secrets = SecretsFixture::configSecrets(); + $this->entityManager->getEventManager()->addEventListener( + Events::onFlush, + new EncryptConfigSecretsListener($this->secrets), + ); + } + + public function testInsertAndUpdateStoreSecretsEncrypted(): void + { + $entity = (new ApiConfiguration())->setName('demo')->setConfigJson(SecretsFixture::config()); + $this->entityManager->persist($entity); + $this->entityManager->flush(); + + $column = $this->storedColumn($entity); + self::assertStringNotContainsString('s3cr3t-password', $column); + self::assertStringContainsString(ConfigSecrets::PREFIX, $column); + self::assertTrue(ConfigSecrets::isEncrypted($entity->getConfigJson()['password']), 'entity matches the row'); + + $entity->setConfigJson(array_replace($entity->getConfigJson(), ['token' => 'new-token'])); + $this->entityManager->flush(); + + $column = $this->storedColumn($entity); + self::assertStringNotContainsString('new-token', $column); + self::assertSame('new-token', $this->secrets->decrypt($entity->getConfigJson())['token']); + } + + public function testCommandEncryptsExistingRowsIdempotently(): void + { + // A row written in clear before encryption at rest existed + $this->entityManager->getConnection()->insert('dmstr_api_configuration', [ + // UuidType stores binary(16) on platforms without a native GUID type + 'id' => Uuid::v4()->toBinary(), + 'name' => 'legacy', + 'type' => 'demo', + 'endpoint_type' => 'rest', + 'config_json' => json_encode(SecretsFixture::config()), + 'active' => 1, + 'created_at' => '2026-01-01 00:00:00', + 'updated_at' => '2026-01-01 00:00:00', + ], ['id' => ParameterType::BINARY]); + + $tester = new CommandTester(new EncryptApiConfigurationSecretsCommand($this->entityManager, $this->secrets)); + + self::assertSame(0, $tester->execute(['--dry-run' => true])); + self::assertStringContainsString('s3cr3t-password', $this->rawColumn('legacy'), 'dry run changes nothing'); + + self::assertSame(0, $tester->execute([])); + self::assertStringContainsString('1 configuration(s)', $tester->getDisplay()); + $encrypted = $this->rawColumn('legacy'); + self::assertStringNotContainsString('s3cr3t-password', $encrypted); + + self::assertSame(0, $tester->execute([])); + self::assertStringContainsString('nothing to do', $tester->getDisplay()); + self::assertSame($encrypted, $this->rawColumn('legacy'), 'second run leaves encrypted values alone'); + + self::assertSame( + 's3cr3t-password', + $this->secrets->decrypt(json_decode($encrypted, true))['password'], + ); + } + + private function storedColumn(ApiConfiguration $entity): string + { + return $this->rawColumn($entity->getName()); + } + + private function rawColumn(string $name): string + { + return (string) $this->entityManager->getConnection()->fetchOne( + 'SELECT config_json FROM dmstr_api_configuration WHERE name = ?', + [$name], + ); + } +} diff --git a/tests/Entity/ApiConfigurationSecurityTest.php b/tests/Entity/ApiConfigurationSecurityTest.php new file mode 100644 index 0000000..8bedfaa --- /dev/null +++ b/tests/Entity/ApiConfigurationSecurityTest.php @@ -0,0 +1,38 @@ +getAttributes(ApiResource::class); + self::assertCount(1, $attributes); + + /** @var ApiResource $resource */ + $resource = $attributes[0]->newInstance(); + self::assertSame(self::ADMIN, $resource->getSecurity()); + + foreach ($resource->getOperations() ?? [] as $operation) { + // An operation without its own expression inherits the resource's + self::assertContains( + $operation->getSecurity(), + [null, self::ADMIN], + sprintf('%s %s', $operation->getMethod(), $operation->getUriTemplate() ?? $operation::class), + ); + } + } +} diff --git a/tests/Fixtures/SecretsFixture.php b/tests/Fixtures/SecretsFixture.php new file mode 100644 index 0000000..3be27eb --- /dev/null +++ b/tests/Fixtures/SecretsFixture.php @@ -0,0 +1,66 @@ +register(new class implements SchemaProviderInterface { + public function getName(): string + { + return 'demo'; + } + + public function getSchemaPath(): string + { + return __DIR__ . '/demo.schema.json'; + } + }); + + return $registry; + } + + /** + * @param (\Closure(): CredentialEncryption)|null $encryption defaults to a working key + */ + public static function configSecrets(?\Closure $encryption = null): ConfigSecrets + { + return new ConfigSecrets( + new SecretSchemaResolver(self::schemaRegistry(), new ApiExtensionRegistry()), + $encryption ?? static fn (): CredentialEncryption => new CredentialEncryption(self::KEY), + ); + } + + public static function config(array $overrides = []): array + { + return array_replace([ + 'type' => 'demo', + 'endpoint_type' => 'rest', + 'base_url' => 'https://api.example.com', + 'auth_type' => 'basic', + 'username' => 'alice', + 'password' => 's3cr3t-password', + ], $overrides); + } +} diff --git a/tests/Fixtures/demo.schema.json b/tests/Fixtures/demo.schema.json new file mode 100644 index 0000000..3cfe8a9 --- /dev/null +++ b/tests/Fixtures/demo.schema.json @@ -0,0 +1,34 @@ +{ + "$comment": "file generated with AI assistance: Claude Code - 2026-10-05 10:25:39 UTC", + "type": "object", + "required": ["type", "endpoint_type", "base_url", "auth_type"], + "properties": { + "type": { "const": "demo" }, + "endpoint_type": { "const": "rest" }, + "base_url": { "type": "string", "format": "uri" }, + "auth_type": { "enum": ["basic", "bearer"] }, + "username": { "type": "string" }, + "password": { "type": "string", "writeOnly": true }, + "token": { "type": "string", "writeOnly": true }, + "oauth": { "$ref": "#/definitions/oauth" }, + "headers": { + "type": "array", + "items": { + "type": "object", + "properties": { + "name": { "type": "string" }, + "value": { "type": "string", "writeOnly": true } + } + } + } + }, + "definitions": { + "oauth": { + "type": "object", + "properties": { + "client_id": { "type": "string" }, + "client_secret": { "type": "string", "minLength": 4, "writeOnly": true } + } + } + } +} diff --git a/tests/Security/ConfigSecretsTest.php b/tests/Security/ConfigSecretsTest.php new file mode 100644 index 0000000..12e3bc0 --- /dev/null +++ b/tests/Security/ConfigSecretsTest.php @@ -0,0 +1,150 @@ + ['client_id' => 'app', 'client_secret' => 'oauth-secret'], + 'headers' => [['name' => 'X-Api-Key', 'value' => 'header-secret']], + ]); + + $encrypted = $secrets->encrypt($config); + $json = json_encode($encrypted); + + foreach (['s3cr3t-password', 'oauth-secret', 'header-secret'] as $clear) { + self::assertStringNotContainsString($clear, $json); + } + self::assertTrue(ConfigSecrets::isEncrypted($encrypted['password'])); + self::assertTrue(ConfigSecrets::isEncrypted($encrypted['oauth']['client_secret'])); + self::assertTrue(ConfigSecrets::isEncrypted($encrypted['headers'][0]['value'])); + self::assertSame('alice', $encrypted['username']); + self::assertSame('app', $encrypted['oauth']['client_id']); + + self::assertSame($config, $secrets->decrypt($encrypted)); + } + + public function testEncryptIsIdempotent(): void + { + $secrets = SecretsFixture::configSecrets(); + $once = $secrets->encrypt(SecretsFixture::config()); + + self::assertSame($once, $secrets->encrypt($once)); + } + + public function testMasksSecretsInClearAndEncrypted(): void + { + $secrets = SecretsFixture::configSecrets(); + $clear = SecretsFixture::config(['token' => '']); + + $masked = $secrets->mask($clear); + self::assertSame(ConfigSecrets::MASK, $masked['password']); + self::assertSame('', $masked['token'], 'empty values carry no secret'); + self::assertSame('alice', $masked['username']); + + self::assertSame(ConfigSecrets::MASK, $secrets->mask($secrets->encrypt($clear))['password']); + } + + public function testMasksEncryptedValuesTheSchemaNoLongerMarks(): void + { + $secrets = SecretsFixture::configSecrets(); + $config = SecretsFixture::config(['legacy' => ConfigSecrets::PREFIX . 'abc']); + + self::assertSame(ConfigSecrets::MASK, $secrets->mask($config)['legacy']); + } + + public function testUpdateKeepsOmittedAndMaskedSecrets(): void + { + $secrets = SecretsFixture::configSecrets(); + $stored = $secrets->encrypt(SecretsFixture::config([ + 'token' => 'stored-token', + 'headers' => [['name' => 'X-Api-Key', 'value' => 'header-secret']], + ])); + + $incoming = SecretsFixture::config([ + 'password' => ConfigSecrets::MASK, + 'headers' => [['name' => 'X-Api-Key', 'value' => ConfigSecrets::MASK]], + ]); + unset($incoming['token']); + + $merged = $secrets->keepStored($incoming, $stored); + + self::assertSame($stored['password'], $merged['password']); + self::assertSame($stored['token'], $merged['token']); + self::assertSame($stored['headers'][0]['value'], $merged['headers'][0]['value']); + } + + public function testUpdateReplacesWithNewValueAndRemovesWithNull(): void + { + $secrets = SecretsFixture::configSecrets(); + $stored = $secrets->encrypt(SecretsFixture::config(['token' => 'stored-token'])); + + $merged = $secrets->keepStored( + SecretsFixture::config(['password' => 'new-password', 'token' => null]), + $stored, + ); + + self::assertSame('new-password', $merged['password']); + self::assertArrayNotHasKey('token', $merged); + } + + public function testUpdateDoesNotResurrectRemovedParentOrCrossTypes(): void + { + $secrets = SecretsFixture::configSecrets(); + $stored = $secrets->encrypt(SecretsFixture::config([ + 'oauth' => ['client_id' => 'app', 'client_secret' => 'oauth-secret'], + ])); + + $withoutOauth = SecretsFixture::config(['password' => ConfigSecrets::MASK]); + self::assertArrayNotHasKey('oauth', $secrets->keepStored($withoutOauth, $stored)); + + $otherType = ['type' => 'other', 'endpoint_type' => 'rest']; + self::assertSame($otherType, $secrets->keepStored($otherType, $stored)); + } + + public function testFailsLoudlyWithoutKey(): void + { + $secrets = SecretsFixture::configSecrets( + static fn (): CredentialEncryption => new CredentialEncryption(''), + ); + + $this->expectException(SecretEncryptionException::class); + $secrets->encrypt(SecretsFixture::config()); + } + + public function testConfigWithoutSecretsNeedsNoKey(): void + { + $secrets = SecretsFixture::configSecrets( + static fn (): CredentialEncryption => throw new \LogicException('key must not be needed'), + ); + $config = SecretsFixture::config(); + unset($config['password']); + + self::assertSame($config, $secrets->encrypt($config)); + self::assertSame($config, $secrets->decrypt($config)); + self::assertSame($config, $secrets->mask($config)); + } + + public function testDecryptWithWrongKeyFails(): void + { + $encrypted = SecretsFixture::configSecrets()->encrypt(SecretsFixture::config()); + $otherKey = SecretsFixture::configSecrets( + static fn (): CredentialEncryption => new CredentialEncryption(str_repeat('x', SODIUM_CRYPTO_SECRETBOX_KEYBYTES)), + ); + + $this->expectException(SecretEncryptionException::class); + $otherKey->decrypt($encrypted); + } +} diff --git a/tests/Security/SecretSchemaResolverTest.php b/tests/Security/SecretSchemaResolverTest.php new file mode 100644 index 0000000..21d8c16 --- /dev/null +++ b/tests/Security/SecretSchemaResolverTest.php @@ -0,0 +1,80 @@ + 'object', + 'oneOf' => [ + ['properties' => ['password' => ['type' => 'string', 'writeOnly' => true]]], + ['properties' => ['token' => ['type' => 'string', 'writeOnly' => true]]], + ], + 'properties' => [ + 'secret_headers' => [ + 'type' => 'object', + 'additionalProperties' => ['type' => 'string', 'writeOnly' => true], + ], + 'not_secret' => ['type' => 'string', 'writeOnly' => false], + ], + ]; + + self::assertEqualsCanonicalizing( + [['password'], ['token'], ['secret_headers', '*']], + SecretSchemaResolver::collectSecretPaths($schema), + ); + } + + public function testRecursiveRefTerminates(): void + { + $schema = [ + 'properties' => ['node' => ['$ref' => '#/definitions/node']], + 'definitions' => [ + 'node' => [ + 'properties' => [ + 'key' => ['type' => 'string', 'writeOnly' => true], + 'child' => ['$ref' => '#/definitions/node'], + ], + ], + ], + ]; + + self::assertSame([['node', 'key']], SecretSchemaResolver::collectSecretPaths($schema)); + } + + public function testResolvesByProviderName(): void + { + $resolver = new SecretSchemaResolver(SecretsFixture::schemaRegistry(), new ApiExtensionRegistry()); + + self::assertContains(['password'], $resolver->getSecretPaths('demo')); + } + + public function testUnknownTypeFallsBackToAllSchemas(): void + { + $resolver = new SecretSchemaResolver(SecretsFixture::schemaRegistry(), new ApiExtensionRegistry()); + + // Over-masking beats leaking when a type has no schema of its own + self::assertContains(['password'], $resolver->getSecretPaths('unknown')); + self::assertContains(['password'], $resolver->getSecretPaths('')); + } +} diff --git a/tests/Serializer/ApiConfigurationSecretsNormalizerTest.php b/tests/Serializer/ApiConfigurationSecretsNormalizerTest.php new file mode 100644 index 0000000..ca1a94e --- /dev/null +++ b/tests/Serializer/ApiConfigurationSecretsNormalizerTest.php @@ -0,0 +1,125 @@ +secrets = SecretsFixture::configSecrets(); + $this->requestStack = new RequestStack(); + $this->normalizer = new ApiConfigurationSecretsNormalizer($this->secrets, $this->requestStack); + $this->normalizer->setNormalizer(new InnerSerializer()); + $this->normalizer->setDenormalizer(new InnerSerializer()); + } + + public function testResponseNeverContainsSecrets(): void + { + $entity = $this->entity($this->secrets->encrypt(SecretsFixture::config())); + + self::assertTrue($this->normalizer->supportsNormalization($entity, 'jsonld')); + $normalized = $this->normalizer->normalize($entity, 'jsonld'); + + self::assertSame(ConfigSecrets::MASK, $normalized['configJson']['password']); + self::assertSame('alice', $normalized['configJson']['username']); + self::assertStringNotContainsString(ConfigSecrets::PREFIX, json_encode($normalized)); + } + + public function testPatchWithMaskKeepsStoredSecret(): void + { + $entity = $this->entity($this->secrets->encrypt(SecretsFixture::config())); + $storedPassword = $entity->getConfigJson()['password']; + + $result = $this->normalizer->denormalize( + ['configJson' => SecretsFixture::config(['password' => ConfigSecrets::MASK, 'username' => 'bob'])], + ApiConfiguration::class, + 'json', + [AbstractNormalizer::OBJECT_TO_POPULATE => $entity], + ); + + self::assertSame($storedPassword, $result->getConfigJson()['password']); + self::assertSame('bob', $result->getConfigJson()['username']); + } + + public function testStandardPutUsesPreviousData(): void + { + $previous = $this->entity($this->secrets->encrypt(SecretsFixture::config())); + $request = new Request(); + $request->attributes->set('previous_data', $previous); + $this->requestStack->push($request); + + $config = SecretsFixture::config(); + unset($config['password']); + $result = $this->normalizer->denormalize(['configJson' => $config], ApiConfiguration::class, 'json'); + + self::assertNotSame($previous, $result); + self::assertSame($previous->getConfigJson()['password'], $result->getConfigJson()['password']); + } + + public function testPostStoresNewValue(): void + { + $result = $this->normalizer->denormalize( + ['configJson' => SecretsFixture::config()], + ApiConfiguration::class, + 'json', + ); + + self::assertSame('s3cr3t-password', $result->getConfigJson()['password']); + } + + private function entity(array $config): ApiConfiguration + { + return (new ApiConfiguration())->setName('demo')->setConfigJson($config); + } +} + +/** + * Stands in for the API Platform item normalizer. + */ +final class InnerSerializer implements NormalizerInterface, DenormalizerInterface +{ + public function normalize(mixed $data, ?string $format = null, array $context = []): array + { + return ['@id' => '/api/admin/api_configurations/x', 'name' => $data->getName(), 'configJson' => $data->getConfigJson()]; + } + + public function supportsNormalization(mixed $data, ?string $format = null, array $context = []): bool + { + return true; + } + + public function denormalize(mixed $data, string $type, ?string $format = null, array $context = []): ApiConfiguration + { + $object = $context[AbstractNormalizer::OBJECT_TO_POPULATE] ?? new ApiConfiguration(); + + return $object->setConfigJson($data['configJson']); + } + + public function supportsDenormalization(mixed $data, string $type, ?string $format = null, array $context = []): bool + { + return true; + } + + public function getSupportedTypes(?string $format): array + { + return ['*' => false]; + } +} From 5fce347bc614a229f6d9a0cdd98aba2619f641d4 Mon Sep 17 00:00:00 2001 From: "t.munk (workhorse)" Date: Mon, 5 Oct 2026 11:45:12 +0000 Subject: [PATCH 2/7] test(security): use the underscore naming strategy of a Symfony app [*] Co-Authored-By: Claude Opus 5.5 --- tests/Doctrine/EncryptConfigSecretsTest.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/Doctrine/EncryptConfigSecretsTest.php b/tests/Doctrine/EncryptConfigSecretsTest.php index 810d032..47b6adb 100644 --- a/tests/Doctrine/EncryptConfigSecretsTest.php +++ b/tests/Doctrine/EncryptConfigSecretsTest.php @@ -16,6 +16,7 @@ use Doctrine\ORM\EntityManager; use Doctrine\ORM\Events; use Doctrine\ORM\ORMSetup; +use Doctrine\ORM\Mapping\UnderscoreNamingStrategy; use Doctrine\ORM\Tools\SchemaTool; use PHPUnit\Framework\TestCase; use Symfony\Bridge\Doctrine\Types\UuidType; @@ -38,6 +39,8 @@ protected function setUp(): void } $config = ORMSetup::createAttributeMetadataConfiguration([__DIR__ . '/../../src/Entity'], true); + // Column names as in a Symfony app (endpoint_type, created_at, ...) + $config->setNamingStrategy(new UnderscoreNamingStrategy(CASE_LOWER, true)); if (method_exists($config, 'enableNativeLazyObjects')) { $config->enableNativeLazyObjects(true); } From bfc10071f8c5b88f702c72eeae3c1742454d9cd7 Mon Sep 17 00:00:00 2001 From: "t.munk (workhorse)" Date: Mon, 5 Oct 2026 11:46:02 +0000 Subject: [PATCH 3/7] test(security): bind the legacy row id with the uuid type like the ORM [*] Co-Authored-By: Claude Opus 5.5 --- tests/Doctrine/EncryptConfigSecretsTest.php | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/tests/Doctrine/EncryptConfigSecretsTest.php b/tests/Doctrine/EncryptConfigSecretsTest.php index 47b6adb..a0b45a1 100644 --- a/tests/Doctrine/EncryptConfigSecretsTest.php +++ b/tests/Doctrine/EncryptConfigSecretsTest.php @@ -11,7 +11,6 @@ use Dmstr\ApiConfiguration\Security\ConfigSecrets; use Dmstr\ApiConfiguration\Tests\Fixtures\SecretsFixture; use Doctrine\DBAL\DriverManager; -use Doctrine\DBAL\ParameterType; use Doctrine\DBAL\Types\Type; use Doctrine\ORM\EntityManager; use Doctrine\ORM\Events; @@ -80,8 +79,8 @@ public function testCommandEncryptsExistingRowsIdempotently(): void { // A row written in clear before encryption at rest existed $this->entityManager->getConnection()->insert('dmstr_api_configuration', [ - // UuidType stores binary(16) on platforms without a native GUID type - 'id' => Uuid::v4()->toBinary(), + // Converted and bound by the same Doctrine type the ORM uses + 'id' => Uuid::v4(), 'name' => 'legacy', 'type' => 'demo', 'endpoint_type' => 'rest', @@ -89,7 +88,7 @@ public function testCommandEncryptsExistingRowsIdempotently(): void 'active' => 1, 'created_at' => '2026-01-01 00:00:00', 'updated_at' => '2026-01-01 00:00:00', - ], ['id' => ParameterType::BINARY]); + ], ['id' => 'uuid']); $tester = new CommandTester(new EncryptApiConfigurationSecretsCommand($this->entityManager, $this->secrets)); From 85eb1589df6dd789334e1fa7b36115de2a7357d7 Mon Sep 17 00:00:00 2001 From: "t.munk (workhorse)" Date: Mon, 5 Oct 2026 12:09:01 +0000 Subject: [PATCH 4/7] feat(api-client)!: capability interfaces, throwing authenticate, schema-validated config [*] Closes #9. - ApiClientInterface keeps what every source supports; getCustomers(), getTodoLists() and hasChanges() move to CustomerAware-, TodoListAwareApiClientInterface and ChangeProbeInterface. - authenticate(): void throws AuthenticationFailedException with the transport exception as previous. - ApiClientFactory::create() validates the decrypted config against the extension's schema.json and throws InvalidConfigurationException naming the field. - HealthProbeInterface for types without a full client; the shared ApiConfigurationHealthChecker serves route and command. - Health operation normalizes nested arrays raw (api_sub_level), so metadata and error keep their keys in JSON-LD. BREAKING CHANGE: ApiClientInterface loses getCustomers(), getTodoLists() and hasChanges(); authenticate() returns void and throws; invalid configs are rejected by ApiClientFactory before createClient(). Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 10 +- README.md | 19 ++++ src/ApiClient/ApiClientFactory.php | 41 ++++++++- src/ApiClient/ApiClientInterface.php | 50 +++------- .../AuthenticationFailedException.php | 23 +++++ src/ApiClient/ChangeProbeInterface.php | 31 +++++++ .../CustomerAwareApiClientInterface.php | 23 +++++ .../InvalidConfigurationException.php | 37 ++++++++ .../TodoListAwareApiClientInterface.php | 22 +++++ src/ApiConfigurationBundle.php | 6 ++ src/Command/ApiConfigurationHealthCommand.php | 66 ++----------- src/Command/TestApiConnectionCommand.php | 73 +++++++++------ src/Entity/ApiConfiguration.php | 5 +- src/Health/ApiConfigurationHealthChecker.php | 87 ++++++++++++++++++ src/Health/HealthProbeInterface.php | 41 +++++++++ src/Schema/JsonSchemaFile.php | 30 ++++++ src/Security/SecretSchemaResolver.php | 22 +---- src/State/ApiConfigurationHealthProvider.php | 92 ++----------------- tests/ApiClient/ApiClientFactoryTest.php | 74 +++++++++++++++ .../ApiConfigurationHealthOperationTest.php | 35 +++++++ tests/Fixtures/DemoClient.php | 58 ++++++++++++ tests/Fixtures/DemoExtension.php | 40 ++++++++ .../ApiConfigurationHealthCheckerTest.php | 91 ++++++++++++++++++ 23 files changed, 738 insertions(+), 238 deletions(-) create mode 100644 src/ApiClient/AuthenticationFailedException.php create mode 100644 src/ApiClient/ChangeProbeInterface.php create mode 100644 src/ApiClient/CustomerAwareApiClientInterface.php create mode 100644 src/ApiClient/InvalidConfigurationException.php create mode 100644 src/ApiClient/TodoListAwareApiClientInterface.php create mode 100644 src/Health/ApiConfigurationHealthChecker.php create mode 100644 src/Health/HealthProbeInterface.php create mode 100644 src/Schema/JsonSchemaFile.php create mode 100644 tests/ApiClient/ApiClientFactoryTest.php create mode 100644 tests/Entity/ApiConfigurationHealthOperationTest.php create mode 100644 tests/Fixtures/DemoClient.php create mode 100644 tests/Fixtures/DemoExtension.php create mode 100644 tests/Health/ApiConfigurationHealthCheckerTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 9e38ed1..e9a423b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ ## Unreleased -Secure storage and transport of the credentials in `configJson` ([Issue #8](https://github.com/dmstr/api-configuration-bundle/issues/8)). Release as a new minor version: the changes below break consumers in the ways listed under "Breaking changes". +Secure storage and transport of the credentials in `configJson` ([Issue #8](https://github.com/dmstr/api-configuration-bundle/issues/8)) and a slimmer client contract ([Issue #9](https://github.com/dmstr/api-configuration-bundle/issues/9)). Release as a new minor version: the changes below break consumers in the ways listed under "Breaking changes". ### Breaking changes @@ -12,7 +12,10 @@ Secure storage and transport of the credentials in `configJson` ([Issue #8](http - **Secrets are masked in responses.** Keys marked `writeOnly` in the type schema come back as `********`, also for administrators. Clients that read credentials over the API no longer get them. - **Secrets are encrypted at rest.** `ApiConfiguration::getConfigJson()` returns `enc:v1:…` for marked keys after a flush. Code that reads secrets from the entity directly (instead of through `ApiClientFactory`) has to decrypt with `ConfigSecrets::decrypt()`, for example an `AuthorizableExtensionInterface` that reads a client secret in `buildAuthorizeUrl()`. - **A usable encryption key is required** as soon as a configuration holds a marked secret: `dmstr_api_platform_utils.credential_encryption.key: '%env(base64:CREDENTIALS_ENCRYPTION_KEY)%'`. Without it, persisting or using the secret throws `SecretEncryptionException`. -- **Constructor signatures**: `ApiClientFactory`, `ApiConfigurationValidator` and `CreateApiConfigurationCommand` take an additional `ConfigSecrets` argument (only relevant when they are instantiated manually instead of autowired). +- **`ApiClientInterface` is slimmer** (Issue #9). `getCustomers()`, `getTodoLists()` and `hasChanges()` moved to the capability interfaces `CustomerAwareApiClientInterface`, `TodoListAwareApiClientInterface` and `ChangeProbeInterface`. Clients implement only what their source supports and drop their stubs (`return []`, `return true`); callers check `instanceof` and treat a client without `ChangeProbeInterface` as "always changed". +- **`authenticate(): void` throws** (Issue #9) `AuthenticationFailedException` with the transport exception as `previous`, instead of returning `false`. Implementations rethrow via `AuthenticationFailedException::fromPrevious($apiName, $e)`; callers catch instead of checking the return value. +- **`ApiClientFactory::create()` validates the configuration** against the extension's `schema.json` before `createClient()` (Issue #9) and throws `InvalidConfigurationException` (an `\InvalidArgumentException`) naming the failing field. Configurations that an extension accepted despite its own schema are now rejected. Keys starting with `_` (`_apiConfigurationId`) are not validated. +- **Constructor signatures**: `ApiClientFactory`, `ApiConfigurationValidator` and `CreateApiConfigurationCommand` take an additional `ConfigSecrets` argument; `ApiConfigurationHealthProvider` and `ApiConfigurationHealthCommand` take `ApiConfigurationHealthChecker` instead of `ApiClientFactory` and `HealthNormalizer` (only relevant when they are instantiated manually instead of autowired). ### Added @@ -21,9 +24,12 @@ Secure storage and transport of the credentials in `configJson` ([Issue #8](http - Masking of the marked keys in API responses and in the validator's logs. - Update rule: an omitted secret or the mask keeps the stored value, `null` removes it. - Console command `app:api-configuration:encrypt-secrets` (`--dry-run`) to encrypt the secrets of existing rows, idempotent. +- `HealthProbeInterface` (Issue #9): a health check per type without the project-management client methods, autoconfigured. Schema-only types (no `ApiExtensionInterface`) answer the health route and command instead of "Unsupported API name"; a probe takes precedence over the client-based check. +- `ApiConfigurationHealthChecker`: the health check shared by the health route, the health command and application dashboards. ### Fixed +- `GET /api/admin/api_configurations/{id}/health` in JSON-LD returned `metadata` and `error` as `hydra:Collection` without their keys (Issue #9); the operation now normalizes nested arrays raw (`api_sub_level`). - `app:api:test-connection` called the non-existent `ApiConfiguration::getCredentials()` and failed with a PHP error; it now builds the client from the entity. ## 0.4.0 and earlier diff --git a/README.md b/README.md index 6b1dff2..0a3140c 100644 --- a/README.md +++ b/README.md @@ -14,6 +14,25 @@ Manage external API connections as Doctrine entities. - OAuth callback controller for `authorize` flow - CLI mirrors: `api-configuration:create`, `:health`, `:test-connection` +## API clients + +An extension implements `ApiExtensionInterface` (tagged automatically) and builds a client implementing `ApiClientInterface`, which holds only what every source supports. Optional capabilities are separate interfaces; implement those the source really supports, callers check `instanceof`: + +| Interface | Methods | +|---|---| +| `CustomerAwareApiClientInterface` | `getCustomers()` | +| `TodoListAwareApiClientInterface` | `getTodoLists()` | +| `ChangeProbeInterface` | `hasChanges()` — cheap, fail open; without it a source counts as always changed | +| `UserAwareApiClientInterface` | `getUsers()` | + +`authenticate()` returns nothing and throws `AuthenticationFailedException` with the original exception as `previous`. + +`ApiClientFactory::create()` validates the configuration against the extension's `schema.json` before calling `createClient()`, so extensions need no `isset()` checks of their own; an invalid configuration throws `InvalidConfigurationException` naming the field. + +### Health checks + +`ApiConfigurationHealthChecker` serves the health route, `app:api-configuration:health` and application dashboards. For a type with a `HealthProbeInterface` (tagged automatically) it calls the probe with the decrypted configuration; otherwise it builds the client and calls `getHealthInfo()`. Types without a full client, such as schema-only connection types, only need a probe. + ## Security ### Access diff --git a/src/ApiClient/ApiClientFactory.php b/src/ApiClient/ApiClientFactory.php index f5c907f..8852894 100644 --- a/src/ApiClient/ApiClientFactory.php +++ b/src/ApiClient/ApiClientFactory.php @@ -1,12 +1,16 @@ createClient($this->secrets->decrypt($config)); + $config = $this->secrets->decrypt($config); + $this->validate($extension, $config); + + return $extension->createClient($config); } /** @@ -61,4 +75,25 @@ public function createFromEntity(\Dmstr\ApiConfiguration\Entity\ApiConfiguration $config ); } + + /** + * @throws InvalidConfigurationException + */ + private function validate(ApiExtensionInterface $extension, array $config): void + { + $public = array_filter( + $config, + static fn (int|string $key): bool => !str_starts_with((string) $key, '_'), + ARRAY_FILTER_USE_KEY, + ); + $schema = json_decode(json_encode(JsonSchemaFile::load($extension->getSchemaPath()))); + + $result = (new Validator())->validate(json_decode(json_encode((object) $public)), $schema); + if (!$result->isValid()) { + throw new InvalidConfigurationException( + $extension->getName(), + (new ErrorFormatter())->format($result->error(), true), + ); + } + } } diff --git a/src/ApiClient/ApiClientInterface.php b/src/ApiClient/ApiClientInterface.php index d759a76..92fd82d 100644 --- a/src/ApiClient/ApiClientInterface.php +++ b/src/ApiClient/ApiClientInterface.php @@ -1,4 +1,5 @@ getMessage()), + 0, + $previous, + ); + } +} diff --git a/src/ApiClient/ChangeProbeInterface.php b/src/ApiClient/ChangeProbeInterface.php new file mode 100644 index 0000000..fbd60eb --- /dev/null +++ b/src/ApiClient/ChangeProbeInterface.php @@ -0,0 +1,31 @@ +> $errors messages by JSON pointer + */ + public function __construct( + string $apiName, + private readonly array $errors, + ) { + $lines = []; + foreach ($errors as $pointer => $messages) { + $lines[] = sprintf('%s: %s', $pointer === '' ? '/' : $pointer, implode(' ', $messages)); + } + + parent::__construct(sprintf('Invalid configuration for "%s": %s', $apiName, implode('; ', $lines))); + } + + /** + * @return array> + */ + public function getErrors(): array + { + return $this->errors; + } +} diff --git a/src/ApiClient/TodoListAwareApiClientInterface.php b/src/ApiClient/TodoListAwareApiClientInterface.php new file mode 100644 index 0000000..e8b1d72 --- /dev/null +++ b/src/ApiClient/TodoListAwareApiClientInterface.php @@ -0,0 +1,22 @@ +registerForAutoconfiguration(ApiExtensionInterface::class) ->addTag(self::EXTENSION_TAG); + + // Same for health probes of types without a full API client + $container->registerForAutoconfiguration(HealthProbeInterface::class) + ->addTag(self::HEALTH_PROBE_TAG); } public function loadExtension( diff --git a/src/Command/ApiConfigurationHealthCommand.php b/src/Command/ApiConfigurationHealthCommand.php index e8a969f..0490348 100644 --- a/src/Command/ApiConfigurationHealthCommand.php +++ b/src/Command/ApiConfigurationHealthCommand.php @@ -1,13 +1,12 @@ section('Performing Health Check...'); - // Measure response time - $startTime = microtime(true); - $client = null; + $normalizedHealth = $this->healthChecker->check($config); + $this->displayHealthResult($io, $normalizedHealth); - try { - // Create API client - $client = $this->clientFactory->createFromEntity($config); - - // Get health info from client - $rawHealthInfo = $client->getHealthInfo(); - - // Calculate response time - $responseTime = (microtime(true) - $startTime) * 1000; // Convert to milliseconds - - // Normalize health info — endpoint comes from the client itself - // (mirrors ApiConfigurationHealthProvider) - $normalizedHealth = $this->healthNormalizer->normalize( - $rawHealthInfo, - $client->getEndpoint(), - $responseTime - ); - - // Display results - $this->displayHealthResult($io, $normalizedHealth); - - return $normalizedHealth['status'] === 'ok' ? Command::SUCCESS : Command::FAILURE; - } catch (\Exception $e) { - $responseTime = (microtime(true) - $startTime) * 1000; - - // Fall back to a URN identifying the ApiConfiguration when the - // client could not be created — still a valid URI per RFC 8141. - $endpoint = $client?->getEndpoint() - ?? sprintf('urn:za7:api-configuration:%s', $config->getId()); - - $normalizedHealth = $this->healthNormalizer->normalize( - [ - 'status' => 'error', - 'authenticated' => false, - 'message' => 'Health check failed: ' . $e->getMessage(), - 'error' => [ - 'code' => 'HEALTH_CHECK_ERROR', - 'message' => $e->getMessage(), - ], - ], - $endpoint, - $responseTime - ); - - $this->displayHealthResult($io, $normalizedHealth); - - if ($output->isVerbose()) { - $io->writeln("\n" . $e->getTraceAsString()); - } - - return Command::FAILURE; - } + return $normalizedHealth['status'] === 'ok' ? Command::SUCCESS : Command::FAILURE; } /** diff --git a/src/Command/TestApiConnectionCommand.php b/src/Command/TestApiConnectionCommand.php index 6ed046f..fae1191 100644 --- a/src/Command/TestApiConnectionCommand.php +++ b/src/Command/TestApiConnectionCommand.php @@ -6,6 +6,7 @@ namespace Dmstr\ApiConfiguration\Command; use Dmstr\ApiConfiguration\ApiClient\ApiClientFactory; +use Dmstr\ApiConfiguration\ApiClient\AuthenticationFailedException; use Dmstr\ApiConfiguration\Entity\ApiConfiguration; use Dmstr\ApiPlatformUtils\Service\UuidResolver; use Symfony\Component\Console\Attribute\AsCommand; @@ -77,39 +78,51 @@ protected function execute(InputInterface $input, OutputInterface $output): int try { $client = $this->clientFactory->createFromEntity($config); - - if ($client->authenticate()) { - $io->success('Authentication successful!'); - - $io->section('Fetching projects...'); - $projects = $client->getProjects(); - - $io->success(sprintf('Successfully fetched %d projects', count($projects))); - - if (count($projects) > 0) { - $io->writeln('First 5 projects:'); - $projectList = array_slice($projects, 0, 5); - $rows = []; - foreach ($projectList as $project) { - $rows[] = [ - $project['id'] ?? $project['name'] ?? 'N/A', - $project['name'] ?? 'N/A', - ]; - } - $io->table(['ID', 'Name'], $rows); - } - - return Command::SUCCESS; - } else { - $io->error('Authentication failed. Please check your credentials.'); - return Command::FAILURE; + $client->authenticate(); + } catch (AuthenticationFailedException $e) { + $io->error($e->getMessage()); + // The cause (401, DNS, TLS, ...) is the reason to run this command + for ($cause = $e->getPrevious(); $cause !== null; $cause = $cause->getPrevious()) { + $io->writeln(sprintf(' Caused by %s: %s', $cause::class, $cause->getMessage())); } + return Command::FAILURE; } catch (\Exception $e) { - $io->error(sprintf('Error: %s', $e->getMessage())); - if ($output->isVerbose()) { - $io->writeln($e->getTraceAsString()); + return $this->failWith($io, $output, $e); + } + + $io->success('Authentication successful!'); + + try { + $io->section('Fetching projects...'); + $projects = $client->getProjects(); + } catch (\Exception $e) { + return $this->failWith($io, $output, $e); + } + + $io->success(sprintf('Successfully fetched %d projects', count($projects))); + + if (count($projects) > 0) { + $io->writeln('First 5 projects:'); + $rows = []; + foreach (array_slice($projects, 0, 5) as $project) { + $rows[] = [ + $project['id'] ?? $project['name'] ?? 'N/A', + $project['name'] ?? 'N/A', + ]; } - return Command::FAILURE; + $io->table(['ID', 'Name'], $rows); } + + return Command::SUCCESS; + } + + private function failWith(SymfonyStyle $io, OutputInterface $output, \Exception $e): int + { + $io->error(sprintf('Error: %s', $e->getMessage())); + if ($output->isVerbose()) { + $io->writeln($e->getTraceAsString()); + } + + return Command::FAILURE; } } diff --git a/src/Entity/ApiConfiguration.php b/src/Entity/ApiConfiguration.php index a47738c..e27a6a5 100644 --- a/src/Entity/ApiConfiguration.php +++ b/src/Entity/ApiConfiguration.php @@ -46,7 +46,10 @@ new Get( uriTemplate: '/api_configurations/{id}/health', name: 'api_configuration_health', - provider: ApiConfigurationHealthProvider::class + provider: ApiConfigurationHealthProvider::class, + // Nested arrays (metadata, error) stay plain objects in JSON-LD + // instead of becoming hydra:Collection (Issue #9) + normalizationContext: ['api_sub_level' => true], ), new Get( uriTemplate: '/api_configurations/{id}/authorize', diff --git a/src/Health/ApiConfigurationHealthChecker.php b/src/Health/ApiConfigurationHealthChecker.php new file mode 100644 index 0000000..61fdf29 --- /dev/null +++ b/src/Health/ApiConfigurationHealthChecker.php @@ -0,0 +1,87 @@ + */ + private array $probes = []; + + /** + * @param iterable $probes + */ + public function __construct( + private readonly ApiClientFactory $clientFactory, + private readonly HealthNormalizer $healthNormalizer, + private readonly ConfigSecrets $secrets, + #[AutowireIterator(ApiConfigurationBundle::HEALTH_PROBE_TAG)] + iterable $probes = [], + ) { + foreach ($probes as $probe) { + $this->probes[$probe->getName()] = $probe; + } + } + + /** + * @return array normalized health result, see health.json + */ + public function check(ApiConfiguration $apiConfiguration): array + { + $startTime = microtime(true); + $endpoint = null; + + try { + $probe = $this->probes[$apiConfiguration->getType()] ?? null; + if ($probe !== null) { + $config = $this->secrets->decrypt($apiConfiguration->getConfigJson()); + $endpoint = $probe->getEndpoint($config); + $rawHealthInfo = $probe->probe($config); + } else { + $client = $this->clientFactory->createFromEntity($apiConfiguration); + $endpoint = $client->getEndpoint(); + $rawHealthInfo = $client->getHealthInfo(); + } + + return $this->healthNormalizer->normalize($rawHealthInfo, $endpoint, $this->elapsedMs($startTime)); + } catch (\Exception $e) { + // Without an endpoint (client/probe failed early) fall back to a + // URN identifying the ApiConfiguration — a valid URI per RFC 8141. + return $this->healthNormalizer->normalize( + [ + 'status' => 'error', + 'authenticated' => false, + 'message' => 'Health check failed: ' . $e->getMessage(), + 'error' => [ + 'code' => 'HEALTH_CHECK_ERROR', + 'message' => $e->getMessage(), + ], + ], + $endpoint ?? sprintf('urn:za7:api-configuration:%s', $apiConfiguration->getId()), + $this->elapsedMs($startTime), + ); + } + } + + private function elapsedMs(float $startTime): float + { + return (microtime(true) - $startTime) * 1000; + } +} diff --git a/src/Health/HealthProbeInterface.php b/src/Health/HealthProbeInterface.php new file mode 100644 index 0000000..b6eac39 --- /dev/null +++ b/src/Health/HealthProbeInterface.php @@ -0,0 +1,41 @@ +schemaRegistry->getProvider($type)) !== null) { - $schemas[] = $this->load($provider->getSchemaPath()); + $schemas[] = JsonSchemaFile::load($provider->getSchemaPath()); } elseif ($type !== '' && ($extension = $this->extensionRegistry->get($type)) !== null) { - $schemas[] = $this->load($extension->getSchemaPath()); + $schemas[] = JsonSchemaFile::load($extension->getSchemaPath()); } else { $all = array_map( - fn ($provider): array => $this->load($provider->getSchemaPath()), + static fn ($provider): array => JsonSchemaFile::load($provider->getSchemaPath()), array_values($this->schemaRegistry->getProviders()), ); $matching = array_filter($all, static fn (array $schema): bool => self::declaresType($schema, $type)); @@ -92,21 +93,6 @@ private function resolve(string $type): array return array_values($paths); } - private function load(string $schemaPath): array - { - $content = is_file($schemaPath) ? file_get_contents($schemaPath) : false; - if ($content === false) { - throw new \RuntimeException(sprintf('Cannot read API configuration schema "%s"', $schemaPath)); - } - - $schema = json_decode($content, true); - if (!is_array($schema)) { - throw new \RuntimeException(sprintf('Invalid JSON in API configuration schema "%s"', $schemaPath)); - } - - return $schema; - } - private static function declaresType(array $schema, string $type): bool { $typeSchema = $schema['properties']['type'] ?? null; diff --git a/src/State/ApiConfigurationHealthProvider.php b/src/State/ApiConfigurationHealthProvider.php index 198c3ca..c8b5541 100755 --- a/src/State/ApiConfigurationHealthProvider.php +++ b/src/State/ApiConfigurationHealthProvider.php @@ -1,5 +1,5 @@ clientFactory->createFromEntity($apiConfiguration); - - // Get health info from client - $rawHealthInfo = $client->getHealthInfo(); - - // Calculate response time - $responseTime = (microtime(true) - $startTime) * 1000; // Convert to milliseconds - - // Normalize health info — endpoint comes from the client itself - $normalizedHealth = $this->healthNormalizer->normalize( - $rawHealthInfo, - $client->getEndpoint(), - $responseTime - ); - - // Return as stdClass for proper API Platform serialization - $result = new \stdClass(); - $result->status = $normalizedHealth['status']; - $result->authenticated = $normalizedHealth['authenticated']; - $result->endpoint = $normalizedHealth['endpoint']; - $result->responseTime = $normalizedHealth['responseTime']; - - if (isset($normalizedHealth['message'])) { - $result->message = $normalizedHealth['message']; - } - if (isset($normalizedHealth['metadata'])) { - $result->metadata = $normalizedHealth['metadata']; - } - if (isset($normalizedHealth['error'])) { - $result->error = $normalizedHealth['error']; - } - - return $result; - } catch (\Exception $e) { - $responseTime = (microtime(true) - $startTime) * 1000; - - // If client failed to instantiate, fall back to a URN identifying the - // ApiConfiguration — still a valid URI per RFC 8141 / `format: uri`. - $endpoint = $client?->getEndpoint() - ?? sprintf('urn:za7:api-configuration:%s', $apiConfiguration->getId()); - - $normalizedHealth = $this->healthNormalizer->normalize( - [ - 'status' => 'error', - 'authenticated' => false, - 'message' => 'Health check failed: ' . $e->getMessage(), - 'error' => [ - 'code' => 'HEALTH_CHECK_ERROR', - 'message' => $e->getMessage(), - ], - ], - $endpoint, - $responseTime - ); - - // Return as stdClass for proper API Platform serialization - $result = new \stdClass(); - $result->status = $normalizedHealth['status']; - $result->authenticated = $normalizedHealth['authenticated']; - $result->endpoint = $normalizedHealth['endpoint']; - $result->responseTime = $normalizedHealth['responseTime']; - - if (isset($normalizedHealth['message'])) { - $result->message = $normalizedHealth['message']; - } - if (isset($normalizedHealth['metadata'])) { - $result->metadata = $normalizedHealth['metadata']; - } - if (isset($normalizedHealth['error'])) { - $result->error = $normalizedHealth['error']; - } - - return $result; - } + // stdClass: API Platform serializes it as the operation's output. + // Nested arrays keep their keys thanks to `api_sub_level` on the + // operation (see ApiConfiguration). + return (object) $this->healthChecker->check($apiConfiguration); } } diff --git a/tests/ApiClient/ApiClientFactoryTest.php b/tests/ApiClient/ApiClientFactoryTest.php new file mode 100644 index 0000000..4c0118d --- /dev/null +++ b/tests/ApiClient/ApiClientFactoryTest.php @@ -0,0 +1,74 @@ +extension = new DemoExtension(); + $this->factory = new ApiClientFactory( + new ApiExtensionRegistry([$this->extension]), + SecretsFixture::configSecrets(), + ); + } + + public function testClientGetsDecryptedConfigWithInternalId(): void + { + $secrets = SecretsFixture::configSecrets(); + $entity = (new ApiConfiguration()) + ->setName('demo') + ->setConfigJson($secrets->encrypt(SecretsFixture::config())); + + $this->factory->createFromEntity($entity); + + self::assertSame('s3cr3t-password', $this->extension->lastConfig['password']); + self::assertSame((string) $entity->getId(), $this->extension->lastConfig['_apiConfigurationId']); + } + + public function testRejectsConfigViolatingTheExtensionSchemaNamingTheField(): void + { + $config = SecretsFixture::config(); + unset($config['base_url']); + + try { + $this->factory->create('demo', $config); + self::fail('InvalidConfigurationException expected'); + } catch (InvalidConfigurationException $e) { + self::assertStringContainsString('base_url', $e->getMessage()); + self::assertNotEmpty($e->getErrors()); + self::assertNull($this->extension->lastConfig, 'client must not be built'); + } + } + + public function testValidatesTheDecryptedValue(): void + { + // client_secret has minLength 4: the clear value is checked, not the ciphertext + $secrets = SecretsFixture::configSecrets(); + $config = $secrets->encrypt(SecretsFixture::config(['oauth' => ['client_secret' => 'abc']])); + + $this->expectException(InvalidConfigurationException::class); + $this->factory->create('demo', $config); + } + + public function testUnknownApiNameStillThrowsInvalidArgument(): void + { + $this->expectException(\InvalidArgumentException::class); + $this->expectExceptionMessage('Unsupported API name'); + $this->factory->create('nope', []); + } +} diff --git a/tests/Entity/ApiConfigurationHealthOperationTest.php b/tests/Entity/ApiConfigurationHealthOperationTest.php new file mode 100644 index 0000000..a2340f9 --- /dev/null +++ b/tests/Entity/ApiConfigurationHealthOperationTest.php @@ -0,0 +1,35 @@ +getAttributes(ApiResource::class)[0] + ->newInstance(); + + foreach ($resource->getOperations() ?? [] as $operation) { + if ($operation->getName() === 'api_configuration_health') { + self::assertTrue($operation->getNormalizationContext()['api_sub_level'] ?? false); + + return; + } + } + + self::fail('health operation not found'); + } +} diff --git a/tests/Fixtures/DemoClient.php b/tests/Fixtures/DemoClient.php new file mode 100644 index 0000000..186998e --- /dev/null +++ b/tests/Fixtures/DemoClient.php @@ -0,0 +1,58 @@ + 'ok', 'authenticated' => true, 'metadata' => ['apiName' => 'demo']]; + } + + public function getEndpoint(): string + { + return $this->config['base_url']; + } + + public function getProjects(): array + { + return []; + } + + public function getTodos(string $projectId): array + { + return []; + } + + public function getTodo(string $projectId, string $todoId): ?array + { + return null; + } +} diff --git a/tests/Fixtures/DemoExtension.php b/tests/Fixtures/DemoExtension.php new file mode 100644 index 0000000..7de170d --- /dev/null +++ b/tests/Fixtures/DemoExtension.php @@ -0,0 +1,40 @@ +lastConfig = $config; + + return new DemoClient($config); + } +} diff --git a/tests/Health/ApiConfigurationHealthCheckerTest.php b/tests/Health/ApiConfigurationHealthCheckerTest.php new file mode 100644 index 0000000..76e0baf --- /dev/null +++ b/tests/Health/ApiConfigurationHealthCheckerTest.php @@ -0,0 +1,91 @@ +checker([new DemoExtension()])->check($this->entity(SecretsFixture::config())); + + self::assertSame('ok', $result['status']); + self::assertSame('https://api.example.com', $result['endpoint']); + self::assertSame('demo', $result['metadata']['apiName']); + } + + public function testSchemaOnlyTypeIsCheckedByItsProbeWithDecryptedConfig(): void + { + $probe = new class implements HealthProbeInterface { + public ?array $config = null; + + public function getName(): string + { + return 'mcp'; + } + + public function getEndpoint(array $config): string + { + return $config['url']; + } + + public function probe(array $config): array + { + $this->config = $config; + + return ['status' => 'ok', 'authenticated' => true, 'metadata' => ['custom' => ['toolCount' => 3]]]; + } + }; + + $secrets = SecretsFixture::configSecrets(); + // No extension for "mcp": without the probe this was "Unsupported API name" + $entity = $this->entity(['type' => 'mcp', 'endpoint_type' => 'rest', 'url' => 'https://mcp.example.com', 'token' => $secrets->encrypt(['type' => 'demo', 'token' => 't0ken'])['token']]); + + $result = $this->checker([], [$probe])->check($entity); + + self::assertSame('ok', $result['status']); + self::assertSame('https://mcp.example.com', $result['endpoint']); + self::assertSame(3, $result['metadata']['custom']['toolCount']); + self::assertSame('t0ken', $probe->config['token']); + } + + public function testFailureBecomesAnErrorResult(): void + { + $entity = $this->entity(['type' => 'unknown', 'endpoint_type' => 'rest']); + + $result = $this->checker([])->check($entity); + + self::assertSame('error', $result['status']); + self::assertSame('HEALTH_CHECK_ERROR', $result['error']['code']); + self::assertStringStartsWith('urn:za7:api-configuration:', $result['endpoint']); + } + + private function checker(array $extensions, array $probes = []): ApiConfigurationHealthChecker + { + $secrets = SecretsFixture::configSecrets(); + + return new ApiConfigurationHealthChecker( + new ApiClientFactory(new ApiExtensionRegistry($extensions), $secrets), + new HealthNormalizer(), + $secrets, + $probes, + ); + } + + private function entity(array $config): ApiConfiguration + { + return (new ApiConfiguration())->setName('test')->setConfigJson($config); + } +} From 6f031932350c709d9862e33dd6d4a2ce5cc8ae78 Mon Sep 17 00:00:00 2001 From: "t.munk (workhorse)" Date: Mon, 5 Oct 2026 12:50:52 +0000 Subject: [PATCH 5/7] fix(command): delegate app:api:validate-file to the file client [*] The command read ApiConfiguration::getFileConfig(), which no longer exists since file sources are configured by directory (Jira XML), and failed with a PHP error. Build the client instead and use FileApiClientInterface::validateFile()/parseFile(), so the format checks live with the type, not in the command. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 1 + src/Command/ValidateApiFileCommand.php | 122 +++++++++---------------- 2 files changed, 44 insertions(+), 79 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e9a423b..2753e8d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,7 @@ Secure storage and transport of the credentials in `configJson` ([Issue #8](http ### Fixed - `GET /api/admin/api_configurations/{id}/health` in JSON-LD returned `metadata` and `error` as `hydra:Collection` without their keys (Issue #9); the operation now normalizes nested arrays raw (`api_sub_level`). +- `app:api:validate-file` called the non-existent `ApiConfiguration::getFileConfig()` (a leftover from file configurations with a `format` key) and failed with a PHP error; it now builds the client and delegates to `FileApiClientInterface::validateFile()` and `parseFile()`, so every file type validates its own format. - `app:api:test-connection` called the non-existent `ApiConfiguration::getCredentials()` and failed with a PHP error; it now builds the client from the entity. ## 0.4.0 and earlier diff --git a/src/Command/ValidateApiFileCommand.php b/src/Command/ValidateApiFileCommand.php index b177226..d5f0efe 100644 --- a/src/Command/ValidateApiFileCommand.php +++ b/src/Command/ValidateApiFileCommand.php @@ -1,9 +1,12 @@ getEndpointType() !== 'file') { - $io->warning('This configuration is not a file-based API. Use app:api:test-connection for REST APIs.'); - return Command::INVALID; - } - - // Check if file exists - if (!file_exists($filePath)) { - $io->error(sprintf('File not found: %s', $filePath)); + try { + $client = $this->clientFactory->createFromEntity($config); + } catch (\Exception $e) { + $io->error(sprintf('Cannot create the client: %s', $e->getMessage())); return Command::FAILURE; } - if (!is_readable($filePath)) { - $io->error(sprintf('File is not readable: %s', $filePath)); - return Command::FAILURE; + if (!$client instanceof FileApiClientInterface) { + $io->warning('This configuration is not a file-based API. Use app:api:test-connection for REST APIs.'); + return Command::INVALID; } - $fileConfig = $config->getFileConfig(); - if ($fileConfig === null || $fileConfig === []) { - $io->error('No file configuration found for this API.'); + if (!$client->validateFile($filePath)) { + $io->error(sprintf( + 'File is missing, not readable or not a supported format (%s): %s', + implode(', ', $client->getSupportedFormats()), + $filePath, + )); return Command::FAILURE; } - $io->section('File Information'); - $fileSize = filesize($filePath); - $io->table( - ['Property', 'Value'], - [ - ['Size', sprintf('%s KB', number_format($fileSize / 1024, 2))], - ['Type', mime_content_type($filePath)], - ['Expected Format', $fileConfig['format'] ?? 'N/A'], - ] - ); - - // Basic validation based on type - $expectedFormat = $fileConfig['format'] ?? 'xml'; - try { - if ($expectedFormat === 'xml') { - $io->section('Validating XML...'); - - libxml_use_internal_errors(true); - $xml = simplexml_load_file($filePath); - - if ($xml === false) { - $errors = libxml_get_errors(); - $io->error('XML validation failed:'); - foreach ($errors as $error) { - $io->writeln(sprintf('Line %d: %s', $error->line, trim($error->message))); - } - libxml_clear_errors(); - return Command::FAILURE; - } - - $io->success('XML is valid!'); - - // Show basic stats - $io->section('XML Statistics'); - $io->writeln(sprintf('Root element: %s', $xml->getName())); - $io->writeln(sprintf('Child elements: %d', count($xml->children()))); - - } elseif ($expectedFormat === 'json') { - $io->section('Validating JSON...'); - - $content = file_get_contents($filePath); - $data = json_decode($content, true); - - if (json_last_error() !== JSON_ERROR_NONE) { - $io->error(sprintf('JSON validation failed: %s', json_last_error_msg())); - return Command::FAILURE; - } - - $io->success('JSON is valid!'); - - $io->section('JSON Statistics'); - $io->writeln(sprintf('Type: %s', gettype($data))); - if (is_array($data)) { - $io->writeln(sprintf('Elements: %d', count($data))); - } - } else { - $io->warning(sprintf('Unknown format: %s. Skipping format validation.', $expectedFormat)); - } - - $io->success(sprintf('File validation completed successfully for: %s', basename($filePath))); - return Command::SUCCESS; - + $parsed = $client->parseFile($filePath); } catch (\Exception $e) { - $io->error(sprintf('Error during validation: %s', $e->getMessage())); + $io->error(sprintf('Parsing failed: %s', $e->getMessage())); if ($output->isVerbose()) { $io->writeln($e->getTraceAsString()); } return Command::FAILURE; } + + $io->section('Parse Result'); + $rows = [['Size', sprintf('%s KB', number_format((filesize($filePath) ?: 0) / 1024, 2))]]; + foreach ($parsed['metadata'] ?? [] as $key => $value) { + $rows[] = [$key, is_scalar($value) ? (string) $value : json_encode($value)]; + } + if (isset($parsed['items']) && is_array($parsed['items'])) { + $rows[] = ['Items', count($parsed['items'])]; + } + $io->table(['Property', 'Value'], $rows); + + $io->success(sprintf('File validation completed successfully for: %s', basename($filePath))); + return Command::SUCCESS; } } From 4205ee5d2cdcd0b818a5171b07ed939dd26072a6 Mon Sep 17 00:00:00 2001 From: "t.munk (workhorse)" Date: Mon, 5 Oct 2026 12:55:37 +0000 Subject: [PATCH 6/7] fix(validator): report missing config schemas as violation instead of 500 [*] Closes #5. With no schema provider registered, SchemaRegistry returns `anyOf: []`, which opis rejects while parsing the schema; the uncaught exception became HTTP 500 on every write. Add a violation instead (HTTP 422). Also declare symfony/validator, used directly but only installed transitively. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 2 + composer.json | 3 +- src/Validator/ApiConfigurationValidator.php | 12 +++- .../ApiConfigurationValidatorTest.php | 64 +++++++++++++++++++ 4 files changed, 79 insertions(+), 2 deletions(-) create mode 100644 tests/Validator/ApiConfigurationValidatorTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 2753e8d..f1a2351 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,8 @@ Secure storage and transport of the credentials in `configJson` ([Issue #8](http ### Fixed - `GET /api/admin/api_configurations/{id}/health` in JSON-LD returned `metadata` and `error` as `hydra:Collection` without their keys (Issue #9); the operation now normalizes nested arrays raw (`api_sub_level`). +- Write operations returned HTTP 500 (`anyOf must have at least one element`) when no configuration schema was registered ([Issue #5](https://github.com/dmstr/api-configuration-bundle/issues/5)); the validator now reports a violation (HTTP 422). The root cause, the empty `anyOf` from `SchemaRegistry`, is tracked in dmstr/openapi-json-schema-bundle#3. +- `symfony/validator` is declared as a dependency; it was used directly but only installed transitively. - `app:api:validate-file` called the non-existent `ApiConfiguration::getFileConfig()` (a leftover from file configurations with a `format` key) and failed with a PHP error; it now builds the client and delegates to `FileApiClientInterface::validateFile()` and `parseFile()`, so every file type validates its own format. - `app:api:test-connection` called the non-existent `ApiConfiguration::getCredentials()` and failed with a PHP error; it now builds the client from the entity. diff --git a/composer.json b/composer.json index 457d7f4..d3cbbdd 100644 --- a/composer.json +++ b/composer.json @@ -17,7 +17,8 @@ "doctrine/doctrine-bundle": "^2.8", "dmstr/api-platform-utils-bundle": "*", "dmstr/openapi-json-schema-bundle": "*", - "opis/json-schema": "^2.3" + "opis/json-schema": "^2.3", + "symfony/validator": "^7.0|^8.0" }, "require-dev": { "phpunit/phpunit": "^12.0", diff --git a/src/Validator/ApiConfigurationValidator.php b/src/Validator/ApiConfigurationValidator.php index b502734..1a92a09 100644 --- a/src/Validator/ApiConfigurationValidator.php +++ b/src/Validator/ApiConfigurationValidator.php @@ -1,5 +1,5 @@ schemaRegistry->getUnifiedSchema(); + // Without any registered type the registry returns `anyOf: []`, which + // is invalid JSON Schema and makes opis throw (HTTP 500, Issue #5) + if (($unifiedSchema['anyOf'] ?? null) === []) { + $this->logger->warning('API configuration rejected: no configuration schema is registered'); + $this->context->buildViolation($constraint->message) + ->setParameter('{{ error }}', 'No API configuration type is registered; install or enable an extension that provides a configuration schema') + ->addViolation(); + return; + } + // Secrets kept from the stored configuration are encrypted; validate // their clear values. Logs only ever see the masked configuration. $loggable = $this->secrets->mask($value); diff --git a/tests/Validator/ApiConfigurationValidatorTest.php b/tests/Validator/ApiConfigurationValidatorTest.php new file mode 100644 index 0000000..3056e7e --- /dev/null +++ b/tests/Validator/ApiConfigurationValidatorTest.php @@ -0,0 +1,64 @@ + + */ +final class ApiConfigurationValidatorTest extends ConstraintValidatorTestCase +{ + private ?SchemaRegistry $registry = null; + + protected function createValidator(): ApiConfigurationValidator + { + return new ApiConfigurationValidator( + $this->registry ?? SecretsFixture::schemaRegistry(), + new NullLogger(), + SecretsFixture::configSecrets(), + ); + } + + public function testNoRegisteredSchemaIsAViolationNotAnError(): void + { + // Issue #5: an empty registry yields `anyOf: []`, which opis rejects + $this->registry = new SchemaRegistry(new ArrayAdapter(), new NullLogger()); + $this->validator = $this->createValidator(); + $this->validator->initialize($this->context); + + $this->validator->validate(['type' => 't', 'endpoint_type' => 'rest'], new ApiConfigurationConstraint()); + + $this->buildViolation('Invalid API configuration: {{ error }}') + ->setParameter('{{ error }}', 'No API configuration type is registered; install or enable an extension that provides a configuration schema') + ->assertRaised(); + } + + public function testValidConfigurationPasses(): void + { + $this->validator->validate(SecretsFixture::config(), new ApiConfigurationConstraint()); + + $this->assertNoViolation(); + } + + public function testKeptEncryptedSecretIsValidatedInClear(): void + { + // client_secret has minLength 4; the ciphertext alone would pass + $config = SecretsFixture::configSecrets()->encrypt( + SecretsFixture::config(['oauth' => ['client_secret' => 'abc']]), + ); + + $this->validator->validate($config, new ApiConfigurationConstraint()); + + self::assertCount(1, $this->context->getViolations()); + } +} From 7228ff65455301866acebb1ed01b42a55e884ccc Mon Sep 17 00:00:00 2001 From: "t.munk (workhorse)" Date: Mon, 5 Oct 2026 12:56:31 +0000 Subject: [PATCH 7/7] test(validator): check the clear secret without a local $ref [*] The unified anyOf schema of SchemaRegistry cannot resolve a type schema's local #/definitions references. Co-Authored-By: Claude Opus 5.5 --- tests/Fixtures/demo.schema.json | 2 +- tests/Validator/ApiConfigurationValidatorTest.php | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/tests/Fixtures/demo.schema.json b/tests/Fixtures/demo.schema.json index 3cfe8a9..6f390ee 100644 --- a/tests/Fixtures/demo.schema.json +++ b/tests/Fixtures/demo.schema.json @@ -9,7 +9,7 @@ "auth_type": { "enum": ["basic", "bearer"] }, "username": { "type": "string" }, "password": { "type": "string", "writeOnly": true }, - "token": { "type": "string", "writeOnly": true }, + "token": { "type": "string", "minLength": 4, "writeOnly": true }, "oauth": { "$ref": "#/definitions/oauth" }, "headers": { "type": "array", diff --git a/tests/Validator/ApiConfigurationValidatorTest.php b/tests/Validator/ApiConfigurationValidatorTest.php index 3056e7e..f41ed92 100644 --- a/tests/Validator/ApiConfigurationValidatorTest.php +++ b/tests/Validator/ApiConfigurationValidatorTest.php @@ -52,9 +52,11 @@ public function testValidConfigurationPasses(): void public function testKeptEncryptedSecretIsValidatedInClear(): void { - // client_secret has minLength 4; the ciphertext alone would pass + // token has minLength 4; the ciphertext alone would pass. Not via the + // $ref'd oauth.client_secret: the unified anyOf schema cannot resolve a + // type schema's local `#/definitions/...` (SchemaRegistry limitation) $config = SecretsFixture::configSecrets()->encrypt( - SecretsFixture::config(['oauth' => ['client_secret' => 'abc']]), + SecretsFixture::config(['auth_type' => 'bearer', 'token' => 'abc']), ); $this->validator->validate($config, new ApiConfigurationConstraint());