fix: prevent crashing using itemgetter#21
Conversation
There was a problem hiding this comment.
Code Review
This pull request refactors the authentication service to handle user information extraction more safely by replacing itemgetter with dictionary access and providing default values for optional fields. It also includes a minor formatting cleanup in a migration file. The review feedback correctly identifies that the email field is still accessed directly, which could lead to a KeyError, and suggests implementing a more graceful error handling approach using a ValidationError.
| ) | ||
| given_name = user_info_dict.get("given_name", "") | ||
| family_name = user_info_dict.get("family_name", "") | ||
| email = user_info_dict["email"] |
There was a problem hiding this comment.
While the removal of itemgetter prevents crashes when given_name or family_name are missing, accessing user_info_dict["email"] directly will still raise a KeyError if the "email" key is absent. Since the email is essential for user identification and the primary goal of this PR is to prevent crashes, it is better to handle its absence gracefully by raising a ValidationError.
email = user_info_dict.get("email")
if not email:
raise ValidationError({"error": "Email not provided by the authentication provider."})
What?
Prevent crashing using itemgetter
Why?
From the request
How?
Testing?
Anything Else?