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 logic refactor #1586

Merged
merged 5 commits into from
Feb 24, 2025
Merged

Authentication logic refactor #1586

merged 5 commits into from
Feb 24, 2025

Conversation

Tansito
Copy link
Member

@Tansito Tansito commented Feb 19, 2025

Summary

This PR will refactor the entire authentication flow to prepare the project to introduce additional services like IBM Cloud. This is a 100% refactor, there is no change in the logic or the output. So the main purpose is the simplification and improvement of the codebase.

Details and comments

  • The current logic was divided in two additional classes:
    • AuthenticationUseCase: it will manage the main flow for the authentication process. User authentication, validation and repository access.
    • QuantumPlatformService: it will manage all the access to these 3rd party end-points.
  • QuantumUserProxy was removed in favor of refactoring some logic between QuantumPlatformService and UserRepository classes what it has more sense from a responsibility point of view.
  • Environment variables were simplified also to reduce the number of required configurations along the authentication flow.

@Tansito Tansito requested a review from paaragon February 19, 2025 20:58
self.authorization_token = authorization_token
self.access_token = None

def _get_network(self, access_token: str):
Copy link
Member Author

Choose a reason for hiding this comment

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

This is something very interesting @paaragon . If I use __get_network here I cannot mock the method because as a private method the test cannot access to the method.

I didn't remember this so probably we should stop using that nomenclature, what do you think?

"""
The main objective of this class is to manage the access to the model
"""

def get_or_create_by_id(self, user_id: str) -> type[AbstractUser]:
Copy link
Member Author

Choose a reason for hiding this comment

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

I also found a way to type that we are returning a user with type[AbstractUser]. This way the linter doesn't complain 😂

@Tansito Tansito marked this pull request as ready for review February 19, 2025 22:08
Copy link
Collaborator

@paaragon paaragon left a comment

Choose a reason for hiding this comment

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

Hi @Tansito . Very good job! this organisation looks so clean and performant.

Just one little change request 😉

@Tansito Tansito requested a review from paaragon February 21, 2025 15:45
@Tansito Tansito merged commit dee2888 into main Feb 24, 2025
8 checks passed
@Tansito Tansito deleted the authencation-logic-refactor branch February 24, 2025 14:55
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