From ecb597e02e270f7c93b55bbed7a9c7d16e9113ae Mon Sep 17 00:00:00 2001 From: Benjamin Dauvergne Date: Thu, 27 Jun 2019 17:18:02 +0200 Subject: [PATCH 2/2] add new switch-user tool (#34308) --- .../templates/authentic2/manager/user_su.html | 15 ++++++ src/authentic2/manager/urls.py | 2 + src/authentic2/manager/user_views.py | 42 ++++++++++++--- src/authentic2/urls.py | 1 + src/authentic2/utils.py | 51 ++++++++++--------- src/authentic2/views.py | 25 ++++----- tests/conftest.py | 15 ++++-- tests/test_user_manager.py | 34 +++++++++++++ 8 files changed, 135 insertions(+), 50 deletions(-) create mode 100644 src/authentic2/manager/templates/authentic2/manager/user_su.html diff --git a/src/authentic2/manager/templates/authentic2/manager/user_su.html b/src/authentic2/manager/templates/authentic2/manager/user_su.html new file mode 100644 index 00000000..99842dc2 --- /dev/null +++ b/src/authentic2/manager/templates/authentic2/manager/user_su.html @@ -0,0 +1,15 @@ +{% extends "authentic2/manager/base.html" %} +{% load i18n %} + +{% block content %} +
+

+ {% blocktrans trimmed with fullname=user.get_full_name %} + To switch to user {{ fullname }}, use the following link. + {% endblocktrans %} +

+

+ {{ su_url }} +

+
+{% endblock %} diff --git a/src/authentic2/manager/urls.py b/src/authentic2/manager/urls.py index d64892bd..408f6b83 100644 --- a/src/authentic2/manager/urls.py +++ b/src/authentic2/manager/urls.py @@ -64,6 +64,8 @@ urlpatterns = required( url(r'^users/(?P\d+)/change-email/$', user_views.user_change_email, name='a2-manager-user-change-email'), + url(r'^users/(?P\d+)/su/$', user_views.su, + name='a2-manager-user-su'), # by uuid url(r'^users/uuid:(?P[a-z0-9]+)/$', user_views.user_detail, name='a2-manager-user-by-uuid-detail'), diff --git a/src/authentic2/manager/user_views.py b/src/authentic2/manager/user_views.py index b679d90b..019bbb95 100644 --- a/src/authentic2/manager/user_views.py +++ b/src/authentic2/manager/user_views.py @@ -21,19 +21,20 @@ import operator from django.db import models from django.utils.translation import ugettext_lazy as _, ugettext from django.utils.html import format_html +from django.core.exceptions import PermissionDenied from django.core.mail import EmailMultiAlternatives from django.template import loader from django.core.urlresolvers import reverse -from django.contrib.auth import get_user_model +from django.contrib.auth import get_user_model, REDIRECT_FIELD_NAME from django.contrib.contenttypes.models import ContentType from django.contrib import messages -from django.views.generic import FormView, TemplateView +from django.views.generic import FormView, TemplateView, DetailView from django.http import Http404, FileResponse import tablib from authentic2.models import Attribute, AttributeValue, PasswordReset -from authentic2.utils import switch_user, send_password_reset_mail, redirect, select_next_url +from authentic2.utils import build_su_url, send_password_reset_mail, redirect, select_next_url, make_url from authentic2.a2_rbac.utils import get_default_ou from authentic2 import hooks from django_rbac.utils import get_role_model, get_role_parenting_model, get_ou_model @@ -42,7 +43,7 @@ from django_rbac.utils import get_role_model, get_role_parenting_model, get_ou_m from .views import (BaseTableView, BaseAddView, BaseEditView, ActionMixin, OtherActionsMixin, Action, ExportMixin, BaseSubTableView, HideOUColumnMixin, BaseDeleteView, BaseDetailView, - PermissionMixin, MediaMixin) + TitleMixin, PermissionMixin, MediaMixin) from .tables import UserTable, UserRolesTable, OuUserRolesTable from .forms import (UserSearchForm, UserAddForm, UserEditForm, UserChangePasswordForm, ChooseUserRoleForm, @@ -52,6 +53,8 @@ from .resources import UserResource from .utils import get_ou_count, has_show_username from . import app_settings +User = get_user_model() + class UsersView(HideOUColumnMixin, BaseTableView): template_name = 'authentic2/manager/users.html' @@ -240,7 +243,8 @@ class UserDetailView(OtherActionsMixin, BaseDetailView): url_name='a2-manager-user-change-password', permission='custom_user.change_password_user') if self.request.user.is_superuser: - yield Action('switch_user', _('Impersonate this user')) + yield Action('su', _('Impersonate this user'), + url_name='a2-manager-user-su') if self.object.ou and self.object.ou.validate_emails: yield Action('change_email', _('Change user email'), url_name='a2-manager-user-change-email', @@ -274,8 +278,9 @@ class UserDetailView(OtherActionsMixin, BaseDetailView): def action_delete_password_reset(self, request, *args, **kwargs): PasswordReset.objects.filter(user=self.object).delete() - def action_switch_user(self, request, *args, **kwargs): - return switch_user(request, self.object) + def action_su(self, request, *args, **kwargs): + return redirect(request, 'auth_logout', + params={REDIRECT_FIELD_NAME: build_su_url(self.object)}) # Copied from PasswordResetForm implementation def send_mail(self, subject_template_name, email_template_name, @@ -752,3 +757,26 @@ class UserImportReportView(MediaMixin, PermissionMixin, TemplateView): return ctx user_import_report = UserImportReportView.as_view() + + +class UserSuView(TitleMixin, PermissionMixin, DetailView): + model = User + template_name = 'authentic2/manager/user_su.html' + title = _('Su user') + + def dispatch(self, request, *args, **kwargs): + if not request.user.is_superuser: + raise PermissionDenied + return super(UserSuView, self).dispatch(request, *args, **kwargs) + + def get_context_data(self, **kwargs): + ctx = super(UserSuView, self).get_context_data(**kwargs) + ctx['su_url'] = make_url( + 'auth_logout', + + params={REDIRECT_FIELD_NAME: build_su_url(self.object)}, + request=self.request, + absolute=True) + return ctx + +su = UserSuView.as_view() diff --git a/src/authentic2/urls.py b/src/authentic2/urls.py index 7d3df248..7c5d9754 100644 --- a/src/authentic2/urls.py +++ b/src/authentic2/urls.py @@ -105,6 +105,7 @@ urlpatterns = [ url(r'^$', views.homepage, name='auth_homepage'), url(r'^login/$', views.login, name='auth_login'), url(r'^logout/$', views.logout, name='auth_logout'), + url(r'^su/(?P[a-f0-9]+)/$', views.su, name='su'), url(r'^accounts/', include(accounts_urlpatterns)), url(r'^admin/', include(admin.site.urls)), url(r'^idp/', include('authentic2.idp.urls')), diff --git a/src/authentic2/utils.py b/src/authentic2/utils.py index 761975ec..8b5d8d62 100644 --- a/src/authentic2/utils.py +++ b/src/authentic2/utils.py @@ -22,6 +22,7 @@ import uuid import datetime import copy import ctypes +import re from functools import wraps from itertools import islice, chain, count @@ -821,32 +822,32 @@ def to_dict_of_set(d): return dict((k, set(v)) for k, v in d.items()) -def switch_user(request, new_user): - '''Switch to another user and remember currently logged in user in the - session. Reserved to superusers.''' +def build_su_url(user, duration=30): + token = get_hex_uuid() + data = {'user_pk': user.pk} + cache.set('switch-%s' % token, data, duration) + return make_url('su', kwargs={'token': token}) - logger = logging.getLogger(__name__) - if constants.SWITCH_USER_SESSION_KEY in request.session: - messages.error(request, _('Your user is already switched, go to your ' - 'account page and come back to your original ' - 'user to do it again.')) - else: - if not request.user.is_superuser: - raise PermissionDenied - switched = {} - for key in (SESSION_KEY, BACKEND_SESSION_KEY, HASH_SESSION_KEY, - constants.LAST_LOGIN_SESSION_KEY): - switched[key] = request.session[key] - user = authenticate(user=new_user) - login(request, user, 'switch') - request.session[constants.SWITCH_USER_SESSION_KEY] = switched - if constants.LAST_LOGIN_SESSION_KEY not in request.session: - request.session[constants.LAST_LOGIN_SESSION_KEY] = \ - localize(to_current_timezone(new_user.last_login), True) - messages.info(request, _('Successfully switched to user %s') % - new_user.get_full_name()) - logger.info(u'switched to user %s', new_user) - return continue_to_next_url(request) +HEX_RE = re.compile('^[a-f0-9]+$') + + +def get_su_user(token): + User = get_user_model() + if not token: + return None + if not HEX_RE.match(token): + return None + key = 'switch-%s' % token + data = cache.get(key) + if not isinstance(data, dict): + return None + if not data.get('user_pk'): + return None + cache.delete(key) + try: + return User.objects.get(pk=data['user_pk']) + except User.DoesNotExist: + return None def datetime_to_utc(dt): diff --git a/src/authentic2/views.py b/src/authentic2/views.py index d9c40e09..d0940aae 100644 --- a/src/authentic2/views.py +++ b/src/authentic2/views.py @@ -163,21 +163,6 @@ edit_profile = decorators.setting_enabled('A2_PROFILE_CAN_EDIT_PROFILE')( login_required(EditProfile.as_view())) -def su(request, username, redirect_url='/'): - '''To use this view add: - - url(r'^su/(?P.*)/$', 'authentic2.views.su', {'redirect_url': '/'}), - ''' - if request.user.is_superuser or request.session.get('has_superuser_power'): - su_user = shortcuts.get_object_or_404(User, username=username) - if su_user.is_active: - request.session[SESSION_KEY] = su_user.id - request.session['has_superuser_power'] = True - return http.HttpResponseRedirect(redirect_url) - else: - return http.HttpResponseRedirect('/') - - class EmailChangeView(cbv.TemplateNamesMixin, FormView): template_names = [ 'profiles/email_change.html', @@ -1163,3 +1148,13 @@ password_change.do_not_call_in_templates = True def notimplemented_view(request): raise NotImplementedError + + +class SuView(View): + def get(self, request, token): + user = utils.get_su_user(token) + if not user: + raise Http404 + return utils.simulate_authentication(request, user, 'su') + +su = SuView.as_view() diff --git a/tests/conftest.py b/tests/conftest.py index c5ecca05..29eb89b4 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -38,11 +38,20 @@ Role = get_role_model() @pytest.fixture -def app(request): +def app_factory(): wtm = django_webtest.WebTestMixin() wtm._patch_settings() - request.addfinalizer(wtm._unpatch_settings) - return django_webtest.DjangoTestApp(extra_environ={'HTTP_HOST': 'localhost'}) + try: + def factory(hostname='localhost'): + return django_webtest.DjangoTestApp(extra_environ={'HTTP_HOST': hostname}) + yield factory + finally: + wtm._unpatch_settings() + + +@pytest.fixture +def app(app_factory): + return app_factory() @pytest.fixture diff --git a/tests/test_user_manager.py b/tests/test_user_manager.py index 29e822da..5a98b778 100644 --- a/tests/test_user_manager.py +++ b/tests/test_user_manager.py @@ -316,3 +316,37 @@ x,x,x,x'''.encode(encoding), app.get('/manage/users/import/%s/' % _import.uuid, status=403) app.get('/manage/users/import/%s/%s/' % (_import.uuid, simulate.uuid), status=403) app.get('/manage/users/import/%s/%s/' % (_import.uuid, execute.uuid), status=403) + + +def test_su_permission(app, admin, simple_user): + resp = login(app, admin, '/manage/users/%s/' % simple_user.pk) + assert len(resp.pyquery('button[name="su"]')) == 0 + assert app.get('/manage/users/%s/su/' % simple_user.pk, status=403) + + +def test_su_superuser_post(app, app_factory, superuser, simple_user): + resp = login(app, superuser, '/manage/users/%s/' % simple_user.pk) + assert len(resp.pyquery('button[name="su"]')) == 1 + su_resp = resp.form.submit(name='su') + + new_app = app_factory() + new_app.get(su_resp.location).maybe_follow() + assert new_app.session['_auth_user_id'] == str(simple_user.pk) + + +def test_su_superuser_dialog(app, app_factory, superuser, simple_user): + resp = login(app, superuser, '/manage/users/%s/' % simple_user.pk) + assert len(resp.pyquery('button[name="su"]')) == 1 + + su_view_url = resp.pyquery('button[name="su"]')[0].get('data-url') + + resp = app.get(su_view_url) + + anchors = resp.pyquery('a#su-link') + assert len(anchors) == 1 + + su_url = anchors[0].get('href') + + new_app = app_factory() + new_app.get(su_url).maybe_follow() + assert new_app.session['_auth_user_id'] == str(simple_user.pk) -- 2.20.1