Skip to content
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.

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

Hijack session variables are not available until after hijack is fully completed #110

Closed
alex-kaufman opened this issue Feb 29, 2016 · 12 comments

Comments

@alex-kaufman
Copy link

I am using the builtin django auth signal user_logged_in to record when user logins and store metadata about their login. However, I do not want to keep track of hijacked logins. This presents an issue because at the point a user is logged in via hijack https://github.com/arteria/django-hijack/blob/master/hijack/helpers.py#L110, the session variables are not set, which does make sense since you wouldn't want to set session variables before a login completely succeeds.

Perhaps, there could be some other session variable set at the beginning of the login_user method that would give an indication that the user's login is being handled from django-hijack.

@jvamvas
Copy link
Contributor

jvamvas commented Mar 2, 2016

@alex-kaufman Thank you for your feedback. I currently do not see a way to solve this issue with an extra session variable. Django creates a new session when login is called (https://github.com/django/django/blob/stable/1.8.x/django/contrib/auth/__init__.py#L108), so the variable would be deleted before the login signal is sent.

However, django-hijack has its own signals (http://django-hijack.readthedocs.org/en/latest/#signals). Perhaps you could catch hijack_started to delete the unused login records?

@zopieux
Copy link
Contributor

zopieux commented Mar 7, 2016

@alex-kaufman Does df7a3f9 fixes your issue? tl;dr the user_logged_in signal is now ignored during hijacked logins.

If it does, please remember to close this issue! Thanks.

@alex-kaufman
Copy link
Author

Unfortunately it does not. The user_logged_in signal does not seem to be disconnected. df7a3f9#diff-49fd28c4615b7a75d1da58eabf1f4f8eR111, in djangos login method it still is sending the user_logged_in signal

@zopieux
Copy link
Contributor

zopieux commented Mar 8, 2016

Signals cannot be disconnected in the meaning you seem to understand. What can be disconnected are signal handlers, that are functions called when a specific signal send() method is called. This is what df7a3f9 does. I was mistaken about your issue though, and indeed this commit will not help you.

AFAICT, what we need is a custom django-hijack authentication backend that would be applied to the hijacked user – nothing complicated, just a documented class. That way, you can check in your signal handler what backend the user was logged in with, as it is storred in user.backend. Currently django-hijack uses a hack to fake the backend to the "default" one. It's lying and as a consequence you cannot test if it was a hijack or real log-in.

I'll try to test that solution in the coming days.

@fission6
Copy link

fission6 commented May 3, 2016

Is there a fix for this. I have handlers listening to user_logged_in which is being fired whenever someone hijacks an account. I do not want my handler to account for hijacked accounts. Is there a way to avoid this?

@fission6
Copy link

fission6 commented Jun 6, 2016

@alex-kaufman just wanted to double check if there was a solution for figuring out if a user session is hijacked for a signal receiver for user_logged_in

@philippeowagner
Copy link
Contributor

Could we close this?

@hirokiky
Copy link

hirokiky commented May 10, 2018

@zopieux @philippeowagner No. I think this issue can't be closed.

The commit df7a3f9 (@zopieux said) will disconnect update_last_login handler while calling actual django's login function.
At current version (version 2.1.7), this behavior is covered by no_update_last_login context manager https://github.com/arteria/django-hijack/blob/master/hijack/helpers.py#L19

But the problem is that it will only disconnect the update_last_login handler, not for other handlers like functions added by developers itself.
Still, hijack's login send the signal and call handlers (excluding update_last_login) with hijacked users.

Currently I avoid using the user_logged_in signal as workaround.

@armisael
Copy link

armisael commented Oct 8, 2018

I feel dirty to suggest it, but until this is properly fixed, a possible workaround would be to test the request.path:

@receiver(user_logged_in, dispatch_uid='my_signals.user_logged_in')
def user_logged_in(sender, request, user, **kwargs):
    is_hijacked_user = request.session.get('is_hijacked_user', False) or request.path.startswith('/admin/hijack')
    if is_hijacked_user:
        return

@NikolaiT
Copy link

NikolaiT commented Sep 9, 2019

Why is there still no clear solution for this?

@Mogost
Copy link
Member

Mogost commented Mar 20, 2021

Most likely your request is no longer relevant after the release of version 3.0 (currently in pre-release status). If it is not, please reopen your ticket.

@DanieleIsoni
Copy link

Hello, I think this has not been solved with 3.0. If I do:

@receiver(user_logged_in, dispatch_uid='my_signals.user_logged_in')
def user_logged_in(sender, request, user, **kwargs):
    is_hijacked_user = getattr(user, "is_hijacked", False)
    if is_hijacked_user:
        return

here is_hijacked_user is False.

The "dirty" trick suggested here still works, but it would be great if this was not necessary

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests