Getting user ID from a request shouldn't hit the database

[Bertrand Bordage]

In a view, it is a common case to use the current user ID.
It is especially useful to limit the queryset to what the user authored.

To do so, one must use the AuthenticationMiddleware and write something like qs.filter(author_id=request.user.pk) in the view.

Unfortunately, request.user is a lazy object that generates a database hit when used.
This DB hit is useless since user ID is taken from the request object (​as shown here).
There is a similar issue in templates when we use context_processors.auth.

I suggest we add a user_id attribute to the request in AuthenticationMiddleware. That would be consistent with the foreign key idiom that ID can be directly accessed from [fk_name]_id without any extra DB hit.
I also suggest we add the same user_id to context_processors.auth.

[Tim Graham]

I haven't thought it through completely, but I'd want to be cautious about this change for security reasons. For example, without checking if the user exists in the database, I think we could end up with a situation where request.user_id returns some integer value in a spoofed session, but request.user is an AnonymousUser. This could encourage users to write insecure or error prone code.

[Bertrand Bordage]

Hum… But isn't request.session[SESSION_KEY] (what would be known as request.user_id) created on login and destroyed on logout? ​From what I can read in the code, we already check that the user exists during login.

[Tim Graham]

Yes, and I guess the session should be safe from tampering since we are either storing in the database or signing it for the cookie backend. I'm still cautious about the change, but tentatively marking as accepted.

[Carl Meyer]

The inconsistency would still occur in the case where a user logs in, an admin deletes their User from the database, and then they continue using the same browser session, never having logged out. In this situation any view depending on request.user would see an AnonymousUser(), even though request.userid would still be set. It's perhaps a judgement call whether the admin in this scenario should have a reasonable expectation that deleting a User from the database would prevent any further access by that user.

This is a variant of the same type of out-of-date-session concern that led to introduction of SessionAuthenticationMiddleware in 1.7, which invalidates pre-existing sessions when a user even just changes their password. SessionAuthenticationMiddleware (which is now included in MIDDLEWARE_CLASSES in the default startproject template) accesses request.user, triggering the database query anyway. So nobody using a default 1.7+ project would ever see any benefit from using a hypothetical request.user_id. In my mind, that means it's a feature that's not worth adding. It's possible to implement it yourself if you really want it, and don't mind the odd inconsistency (and lack of user-existence verification) that it introduces.

[Aymeric Augustin]

I agree with Carl. (I just didn't take the time to write it down before he did.)

[Bertrand Bordage]

OK. I find it sad to have an extra SQL request on every view with a logged user, but since we can’t fetch all the sessions of a user to invalidate them on delete or password change, I guess there is no other option…