yourlabs / django-session-security

A little javascript and middleware work together to ensure that the user was active during the past X minutes in any tab he has open. Otherwise, display a warning leaving a couple of minutes to show any kind of activity like moving the mouse. Otherwise, logout the user.
http://django-session-security.rtfd.org
MIT License
309 stars 142 forks source link

Can't subclass `SessionSecurityMiddleware` #136

Closed theunraveler closed 5 months ago

theunraveler commented 4 years ago

Because of the way that this library's settings are stored an imported, when I try to subclass SessionSecurityMiddleware and use my own middleware class instead, Django raises an ImproperlyConfigured exception with the message "The SECRET_KEY setting must not be empty."

I think this is because importing the session_security.middleware also imports session_security.settings, which doesn't work because Django is not done configuring itself yet when the module is imported. This problem doesn't happen when you just add session_security.middleware.SessionSecurityMiddleware to MIDDLEWARE because the module is lazily imported after Django is done configuring itself.

Here's an example of some code that will cause the issue:

# myproject/middleware.py
from session_security.middleware import SessionSecurityMiddleware

class MySessionSecurityMiddleware(SessionSecurityMiddleware):
    pass

# myproject/settings.py
MIDDLEWARE = (
    # ...
    'myproject.middleware.MySessionSecurityMiddleware',
    # ...
)

So this PR just delays the importing of things from session_security.settings until they are actually needed, so that they aren't imported while Django is still configuring itself.

Another way I thought about doing this is to make the constants in session_security.settings lazy-loaded using django.utils.functional.lazy. I think that is the more sustainable approach, because it would also mitigate similar problems if someone tries to directly import session_security.templatetags.session_security_tags for whatever reason, but that seemed a little more complicated. But if you aren't a fan of having import statements in function bodies, I can take a look at doing it that way instead.

Thanks!

jpic commented 5 months ago

Nice! merged manually