feat(jwt-auth): support configuring `key_claim_name` #11282

mikyll · 2024-05-23T14:45:18Z

Description

New feature:

Added a new configuration parameter to the schema: key_claim_name (default = "key"), so for example one could use "iss" to check the validity of the JWT;

(similar behaviour to Kong JWT plugin)

Checklist

I have explained the need for this PR and the problem it solves
I have explained the changes or the new features added to this PR
I have added tests corresponding to this change
I have updated the documentation to reflect this change
I have verified that this change is backward compatible (If not, please discuss on the APISIX mailing list first)

Features: - config param "key_claim_name" (default = "key"), so for example one could use "iss" to check the validity of the JWT; Style: - 2 blank lines between functions; - 1 blank like before "else" and "elseif"; - jwt -> JWT; - Capitalized logs and response messages; - Added description for each schema configuration parameter;

We need more eyes to improve code quality

shreemaan-abhishek

Please remove the style changes and add test cases to support your fix. Thanks.

mikyll · 2024-05-29T06:28:34Z

@shreemaan-abhishek thank you for the feedback. I understand the importance of keeping PRs focused, but I made the style changes to align with the code_style guide. Could you please clarify if:

the style changes should be reverted entirely, despite the guidelines?
generally speaking, you prefer these style changes to be submitted as a separate PR, or not be carried out at all?

Anyways, I'll be happy to add the necessary test cases, as soon as possible :-)

…pentelemetry plugin (#11247)

Co-authored-by: Navendu Pottekkat <[email protected]> Co-authored-by: Abhishek Choudhary <[email protected]>

I added a new test case for feature #11276 Since the default value of the new config parameter "key_claim_name" is "key", "default behaviour" is already validated by other tests.

+ Removed external httpbin upstream in favor of local endpoint (/hello); + Cleaned code;

mikyll · 2024-06-04T15:03:38Z

@shreemaan-abhishek
I did the following:

pushed commit bd76c435a33c7dac91a938654c334eb3d10bd2ae to remove the style changes and add a part that was missing for the token generation (I actually forgot to mention this part in the commit description);
pushed commit 2af7e156aa07ca29cf19f4934daa605eb0571902 which includes a test case for the new feature;
pushed commit 92bd0257ace006260366d96dd2171199ab8ef260 to clean the test code and use /hello local endpoint instead of external httpbin upstream;
updated the PR title and details to remove anything style-related;

…11329)

…1339)

Co-authored-by: Traky Deng <[email protected]>

github-actions · 2024-08-16T10:05:13Z

This pull request has been marked as stale due to 60 days of inactivity. It will be closed in 4 weeks if no further activity occurs. If you think that's incorrect or this pull request should instead be reviewed, please simply write any comment. Even if closed, you can still revive the PR at any time or discuss it on the [email protected] list. Thank you for your contributions.

mikyll · 2024-09-13T10:29:39Z

Hello, any news on this PR?

shreemaan-abhishek · 2024-09-20T05:43:22Z

apisix/plugins/jwt-auth.lua

+local function get_real_payload(key, auth_conf, payload, key_claim_name)
    local real_payload = {
-        key = key,
+        [key_claim_name] = key,


although the key_claim_name has changed, the value of key_claim_name will always be key. That doesn't fit into the scene. For example, if the key_claim_name is exp then the value should be the expiry time, not the key. Right?

@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_name parameter 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 the key claim, but do include iss instead.

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_name was "exp", then no, the value shouldn't be the expiry time, but still the key. However, that would cause some problem:

even though according to JWT standard, RFC 7519, the exp claim is optional, the jwt-auth plugin requires it to validate the token, and returns 401 if it's not present (at this line):
[warn] 64#64: *16957 [lua] jwt-auth.lua:442: phase_func(): failed to verify jwt: Missing one of claims - [ nbf, exp ].,

setting key_claim_name to "exp" is not feasible, since that would generate a token with duplicated exp claim.

At most, we could handle this case by preventing the usage of "exp" and "nbf" as key_claim_name, what do you think?

shreemaan-abhishek · 2024-09-20T05:44:06Z

@mikyll, please rebase with master there have been some changes in the CI.

Features: - config param "key_claim_name" (default = "key"), so for example one could use "iss" to check the validity of the JWT; Style: - 2 blank lines between functions; - 1 blank like before "else" and "elseif"; - jwt -> JWT; - Capitalized logs and response messages; - Added description for each schema configuration parameter;

I added a new test case for feature #11276 Since the default value of the new config parameter "key_claim_name" is "key", "default behaviour" is already validated by other tests.

+ Removed external httpbin upstream in favor of local endpoint (/hello); + Cleaned code;

…into jwt_key_claim_name

mikyll · 2024-09-22T12:59:47Z

@shreemaan-abhishek I've done the rebase with master, and I also left a reply to your comment.

Possible enhancements for that get_real_payload() concern, that come to my mind are:

(simplest one) reverting the changes to the public API and make a test case for the key_claim_name scenario that just uses a hardcoded token, without generating it with the public API;
preventing the usage of exp and nbf claims as values for the key_claim_name query parameter in the public API (e.g. calls to apisix_host:9180/apisix/plugin/jwt/sign?key=custom-user-key&key_claim_name=exp would return an error, such as "400: cannot use 'exp' or 'nbf' as key_claim_name");

github-actions · 2024-11-22T10:06:37Z

This pull request has been marked as stale due to 60 days of inactivity. It will be closed in 4 weeks if no further activity occurs. If you think that's incorrect or this pull request should instead be reviewed, please simply write any comment. Even if closed, you can still revive the PR at any time or discuss it on the [email protected] list. Thank you for your contributions.

mikyll · 2024-11-22T20:55:45Z

Closing this since it's outdated and is being implemented in #11772 👀

mikyll changed the title ~~Fix #11276 + Little style refactor~~ feat(jwt-auth): Fix #11276 + Little style refactor May 23, 2024

mikyll marked this pull request as ready for review May 23, 2024 14:50

mikyll and others added 2 commits May 23, 2024 17:03

Update jwt-auth.md

afb3d6b

infra: Increase PR reviewers to 3 when merge to master. (#11280)

b88cf9d

We need more eyes to improve code quality

shreemaan-abhishek reviewed May 29, 2024

View reviewed changes

flearc and others added 7 commits May 30, 2024 10:00

docs: correct the default collector config apisix actually used for o…

55eeb6b

…pentelemetry plugin (#11247)

docs: added Write a Review link (#11313)

c2ba478

docs: add http3 docs (#11302)

08cb3ad

Co-authored-by: Navendu Pottekkat <[email protected]> Co-authored-by: Abhishek Choudhary <[email protected]>

feat: move tinyyaml to lyaml (#11312)

cf84292

fix(ssl): ssl key rotation caused request failure (#11305)

d106de5

Revert style changes to jwt-auth plugin

bd76c43

Added test case for new jwt-auth feature

2af7e15

I added a new test case for feature #11276 Since the default value of the new config parameter "key_claim_name" is "key", "default behaviour" is already validated by other tests.

mikyll changed the title ~~feat(jwt-auth): Fix #11276 + Little style refactor~~ feat(jwt-auth): Fix #11276 Jun 4, 2024

Update jwt-auth4.t

92bd025

+ Removed external httpbin upstream in favor of local endpoint (/hello); + Cleaned code;

adam-huganir and others added 7 commits June 5, 2024 15:12

fix: add libyaml-dev dependency for apt. (#11291)

d77d672

fix: after updating the header, get the old value from the ctx.var (#…

953be46

…11329)

docs: add plugin config to standalone deployment doc (#11332)

0cacb90

docs: add http/3 in README.md. (#11318)

fec3137

build(undeps): remove all rocks before remove openresty (#11333)

3ad9c28

feat(secret): support store ssl.keys ssl.certs in secrets mamager (#1…

5e383e0

…1339)

fix(datadog): report consumer username tag (#11354)

1a45d1d

mikyll requested a review from shreemaan-abhishek June 17, 2024 06:48

kayx23 and others added 5 commits June 26, 2024 11:18

docs: improve debug mode yaml comments (#11373)

de1669d

fix: make the message clearer when API key is missing (#11370)

6d2de7e

docs: add http-dubbo docs (#11322)

4dbecfd

Co-authored-by: Traky Deng <[email protected]>

ci: removed centos, chaos, fuzzing and fips CIs. (#11394)

7f649bc

fix(grpc-transcode): filter out illegal INT(string) formats (#11367)

1164374

github-actions bot added the stale label Aug 16, 2024

github-actions bot removed the stale label Sep 14, 2024

shreemaan-abhishek reviewed Sep 20, 2024

View reviewed changes

shreemaan-abhishek changed the title ~~feat(jwt-auth): Fix #11276~~ feat(jwt-auth): support configuring key_claim_name Sep 20, 2024

dosubot bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Sep 20, 2024

shreemaan-abhishek mentioned this pull request Sep 20, 2024

Enhance jwt-auth Plugin with Configurable Claim Name and Support for Asymmetric Algorithms #11511

Closed

5 tasks

mikyll added 6 commits September 20, 2024 17:21

Update jwt-auth.md

b2ed6b8

Revert style changes to jwt-auth plugin

c944947

Added test case for new jwt-auth feature

1ddbc8f

I added a new test case for feature #11276 Since the default value of the new config parameter "key_claim_name" is "key", "default behaviour" is already validated by other tests.

Update jwt-auth4.t

76816f1

+ Removed external httpbin upstream in favor of local endpoint (/hello); + Cleaned code;

Merge branch 'jwt_key_claim_name' of https://github.com/mikyll/apisix …

1203a5a

…into jwt_key_claim_name

dosubot bot added size:XXL This PR changes 1000+ lines, ignoring generated files. and removed size:L This PR changes 100-499 lines, ignoring generated files. labels Sep 22, 2024

github-actions bot added the stale label Nov 22, 2024

mikyll closed this Nov 22, 2024

mikyll deleted the jwt_key_claim_name branch March 6, 2025 14:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat(jwt-auth): support configuring `key_claim_name` #11282

feat(jwt-auth): support configuring `key_claim_name` #11282

Uh oh!

mikyll commented May 23, 2024 •

edited

Loading

Uh oh!

shreemaan-abhishek left a comment •

edited

Loading

Uh oh!

mikyll commented May 29, 2024

Uh oh!

mikyll commented Jun 4, 2024 •

edited

Loading

Uh oh!

github-actions bot commented Aug 16, 2024

Uh oh!

mikyll commented Sep 13, 2024

Uh oh!

shreemaan-abhishek Sep 20, 2024

Uh oh!

mikyll Sep 22, 2024 •

edited

Loading

Uh oh!

shreemaan-abhishek commented Sep 20, 2024

Uh oh!

mikyll commented Sep 22, 2024 •

edited

Loading

Uh oh!

github-actions bot commented Nov 22, 2024

Uh oh!

mikyll commented Nov 22, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

12 participants

feat(jwt-auth): support configuring key_claim_name #11282

feat(jwt-auth): support configuring key_claim_name #11282

Uh oh!

Conversation

mikyll commented May 23, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Uh oh!

shreemaan-abhishek left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mikyll commented May 29, 2024

Uh oh!

mikyll commented Jun 4, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Aug 16, 2024

Uh oh!

mikyll commented Sep 13, 2024

Uh oh!

shreemaan-abhishek Sep 20, 2024

Choose a reason for hiding this comment

Uh oh!

mikyll Sep 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shreemaan-abhishek commented Sep 20, 2024

Uh oh!

mikyll commented Sep 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Nov 22, 2024

Uh oh!

mikyll commented Nov 22, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

12 participants

feat(jwt-auth): support configuring `key_claim_name` #11282

feat(jwt-auth): support configuring `key_claim_name` #11282

mikyll commented May 23, 2024 •

edited

Loading

shreemaan-abhishek left a comment •

edited

Loading

mikyll commented Jun 4, 2024 •

edited

Loading

mikyll Sep 22, 2024 •

edited

Loading

mikyll commented Sep 22, 2024 •

edited

Loading