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

[proxy] fix accessing undefined variable #316

Merged
merged 3 commits into from
Mar 20, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 3 additions & 0 deletions .env
Original file line number Diff line number Diff line change
Expand Up @@ -28,3 +28,6 @@ REDIS_HOST=redis
# How much to open the Management API. Allowed values are: disabled, status, debug.
# OpenShift template has status as the default value to use it for healt checks.
APICAST_MANAGEMENT_API=debug

# Log level. One of the following: debug, info, notice, warn, error, crit, alert, or emerg.
APICAST_LOG_LEVEL=debug
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
### Fixed
- `http_ng` client supports auth passsed in the url, and default client options if the request options are missing for methods with body (POST, PUT, etc.) [PR #310](https://github.com/3scale/apicast/pull/310)
- Fixed lazy configuration loader to recover from failures [PR #313](https://github.com/3scale/apicast/pull/313)
- Fixed undefined variable `p` in post\_action [PR #316](https://github.com/3scale/apicast/pull/316)

### Removed

Expand Down
22 changes: 15 additions & 7 deletions apicast/src/apicast.lua
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ end
function _M:rewrite()
ngx.on_abort(_M.cleanup)

ngx.var.original_request_id = ngx.var.request_id

local host = ngx.var.host
-- load configuration if not configured
-- that is useful when lua_code_cache is off
Expand All @@ -65,18 +67,24 @@ end

function _M.post_action()
local request_id = ngx.var.original_request_id
local p = post_action_proxy[request_id]
post_action_proxy[request_id] = nil
p:post_action()
local p = ngx.ctx.proxy or post_action_proxy[request_id]

if p then
p:post_action()
else
ngx.log(ngx.INFO, 'could not find proxy for request id: ', request_id)
end
end

function _M.access()
local p = ngx.ctx.proxy
local fun = p:call() -- proxy:access() or oauth handler
local request_id = ngx.var.request_id
post_action_proxy[request_id] = p
ngx.var.original_request_id = request_id
return fun()

local ok, err = fun()

post_action_proxy[ngx.var.original_request_id] = p

return ok, err
end

_M.body_filter = noop
Expand Down
2 changes: 1 addition & 1 deletion apicast/src/proxy.lua
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ local function find_service_strict(self, host)
if found then break end
end

return found or ngx.log(ngx.ERR, 'service not found for host ', host)
return found or ngx.log(ngx.WARN, 'service not found for host ', host)
end

local function find_service_cascade(self, host)
Expand Down
14 changes: 13 additions & 1 deletion t/003-apicast.t
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ GET /?user_key=value
no mapping rules!
--- error_code: 412
--- error_log
skipping after action, no cached key
could not find proxy for request

=== TEST 3: authentication credentials invalid
The message is configurable and default status is 403.
Expand Down Expand Up @@ -517,3 +517,15 @@ all ok
$::dns->("localhost.example.com", "127.0.0.1")
--- no_error_log
[error]

=== TEST 15: invalid service
The message is configurable and default status is 403.
--- http_config
lua_package_path "$TEST_NGINX_LUA_PATH";
--- config
include $TEST_NGINX_APICAST_CONFIG;
--- request
GET /?user_key=value
--- error_code: 404
--- no_error_log
[error]
59 changes: 57 additions & 2 deletions t/005-apicast-oauth.t
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,23 @@ __DATA__
}
--- config
include $TEST_NGINX_APICAST_CONFIG;
set $backend_endpoint 'http://127.0.0.1:$TEST_NGINX_SERVER_PORT/backend';

location /backend/transactions/oauth_authorize.xml {
content_by_lua_block {
ngx.log(ngx.WARN, 'called oauth_authorize.xml')
ngx.exit(403)
}
}
--- request
GET /authorize
--- error_code: 302
--- response_headers
Location: http://example.com/redirect?error=invalid_client

--- error_log
called oauth_authorize.xml
--- no_error_log
[error]

=== TEST 2: calling /authorize works (Authorization Code)
[Section 1.3.1 of RFC 6749](https://tools.ietf.org/html/rfc6749#section-1.3.1)
Expand Down Expand Up @@ -135,11 +146,23 @@ Location: http://example.com/redirect\?scope=whatever&response_type=token&error=
}
--- config
include $TEST_NGINX_APICAST_CONFIG;
set $backend_endpoint 'http://127.0.0.1:$TEST_NGINX_SERVER_PORT/backend';

location /backend/transactions/oauth_authorize.xml {
content_by_lua_block {
ngx.log(ngx.WARN, 'called oauth_authorize.xml')
ngx.exit(403)
}
}
--- request
POST /oauth/token
--- response_body chomp
{"error":"invalid_client"}
--- error_code: 401
--- no_error_log
[error]
--- error_log
called oauth_authorize.xml

=== TEST 5: calling /oauth/token returns correct error message on invalid parameters
--- http_config
Expand All @@ -153,11 +176,23 @@ POST /oauth/token
}
--- config
include $TEST_NGINX_APICAST_CONFIG;
set $backend_endpoint 'http://127.0.0.1:$TEST_NGINX_SERVER_PORT/backend';

location /backend/transactions/oauth_authorize.xml {
content_by_lua_block {
ngx.log(ngx.WARN, 'called oauth_authorize.xml')
ngx.exit(403)
}
}
--- request
POST /oauth/token?grant_type=authorization_code&client_id=client_id&redirect_uri=redirect_uri&client_secret=client_secret&code=code
--- response_body chomp
{"error":"invalid_client"}
--- error_code: 401
--- no_error_log
[error]
--- error_log
called oauth_authorize.xml

=== TEST 6: calling /callback without params returns correct erro message
--- http_config
Expand All @@ -176,6 +211,8 @@ GET /callback
--- response_body chomp
{"error":"missing redirect_uri"}
--- error_code: 400
--- no_error_log
[error]

=== TEST 7: calling /callback redirects to correct error when state is missing
--- http_config
Expand All @@ -196,6 +233,8 @@ include $TEST_NGINX_APICAST_CONFIG;
"Location: http://127.0.0.1:$ENV{TEST_NGINX_SERVER_PORT}/redirect_uri#error=invalid_request&error_description=missing_state"
--- response_body_like chomp
^<html>
--- no_error_log
[error]

=== TEST 8: calling /callback redirects to correct error when state is missing
--- main_config
Expand All @@ -217,6 +256,8 @@ include $TEST_NGINX_APICAST_CONFIG;
--- error_code: 302
--- response_headers eval
"Location: http://127.0.0.1:$ENV{TEST_NGINX_SERVER_PORT}/redirect_uri#error=invalid_request&error_description=invalid_or_expired_state&state=foo"
--- no_error_log
[error]

=== TEST 9: calling /callback works
Not part of the RFC. This is the Gateway API to create access tokens and redirect back to the Client.
Expand Down Expand Up @@ -252,6 +293,8 @@ Not part of the RFC. This is the Gateway API to create access tokens and redirec
--- request
GET /fake-authorize
--- error_code: 302
--- no_error_log
[error]
--- response_body_like chomp
^<html>
--- response_headers_like
Expand Down Expand Up @@ -323,7 +366,8 @@ GET /t
--- response_body_like
{"token_type":"bearer","expires_in":604800,"access_token":"\w+"}
--- error_code: 200

--- no_error_log
[error]

=== TEST 11: calling with correct access_token in query proxies to the api upstream
--- http_config
Expand Down Expand Up @@ -367,6 +411,8 @@ GET /?access_token=foobar
--- error_code: 200
--- response_body
yay, upstream
--- no_error_log
[error]

=== TEST 12: calling /authorize with state returns same value back on redirect_uri
--- main_config
Expand Down Expand Up @@ -402,6 +448,8 @@ GET /fake-authorize?client_id=id&redirect_uri=http://example.com/redirect&respon
^<html>
--- response_headers_like
Location: http://example.com/redirect\?code=\w+&state=12345
--- no_error_log
[error]

=== TEST 13: calling with correct access_token in Authorization header proxies to the api upstream
--- http_config
Expand Down Expand Up @@ -446,6 +494,8 @@ Authorization: Bearer foobar
--- error_code: 200
--- response_body
yay, upstream
--- no_error_log
[error]

=== TEST 14: calling with access_token in query when credentials location is 'headers' fails with 'auth missing'
--- http_config
Expand Down Expand Up @@ -477,6 +527,8 @@ GET /?access_token=foobar
--- error_code: 401
--- response_body chomp
credentials missing!
--- no_error_log
[error]

=== TEST 15: calling with access_token in header when the type is not 'Bearer' (case-sensitive) fails with 'auth missing'
--- http_config
Expand Down Expand Up @@ -510,3 +562,6 @@ Authorization: bearer foobar
--- error_code: 401
--- response_body chomp
credentials missing!
--- no_error_log
[error]