From 83eb021e901810c96be07c78aa6cfad9471a623c Mon Sep 17 00:00:00 2001 From: aubes <3941035+aubes@users.noreply.github.com> Date: Tue, 29 Sep 2026 18:21:19 +0200 Subject: [PATCH 1/3] feat: resolve the evaluation context lazily and detect SecurityBundle for user_provider and on_disabled --- CHANGELOG.md | 6 + docs/configuration.md | 30 +- docs/features/evaluation-context.md | 34 +- docs/features/feature-gate.md | 6 +- docs/profiler.md | 34 +- .../EvaluationContextProviderInterface.php | 7 +- .../LazyEvaluationContext.php | 60 +++ .../UserEvaluationContextProvider.php | 4 +- .../EvaluationContextListener.php | 65 +++- src/OpenFeatureBundle.php | 24 +- src/Profiler/OpenFeatureDataCollector.php | 21 +- src/Resources/config/services.php | 1 + .../views/Collector/openfeature.html.twig | 6 +- .../OpenFeatureBundleTest.php | 343 ++++++++++++++++++ .../LazyEvaluationContextTest.php | 130 +++++++ .../UserEvaluationContextProviderTest.php | 7 + .../EvaluationContextListenerLazinessTest.php | 181 +++++++++ ...penFeatureDataCollectorLazyContextTest.php | 65 ++++ 18 files changed, 963 insertions(+), 61 deletions(-) create mode 100644 src/EvaluationContext/LazyEvaluationContext.php create mode 100644 tests/DependencyInjection/OpenFeatureBundleTest.php create mode 100644 tests/EvaluationContext/LazyEvaluationContextTest.php create mode 100644 tests/EventListener/EvaluationContextListenerLazinessTest.php create mode 100644 tests/Profiler/OpenFeatureDataCollectorLazyContextTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 6dc24cf..4f82959 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,10 +14,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `EnvVarProvider` and `RedisProvider` now return a `PARSE_ERROR` (and the default value) for raw values that do not match the requested type, instead of silently casting them (`"abc"` as integer used to resolve to `0`, `"banana"` as boolean to `false`). - `InMemoryProvider` returns a `TYPE_MISMATCH` instead of casting for integers other than `0`/`1` requested as boolean (e.g. `2` or `-1`, previously `true`), non-string values requested as string (e.g. `42`, previously `"42"`), and floats requested as integer (e.g. `1.5`, previously `1`). - `ResolutionDetailsTrait::toBool()` is replaced by `parseBool()`, `parseInt()`, `parseFloat()`, and `parseObject()`. +- Evaluation context providers now run on the first flag evaluation of the request instead of on `kernel.request`. A request that evaluates no flag no longer reads the security token, so a `lazy` firewall stays lazy and the response stays HTTP-cacheable. `EvaluationContextContributedEvent` is dispatched at that time. +- An exception thrown by an evaluation context provider, or by a listener of `EvaluationContextContributedEvent`, is now logged instead of failing the request (the failing provider is skipped). - Provider services (`provider`, `providers`) must now declare their class, as Symfony already requires for services built by a factory. A provider created by a factory, or inheriting its class from a `parent` under an id that is not a class name, now fails at compile time without an explicit `class` option instead of skipping validation. ### Fixed +- `evaluation_context.user_provider` now works: `auto` always resolved to `false` and `true` always failed, even with SecurityBundle enabled. With SecurityBundle enabled but not configured (Symfony 6.4), the user provider now does nothing instead of breaking the container compilation. +- `feature_flag.on_disabled: auto` now picks `access_denied` only when SecurityBundle is enabled. With `symfony/security-core` installed but no SecurityBundle (e.g. pulled by `symfony/security-csrf`), a disabled feature gate returned a 500 instead of a 403. Set `on_disabled: access_denied` explicitly to keep the previous exception. - A provider service whose class does not exist now fails with an explicit "cannot be found" message instead of "must implement Provider", and a provider class whose parent class or interface is missing reports that missing class. - Providers now receive the application logger when MonologBundle is not installed; previously they got none. A logger already set on a provider service (e.g. a dedicated Monolog channel) is no longer overridden. - `flags` and `providers`: keys are now kept as declared. Dashes were converted to underscores (a flag declared as `new-checkout` could only be evaluated as `new_checkout`), and an object flag holding a `name` key was renamed after that value. The undocumented list form `flags: [{name: ..., value: ...}]` is no longer supported. @@ -29,6 +33,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Code calling `OpenFeatureAPI::getInstance()` directly now gets an instance distinct from the bundle's `API` service (different provider, hooks, and evaluation context). Inject the `API` or `Client` service instead. - **Check your raw flag values.** `EnvVarProvider` and `RedisProvider` no longer cast unparsable values: a boolean flag set to anything other than `true`/`false`/`1`/`0`/`yes`/`no`/`on`/`off`/empty (e.g. `FEATURE_X=enabled`, previously `false`) or a numeric flag with a non-numeric value (for integers, decimal or exponent notation such as `10.0` or `1e3` too; leading zeros such as `08` are accepted) now resolves to the default value with a `PARSE_ERROR`. `InMemoryProvider` flags declared with a mismatching type (e.g. `max_items: 1.5` read as integer, `label: 42` read as string) now resolve to the default value with a `TYPE_MISMATCH`, like with typed providers such as flagd. In Twig, `{{ feature_value('max_items') }}` without a default reads the flag as a string and now renders `''`: pass a typed default (`feature_value('max_items', 10)`). These errors are not logged by the SDK: check the `open_feature` profiler panel in dev (error column), or register a hook that logs `ResolutionDetails::getError()` in `after()`. - **Provider services created by a factory, or defined through `parent` under an id that is not a class name,** must set the `class` option (e.g. `class: App\FeatureFlag\MyProvider` next to `factory:`; `OpenFeature\interfaces\provider\Provider` is accepted when the concrete class is unknown), otherwise the container fails to compile with `Class "" used for OpenFeature provider service "..." cannot be found`. +- **`user_provider: auto` now takes effect.** With SecurityBundle enabled, the authenticated user identifier becomes the targeting key. Set `evaluation_context.user_provider: false` to keep the previous behavior. +- **Evaluation context providers run lazily.** Logic that relies on running at the start of every request (side effects, timing) must move to its own `kernel.request` listener. The API-level context is now an internal `LazyEvaluationContext`: an `instanceof MutableEvaluationContext` check on `API::getEvaluationContext()` no longer matches, and reading its targeting key or attributes runs the providers. - Custom providers using `ResolutionDetailsTrait::toBool()` must switch to `parseBool($flagKey, $raw, $defaultValue)`, which returns a `ResolutionDetails` instead of a `bool`. ## [0.3.0] - 2026-06-15 diff --git a/docs/configuration.md b/docs/configuration.md index f3b56d4..2e33eaf 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -7,17 +7,17 @@ Full configuration tree for `open_feature`: open_feature: # Service ID of the OpenFeature provider - # Default: Aubes\OpenFeatureBundle\Provider\InMemoryProvider + # Default: none (the InMemoryProvider is used when neither "provider" nor "providers" is set) # Mutually exclusive with "providers" - provider: Aubes\OpenFeatureBundle\Provider\InMemoryProvider + provider: ~ # Multiple providers combined through the SDK MultiProvider # Keys are provider names, values are service IDs # Evaluation follows declaration order # Mutually exclusive with "provider" - providers: - remote: App\OpenFeature\MyProvider - local: Aubes\OpenFeatureBundle\Provider\InMemoryProvider + providers: {} + # remote: App\OpenFeature\MyProvider + # local: Aubes\OpenFeatureBundle\Provider\InMemoryProvider # Evaluation strategy for the MultiProvider (only when using "providers") # Shorthand: strategy: first_match @@ -36,14 +36,14 @@ open_feature: # EvaluationContext settings evaluation_context: # Populate targeting key from the authenticated Symfony user - # auto: enabled if symfony/security-core is installed - # true: always enabled (requires symfony/security-core) + # auto: enabled if SecurityBundle is enabled + # true: always enabled (requires SecurityBundle) # false: disabled user_provider: auto # auto | true | false # Exception behavior for #[FeatureGate] feature_flag: - # auto: AccessDeniedException if security-core is available, HttpException otherwise + # auto: AccessDeniedException if SecurityBundle is enabled, HttpException otherwise # access_denied: always throw AccessDeniedException # http_exception: always throw HttpException on_disabled: auto # auto | access_denied | http_exception @@ -52,11 +52,11 @@ open_feature: status_code: 403 # Redis provider settings (only when using RedisProvider) - redis: - # Service implementing RedisClientInterface - client: ~ - # Key prefix for flag lookup - prefix: 'feature:' + # redis: + # # Service implementing RedisClientInterface (required) + # client: App\OpenFeature\MyRedisClient + # # Key prefix for flag lookup + # prefix: 'feature:' ``` ## Provider @@ -72,7 +72,7 @@ See [Providers](providers/index.md) for available options. ## Multiple providers -Declare several providers under `providers` to combine them through the SDK `MultiProvider` (requires `open-feature/sdk` >= 2.2). Each key is a provider name, each value a service ID. Providers are evaluated in declaration order: +Declare several providers under `providers` to combine them through the SDK `MultiProvider`. Each key is a provider name, each value a service ID. Providers are evaluated in declaration order: ```yaml open_feature: @@ -126,6 +126,6 @@ The `feature_flag.on_disabled` setting controls what happens when a `#[FeatureGa | Value | Exception type | When to use | |---|---|---| -| `auto` (default) | `AccessDeniedException` if `symfony/security-core` is installed, `HttpException` otherwise | Most apps | +| `auto` (default) | `AccessDeniedException` if SecurityBundle is enabled, `HttpException` otherwise | Most apps | | `access_denied` | `AccessDeniedException` | When you have a security error handler | | `http_exception` | `HttpException` with configurable status code | APIs, custom error pages | diff --git a/docs/features/evaluation-context.md b/docs/features/evaluation-context.md index 1e3d58b..3aa67c4 100644 --- a/docs/features/evaluation-context.md +++ b/docs/features/evaluation-context.md @@ -4,7 +4,7 @@ The `EvaluationContext` carries targeting information (user ID, attributes) used ## Auto-populate from the Symfony user -When `symfony/security-core` is available, the authenticated user's identifier is automatically set as the `targeting_key`: +When SecurityBundle is enabled, the authenticated user's identifier is automatically set as the `targeting_key`: ```yaml open_feature: @@ -14,10 +14,12 @@ open_feature: | Value | Behavior | |---|---| -| `auto` (default) | Enabled if `symfony/security-core` is installed | -| `true` | Always enabled (requires `symfony/security-core`) | +| `auto` (default) | Enabled if SecurityBundle is enabled | +| `true` | Always enabled (requires SecurityBundle) | | `false` | Disabled | +> **Note:** The user identifier is sent as-is to the flag provider, which may be a remote service (flagd, GO Feature Flag relay proxy, LaunchDarkly). If it is personal data, such as an email address, set `user_provider: false` and register a [custom context provider](#custom-context-provider) that sets a non-personal targeting key (internal user ID, hash). The profiler only displays a hash of the targeting key. + ## Custom context provider Implement `EvaluationContextProviderInterface` to contribute additional attributes: @@ -45,7 +47,31 @@ The service is picked up automatically, no tag or config needed. ## Multiple providers -Multiple context providers are supported. Their contexts are merged via the SDK's `EvaluationContext::merge()` method. +Multiple context providers are supported. They run highest priority first, then their contexts are merged by the SDK: on conflicts, the last context wins. A lower-priority provider therefore overrides a higher-priority one for the targeting key and any shared attribute. The built-in user provider runs at priority `0`. + +Set the priority with the `#[AsTaggedItem]` attribute on the provider class, or with the `priority` attribute of the `openfeature.evaluation_context_provider` tag: + +```php +use Symfony\Component\DependencyInjection\Attribute\AsTaggedItem; + +#[AsTaggedItem(priority: 100)] +class FallbackContextProvider implements EvaluationContextProviderInterface +``` + +Register a fallback (e.g. an anonymous targeting key) at a high priority, so that a real identity contributed at a lower priority wins. + +## When context providers run + +Context providers run on the first flag evaluation of each main request, not when the request starts. Their merged context is reused for the other evaluations of the same request. + +A request that evaluates no flag never runs them. This keeps a `lazy` firewall lazy: the user provider reads the security token, which authenticates the user and makes the response private (not HTTP-cacheable). Pages without flags stay cacheable. + +- `EvaluationContextContributedEvent` is dispatched at that time, not on `kernel.request`. +- An exception thrown by a context provider is logged (`error` level) and the provider is skipped. Evaluation goes on with the other contexts. An exception thrown by a listener of `EvaluationContextContributedEvent` is logged too, and the provider's context is kept. +- A flag evaluated from inside a context provider gets an empty context. +- Reading the targeting key or attributes of `API::getEvaluationContext()` also runs the providers. + +Logic that must run at the start of every request belongs in its own `kernel.request` listener, not in a context provider. ## FrankenPHP worker mode diff --git a/docs/features/feature-gate.md b/docs/features/feature-gate.md index 6819e45..274ea1b 100644 --- a/docs/features/feature-gate.md +++ b/docs/features/feature-gate.md @@ -14,7 +14,9 @@ public function checkout(): Response } ``` -When the flag evaluates to `false`, an exception is thrown (403 by default). The exception message includes the flag name for easier debugging. +When the flag evaluates to `false`, access is denied with an exception whose message includes the flag name. The response depends on `on_disabled` (see below): with SecurityBundle, the firewall returns a 403 to an authenticated user and starts authentication for an anonymous one (redirect to the login page, or 401). Without SecurityBundle, the response is a 403 by default. + +The flag is evaluated with `false` as default value: if it cannot be evaluated (unknown flag, provider error), the gate stays closed. ## Stacking multiple gates @@ -35,7 +37,7 @@ The exception type is auto-detected: | `on_disabled` | Exception | |---|---| -| `auto` (default) | `AccessDeniedException` if `symfony/security-core` is installed, `HttpException` otherwise | +| `auto` (default) | `AccessDeniedException` if SecurityBundle is enabled, `HttpException` otherwise | | `access_denied` | `AccessDeniedException` (always) | | `http_exception` | `HttpException` (always, with configurable status code) | diff --git a/docs/profiler.md b/docs/profiler.md index 71c52fe..90da890 100644 --- a/docs/profiler.md +++ b/docs/profiler.md @@ -8,6 +8,8 @@ The bundle registers an **OpenFeature panel** in the Symfony Web Debug Toolbar s - In multi-provider mode: the evaluation strategy, the fallback marker, and the sub-providers in evaluation order - All flags evaluated during the request (key, type, resolved value, reason, error) - Global EvaluationContext (targeting key and attributes) +- Context providers that contributed to it. They only run on the first flag evaluation, so the panel reports that they did not run when the request evaluated no flag +- Registered hooks (the profiler's own hook is hidden) The profiler panel is automatically enabled in `debug` mode. No configuration needed. @@ -26,11 +28,29 @@ The command scans routes for `#[FeatureFlag]` and `#[FeatureGate]` attributes an Example output: ``` - -------------- ------ ------- ----------- - Flag Type Value Attribute - -------------- ------ ------- ----------- - new_checkout bool true FeatureGate - dark_mode bool false FeatureFlag - max_items int 10 FeatureFlag - -------------- ------ ------- ----------- +Provider +-------- + + MultiProvider + +Feature flags +------------- + + -------------- ------------- -------- ------- ---------------------------------------------- + Flag Attribute Type Value Used in + -------------- ------------- -------- ------- ---------------------------------------------- + dark_mode FeatureGate bool false App\Controller\DemoController::darkModeOnly + max_items FeatureFlag int 10 App\Controller\DemoController::valueResolver + new_checkout FeatureFlag bool true App\Controller\DemoController::valueResolver + -------------- ------------- -------- ------- ---------------------------------------------- + +Evaluation context +------------------ + + (none) + +Hooks +----- + + App\OpenFeature\LoggerHook ``` diff --git a/src/EvaluationContext/EvaluationContextProviderInterface.php b/src/EvaluationContext/EvaluationContextProviderInterface.php index 0872dcf..9f26f18 100644 --- a/src/EvaluationContext/EvaluationContextProviderInterface.php +++ b/src/EvaluationContext/EvaluationContextProviderInterface.php @@ -9,7 +9,12 @@ /** * Implement this interface to contribute attributes to the global OpenFeature - * EvaluationContext on each request. + * EvaluationContext of each request. + * + * Providers run on the first flag evaluation of the request, not on kernel.request: + * a request that evaluates no flag never calls them. An exception thrown by a provider + * is logged and the provider skipped. A flag evaluated from inside getContext() gets + * an empty context. * * Multiple providers are supported. They are iterated highest priority first * (set the "priority" attribute on the tag), then their contexts are merged by the diff --git a/src/EvaluationContext/LazyEvaluationContext.php b/src/EvaluationContext/LazyEvaluationContext.php new file mode 100644 index 0000000..4a01b57 --- /dev/null +++ b/src/EvaluationContext/LazyEvaluationContext.php @@ -0,0 +1,60 @@ +resolve()->getTargetingKey(); + } + + public function getAttributes(): Attributes + { + return $this->resolve()->getAttributes(); + } + + public function isResolved(): bool + { + return $this->resolved !== null; + } + + private function resolve(): EvaluationContext + { + if ($this->resolved !== null) { + return $this->resolved; + } + + // A flag evaluated during resolution (e.g. by a context provider) gets an empty context instead of recursing + if ($this->resolving) { + return new MutableEvaluationContext(); + } + + $this->resolving = true; + + try { + return $this->resolved = ($this->resolver)(); + } finally { + $this->resolving = false; + } + } +} diff --git a/src/EvaluationContext/UserEvaluationContextProvider.php b/src/EvaluationContext/UserEvaluationContextProvider.php index b88bfe0..2f02445 100644 --- a/src/EvaluationContext/UserEvaluationContextProvider.php +++ b/src/EvaluationContext/UserEvaluationContextProvider.php @@ -11,13 +11,13 @@ class UserEvaluationContextProvider implements EvaluationContextProviderInterface { - public function __construct(private readonly TokenStorageInterface $tokenStorage) + public function __construct(private readonly ?TokenStorageInterface $tokenStorage) { } public function getContext(Request $request): ?EvaluationContext { - $token = $this->tokenStorage->getToken(); + $token = $this->tokenStorage?->getToken(); if ($token === null) { return null; diff --git a/src/EventListener/EvaluationContextListener.php b/src/EventListener/EvaluationContextListener.php index 8889882..d8809b8 100644 --- a/src/EventListener/EvaluationContextListener.php +++ b/src/EventListener/EvaluationContextListener.php @@ -5,11 +5,15 @@ namespace Aubes\OpenFeatureBundle\EventListener; use Aubes\OpenFeatureBundle\EvaluationContext\EvaluationContextProviderInterface; +use Aubes\OpenFeatureBundle\EvaluationContext\LazyEvaluationContext; use Aubes\OpenFeatureBundle\Event\EvaluationContextContributedEvent; use OpenFeature\implementation\flags\EvaluationContext; use OpenFeature\implementation\flags\MutableEvaluationContext; use OpenFeature\interfaces\flags\API; +use OpenFeature\interfaces\flags\EvaluationContext as EvaluationContextInterface; use Psr\EventDispatcher\EventDispatcherInterface; +use Psr\Log\LoggerInterface; +use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpKernel\Event\RequestEvent; use Symfony\Contracts\Service\ResetInterface; @@ -20,6 +24,7 @@ public function __construct( private readonly API $api, private readonly iterable $providers = [], private readonly ?EventDispatcherInterface $dispatcher = null, + private readonly ?LoggerInterface $logger = null, ) { } @@ -29,31 +34,59 @@ public function onKernelRequest(RequestEvent $event): void return; } + $request = $event->getRequest(); + $previous = $this->api->getEvaluationContext(); + + // Providers run on the first flag evaluation, so requests without flags never trigger them (e.g. a lazy firewall) + $this->api->setEvaluationContext(new LazyEvaluationContext( + fn (): EvaluationContextInterface => $this->collect($request) ?? $previous ?? EvaluationContext::createNull(), + )); + } + + /** + * Clears the API-level evaluation context between requests. + * Required under any long-running runtime (FrankenPHP worker, Messenger) + * where the API service survives across requests. + */ + public function reset(): void + { + $this->api->setEvaluationContext(new MutableEvaluationContext()); + } + + private function collect(Request $request): ?EvaluationContextInterface + { $contexts = []; foreach ($this->providers as $provider) { - $context = $provider->getContext($event->getRequest()); + try { + $context = $provider->getContext($request); + } catch (\Throwable $e) { + // Runs inside flag evaluation, which must not throw + $this->logger?->error('OpenFeature evaluation context provider "{provider}" failed: {message}', [ + 'provider' => $provider::class, + 'message' => $e->getMessage(), + 'exception' => $e, + ]); + + continue; + } + if ($context === null) { continue; } $contexts[] = $context; - $this->dispatcher?->dispatch(new EvaluationContextContributedEvent($provider, $context)); - } - if ($contexts === []) { - return; + try { + $this->dispatcher?->dispatch(new EvaluationContextContributedEvent($provider, $context)); + } catch (\Throwable $e) { + $this->logger?->error('OpenFeature listener of "{event}" failed: {message}', [ + 'event' => EvaluationContextContributedEvent::class, + 'message' => $e->getMessage(), + 'exception' => $e, + ]); + } } - $this->api->setEvaluationContext(EvaluationContext::merge(...$contexts)); - } - - /** - * Clears the OpenFeature global evaluation context between requests. - * Required under any long-running runtime (FrankenPHP worker, Messenger) - * where the SDK singleton survives across requests. - */ - public function reset(): void - { - $this->api->setEvaluationContext(new MutableEvaluationContext()); + return $contexts === [] ? null : EvaluationContext::merge(...$contexts); } } diff --git a/src/OpenFeatureBundle.php b/src/OpenFeatureBundle.php index ff52ce1..8ac3b1d 100644 --- a/src/OpenFeatureBundle.php +++ b/src/OpenFeatureBundle.php @@ -19,6 +19,7 @@ use Symfony\Component\Config\Definition\Exception\InvalidConfigurationException; use Symfony\Component\Config\FileLocator; use Symfony\Component\DependencyInjection\ContainerBuilder; +use Symfony\Component\DependencyInjection\ContainerInterface; use Symfony\Component\DependencyInjection\Definition; use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator; use Symfony\Component\DependencyInjection\Loader\PhpFileLoader; @@ -86,7 +87,7 @@ public function configure(DefinitionConfigurator $definition): void ->addDefaultsIfNotSet() ->children() ->enumNode('user_provider') - ->info('Populate EvaluationContext targeting key from the authenticated Symfony user. "auto" enables it if symfony/security-core is available.') + ->info('Populate EvaluationContext targeting key from the authenticated Symfony user. "auto" enables it if SecurityBundle is enabled.') ->beforeNormalization() ->ifTrue(\is_bool(...)) ->then(static fn (bool $v): string => $v ? 'true' : 'false') @@ -100,7 +101,7 @@ public function configure(DefinitionConfigurator $definition): void ->addDefaultsIfNotSet() ->children() ->enumNode('on_disabled') - ->info('Exception thrown when a #[FeatureFlag] method-level flag is disabled. "auto" detects symfony/security-core availability.') + ->info('Exception thrown when a #[FeatureGate] flag is disabled. "auto" uses "access_denied" if SecurityBundle is enabled, "http_exception" otherwise.') ->values(['auto', 'access_denied', 'http_exception']) ->defaultValue('auto') ->end() @@ -193,13 +194,18 @@ public function loadExtension(array $config, ContainerConfigurator $container, C $builder->setDefinition(Provider\RedisProvider::class, $definition); } + // Services from other bundles are not visible here (isolated merge container), only parameters are + /** @var array $bundles */ + $bundles = $builder->getParameter('kernel.bundles'); + $hasSecurityBundle = isset($bundles['SecurityBundle']); + $onDisabled = $config['feature_flag']['on_disabled']; - $hasSecurityCore = \class_exists(\Symfony\Component\Security\Core\Exception\AccessDeniedException::class); if ($onDisabled === 'auto') { - $onDisabled = $hasSecurityCore + // Only the SecurityBundle firewall turns an AccessDeniedException into a 403 (or a login redirect) + $onDisabled = $hasSecurityBundle ? 'access_denied' : 'http_exception'; - } elseif ($onDisabled === 'access_denied' && !$hasSecurityCore) { + } elseif ($onDisabled === 'access_denied' && !\class_exists(\Symfony\Component\Security\Core\Exception\AccessDeniedException::class)) { throw new \LogicException('Setting "on_disabled" to "access_denied" requires symfony/security-core. Install it or use "http_exception".'); } @@ -207,19 +213,19 @@ public function loadExtension(array $config, ContainerConfigurator $container, C $builder->setParameter('open_feature.feature_flag.status_code', $config['feature_flag']['status_code']); $userProvider = $config['evaluation_context']['user_provider']; - $hasTokenStorage = $builder->has('security.token_storage'); if ($userProvider === 'auto') { - $userProvider = $hasTokenStorage + $userProvider = $hasSecurityBundle ? 'true' : 'false'; - } elseif ($userProvider === 'true' && !$hasTokenStorage) { + } elseif ($userProvider === 'true' && !$hasSecurityBundle) { throw new \LogicException('Setting "user_provider" to "true" requires symfony/security-bundle to be enabled. Install and enable it or use "false".'); } $builder->setParameter('open_feature.evaluation_context.user_provider', $userProvider); if ($userProvider === 'true') { $definition = new Definition(UserEvaluationContextProvider::class); - $definition->addArgument(new Reference('security.token_storage')); + // Symfony 6.4 registers no security service when SecurityBundle is enabled but not configured + $definition->addArgument(new Reference('security.token_storage', ContainerInterface::NULL_ON_INVALID_REFERENCE)); $definition->addTag('openfeature.evaluation_context_provider', ['priority' => 0]); $builder->setDefinition(UserEvaluationContextProvider::class, $definition); } diff --git a/src/Profiler/OpenFeatureDataCollector.php b/src/Profiler/OpenFeatureDataCollector.php index 2a433e3..d4c816f 100644 --- a/src/Profiler/OpenFeatureDataCollector.php +++ b/src/Profiler/OpenFeatureDataCollector.php @@ -4,7 +4,9 @@ namespace Aubes\OpenFeatureBundle\Profiler; +use Aubes\OpenFeatureBundle\EvaluationContext\LazyEvaluationContext; use OpenFeature\interfaces\flags\API; +use OpenFeature\interfaces\flags\EvaluationContext; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; use Symfony\Component\HttpKernel\DataCollector\DataCollector; @@ -26,12 +28,17 @@ public function __construct( public function collect(Request $request, Response $response, ?\Throwable $exception = null): void { + $context = $this->api->getEvaluationContext(); + // Reading an unresolved lazy context would run the context providers on every profiled request + $resolved = !$context instanceof LazyEvaluationContext || $context->isResolved(); + $this->data = [ 'evaluations' => $this->hook->getEvaluations(), 'provider' => $this->api->getProviderMetadata()->getName(), 'providers' => $this->providers, 'strategy' => $this->providers === [] ? null : $this->strategy, - 'evaluation_context' => $this->serializeContext(), + 'evaluation_context' => $resolved ? $this->serializeContext($context) : [], + 'evaluation_context_resolved' => $resolved, 'hooks' => $this->collectHooks(), 'context_providers' => $this->collectContextProviders(), ]; @@ -86,6 +93,14 @@ public function getEvaluationContext(): array return $context; } + public function isEvaluationContextResolved(): bool + { + /** @var bool $resolved */ + $resolved = $this->data['evaluation_context_resolved'] ?? true; + + return $resolved; + } + /** @return list */ public function getHooks(): array { @@ -124,10 +139,8 @@ private function anonymizeTargetingKey(?string $key): ?string } /** @return array */ - private function serializeContext(): array + private function serializeContext(?EvaluationContext $context): array { - $context = $this->api->getEvaluationContext(); - if ($context === null) { return []; } diff --git a/src/Resources/config/services.php b/src/Resources/config/services.php index a06f9d5..153fbff 100644 --- a/src/Resources/config/services.php +++ b/src/Resources/config/services.php @@ -37,6 +37,7 @@ service(API::class), tagged_iterator('openfeature.evaluation_context_provider'), service('event_dispatcher')->nullOnInvalid(), + service('logger')->nullOnInvalid(), ]) ->tag('kernel.event_listener', ['event' => 'kernel.request', 'method' => 'onKernelRequest', 'priority' => 4]) ->tag('kernel.reset', ['method' => 'reset']); diff --git a/src/Resources/views/Collector/openfeature.html.twig b/src/Resources/views/Collector/openfeature.html.twig index 0a2b8d2..24b1aee 100644 --- a/src/Resources/views/Collector/openfeature.html.twig +++ b/src/Resources/views/Collector/openfeature.html.twig @@ -124,7 +124,11 @@

Context Providers

- {% if collector.contextProviders is empty %} + {% if not collector.evaluationContextResolved %} +
+

Context providers did not run: no flag was evaluated during this request.

+
+ {% elseif collector.contextProviders is empty %}

No context provider contributed during this request.

diff --git a/tests/DependencyInjection/OpenFeatureBundleTest.php b/tests/DependencyInjection/OpenFeatureBundleTest.php new file mode 100644 index 0000000..65cebcb --- /dev/null +++ b/tests/DependencyInjection/OpenFeatureBundleTest.php @@ -0,0 +1,343 @@ + 'Symfony\Bundle\SecurityBundle\SecurityBundle']; + + /** + * Loads the extension like a kernel does: in the isolated container of MergeExtensionConfigurationPass, where only parameters are visible. + * + * @param array $config + * @param array $bundles + */ + private function createContainer(array $config = [], bool $debug = false, array $bundles = []): ContainerBuilder + { + $container = new ContainerBuilder(new ParameterBag([ + 'kernel.debug' => $debug, + 'kernel.environment' => $debug ? 'dev' : 'prod', + 'kernel.project_dir' => __DIR__, + 'kernel.build_dir' => __DIR__ . '/cache', + 'kernel.cache_dir' => __DIR__ . '/cache', + 'kernel.bundles' => $bundles, + 'kernel.bundles_metadata' => [], + ])); + + $bundle = new OpenFeatureBundle(); + $bundle->build($container); + $extension = $bundle->getContainerExtension(); + self::assertNotNull($extension); + $container->registerExtension($extension); + $container->loadFromExtension('open_feature', $config); + + (new MergeExtensionConfigurationPass())->process($container); + // Already merged above: merging again on compile() would load the extension twice + $container->getCompilerPassConfig()->setMergePass(new class extends MergeExtensionConfigurationPass { + public function process(ContainerBuilder $container): void + { + } + }); + + return $container; + } + + /** @param array $config */ + private function buildContainer(array $config = [], bool $debug = false): ContainerBuilder + { + $container = $this->createContainer($config, $debug); + $container->compile(); + + return $container; + } + + public function testRegistersTheCoreServices(): void + { + $container = $this->createContainer(); + + $this->assertTrue($container->hasDefinition(API::class)); + $this->assertTrue($container->hasDefinition(Client::class)); + $this->assertTrue($container->hasDefinition(InMemoryProvider::class)); + $this->assertTrue($container->hasDefinition(EvaluationContextListener::class)); + $this->assertTrue($container->hasDefinition(FeatureGateListener::class)); + $this->assertTrue($container->hasDefinition(FeatureFlagValueResolver::class)); + $this->assertTrue($container->hasDefinition(OpenFeatureExtension::class)); + + $listenerDef = $container->getDefinition(EvaluationContextListener::class); + $this->assertTrue($listenerDef->hasTag('kernel.reset')); + } + + public function testDefaultProviderIsInMemory(): void + { + $container = $this->buildContainer(); + + $this->assertSame(InMemoryProvider::class, $container->getParameter('open_feature.provider')); + } + + public function testFlagsParameter(): void + { + $container = $this->buildContainer([ + 'flags' => ['dark_mode' => true, 'max_items' => 10], + ]); + + $this->assertSame( + ['dark_mode' => true, 'max_items' => 10], + $container->getParameter('open_feature.flags'), + ); + } + + public function testCustomProvider(): void + { + $container = $this->createContainer(['provider' => 'app.custom_provider']); + $container->register('app.custom_provider', InMemoryProvider::class); + $container->compile(); + + $this->assertSame('app.custom_provider', $container->getParameter('open_feature.provider')); + } + + public function testCompilationValidatesTheProvider(): void + { + $this->expectException(\InvalidArgumentException::class); + $this->expectExceptionMessage('OpenFeature provider service "app.bad_provider" (class "stdClass") must implement interface "OpenFeature\interfaces\provider\Provider".'); + + $container = $this->createContainer(['provider' => 'app.bad_provider']); + $container->register('app.bad_provider', \stdClass::class); + $container->compile(); + } + + public function testProfilerRegisteredInDebugMode(): void + { + $container = $this->createContainer([], debug: true); + + $this->assertTrue($container->hasDefinition(ProfilerHook::class)); + $this->assertTrue($container->hasDefinition(OpenFeatureDataCollector::class)); + $this->assertTrue($container->getDefinition(ContextProviderRecorder::class)->hasTag('kernel.reset')); + } + + public function testProfilerNotRegisteredInProdMode(): void + { + $container = $this->createContainer([], debug: false); + + $this->assertFalse($container->hasDefinition(ProfilerHook::class)); + $this->assertFalse($container->hasDefinition(OpenFeatureDataCollector::class)); + } + + public function testFeatureFlagOnDisabledAutoWithSecurityBundle(): void + { + $container = $this->createContainer(bundles: self::SECURITY_BUNDLE); + + $this->assertSame('access_denied', $container->getParameter('open_feature.feature_flag.on_disabled')); + } + + // symfony/security-core is installed (dev dependency), but only the SecurityBundle firewall turns the exception into a 403 + public function testFeatureFlagOnDisabledAutoWithoutSecurityBundle(): void + { + $container = $this->createContainer(); + + $this->assertSame('http_exception', $container->getParameter('open_feature.feature_flag.on_disabled')); + } + + public function testFeatureFlagOnDisabledHttpException(): void + { + $container = $this->buildContainer([ + 'feature_flag' => ['on_disabled' => 'http_exception', 'status_code' => 404], + ]); + + $this->assertSame('http_exception', $container->getParameter('open_feature.feature_flag.on_disabled')); + $this->assertSame(404, $container->getParameter('open_feature.feature_flag.status_code')); + } + + public function testUserProviderAutoEnabledWithSecurityBundle(): void + { + $container = $this->createContainer(bundles: self::SECURITY_BUNDLE); + + $this->assertSame('true', $container->getParameter('open_feature.evaluation_context.user_provider')); + + $definition = $container->getDefinition(UserEvaluationContextProvider::class); + $this->assertEquals([new Reference('security.token_storage', ContainerInterface::NULL_ON_INVALID_REFERENCE)], $definition->getArguments()); + $this->assertSame([['priority' => 0]], $definition->getTag('openfeature.evaluation_context_provider')); + } + + public function testUserProviderToleratesSecurityBundleWithoutTokenStorage(): void + { + // Symfony 6.4 registers no security service when SecurityBundle is enabled but not configured + $container = $this->createContainer(bundles: self::SECURITY_BUNDLE); + (new CheckExceptionOnInvalidReferenceBehaviorPass())->process($container); + + $provider = $container->get(UserEvaluationContextProvider::class); + $this->assertInstanceOf(UserEvaluationContextProvider::class, $provider); + $this->assertNull($provider->getContext(Request::create('/'))); + } + + public function testUserProviderAutoDisabledWithoutSecurityBundle(): void + { + $container = $this->createContainer(); + + $this->assertSame('false', $container->getParameter('open_feature.evaluation_context.user_provider')); + $this->assertFalse($container->hasDefinition(UserEvaluationContextProvider::class)); + } + + public function testUserProviderExplicitlyEnabledWithSecurityBundle(): void + { + $container = $this->createContainer(['evaluation_context' => ['user_provider' => true]], bundles: self::SECURITY_BUNDLE); + + $this->assertTrue($container->hasDefinition(UserEvaluationContextProvider::class)); + } + + public function testUserProviderExplicitlyEnabledWithoutSecurityBundleThrows(): void + { + $this->expectException(\LogicException::class); + $this->expectExceptionMessage('Setting "user_provider" to "true" requires symfony/security-bundle to be enabled. Install and enable it or use "false".'); + + $this->createContainer(['evaluation_context' => ['user_provider' => true]]); + } + + public function testUserProviderDisabled(): void + { + $container = $this->buildContainer([ + 'evaluation_context' => ['user_provider' => 'false'], + ]); + + $this->assertSame('false', $container->getParameter('open_feature.evaluation_context.user_provider')); + } + + public function testDebugCommandRegistered(): void + { + $container = $this->createContainer(debug: true); + + $this->assertTrue($container->hasDefinition(DebugFeatureFlagsCommand::class)); + } + + public function testDebugCommandNotRegisteredInProd(): void + { + $container = $this->createContainer(debug: false); + + $this->assertFalse($container->hasDefinition(DebugFeatureFlagsCommand::class)); + } + + public function testDataCollectorReceivesProvidersAndStrategy(): void + { + $container = $this->createContainer([ + 'providers' => ['local' => InMemoryProvider::class], + 'strategy' => 'first_successful', + ], debug: true); + + $definition = $container->getDefinition(OpenFeatureDataCollector::class); + $this->assertSame(['local' => InMemoryProvider::class], $definition->getArgument(3)); + $this->assertSame(['type' => 'first_successful', 'fallback' => null], $definition->getArgument(4)); + } + + public function testDataCollectorStrategyIsNullInSingleProviderMode(): void + { + $container = $this->createContainer([], debug: true); + + $definition = $container->getDefinition(OpenFeatureDataCollector::class); + $this->assertSame([], $definition->getArgument(3)); + $this->assertNull($definition->getArgument(4)); + } + + public function testProviderAndProvidersAreMutuallyExclusive(): void + { + $this->expectException(InvalidConfigurationException::class); + $this->expectExceptionMessage('Invalid "open_feature" configuration: "provider" and "providers" are mutually exclusive.'); + + $this->createContainer([ + 'provider' => 'app.custom_provider', + 'providers' => ['local' => InMemoryProvider::class], + ]); + } + + public function testStrategyRequiresProviders(): void + { + $this->expectException(InvalidConfigurationException::class); + $this->expectExceptionMessage('Invalid "open_feature" configuration: "strategy" requires "providers".'); + + $this->createContainer(['strategy' => 'first_successful']); + } + + public function testComparisonStrategyRequiresFallback(): void + { + $this->expectException(InvalidConfigurationException::class); + $this->expectExceptionMessage('Invalid "open_feature" configuration: the "comparison" strategy requires "strategy.fallback".'); + + $this->createContainer([ + 'providers' => ['local' => InMemoryProvider::class], + 'strategy' => 'comparison', + ]); + } + + public function testFallbackOnlyAllowedForComparison(): void + { + $this->expectException(InvalidConfigurationException::class); + $this->expectExceptionMessage('Invalid "open_feature" configuration: "strategy.fallback" is only allowed when "strategy.type" is "comparison".'); + + $this->createContainer([ + 'providers' => ['local' => InMemoryProvider::class], + 'strategy' => ['type' => 'first_match', 'fallback' => 'local'], + ]); + } + + public function testFallbackMustBeADeclaredProviderName(): void + { + $this->expectException(InvalidConfigurationException::class); + $this->expectExceptionMessage('Invalid "open_feature" configuration: "strategy.fallback" must be one of the names declared in "providers".'); + + $this->createContainer([ + 'providers' => ['local' => InMemoryProvider::class], + 'strategy' => ['type' => 'comparison', 'fallback' => 'unknown'], + ]); + } + + public function testProviderNamesMustBeUniqueCaseInsensitive(): void + { + $this->expectException(InvalidConfigurationException::class); + $this->expectExceptionMessage('Invalid "open_feature" configuration: provider names in "providers" must be unique (case-insensitive, the SDK normalizes them to lowercase).'); + + $this->createContainer([ + 'providers' => [ + 'Local' => InMemoryProvider::class, + 'local' => 'app.custom_provider', + ], + ]); + } + + public function testHookInterfaceIsAutoconfigured(): void + { + $container = $this->createContainer(); + $container->register('app.custom_hook', ContextRecordingHook::class) + ->setAutoconfigured(true) + ->setPublic(true); + $container->compile(); + + $definition = $container->getDefinition('app.custom_hook'); + $this->assertTrue($definition->hasTag('openfeature.hook')); + } +} diff --git a/tests/EvaluationContext/LazyEvaluationContextTest.php b/tests/EvaluationContext/LazyEvaluationContextTest.php new file mode 100644 index 0000000..ae37f0f --- /dev/null +++ b/tests/EvaluationContext/LazyEvaluationContextTest.php @@ -0,0 +1,130 @@ + $this->fail('The resolver must not run before the first read')); + + $this->assertFalse($context->isResolved()); + } + + public function testResolvesOnFirstReadOnly(): void + { + $calls = 0; + $context = new LazyEvaluationContext(static function () use (&$calls): EvaluationContext { + ++$calls; + + return new MutableEvaluationContext('user-1', new MutableAttributes(['plan' => 'premium'])); + }); + + $this->assertSame('user-1', $context->getTargetingKey()); + $this->assertSame(['plan' => 'premium'], $context->getAttributes()->toArray()); + $this->assertTrue($context->isResolved()); + $this->assertSame(1, $calls); + } + + // The SDK must read the API-level context at evaluation time, not when it is set + public function testSdkDoesNotResolveContextWithoutEvaluation(): void + { + $context = new LazyEvaluationContext(static fn (): EvaluationContext => new MutableEvaluationContext('user-1')); + + $api = $this->makeApi(new ContextRecordingHook()); + $api->setEvaluationContext($context); + $api->getClient(); + + $this->assertFalse($context->isResolved()); + } + + public function testSdkEvaluationSeesResolvedContext(): void + { + $hook = new ContextRecordingHook(); + $api = $this->makeApi($hook); + $api->setEvaluationContext(new LazyEvaluationContext(static fn (): EvaluationContext => new MutableEvaluationContext('user-1'))); + + $this->assertTrue($api->getClient()->getBooleanValue('enabled', false)); + $this->assertSame('user-1', $hook->context?->getTargetingKey()); + } + + public function testSdkEvaluationMergesResolvedContextWithInvocationContext(): void + { + $hook = new ContextRecordingHook(); + $api = $this->makeApi($hook); + $api->setEvaluationContext(new LazyEvaluationContext(static fn (): EvaluationContext => new MutableEvaluationContext('user-1'))); + + $api->getClient()->getBooleanValue('enabled', false, new MutableEvaluationContext(null, new MutableAttributes(['page' => 'home']))); + + $this->assertNotNull($hook->context); + $this->assertSame('user-1', $hook->context->getTargetingKey()); + $this->assertSame('home', $hook->context->getAttributes()->get('page')); + } + + public function testFlagEvaluatedDuringResolutionGetsAnEmptyContext(): void + { + $hook = new ContextRecordingHook(); + $api = $this->makeApi($hook); + + $calls = 0; + $innerDetails = null; + $innerContext = null; + $api->setEvaluationContext(new LazyEvaluationContext(static function () use ($api, $hook, &$calls, &$innerDetails, &$innerContext): EvaluationContext { + ++$calls; + $innerDetails = $api->getClient()->getBooleanDetails('enabled', false); + $innerContext = $hook->context; + + return new MutableEvaluationContext('user-1'); + })); + + $api->getClient()->getBooleanValue('enabled', false); + + $this->assertSame(1, $calls); + $this->assertNull($innerDetails?->getError()); + $this->assertNotNull($innerContext); + $this->assertNull($innerContext->getTargetingKey()); + $this->assertSame([], $innerContext->getAttributes()->toArray()); + $this->assertSame('user-1', $hook->context?->getTargetingKey()); + } + + public function testRetriesResolutionAfterAFailure(): void + { + $calls = 0; + $context = new LazyEvaluationContext(static function () use (&$calls): EvaluationContext { + ++$calls; + + return $calls === 1 ? throw new \RuntimeException('boom') : new MutableEvaluationContext('user-1'); + }); + + try { + $context->getTargetingKey(); + $this->fail('The resolver exception must propagate'); + } catch (\RuntimeException) { + } + + $this->assertFalse($context->isResolved()); + $this->assertSame('user-1', $context->getTargetingKey()); + } + + private function makeApi(ContextRecordingHook $hook): OpenFeatureAPI + { + $api = new OpenFeatureAPI(); + $api->setProvider(new InMemoryProvider(['enabled' => true])); + $api->addHooks($hook); + + return $api; + } +} diff --git a/tests/EvaluationContext/UserEvaluationContextProviderTest.php b/tests/EvaluationContext/UserEvaluationContextProviderTest.php index 57e6348..4be922d 100644 --- a/tests/EvaluationContext/UserEvaluationContextProviderTest.php +++ b/tests/EvaluationContext/UserEvaluationContextProviderTest.php @@ -15,6 +15,13 @@ #[CoversClass(UserEvaluationContextProvider::class)] class UserEvaluationContextProviderTest extends TestCase { + public function testReturnsNullWithoutTokenStorage(): void + { + $provider = new UserEvaluationContextProvider(null); + + $this->assertNull($provider->getContext(Request::create('/'))); + } + public function testReturnsNullWhenNoToken(): void { $tokenStorage = $this->createStub(TokenStorageInterface::class); diff --git a/tests/EventListener/EvaluationContextListenerLazinessTest.php b/tests/EventListener/EvaluationContextListenerLazinessTest.php new file mode 100644 index 0000000..719d78c --- /dev/null +++ b/tests/EventListener/EvaluationContextListenerLazinessTest.php @@ -0,0 +1,181 @@ +createMock(EvaluationContextProviderInterface::class); + $provider->expects($this->never())->method('getContext'); + + $api = new OpenFeatureAPI(); + (new EvaluationContextListener($api, [$provider]))->onKernelRequest($this->makeEvent()); + + $this->assertInstanceOf(LazyEvaluationContext::class, $api->getEvaluationContext()); + } + + public function testIgnoresSubRequests(): void + { + $api = new OpenFeatureAPI(); + $listener = new EvaluationContextListener($api, [$this->providerReturning(new MutableEvaluationContext('user-1'))]); + $listener->onKernelRequest($this->makeEvent(HttpKernelInterface::SUB_REQUEST)); + + $this->assertNotInstanceOf(LazyEvaluationContext::class, $api->getEvaluationContext()); + } + + public function testRunsProvidersOnceOnFirstRead(): void + { + $provider = $this->createMock(EvaluationContextProviderInterface::class); + $provider->expects($this->once())->method('getContext')->willReturn(new MutableEvaluationContext('user-1')); + + $api = new OpenFeatureAPI(); + (new EvaluationContextListener($api, [$provider]))->onKernelRequest($this->makeEvent()); + + $context = $api->getEvaluationContext(); + $this->assertNotNull($context); + $this->assertSame('user-1', $context->getTargetingKey()); + $this->assertSame('user-1', $context->getTargetingKey()); + } + + public function testMergesContextsFromMultipleProviders(): void + { + $api = new OpenFeatureAPI(); + $listener = new EvaluationContextListener($api, [ + $this->providerReturning(new MutableEvaluationContext('user-1', new MutableAttributes(['plan' => 'free', 'region' => 'eu']))), + $this->providerReturning(null), + $this->providerReturning(new MutableEvaluationContext('user-2', new MutableAttributes(['plan' => 'premium']))), + ]); + $listener->onKernelRequest($this->makeEvent()); + + $context = $api->getEvaluationContext(); + $this->assertNotNull($context); + $this->assertSame('user-2', $context->getTargetingKey()); + $this->assertSame(['plan' => 'premium', 'region' => 'eu'], $context->getAttributes()->toArray()); + } + + public function testKeepsPreviousContextWhenNoProviderContributes(): void + { + $api = new OpenFeatureAPI(); + $api->setEvaluationContext(new MutableEvaluationContext('boot')); + + (new EvaluationContextListener($api, [$this->providerReturning(null)]))->onKernelRequest($this->makeEvent()); + + $this->assertSame('boot', $api->getEvaluationContext()?->getTargetingKey()); + } + + public function testFallsBackToAnEmptyContextWhenNothingContributes(): void + { + $api = new OpenFeatureAPI(); + (new EvaluationContextListener($api, [$this->providerReturning(null)]))->onKernelRequest($this->makeEvent()); + + $context = $api->getEvaluationContext(); + $this->assertNotNull($context); + $this->assertNull($context->getTargetingKey()); + $this->assertSame([], $context->getAttributes()->toArray()); + } + + public function testDispatchesContributedEventsOnResolution(): void + { + /** @var \ArrayObject $dispatched */ + $dispatched = new \ArrayObject(); + $dispatcher = $this->createStub(EventDispatcherInterface::class); + $dispatcher->method('dispatch')->willReturnCallback(static function (object $event) use ($dispatched): object { + $dispatched->append($event); + + return $event; + }); + + $api = new OpenFeatureAPI(); + $listener = new EvaluationContextListener($api, [$this->providerReturning(new MutableEvaluationContext('user-1'))], $dispatcher); + $listener->onKernelRequest($this->makeEvent()); + + $this->assertCount(0, $dispatched); + + $api->getEvaluationContext()?->getTargetingKey(); + + $this->assertCount(1, $dispatched); + $this->assertInstanceOf(EvaluationContextContributedEvent::class, $dispatched[0]); + } + + public function testLogsAndSkipsFailingProvider(): void + { + $failing = $this->createStub(EvaluationContextProviderInterface::class); + $failing->method('getContext')->willThrowException(new \RuntimeException('boom')); + + $logger = $this->createMock(LoggerInterface::class); + $logger->expects($this->once()) + ->method('error') + ->with($this->anything(), $this->callback(static fn (array $context): bool => $context['message'] === 'boom')); + + $api = new OpenFeatureAPI(); + $listener = new EvaluationContextListener($api, [$failing, $this->providerReturning(new MutableEvaluationContext('user-1'))], null, $logger); + $listener->onKernelRequest($this->makeEvent()); + + $this->assertSame('user-1', $api->getEvaluationContext()?->getTargetingKey()); + } + + public function testLogsAFailingContributedEventListener(): void + { + $dispatcher = $this->createStub(EventDispatcherInterface::class); + $dispatcher->method('dispatch')->willThrowException(new \RuntimeException('boom')); + + $logger = $this->createMock(LoggerInterface::class); + $logger->expects($this->once()) + ->method('error') + ->with($this->anything(), $this->callback(static fn (array $context): bool => $context['message'] === 'boom')); + + $api = new OpenFeatureAPI(); + $listener = new EvaluationContextListener($api, [$this->providerReturning(new MutableEvaluationContext('user-1'))], $dispatcher, $logger); + $listener->onKernelRequest($this->makeEvent()); + + $this->assertSame('user-1', $api->getEvaluationContext()?->getTargetingKey()); + } + + public function testResetDropsPendingContextWithoutResolvingIt(): void + { + $provider = $this->createMock(EvaluationContextProviderInterface::class); + $provider->expects($this->never())->method('getContext'); + + $api = new OpenFeatureAPI(); + $listener = new EvaluationContextListener($api, [$provider]); + $listener->onKernelRequest($this->makeEvent()); + $listener->reset(); + + $context = $api->getEvaluationContext(); + $this->assertNotInstanceOf(LazyEvaluationContext::class, $context); + $this->assertNull($context?->getTargetingKey()); + } + + private function makeEvent(int $requestType = HttpKernelInterface::MAIN_REQUEST): RequestEvent + { + return new RequestEvent($this->createStub(HttpKernelInterface::class), Request::create('/'), $requestType); + } + + private function providerReturning(?EvaluationContext $context): EvaluationContextProviderInterface + { + $provider = $this->createStub(EvaluationContextProviderInterface::class); + $provider->method('getContext')->willReturn($context); + + return $provider; + } +} diff --git a/tests/Profiler/OpenFeatureDataCollectorLazyContextTest.php b/tests/Profiler/OpenFeatureDataCollectorLazyContextTest.php new file mode 100644 index 0000000..6924ad8 --- /dev/null +++ b/tests/Profiler/OpenFeatureDataCollectorLazyContextTest.php @@ -0,0 +1,65 @@ +setEvaluationContext(new LazyEvaluationContext(fn (): EvaluationContext => $this->fail('The collector must not resolve the context'))); + + $collector = $this->collect($api); + + $this->assertFalse($collector->isEvaluationContextResolved()); + $this->assertSame([], $collector->getEvaluationContext()); + } + + public function testSerializesResolvedLazyContext(): void + { + $context = new LazyEvaluationContext(static fn (): EvaluationContext => new MutableEvaluationContext('user-1', new MutableAttributes(['plan' => 'premium']))); + $context->getTargetingKey(); + + $api = new OpenFeatureAPI(); + $api->setEvaluationContext($context); + + $collector = $this->collect($api); + + $this->assertTrue($collector->isEvaluationContextResolved()); + $this->assertSame([ + 'targeting_key' => \substr(\hash('sha256', 'user-1'), 0, 12), + 'attributes' => ['plan' => 'premium'], + ], $collector->getEvaluationContext()); + } + + public function testRegularContextIsReportedAsResolved(): void + { + $api = new OpenFeatureAPI(); + $api->setEvaluationContext(new MutableEvaluationContext('user-1')); + + $this->assertTrue($this->collect($api)->isEvaluationContextResolved()); + } + + private function collect(OpenFeatureAPI $api): OpenFeatureDataCollector + { + $collector = new OpenFeatureDataCollector(new ProfilerHook(), $api); + $collector->collect(Request::create('/'), new Response()); + + return $collector; + } +} From 31a1ad5ebc1cfc0090fc66c7b4b6cc557af7872d Mon Sep 17 00:00:00 2001 From: aubes <3941035+aubes@users.noreply.github.com> Date: Tue, 29 Sep 2026 18:23:31 +0200 Subject: [PATCH 2/3] fix: dump structured values in the profiler panel --- CHANGELOG.md | 19 ++- UPGRADE.md | 135 ++++++++++++++++++ src/Profiler/OpenFeatureDataCollector.php | 25 +++- .../views/Collector/openfeature.html.twig | 8 +- ...penFeatureDataCollectorLazyContextTest.php | 12 +- .../OpenFeatureDataCollectorValueDumpTest.php | 65 +++++++++ 6 files changed, 242 insertions(+), 22 deletions(-) create mode 100644 UPGRADE.md create mode 100644 tests/Profiler/OpenFeatureDataCollectorValueDumpTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 4f82959..f3df8a4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,16 +26,19 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Providers now receive the application logger when MonologBundle is not installed; previously they got none. A logger already set on a provider service (e.g. a dedicated Monolog channel) is no longer overridden. - `flags` and `providers`: keys are now kept as declared. Dashes were converted to underscores (a flag declared as `new-checkout` could only be evaluated as `new_checkout`), and an object flag holding a `name` key was renamed after that value. The undocumented list form `flags: [{name: ..., value: ...}]` is no longer supported. - `RedisProvider` now logs Redis client failures at `error` level. Previously, an unavailable Redis silently resolved every flag to its default value, with nothing in the logs. +- The profiler panel no longer breaks on object flags or on array and date context attributes: these values are now shown with the VarDumper, like in other Symfony panels. ### Upgrade notes -- Run `composer update open-feature/sdk` if your lock file pins a version below 2.3.0. -- Code calling `OpenFeatureAPI::getInstance()` directly now gets an instance distinct from the bundle's `API` service (different provider, hooks, and evaluation context). Inject the `API` or `Client` service instead. -- **Check your raw flag values.** `EnvVarProvider` and `RedisProvider` no longer cast unparsable values: a boolean flag set to anything other than `true`/`false`/`1`/`0`/`yes`/`no`/`on`/`off`/empty (e.g. `FEATURE_X=enabled`, previously `false`) or a numeric flag with a non-numeric value (for integers, decimal or exponent notation such as `10.0` or `1e3` too; leading zeros such as `08` are accepted) now resolves to the default value with a `PARSE_ERROR`. `InMemoryProvider` flags declared with a mismatching type (e.g. `max_items: 1.5` read as integer, `label: 42` read as string) now resolve to the default value with a `TYPE_MISMATCH`, like with typed providers such as flagd. In Twig, `{{ feature_value('max_items') }}` without a default reads the flag as a string and now renders `''`: pass a typed default (`feature_value('max_items', 10)`). These errors are not logged by the SDK: check the `open_feature` profiler panel in dev (error column), or register a hook that logs `ResolutionDetails::getError()` in `after()`. -- **Provider services created by a factory, or defined through `parent` under an id that is not a class name,** must set the `class` option (e.g. `class: App\FeatureFlag\MyProvider` next to `factory:`; `OpenFeature\interfaces\provider\Provider` is accepted when the concrete class is unknown), otherwise the container fails to compile with `Class "" used for OpenFeature provider service "..." cannot be found`. -- **`user_provider: auto` now takes effect.** With SecurityBundle enabled, the authenticated user identifier becomes the targeting key. Set `evaluation_context.user_provider: false` to keep the previous behavior. -- **Evaluation context providers run lazily.** Logic that relies on running at the start of every request (side effects, timing) must move to its own `kernel.request` listener. The API-level context is now an internal `LazyEvaluationContext`: an `instanceof MutableEvaluationContext` check on `API::getEvaluationContext()` no longer matches, and reading its targeting key or attributes runs the providers. -- Custom providers using `ResolutionDetailsTrait::toBool()` must switch to `parseBool($flagKey, $raw, $defaultValue)`, which returns a `ResolutionDetails` instead of a `bool`. +See [UPGRADE.md][upgrade-0.4] for details and examples. + +- Update `open-feature/sdk` to 2.3 (`composer update open-feature/sdk`). +- Inject the `API` or `Client` service instead of calling `OpenFeatureAPI::getInstance()`. +- Check the raw values of `EnvVarProvider` and `RedisProvider` flags and the types of `InMemoryProvider` flags: a mismatch now resolves to the default value with an error. In Twig, pass a typed default to `feature_value()`. +- Set the `class` option on provider services created by a factory or defined through `parent`. +- `user_provider: auto` now takes effect with SecurityBundle: set it to `false` to keep the previous behavior. +- Move logic that must run on every request out of evaluation context providers, which now run on the first flag evaluation. `API::getEvaluationContext()` no longer returns a `MutableEvaluationContext`. +- Replace `ResolutionDetailsTrait::toBool()` with `parseBool()`. ## [0.3.0] - 2026-06-15 @@ -80,3 +83,5 @@ Initial release. [0.2.0]: https://github.com/aubes/openfeature-bundle/compare/v0.1.1...v0.2.0 [0.1.1]: https://github.com/aubes/openfeature-bundle/compare/v0.1.0...v0.1.1 [0.1.0]: https://github.com/aubes/openfeature-bundle/releases/tag/v0.1.0 + +[upgrade-0.4]: UPGRADE.md#upgrading-from-03-to-04 diff --git a/UPGRADE.md b/UPGRADE.md new file mode 100644 index 0000000..c13efac --- /dev/null +++ b/UPGRADE.md @@ -0,0 +1,135 @@ +# Upgrade guide + +## Upgrading from 0.3 to 0.4 + +0.4 is a minor release with breaking changes, as allowed for 0.x versions by [Semantic Versioning][semver-4]. This guide lists what you may have to change, grouped by how you use the bundle. The [changelog][changelog] has the complete list of changes. + +### Everyone + +**The bundle requires `open-feature/sdk` 2.3.** Update it if your lock file pins an older version: + +```bash +composer update open-feature/sdk +``` + +**The `API` service is an isolated instance.** It no longer shares its provider, hooks, and evaluation context with the `OpenFeatureAPI::getInstance()` singleton. Code calling `getInstance()` directly now gets a different, unconfigured instance: inject the `API` or `Client` service instead. + +```php +// Before +$client = OpenFeatureAPI::getInstance()->getClient(); + +// After +public function __construct(private readonly Client $client) {} +``` + +### If you configure the bundle + +**`evaluation_context.user_provider: auto` now takes effect.** It used to resolve to `false` in every application. With SecurityBundle enabled, the identifier of the authenticated user now becomes the targeting key, and it is sent to your flag provider. To keep the previous behavior, disable it: + +```yaml +open_feature: + evaluation_context: + user_provider: false +``` + +If the user identifier is personal data (an email address, for example), read the note in [Evaluation context][evaluation-context]. + +**`feature_flag.on_disabled: auto` depends on SecurityBundle.** It picks `access_denied` only when SecurityBundle is enabled, and `http_exception` otherwise. Only an application with `symfony/security-core` installed but no SecurityBundle is affected: a closed `#[FeatureGate]` now returns a 403 instead of a 500. To keep throwing an `AccessDeniedException`, set it explicitly: + +```yaml +open_feature: + feature_flag: + on_disabled: access_denied +``` + +**Keys under `flags` and `providers` are kept as declared.** Dashes were converted to underscores, so a flag declared as `new-checkout` could only be evaluated as `new_checkout`. Evaluate it with its declared key, or rename it. The same applies to provider names referenced by `strategy.fallback`. + +The undocumented list form of `flags` is no longer supported. Use a map: + +```yaml +# Before +open_feature: + flags: + - { name: dark_mode, value: true } + +# After +open_feature: + flags: + dark_mode: true +``` + +**Provider services must declare their class.** A provider created by a factory, or defined through `parent` under an id that is not a class name, needs an explicit `class` option. Without it, the container fails to compile with `Class "" used for OpenFeature provider service "..." cannot be found`. + +```yaml +services: + app.feature_provider: + class: App\FeatureFlag\MyProvider + factory: ['@App\FeatureFlag\ProviderFactory', 'create'] +``` + +When the concrete class is unknown, `class: OpenFeature\interfaces\provider\Provider` is accepted. + +### If you use the built-in providers + +**`EnvVarProvider` and `RedisProvider` no longer cast raw values.** A value that does not match the requested type now resolves to the default value with a `PARSE_ERROR`, instead of being silently cast. Check your environment variables and Redis keys against the accepted values listed in [EnvVar provider][env-var] and [Redis provider][redis]: + +```bash +# Before: resolved to false and 10 +FEATURE_NEW_CHECKOUT=enabled +FEATURE_MAX_ITEMS=10.0 + +# After +FEATURE_NEW_CHECKOUT=true +FEATURE_MAX_ITEMS=10 +``` + +**`InMemoryProvider` checks the type of each flag.** A flag read with a method that does not match its YAML type now resolves to the default value with a `TYPE_MISMATCH`, as with typed providers such as flagd. The only conversions left are `0`/`1` read as booleans and integers read as floats. See the table in [InMemory provider][in-memory]: + +```yaml +open_feature: + flags: + max_items: 10 # was 1.5, read as an integer + label: '42' # was 42, read as a string +``` + +**In Twig, pass a default of the flag's type to `feature_value()`.** Without a default, the flag is read as a string, so a boolean or integer flag now renders `''`: + +```twig +{# Before #} +{{ feature_value('max_items') }} + +{# After #} +{{ feature_value('max_items', 10) }} +``` + +These errors are not logged by the SDK. In dev, the error column of the `open_feature` profiler panel shows them. + +**`RedisProvider` logs client failures** at `error` level, once per flag evaluation while Redis is unavailable. See [Redis provider][redis] to limit the volume. + +### If you wrote code around the bundle + +**Evaluation context providers run on the first flag evaluation**, not on `kernel.request`. A request that evaluates no flag never runs them. Logic that must run at the start of every request (side effects, timing) belongs in its own `kernel.request` listener. Two related changes: + +- An exception thrown by a context provider, or by a listener of `EvaluationContextContributedEvent`, is now logged instead of failing the request. +- A flag evaluated from inside a context provider gets an empty context. + +**The API-level evaluation context is lazy.** `API::getEvaluationContext()` now returns an internal lazy context: an `instanceof MutableEvaluationContext` check no longer matches, and reading its targeting key or attributes runs the context providers. To add data to the context, implement `EvaluationContextProviderInterface`, or pass an invocation context to the evaluation. + +**`ResolutionDetailsTrait::toBool()` is removed.** Custom providers using the trait switch to `parseBool()`, which returns the `ResolutionDetails` directly. `parseInt()`, `parseFloat()`, and `parseObject()` follow the same pattern: + +```php +// Before +return $this->found($this->toBool($raw)); + +// After +return $this->parseBool($flagKey, $raw, $defaultValue); +``` + +**Provider validation messages changed.** If your tests assert them, a class that does not exist now reports `Class "..." used for OpenFeature provider service "..." cannot be found`, and a class that is not a provider reports `OpenFeature provider service "..." (class "...") must implement interface "OpenFeature\interfaces\provider\Provider"`. + +[semver-4]: https://semver.org/#spec-item-4 +[changelog]: CHANGELOG.md +[evaluation-context]: docs/features/evaluation-context.md +[env-var]: docs/providers/env-var.md +[redis]: docs/providers/redis.md +[in-memory]: docs/providers/in-memory.md diff --git a/src/Profiler/OpenFeatureDataCollector.php b/src/Profiler/OpenFeatureDataCollector.php index d4c816f..ed33977 100644 --- a/src/Profiler/OpenFeatureDataCollector.php +++ b/src/Profiler/OpenFeatureDataCollector.php @@ -10,6 +10,7 @@ use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; use Symfony\Component\HttpKernel\DataCollector\DataCollector; +use Symfony\Component\VarDumper\Cloner\Data; class OpenFeatureDataCollector extends DataCollector { @@ -33,7 +34,7 @@ public function collect(Request $request, Response $response, ?\Throwable $excep $resolved = !$context instanceof LazyEvaluationContext || $context->isResolved(); $this->data = [ - 'evaluations' => $this->hook->getEvaluations(), + 'evaluations' => $this->collectEvaluations(), 'provider' => $this->api->getProviderMetadata()->getName(), 'providers' => $this->providers, 'strategy' => $this->providers === [] ? null : $this->strategy, @@ -110,10 +111,10 @@ public function getHooks(): array return $hooks; } - /** @return list}> */ + /** @return list */ public function getContextProviders(): array { - /** @var list}> $providers */ + /** @var list $providers */ $providers = $this->data['context_providers'] ?? []; return $providers; @@ -147,10 +148,22 @@ private function serializeContext(?EvaluationContext $context): array return [ 'targeting_key' => $this->anonymizeTargetingKey($context->getTargetingKey()), - 'attributes' => $context->getAttributes()->toArray(), + 'attributes' => \array_map($this->cloneVar(...), $context->getAttributes()->toArray()), ]; } + /** @return array> */ + private function collectEvaluations(): array + { + return \array_map( + // Booleans stay raw for the true/false badges; other values (arrays, dates) go through VarDumper + fn (array $evaluation): array => \array_replace($evaluation, [ + 'value' => \is_bool($evaluation['value']) ? $evaluation['value'] : $this->cloneVar($evaluation['value']), + ]), + $this->hook->getEvaluations(), + ); + } + /** @return list */ private function collectHooks(): array { @@ -166,7 +179,7 @@ private function collectHooks(): array return $hooks; } - /** @return list}> */ + /** @return list */ private function collectContextProviders(): array { if ($this->recorder === null) { @@ -177,7 +190,7 @@ private function collectContextProviders(): array fn (array $c): array => [ 'provider' => $c['provider'], 'targeting_key' => $this->anonymizeTargetingKey($c['targeting_key']), - 'attributes' => $c['attributes'], + 'attributes' => $this->cloneVar($c['attributes']), ], $this->recorder->getContributions(), ); diff --git a/src/Resources/views/Collector/openfeature.html.twig b/src/Resources/views/Collector/openfeature.html.twig index 24b1aee..4337d50 100644 --- a/src/Resources/views/Collector/openfeature.html.twig +++ b/src/Resources/views/Collector/openfeature.html.twig @@ -115,7 +115,7 @@ {% for key, value in collector.evaluationContext.attributes ?? {} %} {{ key }} - {{ value }} + {{ profiler_dump(value) }} {% endfor %} @@ -150,9 +150,7 @@ {% if contribution.attributes is empty %} - {% else %} - {% for key, value in contribution.attributes %} - {{ key }}: {{ value }}{{ not loop.last ? ', ' : '' }} - {% endfor %} + {{ profiler_dump(contribution.attributes) }} {% endif %} @@ -213,7 +211,7 @@ {% elseif eval.value is same as false %} false {% else %} - {{ eval.value|e }} + {{ profiler_dump(eval.value) }} {% endif %} {{ eval.variant ?? '-' }} diff --git a/tests/Profiler/OpenFeatureDataCollectorLazyContextTest.php b/tests/Profiler/OpenFeatureDataCollectorLazyContextTest.php index 6924ad8..13719a9 100644 --- a/tests/Profiler/OpenFeatureDataCollectorLazyContextTest.php +++ b/tests/Profiler/OpenFeatureDataCollectorLazyContextTest.php @@ -15,6 +15,7 @@ use PHPUnit\Framework\TestCase; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; +use Symfony\Component\VarDumper\Cloner\Data; #[CoversClass(OpenFeatureDataCollector::class)] class OpenFeatureDataCollectorLazyContextTest extends TestCase @@ -41,10 +42,13 @@ public function testSerializesResolvedLazyContext(): void $collector = $this->collect($api); $this->assertTrue($collector->isEvaluationContextResolved()); - $this->assertSame([ - 'targeting_key' => \substr(\hash('sha256', 'user-1'), 0, 12), - 'attributes' => ['plan' => 'premium'], - ], $collector->getEvaluationContext()); + + $context = $collector->getEvaluationContext(); + $this->assertSame(\substr(\hash('sha256', 'user-1'), 0, 12), $context['targeting_key']); + $this->assertIsArray($context['attributes']); + $plan = $context['attributes']['plan'] ?? null; + $this->assertInstanceOf(Data::class, $plan); + $this->assertSame('premium', $plan->getValue()); } public function testRegularContextIsReportedAsResolved(): void diff --git a/tests/Profiler/OpenFeatureDataCollectorValueDumpTest.php b/tests/Profiler/OpenFeatureDataCollectorValueDumpTest.php new file mode 100644 index 0000000..6ef2483 --- /dev/null +++ b/tests/Profiler/OpenFeatureDataCollectorValueDumpTest.php @@ -0,0 +1,65 @@ +after((new HookContextBuilder())->withFlagKey('enabled')->withType(FlagValueType::BOOLEAN)->build(), (new ResolutionDetailsBuilder())->withValue(true)->build(), new HookHints()); + $hook->after((new HookContextBuilder())->withFlagKey('config')->withType(FlagValueType::OBJECT)->build(), (new ResolutionDetailsBuilder())->withValue(['color' => 'blue'])->build(), new HookHints()); + + $collector = new OpenFeatureDataCollector($hook, new OpenFeatureAPI()); + $collector->collect(Request::create('/'), new Response()); + + [$enabled, $config] = $collector->getEvaluations(); + $this->assertTrue($enabled['value']); + $this->assertInstanceOf(Data::class, $config['value']); + $this->assertEquals(['color' => 'blue'], $config['value']->getValue(true)); + } + + public function testDumpsContextAttributesAndContributions(): void + { + $context = new MutableEvaluationContext('user-1', new MutableAttributes(['plan' => 'premium', 'since' => new \DateTime('2026-09-28')])); + + $api = new OpenFeatureAPI(); + $api->setEvaluationContext($context); + $recorder = new ContextProviderRecorder(); + $recorder(new EvaluationContextContributedEvent($this->createStub(EvaluationContextProviderInterface::class), $context)); + + $collector = new OpenFeatureDataCollector(new ProfilerHook(), $api, $recorder); + $collector->collect(Request::create('/'), new Response()); + + $attributes = $collector->getEvaluationContext()['attributes'] ?? null; + $this->assertIsArray($attributes); + $this->assertContainsOnlyInstancesOf(Data::class, $attributes); + $this->assertSame('premium', $attributes['plan']->getValue()); + + $this->assertCount(2, $collector->getContextProviders()[0]['attributes']); + } +} From d1e44913cb4b1624caee613e98a21ef04684f0cd Mon Sep 17 00:00:00 2001 From: aubes <3941035+aubes@users.noreply.github.com> Date: Sun, 4 Oct 2026 16:54:50 +0200 Subject: [PATCH 3/3] feat: disable user_provider by default --- CHANGELOG.md | 3 ++- UPGRADE.md | 6 +++--- docs/configuration.md | 6 +++--- docs/features/evaluation-context.md | 10 +++++----- src/OpenFeatureBundle.php | 4 ++-- tests/DependencyInjection/ConfigurationTest.php | 2 +- .../DependencyInjection/OpenFeatureBundleTest.php | 14 +++++++++++--- 7 files changed, 27 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f3df8a4..69148e7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Evaluation context providers now run on the first flag evaluation of the request instead of on `kernel.request`. A request that evaluates no flag no longer reads the security token, so a `lazy` firewall stays lazy and the response stays HTTP-cacheable. `EvaluationContextContributedEvent` is dispatched at that time. - An exception thrown by an evaluation context provider, or by a listener of `EvaluationContextContributedEvent`, is now logged instead of failing the request (the failing provider is skipped). - Provider services (`provider`, `providers`) must now declare their class, as Symfony already requires for services built by a factory. A provider created by a factory, or inheriting its class from a `parent` under an id that is not a class name, now fails at compile time without an explicit `class` option instead of skipping validation. +- `evaluation_context.user_provider` now defaults to `false` instead of `auto`: the user identifier is only sent to the flag provider when explicitly enabled. Since `auto` always resolved to `false` in 0.3, the effective default does not change. ### Fixed @@ -36,7 +37,7 @@ See [UPGRADE.md][upgrade-0.4] for details and examples. - Inject the `API` or `Client` service instead of calling `OpenFeatureAPI::getInstance()`. - Check the raw values of `EnvVarProvider` and `RedisProvider` flags and the types of `InMemoryProvider` flags: a mismatch now resolves to the default value with an error. In Twig, pass a typed default to `feature_value()`. - Set the `class` option on provider services created by a factory or defined through `parent`. -- `user_provider: auto` now takes effect with SecurityBundle: set it to `false` to keep the previous behavior. +- `user_provider` now defaults to `false`, as it effectively was in 0.3. An explicit `auto` now takes effect with SecurityBundle, and `true` no longer fails. - Move logic that must run on every request out of evaluation context providers, which now run on the first flag evaluation. `API::getEvaluationContext()` no longer returns a `MutableEvaluationContext`. - Replace `ResolutionDetailsTrait::toBool()` with `parseBool()`. diff --git a/UPGRADE.md b/UPGRADE.md index c13efac..1ad7d6d 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -24,15 +24,15 @@ public function __construct(private readonly Client $client) {} ### If you configure the bundle -**`evaluation_context.user_provider: auto` now takes effect.** It used to resolve to `false` in every application. With SecurityBundle enabled, the identifier of the authenticated user now becomes the targeting key, and it is sent to your flag provider. To keep the previous behavior, disable it: +**`evaluation_context.user_provider` defaults to `false`.** The previous default, `auto`, always resolved to `false` because of a bug, so nothing changes by default. If you set `auto` or `true` explicitly, it now works: with SecurityBundle enabled, the identifier of the authenticated user becomes the targeting key, and it is sent to your flag provider. To enable it: ```yaml open_feature: evaluation_context: - user_provider: false + user_provider: auto ``` -If the user identifier is personal data (an email address, for example), read the note in [Evaluation context][evaluation-context]. +If the user identifier is personal data (an email address, for example), read the note in [Evaluation context][evaluation-context] first. **`feature_flag.on_disabled: auto` depends on SecurityBundle.** It picks `access_denied` only when SecurityBundle is enabled, and `http_exception` otherwise. Only an application with `symfony/security-core` installed but no SecurityBundle is affected: a closed `#[FeatureGate]` now returns a 403 instead of a 500. To keep throwing an `AccessDeniedException`, set it explicitly: diff --git a/docs/configuration.md b/docs/configuration.md index 2e33eaf..72c0f73 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -35,11 +35,11 @@ open_feature: # EvaluationContext settings evaluation_context: - # Populate targeting key from the authenticated Symfony user + # Populate targeting key from the authenticated Symfony user (sent to the flag provider) + # false: disabled (default) # auto: enabled if SecurityBundle is enabled # true: always enabled (requires SecurityBundle) - # false: disabled - user_provider: auto # auto | true | false + user_provider: false # false | auto | true # Exception behavior for #[FeatureGate] feature_flag: diff --git a/docs/features/evaluation-context.md b/docs/features/evaluation-context.md index 3aa67c4..1a07c45 100644 --- a/docs/features/evaluation-context.md +++ b/docs/features/evaluation-context.md @@ -4,21 +4,21 @@ The `EvaluationContext` carries targeting information (user ID, attributes) used ## Auto-populate from the Symfony user -When SecurityBundle is enabled, the authenticated user's identifier is automatically set as the `targeting_key`: +Enable `user_provider` to set the authenticated user's identifier as the `targeting_key`: ```yaml open_feature: evaluation_context: - user_provider: true # or "auto" (default) + user_provider: auto # or true ``` | Value | Behavior | |---|---| -| `auto` (default) | Enabled if SecurityBundle is enabled | +| `false` (default) | Disabled | +| `auto` | Enabled if SecurityBundle is enabled | | `true` | Always enabled (requires SecurityBundle) | -| `false` | Disabled | -> **Note:** The user identifier is sent as-is to the flag provider, which may be a remote service (flagd, GO Feature Flag relay proxy, LaunchDarkly). If it is personal data, such as an email address, set `user_provider: false` and register a [custom context provider](#custom-context-provider) that sets a non-personal targeting key (internal user ID, hash). The profiler only displays a hash of the targeting key. +> **Note:** The user identifier is sent as-is to the flag provider, which may be a remote service (flagd, GO Feature Flag relay proxy, LaunchDarkly). This is why `user_provider` is disabled by default. If the identifier is personal data, such as an email address, keep it disabled and register a [custom context provider](#custom-context-provider) that sets a non-personal targeting key (internal user ID, hash). The profiler only displays a hash of the targeting key. ## Custom context provider diff --git a/src/OpenFeatureBundle.php b/src/OpenFeatureBundle.php index 8ac3b1d..cee792f 100644 --- a/src/OpenFeatureBundle.php +++ b/src/OpenFeatureBundle.php @@ -87,13 +87,13 @@ public function configure(DefinitionConfigurator $definition): void ->addDefaultsIfNotSet() ->children() ->enumNode('user_provider') - ->info('Populate EvaluationContext targeting key from the authenticated Symfony user. "auto" enables it if SecurityBundle is enabled.') + ->info('Populate the EvaluationContext targeting key from the authenticated Symfony user identifier, which is sent to the flag provider. "auto" enables it if SecurityBundle is enabled. Disabled by default.') ->beforeNormalization() ->ifTrue(\is_bool(...)) ->then(static fn (bool $v): string => $v ? 'true' : 'false') ->end() ->values(['auto', 'true', 'false']) - ->defaultValue('auto') + ->defaultValue('false') ->end() ->end() ->end() diff --git a/tests/DependencyInjection/ConfigurationTest.php b/tests/DependencyInjection/ConfigurationTest.php index 12c7fe0..667f1a3 100644 --- a/tests/DependencyInjection/ConfigurationTest.php +++ b/tests/DependencyInjection/ConfigurationTest.php @@ -25,7 +25,7 @@ public function testDefaultConfiguration(): void 'providers' => [], 'strategy' => ['type' => 'first_match', 'fallback' => null], 'flags' => [], - 'evaluation_context' => ['user_provider' => 'auto'], + 'evaluation_context' => ['user_provider' => 'false'], 'feature_flag' => ['on_disabled' => 'auto', 'status_code' => 403], ], $this->process([])); } diff --git a/tests/DependencyInjection/OpenFeatureBundleTest.php b/tests/DependencyInjection/OpenFeatureBundleTest.php index 65cebcb..7cbf29a 100644 --- a/tests/DependencyInjection/OpenFeatureBundleTest.php +++ b/tests/DependencyInjection/OpenFeatureBundleTest.php @@ -175,10 +175,18 @@ public function testFeatureFlagOnDisabledHttpException(): void $this->assertSame(404, $container->getParameter('open_feature.feature_flag.status_code')); } - public function testUserProviderAutoEnabledWithSecurityBundle(): void + public function testUserProviderDisabledByDefault(): void { $container = $this->createContainer(bundles: self::SECURITY_BUNDLE); + $this->assertSame('false', $container->getParameter('open_feature.evaluation_context.user_provider')); + $this->assertFalse($container->hasDefinition(UserEvaluationContextProvider::class)); + } + + public function testUserProviderAutoEnabledWithSecurityBundle(): void + { + $container = $this->createContainer(['evaluation_context' => ['user_provider' => 'auto']], bundles: self::SECURITY_BUNDLE); + $this->assertSame('true', $container->getParameter('open_feature.evaluation_context.user_provider')); $definition = $container->getDefinition(UserEvaluationContextProvider::class); @@ -189,7 +197,7 @@ public function testUserProviderAutoEnabledWithSecurityBundle(): void public function testUserProviderToleratesSecurityBundleWithoutTokenStorage(): void { // Symfony 6.4 registers no security service when SecurityBundle is enabled but not configured - $container = $this->createContainer(bundles: self::SECURITY_BUNDLE); + $container = $this->createContainer(['evaluation_context' => ['user_provider' => 'auto']], bundles: self::SECURITY_BUNDLE); (new CheckExceptionOnInvalidReferenceBehaviorPass())->process($container); $provider = $container->get(UserEvaluationContextProvider::class); @@ -199,7 +207,7 @@ public function testUserProviderToleratesSecurityBundleWithoutTokenStorage(): vo public function testUserProviderAutoDisabledWithoutSecurityBundle(): void { - $container = $this->createContainer(); + $container = $this->createContainer(['evaluation_context' => ['user_provider' => 'auto']]); $this->assertSame('false', $container->getParameter('open_feature.evaluation_context.user_provider')); $this->assertFalse($container->hasDefinition(UserEvaluationContextProvider::class));