django-hijack / django-hijack Goto Github PK

The answer for using django-hijack with a custom User model and the hijack button is not obvious enough.

I suggest to change it to something like:

from django.contrib import admin
from custom_app.admin import MyCustomUserAdmin
from custom_app.models import MyCustomUser

from hijack.admin import HijackUserAdminMixin

admin.site.unregister(MyCustomUser)


class MyCustomUserAdminWithHijackButton(HijackUserAdminMixin, MyCustomUserAdmin):
    """
    We're subclassing HijackUserAdminMixin to display the hijack button in the admin.
    """
    list_display = MyCustomUserAdmin.list_display + ('hijack_field', )

admin.site.register(MyCustomUser, MyCustomUserAdminWithHijackButton)

relevant source

https://github.com/arteria/django-hijack/blob/master/hijack/admin.py#L55-L63

We should create an companion app for that including session key to user lookup.

Optional: Send an email to the user when hijacked.

Sometimes it would make sense to disable the hijack mechanism for inactive users. Hijacking an inactive use breaks a password reset token for example.

Hi !

Nice package !

Features that would be nice:

• hooks to handle these cases:
  • hijacker has to be in a specific group or have a specific permission
  • hijacked user must respect some conditions (for instance : filter the user queryset)
• some internationalisation
• interpret LOGIN_REDIRECT_URL as a named url (cf django.shortcuts.resolve_url and its usage in the django login view : https://github.com/django/django/blob/002425fe39f62faafaa32e400f7531809181a1a0/django/contrib/auth/views.py#L48 )

When you set the ALLOW_STAFF_TO_HIJACKUSER setting, any staff member can hijack admins.

Hello! I'm really enjoying this plugin so far. One issue I've noticed is the positioning of the notification when paired with a Bootstrap navbar:

The notification almost completely overlaps the navbar. There are a couple of ways around this:

1. Add a container around #hijacked-warning with a height: 30px;.
2. Make the hijack CSS file a template tag and dynamically insert it (as with the notification). We could add a body { margin-top: 30px; } to the CSS file to make room for the notification.

I'm sure there are other options, too. I've tested both of these and they work.

I was using django 1.7 where everything works fine, and now I'm trying to switch to django 1.8.
When using default session backend problem doesn't occur, but with memcached+db through cached_db hijack/<user_id> returns these headers

Content-Type:text/html; charset=utf-8
Date:Sat, 21 Nov 2015 11:25:08 GMT
Location:http://192.168.99.100:8000/roles/
Server:WSGIServer/0.1 Python/2.7.6
Set-Cookie:csrftoken=pRzupCqWDOBR6aN1C8Tg4Es8a1WZDcKy; expires=Sat, 19-Nov-2016 11:25:08 GMT; Max-Age=31449600; Path=/
Set-Cookie:sessionid=; expires=Sat, 05-Dec-2015 11:25:08 GMT; httponly; Max-Age=1209600; Path=/
Vary:Cookie

As you can see sessionid is empty, and when I comment this line

SESSION_ENGINE = "django.contrib.sessions.backends.cached_db"

sessionid is there as expected.

In addition to post_superuser_login, there is also post_superuser_logout in signals.py.

• Check source and adapt if required
• Update setup.py when Python 3 is supported

Support user models created through
https://docs.djangoproject.com/en/1.5/topics/auth/customizing/#substituting-a-custom-user-model

In my application I have users that are designated as agents (but not staff) and users that those agents are assigned to.

I've written a custom hijack function to determine permission, but the hijacking seems to only works if I make the agents staff also.

Is there any way around this limitation?

I just installed django-hijack with django==1.8 and a custom user model.

After adding hijack to INSTALLED_APPS and including the urls, the project won't start because of the custom user model: see here

after setting SHOW_HIJACKUSER_IN_ADMIN = False and subclassing my custom user model admin from admin.HijackUserAdminMixin the project was up and running.

I think the application should not raise an Error when installed with the default settings. Displaying a Waring would be enough.

Last time I worked on my small improvement to add extra control regarding to the user permissions I noticed that there are quite some PEP8 violations and that some parts need a refactor to be better readable and easier to work with. (Check for instance the permission tests. They do their job, but are hard to understand.)

Fixing the PEP8 violations is quite easy, but refactoring large parts of the code may take several individual pull requests since it covers large parts of the codebase.

My proposal is to start with fixing all the PEP8 errors ASAP and to split up the refactor of parts like the test, the permission system etc. in order to limit the amount of time that has to be invested before a new project can be merged.

I can start with fixing all the PEP8 errors in the upcoming week if people can check and merge it soon after I propose the PR. Please let me know if we can start with this. :-)

This only works if the urls are included under /hijack/.

• Modify README
• Modify setup.py
• ...

This refactoring extends PR #21. This will be the new default afterwards.

Instead of

{{ request | hijackNotification }}

it would be more clear to just have

{{ hijack_notification }}

• Default notification CSS here: https://gist.github.com/philippeowagner/7552397
• Templatetags, based on comments in #3 (2eb1f54#comments)

Hello, I really like the idea of this plugin and I'm glad to see it's active.

I started to try integrating it to my project but I noticed an error during the Hijack by User ID (with a custom User) on Django 1.7.4.

I got the error ValueError at /hijack/230/
Object is of type 'str', but must be a Django Model, Manager, or QuerySet

So I went in the code and I saw that the faulty part is there:

from compat import get_user_model
...
user = get_object_or_404(get_user_model(), pk=userId)

Apparently get_user_model() returns a string, where get_object_or_404() expect a Django model as first parameter... Any idea?

When Django 1.10 comes out it looks like there will be some rendering, template, and context deprecations.

https://docs.djangoproject.com/en/1.9/internals/deprecation/#deprecation-removed-in-1-10

In Django 1.9 now and am getting warnings about this.

We now have several settings attributes (some of them are named confusingly or have an unexpected default).

I'm currently working on a solution to unify the naming and a better documentation in the readme.

Please feel free to make your suggestions.

• Add table of contents
• Add screenshot
• Add section with all configuration parameters
• Supported versions and build notice

It would be nice (even as an option?) to not update the users last login date when hijacking them. I would like to use/view the last login date as when a user last used the system, not when I was last pretending to be them.

• Note I also use django-allauth which may or may not be relevant

I was reading the documentation at https://django-hijack.readthedocs.org/en/latest/ and was expecting to see a link to the repo or "Edit on Github".

This looks like a great package. I was considering building something similar for our system to help with support.

I'd like to request a feature: Requiring a second user's credentials to initiate hijacking. The scenario is this: I don't want to hand out superuser status willy-nilly. Use the staff option to allow staff to hijack user accounts for support. However, they need an office-mate to authorize the session with them. That way if a single staff user's credentials are compromised, an attacker still won't be able to hijack user accounts.

Was there a reason these URLs are unamed?

https://github.com/arteria/django-hijack/blob/master/hijack/urls.py#L6-L8

If not I could whip up a PR with those changes.

https://github.com/arteria/django-hijack/blob/master/hijack/admin.py#L11-L23
Uses login_with_id and login_with_email

https://github.com/arteria/django-hijack/blob/ee19202fa24bb694141ebbea7ecc19626de3b847/hijack/urls.py#L16-L38
uses user_id

The readme settings sections specifies these as allowed values.
ALLOWED_HIJACKING_USER_ATTRIBUTES = ('user_id', 'email', 'username')

I can submit a PR with a fixed admin.py if we intend to follow the settings version.

Came across the issue when attempting to only use ids and having the admin functionality enabled.

Hijack loses authentification while
'django.contrib.auth.middleware.RemoteUserMiddleware' is in MIDDLEWARE_CLASSES

Standard fix navbars on bootstrap is block by the hijack notification.

I noticed that in the absence of HIJACK_NOTIFY_ADMIN in settings it defaults to False for the new conditional URL inclusion (disable_hijack_warning). According to the doc, in its absence it should default to True...

if getattr(settings, "HIJACK_NOTIFY_ADMIN", False):

• interpret LOGIN_REDIRECT_URL as a named url (cf django.shortcuts.resolve_url and its usage in the django login view : https://github.com/django/django/blob/002425fe39f62faafaa32e400f7531809181a1a0/django/contrib/auth/views.py#L48 )
• support for sites without the admin app : specifically, use the LOGIN_URL setting when decorating views with staff_member_required, because otherwise it looks for views in that namespace
• there's a method django.utils.http.is_safe_url to determine if a url is safe to use for redirection (instead of blindingly using request.GET["next"]), in login_user and disable_hijack_warning

• i18n of templates/hijack/notifications.html
• i18n of HijackUserAdminMixin
• specify steps on how to create/edit translations

It would be great if we could define a function to dynamically determine if a user can hijack another one:

def default_can_hijack(hijacker, hijacked):
    if hijacker.is_superuser:
        return True

    if hijacker.is_staff and settings.ALLOW_STAFF_TO_HIJACKUSER:
        if hijacked.is_staff and not settings.ALLOW_STAFF_TO_HIJACK_STAFF_USER:
            return False
        return True

    return False

It could deprecate some settings like ALLOW_STAFF_TO_HIJACKUSER and ALLOW_STAFF_TO_HIJACK_STAFF_USER (I find them cumbersome).

Setting ALLOW_STAFF_TO_HIJACKUSER to True allows staff members to hijack all the users except the superusers. I think it's also interesting to block hijacking other staff members since you otherwise would allow people to take over the account of other staff members. When people do this they could perform actions without taking the proper responsibility and the wrong people get the blame.

So my proposal is to add another settings which enables the permission to hijack other staff members.
This means that allowing staff members to hijack other staff members becomes opt-in and may require people to add an additional setting to the settings file after an upgrade since it is not enabled by setting ALLOW_STAFF_TO_HIJACKUSER to True any more.

What do you think about this idea?

(I can start working on this issue in the near future, but I'm asking your opinion in advance before opening a PR)

But the documentation doesn't mention this in Custom user models section

01a6035 produced an setup error for me.

  Running setup.py (path:/src/hijack-master/setup.py) egg_info for package hijack-master
    Traceback (most recent call last):
      File "<string>", line 17, in <module>
      File "/src/hijack-master/setup.py", line 48, in <module>
        description=read('DESCRIPTION'),
      File "/src/hijack-master/setup.py", line 41, in read
        encoding='utf8').read()
    TypeError: 'encoding' is an invalid keyword argument for this function
    Complete output from command python setup.py egg_info:
    Traceback (most recent call last):

  File "<string>", line 17, in <module>

  File "/src/hijack-master/setup.py", line 48, in <module>

    description=read('DESCRIPTION'),

  File "/src/hijack-master/setup.py", line 41, in read

    encoding='utf8').read()

• http://www.reddit.com/r/django/comments/39aggd/djangohijack_108_comes_with_a_custom_hijack/cs1z5y5

When merging PR #12 django-hijack should build on django-compat to provide Django LTS (1.4.x) to current (1.7.x) support.

I believe it's not appropriate for the Hijack URLs to respond to GET requests. Masquerade is a fairly heavy side-effect, and right now a malicious attacker could craft a URL, send it to an administrator, who might then be masquerading as another user without their knowledge. See https://docs.djangoproject.com/en/1.8/ref/csrf/

This account could potentially be under the control of a malicious user (think of a forum site where users can create their own accounts). The administrator might then be induced to enter a password or some other form of unsafe information without their knowledge.

This can be resolved by making the hijack endpoints require a POST request. This should have only minimal impact on the admin button, but might be a bit more significant in terms of impact elsewhere. One way to mitigate could be to make the hijack URLs show a warning and a Confirm button when visited via a GET request.

First, good job at fixing the previous issue, it's really nice to see that much reactivity.

I'm reporting another issue I noticed with the hijackNotification in template. This error only happen to me when I'm running my tests (if I recreate the condition locally for real it doesn't happen).

It seems that any test I write that include an assertRaises() will fail once the exception is raised because of the hijackNotification (I hope I'm being clear enough, it's kind of tricky). I'm not sure why hijackNotification is called because most of these tests involve ajax responses and doesn't use the templating stack at all...

So to resume, how to recreate the issue:

• activate Django-hijack and include {% load hijack_tags %} {{request|hijackNotification}} in your base template
• in your view, raise an exception like ValueError('something is wrong')
• write a test that expect the error: with self.assertRaises(ValueError): post_to_view

In my case, here is the error:

File "/Users/remi/.virtualenvs/sg/lib/python2.7/site-packages/hijack/templatetags/hijack_tags.py", line 12, in hijackNotification
    if getattr(settings, 'HIJACK_NOTIFY_ADMIN', True) and request.session.get('is_hijacked_user', False):
AttributeError: 'str' object has no attribute 'session'

Need:

As a maintainer of an application that uses hijack
In order to keep track of important errors
I need to not see errors generated by the misuse of the hijack endpoints

Solution:

Validate GET params before using them.
Apply solution to all endpoints.

from django.http import HttpResponseBadRequest

@staff_member_required
def login_with_id(request, userId):
    if isinstance(userId, int):
        return HttpResponseBadRequest('userId must be and integer')
    user = get_object_or_404(User, pk=userId)

Bug reenactment:

Accessing the user id endpoint with a string raises a ValueError with invalid literal for int() with base 10

@owner please confirm/change this spec as you see fit.