Fix type validation errors in User model and get_auth_factors API #114
Conversation
- Add field validator to convert integer manager_user_id to string - Update get_auth_factors return type from single object to List - Add comprehensive tests for both fixes Co-authored-by: Subterrane <5290140+Subterrane@users.noreply.github.com>
Co-authored-by: Subterrane <5290140+Subterrane@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes type validation errors in the OneLogin Python SDK that occur when the API returns data in formats not matching the model's strict type expectations.
Changes:
- Added a field validator to the User model to handle
manager_user_idvalues returned as integers by converting them to strings - Updated the
get_auth_factorsmethod to return a list ofGetAuthFactors200Responseobjects instead of a single object, matching the actual API response format - Added comprehensive test coverage for both fixes
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
onelogin/models/user.py |
Added mode='before' validator to coerce integer manager_user_id to string |
onelogin/api/multi_factor_authentication_api.py |
Changed return type from single object to list for get_auth_factors method |
test/test_user_manager_user_id_fix.py |
Added test case for integer manager_user_id conversion |
test/test_get_auth_factors_fix.py |
Added comprehensive test coverage for list-based auth factors response |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
onelogin/models/user.py
Outdated
| @field_validator('manager_user_id', mode='before') | ||
| @classmethod | ||
| def manager_user_id_to_string(cls, value): | ||
| """Converts manager_user_id to string if it's an integer""" |
There was a problem hiding this comment.
The docstring should specify that this validator also handles None values by returning them unchanged, as this is important behavior documented in the code.
| """Converts manager_user_id to string if it's an integer""" | |
| """Converts manager_user_id to string if it's an integer and returns None values unchanged""" |
There was a problem hiding this comment.
Updated the docstring in commit 13d87c2 to document that None values are returned unchanged.
| def test_get_auth_factors_list_response(self): | ||
| """ | ||
| Test that GetAuthFactors200Response can be deserialized from a list. | ||
| This test validates the fix where API returns a list of factors | ||
| but model expected a single object. | ||
| """ |
There was a problem hiding this comment.
This test validates model deserialization but doesn't test the actual get_auth_factors API method. Consider adding an integration test that verifies the method returns List[GetAuthFactors200Response] when called with a mocked API response.
There was a problem hiding this comment.
Added an integration test in commit 13d87c2 that verifies the get_auth_factors method returns List[GetAuthFactors200Response] with a mocked API response. The test also validates that the response_types_map is correctly set to 'List[GetAuthFactors200Response]'.
|
@copilot Address PR comments, please. |
- Improve docstring for manager_user_id validator to mention None handling - Add integration test for get_auth_factors API method with mocked response Co-authored-by: Subterrane <5290140+Subterrane@users.noreply.github.com>
Fix type validation errors in OneLogin Python SDK
Issues Fixed:
manager_user_idtype validation in User model to accept integers from APIget_auth_factorsreturn type to handle list response from APIChanges Made:
manager_user_idto string (mode='before')get_auth_factorsAPI method return type fromGetAuthFactors200ResponsetoList[GetAuthFactors200Response]Security Summary:
No security vulnerabilities were introduced or detected in this change. CodeQL analysis completed successfully with 0 alerts.
Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.