fix(ers): preserve resolved token-chain context - #3808
Open
jrschumacher wants to merge 4 commits into
Open
Conversation
Contributor
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
jrschumacher
force-pushed
the
fix/3801-chain-no-rehydrate
branch
from
July 31, 2026 08:02
77907ff to
9febb1d
Compare
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe change preserves resolved claims in multi-strategy token entity chains. It adds explicit strategy execution, claims-based entity serialization, direct PDP conversion with hydration fallback, expanded contract validation, and token-based BDD authorization coverage. ChangesEntity resolution and chain preservation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant ERS
participant EntityChain
participant PDP
participant PolicyDecision
Client->>ERS: CreateEntityChainsFromTokens
ERS->>EntityChain: store selected-strategy claims
Client->>PDP: submit token decision
PDP->>EntityChain: inspect claims
EntityChain-->>PDP: entity representations
PDP->>PolicyDecision: evaluate authorization
PolicyDecision-->>Client: PERMIT or DENY
Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
Error Summary
TDF3 Benchmark Results:
Error Summary:
|
jrschumacher
changed the title
jrschumacher
force-pushed
the
fix/3801-chain-no-rehydrate
branch
from
July 31, 2026 08:16
9febb1d to
afd0ac4
Compare
Contributor
Contributor
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
Contributor
strantalis
reviewed
Aug 3, 2026
Contributor
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
Contributor
strantalis
previously approved these changes
Aug 4, 2026
jrschumacher
dismissed
strantalis’s stale review
August 4, 2026 18:57
The merge-base changed after approval.
jrschumacher
force-pushed
the
fix/3801-chain-no-rehydrate
branch
from
August 4, 2026 18:58
cd00177 to
d3a4357
Compare
Contributor
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
Contributor
strantalis
previously approved these changes
Aug 4, 2026
elizabethhealy
previously approved these changes
Aug 4, 2026
Base automatically changed from
fix/3790-resolve-entities-jwt-claims-context
to
main
August 4, 2026 22:19
jrschumacher
dismissed stale reviews from elizabethhealy and strantalis
August 4, 2026 23:23
The merge-base changed after approval.
elizabethhealy
previously approved these changes
Aug 5, 2026
Signed-off-by: Ryan Schumacher <jschumacher@virtru.com>
Signed-off-by: Ryan Schumacher <j.r.schumacher@gmail.com>
jrschumacher
force-pushed
the
fix/3801-chain-no-rehydrate
branch
from
August 5, 2026 14:42
d3a4357 to
345a5aa
Compare
policy-bot-opentdf
Bot
dismissed stale reviews from khvirtru
August 5, 2026 14:42
Invalidated by push of 345a5aa
Contributor
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@service/entityresolution/integration/multistrategy_comprehensive_test.go`:
- Around line 922-934: Update chainEntityClaimsMap to fail the test immediately
when ent.GetClaims() returns nil, using t.Fatalf with a clear message that the
entity has no claims; remove the empty-map return while preserving the existing
claims unmarshalling and map conversion behavior.
- Around line 1049-1051: Replace the fixed time.Sleep after container startup
with a readiness wait using wait.ForExec that runs an ldapsearch against
ou=users,dc=opentdf,dc=test and retries until the LDAP bootstrap is complete.
Apply the wait before tests search the directory, preserving the existing
container-start flow while removing the unconditional five-second delay.
In `@service/entityresolution/multi-strategy/service_test.go`:
- Around line 58-60: Extend the assertions in the ResolveEntityWithStrategy test
after the strategy_name check to validate
result.Metadata["attempted_strategies"] is exactly a []interface{} containing
only "client_strategy". This must verify both the collection type and its single
selected-strategy value.
In `@service/entityresolution/multi-strategy/service.go`:
- Around line 157-165: Extract the duplicated strategy metadata population into
a shared helper, preserving the default failure strategy and the
structpb-compatible []interface{} value for attempted_strategies. Update both
ResolveEntity and ResolveEntityWithStrategy to call this helper instead of
maintaining separate metadata blocks, while retaining each method’s existing
strategy inputs.
In `@service/entityresolution/multi-strategy/v2/registration.go`:
- Around line 332-361: Update createEntityFromResultV2 to return
(*entity.Entity, error) and propagate failures from structpb.NewStruct and
anypb.New instead of substituting fallback claims or discarding errors. Update
its caller in createEntityChainFromSingleTokenV2 to handle and return the error
so CreateEntityChainsFromTokens uses its existing fail-safe path and does not
append a partial entity.
In `@service/internal/access/v2/just_in_time_pdp_test.go`:
- Around line 58-124: Extend the tests around resolveEntitiesFromToken to cover
a claims-based ERS response returned directly from the token-chain path. Assert
that the result contains the expected AdditionalProps,
CreateEntityChainsFromTokens is called exactly once, and ResolveEntities is
never called, preserving the no-rehydrate behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: e46dbdde-ae94-44fb-8386-be621bd1a53a
📒 Files selected for processing (12)
service/entityresolution/integration/internal/contract_tests.goservice/entityresolution/integration/multistrategy_comprehensive_test.goservice/entityresolution/integration/multistrategy_test.goservice/entityresolution/integration/multistrategy_v2_serialization_test.goservice/entityresolution/multi-strategy/service.goservice/entityresolution/multi-strategy/service_test.goservice/entityresolution/multi-strategy/v2/registration.goservice/entityresolution/multi-strategy/v2/registration_test.goservice/internal/access/v2/just_in_time_pdp.goservice/internal/access/v2/just_in_time_pdp_test.gotests-bdd/cukes/steps_authorization.gotests-bdd/features/multi-strategy-ers-token.feature
Contributor
strantalis
previously approved these changes
Aug 5, 2026
Signed-off-by: Ryan Schumacher <jschumacher@virtru.com>
Contributor
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
Contributor
Signed-off-by: Ryan Schumacher <jschumacher@virtru.com>
Contributor
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
Contributor
|
Contributor
3 tasks
strantalis
approved these changes
Aug 6, 2026
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Aug 6, 2026
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
4 participants
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.
Summary
Testing
Notes
Fixes #3801
Summary by CodeRabbit
New Features
Bug Fixes
Tests