diff --git a/oioioi/contests/controllers.py b/oioioi/contests/controllers.py index 7c0df0758..48c1bc6b3 100644 --- a/oioioi/contests/controllers.py +++ b/oioioi/contests/controllers.py @@ -6,7 +6,7 @@ from django.core.exceptions import PermissionDenied from django.core.mail import EmailMessage from django.db import transaction -from django.db.models import Q +from django.db.models import Subquery from django.template.loader import render_to_string from django.urls import reverse from django.utils.safestring import mark_safe @@ -295,8 +295,7 @@ def filter_participants(self, queryset): def filter_users_with_accessible_personal_data(self, queryset): submissions = Submission.objects.filter(problem_instance__contest=self.contest) - authors = [s.user for s in submissions] - return [q for q in queryset if q in authors] + return queryset.filter(id__in=Subquery(submissions.values('user_id'))) class ContestControllerContext(object): diff --git a/oioioi/deployment/settings.py.template b/oioioi/deployment/settings.py.template index 8e20cea92..8888f7e85 100755 --- a/oioioi/deployment/settings.py.template +++ b/oioioi/deployment/settings.py.template @@ -581,3 +581,9 @@ ZEUS_INSTANCES = { # Experimental # USE_ACE_EDITOR = False + +# If set to True, contest admins will be able to log in as participants of contests they admin. +CONTEST_ADMINS_CAN_SU = False + +# If set to False, contest admins switched to a participant will be able to make any type of request. +ALLOW_ONLY_GET_FOR_SU_CONTEST_ADMINS = True diff --git a/oioioi/su/README.rst b/oioioi/su/README.rst index 094f13792..f2be3722c 100644 --- a/oioioi/su/README.rst +++ b/oioioi/su/README.rst @@ -1,2 +1,6 @@ An option for admins to log in as other user for testing purposes. + +You can enable su for contest admins with ``CONTEST_ADMINS_CAN_SU`` option set to ``True`` in ``settings.py``. By default +this option is disabled. You can allow contest admins using su to make other request than `GET` by setting +``ALLOW_ONLY_GET_FOR_SU_CONTEST_ADMINS`` to ``True`` (default ``False``). diff --git a/oioioi/su/__init__.py b/oioioi/su/__init__.py index 1d6266649..f66d8d2fd 100644 --- a/oioioi/su/__init__.py +++ b/oioioi/su/__init__.py @@ -13,3 +13,13 @@ SU_UID_SESSION_KEY = 'su_effective_user_id' SU_BACKEND_SESSION_KEY = 'su_effective_backend' +SU_REAL_USER_IS_SUPERUSER = 'su_real_user_is_superuser' +SU_ORIGINAL_CONTEST = 'su_original_contest' + +BLOCKED_URLS = [ + 'api_token', 'api_regenerate_key', + 'submitservice_view_user_token', 'submitservice_clear_user_token', + 'edit_profile', 'delete_profile', + 'auth_password_change', 'auth_password_done', +] +BLOCKED_URL_NAMESPACES = ['two_factor', 'oioioiadmin'] diff --git a/oioioi/su/middleware.py b/oioioi/su/middleware.py index 15ef020cc..f2cca60bf 100644 --- a/oioioi/su/middleware.py +++ b/oioioi/su/middleware.py @@ -1,10 +1,20 @@ from django.core.exceptions import ImproperlyConfigured, PermissionDenied from django.http.response import HttpResponseForbidden from django.shortcuts import redirect +from django.conf import settings +from django.urls import resolve from oioioi.base.utils.middleware import was_response_generated_by_exception -from oioioi.su import SU_BACKEND_SESSION_KEY, SU_UID_SESSION_KEY -from oioioi.su.utils import get_user +from oioioi.contests.current_contest import contest_re +from oioioi.su import ( + SU_BACKEND_SESSION_KEY, + SU_UID_SESSION_KEY, + SU_REAL_USER_IS_SUPERUSER, + SU_ORIGINAL_CONTEST, + BLOCKED_URL_NAMESPACES, + BLOCKED_URLS +) +from oioioi.su.utils import get_user, reset_to_real_user REDIRECTION_AFTER_SU_KEY = "redirection_after_su" @@ -20,7 +30,9 @@ def __init__(self, get_response): self.get_response = get_response def __call__(self, request): - self._process_request(request) + response = self._process_request(request) + if response: + return response return self.get_response(request) @@ -39,6 +51,45 @@ def _process_request(self, request): request.session[SU_UID_SESSION_KEY], request.session[SU_BACKEND_SESSION_KEY], ) + # Check if the user is contest admin. + if not request.session.get(SU_REAL_USER_IS_SUPERUSER, True): + # Might happen when switching to a user which then becomes a superuser. + if request.user.is_superuser: + reset_to_real_user(request) + return redirect('index') + + original_contest_id = request.session.get(SU_ORIGINAL_CONTEST) + contest_id = None + m = contest_re.match(request.path) + if m is not None: + contest_id = m.group('c_name') + url = resolve(request.path_info) + is_su_reset_url = url.url_name == 'su_reset' + nonoioioi_namespace = url.namespaces == [] + for ns in url.namespaces: + if ns != 'contest' and ns != 'noncontest': + nonoioioi_namespace = True + break + # Redirect if the url is not in the same contest, is not a su reset url and is an url made by oioioi. + # For example, `nonoioioi_namespace` can be True when the url is /jsi18n/ + if ( + not is_su_reset_url and + not nonoioioi_namespace and + (contest_id is None or contest_id != original_contest_id) + ): + return redirect('su_url_not_allowed', contest_id=original_contest_id) + + for ns in url.namespaces: + if ns in BLOCKED_URL_NAMESPACES: + return redirect('su_url_not_allowed', contest_id=original_contest_id) + if url.url_name in BLOCKED_URLS: + return redirect('su_url_not_allowed', contest_id=original_contest_id) + + if ( + not is_su_reset_url and + getattr(settings, 'ALLOW_ONLY_GET_FOR_SU_CONTEST_ADMINS', True) and request.method != 'GET' + ): + return redirect('su_method_not_allowed', contest_id=original_contest_id) class SuFirstTimeRedirectionMiddleware(object): diff --git a/oioioi/su/templates/su/method-not-allowed.html b/oioioi/su/templates/su/method-not-allowed.html new file mode 100644 index 000000000..6d2781462 --- /dev/null +++ b/oioioi/su/templates/su/method-not-allowed.html @@ -0,0 +1,20 @@ +{% extends "simple-centered.html" %} + +{% load i18n %} + +{% block title %}{% trans "SU - Method not allowed" %}{% endblock %} + +{% block content %} + +
+

+ {% blocktrans %} + Sorry, this OIOIOI instance only allows GET requests for contest admins using su. + {% endblocktrans %} +

+ + {% trans "Return to the contest" %} + +
+ +{% endblock %} diff --git a/oioioi/su/templates/su/navbar-su-form.html b/oioioi/su/templates/su/navbar-su-form.html index e20c8caa5..da8511cb1 100644 --- a/oioioi/su/templates/su/navbar-su-form.html +++ b/oioioi/su/templates/su/navbar-su-form.html @@ -1,6 +1,6 @@ {% load i18n simple_filters %} -{% if ctx.user.is_superuser %} +{% if ctx.user.is_superuser or is_valid_contest_admin %}
{% csrf_token %} diff --git a/oioioi/su/templates/su/url-not-allowed.html b/oioioi/su/templates/su/url-not-allowed.html new file mode 100644 index 000000000..253f08021 --- /dev/null +++ b/oioioi/su/templates/su/url-not-allowed.html @@ -0,0 +1,20 @@ +{% extends "simple-centered.html" %} + +{% load i18n %} + +{% block title %}{% trans "SU - URL not allowed" %}{% endblock %} + +{% block content %} + +
+

+ {% blocktrans %} + Sorry, you can't access this page. + {% endblocktrans %} +

+ + {% trans "Return to the contest" %} + +
+ +{% endblock %} diff --git a/oioioi/su/templatetags/get_su.py b/oioioi/su/templatetags/get_su.py index a2b7f93dc..31f3a498f 100644 --- a/oioioi/su/templatetags/get_su.py +++ b/oioioi/su/templatetags/get_su.py @@ -7,11 +7,22 @@ @register.inclusion_tag('su/navbar-su-form.html', takes_context=True) def su_dropdown_form(context): from oioioi.su.forms import SuForm - from oioioi.su.utils import is_under_su + from oioioi.contests.utils import is_contest_basicadmin, contest_exists + from oioioi.su.utils import is_under_su, can_contest_admins_su, is_real_superuser + + request = context['request'] + # Checking if real user is a superuser blocks the ability to switch when switched to contest admin. + is_valid_contest_admin = ( + can_contest_admins_su(request) and + contest_exists(request) and + is_contest_basicadmin(request) and + not is_real_superuser(request) + ) return { 'ctx': context, 'form': SuForm(auto_id='su-%s'), 'is_under_su': is_under_su(context['request']), 'num_hints': getattr(settings, 'NUM_HINTS', 10), + 'is_valid_contest_admin': is_valid_contest_admin, } diff --git a/oioioi/su/tests.py b/oioioi/su/tests.py index 0c015e028..9e7d96612 100644 --- a/oioioi/su/tests.py +++ b/oioioi/su/tests.py @@ -6,7 +6,9 @@ from oioioi.base.tests import TestCase from oioioi.contests.current_contest import ContestMode -from oioioi.su import SU_BACKEND_SESSION_KEY, SU_UID_SESSION_KEY +from oioioi.contests.models import Contest +from oioioi.participants.models import Participant +from oioioi.su import SU_BACKEND_SESSION_KEY, SU_UID_SESSION_KEY, SU_REAL_USER_IS_SUPERUSER, SU_ORIGINAL_CONTEST from oioioi.su.utils import get_user, su_to_user @@ -216,3 +218,189 @@ def test_su_status(self): self.assertEqual(False, response['is_superuser']) self.assertEqual('test_admin', response['real_user']) self.assertEqual('test_user', response['user']) + + +@override_settings(CONTEST_ADMINS_CAN_SU=True) +class TestContestAdminsSu(TestCase): + fixtures = ['test_users', 'test_contest', 'test_permissions'] + + def setUp(self): + super().setUp() + c = Contest.objects.get() + # Use a controller with participant registration. + c.controller_name = 'oioioi.oi.controllers.OIOnsiteContestController' + c.save() + + def _test_su_visibility(self, contest, expected): + self.assertTrue(self.client.login(username='test_contest_basicadmin')) + url = reverse('contest_dashboard', kwargs={'contest_id': contest.id}) + response = self.client.get(url, follow=True) + self.assertEqual(200, response.status_code) + self.assertEqual(expected, 'Login as user' in response.content.decode()) + + def _add_user_to_contest(self, username): + contest = Contest.objects.get() + p = Participant() + p.user = User.objects.get(username=username) + p.contest = contest + p.save() + + def _do_su(self, username, backend, expected_fail, fail_code=400): + contest = Contest.objects.get() + user = User.objects.get(username=username) + response = self.client.post( + reverse('su', kwargs={'contest_id': contest.id}), + { + 'user': username, + 'backend': backend, + } + ) + if expected_fail: + self.assertEqual(fail_code, response.status_code) + else: + self.assertEqual(302, response.status_code) + session = self.client.session + self.assertEqual(user.id, session[SU_UID_SESSION_KEY]) + self.assertEqual(backend, session[SU_BACKEND_SESSION_KEY]) + self.assertFalse(session[SU_REAL_USER_IS_SUPERUSER]) + self.assertEqual(contest.id, session[SU_ORIGINAL_CONTEST]) + + @override_settings(CONTEST_ADMINS_CAN_SU=False) + def test_su_unavailable(self): + contest = Contest.objects.get() + self._test_su_visibility(contest, False) + + def test_su_available(self): + contest = Contest.objects.get() + self._test_su_visibility(contest, True) + + def test_users_list(self): + # Tests if contest admin can only see hints with participants of the contest. + self.assertTrue(self.client.login(username='test_contest_basicadmin')) + contest = Contest.objects.get() + self._add_user_to_contest('test_user') + + response = self.client.get( + reverse('get_suable_users', kwargs={'contest_id': contest.id}), + {'substr': 'te'} + ) + response = response.json() + self.assertListEqual( + [ + 'test_user (Test User)', + ], + response, + ) + + def test_su(self): + self.assertTrue(self.client.login(username='test_contest_basicadmin')) + contest = Contest.objects.get() + self._add_user_to_contest('test_user') + + # Should fail because this user is not in the contest. + self._do_su('test_user2', 'django.contrib.auth.backends.ModelBackend', True) + + # Should work because this user is in the contest. + self._do_su('test_user', 'django.contrib.auth.backends.ModelBackend', False) + + # Su-ed contest admin shouldn't be able to go to urls outside the contest. + second_contest = Contest.objects.create( + id='c2', + controller_name='oioioi.programs.controllers.ProgrammingContestController', + name='Test contest', + ) + urls = [ + reverse('contest_dashboard', kwargs={'contest_id': second_contest.id}), + reverse('select_contest', kwargs={'contest_id': None}), + ] + for url in urls: + response = self.client.get(url) + self.assertEqual(302, response.status_code) + self.assertEqual( + reverse('su_url_not_allowed', kwargs={'contest_id': contest.id}), + response.url + ) + + # Contest admin should be able to reset su. + response = self.client.post(reverse('su_reset')) + self.assertEqual(302, response.status_code) + session = self.client.session + self.assertNotIn(SU_UID_SESSION_KEY, session) + self.assertNotIn(SU_BACKEND_SESSION_KEY, session) + self.assertNotIn(SU_REAL_USER_IS_SUPERUSER, session) + self.assertNotIn(SU_ORIGINAL_CONTEST, session) + + # The user is not a contest admin in the second contest. + self._test_su_visibility(second_contest, False) + + def _test_post(self, can_post): + self.assertTrue(self.client.login(username='test_contest_basicadmin')) + self._add_user_to_contest('test_user') + self._do_su('test_user', 'django.contrib.auth.backends.ModelBackend', False) + contest = Contest.objects.get() + response = self.client.post(reverse('contest_dashboard', kwargs={'contest_id': contest.id})) + if can_post: + self.assertEqual(200, response.status_code) + else: + self.assertEqual(302, response.status_code) + self.assertEqual( + reverse('su_method_not_allowed', kwargs={'contest_id': contest.id}), + response.url + ) + + @override_settings(ALLOW_ONLY_GET_FOR_SU_CONTEST_ADMINS=True) + def test_cant_post(self): + self._test_post(False) + + @override_settings(ALLOW_ONLY_GET_FOR_SU_CONTEST_ADMINS=False) + def test_can_post(self): + self._test_post(True) + + def test_blocked_accounts(self): + # Tests if contest admins can't su to superusers and other contest admins. + self.assertTrue(self.client.login(username='test_contest_basicadmin')) + contest = Contest.objects.get() + self._add_user_to_contest('test_admin') + self._add_user_to_contest('test_contest_admin') + + for username in ['test_admin', 'test_contest_admin']: + response = self.client.get( + reverse('get_suable_users', kwargs={'contest_id': contest.id}), + {'substr': username[:2]} + ) + response = response.json() + self.assertNotIn(username, response) + + self._do_su('test_contest_admin', 'django.contrib.auth.backends.ModelBackend', True) + + # Shows the su form with an error message that switching to superuser is forbidden. + self._do_su('test_admin', 'django.contrib.auth.backends.ModelBackend', True, 200) + session = self.client.session + self.assertNotIn(SU_UID_SESSION_KEY, session) + self.assertNotIn(SU_BACKEND_SESSION_KEY, session) + self.assertNotIn(SU_REAL_USER_IS_SUPERUSER, session) + self.assertNotIn(SU_ORIGINAL_CONTEST, session) + + def superusers_cant_su(self): + # Tests if superusers switched to contest admins can't switch to other contest admins. + self.assertTrue(self.client.login(username='test_admin')) + contest = Contest.objects.get() + self._add_user_to_contest('test_contest_admin') + self._add_user_to_contest('test_contest_basicadmin') + self._do_su('test_contest_admin', 'django.contrib.auth.backends.ModelBackend', False) + self._do_su('test_contest_basicadmin', 'django.contrib.auth.backends.ModelBackend', True) + + def user_becomes_superuser(self): + # Test if being switched to a user which then becomes a superuser resets the user. + self.assertTrue(self.client.login(username='test_contest_basicadmin')) + contest = Contest.objects.get() + self._add_user_to_contest('test_user') + self._do_su('test_user', 'django.contrib.auth.backends.ModelBackend', False) + + user = User.objects.get(username='test_user') + user.is_superuser = True + user.save() + + response = self.client.get(reverse('contest_dashboard', kwargs={'contest_id': contest.id})) + self.assertEqual(302, response.status_code) + self.assertEqual(reverse('index'), response.url) diff --git a/oioioi/su/urls.py b/oioioi/su/urls.py index 112488ec6..ef7267b33 100644 --- a/oioioi/su/urls.py +++ b/oioioi/su/urls.py @@ -4,10 +4,15 @@ app_name = 'su' -noncontest_patterns = [ +urlpatterns = [ re_path(r'^su/$', views.su_view, name='su'), re_path( r'^get_suable_usernames/', views.get_suable_users_view, name='get_suable_users' ), re_path(r'^su_reset/$', views.su_reset_view, name='su_reset'), ] + +contest_patterns = [ + re_path(r'^su_method_not_allowed/', views.method_not_allowed_view, name='su_method_not_allowed'), + re_path(r'^su_url_not_allowed/', views.url_not_allowed_view, name='su_url_not_allowed'), +] diff --git a/oioioi/su/utils.py b/oioioi/su/utils.py index 09380a331..e7163ceea 100644 --- a/oioioi/su/utils.py +++ b/oioioi/su/utils.py @@ -1,7 +1,9 @@ from django.contrib import auth +from django.conf import settings from oioioi.base.permissions import is_superuser, make_request_condition -from oioioi.su import SU_BACKEND_SESSION_KEY, SU_UID_SESSION_KEY +from oioioi.contests.utils import is_contest_basicadmin, contest_exists +from oioioi.su import SU_BACKEND_SESSION_KEY, SU_UID_SESSION_KEY, SU_REAL_USER_IS_SUPERUSER, SU_ORIGINAL_CONTEST @make_request_condition @@ -17,6 +19,11 @@ def is_under_su(request): return SU_UID_SESSION_KEY in request.session +@make_request_condition +def can_contest_admins_su(_): + return getattr(settings, 'CONTEST_ADMINS_CAN_SU', False) + + def get_user(request, user_id, backend_path): backend = auth.load_backend(backend_path) user = backend.get_user(user_id) @@ -37,6 +44,9 @@ def su_to_user(request, user, backend_path=None): request.session[SU_UID_SESSION_KEY] = user.id request.session[SU_BACKEND_SESSION_KEY] = backend_path + request.session[SU_REAL_USER_IS_SUPERUSER] = is_superuser(request) + if not is_superuser(request) and contest_exists(request) and is_contest_basicadmin(request): + request.session[SU_ORIGINAL_CONTEST] = request.contest.id request.real_user = request.user request.user = get_user(request, user.id, backend_path) @@ -48,3 +58,7 @@ def reset_to_real_user(request): del request.session[SU_UID_SESSION_KEY] del request.session[SU_BACKEND_SESSION_KEY] + if SU_REAL_USER_IS_SUPERUSER in request.session: + del request.session[SU_REAL_USER_IS_SUPERUSER] + if SU_ORIGINAL_CONTEST in request.session: + del request.session[SU_ORIGINAL_CONTEST] diff --git a/oioioi/su/views.py b/oioioi/su/views.py index 72bd041ba..73bcdfcf6 100644 --- a/oioioi/su/views.py +++ b/oioioi/su/views.py @@ -9,6 +9,9 @@ from oioioi.base.permissions import enforce_condition, is_superuser from oioioi.base.utils.redirect import safe_redirect from oioioi.base.utils.user_selection import get_user_hints_view +from oioioi.contests.models import ContestPermission +from oioioi.contests.utils import is_contest_basicadmin, contest_exists, can_admin_contest +from oioioi.participants.models import Participant from oioioi.status.registry import status_registry from oioioi.su.forms import SuForm from oioioi.su.middleware import REDIRECTION_AFTER_SU_KEY @@ -17,10 +20,11 @@ is_under_su, reset_to_real_user, su_to_user, + can_contest_admins_su, ) -@enforce_condition(is_superuser) +@enforce_condition(is_superuser | (can_contest_admins_su & contest_exists & is_contest_basicadmin)) @require_POST def su_view(request, next_page=None, redirect_field_name=REDIRECT_FIELD_NAME): form = SuForm(request.POST) @@ -39,6 +43,23 @@ def su_view(request, next_page=None, redirect_field_name=REDIRECT_FIELD_NAME): if is_under_su(request): raise SuspiciousOperation + # Don't allow contest admins to switch to users that are not participants of the contest + if ( + not is_superuser(request) and + request.contest.controller.registration_controller().filter_users_with_accessible_personal_data( + User.objects.filter(id=user.id) + ).count() == 0 + ): + raise SuspiciousOperation + + # Don't allow contest admins to switch to other contest admins + if not is_superuser(request) and can_admin_contest(user, request.contest): + raise SuspiciousOperation + + # Don't allow real superusers switched to contest admins to switch to other contest admins + if is_real_superuser(request) and can_admin_contest(user, request.contest): + raise SuspiciousOperation + su_to_user(request, user, form.cleaned_data['backend']) if redirect_field_name in request.GET: @@ -63,9 +84,27 @@ def su_reset_view(request, next_page=None, redirect_field_name=REDIRECT_FIELD_NA return safe_redirect(request, next_page) -@enforce_condition(is_superuser) +@enforce_condition(is_superuser | (can_contest_admins_su & contest_exists & is_contest_basicadmin)) def get_suable_users_view(request): - users = User.objects.filter(is_superuser=False, is_active=True) + if not is_superuser(request): + cps = ContestPermission.objects.filter( + contest=request.contest, + permission__in=[ + "contests.contest_owner", + "contests.contest_admin", + "contests.contest_basicadmin", + ] + ).select_related("user") + contest_admins = [cp.user.id for cp in cps] + + users = request.contest.controller.registration_controller().filter_users_with_accessible_personal_data( + User.objects.filter( + is_active=True, + is_superuser=False, + ).exclude(id__in=contest_admins) + ) + else: + users = User.objects.filter(is_superuser=False, is_active=True) return get_user_hints_view(request, 'substr', users) @@ -78,3 +117,11 @@ def get_su_status(request, response): response['sync_time'] = min(10000, response.get('sync_time', 10000)) return response + + +def method_not_allowed_view(request): + return TemplateResponse(request, 'su/method-not-allowed.html') + + +def url_not_allowed_view(request): + return TemplateResponse(request, 'su/url-not-allowed.html')