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

feat(jwt) : add feature to jwt plugin:allow set headers from claim #13368

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mtparet
Copy link

@mtparet mtparet commented Jul 12, 2024

Summary
Add feature to jwt plugin: allow set header from claims.

Reopenning of #4761 #5713

Full changelog

[Implement] jwt plugin: allow set header from claims
if you configure headers_to_set(set) , headers with prefix "X-Jwt-Claim-" will be set to the response. For example, if 'sub' and 'app' are configured, and both of them are found in jwt's claims, two headers will be set to the response: X-Jwt-Claim-Sub and X-Jwt-Claim-App.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@github-actions github-actions bot added plugins/jwt schema-change-noteworthy cherry-pick kong-ee schedule this PR for cherry-picking to kong/kong-ee labels Jul 12, 2024
@team-eng-enablement team-eng-enablement added the author/community PRs from the open-source community (not Kong Inc) label Jul 12, 2024
@hanshuebner hanshuebner requested a review from jschmid1 July 15, 2024 12:31
Copy link
Contributor

@hanshuebner hanshuebner left a comment

Choose a reason for hiding this comment

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

Looks good, but a test, changelog file and an entry to the removed_fields list is required to accommodate for the schema change. The test should cover that the headers_to_set configuration is handled properly when not specified as well as a variety of sensible configuration values.

Also, you need to sign our CLA.

@jschmid1 any thoughts from you as the subject matter expert?

Comment on lines +15 to +17
local consts = {
JWT_CLAIM_HEADER_PREFIX = "X-Jwt-Claim"
}
Copy link
Contributor

Choose a reason for hiding this comment

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

We don't generally create extra tables for local constants. Please change this to just

Suggested change
local consts = {
JWT_CLAIM_HEADER_PREFIX = "X-Jwt-Claim"
}
JWT_CLAIM_HEADER_PREFIX = "X-Jwt-Claim"

@@ -150,6 +153,15 @@ local function unauthorized(message, www_auth_content, errors)
return { status = 401, message = message, headers = { ["WWW-Authenticate"] = www_auth_content }, errors = errors }
end

-- set header keys from claims
local function set_headers(conf, claims)
Copy link
Contributor

Choose a reason for hiding this comment

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

If you rename the function to set_headers_from_claims, the comment is no longer needed.

@pylaligand pylaligand force-pushed the feature-jwt-add-header branch from b3c5edc to 9cb6550 Compare December 10, 2024 12:46
@pull-request-size pull-request-size bot added size/M and removed size/S labels Dec 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author/community PRs from the open-source community (not Kong Inc) cherry-pick kong-ee schedule this PR for cherry-picking to kong/kong-ee plugins/jwt schema-change-noteworthy size/M
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants