fix(ers): Do not use auth header jwt in MultiStrategy ERS #2862
Conversation
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
elizabethhealy
changed the title
Contributor
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Standard Benchmark Metrics Skipped or FailedBulk Benchmark Results
TDF3 Benchmark Results:
NANOTDF Benchmark Results:
|
elizabethhealy
changed the title
Contributor
Contributor
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Standard Benchmark Metrics Skipped or FailedBulk Benchmark Results
TDF3 Benchmark Results:
NANOTDF Benchmark Results:
|
Contributor
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Standard Benchmark Metrics Skipped or FailedBulk Benchmark Results
TDF3 Benchmark Results:
NANOTDF Benchmark Results:
|
Contributor
Contributor
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Standard Benchmark Metrics Skipped or FailedBulk Benchmark Results
TDF3 Benchmark Results:
NANOTDF Benchmark Results:
|
Contributor
Contributor
alkalescent
reviewed
Nov 3, 2025
Comment on lines
+66
to
89
| var claimsMap types.JWTClaims | ||
| switch entityV2.GetEntityType().(type) { | ||
| case *entity.Entity_Claims: | ||
| claims := entityV2.GetClaims() | ||
| if claims != nil { | ||
| // First unmarshal to structpb.Struct | ||
| var claimsStruct structpb.Struct | ||
| err := claims.UnmarshalTo(&claimsStruct) | ||
| if err != nil { | ||
| return nil, connect.NewError(connect.CodeInvalidArgument, fmt.Errorf("error unpacking anypb.Any to structpb.Struct: %w", err)) | ||
| } | ||
| // Convert to map[string]interface{} | ||
| claimsMap = claimsStruct.AsMap() | ||
| } | ||
| default: | ||
| entityBytes, err := protojson.Marshal(entityV2) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| err = json.Unmarshal(entityBytes, &claimsMap) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Given this logic block is almost identical to the one in the v2 version (besides getting the claims). Is there a way to abstract this logic to a function?
Member
Author
There was a problem hiding this comment.
good call out, do you know if we have a recommended strategy around shared code for v1/v2 versions? looking at the rest of the code at least within the ERS implementations it seems like its all duplication instead of shared functions, so i kind of just followed that strategy, but if thats not the case we should refactor across ERS
Contributor
There was a problem hiding this comment.
We copied over the entirety of the v2 ERS initially when it was scaffolded for Auth Service v2. As the intention is to deprecate the entirety of ERS v1 when Auth Service v1 is deprecated (within the next few releases), I would not recommend expending too much effort on DRYness unless the DRY changes also benefit logic within the single v2 service version alone.
Member
Author
There was a problem hiding this comment.
ya the logic doesnt really repeat within v2 so not sure a separate func would be needed, also not sure where i would put that if it were to be shared between the two, we dont really have a place for that right now. so i may just opt to go with the duplication for now if thats ok
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.
Proposed Changes
Checklist
Testing Instructions