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 2020/08/25 11:32:54 UTC
[GitHub] [apisix] ShiningRush opened a new pull request #2120: [key-auth]fix: skip consumer when config has no key
ShiningRush opened a new pull request #2120:
URL: https://github.com/apache/apisix/pull/2120
resolve #2045 .
----------------------------------------------------------------
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] ShiningRush commented on a change in pull request #2120: [key-auth]fix: skip consumer when config has no key
Posted by GitBox <gi...@apache.org>.
ShiningRush commented on a change in pull request #2120:
URL: https://github.com/apache/apisix/pull/2120#discussion_r477075791
##########
File path: t/plugin/key-auth.t
##########
@@ -215,3 +215,56 @@ apikey: auth-13
["passed\n", "hello world\n"]
--- no_error_log
[error]
+
+
+
+=== TEST 9: add consumer with empty key
+--- 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": "error",
+ "plugins": {
+ "key-auth": {
+ }
Review comment:
I think we should not simply throw errors in the consumer, but have a mechanism for the plugin to define its own verification behavior
----------------------------------------------------------------
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] moonming commented on a change in pull request #2120: [key-auth]fix: skip consumer when config has no key
Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #2120:
URL: https://github.com/apache/apisix/pull/2120#discussion_r489208626
##########
File path: t/plugin/key-auth.t
##########
@@ -215,3 +215,56 @@ apikey: auth-13
["passed\n", "hello world\n"]
--- no_error_log
[error]
+
+
+
+=== TEST 9: add consumer with empty key
+--- 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": "error",
+ "plugins": {
+ "key-auth": {
+ }
Review comment:
any update?
----------------------------------------------------------------
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] membphis commented on a change in pull request #2120: [key-auth]fix: skip consumer when config has no key
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #2120:
URL: https://github.com/apache/apisix/pull/2120#discussion_r477153261
##########
File path: t/plugin/key-auth.t
##########
@@ -215,3 +215,56 @@ apikey: auth-13
["passed\n", "hello world\n"]
--- no_error_log
[error]
+
+
+
+=== TEST 9: add consumer with empty key
+--- 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": "error",
+ "plugins": {
+ "key-auth": {
+ }
Review comment:
Add a new parameter, which is the resource type to which the plugin is bound.
Determine the content of the authentication plug-in according to the type of resource created.
Here is an example of `key-auth`:
```lua
function _M.check_schema(conf, parrent_type)
local ok, err = core.schema.check(schema, conf)
if not ok then
return false, err
end
if parrent_type == "consumer" then
if core.table.nkeys(conf) == 0 then
return false, "missing properties"
end
end
return true
end
```
----------------------------------------------------------------
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] membphis merged pull request #2120: [key-auth]fix: skip consumer when config has no key
Posted by GitBox <gi...@apache.org>.
membphis merged pull request #2120:
URL: https://github.com/apache/apisix/pull/2120
----------------------------------------------------------------
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] ShiningRush commented on a change in pull request #2120: [key-auth]fix: skip consumer when config has no key
Posted by GitBox <gi...@apache.org>.
ShiningRush commented on a change in pull request #2120:
URL: https://github.com/apache/apisix/pull/2120#discussion_r478213326
##########
File path: t/plugin/key-auth.t
##########
@@ -215,3 +215,56 @@ apikey: auth-13
["passed\n", "hello world\n"]
--- no_error_log
[error]
+
+
+
+=== TEST 9: add consumer with empty key
+--- 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": "error",
+ "plugins": {
+ "key-auth": {
+ }
Review comment:
I think it is good, but maybe we should have a discuss in mail list like the changes about core?
----------------------------------------------------------------
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] membphis commented on a change in pull request #2120: [key-auth]fix: skip consumer when config has no key
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #2120:
URL: https://github.com/apache/apisix/pull/2120#discussion_r476926154
##########
File path: t/plugin/key-auth.t
##########
@@ -215,3 +215,56 @@ apikey: auth-13
["passed\n", "hello world\n"]
--- no_error_log
[error]
+
+
+
+=== TEST 9: add consumer with empty key
+--- 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": "error",
+ "plugins": {
+ "key-auth": {
+ }
Review comment:
> add consumer with empty key
should throw an error message if there is no argument when create a new consumer.
----------------------------------------------------------------
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] membphis commented on a change in pull request #2120: [key-auth]fix: skip consumer when config has no key
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #2120:
URL: https://github.com/apache/apisix/pull/2120#discussion_r478384402
##########
File path: t/plugin/key-auth.t
##########
@@ -215,3 +215,56 @@ apikey: auth-13
["passed\n", "hello world\n"]
--- no_error_log
[error]
+
+
+
+=== TEST 9: add consumer with empty key
+--- 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": "error",
+ "plugins": {
+ "key-auth": {
+ }
Review comment:
nice, you can create a topic about this ^_^
----------------------------------------------------------------
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