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

PR Feature/ new /verify-request for nginx #140

Merged
merged 3 commits into from
Jun 6, 2024

Conversation

alexandru-m-g
Copy link
Member

@alexandru-m-g alexandru-m-g commented Jun 6, 2024

This is based on @teodorescuserban's idea. The problem that we're trying to solve is related to the nginx cache. When nginx was serving a request from the cache, the app_identifier was no longer checked because the request was not reaching HAPI.

Before responding to a request from cache, nginx will now first check with HAPI that the app_identifier provided in the request is correct.
This happens by nginx sending a request to /api/util/verify-request . The endpoint doesn't really do anything, but if it gets to return HTTP code 200 with an empty body then nginx knows it's allowed to serve the request from the cache.

The request will have the same HTTP headers as the original request received by nginx + it will contain an additional header X-Original-URI that will contain the URL of the original request.

With the added complexity, I had to refactor the app identifier middleware to be a bit more readable

Copy link
Collaborator

@IanHopkinson IanHopkinson left a comment

Choose a reason for hiding this comment

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

Thanks for the explanation in the PR opening - that was really useful for this one.

Technically I don't have much to contribute on this, my only question was whether the new verify-request endpoint would appear in the docs UI - my reading is that it doesn't because of the include_in_schema=False entries

@danmihaila danmihaila merged commit 505af1a into dev Jun 6, 2024
3 checks passed
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.

3 participants