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

Authentication Middleware Refactor #145

Closed
AntonioMrtz opened this issue Jun 11, 2024 · 1 comment · Fixed by #161
Closed

Authentication Middleware Refactor #145

AntonioMrtz opened this issue Jun 11, 2024 · 1 comment · Fixed by #161
Labels
backend Backend related issues good first issue Good for newcomers help-wanted Help is wanted python Pull requests that update Python code refactor Code changes or improvements that do not change code behaviour

Comments

@AntonioMrtz
Copy link
Owner

Description

This issue aims to refactor the existing Authentication Middleware located in our backend API. In the process we will want to rename the middleware to be more significative and improve exception handling to unexpected errors.

Context

Current authentication Middleware is named CheckJwtAuthMiddleware. It serve its purpouse in the early stages of auth in the app but time has come from it to have a more significative name. See CheckJwtAuthMiddleware.

Furthermore, we want to improve the unexpected exceptions that we can have in our app ( although they shouldnt exists ). Our current implementation of the dispatch method does the following:

  1. Intercepts any incoming HTTP Requests into the API.
  2. Decides wheter or not they should be blocked or bypassed depending on endpoint security and JWT Token provided.
  3. call_next method redirects the HTTP Request into our endpoints.
  4. If an unhandled exception is raised during call_next the try except block will be trigger ( line 85 ) and a response with 401 Unauthorized will be sent into the client.

imagen

imagen

The objective is to make exception handling more precise. Its not correct to return 401 code if an unexpected error non related to authentication happens. As you can see theres code duplication and the code execution flow can be improved.

How to do it

  • Rename CheckJwtAuthMiddleware to JwtAuthMiddleware
  • Improve code execution flow. Delete duplicate code and improve legibility and clearness. Update modified method docs and logging messages if needed.
# Expected code execution flow

1. Check if the request has to be bypassed ( already done ). If has to be bypassed use `call_next`.
2. If not bypassed extract JWT token from the request and validate it.
3. If non valid return raise `JWTValidationException`.
4. If valid continue execution flow using `call_next`.
5. Handle exceptions correctly inside dispatch block. Differentiate Auth Exceptions from unhandled ones from `call_next` methods. Use 401 or 500 depending on the exception and log the process.

Testing

  • All tests keep passing.
  • Mock health endpoint, so it returns an exception and check that were processing it correctly and returning 500 HTTP error.
@AntonioMrtz AntonioMrtz added good first issue Good for newcomers backend Backend related issues refactor Code changes or improvements that do not change code behaviour help-wanted Help is wanted python Pull requests that update Python code labels Jun 11, 2024
@AntonioMrtz AntonioMrtz pinned this issue Jun 11, 2024
@AntonioMrtz
Copy link
Owner Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend Backend related issues good first issue Good for newcomers help-wanted Help is wanted python Pull requests that update Python code refactor Code changes or improvements that do not change code behaviour
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant