Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: multi-auth raise 500 error when use default conf #11145

Merged
merged 4 commits into from
Apr 24, 2024
Merged

fix: multi-auth raise 500 error when use default conf #11145

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
bbef788
fix
yuweizzz Apr 11, 2024
f6c696b
perf
yuweizzz Apr 11, 2024
38206e2
t
yuweizzz Apr 13, 2024
d66930b
update t
yuweizzz Apr 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions apisix/plugins/multi-auth.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,10 @@ function _M.check_schema(conf)
if auth.type ~= 'auth' then
return false, auth_plugin_name .. " plugin is not supported"
end
local ok, err = auth.check_schema(auth_plugin_conf, auth.schema)
if not ok then
return false, "plugin " .. auth_plugin_name .. " check schema failed: " .. err
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you also add a test case which shows this error would be raised if any auth_plugin has wrong configuration.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

perfect, really appreciate it <3. I will re-review once the CI has completed running.

end
end
end
end
Expand Down
256 changes: 250 additions & 6 deletions t/plugin/multi-auth.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,251 @@ apikey: auth-two



=== TEST 8: enable multi auth plugin using admin api, without any auth_plugins configuration
=== TEST 8: enable multi auth plugin with invalid plugin conf in first auth_plugin
--- 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": {
"multi-auth": {
"auth_plugins": [
{
"basic-auth": {
"hide_credentials": "false"
}
},
{
"key-auth": {}
},
{
"jwt-auth": {}
}
]
}
},
"upstream": {
"nodes": {
"127.0.0.1:1980": 1
},
"type": "roundrobin"
},
"uri": "/hello"
}]]
)

if code >= 300 then
ngx.status = code
end
ngx.print(body)
}
}
--- request
GET /t
--- error_code: 400
--- response_body
{"error_msg":"failed to check the configuration of plugin multi-auth err: plugin basic-auth check schema failed: property \"hide_credentials\" validation failed: wrong type: expected boolean, got string"}



=== TEST 9: enable multi auth plugin with invalid plugin conf in second auth_plugins
--- 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": {
"multi-auth": {
"auth_plugins": [
{
"key-auth": {}
},
{
"basic-auth": "blah"
},
{
"jwt-auth": {}
}
]
}
},
"upstream": {
"nodes": {
"127.0.0.1:1980": 1
},
"type": "roundrobin"
},
"uri": "/hello"
}]]
)

if code >= 300 then
ngx.status = code
end
ngx.print(body)
}
}
--- request
GET /t
--- error_code: 400
--- response_body
{"error_msg":"failed to check the configuration of plugin multi-auth err: plugin basic-auth check schema failed: wrong type: expected object, got string"}



=== TEST 10: enable multi auth plugin with invalid plugin conf in third auth_plugins
--- 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": {
"multi-auth": {
"auth_plugins": [
{
"key-auth": {}
},
{
"basic-auth": {}
},
{
"jwt-auth": {
"header": 123
}
}
]
}
},
"upstream": {
"nodes": {
"127.0.0.1:1980": 1
},
"type": "roundrobin"
},
"uri": "/hello"
}]]
)

if code >= 300 then
ngx.status = code
end
ngx.print(body)
}
}
--- request
GET /t
--- error_code: 400
--- response_body
{"error_msg":"failed to check the configuration of plugin multi-auth err: plugin jwt-auth check schema failed: property \"header\" validation failed: wrong type: expected string, got number"}



=== TEST 11: enable multi auth plugin with default plugin conf
--- 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": {
"multi-auth": {
"auth_plugins": [
{
"basic-auth": {}
},
{
"key-auth": {}
},
{
"jwt-auth": {}
}
]
}
},
"upstream": {
"nodes": {
"127.0.0.1:1980": 1
},
"type": "roundrobin"
},
"uri": "/hello"
}]]
)

if code >= 300 then
ngx.status = code
end
ngx.say(body)
}
}
--- request
GET /t
--- response_body
passed



=== TEST 12: verify, missing authorization
--- request
GET /hello
--- error_code: 401
--- response_body
{"message":"Authorization Failed"}



=== TEST 13: verify basic-auth
--- request
GET /hello
--- more_headers
Authorization: Basic Zm9vOmJhcg==
--- response_body
hello world
--- error_log
find consumer foo



=== TEST 14: verify key-auth
--- request
GET /hello
--- more_headers
apikey: auth-one
--- response_body
hello world



=== TEST 15: verify, invalid basic credentials
--- request
GET /hello
--- more_headers
Authorization: Basic YmFyOmJhcgo=
--- error_code: 401
--- response_body
{"message":"Authorization Failed"}



=== TEST 16: verify, invalid api key
--- request
GET /hello
--- more_headers
apikey: auth-two
--- error_code: 401
--- response_body
{"message":"Authorization Failed"}



=== TEST 17: enable multi auth plugin using admin api, without any auth_plugins configuration
--- config
location /t {
content_by_lua_block {
Expand Down Expand Up @@ -200,7 +444,7 @@ qr/\{"error_msg":"failed to check the configuration of plugin multi-auth err: pr



=== TEST 9: enable multi auth plugin using admin api, with auth_plugins configuration but with one authorization plugin
=== TEST 18: enable multi auth plugin using admin api, with auth_plugins configuration but with one authorization plugin
--- config
location /t {
content_by_lua_block {
Expand Down Expand Up @@ -241,7 +485,7 @@ qr/\{"error_msg":"failed to check the configuration of plugin multi-auth err: pr



=== TEST 10: create public API route (jwt-auth sign)
=== TEST 19: create public API route (jwt-auth sign)
--- config
location /t {
content_by_lua_block {
Expand Down Expand Up @@ -269,7 +513,7 @@ passed



=== TEST 11: add consumer with username and jwt-auth plugins
=== TEST 20: add consumer with username and jwt-auth plugins
--- config
location /t {
content_by_lua_block {
Expand Down Expand Up @@ -300,7 +544,7 @@ passed



=== TEST 12: sign / verify jwt-auth
=== TEST 21: sign / verify jwt-auth
--- config
location /t {
content_by_lua_block {
Expand Down Expand Up @@ -330,7 +574,7 @@ hello world



=== TEST 13: verify multi-auth with plugin config will cause the conf_version change
=== TEST 22: verify multi-auth with plugin config will cause the conf_version change
--- config
location /t {
content_by_lua_block {
Expand Down
Loading