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 2022/10/30 18:21:53 UTC
[GitHub] [apisix] pixeldin opened a new pull request, #8206: feat: support hide credentials for jwt-auth plugin
pixeldin opened a new pull request, #8206:
URL: https://github.com/apache/apisix/pull/8206
### Description
Provides an optional feat for the jwt-auth plugin to hide authentication upstream as requested in [#8122](https://github.com/apache/apisix/issues/8122).
Setting hide_credentials to true in the plugin config will hide related auth parameters in upstream.
<!-- Please also include relevant motivation and context. -->
Fixes [#8122](https://github.com/apache/apisix/issues/8122)
### Checklist
- [x] I have explained the need for this PR and the problem it solves
- [x] I have explained the changes or the new features added to this PR
- [x] I have added tests corresponding to this change
- [x] 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](https://github.com/apache/apisix/tree/master#community) first)
<!--
Note
1. Mark the PR as draft until it's ready to be reviewed.
2. Always add/update tests for any changes unless you have a good reason.
3. Always update the documentation to reflect the changes made in the PR.
4. Make a new commit to resolve conversations instead of `push -f`.
5. To resolve merge conflicts, merge master instead of rebasing.
6. Use "request review" to notify the reviewer after making changes.
7. Only a reviewer can mark a conversation as resolved.
-->
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] tokers commented on a diff in pull request #8206: feat: support hide credentials for jwt-auth plugin
Posted by GitBox <gi...@apache.org>.
tokers commented on code in PR #8206:
URL: https://github.com/apache/apisix/pull/8206#discussion_r1008965600
##########
apisix/plugins/jwt-auth.lua:
##########
@@ -407,6 +434,27 @@ function _M.rewrite(conf, ctx)
return 401, {message = "failed to verify jwt"}
end
+ -- check for hiding `Authorization` request header if `hide_credentials` is `true`
+ if conf.hide_credentials then
+ -- hide sensitive field
+ if from_header then
+ -- hide for header
+ local temp_token = core.request.header(ctx, conf.header)
+ core.request.set_header(ctx, conf.header, nil)
+
+
Review Comment:
Redundant empty lines? I think one line is enough.
##########
t/plugin/jwt-auth3.t:
##########
@@ -0,0 +1,278 @@
+use t::APISIX 'no_plan';
+
+repeat_each(1);
+no_long_string();
+no_root_location();
+no_shuffle();
+
+add_block_preprocessor(sub {
+ my ($block) = @_;
+
+ if ((!defined $block->error_log) && (!defined $block->no_error_log)) {
+ $block->set_value("no_error_log", "[error]");
+ }
+
+ if (!defined $block->request) {
+ $block->set_value("request", "GET /t");
+ }
+});
+
+run_tests;
+
+__DATA__
+
+=== TEST 1: add consumer with username and plugins
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/consumers',
+ ngx.HTTP_PUT,
+ [[{
+ "username": "jack",
+ "plugins": {
+ "jwt-auth": {
+ "key": "user-key",
+ "secret": "my-secret-key"
+ }
+ }
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- response_body
+passed
+
+
+
+=== TEST 2: enable jwt auth plugin using admin api with custom parameter
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PUT,
+ [[{
+ "plugins": {
+ "jwt-auth": {
+ "header": "jwt-header",
+ "query": "jwt-query",
+ "cookie": "jwt-cookie",
+ "hide_credentials": false
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "uri": "/echo"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+
+
+
+=== TEST 3: verify (in header) with not hidden auth
Review Comment:
```suggestion
=== TEST 3: verify (in header) with not hide credentials
```
##########
t/plugin/jwt-auth3.t:
##########
@@ -0,0 +1,278 @@
+use t::APISIX 'no_plan';
+
+repeat_each(1);
+no_long_string();
+no_root_location();
+no_shuffle();
+
+add_block_preprocessor(sub {
+ my ($block) = @_;
+
+ if ((!defined $block->error_log) && (!defined $block->no_error_log)) {
+ $block->set_value("no_error_log", "[error]");
+ }
+
+ if (!defined $block->request) {
+ $block->set_value("request", "GET /t");
+ }
+});
+
+run_tests;
+
+__DATA__
+
+=== TEST 1: add consumer with username and plugins
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/consumers',
+ ngx.HTTP_PUT,
+ [[{
+ "username": "jack",
+ "plugins": {
+ "jwt-auth": {
+ "key": "user-key",
+ "secret": "my-secret-key"
+ }
+ }
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- response_body
+passed
+
+
+
+=== TEST 2: enable jwt auth plugin using admin api with custom parameter
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PUT,
+ [[{
+ "plugins": {
+ "jwt-auth": {
+ "header": "jwt-header",
+ "query": "jwt-query",
+ "cookie": "jwt-cookie",
+ "hide_credentials": false
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "uri": "/echo"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+
+
+
+=== TEST 3: verify (in header) with not hidden auth
+--- request
+GET /echo
+--- more_headers
+jwt-header: Bearer eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJrZXkiOiJ1c2VyLWtleSIsImV4cCI6MTg3OTMxODU0MX0.fNtFJnNmJgzbiYmGB0Yjvm-l6A6M4jRV1l4mnVFSYjs
+--- response_headers
+jwt-header: Bearer eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJrZXkiOiJ1c2VyLWtleSIsImV4cCI6MTg3OTMxODU0MX0.fNtFJnNmJgzbiYmGB0Yjvm-l6A6M4jRV1l4mnVFSYjs
+
+
+
+=== TEST 4: verify (in cookie) with not hidden auth
Review Comment:
```suggestion
=== TEST 4: verify (in cookie) with not hide credentials
```
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] tzssangglass commented on a diff in pull request #8206: feat: support hide credentials for jwt-auth plugin
Posted by GitBox <gi...@apache.org>.
tzssangglass commented on code in PR #8206:
URL: https://github.com/apache/apisix/pull/8206#discussion_r1019797603
##########
t/plugin/jwt-auth3.t:
##########
@@ -0,0 +1,309 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+use t::APISIX 'no_plan';
+
+repeat_each(1);
+no_long_string();
+no_root_location();
+no_shuffle();
+
+add_block_preprocessor(sub {
+ my ($block) = @_;
+
+ if ((!defined $block->error_log) && (!defined $block->no_error_log)) {
+ $block->set_value("no_error_log", "[error]");
+ }
+
+ if (!defined $block->request) {
+ $block->set_value("request", "GET /t");
+ }
+
+ if (!$block->response_body) {
+ $block->set_value("response_body", "passed\n");
+ }
+});
+
+run_tests;
+
+__DATA__
+
+=== TEST 1: add consumer with username and plugins
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/consumers',
+ ngx.HTTP_PUT,
+ [[{
+ "username": "jack",
+ "plugins": {
+ "jwt-auth": {
+ "key": "user-key",
+ "secret": "my-secret-key"
+ }
+ }
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+
+
+
+=== TEST 2: enable jwt auth plugin using admin api with custom parameter
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PUT,
+ [[{
+ "plugins": {
+ "jwt-auth": {
+ "header": "jwt-header",
+ "query": "jwt-query",
+ "cookie": "jwt-cookie",
+ "hide_credentials": false
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "uri": "/echo"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+
+
+
+=== TEST 3: verify (in header) not hiding credentials
+--- request
+GET /echo
+--- more_headers
+jwt-header: Bearer eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJrZXkiOiJ1c2VyLWtleSIsImV4cCI6MTg3OTMxODU0MX0.fNtFJnNmJgzbiYmGB0Yjvm-l6A6M4jRV1l4mnVFSYjs
+--- response_headers
+jwt-header: Bearer eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJrZXkiOiJ1c2VyLWtleSIsImV4cCI6MTg3OTMxODU0MX0.fNtFJnNmJgzbiYmGB0Yjvm-l6A6M4jRV1l4mnVFSYjs
+--- response_body eval
+qr/^$/
+
+
+
+=== TEST 4: verify (in cookie) not hiding credentials
+--- request
+GET /echo
+--- more_headers
+Cookie: jwt-cookie=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJrZXkiOiJ1c2VyLWtleSIsImV4cCI6MTg3OTMxODU0MX0.fNtFJnNmJgzbiYmGB0Yjvm-l6A6M4jRV1l4mnVFSYjs
+--- response_headers
+Cookie: jwt-cookie=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJrZXkiOiJ1c2VyLWtleSIsImV4cCI6MTg3OTMxODU0MX0.fNtFJnNmJgzbiYmGB0Yjvm-l6A6M4jRV1l4mnVFSYjs
+--- response_body eval
+qr/^$/
+
+
+
+=== TEST 5: enable jwt auth plugin using admin api without hiding credentials
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PUT,
+ [[{
+ "plugins": {
+ "jwt-auth": {
+ "header": "jwt-header",
+ "query": "jwt-query",
+ "cookie": "jwt-cookie",
+ "hide_credentials": false
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "uri": "/plugin_proxy_rewrite_args"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+
+
+
+=== TEST 6: verify (in query) without hiding credentials
+--- request
+GET /plugin_proxy_rewrite_args?foo=bar&hello=world&jwt-query=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJrZXkiOiJ1c2VyLWtleSIsImV4cCI6MTg3OTMxODU0MX0.fNtFJnNmJgzbiYmGB0Yjvm-l6A6M4jRV1l4mnVFSYjs
+--- response_body
+uri: /plugin_proxy_rewrite_args
+foo: bar
+hello: world
+jwt-query: eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJrZXkiOiJ1c2VyLWtleSIsImV4cCI6MTg3OTMxODU0MX0.fNtFJnNmJgzbiYmGB0Yjvm-l6A6M4jRV1l4mnVFSYjs
+
+
+
+=== TEST 7: enable jwt auth plugin using admin api with hiding credentials
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PUT,
+ [[{
+ "plugins": {
+ "jwt-auth": {
+ "header": "jwt-header",
+ "query": "jwt-query",
+ "cookie": "jwt-cookie",
+ "hide_credentials": true
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "uri": "/echo"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+
+
+
+=== TEST 8: verify (in header) with hiding credentials
+--- request
+GET /echo
+--- more_headers
+jwt-header: eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJrZXkiOiJ1c2VyLWtleSIsImV4cCI6MTg3OTMxODU0MX0.fNtFJnNmJgzbiYmGB0Yjvm-l6A6M4jRV1l4mnVFSYjs
+--- response_headers
+!jwt-header
+--- response_body eval
+qr/^$/
+
+
+
+=== TEST 9: enable jwt auth plugin using admin api with hiding credentials
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PUT,
+ [[{
+ "plugins": {
+ "jwt-auth": {
+ "header": "jwt-header",
+ "query": "jwt-query",
+ "cookie": "jwt-cookie",
+ "hide_credentials": true
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "uri": "/plugin_proxy_rewrite_args"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+
+
+
+=== TEST 10: verify (in query) with hiding credentials
+--- request
+GET /plugin_proxy_rewrite_args?foo=bar&hello=world&jwt-query=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJrZXkiOiJ1c2VyLWtleSIsImV4cCI6MTg3OTMxODU0MX0.fNtFJnNmJgzbiYmGB0Yjvm-l6A6M4jRV1l4mnVFSYjs
+--- response_body
+uri: /plugin_proxy_rewrite_args
+foo: bar
+hello: world
+
+
+
+=== TEST 11: verify (in cookie) with hiding credentials
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PUT,
+ [[{
+ "plugins": {
+ "jwt-auth": {
+ "header": "jwt-header",
+ "query": "jwt-query",
+ "cookie": "jwt-cookie",
+ "hide_credentials": true
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "httpbin.org:80": 1
+ },
+ "type": "roundrobin"
+ },
+ "uri": "/get"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+
+
+
+=== TEST 12: verify (in cookie) with hiding credentials
+--- request
+GET /get
+--- more_headers
+Cookie: hello=world; jwt-cookie=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJrZXkiOiJ1c2VyLWtleSIsImV4cCI6MTg3OTMxODU0MX0.fNtFJnNmJgzbiYmGB0Yjvm-l6A6M4jRV1l4mnVFSYjs; foo=bar
+--- response_body eval
+qr/^(?:(?!jwt-cookie).)*\z/s
Review Comment:
to be on the safe side, I want this test case to visually see that the upstream cookie is `hello=world; foo=bar`
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] pixeldin commented on a diff in pull request #8206: feat: support hide credentials for jwt-auth plugin
Posted by GitBox <gi...@apache.org>.
pixeldin commented on code in PR #8206:
URL: https://github.com/apache/apisix/pull/8206#discussion_r1011805122
##########
apisix/plugins/jwt-auth.lua:
##########
@@ -200,15 +210,31 @@ local function fetch_jwt_token(conf, ctx)
return token
end
- token = ctx.var["arg_" .. conf.query]
+ local uri_args = core.request.get_uri_args(ctx) or {}
+ token = uri_args[conf.query]
if token then
+ if conf.hide_credentials then
+ -- hide for query
+ uri_args[conf.query] = nil
+ core.request.set_uri_args(ctx, uri_args)
+ end
return token
end
local val = ctx.var["cookie_" .. conf.cookie]
if not val then
return nil, "JWT not found in cookie"
end
+
+ if conf.hide_credentials then
+ -- hide for cookie
+ ck:new():set({
Review Comment:
Thanks, so what should be the correct approach to hide the credential of cookie from the upstream, can you provide some concrete examples?
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] pixeldin commented on pull request #8206: feat: support hide credentials for jwt-auth plugin
Posted by GitBox <gi...@apache.org>.
pixeldin commented on PR #8206:
URL: https://github.com/apache/apisix/pull/8206#issuecomment-1314873970
> @pixeldin Could you apply this patch in your code before we can merge it? Thanks!
>
> ```
> diff --git t/plugin/jwt-auth3.t t/plugin/jwt-auth3.t
> index c0a1961d..fd53d56a 100755
> --- t/plugin/jwt-auth3.t
> +++ t/plugin/jwt-auth3.t
> @@ -30,10 +30,10 @@ add_block_preprocessor(sub {
>
> if (!defined $block->request) {
> $block->set_value("request", "GET /t");
> - }
>
> - if (!$block->response_body) {
> - $block->set_value("response_body eval", "qr/^$/");
> + if (!$block->response_body) {
> + $block->set_value("response_body", "passed\n");
> + }
> }
> });
> ```
@spacewander Thanks, Done! Please review it when you have time.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a diff in pull request #8206: feat: support hide credentials for jwt-auth plugin
Posted by GitBox <gi...@apache.org>.
spacewander commented on code in PR #8206:
URL: https://github.com/apache/apisix/pull/8206#discussion_r1014819332
##########
apisix/plugins/jwt-auth.lua:
##########
@@ -200,15 +209,28 @@ local function fetch_jwt_token(conf, ctx)
return token
end
- token = ctx.var["arg_" .. conf.query]
+ local uri_args = core.request.get_uri_args(ctx) or {}
+ token = uri_args[conf.query]
if token then
+ if conf.hide_credentials then
+ -- hide for query
+ uri_args[conf.query] = nil
+ core.request.set_uri_args(ctx, uri_args)
+ end
return token
end
local val = ctx.var["cookie_" .. conf.cookie]
if not val then
return nil, "JWT not found in cookie"
end
+
+ if conf.hide_credentials then
+ -- hide for cookie
+ local reset_val = conf.cookie .. "=deleted; Max-Age=0"
Review Comment:
The Max-Age attribute is for Set-Cookie. What about parsing the Cookie header and removing the relative field?
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] pixeldin commented on a diff in pull request #8206: feat: support hide credentials for jwt-auth plugin
Posted by GitBox <gi...@apache.org>.
pixeldin commented on code in PR #8206:
URL: https://github.com/apache/apisix/pull/8206#discussion_r1015045563
##########
apisix/plugins/jwt-auth.lua:
##########
@@ -200,15 +209,28 @@ local function fetch_jwt_token(conf, ctx)
return token
end
- token = ctx.var["arg_" .. conf.query]
+ local uri_args = core.request.get_uri_args(ctx) or {}
+ token = uri_args[conf.query]
if token then
+ if conf.hide_credentials then
+ -- hide for query
+ uri_args[conf.query] = nil
+ core.request.set_uri_args(ctx, uri_args)
+ end
return token
end
local val = ctx.var["cookie_" .. conf.cookie]
if not val then
return nil, "JWT not found in cookie"
end
+
+ if conf.hide_credentials then
+ -- hide for cookie
+ local reset_val = conf.cookie .. "=deleted; Max-Age=0"
Review Comment:
Thanks, I will rewrite 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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] starsz commented on a diff in pull request #8206: feat: support hide credentials for jwt-auth plugin
Posted by GitBox <gi...@apache.org>.
starsz commented on code in PR #8206:
URL: https://github.com/apache/apisix/pull/8206#discussion_r1009044699
##########
apisix/plugins/jwt-auth.lua:
##########
@@ -357,8 +361,31 @@ local function algorithm_handler(consumer, method_only)
end
end
+local function set_our_cookie(name, val)
+ core.response.add_header("Set-Cookie", name .. "=" .. val)
+end
+
function _M.rewrite(conf, ctx)
+ local from_header = true
+ local header_key = core.request.header(ctx, conf.header)
+
+ local from_query = true
+
+ if not header_key then
+ from_header = false
+ local uri_args = core.request.get_uri_args(ctx) or {}
+ header_key = uri_args[conf.query]
+ if not header_key then
+ from_query = false
+ local cookie = ctx.var["cookie_" .. conf.cookie]
+ if not cookie then
+ core.log.info("failed to fetch JWT token")
+ return 401, {message = "Missing JWT token in request"}
+ end
+ end
+ end
Review Comment:
I think you can put this logic under the `fetch_jwt_token` function.
##########
apisix/plugins/jwt-auth.lua:
##########
@@ -407,6 +434,25 @@ function _M.rewrite(conf, ctx)
return 401, {message = "failed to verify jwt"}
end
+ -- check for hiding `Authorization` request header if `hide_credentials` is `true`
+ if conf.hide_credentials then
+ -- hide sensitive field
+ if from_header then
+ -- hide for header
+ core.request.set_header(ctx, conf.header, nil)
+
+ elseif from_query then
+ -- hide for query
+ local args = core.request.get_uri_args(ctx)
+ args[conf.query] = nil
+ core.request.set_uri_args(ctx, args)
+
+ else
+ -- hide for cookie
+ set_our_cookie(conf.cookie, "deleted; Max-Age=0")
Review Comment:
I think it's not a good way to hide the cookie.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a diff in pull request #8206: feat: support hide credentials for jwt-auth plugin
Posted by GitBox <gi...@apache.org>.
spacewander commented on code in PR #8206:
URL: https://github.com/apache/apisix/pull/8206#discussion_r1010168974
##########
apisix/plugins/jwt-auth.lua:
##########
@@ -357,8 +361,31 @@ local function algorithm_handler(consumer, method_only)
end
end
+local function set_our_cookie(name, val)
+ core.response.add_header("Set-Cookie", name .. "=" .. val)
Review Comment:
This function doesn't hide the cookie from the upstream, right?
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a diff in pull request #8206: feat: support hide credentials for jwt-auth plugin
Posted by GitBox <gi...@apache.org>.
spacewander commented on code in PR #8206:
URL: https://github.com/apache/apisix/pull/8206#discussion_r1015033592
##########
apisix/plugins/jwt-auth.lua:
##########
@@ -200,15 +209,28 @@ local function fetch_jwt_token(conf, ctx)
return token
end
- token = ctx.var["arg_" .. conf.query]
+ local uri_args = core.request.get_uri_args(ctx) or {}
+ token = uri_args[conf.query]
if token then
+ if conf.hide_credentials then
+ -- hide for query
+ uri_args[conf.query] = nil
+ core.request.set_uri_args(ctx, uri_args)
+ end
return token
end
local val = ctx.var["cookie_" .. conf.cookie]
if not val then
return nil, "JWT not found in cookie"
end
+
+ if conf.hide_credentials then
+ -- hide for cookie
+ local reset_val = conf.cookie .. "=deleted; Max-Age=0"
Review Comment:
Yes
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] pixeldin commented on a diff in pull request #8206: feat: support hide credentials for jwt-auth plugin
Posted by GitBox <gi...@apache.org>.
pixeldin commented on code in PR #8206:
URL: https://github.com/apache/apisix/pull/8206#discussion_r1014967365
##########
t/plugin/jwt-auth3.t:
##########
@@ -0,0 +1,333 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+use t::APISIX 'no_plan';
+
+repeat_each(1);
+no_long_string();
+no_root_location();
+no_shuffle();
+
+add_block_preprocessor(sub {
+ my ($block) = @_;
+
+ if ((!defined $block->error_log) && (!defined $block->no_error_log)) {
+ $block->set_value("no_error_log", "[error]");
+ }
+
+ if (!defined $block->request) {
+ $block->set_value("request", "GET /t");
+ }
+});
+
+run_tests;
+
+__DATA__
+
+=== TEST 1: add consumer with username and plugins
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/consumers',
+ ngx.HTTP_PUT,
+ [[{
+ "username": "jack",
+ "plugins": {
+ "jwt-auth": {
+ "key": "user-key",
+ "secret": "my-secret-key"
+ }
+ }
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- response_body
+passed
+
+
+
+=== TEST 2: enable jwt auth plugin using admin api with custom parameter
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PUT,
+ [[{
+ "plugins": {
+ "jwt-auth": {
+ "header": "jwt-header",
+ "query": "jwt-query",
+ "cookie": "jwt-cookie",
+ "hide_credentials": false
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "uri": "/echo"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+
+
+
+=== TEST 3: verify (in header) not hiding credentials
+--- request
+GET /echo
+--- more_headers
+jwt-header: Bearer eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJrZXkiOiJ1c2VyLWtleSIsImV4cCI6MTg3OTMxODU0MX0.fNtFJnNmJgzbiYmGB0Yjvm-l6A6M4jRV1l4mnVFSYjs
+--- response_headers
+jwt-header: Bearer eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJrZXkiOiJ1c2VyLWtleSIsImV4cCI6MTg3OTMxODU0MX0.fNtFJnNmJgzbiYmGB0Yjvm-l6A6M4jRV1l4mnVFSYjs
+
+
+
+=== TEST 4: verify (in cookie) not hiding credentials
+--- request
+GET /echo
+--- more_headers
+Cookie: jwt-cookie=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJrZXkiOiJ1c2VyLWtleSIsImV4cCI6MTg3OTMxODU0MX0.fNtFJnNmJgzbiYmGB0Yjvm-l6A6M4jRV1l4mnVFSYjs
+--- response_headers
+Cookie: jwt-cookie=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJrZXkiOiJ1c2VyLWtleSIsImV4cCI6MTg3OTMxODU0MX0.fNtFJnNmJgzbiYmGB0Yjvm-l6A6M4jRV1l4mnVFSYjs
+
+
+
+=== TEST 5: enable jwt auth plugin using admin api without hiding credentials
+# the `proxy-rewrite` play role as upstream to check sensitive param
Review Comment:
Thanks your opinion, I figure it out lately. I will delete the forward and request directly.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a diff in pull request #8206: feat: support hide credentials for jwt-auth plugin
Posted by GitBox <gi...@apache.org>.
spacewander commented on code in PR #8206:
URL: https://github.com/apache/apisix/pull/8206#discussion_r1016211700
##########
t/plugin/jwt-auth3.t:
##########
@@ -0,0 +1,327 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+use t::APISIX 'no_plan';
+
+repeat_each(1);
+no_long_string();
+no_root_location();
+no_shuffle();
+
+add_block_preprocessor(sub {
+ my ($block) = @_;
+
+ if ((!defined $block->error_log) && (!defined $block->no_error_log)) {
+ $block->set_value("no_error_log", "[error]");
+ }
+
+ if (!defined $block->request) {
+ $block->set_value("request", "GET /t");
+ }
+});
+
+run_tests;
+
+__DATA__
+
+=== TEST 1: add consumer with username and plugins
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/consumers',
+ ngx.HTTP_PUT,
+ [[{
+ "username": "jack",
+ "plugins": {
+ "jwt-auth": {
+ "key": "user-key",
+ "secret": "my-secret-key"
+ }
+ }
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- response_body
+passed
+
+
+
+=== TEST 2: enable jwt auth plugin using admin api with custom parameter
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PUT,
+ [[{
+ "plugins": {
+ "jwt-auth": {
+ "header": "jwt-header",
+ "query": "jwt-query",
+ "cookie": "jwt-cookie",
+ "hide_credentials": false
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "uri": "/echo"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+
+
+
+=== TEST 3: verify (in header) not hiding credentials
+--- request
+GET /echo
+--- more_headers
+jwt-header: Bearer eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJrZXkiOiJ1c2VyLWtleSIsImV4cCI6MTg3OTMxODU0MX0.fNtFJnNmJgzbiYmGB0Yjvm-l6A6M4jRV1l4mnVFSYjs
+--- response_headers
+jwt-header: Bearer eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJrZXkiOiJ1c2VyLWtleSIsImV4cCI6MTg3OTMxODU0MX0.fNtFJnNmJgzbiYmGB0Yjvm-l6A6M4jRV1l4mnVFSYjs
+
+
+
+=== TEST 4: verify (in cookie) not hiding credentials
+--- request
+GET /echo
+--- more_headers
+Cookie: jwt-cookie=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJrZXkiOiJ1c2VyLWtleSIsImV4cCI6MTg3OTMxODU0MX0.fNtFJnNmJgzbiYmGB0Yjvm-l6A6M4jRV1l4mnVFSYjs
+--- response_headers
+Cookie: jwt-cookie=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJrZXkiOiJ1c2VyLWtleSIsImV4cCI6MTg3OTMxODU0MX0.fNtFJnNmJgzbiYmGB0Yjvm-l6A6M4jRV1l4mnVFSYjs
+
+
+
+=== TEST 5: enable jwt auth plugin using admin api without hiding credentials
+# the `proxy-rewrite` play role as upstream to check sensitive param
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PUT,
+ [[{
+ "plugins": {
+ "jwt-auth": {
+ "header": "jwt-header",
+ "query": "jwt-query",
+ "cookie": "jwt-cookie",
+ "hide_credentials": false
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "uri": "/echo"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+
+
+
+=== TEST 6: verify (in query) without hiding credentials
+--- request
+GET /plugin_proxy_rewrite_args?foo=bar&hello=world&jwt-query=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJrZXkiOiJ1c2VyLWtleSIsImV4cCI6MTg3OTMxODU0MX0.fNtFJnNmJgzbiYmGB0Yjvm-l6A6M4jRV1l4mnVFSYjs
+--- response_body
+uri: /plugin_proxy_rewrite_args
+foo: bar
+hello: world
+jwt-query: eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJrZXkiOiJ1c2VyLWtleSIsImV4cCI6MTg3OTMxODU0MX0.fNtFJnNmJgzbiYmGB0Yjvm-l6A6M4jRV1l4mnVFSYjs
+--- no_error_log
+[error]
+
+
+
+=== TEST 7: enable jwt auth plugin using admin api with hiding credentials
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PUT,
+ [[{
+ "plugins": {
+ "jwt-auth": {
+ "header": "jwt-header",
+ "query": "jwt-query",
+ "cookie": "jwt-cookie",
+ "hide_credentials": true
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "uri": "/echo"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
Review Comment:
We don't need to set `GET /t` as we already set it at the top of this file.
##########
t/plugin/jwt-auth3.t:
##########
@@ -0,0 +1,327 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+use t::APISIX 'no_plan';
+
+repeat_each(1);
+no_long_string();
+no_root_location();
+no_shuffle();
+
+add_block_preprocessor(sub {
+ my ($block) = @_;
+
+ if ((!defined $block->error_log) && (!defined $block->no_error_log)) {
+ $block->set_value("no_error_log", "[error]");
+ }
+
+ if (!defined $block->request) {
+ $block->set_value("request", "GET /t");
+ }
+});
+
+run_tests;
+
+__DATA__
+
+=== TEST 1: add consumer with username and plugins
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/consumers',
+ ngx.HTTP_PUT,
+ [[{
+ "username": "jack",
+ "plugins": {
+ "jwt-auth": {
+ "key": "user-key",
+ "secret": "my-secret-key"
+ }
+ }
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- response_body
+passed
+
+
+
+=== TEST 2: enable jwt auth plugin using admin api with custom parameter
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PUT,
+ [[{
+ "plugins": {
+ "jwt-auth": {
+ "header": "jwt-header",
+ "query": "jwt-query",
+ "cookie": "jwt-cookie",
+ "hide_credentials": false
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "uri": "/echo"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+
+
+
+=== TEST 3: verify (in header) not hiding credentials
+--- request
+GET /echo
+--- more_headers
+jwt-header: Bearer eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJrZXkiOiJ1c2VyLWtleSIsImV4cCI6MTg3OTMxODU0MX0.fNtFJnNmJgzbiYmGB0Yjvm-l6A6M4jRV1l4mnVFSYjs
+--- response_headers
+jwt-header: Bearer eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJrZXkiOiJ1c2VyLWtleSIsImV4cCI6MTg3OTMxODU0MX0.fNtFJnNmJgzbiYmGB0Yjvm-l6A6M4jRV1l4mnVFSYjs
+
+
+
+=== TEST 4: verify (in cookie) not hiding credentials
+--- request
+GET /echo
+--- more_headers
+Cookie: jwt-cookie=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJrZXkiOiJ1c2VyLWtleSIsImV4cCI6MTg3OTMxODU0MX0.fNtFJnNmJgzbiYmGB0Yjvm-l6A6M4jRV1l4mnVFSYjs
+--- response_headers
+Cookie: jwt-cookie=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJrZXkiOiJ1c2VyLWtleSIsImV4cCI6MTg3OTMxODU0MX0.fNtFJnNmJgzbiYmGB0Yjvm-l6A6M4jRV1l4mnVFSYjs
+
+
+
+=== TEST 5: enable jwt auth plugin using admin api without hiding credentials
+# the `proxy-rewrite` play role as upstream to check sensitive param
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PUT,
+ [[{
+ "plugins": {
+ "jwt-auth": {
+ "header": "jwt-header",
+ "query": "jwt-query",
+ "cookie": "jwt-cookie",
+ "hide_credentials": false
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "uri": "/echo"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+
+
+
+=== TEST 6: verify (in query) without hiding credentials
+--- request
+GET /plugin_proxy_rewrite_args?foo=bar&hello=world&jwt-query=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJrZXkiOiJ1c2VyLWtleSIsImV4cCI6MTg3OTMxODU0MX0.fNtFJnNmJgzbiYmGB0Yjvm-l6A6M4jRV1l4mnVFSYjs
Review Comment:
We need to setup a route for `plugin_proxy_rewrite_args` first.
##########
docs/zh/latest/plugins/jwt-auth.md:
##########
@@ -66,6 +66,7 @@ Route 端：
| header | string | 否 | authorization | 设置我们从哪个 header 获取 token。 |
| query | string | 否 | jwt | 设置我们从哪个 query string 获取 token，优先级低于 header。 |
| cookie | string | 否 | jwt | 设置我们从哪个 cookie 获取 token，优先级低于 query。 |
+| hide_credentials | boolean | 否 | false | 该参数设置为 `true` 时，则不会将含有认证信息的 header\query\cookie string 传递给 Upstream。|
Review Comment:
```suggestion
| hide_credentials | boolean | 否 | false | 该参数设置为 `true` 时，则不会将含有认证信息的 header\query\cookie 传递给 Upstream。|
```
##########
apisix/plugins/jwt-auth.lua:
##########
@@ -188,10 +192,33 @@ function _M.check_schema(conf, schema_type)
return true
end
+local function remove_specified_cookie(src, key)
+ local ret = ""
+ local append = false
+ local cookie_key_pattern = "([a-zA-Z0-9-_]*)"
+ local cookie_val_pattern = "([a-zA-Z0-9-._]*)"
+
+ for k, v in string.gmatch(src, cookie_key_pattern .. "=" .. cookie_val_pattern) do
+ if k ~= key then
+ if append then
+ ret = ret .. "; "
+ end
+ ret = ret .. k .. "=" .. v
+ append = true
Review Comment:
We can use table.concat to do the same thing?
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] tzssangglass commented on a diff in pull request #8206: feat: support hide credentials for jwt-auth plugin
Posted by GitBox <gi...@apache.org>.
tzssangglass commented on code in PR #8206:
URL: https://github.com/apache/apisix/pull/8206#discussion_r1017509546
##########
t/plugin/jwt-auth3.t:
##########
@@ -0,0 +1,303 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+use t::APISIX 'no_plan';
+
+repeat_each(1);
+no_long_string();
+no_root_location();
+no_shuffle();
+
+add_block_preprocessor(sub {
+ my ($block) = @_;
+
+ if ((!defined $block->error_log) && (!defined $block->no_error_log)) {
+ $block->set_value("no_error_log", "[error]");
+ }
+
+ if (!defined $block->request) {
+ $block->set_value("request", "GET /t");
+ }
+});
+
+run_tests;
+
+__DATA__
+
+=== TEST 1: add consumer with username and plugins
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/consumers',
+ ngx.HTTP_PUT,
+ [[{
+ "username": "jack",
+ "plugins": {
+ "jwt-auth": {
+ "key": "user-key",
+ "secret": "my-secret-key"
+ }
+ }
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- response_body
+passed
+
+
+
+=== TEST 2: enable jwt auth plugin using admin api with custom parameter
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PUT,
+ [[{
+ "plugins": {
+ "jwt-auth": {
+ "header": "jwt-header",
+ "query": "jwt-query",
+ "cookie": "jwt-cookie",
+ "hide_credentials": false
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "uri": "/echo"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+
+
+
+=== TEST 3: verify (in header) not hiding credentials
+--- request
+GET /echo
+--- more_headers
+jwt-header: Bearer eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJrZXkiOiJ1c2VyLWtleSIsImV4cCI6MTg3OTMxODU0MX0.fNtFJnNmJgzbiYmGB0Yjvm-l6A6M4jRV1l4mnVFSYjs
+--- response_headers
+jwt-header: Bearer eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJrZXkiOiJ1c2VyLWtleSIsImV4cCI6MTg3OTMxODU0MX0.fNtFJnNmJgzbiYmGB0Yjvm-l6A6M4jRV1l4mnVFSYjs
+
+
+
+=== TEST 4: verify (in cookie) not hiding credentials
+--- request
+GET /echo
+--- more_headers
+Cookie: jwt-cookie=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJrZXkiOiJ1c2VyLWtleSIsImV4cCI6MTg3OTMxODU0MX0.fNtFJnNmJgzbiYmGB0Yjvm-l6A6M4jRV1l4mnVFSYjs
+--- response_headers
+Cookie: jwt-cookie=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJrZXkiOiJ1c2VyLWtleSIsImV4cCI6MTg3OTMxODU0MX0.fNtFJnNmJgzbiYmGB0Yjvm-l6A6M4jRV1l4mnVFSYjs
+
+
+
+=== TEST 5: enable jwt auth plugin using admin api without hiding credentials
+# the `proxy-rewrite` play role as upstream to check sensitive param
Review Comment:
so we don't need this comment?
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] pixeldin commented on a diff in pull request #8206: feat: support hide credentials for jwt-auth plugin
Posted by GitBox <gi...@apache.org>.
pixeldin commented on code in PR #8206:
URL: https://github.com/apache/apisix/pull/8206#discussion_r1009591974
##########
apisix/plugins/jwt-auth.lua:
##########
@@ -357,8 +361,31 @@ local function algorithm_handler(consumer, method_only)
end
end
+local function set_our_cookie(name, val)
+ core.response.add_header("Set-Cookie", name .. "=" .. val)
+end
+
function _M.rewrite(conf, ctx)
+ local from_header = true
+ local header_key = core.request.header(ctx, conf.header)
+
+ local from_query = true
+
+ if not header_key then
+ from_header = false
+ local uri_args = core.request.get_uri_args(ctx) or {}
+ header_key = uri_args[conf.query]
+ if not header_key then
+ from_query = false
+ local cookie = ctx.var["cookie_" .. conf.cookie]
+ if not cookie then
+ core.log.info("failed to fetch JWT token")
+ return 401, {message = "Missing JWT token in request"}
+ end
+ end
+ end
Review Comment:
Sorry, can you provide some description more specific? I mean this logic is similar, but jwt-auth and key-auth is different plugin.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] pixeldin commented on a diff in pull request #8206: feat: support hide credentials for jwt-auth plugin
Posted by GitBox <gi...@apache.org>.
pixeldin commented on code in PR #8206:
URL: https://github.com/apache/apisix/pull/8206#discussion_r1010470330
##########
apisix/plugins/jwt-auth.lua:
##########
@@ -357,8 +361,31 @@ local function algorithm_handler(consumer, method_only)
end
end
+local function set_our_cookie(name, val)
+ core.response.add_header("Set-Cookie", name .. "=" .. val)
Review Comment:
Be glad to hear your advice. The first time I refer to the [cas-auth.lua](https://github.com/apache/apisix/blob/master/apisix/plugins/cas-auth.lua#:~:text=local%20function%20set_our_cookie(name%2C%20val)) plugin(`set_our_cookie()`) to update the cookie as deleted, and then I refer to the operation of the `lua-resty-cookie` [library](https://github.com/cloudflare/lua-resty-cookie).
Finally I realized that it is actually calling the same handling about header's `Set-Cookie`. If you have a better operation suggestion, please let me know.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] pixeldin commented on a diff in pull request #8206: feat: support hide credentials for jwt-auth plugin
Posted by GitBox <gi...@apache.org>.
pixeldin commented on code in PR #8206:
URL: https://github.com/apache/apisix/pull/8206#discussion_r1014841780
##########
apisix/plugins/jwt-auth.lua:
##########
@@ -200,15 +209,28 @@ local function fetch_jwt_token(conf, ctx)
return token
end
- token = ctx.var["arg_" .. conf.query]
+ local uri_args = core.request.get_uri_args(ctx) or {}
+ token = uri_args[conf.query]
if token then
+ if conf.hide_credentials then
+ -- hide for query
+ uri_args[conf.query] = nil
+ core.request.set_uri_args(ctx, uri_args)
+ end
return token
end
local val = ctx.var["cookie_" .. conf.cookie]
if not val then
return nil, "JWT not found in cookie"
end
+
+ if conf.hide_credentials then
+ -- hide for cookie
+ local reset_val = conf.cookie .. "=deleted; Max-Age=0"
Review Comment:
Do you mean handling multiple header cookie fields like this?
Downstream:
`"headers":[ "Cookie": "jwt-cookie=some-credentials; foo=bar" ]`
Upstream:
`"headers":[ "Cookie": "foo=bar" ]`
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] tzssangglass commented on a diff in pull request #8206: feat: support hide credentials for jwt-auth plugin
Posted by GitBox <gi...@apache.org>.
tzssangglass commented on code in PR #8206:
URL: https://github.com/apache/apisix/pull/8206#discussion_r1019797603
##########
t/plugin/jwt-auth3.t:
##########
@@ -0,0 +1,309 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+use t::APISIX 'no_plan';
+
+repeat_each(1);
+no_long_string();
+no_root_location();
+no_shuffle();
+
+add_block_preprocessor(sub {
+ my ($block) = @_;
+
+ if ((!defined $block->error_log) && (!defined $block->no_error_log)) {
+ $block->set_value("no_error_log", "[error]");
+ }
+
+ if (!defined $block->request) {
+ $block->set_value("request", "GET /t");
+ }
+
+ if (!$block->response_body) {
+ $block->set_value("response_body", "passed\n");
+ }
+});
+
+run_tests;
+
+__DATA__
+
+=== TEST 1: add consumer with username and plugins
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/consumers',
+ ngx.HTTP_PUT,
+ [[{
+ "username": "jack",
+ "plugins": {
+ "jwt-auth": {
+ "key": "user-key",
+ "secret": "my-secret-key"
+ }
+ }
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+
+
+
+=== TEST 2: enable jwt auth plugin using admin api with custom parameter
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PUT,
+ [[{
+ "plugins": {
+ "jwt-auth": {
+ "header": "jwt-header",
+ "query": "jwt-query",
+ "cookie": "jwt-cookie",
+ "hide_credentials": false
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "uri": "/echo"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+
+
+
+=== TEST 3: verify (in header) not hiding credentials
+--- request
+GET /echo
+--- more_headers
+jwt-header: Bearer eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJrZXkiOiJ1c2VyLWtleSIsImV4cCI6MTg3OTMxODU0MX0.fNtFJnNmJgzbiYmGB0Yjvm-l6A6M4jRV1l4mnVFSYjs
+--- response_headers
+jwt-header: Bearer eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJrZXkiOiJ1c2VyLWtleSIsImV4cCI6MTg3OTMxODU0MX0.fNtFJnNmJgzbiYmGB0Yjvm-l6A6M4jRV1l4mnVFSYjs
+--- response_body eval
+qr/^$/
+
+
+
+=== TEST 4: verify (in cookie) not hiding credentials
+--- request
+GET /echo
+--- more_headers
+Cookie: jwt-cookie=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJrZXkiOiJ1c2VyLWtleSIsImV4cCI6MTg3OTMxODU0MX0.fNtFJnNmJgzbiYmGB0Yjvm-l6A6M4jRV1l4mnVFSYjs
+--- response_headers
+Cookie: jwt-cookie=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJrZXkiOiJ1c2VyLWtleSIsImV4cCI6MTg3OTMxODU0MX0.fNtFJnNmJgzbiYmGB0Yjvm-l6A6M4jRV1l4mnVFSYjs
+--- response_body eval
+qr/^$/
+
+
+
+=== TEST 5: enable jwt auth plugin using admin api without hiding credentials
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PUT,
+ [[{
+ "plugins": {
+ "jwt-auth": {
+ "header": "jwt-header",
+ "query": "jwt-query",
+ "cookie": "jwt-cookie",
+ "hide_credentials": false
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "uri": "/plugin_proxy_rewrite_args"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+
+
+
+=== TEST 6: verify (in query) without hiding credentials
+--- request
+GET /plugin_proxy_rewrite_args?foo=bar&hello=world&jwt-query=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJrZXkiOiJ1c2VyLWtleSIsImV4cCI6MTg3OTMxODU0MX0.fNtFJnNmJgzbiYmGB0Yjvm-l6A6M4jRV1l4mnVFSYjs
+--- response_body
+uri: /plugin_proxy_rewrite_args
+foo: bar
+hello: world
+jwt-query: eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJrZXkiOiJ1c2VyLWtleSIsImV4cCI6MTg3OTMxODU0MX0.fNtFJnNmJgzbiYmGB0Yjvm-l6A6M4jRV1l4mnVFSYjs
+
+
+
+=== TEST 7: enable jwt auth plugin using admin api with hiding credentials
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PUT,
+ [[{
+ "plugins": {
+ "jwt-auth": {
+ "header": "jwt-header",
+ "query": "jwt-query",
+ "cookie": "jwt-cookie",
+ "hide_credentials": true
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "uri": "/echo"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+
+
+
+=== TEST 8: verify (in header) with hiding credentials
+--- request
+GET /echo
+--- more_headers
+jwt-header: eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJrZXkiOiJ1c2VyLWtleSIsImV4cCI6MTg3OTMxODU0MX0.fNtFJnNmJgzbiYmGB0Yjvm-l6A6M4jRV1l4mnVFSYjs
+--- response_headers
+!jwt-header
+--- response_body eval
+qr/^$/
+
+
+
+=== TEST 9: enable jwt auth plugin using admin api with hiding credentials
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PUT,
+ [[{
+ "plugins": {
+ "jwt-auth": {
+ "header": "jwt-header",
+ "query": "jwt-query",
+ "cookie": "jwt-cookie",
+ "hide_credentials": true
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "uri": "/plugin_proxy_rewrite_args"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+
+
+
+=== TEST 10: verify (in query) with hiding credentials
+--- request
+GET /plugin_proxy_rewrite_args?foo=bar&hello=world&jwt-query=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJrZXkiOiJ1c2VyLWtleSIsImV4cCI6MTg3OTMxODU0MX0.fNtFJnNmJgzbiYmGB0Yjvm-l6A6M4jRV1l4mnVFSYjs
+--- response_body
+uri: /plugin_proxy_rewrite_args
+foo: bar
+hello: world
+
+
+
+=== TEST 11: verify (in cookie) with hiding credentials
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PUT,
+ [[{
+ "plugins": {
+ "jwt-auth": {
+ "header": "jwt-header",
+ "query": "jwt-query",
+ "cookie": "jwt-cookie",
+ "hide_credentials": true
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "httpbin.org:80": 1
+ },
+ "type": "roundrobin"
+ },
+ "uri": "/get"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+
+
+
+=== TEST 12: verify (in cookie) with hiding credentials
+--- request
+GET /get
+--- more_headers
+Cookie: hello=world; jwt-cookie=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJrZXkiOiJ1c2VyLWtleSIsImV4cCI6MTg3OTMxODU0MX0.fNtFJnNmJgzbiYmGB0Yjvm-l6A6M4jRV1l4mnVFSYjs; foo=bar
+--- response_body eval
+qr/^(?:(?!jwt-cookie).)*\z/s
Review Comment:
to be on the safe side, I want this test case to visually see that the cookie that upstream received is `hello=world; foo=bar`
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] pixeldin commented on a diff in pull request #8206: feat: support hide credentials for jwt-auth plugin
Posted by GitBox <gi...@apache.org>.
pixeldin commented on code in PR #8206:
URL: https://github.com/apache/apisix/pull/8206#discussion_r1009592889
##########
apisix/plugins/jwt-auth.lua:
##########
@@ -357,8 +361,31 @@ local function algorithm_handler(consumer, method_only)
end
end
+local function set_our_cookie(name, val)
+ core.response.add_header("Set-Cookie", name .. "=" .. val)
+end
+
function _M.rewrite(conf, ctx)
+ local from_header = true
+ local header_key = core.request.header(ctx, conf.header)
+
+ local from_query = true
+
+ if not header_key then
+ from_header = false
+ local uri_args = core.request.get_uri_args(ctx) or {}
+ header_key = uri_args[conf.query]
+ if not header_key then
+ from_query = false
+ local cookie = ctx.var["cookie_" .. conf.cookie]
+ if not cookie then
+ core.log.info("failed to fetch JWT token")
+ return 401, {message = "Missing JWT token in request"}
+ end
+ end
+ end
Review Comment:
Ok, I 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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a diff in pull request #8206: feat: support hide credentials for jwt-auth plugin
Posted by GitBox <gi...@apache.org>.
spacewander commented on code in PR #8206:
URL: https://github.com/apache/apisix/pull/8206#discussion_r1018693636
##########
apisix/plugins/jwt-auth.lua:
##########
@@ -188,10 +196,41 @@ function _M.check_schema(conf, schema_type)
return true
end
+local function remove_specified_cookie(src, key)
+ local cookie_key_pattern = "([a-zA-Z0-9-_]*)"
+ local cookie_val_pattern = "([a-zA-Z0-9-._]*)"
+ local t = new_tab(1, 0)
+
+ local it, err = ngx_re_gmatch(src, cookie_key_pattern .. "=" .. cookie_val_pattern, "jo")
+ if not it then
+ core.log.error("match origins failed: ", err)
+ return nil
Review Comment:
I think of it again. Look like we need to return the src when an error happens? Otherwise, the whole Cookie will be removed.
##########
t/plugin/jwt-auth3.t:
##########
@@ -0,0 +1,301 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+use t::APISIX 'no_plan';
+
+repeat_each(1);
+no_long_string();
+no_root_location();
+no_shuffle();
+
+add_block_preprocessor(sub {
+ my ($block) = @_;
+
+ if ((!defined $block->error_log) && (!defined $block->no_error_log)) {
+ $block->set_value("no_error_log", "[error]");
+ }
+
+ if (!defined $block->request) {
+ $block->set_value("request", "GET /t");
+ }
Review Comment:
Since we don't set `response_body` in some tests, we should provide the default value "passed" here.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander merged pull request #8206: feat: support hide credentials for jwt-auth plugin
Posted by GitBox <gi...@apache.org>.
spacewander merged PR #8206:
URL: https://github.com/apache/apisix/pull/8206
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a diff in pull request #8206: feat: support hide credentials for jwt-auth plugin
Posted by GitBox <gi...@apache.org>.
spacewander commented on code in PR #8206:
URL: https://github.com/apache/apisix/pull/8206#discussion_r1017467464
##########
t/plugin/jwt-auth3.t:
##########
@@ -0,0 +1,311 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+use t::APISIX 'no_plan';
+
+repeat_each(1);
+no_long_string();
+no_root_location();
+no_shuffle();
+
+add_block_preprocessor(sub {
+ my ($block) = @_;
+
+ if ((!defined $block->error_log) && (!defined $block->no_error_log)) {
+ $block->set_value("no_error_log", "[error]");
+ }
+
+ if (!defined $block->request) {
+ $block->set_value("request", "GET /t");
+ }
+});
+
+run_tests;
+
+__DATA__
+
+=== TEST 1: add consumer with username and plugins
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/consumers',
+ ngx.HTTP_PUT,
+ [[{
+ "username": "jack",
+ "plugins": {
+ "jwt-auth": {
+ "key": "user-key",
+ "secret": "my-secret-key"
+ }
+ }
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- response_body
+passed
+
+
+
+=== TEST 2: enable jwt auth plugin using admin api with custom parameter
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PUT,
+ [[{
+ "plugins": {
+ "jwt-auth": {
+ "header": "jwt-header",
+ "query": "jwt-query",
+ "cookie": "jwt-cookie",
+ "hide_credentials": false
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "uri": "/echo"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
Review Comment:
We can remove all `GET /t` and
```
--- no_error_log
[error]
```
in this file.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] tzssangglass commented on a diff in pull request #8206: feat: support hide credentials for jwt-auth plugin
Posted by GitBox <gi...@apache.org>.
tzssangglass commented on code in PR #8206:
URL: https://github.com/apache/apisix/pull/8206#discussion_r1009234921
##########
apisix/plugins/jwt-auth.lua:
##########
@@ -357,8 +361,31 @@ local function algorithm_handler(consumer, method_only)
end
end
+local function set_our_cookie(name, val)
+ core.response.add_header("Set-Cookie", name .. "=" .. val)
+end
+
function _M.rewrite(conf, ctx)
+ local from_header = true
+ local header_key = core.request.header(ctx, conf.header)
+
+ local from_query = true
+
+ if not header_key then
+ from_header = false
+ local uri_args = core.request.get_uri_args(ctx) or {}
+ header_key = uri_args[conf.query]
+ if not header_key then
+ from_query = false
+ local cookie = ctx.var["cookie_" .. conf.cookie]
+ if not cookie then
+ core.log.info("failed to fetch JWT token")
+ return 401, {message = "Missing JWT token in request"}
+ end
+ end
+ end
Review Comment:
we don't need to add this logic in this PR, we just do same things as: https://github.com/apache/apisix/pull/6670/files
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a diff in pull request #8206: feat: support hide credentials for jwt-auth plugin
Posted by GitBox <gi...@apache.org>.
spacewander commented on code in PR #8206:
URL: https://github.com/apache/apisix/pull/8206#discussion_r1011271355
##########
apisix/plugins/jwt-auth.lua:
##########
@@ -200,15 +210,31 @@ local function fetch_jwt_token(conf, ctx)
return token
end
- token = ctx.var["arg_" .. conf.query]
+ local uri_args = core.request.get_uri_args(ctx) or {}
+ token = uri_args[conf.query]
if token then
+ if conf.hide_credentials then
+ -- hide for query
+ uri_args[conf.query] = nil
+ core.request.set_uri_args(ctx, uri_args)
+ end
return token
end
local val = ctx.var["cookie_" .. conf.cookie]
if not val then
return nil, "JWT not found in cookie"
end
+
+ if conf.hide_credentials then
+ -- hide for cookie
+ ck:new():set({
Review Comment:
It will tell the client to forget the credential, not hide the credential from the upstream.
##########
t/plugin/jwt-auth3.t:
##########
@@ -0,0 +1,294 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+use t::APISIX 'no_plan';
+
+repeat_each(1);
+no_long_string();
+no_root_location();
+no_shuffle();
+
+add_block_preprocessor(sub {
+ my ($block) = @_;
+
+ if ((!defined $block->error_log) && (!defined $block->no_error_log)) {
+ $block->set_value("no_error_log", "[error]");
+ }
+
+ if (!defined $block->request) {
+ $block->set_value("request", "GET /t");
+ }
+});
+
+run_tests;
+
+__DATA__
+
+=== TEST 1: add consumer with username and plugins
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/consumers',
+ ngx.HTTP_PUT,
+ [[{
+ "username": "jack",
+ "plugins": {
+ "jwt-auth": {
+ "key": "user-key",
+ "secret": "my-secret-key"
+ }
+ }
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- response_body
+passed
+
+
+
+=== TEST 2: enable jwt auth plugin using admin api with custom parameter
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PUT,
+ [[{
+ "plugins": {
+ "jwt-auth": {
+ "header": "jwt-header",
+ "query": "jwt-query",
+ "cookie": "jwt-cookie",
+ "hide_credentials": false
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "uri": "/echo"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+
+
+
+=== TEST 3: verify (in header) with not hide credentials
Review Comment:
```suggestion
=== TEST 3: verify (in header) without hiding credentials
```
##########
t/plugin/jwt-auth3.t:
##########
@@ -0,0 +1,294 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+use t::APISIX 'no_plan';
+
+repeat_each(1);
+no_long_string();
+no_root_location();
+no_shuffle();
+
+add_block_preprocessor(sub {
+ my ($block) = @_;
+
+ if ((!defined $block->error_log) && (!defined $block->no_error_log)) {
+ $block->set_value("no_error_log", "[error]");
+ }
+
+ if (!defined $block->request) {
+ $block->set_value("request", "GET /t");
+ }
+});
+
+run_tests;
+
+__DATA__
+
+=== TEST 1: add consumer with username and plugins
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/consumers',
+ ngx.HTTP_PUT,
+ [[{
+ "username": "jack",
+ "plugins": {
+ "jwt-auth": {
+ "key": "user-key",
+ "secret": "my-secret-key"
+ }
+ }
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- response_body
+passed
+
+
+
+=== TEST 2: enable jwt auth plugin using admin api with custom parameter
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PUT,
+ [[{
+ "plugins": {
+ "jwt-auth": {
+ "header": "jwt-header",
+ "query": "jwt-query",
+ "cookie": "jwt-cookie",
+ "hide_credentials": false
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "uri": "/echo"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+
+
+
+=== TEST 3: verify (in header) with not hide credentials
Review Comment:
It seems we can remove all 'without hiding' tests, as this behavior is the default and has been tested previously.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] tzssangglass commented on a diff in pull request #8206: feat: support hide credentials for jwt-auth plugin
Posted by GitBox <gi...@apache.org>.
tzssangglass commented on code in PR #8206:
URL: https://github.com/apache/apisix/pull/8206#discussion_r1011305097
##########
apisix/plugins/jwt-auth.lua:
##########
@@ -16,6 +16,7 @@
--
local core = require("apisix.core")
local jwt = require("resty.jwt")
+local ck = require("resty.cookie")
Review Comment:
we can fetch cookie by `ctx.var.cookie_`, no need to `require("resty.cookie")`
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] pixeldin commented on a diff in pull request #8206: feat: support hide credentials for jwt-auth plugin
Posted by GitBox <gi...@apache.org>.
pixeldin commented on code in PR #8206:
URL: https://github.com/apache/apisix/pull/8206#discussion_r1009998859
##########
apisix/plugins/jwt-auth.lua:
##########
@@ -357,8 +361,31 @@ local function algorithm_handler(consumer, method_only)
end
end
+local function set_our_cookie(name, val)
+ core.response.add_header("Set-Cookie", name .. "=" .. val)
+end
+
function _M.rewrite(conf, ctx)
+ local from_header = true
+ local header_key = core.request.header(ctx, conf.header)
+
+ local from_query = true
+
+ if not header_key then
+ from_header = false
+ local uri_args = core.request.get_uri_args(ctx) or {}
+ header_key = uri_args[conf.query]
+ if not header_key then
+ from_query = false
+ local cookie = ctx.var["cookie_" .. conf.cookie]
+ if not cookie then
+ core.log.info("failed to fetch JWT token")
+ return 401, {message = "Missing JWT token in request"}
+ end
+ end
+ end
Review Comment:
Thank you, I will adjust 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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] tzssangglass commented on a diff in pull request #8206: feat: support hide credentials for jwt-auth plugin
Posted by GitBox <gi...@apache.org>.
tzssangglass commented on code in PR #8206:
URL: https://github.com/apache/apisix/pull/8206#discussion_r1017508338
##########
apisix/plugins/jwt-auth.lua:
##########
@@ -19,13 +19,17 @@ local jwt = require("resty.jwt")
local consumer_mod = require("apisix.consumer")
local resty_random = require("resty.random")
local vault = require("apisix.core.vault")
+local new_tab = require ("table.new")
local ngx_encode_base64 = ngx.encode_base64
local ngx_decode_base64 = ngx.decode_base64
local ipairs = ipairs
local ngx = ngx
local ngx_time = ngx.time
local sub_str = string.sub
+local table_insert = table.insert
+local table_concat = table.concat
+local str_gmatch = string.gmatch
Review Comment:
can we replace `string.gmatch` to `ngx.re.gmatch`?
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] pixeldin commented on a diff in pull request #8206: feat: support hide credentials for jwt-auth plugin
Posted by GitBox <gi...@apache.org>.
pixeldin commented on code in PR #8206:
URL: https://github.com/apache/apisix/pull/8206#discussion_r1008968203
##########
apisix/plugins/jwt-auth.lua:
##########
@@ -407,6 +434,27 @@ function _M.rewrite(conf, ctx)
return 401, {message = "failed to verify jwt"}
end
+ -- check for hiding `Authorization` request header if `hide_credentials` is `true`
+ if conf.hide_credentials then
+ -- hide sensitive field
+ if from_header then
+ -- hide for header
+ local temp_token = core.request.header(ctx, conf.header)
+ core.request.set_header(ctx, conf.header, nil)
+
+
Review Comment:
Thx for your advice, I will cover 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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] tzssangglass commented on a diff in pull request #8206: feat: support hide credentials for jwt-auth plugin
Posted by GitBox <gi...@apache.org>.
tzssangglass commented on code in PR #8206:
URL: https://github.com/apache/apisix/pull/8206#discussion_r1014945741
##########
t/plugin/jwt-auth3.t:
##########
@@ -0,0 +1,333 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+use t::APISIX 'no_plan';
+
+repeat_each(1);
+no_long_string();
+no_root_location();
+no_shuffle();
+
+add_block_preprocessor(sub {
+ my ($block) = @_;
+
+ if ((!defined $block->error_log) && (!defined $block->no_error_log)) {
+ $block->set_value("no_error_log", "[error]");
+ }
+
+ if (!defined $block->request) {
+ $block->set_value("request", "GET /t");
+ }
+});
+
+run_tests;
+
+__DATA__
+
+=== TEST 1: add consumer with username and plugins
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/consumers',
+ ngx.HTTP_PUT,
+ [[{
+ "username": "jack",
+ "plugins": {
+ "jwt-auth": {
+ "key": "user-key",
+ "secret": "my-secret-key"
+ }
+ }
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- response_body
+passed
+
+
+
+=== TEST 2: enable jwt auth plugin using admin api with custom parameter
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PUT,
+ [[{
+ "plugins": {
+ "jwt-auth": {
+ "header": "jwt-header",
+ "query": "jwt-query",
+ "cookie": "jwt-cookie",
+ "hide_credentials": false
+ }
+ },
+ "upstream": {
+ "nodes": {
+ "127.0.0.1:1980": 1
+ },
+ "type": "roundrobin"
+ },
+ "uri": "/echo"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+
+
+
+=== TEST 3: verify (in header) not hiding credentials
+--- request
+GET /echo
+--- more_headers
+jwt-header: Bearer eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJrZXkiOiJ1c2VyLWtleSIsImV4cCI6MTg3OTMxODU0MX0.fNtFJnNmJgzbiYmGB0Yjvm-l6A6M4jRV1l4mnVFSYjs
+--- response_headers
+jwt-header: Bearer eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJrZXkiOiJ1c2VyLWtleSIsImV4cCI6MTg3OTMxODU0MX0.fNtFJnNmJgzbiYmGB0Yjvm-l6A6M4jRV1l4mnVFSYjs
+
+
+
+=== TEST 4: verify (in cookie) not hiding credentials
+--- request
+GET /echo
+--- more_headers
+Cookie: jwt-cookie=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJrZXkiOiJ1c2VyLWtleSIsImV4cCI6MTg3OTMxODU0MX0.fNtFJnNmJgzbiYmGB0Yjvm-l6A6M4jRV1l4mnVFSYjs
+--- response_headers
+Cookie: jwt-cookie=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJrZXkiOiJ1c2VyLWtleSIsImV4cCI6MTg3OTMxODU0MX0.fNtFJnNmJgzbiYmGB0Yjvm-l6A6M4jRV1l4mnVFSYjs
+
+
+
+=== TEST 5: enable jwt auth plugin using admin api without hiding credentials
+# the `proxy-rewrite` play role as upstream to check sensitive param
Review Comment:
Didn't understand here, uri was changed to `/plugin_proxy_rewrite_args` by proxy-rewrite. why not request `/plugin_proxy_rewrite_args` directly?
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] tzssangglass commented on a diff in pull request #8206: feat: support hide credentials for jwt-auth plugin
Posted by GitBox <gi...@apache.org>.
tzssangglass commented on code in PR #8206:
URL: https://github.com/apache/apisix/pull/8206#discussion_r1009996598
##########
apisix/plugins/jwt-auth.lua:
##########
@@ -357,8 +361,31 @@ local function algorithm_handler(consumer, method_only)
end
end
+local function set_our_cookie(name, val)
+ core.response.add_header("Set-Cookie", name .. "=" .. val)
+end
+
function _M.rewrite(conf, ctx)
+ local from_header = true
+ local header_key = core.request.header(ctx, conf.header)
+
+ local from_query = true
+
+ if not header_key then
+ from_header = false
+ local uri_args = core.request.get_uri_args(ctx) or {}
+ header_key = uri_args[conf.query]
+ if not header_key then
+ from_query = false
+ local cookie = ctx.var["cookie_" .. conf.cookie]
+ if not cookie then
+ core.log.info("failed to fetch JWT token")
+ return 401, {message = "Missing JWT token in request"}
+ end
+ end
+ end
Review Comment:
The premise of `hide_credentials` is to find out which part of the request JWT exists, header or cookie, we already have the function `fetch_jwt_token` to find JWT.
We can execute `hide_credentials` once the JWT is found in `fetch_jwt_token`.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] tokers commented on pull request #8206: feat: support hide credentials for jwt-auth plugin
Posted by GitBox <gi...@apache.org>.
tokers commented on PR #8206:
URL: https://github.com/apache/apisix/pull/8206#issuecomment-1308090888
Please fix the code lint to make the CI pass.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a diff in pull request #8206: feat: support hide credentials for jwt-auth plugin
Posted by GitBox <gi...@apache.org>.
spacewander commented on code in PR #8206:
URL: https://github.com/apache/apisix/pull/8206#discussion_r1019872874
##########
t/plugin/jwt-auth3.t:
##########
@@ -0,0 +1,309 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+use t::APISIX 'no_plan';
+
+repeat_each(1);
+no_long_string();
+no_root_location();
+no_shuffle();
+
+add_block_preprocessor(sub {
+ my ($block) = @_;
+
+ if ((!defined $block->error_log) && (!defined $block->no_error_log)) {
+ $block->set_value("no_error_log", "[error]");
+ }
+
+ if (!defined $block->request) {
+ $block->set_value("request", "GET /t");
+ }
+
+ if (!$block->response_body) {
Review Comment:
What about moving this block inside
```
if (!defined $block->request) {
$block->set_value("request", "GET /t");
}
```
so we can get rid of:
```
--- response_body eval
qr/^$/
```
##########
apisix/plugins/jwt-auth.lua:
##########
@@ -19,13 +19,17 @@ local jwt = require("resty.jwt")
local consumer_mod = require("apisix.consumer")
local resty_random = require("resty.random")
local vault = require("apisix.core.vault")
+local new_tab = require ("table.new")
local ngx_encode_base64 = ngx.encode_base64
local ngx_decode_base64 = ngx.decode_base64
local ipairs = ipairs
local ngx = ngx
local ngx_time = ngx.time
local sub_str = string.sub
+local table_insert = table.insert
+local table_concat = table.concat
+local ngx_re_gmatch = ngx.re.gmatch
Review Comment:
```suggestion
local ngx_re_gmatch = ngx.re.gmatch
```
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] pixeldin commented on a diff in pull request #8206: feat: support hide credentials for jwt-auth plugin
Posted by GitBox <gi...@apache.org>.
pixeldin commented on code in PR #8206:
URL: https://github.com/apache/apisix/pull/8206#discussion_r1010472303
##########
apisix/plugins/jwt-auth.lua:
##########
@@ -407,6 +434,25 @@ function _M.rewrite(conf, ctx)
return 401, {message = "failed to verify jwt"}
end
+ -- check for hiding `Authorization` request header if `hide_credentials` is `true`
+ if conf.hide_credentials then
+ -- hide sensitive field
+ if from_header then
+ -- hide for header
+ core.request.set_header(ctx, conf.header, nil)
+
+ elseif from_query then
+ -- hide for query
+ local args = core.request.get_uri_args(ctx)
+ args[conf.query] = nil
+ core.request.set_uri_args(ctx, args)
+
+ else
+ -- hide for cookie
+ set_our_cookie(conf.cookie, "deleted; Max-Age=0")
Review Comment:
I use the [lua-resty-cookie](https://github.com/cloudflare/lua-resty-cookie) lib to update cookie this time.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org