#29449 closed Bug (fixed)
UserCreationForm and UserChangeForm don't work if username isn't a CharField
| Reported by: | Sławek Ehlert | Owned by: | nobody |
|---|---|---|---|
| Component: | contrib.auth | Version: | 2.1 |
| Severity: | Release blocker | Keywords: | auth forms UserCreationForm UserChangeForm |
| Cc: | markush, Carlton Gibson, Mariusz Felisiak | Triage Stage: | Accepted |
| Has patch: | no | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description
In 3333d935d2914cd80cf31f4803821ad5c0e2a51d (part of #28757 ticket) UserCreationForm (and by extension also UserChangeForm) was adjusted to support custom user models without overriding the form. The problem is that the implementation there assumes that the USERNAME_FIELD has to be a subclass of CharField since it uses UsernameField in the field_classes mapping.
Now to the fun part. Django already has some tests that should fail in such case (for example see https://github.com/django/django/blob/825f0beda804e48e9197fcf3b0d909f9f548aa47/tests/auth_tests/test_management.py#L435-L491), but the failure is actually "swallowed" when running the whole test suite. When running those tests individually we can see the failure.
Tim Graham already noticed something similar in https://code.djangoproject.com/ticket/28757#comment:7.
I.e.
./runtests.py auth_tests
works, but
./runtests.py auth_tests.test_management.CreatesuperuserManagementCommandTestCase.test_fields_with_fk
doesn't. I'm not sure why the failure is hidden in the former case (I suspect it has to do something with the override_settings decorator and importlib.reloading done in the reload_auth_forms function from the commit I've mentioned). The stacktrace from the latter case looks like this:
ERROR: test_fields_with_fk (auth_tests.test_management.CreatesuperuserManagementCommandTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/Users/slafs/testing/django/django/django/test/utils.py", line 364, in inner
with self as context:
File "/Users/slafs/testing/django/django/django/test/utils.py", line 336, in __enter__
return self.enable()
File "/Users/slafs/testing/django/django/django/test/utils.py", line 405, in enable
setting=key, value=new_value, enter=True)
File "/Users/slafs/testing/django/django/django/dispatch/dispatcher.py", line 175, in send
for receiver in self._live_receivers(sender)
File "/Users/slafs/testing/django/django/django/dispatch/dispatcher.py", line 175, in <listcomp>
for receiver in self._live_receivers(sender)
File "/Users/slafs/testing/django/django/django/test/signals.py", line 195, in user_model_swapped
from django.contrib.auth import forms
File "/Users/slafs/testing/django/django/django/contrib/auth/forms.py", line 63, in <module>
class UserCreationForm(forms.ModelForm):
File "/Users/slafs/testing/django/django/django/forms/models.py", line 256, in __new__
apply_limit_choices_to=False,
File "/Users/slafs/testing/django/django/django/forms/models.py", line 172, in fields_for_model
formfield = f.formfield(**kwargs)
File "/Users/slafs/testing/django/django/django/db/models/fields/related.py", line 956, in formfield
**kwargs,
File "/Users/slafs/testing/django/django/django/db/models/fields/related.py", line 418, in formfield
return super().formfield(**defaults)
File "/Users/slafs/testing/django/django/django/db/models/fields/__init__.py", line 890, in formfield
return form_class(**defaults)
File "/Users/slafs/testing/django/django/django/forms/fields.py", line 213, in __init__
super().__init__(**kwargs)
TypeError: __init__() got an unexpected keyword argument 'limit_choices_to'
Since auth_tests.CustomUserWithFK model uses a ForeignKey as a USERNAME_FIELD, the instantiation of django.contrib.auth.forms.UsernameField is failing. However, that test is a bit implicit as I'm not entirely sure why Django imports UserCreationForm in the first place (the stacktrace makes me think it's because of the signal mechanisms).
I'd suggest the following steps to go forward with this:
1) Write a more explicit regression test in auth_tests.test_forms that's using auth_tests.CustomUserWithFK as a user model (I'll try to do that).
2) Make adjustments in auth_tests.test_forms so that the mentioned tests will actually fail even when running the whole test suite (I think Tim Graham already tried this in https://code.djangoproject.com/ticket/28757#comment:8).
3) Fix UserCreationForm so it supports different types of fields for USERNAME_FIELD.
In my humble opinion this is a release blocker (I haven't marked this ticket as such, though).
Change History (17)
comment:1 by , 7 years ago
| Severity: | Normal → Release blocker |
|---|---|
| Triage Stage: | Unreviewed → Accepted |
comment:2 by , 7 years ago
I've added some explicit tests in PR#10020.
Unfortunately it causes more failures than the ones I've introduced with those new tests there.
What happens is:
- the test I've added overrides the
AUTH_USER_MODELsetting with a model that has a FK as a username field (auth_tests.CustomUserWithFK). - the test fails on the
override_settingspart already, since thereload_auth_formsreceiver fails to reload thedjango.contrib.auth.formsmodule with that user model in place (fails with a similarTypeErrorthat refers to 'limit_choices_to' argument when trying to instantiate theUsernameField)[*] - because of the
override_settingsbug (see #29467) other tests still see theauth_tests.CustomUserWithFKas a current user model.
[*] - the exact traceback for the mentioned error is:
======================================================================
ERROR: test_custom_form_username_not_charfield (auth_tests.test_forms.UserChangeFormTest)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/usr/local/Cellar/python/3.6.5/Frameworks/Python.framework/Versions/3.6/lib/python3.6/unittest/case.py", line 59, in testPartExecutor
yield
File "/usr/local/Cellar/python/3.6.5/Frameworks/Python.framework/Versions/3.6/lib/python3.6/unittest/case.py", line 605, in run
testMethod()
File "/Users/slafs/testing/django/django/django/test/utils.py", line 364, in inner
with self as context:
File "/Users/slafs/testing/django/django/django/test/utils.py", line 336, in __enter__
return self.enable()
File "/Users/slafs/testing/django/django/django/test/utils.py", line 405, in enable
setting=key, value=new_value, enter=True)
File "/Users/slafs/testing/django/django/django/dispatch/dispatcher.py", line 175, in send
for receiver in self._live_receivers(sender)
File "/Users/slafs/testing/django/django/django/dispatch/dispatcher.py", line 175, in <listcomp>
for receiver in self._live_receivers(sender)
File "/Users/slafs/testing/django/django/tests/auth_tests/test_forms.py", line 38, in reload_auth_forms
reload(django.contrib.auth.forms)
File "/Users/slafs/.virtualenvs/django/lib/python3.6/importlib/__init__.py", line 166, in reload
_bootstrap._exec(spec, module)
File "<frozen importlib._bootstrap>", line 618, in _exec
File "<frozen importlib._bootstrap_external>", line 678, in exec_module
File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
File "/Users/slafs/testing/django/django/django/contrib/auth/forms.py", line 63, in <module>
class UserCreationForm(forms.ModelForm):
File "/Users/slafs/testing/django/django/django/forms/models.py", line 256, in __new__
apply_limit_choices_to=False,
File "/Users/slafs/testing/django/django/django/forms/models.py", line 172, in fields_for_model
formfield = f.formfield(**kwargs)
File "/Users/slafs/testing/django/django/django/db/models/fields/related.py", line 956, in formfield
**kwargs,
File "/Users/slafs/testing/django/django/django/db/models/fields/related.py", line 418, in formfield
return super().formfield(**defaults)
File "/Users/slafs/testing/django/django/django/db/models/fields/__init__.py", line 890, in formfield
return form_class(**defaults)
File "/Users/slafs/testing/django/django/django/forms/fields.py", line 213, in __init__
super().__init__(**kwargs)
TypeError: __init__() got an unexpected keyword argument 'limit_choices_to'
follow-up: 4 comment:3 by , 7 years ago
I don't understand why this issue is described as a regression. Using a custom user model with UserCreationForm and UserChangeForm is a new feature, so I wouldn't describe any issues there as a regression. A regression would be something that worked in older versions of Django that no longer works. Am I misunderstanding?
comment:4 by , 7 years ago
Replying to Tim Graham:
I don't understand why this issue is described as a regression. Using a custom user model with
UserCreationFormandUserChangeFormis a new feature, so I wouldn't describe any issues there as a regression. A regression would be something that worked in older versions of Django that no longer works. Am I misunderstanding?
Nope, all good. You're correct about the naming here. My bad.
comment:5 by , 7 years ago
| Summary: | Regression in UserCreationForm (and UserChangeForm) for custom user models → UserCreationForm and UserChangeForm don't work if username isn't a CharField |
|---|
If a patch isn't forthcoming (the best way to avoid the issue that Markus pointed out in comment 1 isn't obvious to me), I think we could deescalate from a release blocker by documenting this limitation.
comment:6 by , 7 years ago
I'd like to submit a patch for that, but currently I was focusing more on solving #29467 first.
comment:7 by , 7 years ago
| Has patch: | set |
|---|
I've opened PR to adjust the docs here.
I think we could deescalate from a release blocker by documenting this limitation.
Hopefully that lets us move forward.
comment:8 by , 7 years ago
Well, I think the more pressing problem is not the feature set Django supports and the docs, but the fact that the change mentioned here breaks the test suite and can break people's projects.
Also by having a auth_tests.CustomUserWithFK model in the test suite, one would think that this kind of model is supported by Django, which is not true at this point.
Moreover, having a similar user model in a project means that a developer can't even import from django.contrib.auth.forms module. And what if something imports that module that's not in developer's control? E.g. currently django/contrib/admin/forms.py, django/contrib/auth/admin.py and django/contrib/auth/views.py are importing something from that module, so that means on fresh project with admin enabled with that kind of a user model, developer can't even run ./manage.py --help.
comment:9 by , 7 years ago
So I think, regarding this feature - we should either fix it or ditch it completely.
I made some substantial progress on #29467 lately. I'd like to try and tackle this ticket next (although I have some time constraints and the progress may be slow on this).
comment:11 by , 7 years ago
| Has patch: | unset |
|---|
comment:13 by , 7 years ago
| Cc: | added |
|---|
I can confirm this.
Using something like
UserModel._meta.get_field(UserModel.USERNAME_FIELD).formfield()to derive the form field for a the current username field _may_ make sense. However, this won't work with the normalization we currently do in the form'sUsernameField.