-
Notifications
You must be signed in to change notification settings - Fork 34
Refactor authentication handling in OverkizClient and add new credential classes #1867
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
Conversation
c82af05 to
1849f13
Compare
…ial classes - Introduced UsernamePasswordCredentials and LocalTokenCredentials for better credential management. - Updated OverkizClient to utilize the new credential classes and refactored login logic. - Added authentication strategies for various servers, including Somfy and Rexel. - Created new modules for auth strategies and credentials to improve code organization. - Enhanced README with updated usage examples for the new authentication methods.
…Config across the codebase
…fig and update related references
…y and update related server handling
…_auth_strategy function
98d9d6a to
38823d2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR introduces a comprehensive refactoring of authentication handling in the pyoverkiz library by extracting authentication logic into a dedicated module with credential classes and authentication strategies. The refactoring improves code organization and extensibility while introducing breaking changes to the OverkizClient API.
- Introduced new authentication module with credential classes (UsernamePasswordCredentials, LocalTokenCredentials, RexelOAuthCodeCredentials, TokenCredentials) and strategy-based authentication
- Refactored OverkizClient constructor to require ServerConfig and Credentials objects as keyword-only arguments
- Added support for Rexel OAuth2 authentication flow with consent validation
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| pyoverkiz/auth/base.py | Defines AuthContext and AuthStrategy protocol for token management |
| pyoverkiz/auth/credentials.py | Introduces credential classes for different authentication methods |
| pyoverkiz/auth/strategies.py | Implements authentication strategies for various servers (Somfy, Cozytouch, Nexity, Rexel, Local) |
| pyoverkiz/auth/factory.py | Factory function to build appropriate auth strategy based on server and credentials |
| pyoverkiz/auth/init.py | Module exports for the authentication package |
| pyoverkiz/client.py | Refactored to use new authentication module; removed inline auth logic |
| pyoverkiz/models.py | Renamed OverkizServer to ServerConfig with additional server and type properties |
| pyoverkiz/const.py | Updated server configurations to use ServerConfig; added Rexel OAuth constants |
| pyoverkiz/utils.py | Renamed generate_local_server to create_local_server_config; added create_server_config helper |
| tests/test_client.py | Updated to use new credential classes and Server enum |
| tests/test_utils.py | Updated test names to match renamed utility functions |
| README.md | Updated examples to demonstrate new authentication approach |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@copilot can you address the feedback in this PR? |
…ling, and add auth tests (#1875) - [x] Fix datetime.now() calls to use UTC timezone (pyoverkiz/auth/strategies.py lines 203, 406; pyoverkiz/auth/base.py line 24) - [x] Handle 204 No Content responses properly in strategies.py line 123 - [x] Add error handling for OAuth token exchange responses in strategies.py line 396 - [x] Remove duplicate enum conversion logic in utils.py create_server_config function - [x] Fix SSL_CONTEXT_LOCAL_API mutation issue by creating a copy per client instance - [x] Add test coverage for authentication module (strategies.py, factory.py, credentials.py) - [x] Revert SSL context creation to avoid blocking I/O at runtime - [x] Add TODO fix comment for mypy type ignore workaround <!-- START COPILOT CODING AGENT TIPS --> --- ✨ Let Copilot coding agent [set things up for you](https://github.com/iMicknl/python-overkiz-api/issues/new?title=✨+Set+up+Copilot+instructions&body=Configure%20instructions%20for%20this%20repository%20as%20documented%20in%20%5BBest%20practices%20for%20Copilot%20coding%20agent%20in%20your%20repository%5D%28https://gh.io/copilot-coding-agent-tips%29%2E%0A%0A%3COnboard%20this%20repo%3E&assignees=copilot) — coding agent works faster and does higher quality work when set up for your repo. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: iMicknl <1424596+iMicknl@users.noreply.github.com>
|
@tetienne I will start merging some of these, but I won't release all v2 before a final feedback round 👍🏻. And feel free to still comment on old PR's. |
Breaking
OverkizServerclass is renamed toServerConfigand has additionalserverandtype(cloud/local) propertygenerate_local_serveris renamed tocreate_local_server_configclient.api_typeis removed and now available viaServerConfig(e.g.client.server_config.type)OverkizClientconstructor now requires passing aServerConfigviaserverOverkizClientconstructur now requires passing anCredentialsclass viacredentials, e.g.UsernamePasswordCredentials(USERNAME, PASSWORD)for most server.Features
OverkizClient(server=Server.SOMFY_EUROPE, ...).