[Django] #21648: the "is_admin_site" option of password_reset is undocumented and the name ambiguous

Django

unread,

Dec 20, 2013, 11:59:38 AM12/20/13

to django-...@googlegroups.com

#21648: the "is_admin_site" option of password_reset is undocumented and the name
ambiguous
-------------------------------+--------------------
Reporter: brain@… | Owner: nobody
Type: Bug | Status: new
Component: Documentation | Version: 1.6
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------+--------------------
Hello,

Here
https://docs.djangoproject.com/en/1.6/topics/auth/default/#django.contrib.auth.views.password_reset
the option "is_admin_site" is not documented and its name doesn't describe
what it is doing.

According to
https://github.com/django/django/blob/master/django/contrib/auth/views.py#L160
and
https://github.com/django/django/blob/master/django/contrib/auth/forms.py#L248-253
it uses the host of the request instead of the site framework, therefor it
should be name something like "use_domain_from_request".

Kind regards,

--
Ticket URL: <https://code.djangoproject.com/ticket/21648>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,

Dec 21, 2013, 7:36:53 PM12/21/13

to django-...@googlegroups.com

#21648: the "is_admin_site" option of password_reset is undocumented and the name
ambiguous

-------------------------------+------------------------------------

Reporter: brain@… | Owner: nobody
Type: Bug | Status: new
Component: Documentation | Version: 1.6

Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------
Changes (by timo):

* needs_better_patch: => 0
* stage: Unreviewed => Accepted
* easy: 1 => 0
* needs_tests: => 0
* needs_docs: => 0

Comment:

Yes, it seems poorly named, but I'm not sure it's worth renaming due to
backwards compatibility concerns at this point. The parameter was added in
the merge of magic-removal pre-1.0, but the functionality didn't appear to
be used or tested until [9305c0e1].

--
Ticket URL: <https://code.djangoproject.com/ticket/21648#comment:1>

Django

unread,

Feb 23, 2014, 2:21:38 PM2/23/14

to django-...@googlegroups.com

#21648: the "is_admin_site" option of password_reset is undocumented and the name
ambiguous

-------------------------------+------------------------------------

Reporter: brain@… | Owner: nobody
Type: Bug | Status: new
Component: Documentation | Version: 1.6

Comment (by claudep):

Is the use case for that parameter still accurate? Shouldn't we deprecate
it?

--
Ticket URL: <https://code.djangoproject.com/ticket/21648#comment:2>

Django

unread,

Aug 18, 2014, 10:46:05 AM8/18/14

to django-...@googlegroups.com

#21648: Deprecate "is_admin_site" option of auth.views.password_reset()
--------------------------------------+------------------------------------
Reporter: brain@… | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: contrib.auth | Version: master

Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0

--------------------------------------+------------------------------------
Changes (by timgraham):

* type: Bug => Cleanup/optimization
* has_patch: 0 => 1
* version: 1.6 => master
* component: Documentation => contrib.auth

Comment:

Deprecation looks reasonable to me. I couldn't find a use case for it
because if `contrib.sites` isn't installed,
`contrib.sites.requests.RequestSite` (via `get_current_site()` in
`PasswordResetForm`) uses `request.get_host()` as well.

[https://github.com/django/django/pull/3083 PR]

--
Ticket URL: <https://code.djangoproject.com/ticket/21648#comment:3>

Django

unread,

Aug 24, 2014, 12:30:54 PM8/24/14

to django-...@googlegroups.com

#21648: Deprecate "is_admin_site" option of auth.views.password_reset()
--------------------------------------+------------------------------------

Reporter: brain@… | Owner: nobody
Type: Cleanup/optimization | Status: closed
Component: contrib.auth | Version: master
Severity: Normal | Resolution: fixed

Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------

Changes (by Tim Graham <timograham@…>):

* status: new => closed
* resolution: => fixed

Comment:

In [changeset:"e39af5ea592bbe4e7a643e67f0c379adc10ed392"]:
{{{
#!CommitTicketReference repository=""
revision="e39af5ea592bbe4e7a643e67f0c379adc10ed392"
Fixed #21648 -- Deprecated is_admin_site option to
auth.views.password_reset().
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/21648#comment:4>

Django

unread,

Sep 17, 2014, 5:52:45 AM9/17/14

to django-...@googlegroups.com

#21648: Deprecate "is_admin_site" option of auth.views.password_reset()
--------------------------------------+------------------------------------

Reporter: brain@… | Owner: nobody

Comment (by googol7):

A use case is if you are using django CMS (which relies on
django.contrib.sites) to host a website on xyz.domain.tld and you have
several subdomains abc.domain.tld, xyz.domain.tld, etc. which do not use
django CMS but only provide a (branded) login where users can also click
the "forgot password" link and in the resulting email the referring URL
(e.g. xyz.domain.tld) should be used.

django CMS:

/cms/models/pagemodel.py
from django.contrib.sites.models import Site
site = models.ForeignKey(Site, help_text=_('The site the page is
accessible at.'), verbose_name=_("site"))

http://docs.django-cms.org/en/2.4.0/extending_cms/api_references.html
cms.models.pagemodel.Page
site (django.contrib.sites.models.Site instance) – Site to put this page
on

http://docs.django-
cms.org/en/latest/getting_started/integrate.html#installing
Be sure to have 'django.contrib.sites' in INSTALLED_APPS and set SITE_ID
parameter in your settings: they may be missing from the settings file
generated by django-admin depending on your Django version and project
template.

--
Ticket URL: <https://code.djangoproject.com/ticket/21648#comment:5>

Django

unread,

Sep 17, 2014, 8:16:11 AM9/17/14

to django-...@googlegroups.com

#21648: Deprecate "is_admin_site" option of auth.views.password_reset()
--------------------------------------+------------------------------------

Reporter: brain@… | Owner: nobody

Comment (by timgraham):

Do you have each subdomain setup as a site?

--
Ticket URL: <https://code.djangoproject.com/ticket/21648#comment:6>

Django

unread,

Sep 17, 2014, 9:07:13 AM9/17/14

to django-...@googlegroups.com

#21648: Deprecate "is_admin_site" option of auth.views.password_reset()
--------------------------------------+------------------------------------

Reporter: brain@… | Owner: nobody

Comment (by googol7):

No I've got:

- One Django instance with one settings.py which contains SITE_ID = 1
- Django CMS installed and INSTALLED_APPS containing django.contrib.sites
The django CMS pages are only rendered for www.domain.tld
- Several subomains pointing to the same Django instance. nginx is
redirecting / to /admin on these subdomains as we only allow login and
password reset but do not display any django CMS content there:
abc.domain.tld, def.domain.tld, ghi.domain.tld, jkl.domain.tld
- In the templates I use a templatetag which returns different
configurations (CSS, etc.) for the subdomains in use based on the host:
host = context['request'].get_host()

--
Ticket URL: <https://code.djangoproject.com/ticket/21648#comment:7>

Django

unread,

Sep 17, 2014, 11:58:20 AM9/17/14

to django-...@googlegroups.com

#21648: Deprecate "is_admin_site" option of auth.views.password_reset()
--------------------------------------+------------------------------------

Reporter: brain@… | Owner: nobody

Comment (by timgraham):

Thanks for the details. Do you think removing the option isn't acceptable
then? Any sense whether this sort of setup is common? Would updating your
site to use the sites framework be too much work?

--
Ticket URL: <https://code.djangoproject.com/ticket/21648#comment:8>

Django

unread,

Sep 18, 2014, 11:01:53 AM9/18/14

to django-...@googlegroups.com

#21648: Deprecate "is_admin_site" option of auth.views.password_reset()
--------------------------------------+------------------------------------

Reporter: brain@… | Owner: nobody

Comment (by googol7):

Good question. I think that updating to the sites framework would be a lot
of overhead for us as we just provide a branded site based on the URL. I
don't know if this is a common setup but probably its worth thinking about
why the domain defined for a site shall be used for the password reset
link in the email and if it would be sufficient to just use
request.get_host() Because if the user visits the domain of the site
framework it's the current domain of the current request anyway.

--
Ticket URL: <https://code.djangoproject.com/ticket/21648#comment:9>

Django

unread,

Sep 29, 2014, 12:44:08 PM9/29/14

to django-...@googlegroups.com

#21648: Deprecate "is_admin_site" option of auth.views.password_reset()
--------------------------------------+------------------------------------

Reporter: brain@… | Owner: nobody

Comment (by timgraham):

It looks like #15089 would address these concerns, so I will try to move
that ticket forward.

--
Ticket URL: <https://code.djangoproject.com/ticket/21648#comment:10>

Django

unread,

Sep 23, 2015, 7:54:46 PM9/23/15

to django-...@googlegroups.com

#21648: Deprecate "is_admin_site" option of auth.views.password_reset()
--------------------------------------+------------------------------------

Reporter: brain@… | Owner: nobody

Comment (by Tim Graham <timograham@…>):

In [changeset:"f1761e3fefadd9730e8b17c0995e1317c58bd10c" f1761e3]:
{{{
#!CommitTicketReference repository=""
revision="f1761e3fefadd9730e8b17c0995e1317c58bd10c"
Refs #21648 -- Removed is_admin_site option from password_reset() view.