fix(authorization): restore legacy UserState-via-ContextData fallback#22
Open
cliedeman wants to merge 1 commit into
Open
fix(authorization): restore legacy UserState-via-ContextData fallback#22cliedeman wants to merge 1 commit into
cliedeman wants to merge 1 commit into
Conversation
After ChilliCream#8267/ChilliCream#8329 moved UserState out of ContextData (well-known key "HotChocolate.Authorization.UserState") into the typed Features collection, DefaultAuthorizationHandler.GetUserState reads only Features and the rewritten TryCreateUserStateIfNotExists only handled a bare ClaimsPrincipal. Consumers that inject a ready-made UserState under the old key (notably the Strawberry Shake in-memory client, which bypasses the request/socket interceptors and calls SetGlobalState("HotChocolate.Authorization.UserState", new UserState(principal))) silently stopped authenticating. Re-add a legacy-key fallback that promotes a UserState found under the old key into Features, taking precedence over reconstructing one from a ClaimsPrincipal. Adds Authorize_Should_Use_Explicit_UserState_When_Passed_Through_ContextData; all 34 Authorization.Tests pass.
Patch coverage100.0% of changed lines covered (6/6)
Project coverage: 52.4% (215296/411035 lines) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
PRs ChilliCream#8267 / ChilliCream#8329 migrated
UserStateout ofContextData(the well-known key"HotChocolate.Authorization.UserState", whoseWellKnownContextData.UserStateconstant was deleted) into the typedFeaturescollection.DefaultAuthorizationHandler.GetUserStatenow reads onlyFeatures.TryGet<UserState>()and throwsMissingStateExceptionotherwise.The rewritten
TryCreateUserStateIfNotExistsonly handled a bareClaimsPrincipalplaced inContextDataundernameof(ClaimsPrincipal). Consumers that supplied a ready-madeUserStateobject under the old well-known key silently stopped authenticating.This bites the Strawberry Shake in-memory client: it runs in-process and bypasses
DefaultHttpRequestInterceptor/DefaultSocketSessionInterceptor(which normally buildUserStatefromcontext.User), so the user is injected manually viaSetGlobalState("HotChocolate.Authorization.UserState", new UserState(principal)). Post-migration that value never reached the Features-based handler, so authorization saw an unauthenticated user.Fix
AuthorizationFeatureExtensions.TryCreateUserStateIfNotExistsnow promotes aUserStatefound under the legacy key intoFeatures, taking precedence over reconstructing one from aClaimsPrincipal. The legacy key is a localprivate const(no shared constant survives the migration;DefaultAuthorizationHandlerlikewise hardcodes the same string in its error message).Test
Authorize_Should_Use_Explicit_UserState_When_Passed_Through_ContextDatainjects aUserStateviaSetGlobalStateunder the legacy key and asserts the request gets past authentication (it fails later with "The node ID string has an invalid format." rather than a NotAuthenticated error). All 34Authorization.Testspass locally.Targets
mainso the full CI runs the realAuthorization.Tests.— Claude
🤖 Generated with Claude Code