-
-
Notifications
You must be signed in to change notification settings - Fork 7k
Modify ObtainAuthToken to use the User model's USERNAME_FIELD and password for authentication instead of assuming username and password. #9674
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
base: main
Are you sure you want to change the base?
Conversation
…password' instead of 'username' and 'password' for both the built-in and custom User models
|
If any changes are required, please let me know. |
|
I'm not sure this falls under our current maintenance policy:
One could argue that this improves compatibility with custom Django user models, but on the other hand It's also simple to customise in user-land right now, and this is explained in our docs. If we were to ever accept it, we would need some tests to cover the behaviour with a customised user model... |
|
Thank you for reviewing my pull request and for your feedback. I understand that Django REST Framework is considered feature-complete and that new features are usually only accepted if they align with Django’s ongoing development. However, I see this change as more of a compatibility improvement rather than a new feature. While @browniebroke, Are you saying that it won't be possible to accept or merge this change? |
auvipy
left a comment
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.
I think this might break existing projects or functionalities
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
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 modifies the ObtainAuthToken view and AuthTokenSerializer to support custom Django user models that use a different USERNAME_FIELD (e.g., 'email' instead of 'username'). The changes enable dynamic authentication field detection based on the user model's USERNAME_FIELD attribute.
- Dynamically retrieves the
USERNAME_FIELDfrom the active user model instead of hardcoding 'username' - Updates the serializer to create appropriate fields (EmailField or CharField) based on the USERNAME_FIELD type
- Modifies the API schema generation to reflect the correct field name in documentation
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 10 comments.
| File | Description |
|---|---|
| rest_framework/authtoken/serializers.py | Refactored to dynamically create authentication fields based on USERNAME_FIELD and pass credentials to authenticate() |
| rest_framework/authtoken/views.py | Updated schema definition to use USERNAME_FIELD for API documentation |
| docs/api-guide/authentication.md | Added documentation note about custom user model support |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ) | ||
| def __init__(self, instance=None, data=None, **kwargs): | ||
| super().__init__(instance, data=data, **kwargs) | ||
| self.identifier_fiend_name = USER_MODEL.USERNAME_FIELD |
Copilot
AI
Dec 14, 2025
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.
Typo in variable name: 'identifier_fiend_name' should be 'identifier_field_name'. This typo appears throughout the class and will cause the code to break.
| if USER_MODEL.get_email_field_name() == self.identifier_fiend_name: | ||
| self.fields[self.identifier_fiend_name] = serializers.EmailField( |
Copilot
AI
Dec 14, 2025
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.
Typo in variable name: 'identifier_fiend_name' should be 'identifier_field_name'. This typo appears throughout the class and will cause the code to break.
| write_only=True | ||
| ) | ||
| else: | ||
| self.fields[self.identifier_fiend_name] = serializers.CharField( |
Copilot
AI
Dec 14, 2025
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.
Typo in variable name: 'identifier_fiend_name' should be 'identifier_field_name'. This typo appears throughout the class and will cause the code to break.
| username=username, password=password) | ||
| if identifier_value and password: | ||
| credentials = { | ||
| self.identifier_fiend_name: identifier_value, |
Copilot
AI
Dec 14, 2025
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.
Typo in variable name: 'identifier_fiend_name' should be 'identifier_field_name'. This typo appears throughout the class and will cause the code to break.
|
|
||
| from rest_framework import serializers | ||
|
|
||
| USER_MODEL = get_user_model() |
Copilot
AI
Dec 14, 2025
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.
The USER_MODEL is evaluated at module import time, which means if the user model changes dynamically or if this module is imported before Django apps are ready, it could reference the wrong model. Consider moving this inside the init method where it's actually used, similar to how it's done in the views.py file.
| def __init__(self, instance=None, data=None, **kwargs): | ||
| super().__init__(instance, data=data, **kwargs) | ||
| self.identifier_fiend_name = USER_MODEL.USERNAME_FIELD | ||
| if USER_MODEL.get_email_field_name() == self.identifier_fiend_name: | ||
| self.fields[self.identifier_fiend_name] = serializers.EmailField( | ||
| label=_(self.identifier_fiend_name.title()), | ||
| write_only=True | ||
| ) | ||
| else: | ||
| self.fields[self.identifier_fiend_name] = serializers.CharField( | ||
| label=_(self.identifier_fiend_name.title()), | ||
| write_only=True | ||
| ) | ||
| self.fields["password"] = serializers.CharField( | ||
| label=_("Password"), | ||
| style={'input_type': 'password'}, | ||
| trim_whitespace=False, | ||
| write_only=True | ||
| ) |
Copilot
AI
Dec 14, 2025
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.
The new behavior for supporting custom user models with different USERNAME_FIELD values (e.g., 'email') lacks test coverage. Consider adding tests that verify the serializer correctly handles custom user models where USERNAME_FIELD is set to a field other than 'username', such as 'email'.
| USER_MODEL = get_user_model() | ||
| identifier_field_name = USER_MODEL.USERNAME_FIELD |
Copilot
AI
Dec 14, 2025
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.
The USER_MODEL and identifier_field_name are evaluated at class definition time inside the conditional block. This means they are computed once when the module is imported. If the user model is swapped or changed dynamically, this will not be reflected. Consider moving this logic to a method or property that evaluates at runtime.
| schema = ManualSchema( | ||
| fields=[ | ||
| coreapi.Field( | ||
| name="username", | ||
| name=identifier_field_name, | ||
| required=True, | ||
| location='form', | ||
| schema=coreschema.String( | ||
| title="Username", | ||
| description="Valid username for authentication", | ||
| title=identifier_field_name.title(), | ||
| description=f"Valid {identifier_field_name} for authentication", | ||
| ), |
Copilot
AI
Dec 14, 2025
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.
The new dynamic schema generation based on USERNAME_FIELD lacks test coverage. Consider adding tests that verify the schema correctly reflects the USERNAME_FIELD from custom user models (e.g., when USERNAME_FIELD is 'email', the schema should show 'email' instead of 'username').
|
|
||
| def validate(self, attrs): | ||
| username = attrs.get('username') | ||
| identifier_value = attrs.get(self.identifier_fiend_name) |
Copilot
AI
Dec 14, 2025
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.
Typo in variable name: 'identifier_fiend_name' should be 'identifier_field_name'. This typo appears throughout the class and will cause the code to break.
| raise serializers.ValidationError(msg, code='authorization') | ||
| else: | ||
| msg = _('Must include "username" and "password".') | ||
| msg = _(f'Must include "{self.identifier_fiend_name}" and "password".') |
Copilot
AI
Dec 14, 2025
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.
Typo in variable name: 'identifier_fiend_name' should be 'identifier_field_name'. This typo appears throughout the class and will cause the code to break.
Summary
Django’s default user model uses
usernameandpasswordfor authentication. However, when a custom user model is defined withUSERNAME_FIELD = 'email', Django correctly uses email and password for authentication.The issue was that Django REST Framework's
obtain_auth_tokenendpoint (used for TokenAuthentication) still expectedusernameandpassword, even when a custom user model usedemailinstead ofusername.Fix
This update modifies the
ObtainAuthTokenview to dynamically useUSERNAME_FIELDinstead of assumingusername. Now, authentication works consistently, whether using the built-in User model or a custom one, with no inconsistencies.Changes Made:
ObtainAuthTokennow retrieves theUSERNAME_FIELDfrom the user model.USERNAME_FIELDandpassword, ensuring compatibility with both built-in and custom user models.Impact
USERNAME_FIELD = 'email') can now log in usingemailandpasswordinstead ofusernameandpassword.default Usermodel or acustom Usermodel whereUSERNAME_FIELD = 'username'can continue logging in usingusernameandpasswordas expected.