Refactor authentication middleware to include project_id in request tracing #12
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
1. Allow passing --project-id to any subcommand instead of only admin commands. 2. A stricter secret api key matching using regex. Only matching with the prefix was too aggressive matching stuff like "_task_something". 3. Add IP addresses to request tracing
…racing Change the main authentication middleware to be a top level middleware that injects the authentication status into the request extensions. The per route middleware can then just read the AuthenticationStatus and take decisions based on it. This change allows us to capture the project id in the request tracing which wasn't possible before because it get invoked just before the request handlers (due to how middleware ordering works). The main change in behavior here is that the middleware will be invoked regardless of whether the route requires authentication or not. This is ok, unless the customer passes malformed auth headers in which case the request would currently fail with `BadRequest` while it would have succeeded before. Note, that we currently don't have any unauthenticated endpoints, so no "real" change in behavior here.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Refactor authentication middleware to include project_id in request tracing
Change the main authentication middleware to be a top level middleware that
injects the authentication status into the request extensions. The per route
middleware can then just read the AuthenticationStatus and take decisions based
on it. This change allows us to capture the project id in the request tracing
which wasn't possible before because it get invoked just before the request handlers
(due to how middleware ordering works).
The main change in behavior here is that the middleware will be invoked regardless of
whether the route requires authentication or not. This is ok, unless the customer passes
malformed auth headers in which case the request would currently fail with
BadRequestwhile it would have succeeded before. Note, that we currently don't have any unauthenticated
endpoints, so no "real" change in behavior here.
Stack created with Sapling. Best reviewed with ReviewStack.