Skip to content

fix(serializer): gate cache_key in JsonApi and Hal with isCacheKeySafe#8200

Open
soyuka wants to merge 1 commit into
api-platform:4.3from
soyuka:fix/cache-key-security-jsonapi-hal
Open

fix(serializer): gate cache_key in JsonApi and Hal with isCacheKeySafe#8200
soyuka wants to merge 1 commit into
api-platform:4.3from
soyuka:fix/cache-key-security-jsonapi-hal

Conversation

@soyuka
Copy link
Copy Markdown
Member

@soyuka soyuka commented May 25, 2026

Q A
Branch? 4.3
Tickets -
License MIT
Doc PR -

Summary

#[ApiProperty(security: ...)] makes the set of allowed attributes per-user, but the local componentsCache arrays in JsonApi and Hal item normalizers are not user-aware. Without gating, two users hitting the same resource can reuse a cached component map computed for the other user, leaking attributes across users.

PR #7854 fixes the related Symfony AbstractObjectNormalizer::$attributesCache leak by moving isCacheKeySafe() to AbstractItemNormalizer. JsonApi and Hal, however, set $context['cache_key'] themselves before delegating to parent::normalize() and use it for their own componentsCache. Since $context is passed by value, the gating done inside AbstractItemNormalizer::normalize() does not propagate back to those subclasses, so the local cache is still written unconditionally.

This PR:

  • Moves isCacheKeySafe() from GraphQl/Serializer/ItemNormalizer into Serializer/AbstractItemNormalizer (supersedes fix: move isCacheKeySafe from GraphQL to AbstractItemNormalizer #7854) and makes it protected so subclasses can call it.
  • Gates $context['cache_key'] in JsonApi/Serializer/ItemNormalizer::normalize() and Hal/Serializer/ItemNormalizer::normalize() with isCacheKeySafe().
  • Promotes CacheKeyTrait::getCacheKey() from private to protected so subclasses that inherit the trait via AbstractItemNormalizer can call the trait method.

Test plan

  • vendor/bin/phpunit src/Serializer/Tests/AbstractItemNormalizerTest.php — passes (4 new tests for isCacheKeySafe()).
  • vendor/bin/phpunit src/JsonApi/Tests/Serializer/ItemNormalizerTest.php — passes (new testCacheKeyIsFalseWhenAPropertyHasSecurity).
  • vendor/bin/phpunit src/Hal/Tests/Serializer/ItemNormalizerTest.php — passes (new testCacheKeyIsFalseWhenAPropertyHasSecurity).
  • vendor/bin/phpunit src/GraphQl/Tests/Serializer/ItemNormalizerTest.php — passes (logic moved to parent, GraphQL still benefits via inheritance).
  • vendor/bin/phpunit tests/Functional/JsonApiTest.php tests/Functional/JsonLdTest.php — passes.
  • vendor/bin/phpunit tests/Functional/SecurityPropertyInputDtoTest.php tests/Functional/GraphQl tests/Functional/Uuid — passes.

Relates to

`#[ApiProperty(security: ...)]` makes allowed attributes per-user, but the
local `componentsCache` in JsonApi and Hal item normalizers is not
user-aware: a cached component map can leak attributes across users.

Move `isCacheKeySafe()` from `GraphQl/Serializer/ItemNormalizer` into
`Serializer/AbstractItemNormalizer` so it is shared across all formats,
and gate `$context['cache_key']` with it in JsonApi and Hal. Promote
`CacheKeyTrait::getCacheKey()` from `private` to `protected` so
subclasses can reuse the inherited trait method.

Supersedes api-platform#7854.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant