Opened 7 years ago

Closed 7 years ago

Last modified 5 years ago

#23957 closed Cleanup/optimization (fixed)

Start a deprecation path toward requiring session verification

Reported by: Tim Graham Owned by: Tim Graham
Component: contrib.auth Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

From Carl in comments of #23939: "Is there a use case for a long-term simple way to disable this behavior? Or is it just a way to preserve sessions across the upgrade that we need? I think we should be on a deprecation path to making [session verification] always-on; I think it's fine if you have to write your own AuthenticationMiddleware if you don't want it."

As far as I know, the only-use case for disabling it was to provide an upgrade path.

The deprecation path could look like this:

1.8: Raise RemovedInDjango20Warning if AuthenticationMiddleware but not SessionAuthenticationMiddleware is in MIDDLEWARE_CLASSES (because session verification will be mandatory in 2.0)
2.0: It's now safe to remove SessionAuthenticationMiddleware from MIDDLEWARE_CLASSES since the behavior can't be turned off. Raise RemovedInDjango22Warning if it's there so we can eventually remove the class.

Change History (8)

comment:1 Changed 7 years ago by Carl Meyer

I considered this version of the deprecation path (without ever introducing an AUTH_VERIFY_SESSION setting). But I don't like it, because it involves raising one deprecation warning for two versions telling people to add something to their settings, and then raising another deprecation warning the following version telling them they should remove the thing they just added.

So even though it involves a new setting, I still think it's better to add AUTH_VERIFY_SESSION in 1.8 when we deprecate SessionAuthenticationMiddleware, so that people can immediately add AUTH_VERIFY_SESSION = True, remove SessionAuthenticationMiddleware, and never again see another deprecation warning related to this feature. (In 2.0 their AUTH_VERIFY_SESSION = True will become unnecessary, but it still won't be deprecated).

comment:2 Changed 7 years ago by Tim Graham

I'd take another angle and say I'd prefer Django to tell me when I have a useless setting around so I can keep things clean. Am I underestimating the difficulty of adding/removing one line from settings.py? I think that's less of a burden than the cognitive load of a new setting. It seems cleaner to have one way to activate session verification. Alternatively, I'd opt to keep SessionAuthenticationMiddleware around indefinitely (as a no-op) if you think deprecating it immediately at 2.0 will cause pain.

comment:3 Changed 7 years ago by Carl Meyer

I think there's a cumulative burden to deprecation warnings period, somewhat independently of how hard or easy they are to address; as they accumulate, they increase the sense that migrating from one Django version to the next is a chore.

Mostly, I was hoping we could confine the "check for a no-op middleware in MIDDLEWARE_CLASSES" hack to those projects created on 1.7, and deprecate it right away on 1.8, rather than forcing everyone to use it, and keeping it around until 2.2 or later. But I agree with you that adding a new setting that we plan to immediately deprecate is sad, too. So I'm OK with either approach.

comment:4 Changed 7 years ago by Tim Graham

Owner: changed from nobody to Tim Graham
Status: newassigned
Triage Stage: UnreviewedAccepted

comment:5 Changed 7 years ago by Tim Graham

Has patch: set

comment:6 Changed 7 years ago by Tim Graham <timograham@…>

Resolution: fixed
Status: assignedclosed

In b6ea059b4ab7a4ed7e84cad639df95fc9d61dd81:

Fixed #23957 -- Started deprecation toward requiring session verification.

Thanks Carl Meyer for review.

comment:7 Changed 6 years ago by Tim Graham <timograham@…>

In 849037af:

Refs #23957 -- Required session verification per deprecation timeline.

comment:8 Changed 5 years ago by Tim Graham <timograham@…>

In 401c5b2:

Refs #23957 -- Removed the useless SessionAuthenticationMiddleware.

Note: See TracTickets for help on using tickets.
Back to Top