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.

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

unset plugin configuration value -anonymous- should act like default value #2539

Closed
mtmail opened this issue May 17, 2017 · 2 comments
Closed

Comments

@mtmail
Copy link

mtmail commented May 17, 2017

We upgraded kong v0.9 to v0.10.1. In postgres we saw

select config from plugins where name='key-auth';
{"hide_credentials":false,"key_names":["apikey"]}
select consumer_id from plugins where name='key-auth';
(one result, seems to be empty string)

When installing v0.10 directly (without upgrade) the config value is

{"hide_credentials":false,"key_names":["apikey"],"anonymous":""}

Kong started returning error HTTP 500 for valid keys after the upgrade. The kong error log file contained

2017/05/17 13:25:53 [error] 2764#0: *1514 lua entry thread aborted: runtime error: /usr/local/share/lua/5.1/kong/tools/database_cache.lua:223: attempt to concatenate local 'id' (a nil value)
stack traceback:
coroutine 0:
	/usr/local/share/lua/5.1/kong/tools/database_cache.lua: in function 'consumer_key'
	/usr/local/share/lua/5.1/kong/plugins/key-auth/handler.lua:135: in function 'access'
	/usr/local/share/lua/5.1/kong.lua:256: in function 'access'
	access_by_lua(nginx-kong.conf:80):2: in function <access_by_lua(nginx-kong.conf:80):1>, client: 178.203.197.233, server: kong, request: "GET /APINAME?key=VALIDKEY HTTP/1.1", host: "KONG_SERVER:PORT"

The line number (135) is misleading. In handler.lua on the server the error happens around cache.get_or_set https://github.com/Mashape/kong/blob/bb88a8071c8a30caed72b620f60835cc238c13f9/kong/plugins/key-auth/handler.lua#L149

Previous to that line there's a condition

 if conf.anonymous ~= "" and conf.anonymous ~= nil then

I'd argue it should be or instead of and so that an unset configuration value has the same meaning as the default (empty string according to https://getkong.org/plugins/key-authentication/)

After we changed the config value via a SQL statement the HTTP 500 errors stopped.

Checking the master branch other plugins seem to have the same logic

grep -r 'conf.anonymous ~= nil' ./
.//kong/plugins/basic-auth/access.lua:    if conf.anonymous ~= "" and conf.anonymous ~= nil then
.//kong/plugins/hmac-auth/access.lua:    if conf.anonymous ~= "" and conf.anonymous ~= nil then
.//kong/plugins/jwt/handler.lua:    if conf.anonymous ~= "" and conf.anonymous ~= nil then
.//kong/plugins/key-auth/handler.lua:    if conf.anonymous ~= "" and conf.anonymous ~= nil then
.//kong/plugins/ldap-auth/access.lua:    if conf.anonymous ~= "" and conf.anonymous ~= nil then
.//kong/plugins/oauth2/access.lua:    if conf.anonymous ~= "" and conf.anonymous ~= nil then
@p0pr0ck5
Copy link
Contributor

@mtmail thank you for the detailed report! This was set to be fixed via a migration in 0.11, and we have a bandaid that will be released in 0.10.3 (see #2508). If you're using 0.10.1 the logic you've noted is not applicable to your running install, as this PR is not yet released (unless you've patched the PR in directly).

@p0pr0ck5
Copy link
Contributor

Closing, since this is fixed as noted in both upcoming releases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants