Improving docs for User.is_authenticated()
Tom Evans
I don't like this function that much. It doesn't actually check
whether users are authenticated - which is to say, they have presented
credentials which we have accepted and authorized them to use to the
site. Instead it always returns true. is_not_anonymous_user() may be a
better name.
User.is_authenticated() is documented like so:
"""
is_authenticated()
Always returns True. This is a way to tell if the user has been
authenticated. This does not imply any permissions, and doesn't check
if the user is active - it only indicates that the user has provided a
valid username and password.
""""
This is misleading, as it doesn't actually indicate that the user has
provided a valid username and password, since it always returns True.
There can be many ways that a user authenticates without having to
provide username and password, and User objects not automatically
instantiated by the auth middleware (eg, manually looking up a user)
haven't been authenticated at all.
Eg, this code:
def myview(request):
user = User.objects.all().order_by('?')[0]
user.is_authenticated()
At no point has that user object been authenticated, or supplied valid creds.
Obviously, this function cannot change in behaviour or name, so I
suggest altering the docs, dropping the clause about indicating that
the user has provided username and password to make it clearer what
this method does.
Cheers
Tom
Carl Meyer
On 02/23/2012 08:41 AM, Tom Evans wrote:
> I don't like this function that much. It doesn't actually check
> whether users are authenticated - which is to say, they have presented
> credentials which we have accepted and authorized them to use to the
> site. Instead it always returns true. is_not_anonymous_user() may be a
> better name.
>
> User.is_authenticated() is documented like so:
>
> """
> is_authenticated()
> Always returns True. This is a way to tell if the user has been
> authenticated. This does not imply any permissions, and doesn't check
> if the user is active - it only indicates that the user has provided a
> valid username and password.
> """"
>
> This is misleading, as it doesn't actually indicate that the user has
> provided a valid username and password, since it always returns True.
[snip]
> Obviously, this function cannot change in behaviour or name, so I
> suggest altering the docs, dropping the clause about indicating that
> the user has provided username and password to make it clearer what
> this method does.
I agree with you on all counts: the method is poorly named, it can't be
changed now (and is not worth a deprecation process), but the docs
should be less misleading. Can you file a ticket for this?
Carl
Luke Granger-Brown
It does prove that they've authenticated, in that request.user will contain an AnonymousUser if they're not logged in, which overrides this method to always return False. If they are, then they'll get their actual user, which will return True.
>
> Cheers
>
> Tom
>
> --
> You received this message because you are subscribed to the Google Groups "Django developers" group.
> To post to this group, send email to django-d...@googlegroups.com.
> To unsubscribe from this group, send email to django-develop...@googlegroups.com.
> For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
>
Luke Granger-Brown
> >
> > Obviously, this function cannot change in behaviour or name, so I
> > suggest altering the docs, dropping the clause about indicating that
> > the user has provided username and password to make it clearer what
> > this method does.
>
> It does prove that they've authenticated, in that request.user will contain an AnonymousUser if they're not logged in, which overrides this method to always return False. If they are, then they'll get their actual user, which will return True.
Sorry - I only just worked out what your actual objection was.
The function name does indeed only make sense in the form of the user given to you in the request object - I agree with you in that sense, and that the documentation should be clarified to make this clear - perhaps by removing the 'username and password' bit, or maybe by noting that this method is most useful in the sense of the request user?
Cheers,
Luke
Carl Meyer
> It does prove that they've authenticated, in that request.user will
> contain an AnonymousUser if they're not logged in, which overrides this
> method to always return False. If they are, then they'll get their
> actual user, which will return True.
But request.user is far from the only circumstance in which you might
have a User instance. And all User instances have this method, not just
the one attached to a request by the auth middleware. The method name is
only accurate in the latter case.
Carl
Clay McClure
I don't like this function that much.
Luke Sneeringer
On March 12, 2012, at 14:47 , Clay McClure wrote:
> On Thursday, February 23, 2012 10:41:36 AM UTC-5, Tom Evans wrote:
> there is also the possible source of confusion stemming from the fact that in template language we write:
>
> {% if user.is_authenticated %}
>
> but in Python we write:
>
> if user.is_authenticated():
>
> You could easily get used to writing it the first way if you do a lot of template development, and then accidentally write it that way when you switch back to Python:
>
> if user.is_authenticated:
>
> which will happily and quietly always evaluate to True.
I've made this error. It's a pita to debug, too.
Regards,
Luke