-
Notifications
You must be signed in to change notification settings - Fork 2.8k
feat(jwt-auth): support configuring key_claim_name
#11282
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
Closed
Closed
Changes from 5 commits
Commits
Show all changes
29 commits
Select commit
Hold shift + click to select a range
afa520d
Fix #11276 + little style refactor
mikyll afb3d6b
Update jwt-auth.md
mikyll b88cf9d
infra: Increase PR reviewers to 3 when merge to master. (#11280)
moonming 55eeb6b
docs: correct the default collector config apisix actually used for o…
flearc c2ba478
docs: added Write a Review link (#11313)
juzhiyuan 08cb3ad
docs: add http3 docs (#11302)
kayx23 cf84292
feat: move tinyyaml to lyaml (#11312)
bzp2010 d106de5
fix(ssl): ssl key rotation caused request failure (#11305)
AlinsRan bd76c43
Revert style changes to jwt-auth plugin
mikyll 2af7e15
Added test case for new jwt-auth feature
mikyll 92bd025
Update jwt-auth4.t
mikyll d77d672
fix: add libyaml-dev dependency for apt. (#11291)
adam-huganir 953be46
fix: after updating the header, get the old value from the ctx.var (#…
AlinsRan 0cacb90
docs: add plugin config to standalone deployment doc (#11332)
kayx23 fec3137
docs: add http/3 in README.md. (#11318)
moonming 3ad9c28
build(undeps): remove all rocks before remove openresty (#11333)
flearc 5e383e0
feat(secret): support store ssl.keys ssl.certs in secrets mamager (#1…
AlinsRan 1a45d1d
fix(datadog): report consumer username tag (#11354)
bzp2010 de1669d
docs: improve debug mode yaml comments (#11373)
kayx23 6d2de7e
fix: make the message clearer when API key is missing (#11370)
pottekkat 4dbecfd
docs: add http-dubbo docs (#11322)
ShenFeng312 7f649bc
ci: removed centos, chaos, fuzzing and fips CIs. (#11394)
moonming 1164374
fix(grpc-transcode): filter out illegal INT(string) formats (#11367)
zhoujiexiong 667ac7f
Fix #11276 + little style refactor
mikyll b2ed6b8
Update jwt-auth.md
mikyll c944947
Revert style changes to jwt-auth plugin
mikyll 1ddbc8f
Added test case for new jwt-auth feature
mikyll 76816f1
Update jwt-auth4.t
mikyll 1203a5a
Merge branch 'jwt_key_claim_name' of https://github.com/mikyll/apisix…
mikyll File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
although the key_claim_name has changed, the value of
key_claim_namewill always bekey. That doesn't fit into the scene. For example, if thekey_claim_nameisexpthen the value should be the expiry time, not the key. Right?Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@shreemaan-abhishek I'm not sure if I understood your comment correctly, in case I didn't please correct me :)
The purpose of
key_claim_nameparameter is just to tell in which claim the key is stored, so that the plugin can use this for the validation. For example, the company where I work uses JWTs which do not include thekeyclaim, but do includeissinstead.The reason why I also made changes to
get_real_payload()and the like, was to provide a way to generate a signed token with a custom key claim name, through the public API (otherwise I would have needed to make a test case with a hardcoded token, see line 182 of commit 2af7e156aa07ca29cf19f4934daa605eb0571902 for reference).Addressing your comment, if
key_claim_namewas "exp", then no, the value shouldn't be the expiry time, but still the key. However, that would cause some problem:expclaim is optional, the jwt-auth plugin requires it to validate the token, and returns401if it's not present (at this line):key_claim_nameto "exp" is not feasible, since that would generate a token with duplicatedexpclaim.At most, we could handle this case by preventing the usage of "exp" and "nbf" as
key_claim_name, what do you think?