diff --git a/web/guac/test_guac_http_auth.py b/web/guac/test_guac_http_auth.py new file mode 100644 index 00000000000..bc79780b767 --- /dev/null +++ b/web/guac/test_guac_http_auth.py @@ -0,0 +1,71 @@ +"""Regression tests for guac-web backend-agnostic HTTP auth (guac.views.guac_login_required). + +Companion to test_channels_auth.py. The websocket path already resolves identity WITHOUT +loading the session's auth backend, but the HTTP console views (guac/views.py) used Django's +stock ``login_required`` -> ``auth.get_user()``, which ``load_backend()``s the session's exact +backend. For an OIDC/allauth session that backend is not in guac-web's AUTHENTICATION_BACKENDS +(guac_settings deliberately does not install the allauth app stack), so ``get_user()`` returned +AnonymousUser and every interactive-console request for an OIDC user was bounced to the login +page -- even though the browser was logged in on the main web. ``guac_login_required`` must +resolve the same User backend-agnostically and redirect only a genuinely anonymous session. +""" +import pytest +from django.contrib.auth import BACKEND_SESSION_KEY, HASH_SESSION_KEY, SESSION_KEY +from django.contrib.auth.models import User +from django.http import HttpResponse +from django.test import RequestFactory + +from guac.views import guac_login_required + + +def _view(request): + return HttpResponse(getattr(request.user, "username", "")) + + +@pytest.mark.django_db +def test_guac_login_required_resolves_allauth_session_without_backend(): + """An OIDC/allauth-backed session -- the exact backend guac-web cannot load -- must + resolve to the real User over HTTP, with no login redirect.""" + u = User.objects.create_user("consoleuser", password="pw-12345") + request = RequestFactory().get("/guac/1/abc/") + request.session = { + SESSION_KEY: str(u.pk), + BACKEND_SESSION_KEY: "allauth.account.auth_backends.AuthenticationBackend", + HASH_SESSION_KEY: u.get_session_auth_hash(), + } + response = guac_login_required(_view)(request) + assert response.status_code == 200 + assert response.content == b"consoleuser" + assert request.user == u + + +@pytest.mark.django_db +def test_guac_login_required_resolves_modelbackend_session(): + """Local/superuser (ModelBackend) sessions keep working -- non-OIDC / single-node parity.""" + u = User.objects.create_user("localadmin", password="pw-12345") + request = RequestFactory().get("/guac/1/abc/") + request.session = { + SESSION_KEY: str(u.pk), + BACKEND_SESSION_KEY: "django.contrib.auth.backends.ModelBackend", + HASH_SESSION_KEY: u.get_session_auth_hash(), + } + response = guac_login_required(_view)(request) + assert response.status_code == 200 + assert request.user == u + + +@pytest.mark.django_db +def test_guac_login_required_redirects_anonymous(): + """No/invalid session identity -> redirect to the login page, exactly like login_required + (fails closed: an unresolved session never reaches the console view).""" + request = RequestFactory().get("/guac/1/abc/") + request.session = {} # anonymous + response = guac_login_required(_view)(request) + assert response.status_code == 302 + + # a tampered/stale auth-hash must also fail closed (not just a missing session) + u = User.objects.create_user("staleuser", password="pw-12345") + request = RequestFactory().get("/guac/1/abc/") + request.session = {SESSION_KEY: str(u.pk), HASH_SESSION_KEY: "0" * 64} + response = guac_login_required(_view)(request) + assert response.status_code == 302 diff --git a/web/guac/views.py b/web/guac/views.py index 0658aebd944..6fc0aef04e0 100644 --- a/web/guac/views.py +++ b/web/guac/views.py @@ -4,11 +4,15 @@ import json import logging +from functools import wraps + from django.conf import settings -from django.contrib.auth.decorators import login_required +from django.contrib.auth.views import redirect_to_login from django.http import JsonResponse from django.shortcuts import render, redirect +from guac.channels_auth import resolve_session_user + from lib.cuckoo.common.config import Config from lib.cuckoo.core.database import Database from web.tenancy_optional import can_manage_task, multitenancy_config, viewer_for @@ -26,6 +30,33 @@ def __call__(self, func): return func return self.decorator(func) + +def guac_login_required(view_func): + """Backend-agnostic ``@login_required`` for guac-web's HTTP views. + + guac-web (``web.guac_settings``) shares the main web's session store but deliberately + does NOT install the allauth app stack. Django's stock ``login_required`` resolves + ``request.user`` via ``auth.get_user()``, which ``load_backend()``s the exact auth + backend the session was created with; for an OIDC/allauth session that backend is not + in guac-web's ``AUTHENTICATION_BACKENDS`` (importing ``allauth.account.*`` is exactly + what guac_settings avoids), so ``get_user()`` returns ``AnonymousUser`` and every + interactive-console request for an OIDC user is bounced to the login page — even though + the browser is logged in on the main web. The websocket path already solved this with a + backend-agnostic resolver (``guac.channels_auth.resolve_session_user``); the HTTP views + must do the same. Resolve identity straight from the session (user id + auth-hash check, + the same security gate ``get_user`` enforces) without loading the backend. Redirect to + the login page only for a genuinely anonymous session, identical to ``login_required``. + """ + @wraps(view_func) + def _wrapped(request, *args, **kwargs): + user = resolve_session_user(request.session) + if not user.is_authenticated: + return redirect_to_login(request.get_full_path()) + request.user = user + return view_func(request, *args, **kwargs) + + return _wrapped + try: import libvirt LIBVIRT_AVAILABLE = True @@ -52,7 +83,7 @@ def _error(request, task_id, msg): }) -@conditional_login_required(login_required, settings.WEB_AUTHENTICATION) +@conditional_login_required(guac_login_required, settings.WEB_AUTHENTICATION) def index(request, task_id, session_data): # tenant isolation: minting a live-VM session grants keyboard/mouse/framebuffer # control of the running analysis VM — a task ACTION, not passive report viewing. @@ -168,7 +199,7 @@ def index(request, task_id, session_data): pass -@conditional_login_required(login_required, settings.WEB_AUTHENTICATION) +@conditional_login_required(guac_login_required, settings.WEB_AUTHENTICATION) def direct_vnc_host_port(request, host, port): if not is_vnc_console_enabled(): return _error(request, 0, "VNC Console is disabled in configuration") @@ -214,7 +245,7 @@ def direct_vnc_host_port(request, host, port): return response -@conditional_login_required(login_required, settings.WEB_AUTHENTICATION) +@conditional_login_required(guac_login_required, settings.WEB_AUTHENTICATION) def direct_vnc_vm(request, vm_name): if not is_vnc_console_enabled(): return _error(request, 0, "VNC Console is disabled in configuration") @@ -606,7 +637,7 @@ class SYSTEMTIME(ctypes.Structure): pass -@conditional_login_required(login_required, settings.WEB_AUTHENTICATION) +@conditional_login_required(guac_login_required, settings.WEB_AUTHENTICATION) def direct_vnc_vm_start(request, vm_name): if not is_vnc_console_enabled(): return _error(request, 0, "VNC Console is disabled in configuration") @@ -719,7 +750,7 @@ def direct_vnc_vm_start(request, vm_name): pass -@conditional_login_required(login_required, settings.WEB_AUTHENTICATION) +@conditional_login_required(guac_login_required, settings.WEB_AUTHENTICATION) def direct_vnc_vm_shutdown(request, vm_name): if not is_vnc_console_enabled(): return JsonResponse({"status": "error", "message": "VNC Console is disabled in configuration"}, status=403) @@ -823,7 +854,7 @@ def get_route_params(route_name, routing, configured_vpns): return None, None, None, None -@conditional_login_required(login_required, settings.WEB_AUTHENTICATION) +@conditional_login_required(guac_login_required, settings.WEB_AUTHENTICATION) def direct_vnc_vm_route(request, vm_name): if not is_vnc_console_enabled(): return JsonResponse({"status": "error", "message": "VNC Console is disabled in configuration"}, status=403) @@ -911,7 +942,7 @@ def direct_vnc_vm_route(request, vm_name): }, status=500) -@conditional_login_required(login_required, settings.WEB_AUTHENTICATION) +@conditional_login_required(guac_login_required, settings.WEB_AUTHENTICATION) def direct_vnc_vm_snapshots_list(request, vm_name): if not is_vnc_console_enabled(): return JsonResponse({"status": "error", "message": "VNC Console is disabled in configuration"}, status=403) @@ -973,7 +1004,7 @@ def direct_vnc_vm_snapshots_list(request, vm_name): pass -@conditional_login_required(login_required, settings.WEB_AUTHENTICATION) +@conditional_login_required(guac_login_required, settings.WEB_AUTHENTICATION) def direct_vnc_vm_snapshot_create(request, vm_name): if not is_vnc_console_enabled(): return JsonResponse({"status": "error", "message": "VNC Console is disabled in configuration"}, status=403) @@ -1064,7 +1095,7 @@ def direct_vnc_vm_snapshot_create(request, vm_name): pass -@conditional_login_required(login_required, settings.WEB_AUTHENTICATION) +@conditional_login_required(guac_login_required, settings.WEB_AUTHENTICATION) def direct_vnc_vm_snapshot_delete(request, vm_name): if not is_vnc_console_enabled(): return JsonResponse({"status": "error", "message": "VNC Console is disabled in configuration"}, status=403) diff --git a/web/web/guac_settings.py b/web/web/guac_settings.py index 6ecaa5af07e..908302f3da1 100644 --- a/web/web/guac_settings.py +++ b/web/web/guac_settings.py @@ -130,11 +130,15 @@ STATIC_URL = "/static/" -# Additional locations of static files -# STATICFILES_DIRS = [os.path.join(BASE_DIR, "static")] - STATIC_ROOT = os.path.join(BASE_DIR, "static") +# NOTE: Do NOT uncomment STATICFILES_DIRS pointing at BASE_DIR / "static" -- it is +# the same path as STATIC_ROOT above, and Django's staticfiles.E002 check rejects a +# STATICFILES_DIRS entry equal to STATIC_ROOT (source dirs must not overlap the +# collectstatic destination). If guac-web ever needs extra source dirs, use a +# DIFFERENT directory, e.g. STATICFILES_DIRS = [os.path.join(BASE_DIR, "static_src")]. +# STATICFILES_DIRS = [os.path.join(BASE_DIR, "static")] # <-- would trip staticfiles.E002 + STATICFILES_FINDERS = ( "django.contrib.staticfiles.finders.FileSystemFinder", "django.contrib.staticfiles.finders.AppDirectoriesFinder", diff --git a/web/web/local_settings.py b/web/web/local_settings.py index 41c6fd2c0ef..86c46db83da 100644 --- a/web/web/local_settings.py +++ b/web/web/local_settings.py @@ -34,9 +34,13 @@ # host will be accepted. Thus it's usually only necessary to set it in production. ALLOWED_HOSTS = ["*"] -# Uncomment for deployment with NGINX -# STATIC_ROOT = "" -# STATIC_ROOT = os.path.join(os.getcwd(), "static") +# Uncomment for deployment with NGINX, then run: python manage.py collectstatic +# STATIC_ROOT is the collectstatic DESTINATION and must be a DIFFERENT directory +# from STATICFILES_DIRS (which is web/static, the source assets) -- pointing it at +# .../static makes source == destination and trips Django's staticfiles.E002 +# ("STATICFILES_DIRS should not contain the STATIC_ROOT setting"). collectstatic +# creates this directory for you; point NGINX's /static/ alias at it. +# STATIC_ROOT = os.path.join(os.getcwd(), "static_collected") # SOCIALACCOUNT_PROVIDERS removed: managed dynamically from web.conf [oauth_oidc] in settings.py. # The previous stub here (google + github) was dead — the provider apps were commented out in INSTALLED_APPS.