Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7k
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 ourterms of service andprivacy 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
partho-debnath commentedMar 30, 2025
If any changes are required, please let me know. |
browniebroke commentedMar 31, 2025
I'm not sure this falls under ourcurrent 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... |
partho-debnath commentedMar 31, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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 theObtainAuthToken view andAuthTokenSerializer to support custom Django user models that use a differentUSERNAME_FIELD (e.g., 'email' instead of 'username'). The changes enable dynamic authentication field detection based on the user model'sUSERNAME_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 |
CopilotAIDec 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.
| ifUSER_MODEL.get_email_field_name()==self.identifier_fiend_name: | ||
| self.fields[self.identifier_fiend_name]=serializers.EmailField( |
CopilotAIDec 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( |
CopilotAIDec 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) | ||
| ifidentifier_valueandpassword: | ||
| credentials= { | ||
| self.identifier_fiend_name:identifier_value, |
CopilotAIDec 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.
| fromrest_frameworkimportserializers | ||
| USER_MODEL=get_user_model() |
CopilotAIDec 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 theinit 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 | ||
| ifUSER_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 | ||
| ) |
CopilotAIDec 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 |
CopilotAIDec 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="Validusername for authentication", | ||
| title=identifier_field_name.title(), | ||
| description=f"Valid{identifier_field_name} for authentication", | ||
| ), |
CopilotAIDec 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').
| defvalidate(self,attrs): | ||
| username=attrs.get('username') | ||
| identifier_value=attrs.get(self.identifier_fiend_name) |
CopilotAIDec 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.
| raiseserializers.ValidationError(msg,code='authorization') | ||
| else: | ||
| msg=_('Must include "username" and "password".') | ||
| msg=_(f'Must include "{self.identifier_fiend_name}" and "password".') |
CopilotAIDec 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 thatDjango 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.