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

Ensure that the apicast post_action phase runs only if access runs #985

Merged
merged 7 commits into from
Jan 23, 2019

Conversation

davidor
Copy link
Contributor

@davidor davidor commented Jan 23, 2019

This PR fixes a bug that was triggered when combining the APIcast policy with some policies that can deny the request, such as the IP check one.

APIcast can call 3scale backend's authrep in the access and the post_action phases depending on whether the request is cached or not. The problem is that some policies were calling ngx.exit(4xx) to deny a request, and that prevents some phases from running, but it does not skip the post_action one.

In practice, this means that with a policy chain like ip_check + apicast, even when the IP check policy denied the request, post_action in apicast would run, and report to backend in certain cases.

The bug does not affect all the policies that deny a request. In particular, policies that use the errors module (https://github.com/3scale/apicast/blob/64c2b2655051d66a90096c500c33be1f40ec4a75/gateway/src/apicast/errors.lua) instead of calling ngx.exit() directly were not affected. The reason is that the errors module sets ngx.var.cached_key to nil, and that prevents post_action from reporting to the 3scale backend.

So we can add more things to it.
This commit adds a flag to run post_action() only when access() was
executed.

Other policies can call ngx.exit(4xx) on access() or rewrite. If they're
placed before APIcast in the chain, the request will be denied and this
access() phase will no be run. However, ngx.exit(4xx) skips some phases,
but not post_action. Post_action would run if we did not set this flag,
and we want to avoid that. Otherwise, post_action could call authrep()
even when another policy denied the request.
This is no longer needed. `context[self].run_post_action` is used now to
achieve the same result.
@davidor davidor requested a review from a team as a code owner January 23, 2019 14:17
Copy link
Contributor

@mikz mikz left a comment

Choose a reason for hiding this comment

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

Excellent work!

@davidor davidor merged commit e9c8f0a into master Jan 23, 2019
@davidor davidor deleted the apicast-post-action-only-when-access branch January 23, 2019 14:42
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

Successfully merging this pull request may close these issues.

2 participants