Summary
CacheTagKey derives the vary-by-user component of the cache key from HttpContext.User?.Identity?.Name, a value that is nullable and carries no framework-level uniqueness guarantee. As a result, two distinct authenticated principals can produce an equal CacheTagKey, which breaks the per-user isolation property that vary-by-user exists to provide.
What is wrong
- Broken invariant:
vary-by-user="true" is intended to guarantee that a cached fragment produced while one principal is signed in is never returned to a different principal. The current key derivation does not establish that invariant, because the only per-user input is Identity.Name, which may be null or shared across distinct accounts depending on how the application configures authentication.
- Where it originates:
CacheTagKey captures Identity.Name verbatim, then uses it in structured equality, in GetHashCode, and in the generated key string. When that value is null/empty for an authenticated principal, the user component of the key degrades to an empty token that is indistinguishable from the anonymous case and identical for every such principal.
- Both consumers are affected:
CacheTagHelper uses CacheTagKey as an IMemoryCache key, and DistributedCacheTagHelper uses the generated string as the distributed cache key.
- The current behavior is intentional and documented (
vary-by-user is documented as keying on @User.Identity.Name), and the empty-suffix case is locked by existing tests. This is therefore a hardening request rather than a defect report: the framework should not silently collapse distinct authenticated principals into a shared cache bucket when it cannot obtain a stable user key.
Why it matters (defense in depth)
- Correctness:
Equals/GetHashCode on CacheTagKey claim that two keys represent the same cache state. For an authenticated-but-nameless principal that claim is not true, so a cache entry is shared between states that are not equivalent.
- Hardening:
vary-by-user is an isolation primitive. Applications reasonably read it as "this fragment is scoped to the signed-in user." An isolation primitive that silently degrades to no isolation when its single input is absent is a weak default. Failing closed (or falling back to another stable identifier) removes an entire class of misconfiguration from the application's responsibility.
- The documented, standard configuration (ASP.NET Core Identity) sets the name claim to the unique
UserName, so this does not affect default Identity-based applications. The gap is limited to applications whose authentication does not populate a unique ClaimTypes.Name - for example custom cookie or JWT setups that identify users by sub/NameIdentifier only.
Affected code
- src/Mvc/Mvc.TagHelpers/src/Cache/CacheTagKey.cs:98-101 -
_username is assigned directly from httpContext.User?.Identity?.Name with no authenticated/nameless distinction
- src/Mvc/Mvc.TagHelpers/src/Cache/CacheTagKey.cs:144-151 -
GenerateKey() appends the VaryByUser token followed by the possibly empty _username
- src/Mvc/Mvc.TagHelpers/src/Cache/CacheTagKey.cs:208-209 -
Equals compares _username ordinally; equal-or-both-null names compare as equal
- src/Mvc/Mvc.TagHelpers/src/Cache/CacheTagKey.cs:251 -
GetHashCode includes _username, so colliding keys also hash equally
- src/Mvc/Mvc.TagHelpers/src/CacheTagHelper.cs:69-77 - uses
CacheTagKey as the IMemoryCache lookup key
- src/Mvc/Mvc.TagHelpers/src/DistributedCacheTagHelper.cs:61 - constructs
CacheTagKey for the distributed cache path
- src/Mvc/Mvc.TagHelpers/test/CacheTagKeyTest.cs:328-360 - existing tests that pin the current empty-suffix behavior and will need to be revisited
- src/Identity/Extensions.Core/src/UserClaimsPrincipalFactory.cs:79 - reference point showing the standard Identity path sets the name claim to the unique
UserName
Recommended fix
Selected approach: keep Identity.Name as the primary user key so existing keys are byte-for-byte unchanged for principals that have a name, and only change behavior in the case that is currently unsafe. When VaryByUser is true and User.Identity.IsAuthenticated is true but Identity.Name is null or empty, fall back to a stable claim (ClaimTypes.NameIdentifier, then sub) for the user component. If no stable identifier is available either, treat the entry as non-shareable: bypass the cache for that request (render the child content and do not store or serve a shared entry) rather than joining the empty bucket. The unauthenticated case keeps its current empty user component, so anonymous fragment caching is unaffected.
Alternatives considered:
- Always key on
NameIdentifier/sub instead of Identity.Name. Rejected: it contradicts the documented contract, changes the generated key for every application, and invalidates every existing distributed cache entry on upgrade.
- Throw when
VaryByUser is set and no stable user key is available. Rejected: this turns a working (if under-isolated) page into a runtime failure during a patch or minor upgrade, which is too aggressive for a hardening change.
- Documentation-only change noting that
vary-by-user requires a unique, non-null Identity.Name. Worth doing regardless, but it leaves the weak default in place.
- Introduce an opt-in switch (
AppContext flag or option) for the stricter behavior. Reasonable fallback if the behavior change is judged too risky for a servicing branch; the stricter behavior should still become the default in the next major.
Compatibility, migration, and versioning:
CacheTagKey.GenerateKey() and GenerateHashedKey() are public API; their output for named principals must not change. Confining the change to the authenticated-and-nameless branch preserves that.- For
DistributedCacheTagHelper, any key-shape change invalidates existing entries in shared stores. The proposed scoping avoids invalidating entries for named principals; entries previously stored under the empty bucket by nameless authenticated principals simply stop being served, which is the desired outcome.
- Target
main only. Because the current behavior is documented and unit-tested, this is not a servicing candidate; do not backport unless the area owners decide otherwise.
- Behavior change is observable (a fragment that was previously cached for nameless authenticated principals is no longer shared), so it needs a breaking-change note in the release notes.
Acceptance criteria
Summary
CacheTagKeyderives thevary-by-usercomponent of the cache key fromHttpContext.User?.Identity?.Name, a value that is nullable and carries no framework-level uniqueness guarantee. As a result, two distinct authenticated principals can produce an equalCacheTagKey, which breaks the per-user isolation property thatvary-by-userexists to provide.What is wrong
vary-by-user="true"is intended to guarantee that a cached fragment produced while one principal is signed in is never returned to a different principal. The current key derivation does not establish that invariant, because the only per-user input isIdentity.Name, which may benullor shared across distinct accounts depending on how the application configures authentication.CacheTagKeycapturesIdentity.Nameverbatim, then uses it in structured equality, inGetHashCode, and in the generated key string. When that value isnull/empty for an authenticated principal, the user component of the key degrades to an empty token that is indistinguishable from the anonymous case and identical for every such principal.CacheTagHelperusesCacheTagKeyas anIMemoryCachekey, andDistributedCacheTagHelperuses the generated string as the distributed cache key.vary-by-useris documented as keying on@User.Identity.Name), and the empty-suffix case is locked by existing tests. This is therefore a hardening request rather than a defect report: the framework should not silently collapse distinct authenticated principals into a shared cache bucket when it cannot obtain a stable user key.Why it matters (defense in depth)
Equals/GetHashCodeonCacheTagKeyclaim that two keys represent the same cache state. For an authenticated-but-nameless principal that claim is not true, so a cache entry is shared between states that are not equivalent.vary-by-useris an isolation primitive. Applications reasonably read it as "this fragment is scoped to the signed-in user." An isolation primitive that silently degrades to no isolation when its single input is absent is a weak default. Failing closed (or falling back to another stable identifier) removes an entire class of misconfiguration from the application's responsibility.UserName, so this does not affect default Identity-based applications. The gap is limited to applications whose authentication does not populate a uniqueClaimTypes.Name- for example custom cookie or JWT setups that identify users bysub/NameIdentifieronly.Affected code
_usernameis assigned directly fromhttpContext.User?.Identity?.Namewith no authenticated/nameless distinctionGenerateKey()appends theVaryByUsertoken followed by the possibly empty_usernameEqualscompares_usernameordinally; equal-or-both-null names compare as equalGetHashCodeincludes_username, so colliding keys also hash equallyCacheTagKeyas theIMemoryCachelookup keyCacheTagKeyfor the distributed cache pathUserNameRecommended fix
Selected approach: keep
Identity.Nameas the primary user key so existing keys are byte-for-byte unchanged for principals that have a name, and only change behavior in the case that is currently unsafe. WhenVaryByUseristrueandUser.Identity.IsAuthenticatedistruebutIdentity.Nameis null or empty, fall back to a stable claim (ClaimTypes.NameIdentifier, thensub) for the user component. If no stable identifier is available either, treat the entry as non-shareable: bypass the cache for that request (render the child content and do not store or serve a shared entry) rather than joining the empty bucket. The unauthenticated case keeps its current empty user component, so anonymous fragment caching is unaffected.Alternatives considered:
NameIdentifier/subinstead ofIdentity.Name. Rejected: it contradicts the documented contract, changes the generated key for every application, and invalidates every existing distributed cache entry on upgrade.VaryByUseris set and no stable user key is available. Rejected: this turns a working (if under-isolated) page into a runtime failure during a patch or minor upgrade, which is too aggressive for a hardening change.vary-by-userrequires a unique, non-nullIdentity.Name. Worth doing regardless, but it leaves the weak default in place.AppContextflag or option) for the stricter behavior. Reasonable fallback if the behavior change is judged too risky for a servicing branch; the stricter behavior should still become the default in the next major.Compatibility, migration, and versioning:
CacheTagKey.GenerateKey()andGenerateHashedKey()are public API; their output for named principals must not change. Confining the change to the authenticated-and-nameless branch preserves that.DistributedCacheTagHelper, any key-shape change invalidates existing entries in shared stores. The proposed scoping avoids invalidating entries for named principals; entries previously stored under the empty bucket by nameless authenticated principals simply stop being served, which is the desired outcome.mainonly. Because the current behavior is documented and unit-tested, this is not a servicing candidate; do not backport unless the area owners decide otherwise.Acceptance criteria
ClaimTypes.Namevalue never resolve to the sameCacheTagKeyfor the same cache element, and never receive each other's cached fragment.Identity.Name.EqualsandGetHashCodeagree with the new key derivation in all four cases: anonymous, authenticated with name, authenticated without name but with a stable claim, authenticated with no usable identifier.CacheTagHelper(in-memory) andDistributedCacheTagHelper(generated string key) paths.src/Mvc/Mvc.TagHelpers/test/CacheTagKeyTest.csis updated so that the empty user component is asserted only for genuinely unauthenticated principals.vary-by-userisolates byIdentity.Nameand that applications should ensure it is a stable, unique, non-null per-account value.