refactor AuthManager + MFAManager#600
Merged
Mastermind-U merged 31 commits intorefactor_services_unificationfrom Jul 16, 2025
Merged
refactor AuthManager + MFAManager#600Mastermind-U merged 31 commits intorefactor_services_unificationfrom
Mastermind-U merged 31 commits intorefactor_services_unificationfrom
Conversation
…for authentication and MFA handling
Mastermind-U
changed the title
added 3 commits
June 30, 2025 14:39
…in authentication - Simplified the dependency injection for AuthManager and MFAManager by using decorators for cleaner code. - Updated exception handling in AuthManager to raise more specific errors, improving clarity in authentication failures. - Introduced new exception classes for better error management, including AlreadyConfiguredError and MFARequiredError, to enhance user feedback during authentication processes.
- Changed private attributes to protected in AuthManager and MFAManager for improved accessibility and consistency. - Updated all references to these attributes throughout the classes to reflect the new naming convention, enhancing code clarity and maintainability.
…tials - Updated the MFAProvider to return an instance of MultifactorAPI with an empty string when credentials are not provided, ensuring consistent behavior. - Modified the MultifactorAPI constructor to initialize the is_initialized attribute only if both key and secret are present, enhancing error handling and preventing potential issues during instantiation.
added 2 commits
June 30, 2025 16:10
- Updated the is_initialized attribute to use a boolean check for both key and secret, ensuring proper initialization behavior when credentials are provided or absent.
- Updated the is_initialized attribute to use a more accurate boolean check, ensuring it only evaluates to True when both key and secret are present, thereby improving the initialization process.
Mastermind-U
suggested changes
Jul 14, 2025
added 7 commits
July 14, 2025 15:38
- Updated references throughout the codebase to replace AuthManager with IdentityManager, enhancing consistency in naming conventions. - Adjusted dependency injection and method signatures in various modules to reflect the new class name, ensuring seamless integration across the application.
- Adjusted the docstring for the `UnauthorizedError` exception to enhance readability by aligning the exception type on a new line, improving clarity for developers referencing the documentation.
…tegration - Added IdentityManagerFastAPIAdapter to facilitate the integration of IdentityManager with FastAPI, enhancing the authentication process. - Updated the HTTPProvider and authentication routes to utilize the new adapter, ensuring consistent handling of authentication logic. - Refactored dependency injection to provide the adapter in place of the original IdentityManager, streamlining the authentication workflow across the application.
- Introduced MFAManagerFastAPIAdapter to facilitate integration of MFAManager with FastAPI, improving the handling of multi-factor authentication. - Updated dependency injection in the MainProvider and test configurations to utilize the new adapter, ensuring consistent MFA management across the application. - Refactored authentication routes to leverage the adapter, streamlining the MFA setup and callback processes.
- Refactored authentication routes to simplify error handling by removing try-except blocks, allowing exceptions to propagate naturally. - Updated method signatures in various classes for improved clarity and consistency. - Moved MFA-related exceptions to a dedicated module for better organization and maintainability. - Removed the now-unnecessary exceptions module, consolidating exception management within the application.
…erFastAPIAdapter - Changed the parameter type of perform_first_setup to SetupRequest for improved type safety and clarity in the setup process.
…ng clarity - Changed the parameter type of `ip` in `perform_first_setup` to `IPv4Address | IPv6Address` for better type safety. - Revised the docstring for `NotFoundError` to enhance clarity by correcting the phrasing regarding resource absence.
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the authentication and MFA logic by extracting business logic into IdentityManager/MFAManager classes with FastAPI adapters, updating dependency injection providers, and simplifying API routers to delegate to these adapters.
- Introduce
IdentityManagerFastAPIAdapterandMFAManagerFastAPIAdapterin DI (tests andapp/ioc.py) - Add
IdentityManagerandMFAManagerimplementations inapp/api/utils/ - Refactor routers to use adapters instead of inline auth/MFA code
Reviewed Changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/conftest.py | Imported managers/adapters and added DI providers |
| interface | Updated submodule commit |
| app/ldap_protocol/multifactor.py | Added is_initialized flag to MultifactorAPI |
| app/ioc.py | Updated DI scope annotations and added manager providers |
| app/api/utils/mfa_manager.py | New MFAManager domain logic and FastAPI adapter |
| app/api/utils/auth_manager.py | New IdentityManager domain logic and FastAPI adapter |
| app/api/utils/exceptions/*.py | Added MFA- and auth-related exceptions |
| app/api/auth/router_mfa.py | Refactored MFA router to use MFAManagerFastAPIAdapter |
| app/api/auth/router.py | Refactored auth router to use IdentityManagerFastAPIAdapter |
Comments suppressed due to low confidence (3)
app/ioc.py:240
- This
get_entity_type_daomethod is declared without the@providedecorator, so it won't be registered as a DI provider and may silently override or shadow any previous provider. Add the appropriate@provideannotation or remove the duplicate method.
def get_entity_type_dao(
app/ioc.py:335
- The return type annotation and docstring for
get_http_mfawere updated to always returnMultifactorAPI, but the docstring still mentionsNoneas a possible return. Update the docstring to remove the| Nonereference.
) -> MultifactorAPI:
app/api/utils/exceptions/init.py:22
- [nitpick] The
ForbiddenErrorclass defined inexceptions/mfa.pyis not imported or exported in this__init__.py, so consumers may be unable to import it fromapi.utils.exceptions. Consider addingForbiddenErrorto the import list and__all__.
NotFoundError,
Mastermind-U
suggested changes
Jul 15, 2025
… injection - Replaced references to AuthManager with IdentityManager across the codebase for consistency. - Updated dependency injection in HTTPProvider and test configurations to utilize the new IdentityManager and IdentityManagerFastAPIAdapter, streamlining authentication processes. - Removed the now-obsolete auth_manager module, consolidating identity management functionality.
added 2 commits
July 15, 2025 13:41
- Replaced instances of IdentityManagerFastAPIAdapter and MFAManagerFastAPIAdapter with their new counterparts, IdentityFastAPIAdapter and MFAFastAPIAdapter, across the codebase for improved consistency and clarity. - Removed obsolete utility modules related to identity and MFA management, streamlining the codebase. - Updated dependency injection in various components to utilize the new adapters, enhancing the integration of authentication and MFA processes within the application.
- Changed the scope of the `get_krb_class` method from APP to SESSION for better session management. - Consolidated MFA manager and adapter instantiation in both the MainProvider and test configurations, enhancing clarity and consistency in dependency injection.
…on and MFA adapters - Enhanced exception handling in IdentityFastAPIAdapter and MFAFastAPIAdapter by improving formatting and clarity of HTTPException details. - Refactored method signatures to improve readability by separating parameters onto new lines in both adapters. - Updated MFA manager's user authentication method for better parameter clarity.
…arity - Added whitespace for better readability in the MFAManager class methods. - Improved the structure of exception handling to enhance clarity and maintainability in the MFA process.
added 6 commits
July 15, 2025 16:36
…on handling - Updated MFAFastAPIAdapter to create and set session keys upon successful MFA callback, improving user session management. - Refactored MFAManager methods to return user objects instead of redirect responses, enhancing clarity and consistency in MFA processing. - Improved method signatures for better readability and parameter handling in both classes.
…ters - Updated IdentityFastAPIAdapter and MFAFastAPIAdapter to inherit from SessionKeyMixin, centralizing session key creation and management. - Refactored login and MFA callback methods to utilize the new session key handling, improving session management consistency. - Removed the obsolete SessionKeyMixin class, streamlining the codebase and enhancing clarity in session handling across authentication processes.
…y and MFA adapters - Updated IdentityFastAPIAdapter and MFAFastAPIAdapter to inherit from ResponseCookieMixin, enhancing session cookie management. - Refactored login and MFA callback methods to utilize the new cookie handling, improving session management consistency. - Removed the obsolete SessionKeyMixin class, streamlining the codebase and enhancing clarity in session handling across authentication processes.
- Modified the return type of the initiate two-factor protocol method in MFAManager to include the user agent as a string, enhancing the method's clarity and usability in MFA processes.
…for clarity - Updated the MFAFastAPIAdapter to improve parameter formatting in the callback method for better readability. - Changed the parameter type of `ip` in MFAManager to `IPv4Address | IPv6Address` and clarified the return type in the docstring, enhancing type safety and documentation clarity.
- Updated the MFAManager to return an empty string along with the user object in specific error scenarios, improving clarity in the response structure during MFA processing. - Enhanced exception handling to maintain consistency in the return values across different error cases.
Mastermind-U
suggested changes
Jul 16, 2025
added 2 commits
July 16, 2025 11:19
- Updated IdentityFastAPIAdapter and MFAFastAPIAdapter to remove unnecessary user and session parameters from the set_session_cookie method, simplifying the method calls. - Refactored the login and MFA callback methods to directly utilize the session key returned from the IdentityManager and MFAManager, enhancing clarity and consistency in session handling. - Adjusted the return types in the IdentityManager and MFAManager to focus on session keys, improving the overall structure and readability of the code.
- Included the "SLF001" rule in the select section of pyproject.toml to enforce checks for access to private/protected attributes via self, enhancing code quality and adherence to best practices.
Mastermind-U
suggested changes
Jul 16, 2025
added 5 commits
July 16, 2025 11:34
- Changed method names from private to public for `_handle_extensible_match` and `_handle_substring` in ASN1Row, improving code readability and consistency in method access. - Updated references to these methods throughout the class to reflect the new names. - Added a noqa comment for the order_by clause check in PaginationResult to comply with flake8-self rules.
- Modified the set_session_cookie method in ResponseCookieMixin to accept key_ttl instead of storage, enhancing flexibility in session cookie expiration. - Updated IdentityFastAPIAdapter and MFAFastAPIAdapter to pass key_ttl when setting session cookies, improving consistency in session management. - Introduced key_ttl property in IdentityManager and MFAManager to provide session key time-to-live, streamlining access to this value.
…key parameters - Modified the docstring of the set_session_cookie method in ResponseCookieMixin to include key_ttl and key parameters, enhancing clarity on session cookie management. - This change improves documentation consistency with recent updates to session handling across authentication adapters.
… MFAManager - Changed the `storage` attribute to `_storage` in both IdentityManager and MFAManager for consistency and to adhere to naming conventions for private attributes. - Updated method calls to use the new `_storage` attribute, ensuring clarity and uniformity in session key management across the classes.
- Removed the key_ttl property and directly assigned key_ttl from _storage in both IdentityManager and MFAManager, simplifying the code and improving clarity in session key management. - This change enhances consistency in how session key time-to-live is accessed across the classes.
Mastermind-U
approved these changes
Jul 16, 2025
This file contains hidden or 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
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.
No description provided.