Skip to content

Get session using SessionStore #178

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Get session using SessionStore #178

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 5 additions & 6 deletions easyaudit/signals/request_signals.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from django.utils import timezone
from django.conf import settings
from django.utils.module_loading import import_string
from importlib import import_module

# try and get the user from the request; commented for now, may have a bug in this flow.
# from easyaudit.middleware.easyaudit import get_current_user
Expand Down Expand Up @@ -69,14 +70,12 @@ def request_started_handler(sender, **kwargs):
session_cookie_name = settings.SESSION_COOKIE_NAME
if session_cookie_name in cookie:
session_id = cookie[session_cookie_name].value

try:
session = Session.objects.get(session_key=session_id)
except Session.DoesNotExist:
session = None
engine = import_module(settings.SESSION_ENGINE)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't particularly mind this flow, but is there a less brutal way of doing this for users that do not require as much flexibility in the call flow? Maybe a new settings flag?
Also, is there harm in resolving the engine in the module scope, as opposed to the function scope? (Perhaps there is, like a django init thing, but wanted to ask just in case there was no danger.)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you referring to the use of import_module? It's the recommended way in the Django docs. If the user is retrieved from request.user this would not be required at all actually (see my comment in #165).

I agree that the session store should be retrieved once instead of each request. Maybe move it to the top of the module where audit_logger is defined?

SessionStore = engine.SessionStore
session = SessionStore(session_id)

if session:
user_id = session.get_decoded().get('_auth_user_id')
user_id = session.get('_auth_user_id')
try:
user = get_user_model().objects.get(id=user_id)
except:
Expand Down