From 8ec3ec8b96f74c18a6d3f7db331dc6c5d9e93b06 Mon Sep 17 00:00:00 2001 From: "t.munk (workhorse)" Date: Wed, 28 Oct 2026 07:50:32 +0000 Subject: [PATCH 1/5] test: add HTTP test kernel with header authentication and permission voter [*] Boot the bundle in a minimal Symfony kernel (SQLite in-memory, stateless firewall) so tests send real requests through API Platform: - TestHeaderAuthenticator reads X-Test-Roles, X-Test-User and X-Test-Auth: anonymous; anonymous requests get 401 - TestPermissionVoter answers apiconfig:* from an example role map - a kernel variant without the voter for deny-by-default tests - AuthorizableDemoExtension for the 303 browser path of authorize - smoke tests pinning the current behaviour No behaviour change; require-dev only. Co-Authored-By: Claude Opus 5.5 --- composer.json | 8 +- tests/Fixtures/AuthorizableDemoExtension.php | 80 ++++++++ tests/Fixtures/DemoSchemaProvider.php | 26 +++ tests/Fixtures/authorizable-demo.schema.json | 12 ++ tests/Http/ApiTestCase.php | 181 +++++++++++++++++ tests/Http/CurrentBehaviourSmokeTest.php | 91 +++++++++ tests/Http/NoVoterKernelSmokeTest.php | 30 +++ tests/Kernel/TestHeaderAuthenticator.php | 87 ++++++++ tests/Kernel/TestKernel.php | 203 +++++++++++++++++++ tests/Kernel/TestPermissionVoter.php | 70 +++++++ 10 files changed, 787 insertions(+), 1 deletion(-) create mode 100644 tests/Fixtures/AuthorizableDemoExtension.php create mode 100644 tests/Fixtures/DemoSchemaProvider.php create mode 100644 tests/Fixtures/authorizable-demo.schema.json create mode 100644 tests/Http/ApiTestCase.php create mode 100644 tests/Http/CurrentBehaviourSmokeTest.php create mode 100644 tests/Http/NoVoterKernelSmokeTest.php create mode 100644 tests/Kernel/TestHeaderAuthenticator.php create mode 100644 tests/Kernel/TestKernel.php create mode 100644 tests/Kernel/TestPermissionVoter.php diff --git a/composer.json b/composer.json index d3cbbdd..32c1e5a 100644 --- a/composer.json +++ b/composer.json @@ -24,7 +24,13 @@ "phpunit/phpunit": "^12.0", "doctrine/migrations": "^3.9", "doctrine/dbal": "^3.7", - "ext-pdo_sqlite": "*" + "ext-pdo_sqlite": "*", + "doctrine/doctrine-migrations-bundle": "^3.3", + "symfony/browser-kit": "^7.0|^8.0", + "symfony/expression-language": "^7.0|^8.0", + "symfony/http-client": "^7.0|^8.0", + "symfony/security-bundle": "^7.0|^8.0", + "symfony/yaml": "^7.0|^8.0" }, "autoload": { "psr-4": { diff --git a/tests/Fixtures/AuthorizableDemoExtension.php b/tests/Fixtures/AuthorizableDemoExtension.php new file mode 100644 index 0000000..d7db666 --- /dev/null +++ b/tests/Fixtures/AuthorizableDemoExtension.php @@ -0,0 +1,80 @@ + 'code', + 'client_id' => (string) ($config->getConfigJson()['client_id'] ?? ''), + 'redirect_uri' => $redirectUri, + 'state' => $state, + ]); + } + + public function exchangeAuthorizationCode(ApiConfiguration $config, string $code, string $redirectUri): array + { + return ['expiresAt' => null, 'accountId' => null, 'tokenType' => 'Bearer']; + } + + /** + * A valid `configJson` of this type. + * + * @param array $overrides + * @return array + */ + public static function config(array $overrides = []): array + { + return array_replace([ + 'type' => self::TYPE, + 'endpoint_type' => 'rest', + 'base_url' => 'https://api.example.com', + 'client_id' => 'demo-client', + 'client_secret' => 'demo-client-secret', + ], $overrides); + } +} diff --git a/tests/Fixtures/DemoSchemaProvider.php b/tests/Fixtures/DemoSchemaProvider.php new file mode 100644 index 0000000..9623532 --- /dev/null +++ b/tests/Fixtures/DemoSchemaProvider.php @@ -0,0 +1,26 @@ +client = static::createClient(['environment' => static::kernelEnvironment()]); + // A reboot would drop the in-memory database between requests + $this->client->disableReboot(); + + $entityManager = $this->entityManager(); + (new SchemaTool($entityManager))->createSchema($entityManager->getMetadataFactory()->getAllMetadata()); + } + + /** + * Sends a request as a user with the given roles. + * + * @param list $roles + * @param array $headers extra headers, e.g. ['Accept' => 'text/html'] + */ + protected function requestAs( + string $method, + string $uri, + array $roles, + ?array $json = null, + array $headers = [], + string $user = TestHeaderAuthenticator::DEFAULT_USER, + ): Response { + return $this->request($method, $uri, $json, [ + TestHeaderAuthenticator::HEADER_ROLES => implode(',', $roles), + TestHeaderAuthenticator::HEADER_USER => $user, + ...$headers, + ]); + } + + /** + * Sends a request without authentication. + * + * @param array $headers + */ + protected function requestAnonymous(string $method, string $uri, ?array $json = null, array $headers = []): Response + { + return $this->request($method, $uri, $json, [ + TestHeaderAuthenticator::HEADER_AUTH => TestHeaderAuthenticator::ANONYMOUS, + ...$headers, + ]); + } + + /** + * Sends a request with exactly the given headers. Defaults: JSON-LD + * `Accept`, and a body content type matching the method. + * + * @param array $headers + */ + protected function request(string $method, string $uri, ?array $json = null, array $headers = []): Response + { + $headers += ['Accept' => 'application/ld+json']; + if ($json !== null) { + $headers += ['Content-Type' => strtoupper($method) === 'PATCH' + ? 'application/merge-patch+json' + : 'application/ld+json']; + } + + $server = []; + foreach ($headers as $name => $value) { + $key = strtoupper(str_replace('-', '_', $name)); + $server[\in_array($key, ['CONTENT_TYPE', 'CONTENT_LENGTH'], true) ? $key : 'HTTP_' . $key] = $value; + } + + $this->client->request( + $method, + $uri, + server: $server, + content: $json === null ? null : json_encode($json, JSON_THROW_ON_ERROR), + ); + + return $this->client->getResponse(); + } + + /** + * Stores an ApiConfiguration of the `demo` type (secrets encrypted by + * the bundle's Doctrine listener) and detaches it from the entity manager. + * + * @param array $configOverrides merged into {@see SecretsFixture::config()} + */ + protected function createApiConfiguration( + array $configOverrides = [], + string $name = 'Demo configuration', + bool $active = true, + ): ApiConfiguration { + return $this->persistApiConfiguration(SecretsFixture::config($configOverrides), $name, $active); + } + + /** + * Stores an ApiConfiguration of the `authorizable-demo` type, whose + * extension supports the authorize operation. + * + * @param array $configOverrides merged into {@see AuthorizableDemoExtension::config()} + */ + protected function createAuthorizableApiConfiguration( + array $configOverrides = [], + string $name = 'Authorizable configuration', + ): ApiConfiguration { + return $this->persistApiConfiguration(AuthorizableDemoExtension::config($configOverrides), $name, true); + } + + /** + * @param array $configJson + */ + protected function persistApiConfiguration(array $configJson, string $name, bool $active): ApiConfiguration + { + $configuration = (new ApiConfiguration()) + ->setName($name) + ->setConfigJson($configJson) + ->setActive($active); + + $entityManager = $this->entityManager(); + $entityManager->persist($configuration); + $entityManager->flush(); + $entityManager->clear(); + + return $configuration; + } + + protected function itemUri(ApiConfiguration $configuration, string $suffix = ''): string + { + return self::COLLECTION . '/' . $configuration->getId()->toRfc4122() . $suffix; + } + + /** + * @return array + */ + protected static function json(Response $response): array + { + return json_decode((string) $response->getContent(), true, flags: JSON_THROW_ON_ERROR); + } + + protected function entityManager(): EntityManagerInterface + { + return static::getContainer()->get('doctrine.orm.entity_manager'); + } +} diff --git a/tests/Http/CurrentBehaviourSmokeTest.php b/tests/Http/CurrentBehaviourSmokeTest.php new file mode 100644 index 0000000..3d1499b --- /dev/null +++ b/tests/Http/CurrentBehaviourSmokeTest.php @@ -0,0 +1,91 @@ +createApiConfiguration(); + + $response = $this->requestAs('GET', self::COLLECTION, [self::CURRENT_REQUIRED_ROLE]); + + self::assertSame(200, $response->getStatusCode(), (string) $response->getContent()); + self::assertSame(1, self::json($response)['totalItems']); + } + + public function testCollectionIsForbiddenForAnotherRole(): void + { + $response = $this->requestAs('GET', self::COLLECTION, ['reader']); + + self::assertSame(403, $response->getStatusCode()); + } + + public function testCollectionRequiresAuthentication(): void + { + self::assertSame(401, $this->requestAnonymous('GET', self::COLLECTION)->getStatusCode()); + // No test header at all is anonymous too + self::assertSame(401, $this->request('GET', self::COLLECTION)->getStatusCode()); + } + + public function testItemMasksSecrets(): void + { + $configuration = $this->createApiConfiguration(); + + $response = $this->requestAs('GET', $this->itemUri($configuration), [self::CURRENT_REQUIRED_ROLE]); + + self::assertSame(200, $response->getStatusCode(), (string) $response->getContent()); + $config = self::json($response)['configJson']; + self::assertSame(ConfigSecrets::MASK, $config['password']); + self::assertSame('alice', $config['username']); + } + + public function testAuthorizeRedirectsBrowsersAndAnswersJsonClients(): void + { + $configuration = $this->createAuthorizableApiConfiguration(); + $uri = $this->itemUri($configuration, '/authorize'); + + $browser = $this->requestAs('GET', $uri, [self::CURRENT_REQUIRED_ROLE], headers: ['Accept' => 'text/html']); + self::assertSame(303, $browser->getStatusCode()); + self::assertStringStartsWith(AuthorizableDemoExtension::AUTHORIZE_ENDPOINT . '?', (string) $browser->headers->get('Location')); + + $json = $this->requestAs('GET', $uri, [self::CURRENT_REQUIRED_ROLE]); + self::assertSame(200, $json->getStatusCode(), (string) $json->getContent()); + $body = self::json($json); + self::assertStringStartsWith(AuthorizableDemoExtension::AUTHORIZE_ENDPOINT . '?', $body['authorizeUrl']); + self::assertSame(TestKernel::OAUTH_REDIRECT_URI, $body['redirectUri']); + self::assertSame($configuration->getId()->toRfc4122(), $body['state']); + } + + public function testAuthorizeConflictsForATypeWithoutAuthorizationFlow(): void + { + $configuration = $this->createApiConfiguration(); + + $response = $this->requestAs('GET', $this->itemUri($configuration, '/authorize'), [self::CURRENT_REQUIRED_ROLE]); + + self::assertSame(409, $response->getStatusCode()); + } + + public function testPermissionVoterIsRegistered(): void + { + self::assertTrue(static::getContainer()->has(TestPermissionVoter::class)); + } +} diff --git a/tests/Http/NoVoterKernelSmokeTest.php b/tests/Http/NoVoterKernelSmokeTest.php new file mode 100644 index 0000000..43dd3d4 --- /dev/null +++ b/tests/Http/NoVoterKernelSmokeTest.php @@ -0,0 +1,30 @@ +has(TestPermissionVoter::class)); + } + + public function testCollectionStillRequiresAuthentication(): void + { + self::assertSame(401, $this->requestAnonymous('GET', self::COLLECTION)->getStatusCode()); + } +} diff --git a/tests/Kernel/TestHeaderAuthenticator.php b/tests/Kernel/TestHeaderAuthenticator.php new file mode 100644 index 0000000..7d879b9 --- /dev/null +++ b/tests/Kernel/TestHeaderAuthenticator.php @@ -0,0 +1,87 @@ +headers->get(self::HEADER_AUTH, ''))) === self::ANONYMOUS) { + return false; + } + + return $request->headers->has(self::HEADER_ROLES) || $request->headers->has(self::HEADER_USER); + } + + public function authenticate(Request $request): Passport + { + $identifier = trim((string) $request->headers->get(self::HEADER_USER, '')); + if ($identifier === '') { + $identifier = self::DEFAULT_USER; + } + $roles = self::parseRoles((string) $request->headers->get(self::HEADER_ROLES, '')); + + return new SelfValidatingPassport(new UserBadge( + $identifier, + static fn (string $identifier): InMemoryUser => new InMemoryUser($identifier, null, $roles), + )); + } + + public function onAuthenticationSuccess(Request $request, TokenInterface $token, string $firewallName): ?Response + { + return null; + } + + public function onAuthenticationFailure(Request $request, AuthenticationException $exception): ?Response + { + return new JsonResponse(['error' => $exception->getMessageKey()], Response::HTTP_UNAUTHORIZED); + } + + public function start(Request $request, ?AuthenticationException $authException = null): Response + { + return new JsonResponse(['error' => 'Authentication required'], Response::HTTP_UNAUTHORIZED); + } + + /** + * @return list + */ + public static function parseRoles(string $header): array + { + return array_values(array_unique(array_filter( + array_map('trim', explode(',', $header)), + static fn (string $role): bool => $role !== '', + ))); + } +} diff --git a/tests/Kernel/TestKernel.php b/tests/Kernel/TestKernel.php new file mode 100644 index 0000000..efadf16 --- /dev/null +++ b/tests/Kernel/TestKernel.php @@ -0,0 +1,203 @@ +tmpDir() . '/cache'; + } + + public function getLogDir(): string + { + return $this->tmpDir() . '/log'; + } + + /** + * Not the repository's `config/`: in debug mode FrameworkBundle 7.4+ + * dumps `reference.php` into an existing config directory. This one + * does not exist, so nothing is written; the kernel reads no files from + * it ({@see self::configureContainer()} configures everything in code). + */ + private function getConfigDir(): string + { + return $this->tmpDir() . '/config'; + } + + public function hasPermissionVoter(): bool + { + return $this->environment !== self::ENV_NO_VOTER; + } + + private function tmpDir(): string + { + return sys_get_temp_dir() . '/dmstr-api-configuration-bundle-tests/' . $this->environment; + } + + private function configureContainer(ContainerConfigurator $container): void + { + $projectDir = $this->getProjectDir(); + + $container->parameters() + // Autowired by the authorize provider, the redirect listener and the OAuth callback + ->set('app.oauth.redirect_uri', self::OAUTH_REDIRECT_URI); + + $container->extension('framework', [ + 'test' => true, + 'secret' => 'test-secret', + 'http_method_override' => false, + 'handle_all_throwables' => true, + 'php_errors' => ['log' => true], + // No filesystem cache: the unified schema must follow the fixtures + 'cache' => ['app' => 'cache.adapter.array'], + ]); + + $container->extension('security', [ + 'providers' => [ + 'test_users' => ['memory' => ['users' => []]], + ], + 'firewalls' => [ + 'api' => [ + 'pattern' => '^' . self::API_PREFIX, + 'stateless' => true, + 'provider' => 'test_users', + 'custom_authenticators' => [TestHeaderAuthenticator::class], + ], + ], + ]); + + $container->extension('doctrine', [ + 'dbal' => [ + 'url' => 'sqlite:///:memory:', + ], + 'orm' => [ + // PHP 8.4 native lazy objects, no symfony/var-exporter needed + 'enable_native_lazy_objects' => true, + 'naming_strategy' => 'doctrine.orm.naming_strategy.underscore_number_aware', + 'mappings' => [ + 'ApiConfiguration' => [ + 'type' => 'attribute', + 'is_bundle' => false, + 'dir' => $projectDir . '/src/Entity', + 'prefix' => 'Dmstr\\ApiConfiguration\\Entity', + ], + ], + ], + ]); + + $container->extension('api_platform', [ + 'title' => 'api-configuration-bundle tests', + 'mapping' => ['paths' => [$projectDir . '/src/Entity']], + 'formats' => [ + 'jsonld' => ['application/ld+json'], + 'json' => ['application/json'], + ], + 'patch_formats' => [ + 'json' => ['application/merge-patch+json'], + ], + 'enable_swagger_ui' => false, + 'enable_re_doc' => false, + ]); + + $container->extension('dmstr_api_platform_utils', [ + // CredentialEncryption expects the 32 raw bytes, not Base64 + 'credential_encryption' => ['key' => SecretsFixture::KEY], + ]); + + $services = $container->services() + ->defaults() + ->autowire() + ->autoconfigure(); + + // Keep the test output free of framework log lines + $services->set('logger', NullLogger::class); + + $services->set(TestHeaderAuthenticator::class); + + if ($this->hasPermissionVoter()) { + // Autoconfiguration adds the `security.voter` tag; public so that + // tests can tell the two variants apart instead of finding an + // inlined service missing in both + $services->set(TestPermissionVoter::class)->public(); + } + + // ApiExtensionInterface implementations are tagged by the bundle's + // autoconfiguration. Schema providers need the tag explicitly: the + // openapi bundle autotags them only inside its own services file. + $services->set(DemoExtension::class); + $services->set(DemoSchemaProvider::class) + ->tag('dmstr_openapi_json_schema.provider'); + $services->set(AuthorizableDemoExtension::class) + ->tag('dmstr_openapi_json_schema.provider'); + } + + private function configureRoutes(RoutingConfigurator $routes): void + { + $routes->import('.', 'api_platform')->prefix(self::API_PREFIX); + $routes->import($this->getProjectDir() . '/config/routes.yaml'); + } +} diff --git a/tests/Kernel/TestPermissionVoter.php b/tests/Kernel/TestPermissionVoter.php new file mode 100644 index 0000000..fd2f7be --- /dev/null +++ b/tests/Kernel/TestPermissionVoter.php @@ -0,0 +1,70 @@ + [self::READ, self::WRITE, self::AUTHORIZE], + 'reader' => [self::READ], + 'authorizer' => [self::AUTHORIZE], + ]; + + public function vote(TokenInterface $token, mixed $subject, array $attributes, ?Vote $vote = null): int + { + try { + $result = self::ACCESS_ABSTAIN; + $granted = self::grantedPermissions($token->getRoleNames()); + + foreach ($attributes as $attribute) { + if (!is_string($attribute) || !str_starts_with($attribute, self::PREFIX)) { + continue; + } + if (in_array($attribute, $granted, true)) { + return self::ACCESS_GRANTED; + } + $result = self::ACCESS_DENIED; + } + + return $result; + } catch (\Throwable) { + return self::ACCESS_ABSTAIN; + } + } + + /** + * @param list $roles + * @return list + */ + public static function grantedPermissions(array $roles): array + { + $granted = []; + foreach ($roles as $role) { + array_push($granted, ...(self::PERMISSIONS[$role] ?? [])); + } + + return array_values(array_unique($granted)); + } +} From 86c67d4db3f47b428caee621697753bdd5bb7fac Mon Sep 17 00:00:00 2001 From: "t.munk (workhorse)" Date: Wed, 28 Oct 2026 07:58:24 +0000 Subject: [PATCH 2/5] feat(security)!: check permission attributes on every operation [*] Every operation of ApiConfiguration now carries an explicit security expression on a bundle-owned permission attribute instead of a role: - Get, GetCollection, health: apiconfig:read - Post, Put, Patch, Delete: apiconfig:write - authorize: apiconfig:authorize - resource-level fallback: apiconfig:read The attributes live in ApiConfigurationPermissions and are tagged za7_core.permission in config/services.yaml, so an application can collect them. symfony/security-bundle and symfony/expression-language move to require, because security expressions are not evaluated without them. BREAKING CHANGE: the bundle no longer checks ROLE_ADMIN. Without a permission voter that grants apiconfig:* every operation answers 403. Co-Authored-By: Claude Opus 5.5 --- composer.json | 4 +- config/services.yaml | 10 + src/Entity/ApiConfiguration.php | 31 ++- src/Security/ApiConfigurationPermissions.php | 37 ++++ tests/Entity/ApiConfigurationSecurityTest.php | 65 +++++-- tests/Http/CurrentBehaviourSmokeTest.php | 31 +-- tests/Http/NoVoterDeniesEverythingTest.php | 32 ++++ tests/Http/OperationRequests.php | 101 ++++++++++ tests/Http/OperationSecurityTest.php | 178 ++++++++++++++++++ tests/Security/PermissionRegistrationTest.php | 97 ++++++++++ 10 files changed, 546 insertions(+), 40 deletions(-) create mode 100644 src/Security/ApiConfigurationPermissions.php create mode 100644 tests/Http/NoVoterDeniesEverythingTest.php create mode 100644 tests/Http/OperationRequests.php create mode 100644 tests/Http/OperationSecurityTest.php create mode 100644 tests/Security/PermissionRegistrationTest.php diff --git a/composer.json b/composer.json index 32c1e5a..e08ef27 100644 --- a/composer.json +++ b/composer.json @@ -18,6 +18,8 @@ "dmstr/api-platform-utils-bundle": "*", "dmstr/openapi-json-schema-bundle": "*", "opis/json-schema": "^2.3", + "symfony/expression-language": "^7.0|^8.0", + "symfony/security-bundle": "^7.0|^8.0", "symfony/validator": "^7.0|^8.0" }, "require-dev": { @@ -27,9 +29,7 @@ "ext-pdo_sqlite": "*", "doctrine/doctrine-migrations-bundle": "^3.3", "symfony/browser-kit": "^7.0|^8.0", - "symfony/expression-language": "^7.0|^8.0", "symfony/http-client": "^7.0|^8.0", - "symfony/security-bundle": "^7.0|^8.0", "symfony/yaml": "^7.0|^8.0" }, "autoload": { diff --git a/config/services.yaml b/config/services.yaml index f5e9b1c..94f7586 100644 --- a/config/services.yaml +++ b/config/services.yaml @@ -16,6 +16,16 @@ services: - '../src/Entity/' - '../src/Extension/' + # Permission attributes of the resource's operations. An application + # (e.g. za7-core) collects the `za7_core.permission` tags to offer them + # for assignment; the bundle itself depends on nothing for it. Replaces + # the definition discovered above (a constants class, never instantiated). + Dmstr\ApiConfiguration\Security\ApiConfigurationPermissions: + tags: + - { name: 'za7_core.permission', permission: 'apiconfig:read' } + - { name: 'za7_core.permission', permission: 'apiconfig:write' } + - { name: 'za7_core.permission', permission: 'apiconfig:authorize' } + # Explicit health-check state provider tag for API Platform Dmstr\ApiConfiguration\State\ApiConfigurationHealthProvider: tags: diff --git a/src/Entity/ApiConfiguration.php b/src/Entity/ApiConfiguration.php index 29ff28e..0f47415 100644 --- a/src/Entity/ApiConfiguration.php +++ b/src/Entity/ApiConfiguration.php @@ -22,6 +22,7 @@ use Dmstr\OpenApiJsonSchema\Attribute\JsonSchema; use Dmstr\OpenApiJsonSchema\Interface\JsonSchemaProviderInterface; use Dmstr\OpenApiJsonSchema\OpenApi\JsonFieldSchemaDecorator; +use Dmstr\ApiConfiguration\Security\ApiConfigurationPermissions; use Dmstr\ApiConfiguration\State\ApiConfigurationAuthorizeProvider; use Dmstr\ApiConfiguration\State\ApiConfigurationHealthProvider; use Dmstr\ApiConfiguration\Validator\ApiConfigurationConstraint; @@ -37,15 +38,16 @@ #[ORM\HasLifecycleCallbacks] #[ApiResource( operations: [ - new Get(), - new GetCollection(), - new Post(security: "is_granted('ROLE_ADMIN')"), - new Put(security: "is_granted('ROLE_ADMIN')"), - new Patch(security: "is_granted('ROLE_ADMIN')"), - new Delete(security: "is_granted('ROLE_ADMIN')"), + new Get(security: self::SECURITY_READ), + new GetCollection(security: self::SECURITY_READ), + new Post(security: self::SECURITY_WRITE), + new Put(security: self::SECURITY_WRITE), + new Patch(security: self::SECURITY_WRITE), + new Delete(security: self::SECURITY_WRITE), new Get( uriTemplate: '/api_configurations/{id}/health', name: 'api_configuration_health', + security: self::SECURITY_READ, provider: ApiConfigurationHealthProvider::class, // Nested arrays (metadata, error) stay plain objects in JSON-LD // instead of becoming hydra:Collection (Issue #9) @@ -54,13 +56,17 @@ new Get( uriTemplate: '/api_configurations/{id}/authorize', name: 'api_configuration_authorize', - security: "is_granted('ROLE_ADMIN')", + security: self::SECURITY_AUTHORIZE, provider: ApiConfigurationAuthorizeProvider::class, ), ], routePrefix: '/admin', - // Reads included: configJson holds upstream credentials (Issue #8) - security: "is_granted('ROLE_ADMIN')", + // Every operation names its permission explicitly; this is only the + // fallback for an operation added without one. Reads are included: + // configJson holds upstream credentials (Issue #8). The attributes are + // decided by the application's permission voter, without one every + // operation is denied (see ApiConfigurationPermissions). + security: self::SECURITY_READ, paginationEnabled: true, paginationItemsPerPage: 30, openapi: new Operation(tags: ['System']) @@ -75,6 +81,13 @@ #[ApiFilter(OrderFilter::class, properties: ['name', 'type', 'endpointType', 'active', 'createdAt', 'updatedAt'])] class ApiConfiguration implements JsonSchemaProviderInterface { + // Security expressions of the operations above. They only check the + // permission attribute, without `object`, so API Platform evaluates them + // before reading the item: a denied caller gets 403, never 404. + private const string SECURITY_READ = "is_granted('" . ApiConfigurationPermissions::READ . "')"; + private const string SECURITY_WRITE = "is_granted('" . ApiConfigurationPermissions::WRITE . "')"; + private const string SECURITY_AUTHORIZE = "is_granted('" . ApiConfigurationPermissions::AUTHORIZE . "')"; + #[ORM\Id] #[ORM\Column(type: 'uuid', unique: true)] #[ApiProperty(identifier: true, readable: true, writable: false, description: 'Unique identifier')] diff --git a/src/Security/ApiConfigurationPermissions.php b/src/Security/ApiConfigurationPermissions.php new file mode 100644 index 0000000..c14fc92 --- /dev/null +++ b/src/Security/ApiConfigurationPermissions.php @@ -0,0 +1,37 @@ + + */ + public static function all(): array + { + return [self::READ, self::WRITE, self::AUTHORIZE]; + } +} diff --git a/tests/Entity/ApiConfigurationSecurityTest.php b/tests/Entity/ApiConfigurationSecurityTest.php index 8bedfaa..a663fbe 100644 --- a/tests/Entity/ApiConfigurationSecurityTest.php +++ b/tests/Entity/ApiConfigurationSecurityTest.php @@ -6,33 +6,66 @@ namespace Dmstr\ApiConfiguration\Tests\Entity; use ApiPlatform\Metadata\ApiResource; +use ApiPlatform\Metadata\Delete; +use ApiPlatform\Metadata\Get; +use ApiPlatform\Metadata\GetCollection; +use ApiPlatform\Metadata\Operation; +use ApiPlatform\Metadata\Patch; +use ApiPlatform\Metadata\Post; +use ApiPlatform\Metadata\Put; use Dmstr\ApiConfiguration\Entity\ApiConfiguration; +use Dmstr\ApiConfiguration\Security\ApiConfigurationPermissions; use PHPUnit\Framework\TestCase; /** - * Issue #8: configJson holds upstream credentials, so every operation of the - * resource — reads included — is restricted to ROLE_ADMIN. + * Every operation of the resource names its permission attribute + * explicitly; reads are included because configJson holds upstream + * credentials (Issue #8). The resource-level expression is only a fallback. */ final class ApiConfigurationSecurityTest extends TestCase { - private const string ADMIN = "is_granted('ROLE_ADMIN')"; + public function testResourceFallbackRequiresReadPermission(): void + { + self::assertSame(self::expression(ApiConfigurationPermissions::READ), self::resource()->getSecurity()); + } + + public function testEveryOperationHasExactlyTheExpectedExplicitExpression(): void + { + $expected = [ + Get::class . ' ' => ApiConfigurationPermissions::READ, + GetCollection::class . ' ' => ApiConfigurationPermissions::READ, + Post::class . ' ' => ApiConfigurationPermissions::WRITE, + Put::class . ' ' => ApiConfigurationPermissions::WRITE, + Patch::class . ' ' => ApiConfigurationPermissions::WRITE, + Delete::class . ' ' => ApiConfigurationPermissions::WRITE, + Get::class . ' /api_configurations/{id}/health' => ApiConfigurationPermissions::READ, + Get::class . ' /api_configurations/{id}/authorize' => ApiConfigurationPermissions::AUTHORIZE, + ]; + + $actual = []; + foreach (self::resource()->getOperations() ?? [] as $operation) { + self::assertNotNull($operation->getSecurity(), self::label($operation) . ' has no own security'); + $actual[self::label($operation)] = $operation->getSecurity(); + } + + self::assertSame(array_map(self::expression(...), $expected), $actual); + } - public function testEveryOperationRequiresAdmin(): void + public static function resource(): ApiResource { $attributes = (new \ReflectionClass(ApiConfiguration::class))->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), - ); - } + return $attributes[0]->newInstance(); + } + + private static function label(Operation $operation): string + { + return $operation::class . ' ' . ($operation->getUriTemplate() ?? ''); + } + + private static function expression(string $permission): string + { + return sprintf("is_granted('%s')", $permission); } } diff --git a/tests/Http/CurrentBehaviourSmokeTest.php b/tests/Http/CurrentBehaviourSmokeTest.php index 3d1499b..6cebc05 100644 --- a/tests/Http/CurrentBehaviourSmokeTest.php +++ b/tests/Http/CurrentBehaviourSmokeTest.php @@ -11,30 +11,35 @@ use Dmstr\ApiConfiguration\Tests\Kernel\TestPermissionVoter; /** - * Smoke test of the HTTP test infrastructure against the current - * behaviour of the resource. + * Smoke test of the HTTP test infrastructure against the behaviour of the + * resource: a role mapped to every `apiconfig:*` permission by the test + * kernel's voter can use it, other roles and anonymous callers cannot. + * The full permission matrix is {@see OperationSecurityTest}. */ final class CurrentBehaviourSmokeTest extends ApiTestCase { /** - * The role the resource's security expression requires today. Replaced - * by permission attributes in the next change, together with this test. + * A role {@see TestPermissionVoter::PERMISSIONS} maps to every + * permission the resource checks. */ - private const string CURRENT_REQUIRED_ROLE = 'ROLE_ADMIN'; + private const string FULL_PERMISSION_ROLE = 'admin'; - public function testCollectionIsReadableWithTheRequiredRole(): void + /** A role the voter maps to no permission */ + private const string UNMAPPED_ROLE = 'nobody'; + + public function testCollectionIsReadableWithAllPermissions(): void { $this->createApiConfiguration(); - $response = $this->requestAs('GET', self::COLLECTION, [self::CURRENT_REQUIRED_ROLE]); + $response = $this->requestAs('GET', self::COLLECTION, [self::FULL_PERMISSION_ROLE]); self::assertSame(200, $response->getStatusCode(), (string) $response->getContent()); self::assertSame(1, self::json($response)['totalItems']); } - public function testCollectionIsForbiddenForAnotherRole(): void + public function testCollectionIsForbiddenForAnUnmappedRole(): void { - $response = $this->requestAs('GET', self::COLLECTION, ['reader']); + $response = $this->requestAs('GET', self::COLLECTION, [self::UNMAPPED_ROLE]); self::assertSame(403, $response->getStatusCode()); } @@ -50,7 +55,7 @@ public function testItemMasksSecrets(): void { $configuration = $this->createApiConfiguration(); - $response = $this->requestAs('GET', $this->itemUri($configuration), [self::CURRENT_REQUIRED_ROLE]); + $response = $this->requestAs('GET', $this->itemUri($configuration), [self::FULL_PERMISSION_ROLE]); self::assertSame(200, $response->getStatusCode(), (string) $response->getContent()); $config = self::json($response)['configJson']; @@ -63,11 +68,11 @@ public function testAuthorizeRedirectsBrowsersAndAnswersJsonClients(): void $configuration = $this->createAuthorizableApiConfiguration(); $uri = $this->itemUri($configuration, '/authorize'); - $browser = $this->requestAs('GET', $uri, [self::CURRENT_REQUIRED_ROLE], headers: ['Accept' => 'text/html']); + $browser = $this->requestAs('GET', $uri, [self::FULL_PERMISSION_ROLE], headers: ['Accept' => 'text/html']); self::assertSame(303, $browser->getStatusCode()); self::assertStringStartsWith(AuthorizableDemoExtension::AUTHORIZE_ENDPOINT . '?', (string) $browser->headers->get('Location')); - $json = $this->requestAs('GET', $uri, [self::CURRENT_REQUIRED_ROLE]); + $json = $this->requestAs('GET', $uri, [self::FULL_PERMISSION_ROLE]); self::assertSame(200, $json->getStatusCode(), (string) $json->getContent()); $body = self::json($json); self::assertStringStartsWith(AuthorizableDemoExtension::AUTHORIZE_ENDPOINT . '?', $body['authorizeUrl']); @@ -79,7 +84,7 @@ public function testAuthorizeConflictsForATypeWithoutAuthorizationFlow(): void { $configuration = $this->createApiConfiguration(); - $response = $this->requestAs('GET', $this->itemUri($configuration, '/authorize'), [self::CURRENT_REQUIRED_ROLE]); + $response = $this->requestAs('GET', $this->itemUri($configuration, '/authorize'), [self::FULL_PERMISSION_ROLE]); self::assertSame(409, $response->getStatusCode()); } diff --git a/tests/Http/NoVoterDeniesEverythingTest.php b/tests/Http/NoVoterDeniesEverythingTest.php new file mode 100644 index 0000000..cf53941 --- /dev/null +++ b/tests/Http/NoVoterDeniesEverythingTest.php @@ -0,0 +1,32 @@ +requestOperation($operation, ['admin']); + + self::assertSame(403, $response->getStatusCode(), (string) $response->getContent()); + } +} diff --git a/tests/Http/OperationRequests.php b/tests/Http/OperationRequests.php new file mode 100644 index 0000000..7e1b9bf --- /dev/null +++ b/tests/Http/OperationRequests.php @@ -0,0 +1,101 @@ + 200, + self::OP_COLLECTION => 200, + self::OP_HEALTH => 200, + self::OP_POST => 201, + self::OP_PUT => 200, + self::OP_PATCH => 200, + self::OP_DELETE => 204, + self::OP_AUTHORIZE => 200, + ]; + + /** Operations with an `{id}` in the URI */ + public const array ITEM_OPERATIONS = [ + self::OP_GET, + self::OP_HEALTH, + self::OP_PUT, + self::OP_PATCH, + self::OP_DELETE, + self::OP_AUTHORIZE, + ]; + + /** + * Requests an operation as a user with the given roles. Item operations + * address a freshly stored configuration (an authorizable one for + * authorize), or a random id that does not exist. + * + * @param list $roles + * @param array $headers + */ + protected function requestOperation( + string $operation, + array $roles, + bool $existing = true, + array $headers = [], + ): Response { + $id = Uuid::v4()->toRfc4122(); + if ($existing && \in_array($operation, self::ITEM_OPERATIONS, true)) { + $configuration = $operation === self::OP_AUTHORIZE + ? $this->createAuthorizableApiConfiguration() + : $this->createApiConfiguration(); + $id = $configuration->getId()->toRfc4122(); + } + $item = self::COLLECTION . '/' . $id; + + return match ($operation) { + self::OP_GET => $this->requestAs('GET', $item, $roles, headers: $headers), + self::OP_COLLECTION => $this->requestAs('GET', self::COLLECTION, $roles, headers: $headers), + self::OP_HEALTH => $this->requestAs('GET', $item . '/health', $roles, headers: $headers), + self::OP_POST => $this->requestAs('POST', self::COLLECTION, $roles, self::validBody('Created'), $headers), + self::OP_PUT => $this->requestAs('PUT', $item, $roles, self::validBody('Replaced'), $headers), + self::OP_PATCH => $this->requestAs('PATCH', $item, $roles, ['name' => 'Renamed'], $headers), + self::OP_DELETE => $this->requestAs('DELETE', $item, $roles, headers: $headers), + self::OP_AUTHORIZE => $this->requestAs('GET', $item . '/authorize', $roles, headers: $headers), + }; + } + + /** + * @return array + */ + protected static function validBody(string $name): array + { + return ['name' => $name, 'configJson' => SecretsFixture::config(), 'active' => true]; + } + + /** + * @return iterable + */ + public static function allOperations(): iterable + { + foreach (array_keys(self::SUCCESS_STATUS) as $operation) { + yield $operation => [$operation]; + } + } +} diff --git a/tests/Http/OperationSecurityTest.php b/tests/Http/OperationSecurityTest.php new file mode 100644 index 0000000..c74e05b --- /dev/null +++ b/tests/Http/OperationSecurityTest.php @@ -0,0 +1,178 @@ + + */ + public static function grantedCases(): iterable + { + foreach (array_keys(self::SUCCESS_STATUS) as $operation) { + yield "admin $operation" => [$operation, 'admin']; + } + foreach ([self::OP_GET, self::OP_COLLECTION, self::OP_HEALTH] as $operation) { + yield "reader $operation" => [$operation, 'reader']; + } + yield 'authorizer authorize' => [self::OP_AUTHORIZE, 'authorizer']; + } + + #[DataProvider('grantedCases')] + public function testGranted(string $operation, string $role): void + { + $response = $this->requestOperation($operation, [$role]); + + self::assertSame(self::SUCCESS_STATUS[$operation], $response->getStatusCode(), (string) $response->getContent()); + } + + /** + * @return iterable + */ + public static function deniedCases(): iterable + { + foreach ([self::OP_POST, self::OP_PUT, self::OP_PATCH, self::OP_DELETE, self::OP_AUTHORIZE] as $operation) { + yield "reader $operation" => [$operation, 'reader']; + } + // The explicit per-operation attribute: authorize does not imply read + foreach ([self::OP_GET, self::OP_COLLECTION, self::OP_HEALTH, self::OP_POST, self::OP_PUT, self::OP_PATCH, self::OP_DELETE] as $operation) { + yield "authorizer $operation" => [$operation, 'authorizer']; + } + } + + #[DataProvider('deniedCases')] + public function testDenied(string $operation, string $role): void + { + $response = $this->requestOperation($operation, [$role]); + + self::assertSame(403, $response->getStatusCode(), (string) $response->getContent()); + } + + #[DataProvider('allOperations')] + public function testUnmappedRoleIsDeniedEverything(string $operation): void + { + self::assertSame(403, $this->requestOperation($operation, ['nobody'])->getStatusCode()); + } + + /** + * The expressions do not use `object`, so API Platform checks them + * before reading the item: a denied caller cannot probe ids by 404. + * + * @return iterable + */ + public static function itemOperations(): iterable + { + foreach (self::ITEM_OPERATIONS as $operation) { + yield $operation => [$operation]; + } + } + + #[DataProvider('itemOperations')] + public function testDenialDoesNotDependOnTheIdExisting(string $operation): void + { + $existing = $this->requestOperation($operation, ['nobody'], existing: true); + $missing = $this->requestOperation($operation, ['nobody'], existing: false); + + self::assertSame(403, $existing->getStatusCode()); + self::assertSame(403, $missing->getStatusCode(), (string) $missing->getContent()); + } + + /** + * @return iterable + */ + public static function jsonAcceptHeaders(): iterable + { + yield 'any' => ['*/*']; + yield 'json-ld' => ['application/ld+json']; + yield 'json with text fallback' => ['application/json, text/plain;q=0.5']; + } + + #[DataProvider('jsonAcceptHeaders')] + public function testAuthorizeOnTheJsonPathObeysTheOperationSecurity(string $accept): void + { + $headers = ['Accept' => $accept]; + + $denied = $this->requestOperation(self::OP_AUTHORIZE, ['reader'], headers: $headers); + self::assertSame(403, $denied->getStatusCode()); + + $granted = $this->requestOperation(self::OP_AUTHORIZE, ['authorizer'], headers: $headers); + self::assertSame(200, $granted->getStatusCode(), (string) $granted->getContent()); + self::assertStringStartsWith(AuthorizableDemoExtension::AUTHORIZE_ENDPOINT . '?', self::json($granted)['authorizeUrl']); + } + + public function testReaderSeesCredentialsMasked(): void + { + $configuration = $this->createApiConfiguration(); + + $item = $this->requestAs('GET', $this->itemUri($configuration), ['reader']); + self::assertSame(200, $item->getStatusCode()); + self::assertSame(ConfigSecrets::MASK, self::json($item)['configJson']['password']); + self::assertSame('alice', self::json($item)['configJson']['username']); + + $collection = $this->requestAs('GET', self::COLLECTION, ['reader']); + self::assertSame(200, $collection->getStatusCode()); + $members = self::json($collection)['member'] ?? self::json($collection)['hydra:member']; + self::assertSame(ConfigSecrets::MASK, $members[0]['configJson']['password']); + self::assertStringNotContainsString('s3cr3t-password', (string) $collection->getContent()); + } + + /** + * The `hydra:operation` list of item output (added by + * dmstr/api-platform-utils-bundle's AddHydraOperationsSubscriber, not by + * API Platform) is filtered by the operations' security expressions: + * a reader is offered no write and no authorize operation. Collection + * output carries no such list here (the subscriber only recognises the + * `hydra:Collection` type, API Platform 4 emits `Collection`), and the + * API documentation (`hydra:supportedOperation`) is not filtered. + */ + public function testReaderIsOfferedOnlyReadOperations(): void + { + $configuration = $this->createApiConfiguration(); + $item = $this->itemUri($configuration); + + self::assertSame( + [['GET', $item], ['GET', $item . '/health']], + $this->offeredOperations($item, 'reader'), + ); + } + + public function testAdminIsOfferedEveryOperation(): void + { + $configuration = $this->createApiConfiguration(); + $item = $this->itemUri($configuration); + + $offered = $this->offeredOperations($item, 'admin'); + foreach ([['PUT', $item], ['PATCH', $item], ['DELETE', $item], ['GET', $item . '/authorize']] as $operation) { + self::assertContains($operation, $offered); + } + } + + /** + * @return list method and URI of each offered operation + */ + private function offeredOperations(string $uri, string $role): array + { + $response = $this->requestAs('GET', $uri, [$role]); + self::assertSame(200, $response->getStatusCode()); + + return array_map( + static fn (array $operation): array => [$operation['hydra:method'], $operation['@id']], + self::json($response)['hydra:operation'] ?? [], + ); + } +} diff --git a/tests/Security/PermissionRegistrationTest.php b/tests/Security/PermissionRegistrationTest.php new file mode 100644 index 0000000..feeb8d1 --- /dev/null +++ b/tests/Security/PermissionRegistrationTest.php @@ -0,0 +1,97 @@ + + */ + private static function taggedPermissions(): array + { + $container = new ContainerBuilder(); + (new YamlFileLoader($container, new FileLocator(\dirname(__DIR__, 2) . '/config')))->load('services.yaml'); + + $permissions = []; + foreach ($container->findTaggedServiceIds(self::TAG) as $tags) { + foreach ($tags as $attributes) { + self::assertArrayHasKey('permission', $attributes); + $permissions[] = $attributes['permission']; + } + } + + return $permissions; + } + + /** + * Attributes of every `is_granted('…')` in the resource and operation + * security expressions. + * + * @return list + */ + private static function checkedAttributes(): array + { + $resource = ApiConfigurationSecurityTest::resource(); + $expressions = [$resource->getSecurity()]; + foreach ($resource->getOperations() ?? [] as $operation) { + $expressions[] = $operation->getSecurity(); + } + + $attributes = []; + foreach (array_filter($expressions) as $expression) { + preg_match_all("/is_granted\\(\\s*'([^']+)'/", $expression, $matches); + array_push($attributes, ...$matches[1]); + } + + return array_values(array_unique($attributes)); + } +} From 17073c633fe05f3fd5242c9271641f85eaa09beb Mon Sep 17 00:00:00 2001 From: "t.munk (workhorse)" Date: Wed, 28 Oct 2026 07:58:24 +0000 Subject: [PATCH 3/5] fix(security)!: check apiconfig:authorize on the browser path of authorize [*] AuthorizeRedirectListener answered GET .../authorize with Accept: text/html by a 303 redirect before API Platform's security ran, so any authenticated caller could start the OAuth flow. It now checks apiconfig:authorize right after the Accept check and before looking up the configuration: denied callers get 403 (401 when anonymous) for existing and unknown ids alike. BREAKING CHANGE: the browser path of authorize requires the apiconfig:authorize permission. Co-Authored-By: Claude Opus 5.5 --- .../AuthorizeRedirectListener.php | 17 +++- .../Http/AuthorizeBrowserPathSecurityTest.php | 94 +++++++++++++++++++ 2 files changed, 109 insertions(+), 2 deletions(-) create mode 100644 tests/Http/AuthorizeBrowserPathSecurityTest.php diff --git a/src/EventListener/AuthorizeRedirectListener.php b/src/EventListener/AuthorizeRedirectListener.php index 64a016a..2b06cee 100644 --- a/src/EventListener/AuthorizeRedirectListener.php +++ b/src/EventListener/AuthorizeRedirectListener.php @@ -7,6 +7,7 @@ use Dmstr\ApiConfiguration\Entity\ApiConfiguration; use Dmstr\ApiConfiguration\Extension\AuthorizableExtensionInterface; +use Dmstr\ApiConfiguration\Security\ApiConfigurationPermissions; use Dmstr\ApiConfiguration\Service\ApiExtensionRegistry; use Dmstr\ApiPlatformUtils\Service\UuidResolver; use Symfony\Component\DependencyInjection\Attribute\Autowire; @@ -15,6 +16,8 @@ use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpKernel\Event\RequestEvent; use Symfony\Component\HttpKernel\KernelEvents; +use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface; +use Symfony\Component\Security\Core\Exception\AccessDeniedException; /** * Short-circuits the normal API-Platform pipeline for @@ -25,8 +28,14 @@ * JSON-LD (Hydra) and XHR callers keep the regular pipeline → JSON body * with `authorizeUrl`/`state` from {@see ApiConfigurationAuthorizeProvider}. * - * Runs after Symfony Security (priority < 8) so the bearer-token check has - * already happened; the listener trusts that the request is authorized. + * Runs after the Symfony firewall (priority < 8), so the token is + * available, but before API Platform's own security check: the listener + * therefore checks the operation's permission + * ({@see ApiConfigurationPermissions::AUTHORIZE}) itself. It does so before + * looking up the configuration, so a denied caller always gets the same + * answer and cannot probe ids by a 404 or 409. A denial throws Symfony's + * {@see AccessDeniedException}, which the firewall turns into 403 for an + * authenticated caller and into its entry point (401) for an anonymous one. */ #[AsEventListener(event: KernelEvents::REQUEST, priority: 4)] final class AuthorizeRedirectListener @@ -36,6 +45,7 @@ final class AuthorizeRedirectListener public function __construct( private readonly UuidResolver $uuidResolver, private readonly ApiExtensionRegistry $extensionRegistry, + private readonly AuthorizationCheckerInterface $authorizationChecker, #[Autowire(param: 'app.oauth.redirect_uri')] private readonly string $defaultRedirectUri, ) { @@ -56,6 +66,9 @@ public function __invoke(RequestEvent $event): void if (!$this->wantsHtml($request)) { return; } + if (!$this->authorizationChecker->isGranted(ApiConfigurationPermissions::AUTHORIZE)) { + throw new AccessDeniedException(); + } $config = $this->uuidResolver->findByPartialUuid(ApiConfiguration::class, $matches[1]); if (!$config instanceof ApiConfiguration) { diff --git a/tests/Http/AuthorizeBrowserPathSecurityTest.php b/tests/Http/AuthorizeBrowserPathSecurityTest.php new file mode 100644 index 0000000..766a0bb --- /dev/null +++ b/tests/Http/AuthorizeBrowserPathSecurityTest.php @@ -0,0 +1,94 @@ + 'text/html']; + + /** The one test that runs against the kernel without a permission voter */ + private const string NO_VOTER_TEST = 'testWithoutVoterEvenAdminIsDenied'; + + private static string $environment = TestKernel::ENV_WITH_VOTER; + + protected static function kernelEnvironment(): string + { + return self::$environment; + } + + protected function setUp(): void + { + self::$environment = $this->name() === self::NO_VOTER_TEST + ? TestKernel::ENV_NO_VOTER + : TestKernel::ENV_WITH_VOTER; + + parent::setUp(); + } + + public function testAuthorizerIsRedirected(): void + { + $response = $this->requestAs('GET', $this->authorizeUri(), ['authorizer'], headers: self::HTML); + + self::assertSame(303, $response->getStatusCode(), (string) $response->getContent()); + self::assertStringStartsWith(AuthorizableDemoExtension::AUTHORIZE_ENDPOINT . '?', (string) $response->headers->get('Location')); + } + + public function testReaderIsDenied(): void + { + $response = $this->requestAs('GET', $this->authorizeUri(), ['reader'], headers: self::HTML); + + self::assertSame(403, $response->getStatusCode()); + self::assertFalse($response->headers->has('Location')); + } + + /** + * No 404 for a missing id and no 409 for a type without authorization + * flow before the permission check: the answer reveals nothing. + */ + public function testUnmappedRoleIsDeniedWhetherOrNotTheIdExists(): void + { + $existing = $this->authorizeUri(); + $missing = self::COLLECTION . '/' . Uuid::v4()->toRfc4122() . '/authorize'; + $notAuthorizable = $this->itemUri($this->createApiConfiguration(), '/authorize'); + + foreach ([$existing, $missing, $notAuthorizable] as $uri) { + $response = $this->requestAs('GET', $uri, ['nobody'], headers: self::HTML); + self::assertSame(403, $response->getStatusCode(), $uri); + } + } + + public function testAnonymousCallerMustAuthenticate(): void + { + $response = $this->requestAnonymous('GET', $this->authorizeUri(), headers: self::HTML); + + self::assertSame(401, $response->getStatusCode()); + self::assertFalse($response->headers->has('Location')); + } + + public function testWithoutVoterEvenAdminIsDenied(): void + { + self::assertFalse(static::getContainer()->has(TestPermissionVoter::class)); + + $response = $this->requestAs('GET', $this->authorizeUri(), ['admin'], headers: self::HTML); + + self::assertSame(403, $response->getStatusCode()); + } + + private function authorizeUri(): string + { + return $this->itemUri($this->createAuthorizableApiConfiguration(), '/authorize'); + } +} From e53a2490b53e42810bc2485b703d8eca3022a06d Mon Sep 17 00:00:00 2001 From: "t.munk (workhorse)" Date: Wed, 28 Oct 2026 08:01:25 +0000 Subject: [PATCH 4/5] build!: require PHP 8.5 [*] composer.json, the CI matrix, the compose test runner and CLAUDE.md move from PHP 8.4 to 8.5. BREAKING CHANGE: PHP >= 8.5 is required. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/tests.yml | 2 +- CLAUDE.md | 2 +- composer.json | 2 +- docker-compose.yml | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 110ada9..add1f84 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -12,7 +12,7 @@ jobs: strategy: fail-fast: false matrix: - php: ["8.4"] + php: ["8.5"] name: PHPUnit (PHP ${{ matrix.php }}) steps: - uses: actions/checkout@v4 diff --git a/CLAUDE.md b/CLAUDE.md index 2b3ff99..8360985 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -25,4 +25,4 @@ via `.github/workflows/tests.yml`. ## Requirements -PHP >= 8.4. +PHP >= 8.5. diff --git a/composer.json b/composer.json index e08ef27..6f64eb4 100644 --- a/composer.json +++ b/composer.json @@ -10,7 +10,7 @@ } ], "require": { - "php": ">=8.4", + "php": ">=8.5", "symfony/framework-bundle": "^7.0", "api-platform/symfony": "^4.0", "doctrine/orm": "^3.0", diff --git a/docker-compose.yml b/docker-compose.yml index 1455605..4e5b3e8 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -7,7 +7,7 @@ services: php: build: dockerfile_inline: | - FROM php:8.4-cli-alpine + FROM php:8.5-cli-alpine COPY --from=mlocati/php-extension-installer /usr/bin/install-php-extensions /usr/local/bin/ RUN install-php-extensions pdo_sqlite COPY --from=composer:2 /usr/bin/composer /usr/bin/composer From 05d6a5580c42fa2e7ef20e07f3b093672e8e615b Mon Sep 17 00:00:00 2001 From: "t.munk (workhorse)" Date: Wed, 28 Oct 2026 08:01:25 +0000 Subject: [PATCH 5/5] docs: permissions and migration notes for 0.6.0 [*] README: Requirements, the permission table, the za7_core.permission tags and a migration section with a minimal voter. CHANGELOG: [0.6.0] with the breaking changes. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 17 +++++++++++++++++ README.md | 42 +++++++++++++++++++++++++++++++++++++++++- 2 files changed, 58 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cda80e0..cb21cb8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,22 @@ All notable changes to this bundle. The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); versions follow [SemVer](https://semver.org/) and come from the Git tags. +## [0.6.0] - Unreleased + +Access to `ApiConfiguration` is decided by permission attributes instead of a fixed role, so applications choose who may read, write and authorize configurations. The changes break consumers in the ways listed under "Breaking changes"; the migration is described in the README under [Migrating from 0.5](README.md#migrating-from-05). + +### Breaking changes + +- **Permission attributes replace the role check.** `Get`, `GetCollection` and `health` check `apiconfig:read`; `Post`, `Put`, `Patch` and `Delete` check `apiconfig:write`; `authorize` checks `apiconfig:authorize` (before: `ROLE_ADMIN` for every operation). The constants are in `Dmstr\ApiConfiguration\Security\ApiConfigurationPermissions` and are tagged `za7_core.permission` for applications that collect them. +- **Without a permission voter every operation answers `403`.** The bundle grants nothing itself; the application needs a voter that grants the three attributes. Denied callers get `403` before any `404`. +- **The browser path of `authorize` is hardened.** `GET /api/admin/api_configurations/{id}/authorize` with `Accept: text/html` checks `apiconfig:authorize` before looking up the configuration and answers `403` when denied, `401` for anonymous callers; before, any authenticated caller got the `303` redirect. +- **`symfony/security-bundle` and `symfony/expression-language` are required** (`require` instead of `require-dev`); the application needs a firewall in front of `/api/admin`. +- **PHP >= 8.5** is required. + +### Added + +- Tests: an HTTP test kernel under `tests/` (header authentication via `X-Test-Roles`, `X-Test-User` and `X-Test-Auth: anonymous`, a test permission voter, SQLite in memory) that runs the operations through the full request pipeline, including a kernel without a voter. + ## [0.5.0] - 2026-10-10 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)). The changes break consumers in the ways listed under "Breaking changes". 0.5.0-beta1 contained everything below except the removal of `order[id]` ("Changed"). @@ -45,4 +61,5 @@ Secure storage and transport of the credentials in `configJson` ([Issue #8](http See the git history. +[0.6.0]: https://github.com/dmstr/api-configuration-bundle/compare/0.5.0...HEAD [0.5.0]: https://github.com/dmstr/api-configuration-bundle/compare/0.4.0...0.5.0 diff --git a/README.md b/README.md index 3634456..7db6301 100644 --- a/README.md +++ b/README.md @@ -14,6 +14,10 @@ Manage external API connections as Doctrine entities. Changes per version are listed in [CHANGELOG.md](CHANGELOG.md). +## Requirements + +PHP >= 8.5 and `symfony/security-bundle` with a firewall in front of `/api/admin`, see [Access](#access). + ## 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`: @@ -37,7 +41,43 @@ An extension implements `ApiExtensionInterface` (tagged automatically) and build ### 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. +Every operation of the `ApiConfiguration` resource under `/api/admin/api_configurations` checks a permission attribute with `is_granted()`, because `configJson` holds the credentials for the upstream systems: + +| Permission | Operations | +|---|---| +| `apiconfig:read` | `GET` collection and item, `GET /api/admin/api_configurations/{id}/health` | +| `apiconfig:write` | `POST`, `PUT`, `PATCH`, `DELETE` | +| `apiconfig:authorize` | `GET /api/admin/api_configurations/{id}/authorize`, also the browser path (`Accept: text/html`, answered with a `303` redirect) | + +The constants are in `Dmstr\ApiConfiguration\Security\ApiConfigurationPermissions` (`READ`, `WRITE`, `AUTHORIZE`, `all()`). The bundle also registers the three attributes as `za7_core.permission` service tags (attribute `permission`), so applications that collect such tags can offer them for assignment; the bundle itself depends on nothing for this. + +The bundle grants none of the permissions itself. A permission voter in the application maps its roles or users to the permissions; without one, every operation answers `403`. Denied callers get `403` before any `404`, anonymous callers `401` from the firewall. Credentials stay masked for every caller, and the `hydra:operation` list of an item only shows the operations the caller is allowed. + +#### Migrating from 0.5 + +0.5 checked a fixed role; 0.6 checks the permissions above, so an application without a voter for them loses all access after the upgrade. A minimal voter granting all three permissions to an administrator role, with `` replaced by the role your application uses: + +```php +use Dmstr\ApiConfiguration\Security\ApiConfigurationPermissions; +use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; +use Symfony\Component\Security\Core\Authorization\Voter\Vote; +use Symfony\Component\Security\Core\Authorization\Voter\Voter; + +final class ApiConfigurationPermissionVoter extends Voter +{ + protected function supports(string $attribute, mixed $subject): bool + { + return in_array($attribute, ApiConfigurationPermissions::all(), true); + } + + protected function voteOnAttribute(string $attribute, mixed $subject, TokenInterface $token, ?Vote $vote = null): bool + { + return in_array('', $token->getRoleNames(), true); + } +} +``` + +With autoconfiguration the voter is registered by itself. If the application already has a permission voter driven by a role-to-permission map, add the three attributes to the entry of `` instead. Clients that open the authorize URL in a browser need `apiconfig:authorize` as well; before 0.6 any authenticated caller got the redirect. ### Secrets in `configJson`