Make User.is_authenticated() and User.is_anonymous() "callable properties"

[Stavros Korokithakis]

There's an inconsistency between is_authenticated and is_superuser. The former is a method and the latter is a property, leading many people to do:

if request.user.is_authenticated:
    return sensitive_information

which is, of course, always executed.

I propose that is_authenticated be turned into a property, while it can also be a callable, for backwards-compatibility.

[Tim Graham]

I bumped an old ​django-developers discussion to get opinions about this.

[Aymeric Augustin]

The discussion is leaning towards making this change.

A possible deprecation of the callable form wasn't discussed.

[Tim Graham]

#26388 suggested the same for is_anonymous().

[Jeremy Lainé]

I have put up a pull request here:

​https://github.com/django/django/pull/6376

@apollo13 mentioned that we might want to support the case where a custom user model defines is_anonymous / is_authenticated as methods instead of properties. In this case we could do something like this to monkey-patch that user model to the new style:

    def __new__(cls):
        if not isinstance(cls.is_anonymous, property):
            real_is_anonymous = cls.is_anonymous
            cls.is_anonymous = property(lambda x: CallableBool(real_is_anonymous(x)))
        return super(AbstractBaseUser, cls).__new__(cls)

[Tim Graham]

Alternatively, we could add a system check that raises a warning for an incompatible implementation.

[Tim Graham]

Left comments for improvement.

[Tim Graham <timograham@…>]

In c1aec0fe:

Fixed #25847 -- Made User.is_(anonymous|authenticated) properties.

[Florian Apolloner]

@timgraham: I feel this should get reverted for now. We have to consider custom user models which did define this as a method. A warning is not enough, either his is a hard error or we ensure that it is backwards compatible. Allowing custom user models to have a method is a security risk since all checks will now return true…

[Tim Graham]

I was thinking of adding a "compatibility" system check to detect that. I'll do it before 1.10 alpha if no one else takes it.

[Florian Apolloner]

Unless that system check is a hard error I am against it. (Ie it does have to be ERROR, so we can ensure that we do not open ourself to security issues) -- can we justify the backwards incompatibility?

[Tim Graham]

​PR for the system check to detect is_anonymous/is_authenticated as methods.

[Tim Graham <timograham@…>]

In 03efa304:

Refs #25847 -- Added system check for UserModel.is_anonymous/is_authenticated methods.

[Tim Graham <timograham@…>]

In eba093e:

Refs #25847 -- Removed support for User.is_(anonymous|authenticated) as methods.

Per deprecation timeline.