You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by GitBox <gi...@apache.org> on 2021/04/21 09:26:36 UTC
[GitHub] [apisix] benx203 opened a new pull request #4099: feat: jwt-auth support extension payload
benx203 opened a new pull request #4099:
URL: https://github.com/apache/apisix/pull/4099
### What this PR does / why we need it:
<!--- Why is this change required? What problem does it solve? -->
<!--- If it fixes an open issue, please link to the issue here. -->
Make jwt-auth support extension payload.
[Relative issue](https://github.com/apache/apisix/issues/3594)
### Pre-submission checklist:
* [X] Did you explain what problem does this PR solve? Or what new features have been added?
* [ ] Have you added corresponding test cases?
* [X] Have you modified the corresponding document?
* [X] Is this PR backward compatible? **If it is not backward compatible, please discuss on the [mailing list](https://github.com/apache/apisix/tree/master#community) first**
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] benx203 commented on a change in pull request #4099: feat: jwt-auth support extension payload
Posted by GitBox <gi...@apache.org>.
benx203 commented on a change in pull request #4099:
URL: https://github.com/apache/apisix/pull/4099#discussion_r617495003
##########
File path: apisix/plugins/jwt-auth.lua
##########
@@ -104,7 +105,7 @@ do
core.table.clear(consumer_names)
for _, consumer in ipairs(consumers.nodes) do
- core.log.info("consumer node: ", core.json.delay_encode(consumer))
+ log.info("consumer node: ", core.json.delay_encode(consumer))
Review comment:
OK.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a change in pull request #4099: feat: jwt-auth support extension payload
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #4099:
URL: https://github.com/apache/apisix/pull/4099#discussion_r618168934
##########
File path: t/plugin/jwt-auth.t
##########
@@ -1209,7 +1241,39 @@ hello world
-=== TEST 43: test for unsupported algorithm
+=== TEST 44: sign / verify (algorithm = HS512,with extra payload)
Review comment:
Maybe you are misled by my example. We need two tests, one for default and another for RS256 (but not for HS512).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] benx203 commented on a change in pull request #4099: feat: jwt-auth support extension payload
Posted by GitBox <gi...@apache.org>.
benx203 commented on a change in pull request #4099:
URL: https://github.com/apache/apisix/pull/4099#discussion_r618107663
##########
File path: apisix/plugins/jwt-auth.lua
##########
@@ -277,18 +285,44 @@ function _M.rewrite(conf, ctx)
if not consumer then
return 401, {message = "Invalid user key in JWT token"}
end
- core.log.info("consumer: ", core.json.delay_encode(consumer))
+ log.info("consumer: ", core.json.delay_encode(consumer))
local _, auth_secret = algorithm_handler(consumer)
jwt_obj = jwt:verify_jwt_obj(auth_secret, jwt_obj)
- core.log.info("jwt object: ", core.json.delay_encode(jwt_obj))
+ log.info("jwt object: ", core.json.delay_encode(jwt_obj))
if not jwt_obj.verified then
return 401, {message = jwt_obj.reason}
end
consumer_mod.attach_consumer(ctx, consumer, consumer_conf)
- core.log.info("hit jwt-auth rewrite")
+ log.info("hit jwt-auth rewrite")
+end
+
+
+local function user_info()
Review comment:
I get it,thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander merged pull request #4099: feat: jwt-auth support extension payload
Posted by GitBox <gi...@apache.org>.
spacewander merged pull request #4099:
URL: https://github.com/apache/apisix/pull/4099
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a change in pull request #4099: feat: jwt-auth support extension payload
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #4099:
URL: https://github.com/apache/apisix/pull/4099#discussion_r617384247
##########
File path: apisix/plugins/jwt-auth.lua
##########
@@ -104,7 +105,7 @@ do
core.table.clear(consumer_names)
for _, consumer in ipairs(consumers.nodes) do
- core.log.info("consumer node: ", core.json.delay_encode(consumer))
+ log.info("consumer node: ", core.json.delay_encode(consumer))
Review comment:
The original `core.log` works well. Please don't touch unrelative code.
##########
File path: apisix/plugins/jwt-auth.lua
##########
@@ -277,18 +285,44 @@ function _M.rewrite(conf, ctx)
if not consumer then
return 401, {message = "Invalid user key in JWT token"}
end
- core.log.info("consumer: ", core.json.delay_encode(consumer))
+ log.info("consumer: ", core.json.delay_encode(consumer))
local _, auth_secret = algorithm_handler(consumer)
jwt_obj = jwt:verify_jwt_obj(auth_secret, jwt_obj)
- core.log.info("jwt object: ", core.json.delay_encode(jwt_obj))
+ log.info("jwt object: ", core.json.delay_encode(jwt_obj))
if not jwt_obj.verified then
return 401, {message = jwt_obj.reason}
end
consumer_mod.attach_consumer(ctx, consumer, consumer_conf)
- core.log.info("hit jwt-auth rewrite")
+ log.info("hit jwt-auth rewrite")
+end
+
+
+local function user_info()
Review comment:
Is this function necessary for the core feature? We should only accept the necessary API to avoid bloating.
##########
File path: apisix/plugins/jwt-auth.lua
##########
@@ -186,48 +187,55 @@ local function get_secret(conf)
end
-local function sign_jwt_with_HS(key, auth_conf)
+local function get_real_payload(key, auth_conf, payload)
+ local real_payload = {
+ key = key,
+ exp = ngx_time() + auth_conf.exp
+ }
+ if payload then
+ local payloadEx = core.json.decode(payload)
Review comment:
Better to use `extra_payload`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] benx203 commented on a change in pull request #4099: feat: jwt-auth support extension payload
Posted by GitBox <gi...@apache.org>.
benx203 commented on a change in pull request #4099:
URL: https://github.com/apache/apisix/pull/4099#discussion_r618193356
##########
File path: t/plugin/jwt-auth.t
##########
@@ -1209,7 +1241,39 @@ hello world
-=== TEST 43: test for unsupported algorithm
+=== TEST 44: sign / verify (algorithm = HS512,with extra payload)
Review comment:
Test 19 is for RS256.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] benx203 commented on a change in pull request #4099: feat: jwt-auth support extension payload
Posted by GitBox <gi...@apache.org>.
benx203 commented on a change in pull request #4099:
URL: https://github.com/apache/apisix/pull/4099#discussion_r618193356
##########
File path: t/plugin/jwt-auth.t
##########
@@ -1209,7 +1241,39 @@ hello world
-=== TEST 43: test for unsupported algorithm
+=== TEST 44: sign / verify (algorithm = HS512,with extra payload)
Review comment:
Test 19 is for RS256.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] benx203 commented on a change in pull request #4099: feat: jwt-auth support extension payload
Posted by GitBox <gi...@apache.org>.
benx203 commented on a change in pull request #4099:
URL: https://github.com/apache/apisix/pull/4099#discussion_r617490318
##########
File path: apisix/plugins/jwt-auth.lua
##########
@@ -277,18 +285,44 @@ function _M.rewrite(conf, ctx)
if not consumer then
return 401, {message = "Invalid user key in JWT token"}
end
- core.log.info("consumer: ", core.json.delay_encode(consumer))
+ log.info("consumer: ", core.json.delay_encode(consumer))
local _, auth_secret = algorithm_handler(consumer)
jwt_obj = jwt:verify_jwt_obj(auth_secret, jwt_obj)
- core.log.info("jwt object: ", core.json.delay_encode(jwt_obj))
+ log.info("jwt object: ", core.json.delay_encode(jwt_obj))
if not jwt_obj.verified then
return 401, {message = jwt_obj.reason}
end
consumer_mod.attach_consumer(ctx, consumer, consumer_conf)
- core.log.info("hit jwt-auth rewrite")
+ log.info("hit jwt-auth rewrite")
+end
+
+
+local function user_info()
Review comment:
Yes,the function provide ability to check up the payload content.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on pull request #4099: feat: jwt-auth support extension payload
Posted by GitBox <gi...@apache.org>.
spacewander commented on pull request #4099:
URL: https://github.com/apache/apisix/pull/4099#issuecomment-824587413
Would you add two test cases with payload, one for default algorithm and another for RS256?
Like https://github.com/apache/apisix/blob/86e49837d20147471eeb852d60d4c5e2f15a2bd4/t/plugin/jwt-auth.t#L1180
Thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] benx203 commented on pull request #4099: feat: jwt-auth support extension payload
Posted by GitBox <gi...@apache.org>.
benx203 commented on pull request #4099:
URL: https://github.com/apache/apisix/pull/4099#issuecomment-824865435
> Next time, please add new tests **at the bottom**.
@spacewander OK,get it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a change in pull request #4099: feat: jwt-auth support extension payload
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #4099:
URL: https://github.com/apache/apisix/pull/4099#discussion_r617991536
##########
File path: apisix/plugins/jwt-auth.lua
##########
@@ -277,18 +285,44 @@ function _M.rewrite(conf, ctx)
if not consumer then
return 401, {message = "Invalid user key in JWT token"}
end
- core.log.info("consumer: ", core.json.delay_encode(consumer))
+ log.info("consumer: ", core.json.delay_encode(consumer))
local _, auth_secret = algorithm_handler(consumer)
jwt_obj = jwt:verify_jwt_obj(auth_secret, jwt_obj)
- core.log.info("jwt object: ", core.json.delay_encode(jwt_obj))
+ log.info("jwt object: ", core.json.delay_encode(jwt_obj))
if not jwt_obj.verified then
return 401, {message = jwt_obj.reason}
end
consumer_mod.attach_consumer(ctx, consumer, consumer_conf)
- core.log.info("hit jwt-auth rewrite")
+ log.info("hit jwt-auth rewrite")
+end
+
+
+local function user_info()
Review comment:
This feature can be implemented as a local script to check.
There is no need to expose it as a public API of the gateway.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org