diff --git a/UPGRADING.md b/UPGRADING.md index 2f4f4e5..a0e680a 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -10,9 +10,44 @@ read each intermediate section in order — behavioural changes compose. ## Within v1.x The v1.x line is covered end-to-end by SemVer (see "v0.x → v1.0.0" -below for the surface contract). Minor releases are additive by default; -the only behavioural change so far is in v1.3.0 and is gated on an -already-opt-in flag. +below for the surface contract). Minor releases are additive by default. +Two behavioural changes exist so far: v1.3.0 (gated on an already-opt-in +flag) and v1.8.0 (`discriminator.mapping` enforcement, default-on with an +opt-out flag — see directly below). + +### From v1.7.0 → v1.8.0 + +- **`discriminator.mapping` is now enforced** (#262). Previously the + converter stripped `discriminator` and emitted a one-shot + `E_USER_WARNING`; the underlying `oneOf` / `anyOf` was validated only as + a plain union, so a polymorphic body that lied about its type passed as + long as it matched any branch. The converter now lowers `discriminator` + + `mapping` into Draft-07 `if`/`then` conditionals so the discriminator + value steers validation toward a single branch. + - **Behaviour change**: a body whose discriminator value routes to a + branch it does not satisfy (e.g. `kty: RSA` carrying EC-only fields, or + an unknown discriminator value) now **fails** where it previously + passed. This is the contract bug the warning only narrated. + - **The `discriminator.mapping` `E_USER_WARNING` is removed.** This also + fixes Laravel consumers, whose `HandleExceptions` turned that advisory + warning into a fatal `ErrorException` on the first polymorphic contract + test. No per-consumer `set_error_handler` boilerplate is needed any + more. + - **Opt out**: set `enforce_discriminator: false` (Laravel + `config/openapi-contract-testing.php`) or + `` (the PHPUnit + `OpenApiCoverageExtension`; `0` / `no` also work) to keep the old + strip-without-enforce behaviour (now also warning-free). + - **Malformed `discriminator`** blocks (missing/non-string + `propertyName`, non-array `mapping`, non-string mapping value, + unresolvable mapping pointer, non-object target) now surface as a loud + validation failure under enforcement, instead of being silently + dropped. + - **Known limitation**: self-referential discriminator chains (a subtype + that re-contains the same base discriminator via `allOf` + `$ref`) are + enforced at the first recursion level; the inner re-appearance is + stripped without re-lowering (the outer branch already enforces it). + See `docs/supported-features.md` → "Schema features" → `discriminator`. ### From v1.3.0 → v1.4.0 diff --git a/docs/setup.md b/docs/setup.md index 495c1a6..80d312f 100644 --- a/docs/setup.md +++ b/docs/setup.md @@ -58,6 +58,7 @@ Add the coverage extension to your `phpunit.xml`: | `console_output` | No | `default` | Console output mode: `default`, `all`, `uncovered_only`, or `active_only` (overridden by `OPENAPI_CONSOLE_OUTPUT` env var) | | `sidecar_dir` | No | `sys_get_temp_dir()/openapi-coverage-sidecars` | Directory paratest workers drop per-worker JSON sidecars into. Used only under parallel test runners — see [Parallel test runners](parallel.md) | | `default_testsuite_as_full` | No | `false` | Opt-in. When `true` and PHPUnit's `includeTestSuites` resolves exactly to the configured `defaultTestSuite`, treat the run as full instead of partial (so `strict_required` and coverage outputs aren't suppressed). See [default_testsuite_as_full opt-in](ci.md#default_testsuite_as_full-opt-in) for trade-offs | +| `enforce_discriminator` | No | `true` | When `true` (default), `discriminator` + `mapping` is enforced via `if`/`then` lowering so a body that lies about its type fails. Set to `false` (or `0` / `no`) to strip `discriminator` without enforcing (no warning either). See [Schema features → discriminator](supported-features.md#schema-features) | *Not required if you call `OpenApiSpecLoader::configure()` manually. @@ -81,6 +82,11 @@ return [ // 0 = unlimited (reports all errors). 'max_errors' => 20, + // Enforce `discriminator` + `mapping` by lowering it into if/then + // conditionals so a body that lies about its type fails (default true). + // Set false to strip discriminator without enforcing (no warning either). + 'enforce_discriminator' => true, + // Automatically validate every TestResponse produced by Laravel HTTP // helpers (get(), post(), etc.) against the OpenAPI spec. Defaults to // false for backward compatibility. diff --git a/docs/supported-features.md b/docs/supported-features.md index 9e62baf..585e106 100644 --- a/docs/supported-features.md +++ b/docs/supported-features.md @@ -61,12 +61,16 @@ Detection looks at each property schema's own top-level `readOnly` / `writeOnly` - **Validated** (delegated to opis Draft 07): `type`, `enum`, `multipleOf`, `minimum`/`maximum`/`exclusiveMinimum`/`exclusiveMaximum`, `minLength`/`maxLength`/`pattern`, `minItems`/`maxItems`/`uniqueItems`, `minProperties`/`maxProperties`/`required`, `additionalProperties` (`true` / `false` / schema), `allOf` / `oneOf` / `anyOf` / `not`. - **`format`** (validated by opis Draft 06+): the canonical 19-entry set (`email`, `uuid`, `date`, `date-time`, `uri`, `ipv4`, `ipv6`, `hostname`, `regex`, `json-pointer`, …). The full list is the authoritative `KNOWN_OPIS_FORMATS` constant in `src/Spec/OpenApiSchemaConverter.php` — keeping it in one place avoids drift when opis adds formats. Unknown values (e.g. `format: emial` typo for `email`) emit a one-shot `E_USER_WARNING` per format value, since opis silently accepts any value for unrecognised formats. Non-string `format` values fire a separate malformed-spec warning. - **Advisory `format`** (deliberately not enforced, no warning): `int32`, `int64`, `float`, `double`, `byte`, `binary`, `password`. Treated as documentation hints per OAS conventions; see `ADVISORY_FORMATS` constant. -- **Lowered**: `const` → `enum: [value]` (3.1). -- **Stripped**: `discriminator` (including `mapping`), `xml`, `externalDocs`, `example` / `examples`, `deprecated`, OAS-only `nullable`/`readOnly`/`writeOnly` after enforcement (3.0), and Draft 2020-12 keys `$dynamicRef` / `$dynamicAnchor` / `contentSchema` (3.1). +- **Lowered**: `const` → `enum: [value]` (3.1); `discriminator` + `mapping` → an `allOf` of `if`/`then` conditionals (default; see `discriminator` below). +- **Stripped**: `xml`, `externalDocs`, `example` / `examples`, `deprecated`, OAS-only `nullable`/`readOnly`/`writeOnly` after enforcement (3.0), and Draft 2020-12 keys `$dynamicRef` / `$dynamicAnchor` / `contentSchema` (3.1). `discriminator` is also stripped when enforcement is turned off (`enforce_discriminator: false`). - **Validated via opis (Draft 06+)**: `patternProperties`, `contentMediaType`, `contentEncoding`. These are JSON Schema keywords that opis implements natively, so your constraints are enforced. - **Not supported (loud E_USER_WARNING when first encountered)**: `unevaluatedProperties`, `unevaluatedItems`. These are 2019-09 keywords with no Draft 07 equivalent — opis silently ignores them, so the warning surfaces specs that depend on them. Rewrite using `additionalProperties: false` plus explicit `properties` to enforce object closure. - **Advisory-only (loud E_USER_WARNING when first encountered)**: `dependentSchemas`, `dependentRequired`. These 2019-09 property-dependency keywords are not registered by opis Draft 07, so the constraint is dropped wholesale — a payload carrying the trigger property without its dependents passes silently. Rewrite as a Draft 07 conditional with `if` / `then` / `else` (the `if` clause tests for the trigger property, the `then` clause carries the dependent requirement). -- **`discriminator`**: the keyword is dropped; the underlying `oneOf` / `anyOf` is still validated as a union, but `discriminator.mapping` does not steer validation toward a single branch. When `mapping` is non-empty the converter emits a one-shot `E_USER_WARNING` so polymorphic specs with serialiser bugs surface as a loud signal rather than silently passing through any valid branch. +- **`discriminator`** (enforced by default, #262): when a schema declares `discriminator` with a non-empty `mapping`, the converter lowers it into an `allOf` of an unknown-value guard (the discriminator property must be present and one of the mapping keys) plus one `if`/`then` per mapping value, where `then` is the resolved subtype schema. The discriminator value therefore steers validation toward a single branch — a body that lies about its type (e.g. `kty: RSA` carrying EC-only fields) fails instead of passing the underlying `oneOf` / `anyOf` union. This is stricter than the OAS spec strictly requires (the discriminator is officially a tooling hint), which is exactly what a contract-testing tool wants. No `E_USER_WARNING` is emitted. + - **Opt out**: set `enforce_discriminator: false` (Laravel config) or `` (the PHPUnit extension; `0` / `no` also work) to restore the historical behaviour — `discriminator` is stripped and the mapping is not enforced (and no warning is emitted). + - **Malformed blocks**: with enforcement on, a structurally invalid `discriminator` (missing/non-string `propertyName`, non-array `mapping`, non-string mapping value, an unresolvable mapping pointer, or a pointer to a non-object) surfaces as a loud validation failure rather than silently passing. + - **Known limitation**: a self-referential discriminator chain (a subtype that, via `allOf` + `$ref`, re-contains the *same* base discriminator — the inheritance idiom) is enforced at the first recursion level; the inner re-appearance of that same discriminator is stripped without re-lowering (the outer branch already routes to and enforces that exact subtype). This terminates the lowering and avoids combinatorial blow-up while still enforcing the outer branch selection. Subtype-specific constraints (e.g. `required`) are unaffected — they live in the outer `then`. + - **`nullable` + `discriminator`** (3.0): a `null` body fails the discriminated-object branch (the lowered guard requires the discriminator property). Model a null-tolerant polymorphic field with an explicit `oneOf` including `{type: 'null'}` if needed. - **`readOnly` / `writeOnly`**: enforced at the property's own top level only (see [readOnly / writeOnly enforcement](#readonly--writeonly-enforcement)). ## HTTP methods @@ -82,7 +86,7 @@ The library uses PHP's native `trigger_error(..., E_USER_WARNING)` as the loud-s | Category prefix | Source | Dedup key | |---|---|---| | `[security]` | `SecurityValidator` (`oauth2`, `openIdConnect`, `mutualTLS`, `http-basic`, `http-digest`) | scheme name | -| `[OpenAPI Schema]` | `OpenApiSchemaConverter` (`unevaluatedProperties` / `unevaluatedItems`, `dependentSchemas` / `dependentRequired`, `discriminator.mapping`, unknown / malformed `format`) | per-keyword / per-format-value | +| `[OpenAPI Schema]` | `OpenApiSchemaConverter` (`unevaluatedProperties` / `unevaluatedItems`, `dependentSchemas` / `dependentRequired`, unknown / malformed `format`) | per-keyword / per-format-value | **How to consume:** diff --git a/src/Exception/MalformedDiscriminatorException.php b/src/Exception/MalformedDiscriminatorException.php new file mode 100644 index 0000000..5eb8b4c --- /dev/null +++ b/src/Exception/MalformedDiscriminatorException.php @@ -0,0 +1,34 @@ +applyDiscriminatorEnforcementConfig(); $resolvedMaxErrors = $this->resolveMaxErrors(); $resolvedSkipCodes = $this->resolveSkipRequestValidationResponseCodes(); @@ -894,6 +896,7 @@ private function findOperationForRequest(array $paths, string $method, string $p private function getOrCreateValidator(): OpenApiResponseValidator { + $this->applyDiscriminatorEnforcementConfig(); $resolvedMaxErrors = $this->resolveMaxErrors(); $resolvedSkipCodes = $this->resolveSkipResponseCodes(); @@ -918,6 +921,8 @@ private function getOrCreateValidator(): OpenApiResponseValidator */ private function buildOneOffValidator(array $extraSkipResponseCodes): OpenApiResponseValidator { + $this->applyDiscriminatorEnforcementConfig(); + return new OpenApiResponseValidator( maxErrors: $this->resolveMaxErrors(), skipResponseCodes: array_merge( @@ -927,6 +932,18 @@ private function buildOneOffValidator(array $extraSkipResponseCodes): OpenApiRes ); } + /** + * Push the `enforce_discriminator` config flag (Issue #262, default on) + * into the process-global {@see DiscriminatorEnforcement} gate the body + * validators read at conversion time. Called from every validator-build + * path so the current test's config is reflected even when the cached + * validator instance is reused. + */ + private function applyDiscriminatorEnforcementConfig(): void + { + DiscriminatorEnforcement::configure($this->resolveBoolConfig('enforce_discriminator', true)); + } + private function resolveMaxErrors(): int { $maxErrors = config('openapi-contract-testing.max_errors', 20); @@ -1097,10 +1114,14 @@ private function isAutoInjectDummyCredentialsEnabled(): bool * `'auto_X' => env('X')` (strings like "true" / "1") works without an * explicit cast. Anything else raises a loud PHPUnit failure so a typo * is not silently read as "off". + * + * `$default` is returned when the key is entirely absent (most flags + * default off, but `enforce_discriminator` defaults on — Issue #262); an + * explicit `null` value still coerces to false. */ - private function resolveBoolConfig(string $key): bool + private function resolveBoolConfig(string $key, bool $default = false): bool { - $raw = config('openapi-contract-testing.' . $key, false); + $raw = config('openapi-contract-testing.' . $key, $default); if ($raw === true) { return true; diff --git a/src/Laravel/config.php b/src/Laravel/config.php index 1b5ea31..629ac7a 100644 --- a/src/Laravel/config.php +++ b/src/Laravel/config.php @@ -12,6 +12,16 @@ // 0 = unlimited (reports all errors). 'max_errors' => 20, + // When true (the default), a schema's `discriminator` + `mapping` is + // lowered into Draft-07 `if`/`then` conditionals so the discriminator + // value actually steers validation toward a single branch — a body that + // lies about its type (e.g. `kty: RSA` carrying EC-only fields) fails + // instead of passing the underlying oneOf/anyOf union. Set to false to + // restore the historical behaviour (discriminator stripped, mapping not + // enforced) for specs that rely on the loose union semantics. See + // docs/supported-features.md "Discriminator" for the full note. + 'enforce_discriminator' => true, + // When true, every TestResponse produced by Laravel HTTP test helpers // (get(), post(), etc.) is validated against the OpenAPI spec at creation // time, without requiring an explicit assertResponseMatchesOpenApiSchema() diff --git a/src/OpenApiRequestValidator.php b/src/OpenApiRequestValidator.php index e4993bf..d92a8e5 100644 --- a/src/OpenApiRequestValidator.php +++ b/src/OpenApiRequestValidator.php @@ -14,6 +14,8 @@ use Studio\OpenApiContractTesting\Validation\Request\RequestBodyValidationResult; use Studio\OpenApiContractTesting\Validation\Request\RequestBodyValidator; use Studio\OpenApiContractTesting\Validation\Request\SecurityValidator; +use Studio\OpenApiContractTesting\Validation\Support\DiscriminatorContext; +use Studio\OpenApiContractTesting\Validation\Support\DiscriminatorEnforcement; use Studio\OpenApiContractTesting\Validation\Support\MalformedSpecNode; use Studio\OpenApiContractTesting\Validation\Support\PathDiagnosticsFormatter; use Studio\OpenApiContractTesting\Validation\Support\SchemaValidatorRunner; @@ -245,7 +247,10 @@ public function validate( // ValidatorErrorBoundary::safely() like the other sub-validators. // validateBody() runs it behind the same narrow RuntimeException // boundary inline — mirrors OpenApiResponseValidator::validateBody(). - $bodyResult = $this->validateBody($specName, $method, $matchedPath, $operation, $body, $contentType, $version); + // Carry the resolved root + enforce gate so the body validator can + // lower `discriminator.mapping` into enforceable conditionals (#262). + $discriminatorContext = new DiscriminatorContext($spec, DiscriminatorEnforcement::isEnabled()); + $bodyResult = $this->validateBody($specName, $method, $matchedPath, $operation, $body, $contentType, $version, $discriminatorContext); $errors = [ ...$collected->specErrors, @@ -338,9 +343,10 @@ private function validateBody( DecodedBody $body, ?string $contentType, OpenApiVersion $version, + DiscriminatorContext $discriminatorContext, ): RequestBodyValidationResult { try { - return $this->bodyValidator->validate($specName, $method, $matchedPath, $operation, $body, $contentType, $version); + return $this->bodyValidator->validate($specName, $method, $matchedPath, $operation, $body, $contentType, $version, $discriminatorContext); } catch (RuntimeException $e) { $previous = $e->getPrevious(); $previousSuffix = $previous !== null diff --git a/src/OpenApiResponseValidator.php b/src/OpenApiResponseValidator.php index 01ac1bb..df85eb8 100644 --- a/src/OpenApiResponseValidator.php +++ b/src/OpenApiResponseValidator.php @@ -15,6 +15,8 @@ use Studio\OpenApiContractTesting\Validation\Strict\StrictRequiredBodyWalker; use Studio\OpenApiContractTesting\Validation\Strict\StrictRequiredPerCallChecker; use Studio\OpenApiContractTesting\Validation\Strict\StrictRequiredTracker; +use Studio\OpenApiContractTesting\Validation\Support\DiscriminatorContext; +use Studio\OpenApiContractTesting\Validation\Support\DiscriminatorEnforcement; use Studio\OpenApiContractTesting\Validation\Support\MalformedSpecNode; use Studio\OpenApiContractTesting\Validation\Support\PathDiagnosticsFormatter; use Studio\OpenApiContractTesting\Validation\Support\SchemaValidatorRunner; @@ -307,6 +309,12 @@ public function validate( ], $matchedPath, $statusCodeStr); } + // Carry the resolved root + enforce gate so the body validator can + // lower `discriminator.mapping` into enforceable conditionals (#262). + // The mapping pointers reference subtype schemas elsewhere in the + // document, which only the root can resolve. + $discriminatorContext = new DiscriminatorContext($spec, DiscriminatorEnforcement::isEnabled()); + /** @var array $responseSpec */ $bodyResult = $this->validateBody( $specName, @@ -317,6 +325,7 @@ public function validate( $body, $responseContentType, $version, + $discriminatorContext, ); $headerErrors = $this->validateHeaders( @@ -524,6 +533,7 @@ private function validateBody( DecodedBody $responseBody, ?string $responseContentType, OpenApiVersion $version, + DiscriminatorContext $discriminatorContext, ): ResponseBodyValidationResult { // 204 No Content (and similar) declare no `content` block. Nothing // to validate — return empty so the result aggregates cleanly. @@ -570,6 +580,7 @@ private function validateBody( $responseBody, $responseContentType, $version, + $discriminatorContext, ); } catch (RuntimeException $e) { $previous = $e->getPrevious(); diff --git a/src/PHPUnit/OpenApiCoverageExtension.php b/src/PHPUnit/OpenApiCoverageExtension.php index 4a16ba5..3495d25 100644 --- a/src/PHPUnit/OpenApiCoverageExtension.php +++ b/src/PHPUnit/OpenApiCoverageExtension.php @@ -30,6 +30,7 @@ use Studio\OpenApiContractTesting\Validation\Strict\StrictRequiredPerCallChecker; use Studio\OpenApiContractTesting\Validation\Strict\StrictRequiredPerCallMode; use Studio\OpenApiContractTesting\Validation\Strict\StrictRequiredTracker; +use Studio\OpenApiContractTesting\Validation\Support\DiscriminatorEnforcement; use function array_filter; use function array_map; @@ -308,6 +309,17 @@ enumBasePath: $enumBasePath, StrictRequiredPerCallChecker::reset(); StrictRequiredPerCallChecker::configure($strictRequiredPerCallMode); + // Issue #262: discriminator.mapping enforcement gate. Default ON — + // enforcement is the correct contract-testing behaviour; `value="false"` + // (or `0` / `no`) is the escape hatch for specs that rely on the loose + // union semantics. resolveBooleanFlag reads the `enforce_discriminator` + // parameter (absent → the default), and configure() overwrites + // unconditionally so a process reused across bootstraps reflects the + // current run. + DiscriminatorEnforcement::configure( + self::resolveBooleanFlag($parameters, 'enforce_discriminator', true), + ); + if ($facade === null) { return; } diff --git a/src/Spec/OpenApiRefResolver.php b/src/Spec/OpenApiRefResolver.php index 30f44f7..01ddac9 100644 --- a/src/Spec/OpenApiRefResolver.php +++ b/src/Spec/OpenApiRefResolver.php @@ -160,6 +160,41 @@ public static function resolve( return $spec; } + /** + * Resolve a local JSON Pointer (`#/...` form) against an already-loaded, + * fully-resolved root document and return `[found, value]`. + * + * Public seam over the private {@see self::lookup()} so other components + * that need pointer resolution — notably {@see OpenApiSchemaConverter} + * lowering `discriminator.mapping` (Issue #262) — reuse the exact same + * segment-unescape rules (`~0` / `~1`, percent-decode) instead of + * re-implementing them and drifting. `found=false` distinguishes a + * missing segment from a literal `null` leaf. + * + * @param array $root + * + * @return array{0: bool, 1: mixed} + */ + public static function resolvePointer(string $pointer, array $root): array + { + return self::lookup($pointer, $root); + } + + /** + * Escape a single path segment for embedding in a JSON Pointer — the + * inverse of {@see self::unescapePointerSegment()}. `~` must be encoded + * before `/` so a literal `/` does not turn into `~1` and then have its + * `~` re-encoded. Used to build `#/components/schemas/{name}` from the + * OAS discriminator-mapping bare-name shorthand, where `{name}` may + * (exotically) contain `~` or `/`. + */ + public static function escapePointerSegment(string $segment): string + { + $segment = str_replace('~', '~0', $segment); + + return str_replace('/', '~1', $segment); + } + /** * @param array $node * @param array $root currently-walked document's root diff --git a/src/Spec/OpenApiSchemaConverter.php b/src/Spec/OpenApiSchemaConverter.php index b0c98e1..bb817cc 100644 --- a/src/Spec/OpenApiSchemaConverter.php +++ b/src/Spec/OpenApiSchemaConverter.php @@ -6,8 +6,11 @@ use const E_USER_WARNING; +use Studio\OpenApiContractTesting\Exception\MalformedDiscriminatorException; use Studio\OpenApiContractTesting\OpenApiVersion; use Studio\OpenApiContractTesting\SchemaContext; +use Studio\OpenApiContractTesting\Validation\Support\DiscriminatorContext; +use Studio\OpenApiContractTesting\Validation\Support\MalformedSpecNode; use function array_is_list; use function array_key_exists; @@ -17,7 +20,9 @@ use function is_array; use function is_bool; use function is_string; +use function sort; use function sprintf; +use function str_starts_with; use function trigger_error; /** @@ -37,7 +42,6 @@ final class OpenApiSchemaConverter * draft consistent with what the converter actually produces. */ private const OPENAPI_COMMON_KEYS = [ - 'discriminator', 'xml', 'externalDocs', 'example', @@ -138,6 +142,12 @@ final class OpenApiSchemaConverter * context the same happens for `writeOnly`. See `SchemaContext` for the * motivating OpenAPI semantics. * + * `$discriminator` carries the resolved root + enforce gate for + * `discriminator.mapping` lowering (Issue #262). `null` (the default for + * callers that cannot enforce — parameter / header validators, the fuzz + * explorer) is normalised to {@see DiscriminatorContext::disabled()}, under + * which `discriminator` is stripped rather than enforced. + * * @param array $schema * * @return array @@ -146,8 +156,9 @@ public static function convert( array $schema, OpenApiVersion $version = OpenApiVersion::V3_0, SchemaContext $context = SchemaContext::Response, + ?DiscriminatorContext $discriminator = null, ): array { - self::convertInPlace($schema, $version, $context); + self::convertInPlace($schema, $version, $context, $discriminator ?? DiscriminatorContext::disabled()); return $schema; } @@ -167,8 +178,12 @@ public static function resetWarningStateForTesting(): void /** * @param array $schema */ - private static function convertInPlace(array &$schema, OpenApiVersion $version, SchemaContext $context): void - { + private static function convertInPlace( + array &$schema, + OpenApiVersion $version, + SchemaContext $context, + DiscriminatorContext $discriminator, + ): void { if ($version === OpenApiVersion::V3_0) { self::handleNullable($schema); } else { @@ -182,15 +197,16 @@ private static function convertInPlace(array &$schema, OpenApiVersion $version, // just as risky there. self::warnIfUsesUnsupportedKeywords($schema); - // Run discriminator warning BEFORE the common-key removal below — - // once `discriminator` is stripped its `mapping` is gone, and the - // "this constraint was silently lost" signal would be impossible to - // emit. `format` is not in OPENAPI_COMMON_KEYS so its order is - // independent. - self::warnIfDiscriminatorMappingPresent($schema); self::warnIfDependentKeywordsPresent($schema); self::warnIfUnknownFormat($schema); + // `discriminator` is OAS-only (not a JSON Schema keyword) and is + // therefore no longer in OPENAPI_COMMON_KEYS — it is consumed by + // lowerDiscriminator() at the end of this method, which either lowers + // its `mapping` into enforceable `if`/`then` conditionals or strips it + // (see Issue #262). Running the lowering last lets it convert the + // resolved subtype it inlines into each `then` with the recursion guard + // active, without the generic combiner recursion below re-visiting it. self::removeKeys($schema, self::OPENAPI_COMMON_KEYS); // Enforce readOnly/writeOnly on this object's own `properties` before @@ -210,7 +226,7 @@ private static function convertInPlace(array &$schema, OpenApiVersion $version, if (isset($schema['properties']) && is_array($schema['properties'])) { foreach ($schema['properties'] as &$property) { if (is_array($property)) { - self::convertInPlace($property, $version, $context); + self::convertInPlace($property, $version, $context, $discriminator); } } unset($property); @@ -220,12 +236,12 @@ private static function convertInPlace(array &$schema, OpenApiVersion $version, if (array_is_list($schema['items'])) { foreach ($schema['items'] as &$item) { if (is_array($item)) { - self::convertInPlace($item, $version, $context); + self::convertInPlace($item, $version, $context, $discriminator); } } unset($item); } else { - self::convertInPlace($schema['items'], $version, $context); + self::convertInPlace($schema['items'], $version, $context, $discriminator); } } @@ -236,14 +252,14 @@ private static function convertInPlace(array &$schema, OpenApiVersion $version, // directly. None of the other recursion sites in convertInPlace // descend into it, so route it through here. if (isset($schema['additionalItems']) && is_array($schema['additionalItems'])) { - self::convertInPlace($schema['additionalItems'], $version, $context); + self::convertInPlace($schema['additionalItems'], $version, $context, $discriminator); } foreach (['allOf', 'oneOf', 'anyOf'] as $combiner) { if (isset($schema[$combiner]) && is_array($schema[$combiner])) { foreach ($schema[$combiner] as &$item) { if (is_array($item)) { - self::convertInPlace($item, $version, $context); + self::convertInPlace($item, $version, $context, $discriminator); } } unset($item); @@ -251,11 +267,11 @@ private static function convertInPlace(array &$schema, OpenApiVersion $version, } if (isset($schema['additionalProperties']) && is_array($schema['additionalProperties'])) { - self::convertInPlace($schema['additionalProperties'], $version, $context); + self::convertInPlace($schema['additionalProperties'], $version, $context, $discriminator); } if (isset($schema['not']) && is_array($schema['not'])) { - self::convertInPlace($schema['not'], $version, $context); + self::convertInPlace($schema['not'], $version, $context, $discriminator); } // Descend into the remaining subschema positions opis Draft 07 @@ -268,14 +284,14 @@ private static function convertInPlace(array &$schema, OpenApiVersion $version, // the other map-of-schemas positions. foreach (['if', 'then', 'else', 'propertyNames', 'contains'] as $key) { if (isset($schema[$key]) && is_array($schema[$key])) { - self::convertInPlace($schema[$key], $version, $context); + self::convertInPlace($schema[$key], $version, $context, $discriminator); } } if (isset($schema['patternProperties']) && is_array($schema['patternProperties'])) { foreach ($schema['patternProperties'] as &$sub) { if (is_array($sub)) { - self::convertInPlace($sub, $version, $context); + self::convertInPlace($sub, $version, $context, $discriminator); } } unset($sub); @@ -290,11 +306,19 @@ private static function convertInPlace(array &$schema, OpenApiVersion $version, // schema lowering — the same silent-defect class the rest // of this method exists to surface. if (is_array($sub) && !array_is_list($sub)) { - self::convertInPlace($sub, $version, $context); + self::convertInPlace($sub, $version, $context, $discriminator); } } unset($sub); } + + // Consume `discriminator` last: when enforcing, lower its `mapping` + // into `if`/`then` conditionals (resolving + converting each subtype + // it references); otherwise strip it. Running after the combiner + // recursion above means the lowered `allOf` entries we append are + // already converted by lowerDiscriminator itself and are not + // re-visited here. See Issue #262. + self::lowerDiscriminator($schema, $version, $context, $discriminator); } /** @@ -461,40 +485,211 @@ private static function warnIfUsesUnsupportedKeywords(array $schema): void } /** - * Issue a one-shot E_USER_WARNING when a schema declares a non-empty - * `discriminator.mapping`. The converter strips the entire `discriminator` - * keyword (it's OAS-only, not a JSON Schema keyword) before handing the - * schema to opis — the underlying `oneOf` / `anyOf` is still validated - * as a plain union, but the mapping does not steer validation toward a - * single branch. A polymorphic body with the wrong discriminator value - * passes as long as it satisfies any branch, masking serialiser bugs. - * - * Dedup key is the literal `discriminator.mapping` string — fired at - * most once per process regardless of how many polymorphic schemas - * carry the keyword. + * Lower a schema's `discriminator` into enforceable Draft-07 conditionals, + * or strip it when enforcement is off (Issue #262). + * + * `discriminator` is OAS-only — opis (Draft 07) cannot interpret it, so + * historically the converter stripped it and the underlying `oneOf` / + * `anyOf` was validated as a plain union: a body that lied about its type + * (e.g. `kty=RSA` carrying EC-only fields) passed as long as it matched any + * branch. When `$discriminator->enforce` is on, this rewrites the block + * into an `allOf` the union-agnostic validator DOES honour: + * + * - an unknown-value guard requiring the discriminator property to be + * present and one of the mapping keys; and + * - one `if`/`then` per mapping value, where `then` is the resolved + * subtype schema — so the discriminator value steers validation toward a + * single branch. + * + * Each resolved subtype is itself run through {@see convertInPlace()} with + * the discriminator's signature added to the recursion guard: `$ref` is + * eagerly inlined at load time, so a subtype typically re-contains the + * base's `discriminator` (`allOf:[{$ref base}, …]`) and an unguarded + * lowering would recurse forever. + * + * Structural defects (missing / non-string `propertyName`, non-array + * `mapping`, non-string mapping value, unresolvable pointer, non-object + * target) throw {@see MalformedDiscriminatorException} — caught by the body + * validators' boundary and surfaced as one loud failure, consistent with + * the {@see MalformedSpecNode} + * guards. A bare `propertyName`-only discriminator, an empty `mapping`, the + * off gate, and the no-root sentinel all strip silently (the historical + * behaviour, minus the removed warning). * * @param array $schema */ - private static function warnIfDiscriminatorMappingPresent(array $schema): void - { - if (!isset($schema['discriminator']) || !is_array($schema['discriminator'])) { + private static function lowerDiscriminator( + array &$schema, + OpenApiVersion $version, + SchemaContext $context, + DiscriminatorContext $discriminator, + ): void { + if (!array_key_exists('discriminator', $schema)) { return; } - $mapping = $schema['discriminator']['mapping'] ?? null; - if (!is_array($mapping) || $mapping === []) { + + // Off, or no root to resolve mapping pointers against → strip, matching + // the pre-#262 behaviour (now without the E_USER_WARNING). + if (!$discriminator->enforce || $discriminator->root === []) { + unset($schema['discriminator']); + return; } - $dedupKey = 'discriminator.mapping'; - if (isset(self::$warnedKeywords[$dedupKey])) { + $block = $schema['discriminator']; + if (!is_array($block)) { + throw new MalformedDiscriminatorException(sprintf( + "Malformed 'discriminator': expected object, got %s.", + get_debug_type($block), + )); + } + + $propertyName = $block['propertyName'] ?? null; + if (!is_string($propertyName) || $propertyName === '') { + throw new MalformedDiscriminatorException(sprintf( + "Malformed 'discriminator.propertyName': expected a non-empty string, got %s.", + get_debug_type($block['propertyName'] ?? null), + )); + } + + // A bare discriminator (no mapping) is documentation-only — strip it. + if (!array_key_exists('mapping', $block)) { + unset($schema['discriminator']); + return; } - self::$warnedKeywords[$dedupKey] = true; - trigger_error( - "[OpenAPI Schema] 'discriminator.mapping' is silently stripped — the underlying oneOf/anyOf is still validated as a plain union, but the mapping does not steer validation toward a single branch. A body with the wrong discriminator value passes as long as it satisfies any branch, masking serialiser bugs. See README \"Schema features\" for the full limitation note.", - E_USER_WARNING, - ); + $mapping = $block['mapping']; + if (!is_array($mapping)) { + throw new MalformedDiscriminatorException(sprintf( + "Malformed 'discriminator.mapping': expected object, got %s.", + get_debug_type($mapping), + )); + } + if ($mapping === []) { + unset($schema['discriminator']); + + return; + } + + // Self-referential re-appearance via eager $ref inlining: the outer + // lowering already enforces this exact discriminator on the matched + // branch, so strip without re-lowering. This both terminates the + // recursion and avoids combinatorial blow-up for large mappings. + $signature = self::discriminatorSignature($propertyName, $mapping); + if ($discriminator->hasSignature($signature)) { + unset($schema['discriminator']); + + return; + } + + $childContext = $discriminator->withSignature($signature); + $knownValues = []; + $branches = []; + foreach ($mapping as $value => $pointer) { + $value = (string) $value; + $knownValues[] = $value; + + if (!is_string($pointer)) { + throw new MalformedDiscriminatorException(sprintf( + "Malformed 'discriminator.mapping[%s]': expected a string reference, got %s.", + $value, + get_debug_type($pointer), + )); + } + + $subschema = self::resolveMappingTarget($value, $pointer, $discriminator->root); + self::convertInPlace($subschema, $version, $context, $childContext); + + $branches[] = [ + 'if' => [ + 'properties' => [$propertyName => ['enum' => [$value]]], + 'required' => [$propertyName], + ], + 'then' => $subschema, + ]; + } + + $allOf = (isset($schema['allOf']) && is_array($schema['allOf'])) ? $schema['allOf'] : []; + // Unknown-value guard first so its failure surfaces before the + // per-branch conditionals: the discriminator property must be present + // and one of the declared mapping keys. + $allOf[] = [ + 'properties' => [$propertyName => ['enum' => $knownValues]], + 'required' => [$propertyName], + ]; + foreach ($branches as $branch) { + $allOf[] = $branch; + } + $schema['allOf'] = $allOf; + + unset($schema['discriminator']); + } + + /** + * Resolve a single `discriminator.mapping` value to its subtype schema in + * the root document. The value is either a JSON Pointer (`#/...`) or the + * OAS bare-name shorthand, which resolves to `#/components/schemas/{name}`. + * Reuses {@see OpenApiRefResolver}'s pointer logic so escape handling stays + * in one place. + * + * @param array $root + * + * @return array + */ + private static function resolveMappingTarget(string $value, string $pointer, array $root): array + { + $jsonPointer = str_starts_with($pointer, '#/') + ? $pointer + : '#/components/schemas/' . OpenApiRefResolver::escapePointerSegment($pointer); + + [$found, $target] = OpenApiRefResolver::resolvePointer($jsonPointer, $root); + if (!$found) { + throw new MalformedDiscriminatorException(sprintf( + "Malformed 'discriminator.mapping[%s]': '%s' does not resolve to a schema in the spec.", + $value, + $pointer, + )); + } + if (!is_array($target)) { + throw new MalformedDiscriminatorException(sprintf( + "Malformed 'discriminator.mapping[%s]': '%s' must reference a schema object, got %s.", + $value, + $pointer, + get_debug_type($target), + )); + } + + /** @var array $target */ + return $target; + } + + /** + * Stable identity for a discriminator: its property name plus the sorted + * set of `key => target` mapping pairs. The recursion guard uses it to + * detect the SAME discriminator re-appearing through eager `$ref` inlining + * (the base↔subtype cycle), independent of declaration order. Folding the + * resolved target into each pair means two genuinely distinct + * discriminators that merely share a property name and key set do NOT + * collide — only an identical mapping (the self-reference case) matches, so + * a nested *distinct* discriminator is still lowered rather than silently + * stripped (which would be a silent under-enforcement). + * + * A non-string mapping value is folded in by its type token; such a value + * is rejected with a loud throw later in the lowering, so the rendering + * here only needs to be stable, not meaningful. + * + * @param array $mapping + */ + private static function discriminatorSignature(string $propertyName, array $mapping): string + { + $pairs = []; + foreach ($mapping as $key => $value) { + $pairs[] = (string) $key . "\0" . (is_string($value) ? $value : get_debug_type($value)); + } + sort($pairs); + + return $propertyName . "\0" . implode("\0", $pairs); } /** diff --git a/src/Validation/Request/RequestBodyValidator.php b/src/Validation/Request/RequestBodyValidator.php index f83bac3..ce52f89 100644 --- a/src/Validation/Request/RequestBodyValidator.php +++ b/src/Validation/Request/RequestBodyValidator.php @@ -10,6 +10,7 @@ use Studio\OpenApiContractTesting\SchemaContext; use Studio\OpenApiContractTesting\Spec\OpenApiSchemaConverter; use Studio\OpenApiContractTesting\Validation\Support\ContentTypeMatcher; +use Studio\OpenApiContractTesting\Validation\Support\DiscriminatorContext; use Studio\OpenApiContractTesting\Validation\Support\MalformedSpecNode; use Studio\OpenApiContractTesting\Validation\Support\ObjectConverter; use Studio\OpenApiContractTesting\Validation\Support\SchemaValidatorRunner; @@ -44,6 +45,10 @@ public function __construct( * an empty `errors` list plus a non-null `skipReason` (issue #254). * * @param array $operation + * @param null|DiscriminatorContext $discriminatorContext carries the resolved root + enforce gate + * for `discriminator.mapping` lowering (Issue + * #262). `null` (the default for direct + * callers) means no enforcement. */ public function validate( string $specName, @@ -53,6 +58,7 @@ public function validate( DecodedBody $requestBody, ?string $contentType, OpenApiVersion $version, + ?DiscriminatorContext $discriminatorContext = null, ): RequestBodyValidationResult { // OpenAPI: a missing requestBody means the operation accepts no body — treat as success. if (!isset($operation['requestBody'])) { @@ -218,7 +224,7 @@ public function validate( /** @var array $schema */ $schema = $content[$jsonContentType]['schema']; - $jsonSchema = OpenApiSchemaConverter::convert($schema, $version, SchemaContext::Request); + $jsonSchema = OpenApiSchemaConverter::convert($schema, $version, SchemaContext::Request, $discriminatorContext); // PHP's `json_decode($json, true)` returns `[]` for both `[]` and `{}`. // The Laravel adapter's request decoder uses associative-array decoding, diff --git a/src/Validation/Response/ResponseBodyValidator.php b/src/Validation/Response/ResponseBodyValidator.php index 1fb64e6..b9a0750 100644 --- a/src/Validation/Response/ResponseBodyValidator.php +++ b/src/Validation/Response/ResponseBodyValidator.php @@ -12,6 +12,7 @@ use Studio\OpenApiContractTesting\SchemaContext; use Studio\OpenApiContractTesting\Spec\OpenApiSchemaConverter; use Studio\OpenApiContractTesting\Validation\Support\ContentTypeMatcher; +use Studio\OpenApiContractTesting\Validation\Support\DiscriminatorContext; use Studio\OpenApiContractTesting\Validation\Support\MalformedSpecNode; use Studio\OpenApiContractTesting\Validation\Support\ObjectConverter; use Studio\OpenApiContractTesting\Validation\Support\SchemaValidatorRunner; @@ -55,6 +56,10 @@ public function __construct( * Values are media-type objects, but a malformed spec can * carry a scalar — the guard loop below rejects those loudly * before any value is dereferenced as an array. + * @param null|DiscriminatorContext $discriminatorContext carries the resolved root + enforce gate + * for `discriminator.mapping` lowering (Issue + * #262). `null` (the default for direct + * callers) means no enforcement. */ public function validate( string $specName, @@ -65,6 +70,7 @@ public function validate( DecodedBody $responseBody, ?string $responseContentType, OpenApiVersion $version, + ?DiscriminatorContext $discriminatorContext = null, ): ResponseBodyValidationResult { // Pre-scan the content map for malformed media-type entries before any // content negotiation runs. This mirrors RequestBodyValidator's @@ -200,7 +206,7 @@ public function validate( /** @var array $schema */ $schema = $content[$jsonContentType]['schema']; - $jsonSchema = OpenApiSchemaConverter::convert($schema, $version, SchemaContext::Response); + $jsonSchema = OpenApiSchemaConverter::convert($schema, $version, SchemaContext::Response, $discriminatorContext); // PHP's `json_decode($json, true)` returns `[]` for both `[]` and `{}`. // The Laravel trait's response decoder uses associative-array decoding diff --git a/src/Validation/Support/DiscriminatorContext.php b/src/Validation/Support/DiscriminatorContext.php new file mode 100644 index 0000000..ab20437 --- /dev/null +++ b/src/Validation/Support/DiscriminatorContext.php @@ -0,0 +1,79 @@ + $root the resolved root spec, used only for pointer lookups + * @param array $activeSignatures set of discriminator signatures already being enforced on the current path + */ + public function __construct( + public array $root = [], + public bool $enforce = false, + public array $activeSignatures = [], + ) {} + + /** + * Sentinel for call sites that cannot (or should not) enforce: no root to + * resolve mapping pointers against and enforcement off, so the converter + * falls back to stripping `discriminator`. + */ + public static function disabled(): self + { + return new self(); + } + + /** + * Return a copy with `$signature` added to the active set — threaded into + * the recursive conversion of a lowered `then` branch so a self-referential + * subtype does not re-open the same discriminator. + */ + public function withSignature(string $signature): self + { + $signatures = $this->activeSignatures; + $signatures[$signature] = true; + + return new self($this->root, $this->enforce, $signatures); + } + + /** + * True when `$signature` is already being enforced higher up the + * recursion — the signal to strip the re-appearing discriminator instead + * of lowering it again. + */ + public function hasSignature(string $signature): bool + { + return isset($this->activeSignatures[$signature]); + } +} diff --git a/src/Validation/Support/DiscriminatorEnforcement.php b/src/Validation/Support/DiscriminatorEnforcement.php new file mode 100644 index 0000000..23b2f2d --- /dev/null +++ b/src/Validation/Support/DiscriminatorEnforcement.php @@ -0,0 +1,70 @@ +responseValidator = new OpenApiResponseValidator(); $this->requestValidator = new OpenApiRequestValidator(); @@ -58,6 +62,7 @@ protected function tearDown(): void OpenApiCoverageTracker::reset(); OpenApiSpecLoader::reset(); OpenApiSchemaConverter::resetWarningStateForTesting(); + DiscriminatorEnforcement::reset(); parent::tearDown(); } @@ -68,31 +73,28 @@ protected function tearDown(): void #[Test] public function composition_oneof_accepts_each_branch_independently(): void { - // /v1/pets/oneOf carries discriminator.mapping for documentation - // purposes; the schema converter now warns once per process about it. - // This test focuses on the union-validates-each-branch behaviour, so - // the warning is suppressed locally — it's covered explicitly by - // composition_discriminator_mapping_is_silently_stripped. - [$cat, $dog] = $this->suppressSchemaWarnings(['discriminator.mapping'], fn() => [ - $this->requestValidator->validate( - 'composition', - 'POST', - '/v1/pets/oneOf', - [], - [], - ['kind' => 'cat', 'meow' => true], - 'application/json', - ), - $this->requestValidator->validate( - 'composition', - 'POST', - '/v1/pets/oneOf', - [], - [], - ['kind' => 'dog', 'bark' => false], - 'application/json', - ), - ]); + // /v1/pets/oneOf carries discriminator.mapping. With enforcement on + // (the default, #262) a valid Cat / Dog body still passes — it + // satisfies both the union and the discriminator-selected branch. + // No warning is emitted any more, so no suppression is needed. + $cat = $this->requestValidator->validate( + 'composition', + 'POST', + '/v1/pets/oneOf', + [], + [], + ['kind' => 'cat', 'meow' => true], + 'application/json', + ); + $dog = $this->requestValidator->validate( + 'composition', + 'POST', + '/v1/pets/oneOf', + [], + [], + ['kind' => 'dog', 'bark' => false], + 'application/json', + ); $this->assertTrue($cat->isValid(), 'cat branch: ' . implode(' | ', $cat->errors())); $this->assertTrue($dog->isValid(), 'dog branch: ' . implode(' | ', $dog->errors())); @@ -101,9 +103,9 @@ public function composition_oneof_accepts_each_branch_independently(): void #[Test] public function composition_oneof_rejects_body_matching_no_branch(): void { - // Neither shape: no `kind`, no `meow` / `bark`. discriminator.mapping - // warning suppressed — see composition_oneof_accepts_each_branch_independently. - $result = $this->suppressSchemaWarnings(['discriminator.mapping'], fn() => $this->requestValidator->validate( + // Neither shape: no `kind`, no `meow` / `bark`. Fails both the union + // and the discriminator unknown-value guard. + $result = $this->requestValidator->validate( 'composition', 'POST', '/v1/pets/oneOf', @@ -111,28 +113,24 @@ public function composition_oneof_rejects_body_matching_no_branch(): void [], ['unrelated' => 'value'], 'application/json', - )); + ); $this->assertFalse($result->isValid()); $this->assertNotEmpty($result->errors()); } #[Test] - public function composition_discriminator_mapping_is_silently_stripped(): void + public function composition_discriminator_mapping_is_enforced_without_warning(): void { - // README explicitly documents: `discriminator.mapping` is dropped, the - // underlying oneOf still validates as a plain union. Pin this so a - // future change that wires up real mapping support surfaces here as - // a behaviour change. A body whose `kind` would route to the WRONG - // branch (Cat) by mapping but matches the OTHER branch's shape (Dog) - // would FAIL with mapping enforced — but PASS today because mapping - // is stripped and the body is judged against the oneOf union. - // - // The strip now also fires a one-shot E_USER_WARNING; capture it here - // so the silent-pass-plus-warning contract is pinned end-to-end. + // Behaviour change (#262): `discriminator.mapping` is now lowered into + // enforceable if/then conditionals instead of being stripped with a + // one-shot E_USER_WARNING. A truthful body routes to and satisfies its + // mapped branch, and — critically for Laravel consumers whose error + // handler turns E_USER_WARNING into a fatal ErrorException — NO warning + // is emitted. $captured = null; set_error_handler(static function (int $errno, string $errstr) use (&$captured): bool { - if ($errno === E_USER_WARNING && str_contains($errstr, 'discriminator.mapping')) { + if ($errno === E_USER_WARNING && str_contains($errstr, 'discriminator')) { $captured = $errstr; return true; @@ -148,10 +146,6 @@ public function composition_discriminator_mapping_is_silently_stripped(): void '/v1/pets/oneOf', [], [], - // kind=dog routes to Dog by mapping; body is a valid Dog. - // If mapping were enforced and kind said "cat", we would route - // to Cat and fail. As-is, the body passes the union because - // it satisfies Dog's branch. ['kind' => 'dog', 'bark' => true], 'application/json', ); @@ -160,7 +154,112 @@ public function composition_discriminator_mapping_is_silently_stripped(): void } $this->assertTrue($result->isValid(), implode(' | ', $result->errors())); - $this->assertNotNull($captured, 'discriminator.mapping strip must fire its silent-pass warning'); + $this->assertNull($captured, 'discriminator.mapping must no longer emit an E_USER_WARNING'); + } + + // ============================================================ + // jwks.json — implicit-inheritance discriminator enforcement (#262) + // ============================================================ + + #[Test] + public function jwks_valid_rsa_key_passes(): void + { + // kty=RSA routes to RsaJsonWebKey (requires n, e) — a truthful RSA key + // validates. + $result = $this->responseValidator->validate( + 'jwks', + 'GET', + '/v1/jwks', + 200, + ['kty' => 'RSA', 'n' => 'sXch…', 'e' => 'AQAB'], + 'application/json', + ); + + $this->assertTrue($result->isValid(), implode(' | ', $result->errors())); + } + + #[Test] + public function jwks_valid_ec_key_passes(): void + { + $result = $this->responseValidator->validate( + 'jwks', + 'GET', + '/v1/jwks', + 200, + ['kty' => 'EC', 'crv' => 'P-256', 'x' => 'f83O…', 'y' => 'x_FE…'], + 'application/json', + ); + + $this->assertTrue($result->isValid(), implode(' | ', $result->errors())); + } + + #[Test] + public function jwks_lying_type_fails(): void + { + // The issue #262 motivating case: a body that claims kty=RSA but + // carries EC-only fields (and lacks n / e) passes the loose union but + // MUST fail with the mapping enforced — RSA routes to RsaJsonWebKey, + // whose required n / e are absent. + $result = $this->responseValidator->validate( + 'jwks', + 'GET', + '/v1/jwks', + 200, + ['kty' => 'RSA', 'crv' => 'P-256', 'x' => 'f83O…', 'y' => 'x_FE…'], + 'application/json', + ); + + $this->assertFalse($result->isValid(), 'a body lying about its key type must fail'); + $this->assertNotEmpty($result->errors()); + } + + #[Test] + public function jwks_unknown_kty_fails(): void + { + // kty=oct is not in the mapping — the unknown-value guard rejects it. + $result = $this->responseValidator->validate( + 'jwks', + 'GET', + '/v1/jwks', + 200, + ['kty' => 'oct', 'k' => 'GawgguF…'], + 'application/json', + ); + + $this->assertFalse($result->isValid(), 'an unknown discriminator value must fail'); + $this->assertNotEmpty($result->errors()); + } + + #[Test] + public function jwks_request_body_enforces_discriminator(): void + { + // The request-side path enforces the mapping too. Unlike composition's + // self-constraining Cat/Dog, the JWKS subtypes do NOT pin `kty` + // themselves, so a lying body passes the bare union and is caught ONLY + // by the lowered discriminator — proving request-side enforcement + // rather than incidental union rejection. + $valid = $this->requestValidator->validate( + 'jwks', + 'POST', + '/v1/jwks', + [], + [], + ['kty' => 'RSA', 'n' => 'sXch…', 'e' => 'AQAB'], + 'application/json', + ); + $this->assertTrue($valid->isValid(), implode(' | ', $valid->errors())); + + $lying = $this->requestValidator->validate( + 'jwks', + 'POST', + '/v1/jwks', + [], + [], + ['kty' => 'RSA', 'crv' => 'P-256', 'x' => 'f83O…', 'y' => 'x_FE…'], + 'application/json', + ); + $this->assertFalse($lying->isValid(), 'request body lying about its key type must fail'); + $this->assertNotEmpty($lying->errors()); } #[Test] @@ -841,45 +940,4 @@ private function validProfile(array $overrides = []): array 'userId' => '01234567-89ab-cdef-0123-456789abcdef', ]; } - - /** - * Run a callable while absorbing only the converter warnings whose - * messages match one of the explicitly-allowed substrings. Any other - * `E_USER_WARNING` falls through (returns `false` from the handler) so - * unrelated warnings still surface — preventing this helper from - * silently absorbing future converter warnings the test was not written - * to cover. - * - * Earlier revisions of this helper filtered by the broad `[OpenAPI - * Schema]` prefix and would have silently absorbed every converter - * warning emitted in the wrapped call. A code-review audit flagged - * that as a silent-failure trap; the explicit allowlist keeps the - * suppression intent-driven and self-documenting at the call site. - * - * @param string[] $allowedSubstrings non-empty list of substrings. - * A warning is absorbed iff its - * message contains at least one. - */ - private function suppressSchemaWarnings(array $allowedSubstrings, callable $fn): mixed - { - set_error_handler(static function (int $errno, string $errstr) use ($allowedSubstrings): bool { - if ($errno !== E_USER_WARNING) { - return false; - } - - foreach ($allowedSubstrings as $needle) { - if (str_contains($errstr, $needle)) { - return true; - } - } - - return false; - }); - - try { - return $fn(); - } finally { - restore_error_handler(); - } - } } diff --git a/tests/Unit/PHPUnit/OpenApiCoverageExtensionTest.php b/tests/Unit/PHPUnit/OpenApiCoverageExtensionTest.php index 54db84a..e73b814 100644 --- a/tests/Unit/PHPUnit/OpenApiCoverageExtensionTest.php +++ b/tests/Unit/PHPUnit/OpenApiCoverageExtensionTest.php @@ -24,6 +24,7 @@ use Studio\OpenApiContractTesting\Validation\Strict\StrictRequiredPerCallChecker; use Studio\OpenApiContractTesting\Validation\Strict\StrictRequiredPerCallMode; use Studio\OpenApiContractTesting\Validation\Strict\StrictRequiredTracker; +use Studio\OpenApiContractTesting\Validation\Support\DiscriminatorEnforcement; use function fclose; use function file_get_contents; @@ -1129,6 +1130,49 @@ public function strict_required_per_call_absent_keeps_checker_off(): void } } + #[Test] + public function enforce_discriminator_false_disables_gate_at_bootstrap(): void + { + DiscriminatorEnforcement::reset(); + + $extension = new OpenApiCoverageExtension(); + $parameters = ParameterCollection::fromArray([ + 'spec_base_path' => __DIR__ . '/../../fixtures/specs', + 'specs' => 'refs-valid', + 'enforce_discriminator' => 'false', + ]); + + try { + $extension->setupExtension(null, $parameters, null); + $this->assertFalse(DiscriminatorEnforcement::isEnabled()); + } finally { + DiscriminatorEnforcement::reset(); + } + } + + #[Test] + public function enforce_discriminator_absent_keeps_gate_enabled(): void + { + // Default ON (#262): the headline behaviour must stay enabled when the + // parameter is omitted. Reset to a non-default first so the assertion + // proves bootstrap re-enabled it rather than merely inheriting the + // default. + DiscriminatorEnforcement::configure(false); + + $extension = new OpenApiCoverageExtension(); + $parameters = ParameterCollection::fromArray([ + 'spec_base_path' => __DIR__ . '/../../fixtures/specs', + 'specs' => 'refs-valid', + ]); + + try { + $extension->setupExtension(null, $parameters, null); + $this->assertTrue(DiscriminatorEnforcement::isEnabled()); + } finally { + DiscriminatorEnforcement::reset(); + } + } + #[Test] public function strict_required_per_call_unknown_value_throws_invalid_config(): void { diff --git a/tests/Unit/Spec/OpenApiSchemaConverterTest.php b/tests/Unit/Spec/OpenApiSchemaConverterTest.php index fb88555..05443d2 100644 --- a/tests/Unit/Spec/OpenApiSchemaConverterTest.php +++ b/tests/Unit/Spec/OpenApiSchemaConverterTest.php @@ -8,9 +8,11 @@ use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\TestCase; +use Studio\OpenApiContractTesting\Exception\MalformedDiscriminatorException; use Studio\OpenApiContractTesting\OpenApiVersion; use Studio\OpenApiContractTesting\SchemaContext; use Studio\OpenApiContractTesting\Spec\OpenApiSchemaConverter; +use Studio\OpenApiContractTesting\Validation\Support\DiscriminatorContext; use Studio\OpenApiContractTesting\Validation\Support\ObjectConverter; use Studio\OpenApiContractTesting\Validation\Support\SchemaValidatorRunner; @@ -1379,22 +1381,22 @@ public function schema_without_dependent_keywords_emits_no_warning(): void } // ======================================== - // discriminator.mapping silent-strip warning (#147) + // discriminator.mapping enforcement via if/then lowering (#262) // ======================================== #[Test] - public function discriminator_with_mapping_emits_warning(): void + public function discriminator_with_mapping_lowers_to_if_then_when_enforced(): void { - // discriminator.mapping is dropped during conversion (it's OAS-only, - // not a JSON Schema keyword); the underlying oneOf still validates - // as a plain union. Without the warning, a body with the wrong - // discriminator value would silently pass as long as it satisfies - // any branch — masking serialiser bugs. + // With enforcement on, discriminator + mapping is rewritten into an + // allOf of an unknown-value guard plus one if/then per mapping value, + // so the discriminator value actually steers validation toward a single + // branch — the gap #147's warning only narrated, now closed. + $root = ['components' => ['schemas' => [ + 'Cat' => ['type' => 'object', 'required' => ['meow'], 'properties' => ['meow' => ['type' => 'boolean']]], + 'Dog' => ['type' => 'object', 'required' => ['bark'], 'properties' => ['bark' => ['type' => 'boolean']]], + ]]]; $schema = [ - 'oneOf' => [ - ['type' => 'object', 'properties' => ['kind' => ['enum' => ['cat']]]], - ['type' => 'object', 'properties' => ['kind' => ['enum' => ['dog']]]], - ], + 'type' => 'object', 'discriminator' => [ 'propertyName' => 'kind', 'mapping' => [ @@ -1404,11 +1406,271 @@ public function discriminator_with_mapping_emits_warning(): void ], ]; + $result = OpenApiSchemaConverter::convert($schema, OpenApiVersion::V3_0, SchemaContext::Response, $this->enforcing($root)); + + $this->assertArrayNotHasKey('discriminator', $result); + $this->assertArrayHasKey('allOf', $result); + $this->assertCount(3, $result['allOf']); + // [0] unknown-value guard: property present and one of the mapping keys. + $this->assertSame(['kind' => ['enum' => ['cat', 'dog']]], $result['allOf'][0]['properties']); + $this->assertSame(['kind'], $result['allOf'][0]['required']); + // [1]/[2] per-value if/then routing to the resolved subtype. + $this->assertSame(['kind' => ['enum' => ['cat']]], $result['allOf'][1]['if']['properties']); + $this->assertSame(['kind'], $result['allOf'][1]['if']['required']); + $this->assertSame(['meow'], $result['allOf'][1]['then']['required']); + $this->assertSame(['kind' => ['enum' => ['dog']]], $result['allOf'][2]['if']['properties']); + $this->assertSame(['bark'], $result['allOf'][2]['then']['required']); + } + + #[Test] + public function discriminator_mapping_accepts_bare_schema_name_shorthand(): void + { + // OAS allows a mapping value to be a bare schema name, shorthand for + // `#/components/schemas/{name}`. + $root = ['components' => ['schemas' => [ + 'Cat' => ['type' => 'object', 'required' => ['meow']], + ]]]; + $schema = [ + 'type' => 'object', + 'discriminator' => ['propertyName' => 'kind', 'mapping' => ['cat' => 'Cat']], + ]; + + $result = OpenApiSchemaConverter::convert($schema, OpenApiVersion::V3_0, SchemaContext::Response, $this->enforcing($root)); + + $this->assertSame(['meow'], $result['allOf'][1]['then']['required']); + } + + #[Test] + public function discriminator_then_subschema_is_recursively_converted(): void + { + // The resolved subtype is raw OAS and must itself be lowered — a 3.0 + // `nullable` inside it becomes a type array, not survive untouched. + $root = ['components' => ['schemas' => [ + 'Cat' => [ + 'type' => 'object', + 'required' => ['meow'], + 'properties' => ['meow' => ['type' => 'string', 'nullable' => true]], + ], + ]]]; + $schema = [ + 'type' => 'object', + 'discriminator' => ['propertyName' => 'kind', 'mapping' => ['cat' => '#/components/schemas/Cat']], + ]; + + $result = OpenApiSchemaConverter::convert($schema, OpenApiVersion::V3_0, SchemaContext::Response, $this->enforcing($root)); + + $then = $result['allOf'][1]['then']; + $this->assertSame(['string', 'null'], $then['properties']['meow']['type']); + $this->assertArrayNotHasKey('nullable', $then['properties']['meow']); + } + + #[Test] + public function discriminator_with_sibling_oneof_is_preserved(): void + { + // When discriminator accompanies a oneOf, the union survives and the + // lowered branches are appended alongside it (body must satisfy both). + $root = ['components' => ['schemas' => [ + 'Cat' => ['type' => 'object', 'required' => ['meow']], + ]]]; + $schema = [ + 'oneOf' => [['type' => 'object']], + 'discriminator' => ['propertyName' => 'kind', 'mapping' => ['cat' => '#/components/schemas/Cat']], + ]; + + $result = OpenApiSchemaConverter::convert($schema, OpenApiVersion::V3_0, SchemaContext::Response, $this->enforcing($root)); + + $this->assertArrayHasKey('oneOf', $result); + $this->assertArrayHasKey('allOf', $result); + $this->assertCount(2, $result['allOf']); // guard + cat branch + } + + #[Test] + public function discriminator_self_referential_cycle_terminates_and_degrades(): void + { + // Eager $ref inlining means a subtype re-contains the base discriminator + // (RsaJsonWebKey = allOf:[{inlined base}, {required:[n,e]}]). The + // recursion guard strips the re-appearing discriminator instead of + // re-lowering it — terminating without blow-up. + $baseDiscriminator = [ + 'propertyName' => 'kty', + 'mapping' => ['RSA' => '#/components/schemas/Rsa', 'EC' => '#/components/schemas/Ec'], + ]; + $inlinedBase = [ + 'type' => 'object', + 'required' => ['kty'], + 'properties' => ['kty' => ['enum' => ['RSA', 'EC']]], + 'discriminator' => $baseDiscriminator, + ]; + $root = ['components' => ['schemas' => [ + 'Rsa' => ['allOf' => [$inlinedBase, ['type' => 'object', 'required' => ['n', 'e']]]], + 'Ec' => ['allOf' => [$inlinedBase, ['type' => 'object', 'required' => ['crv', 'x', 'y']]]], + ]]]; + $schema = [ + 'type' => 'object', + 'required' => ['kty'], + 'properties' => ['kty' => ['enum' => ['RSA', 'EC']]], + 'discriminator' => $baseDiscriminator, + ]; + + $result = OpenApiSchemaConverter::convert($schema, OpenApiVersion::V3_0, SchemaContext::Response, $this->enforcing($root)); + + // Top level lowered, both branches present. + $this->assertCount(3, $result['allOf']); + // The RSA branch's resolved subtype: its inlined base discriminator was + // stripped (no re-lowering), leaving the required[n,e] constraint. + $rsaThen = $result['allOf'][1]['then']; + $this->assertArrayNotHasKey('discriminator', $rsaThen['allOf'][0]); + $this->assertSame(['n', 'e'], $rsaThen['allOf'][1]['required']); + } + + #[Test] + public function discriminator_enforced_lowering_rejects_lying_body_via_opis(): void + { + // End-to-end: the lowered schema, run through opis, fails a body that + // lies about its type (the issue #262 motivating case) and passes a + // truthful one. + $root = ['components' => ['schemas' => [ + 'Cat' => ['type' => 'object', 'required' => ['meow'], 'properties' => ['meow' => ['type' => 'boolean']]], + 'Dog' => ['type' => 'object', 'required' => ['bark'], 'properties' => ['bark' => ['type' => 'boolean']]], + ]]]; + $schema = [ + 'type' => 'object', + 'discriminator' => [ + 'propertyName' => 'kind', + 'mapping' => ['cat' => '#/components/schemas/Cat', 'dog' => '#/components/schemas/Dog'], + ], + ]; + $lowered = OpenApiSchemaConverter::convert($schema, OpenApiVersion::V3_0, SchemaContext::Response, $this->enforcing($root)); + + $runner = new SchemaValidatorRunner(20); + $schemaObject = ObjectConverter::convert($lowered); + + $this->assertSame( + [], + $runner->validate($schemaObject, ObjectConverter::convert((object) ['kind' => 'cat', 'meow' => true])), + 'a truthful cat body must validate', + ); + $this->assertNotSame( + [], + $runner->validate($schemaObject, ObjectConverter::convert((object) ['kind' => 'cat', 'bark' => true])), + 'a body claiming kind=cat but carrying Dog-only fields must fail', + ); + $this->assertNotSame( + [], + $runner->validate($schemaObject, ObjectConverter::convert((object) ['kind' => 'fish', 'meow' => true])), + 'an unknown discriminator value must fail the guard', + ); + } + + #[Test] + public function discriminator_nested_distinct_discriminators_both_lower(): void + { + // Regression for the recursion-guard signature: a nested discriminator + // that shares the outer's propertyName AND key set but maps to DIFFERENT + // targets must still be lowered (not stripped as a false self-reference). + // The signature folds in the resolved targets so the two do not collide. + $root = ['components' => ['schemas' => [ + 'OuterA' => [ + 'type' => 'object', + 'discriminator' => [ + 'propertyName' => 'type', + 'mapping' => ['a' => '#/components/schemas/Inner1', 'b' => '#/components/schemas/Inner2'], + ], + ], + 'OuterB' => ['type' => 'object'], + 'Inner1' => ['type' => 'object', 'required' => ['i1']], + 'Inner2' => ['type' => 'object', 'required' => ['i2']], + ]]]; + $schema = [ + 'type' => 'object', + 'discriminator' => [ + 'propertyName' => 'type', + 'mapping' => ['a' => '#/components/schemas/OuterA', 'b' => '#/components/schemas/OuterB'], + ], + ]; + + $result = OpenApiSchemaConverter::convert($schema, OpenApiVersion::V3_0, SchemaContext::Response, $this->enforcing($root)); + + // The "a" branch routes to OuterA, whose OWN (distinct) discriminator + // must be lowered — not stripped — because its signature differs. + $outerAThen = $result['allOf'][1]['then']; + $this->assertArrayNotHasKey('discriminator', $outerAThen); + $this->assertArrayHasKey('allOf', $outerAThen, 'nested distinct discriminator must be lowered, not stripped'); + $this->assertSame(['i1'], $outerAThen['allOf'][1]['then']['required']); + $this->assertSame(['i2'], $outerAThen['allOf'][2]['then']['required']); + } + + #[Test] + public function discriminator_with_mapping_not_enforced_with_root_strips(): void + { + // The off path with a POPULATED root (the production "user set the flag + // off" shape, distinct from the empty-root disabled() sentinel): + // discriminator is stripped, nothing is lowered. + $root = ['components' => ['schemas' => [ + 'Cat' => ['type' => 'object', 'required' => ['meow']], + ]]]; + $schema = [ + 'type' => 'object', + 'discriminator' => ['propertyName' => 'kind', 'mapping' => ['cat' => '#/components/schemas/Cat']], + ]; + + $result = OpenApiSchemaConverter::convert($schema, OpenApiVersion::V3_0, SchemaContext::Response, new DiscriminatorContext($root, false)); + + $this->assertArrayNotHasKey('discriminator', $result); + $this->assertArrayNotHasKey('allOf', $result); + } + + #[Test] + public function discriminator_nullable_base_rejects_null_body_via_opis(): void + { + // Documented contract: a 3.0 `nullable` base carrying a discriminator + // enforces the discriminated-object branch — a `null` body fails the + // lowered guard (which requires the discriminator property). + $root = ['components' => ['schemas' => [ + 'Cat' => ['type' => 'object', 'required' => ['meow'], 'properties' => ['meow' => ['type' => 'boolean']]], + ]]]; + $schema = [ + 'type' => 'object', + 'nullable' => true, + 'discriminator' => ['propertyName' => 'kind', 'mapping' => ['cat' => '#/components/schemas/Cat']], + ]; + $lowered = OpenApiSchemaConverter::convert($schema, OpenApiVersion::V3_0, SchemaContext::Response, $this->enforcing($root)); + + $runner = new SchemaValidatorRunner(20); + $schemaObject = ObjectConverter::convert($lowered); + + $this->assertNotSame( + [], + $runner->validate($schemaObject, ObjectConverter::convert(null)), + 'a null body must fail the discriminated-object branch', + ); + $this->assertSame( + [], + $runner->validate($schemaObject, ObjectConverter::convert((object) ['kind' => 'cat', 'meow' => true])), + 'a valid cat body still validates', + ); + } + + #[Test] + public function discriminator_with_mapping_not_enforced_strips_without_warning(): void + { + // Default (no DiscriminatorContext) / disabled gate: discriminator is + // stripped silently — the historical behaviour, now without the warning + // (which was fatal under Laravel's error handler, #262). + $schema = [ + 'oneOf' => [['type' => 'object']], + 'discriminator' => [ + 'propertyName' => 'kind', + 'mapping' => ['cat' => '#/components/schemas/Cat'], + ], + ]; + $warnings = $this->captureWarnings(static fn() => OpenApiSchemaConverter::convert($schema)); + $result = OpenApiSchemaConverter::convert($schema); - $this->assertCount(1, $warnings); - $this->assertStringContainsString('discriminator.mapping', $warnings[0]); - $this->assertStringContainsString('silently stripped', $warnings[0]); + $this->assertSame([], $warnings, 'no warning when not enforcing'); + $this->assertArrayNotHasKey('discriminator', $result); + $this->assertArrayNotHasKey('allOf', $result, 'no lowering when not enforcing'); } #[Test] @@ -1452,39 +1714,86 @@ public function discriminator_with_empty_mapping_does_not_warn(): void } #[Test] - public function discriminator_mapping_warns_only_once_per_process(): void + public function discriminator_missing_property_name_throws_when_enforced(): void { - // Avoid log spam: the mapping warning fires once per process even if - // dozens of polymorphic schemas in one spec carry it. (setUp() - // resets the warned-set.) - $schema = [ - 'oneOf' => [['type' => 'object']], - 'discriminator' => [ - 'propertyName' => 'kind', - 'mapping' => ['a' => '#/components/schemas/A'], - ], - ]; + $this->expectException(MalformedDiscriminatorException::class); + $this->expectExceptionMessage("'discriminator.propertyName'"); - $count = 0; - set_error_handler(static function (int $errno, string $errstr) use (&$count): bool { - if ($errno === E_USER_WARNING && str_contains($errstr, 'discriminator.mapping')) { - $count++; + OpenApiSchemaConverter::convert( + ['discriminator' => ['mapping' => ['a' => '#/components/schemas/A']]], + OpenApiVersion::V3_0, + SchemaContext::Response, + $this->enforcing(['components' => ['schemas' => ['A' => ['type' => 'object']]]]), + ); + } - return true; - } + #[Test] + public function discriminator_non_string_property_name_throws_when_enforced(): void + { + $this->expectException(MalformedDiscriminatorException::class); - return false; - }); + OpenApiSchemaConverter::convert( + ['discriminator' => ['propertyName' => 42, 'mapping' => ['a' => '#/components/schemas/A']]], + OpenApiVersion::V3_0, + SchemaContext::Response, + $this->enforcing(['components' => ['schemas' => ['A' => ['type' => 'object']]]]), + ); + } - try { - OpenApiSchemaConverter::convert($schema); - OpenApiSchemaConverter::convert($schema); - OpenApiSchemaConverter::convert($schema); - } finally { - restore_error_handler(); - } + #[Test] + public function discriminator_non_array_mapping_throws_when_enforced(): void + { + $this->expectException(MalformedDiscriminatorException::class); + $this->expectExceptionMessage("'discriminator.mapping'"); - $this->assertSame(1, $count); + OpenApiSchemaConverter::convert( + ['discriminator' => ['propertyName' => 'kind', 'mapping' => 'not-an-array']], + OpenApiVersion::V3_0, + SchemaContext::Response, + $this->enforcing(['components' => ['schemas' => []]]), + ); + } + + #[Test] + public function discriminator_non_string_mapping_value_throws_when_enforced(): void + { + $this->expectException(MalformedDiscriminatorException::class); + $this->expectExceptionMessage('discriminator.mapping[cat]'); + + OpenApiSchemaConverter::convert( + ['discriminator' => ['propertyName' => 'kind', 'mapping' => ['cat' => 123]]], + OpenApiVersion::V3_0, + SchemaContext::Response, + $this->enforcing(['components' => ['schemas' => []]]), + ); + } + + #[Test] + public function discriminator_unresolvable_pointer_throws_when_enforced(): void + { + $this->expectException(MalformedDiscriminatorException::class); + $this->expectExceptionMessage('does not resolve'); + + OpenApiSchemaConverter::convert( + ['discriminator' => ['propertyName' => 'kind', 'mapping' => ['cat' => '#/components/schemas/Missing']]], + OpenApiVersion::V3_0, + SchemaContext::Response, + $this->enforcing(['components' => ['schemas' => []]]), + ); + } + + #[Test] + public function discriminator_non_object_target_throws_when_enforced(): void + { + $this->expectException(MalformedDiscriminatorException::class); + $this->expectExceptionMessage('must reference a schema object'); + + OpenApiSchemaConverter::convert( + ['discriminator' => ['propertyName' => 'kind', 'mapping' => ['cat' => '#/components/schemas/Cat']]], + OpenApiVersion::V3_0, + SchemaContext::Response, + $this->enforcing(['components' => ['schemas' => ['Cat' => 'not-a-schema']]]), + ); } // ======================================== @@ -1651,13 +1960,12 @@ public function unknown_format_inside_additional_properties_warns(): void } #[Test] - public function discriminator_with_non_array_mapping_does_not_warn(): void + public function discriminator_with_non_array_mapping_is_stripped_when_not_enforced(): void { - // Negative case: `mapping: "not-an-array"` is a malformed spec, but - // the silent-pass concern (mapping wired but not enforced) does not - // apply — there's nothing to silently lose. Pin that the warning - // does NOT fire so the contract is explicit and a future - // tighten-the-check refactor surfaces here. + // Without enforcement the discriminator is stripped before its mapping + // is ever inspected, so even a malformed `mapping` is tolerated + // silently — the throw path is reserved for the enforced gate (see + // discriminator_non_array_mapping_throws_when_enforced). $schema = [ 'oneOf' => [['type' => 'object']], 'discriminator' => [ @@ -1667,19 +1975,19 @@ public function discriminator_with_non_array_mapping_does_not_warn(): void ]; $warnings = $this->captureWarnings(static fn() => OpenApiSchemaConverter::convert($schema)); + $result = OpenApiSchemaConverter::convert($schema); $this->assertSame([], $warnings); + $this->assertArrayNotHasKey('discriminator', $result); } #[Test] - public function unknown_keyword_and_discriminator_mapping_warn_independently(): void + public function unknown_keyword_warns_while_discriminator_is_stripped_when_not_enforced(): void { - // A schema declaring BOTH `unevaluatedProperties` (Draft 07 doesn't - // implement) AND `discriminator.mapping` (silently stripped) must - // fire BOTH warnings — they are independent silent-pass concerns - // with distinct dedup keys. Guards against an early-break refactor - // that consolidates the warning emission and accidentally drops - // one of them. + // `unevaluatedProperties` (Draft 07 doesn't implement) still warns, but + // `discriminator.mapping` no longer does — it is lowered when enforced + // and otherwise stripped silently (#262). So exactly one warning fires + // here, and it is the unevaluatedProperties one. $schema = [ 'type' => 'object', 'unevaluatedProperties' => false, @@ -1692,11 +2000,8 @@ public function unknown_keyword_and_discriminator_mapping_warn_independently(): $warnings = $this->captureWarnings(static fn() => OpenApiSchemaConverter::convert($schema, OpenApiVersion::V3_1)); - $this->assertCount(2, $warnings); - - $joined = implode(' | ', $warnings); - $this->assertStringContainsString('unevaluatedProperties', $joined); - $this->assertStringContainsString('discriminator.mapping', $joined); + $this->assertCount(1, $warnings); + $this->assertStringContainsString('unevaluatedProperties', $warnings[0]); } #[Test] @@ -2389,29 +2694,34 @@ public function unknown_format_inside_pattern_properties_warns(): void } #[Test] - public function discriminator_mapping_inside_then_warns(): void + public function discriminator_inside_then_is_lowered_when_enforced(): void { - $warnings = $this->captureWarnings(static function (): void { - OpenApiSchemaConverter::convert( - [ - 'if' => ['properties' => ['kind' => ['type' => 'string']]], - 'then' => [ - 'discriminator' => [ - 'propertyName' => 'kind', - 'mapping' => ['a' => '#/components/schemas/A'], - ], - 'oneOf' => [['type' => 'object']], + // The converter recurses into `then` (#214), so a discriminator nested + // there is lowered with the same enforce context — it does not survive + // into the validator unenforced. + $root = ['components' => ['schemas' => [ + 'A' => ['type' => 'object', 'required' => ['foo']], + ]]]; + $result = OpenApiSchemaConverter::convert( + [ + 'if' => ['properties' => ['kind' => ['type' => 'string']]], + 'then' => [ + 'discriminator' => [ + 'propertyName' => 'kind', + 'mapping' => ['a' => '#/components/schemas/A'], ], + 'oneOf' => [['type' => 'object']], ], - OpenApiVersion::V3_0, - ); - }); - - $this->assertNotSame([], $warnings, 'discriminator.mapping inside `then` must warn'); - $this->assertTrue( - str_contains($warnings[0], 'discriminator.mapping'), - 'warning must mention discriminator.mapping', + ], + OpenApiVersion::V3_0, + SchemaContext::Response, + $this->enforcing($root), ); + + $then = $result['then']; + $this->assertArrayNotHasKey('discriminator', $then); + $this->assertArrayHasKey('allOf', $then); + $this->assertSame(['foo'], $then['allOf'][1]['then']['required']); } // ======================================== @@ -2453,6 +2763,17 @@ public function v31_const_empty_array_lowered_to_enum(): void $this->assertSame([[]], $result['enum']); } + /** + * Build an enforcing DiscriminatorContext over a stub root spec for the + * `discriminator.mapping` lowering tests (#262). + * + * @param array $root + */ + private function enforcing(array $root): DiscriminatorContext + { + return new DiscriminatorContext($root, true); + } + /** * Run the conversion and collect every `E_USER_WARNING` it triggers, in * order. Returns the empty list when no warnings fire. Multi-warning diff --git a/tests/Unit/ValidatesOpenApiSchemaEnforceDiscriminatorTest.php b/tests/Unit/ValidatesOpenApiSchemaEnforceDiscriminatorTest.php new file mode 100644 index 0000000..a6d6097 --- /dev/null +++ b/tests/Unit/ValidatesOpenApiSchemaEnforceDiscriminatorTest.php @@ -0,0 +1,110 @@ + 'petstore-3.0', + ]; + } + + protected function tearDown(): void + { + self::resetValidatorCache(); + DiscriminatorEnforcement::reset(); + unset($GLOBALS['__openapi_testing_config']); + OpenApiSpecLoader::reset(); + parent::tearDown(); + } + + #[Test] + public function response_validator_build_keeps_enforcement_enabled_by_default(): void + { + // No config key set: building the response validator must (re-)enable + // the gate. Flip it off first so the assertion proves the build path + // set it, not that it merely inherited the static default. + DiscriminatorEnforcement::configure(false); + + $this->getOrCreateValidator(); + + $this->assertTrue(DiscriminatorEnforcement::isEnabled()); + } + + #[Test] + public function response_validator_build_disables_enforcement_when_config_false(): void + { + $GLOBALS['__openapi_testing_config']['openapi-contract-testing.enforce_discriminator'] = false; + + $this->getOrCreateValidator(); + + $this->assertFalse(DiscriminatorEnforcement::isEnabled()); + } + + #[Test] + public function request_validator_build_disables_enforcement_when_config_false(): void + { + // The request-side build path configures the gate independently of the + // response-side path. + $GLOBALS['__openapi_testing_config']['openapi-contract-testing.enforce_discriminator'] = false; + + $this->getOrCreateRequestValidator(); + + $this->assertFalse(DiscriminatorEnforcement::isEnabled()); + } + + #[Test] + public function string_boolean_config_value_is_coerced(): void + { + // `enforce_discriminator => env('...')` yields a string; the shared + // resolveBoolConfig() coercion must honour "false". + $GLOBALS['__openapi_testing_config']['openapi-contract-testing.enforce_discriminator'] = 'false'; + + $this->getOrCreateValidator(); + + $this->assertFalse(DiscriminatorEnforcement::isEnabled()); + } + + #[Test] + public function non_boolean_config_value_fails_loudly(): void + { + // A typo must not silently flip the gate — same three-way coercion the + // other flags use. + $GLOBALS['__openapi_testing_config']['openapi-contract-testing.enforce_discriminator'] = 'yolo'; + + $this->expectException(AssertionFailedError::class); + $this->expectExceptionMessage('enforce_discriminator must be a boolean'); + + $this->getOrCreateValidator(); + } +} diff --git a/tests/fixtures/specs/jwks.json b/tests/fixtures/specs/jwks.json new file mode 100644 index 0000000..53a47f3 --- /dev/null +++ b/tests/fixtures/specs/jwks.json @@ -0,0 +1,86 @@ +{ + "openapi": "3.0.3", + "info": { + "title": "JWKS discriminator fixture", + "version": "1.0.0", + "description": "Implicit-inheritance discriminator (#262): a base schema carries discriminator.mapping and subtypes reference the base via allOf, so the resolved subtype re-contains the base discriminator and exercises the recursion guard end-to-end." + }, + "paths": { + "/v1/jwks": { + "get": { + "summary": "Return a single JSON Web Key", + "operationId": "getJsonWebKey", + "responses": { + "200": { + "description": "OK", + "content": { + "application/json": { + "schema": {"$ref": "#/components/schemas/JsonWebKey"} + } + } + } + } + }, + "post": { + "summary": "Register a JSON Web Key", + "operationId": "createJsonWebKey", + "requestBody": { + "required": true, + "content": { + "application/json": { + "schema": {"$ref": "#/components/schemas/JsonWebKey"} + } + } + }, + "responses": { + "201": {"description": "Created"} + } + } + } + }, + "components": { + "schemas": { + "JsonWebKey": { + "type": "object", + "required": ["kty"], + "properties": { + "kty": {"type": "string", "enum": ["RSA", "EC"]} + }, + "discriminator": { + "propertyName": "kty", + "mapping": { + "RSA": "#/components/schemas/RsaJsonWebKey", + "EC": "#/components/schemas/EcJsonWebKey" + } + } + }, + "RsaJsonWebKey": { + "allOf": [ + {"$ref": "#/components/schemas/JsonWebKey"}, + { + "type": "object", + "required": ["n", "e"], + "properties": { + "n": {"type": "string"}, + "e": {"type": "string"} + } + } + ] + }, + "EcJsonWebKey": { + "allOf": [ + {"$ref": "#/components/schemas/JsonWebKey"}, + { + "type": "object", + "required": ["crv", "x", "y"], + "properties": { + "crv": {"type": "string"}, + "x": {"type": "string"}, + "y": {"type": "string"} + } + } + ] + } + } + } +}