Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added new header to capture customattributes #402

Merged
merged 5 commits into from
Mar 9, 2023
Merged

Conversation

mtalreja16
Copy link
Contributor

No description provided.

server/authorization_test.go Outdated Show resolved Hide resolved
server/header_context_test.go Outdated Show resolved Hide resolved
server/metadata_context.go Outdated Show resolved Hide resolved
@mtalreja16 mtalreja16 requested a review from theganyo March 9, 2023 01:38
@codecov
Copy link

codecov bot commented Mar 9, 2023

Codecov Report

Merging #402 (da202cf) into main (c70fcad) will increase coverage by 1%.
The diff coverage is 100%.

@@         Coverage Diff         @@
##           main   #402   +/-   ##
===================================
+ Coverage    89%    89%   +1%     
===================================
  Files         9      9           
  Lines       762    768    +6     
===================================
+ Hits        674    680    +6     
  Misses       46     46           
  Partials     42     42           
Impacted Files Coverage Δ
server/header_context.go 96% <100%> (+1%) ⬆️
server/metadata_context.go 100% <100%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@@ -56,6 +57,11 @@ func encodeExtAuthzMetadata(api string, ac *auth.Context, authorized bool) *stru
headerOrganization: stringValueFrom(ac.Organization()),
headerScope: stringValueFrom(strings.Join(ac.Scopes, " ")),
}

if ac.CustomAttributes != "" {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one more quick thing: Could you please add a test to verify that this header isn't added when there is no value?

…butes if it is not set in the claims in remoteservice proxy
@theganyo theganyo merged commit 4604974 into apigee:main Mar 9, 2023
@theganyo
Copy link
Member

theganyo commented Mar 9, 2023

Thank you!

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.

2 participants