✨(backend) add subrequest auth view for collaboration server

We need to improve security on the access to The collaboration server We can use the same pattern as for media files leveraging the nginx subrequest feature.
2024-11-18 08:05:54 +01:00
parent 64674b6a73
commit 66553ee236
12 changed files with 118 additions and 18 deletions
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -16,6 +16,7 @@ and this project adheres to

 ## Changed

+- 🔒️(collaboration) increase collaboration access security #472
 - 🔨(frontend) encapsulated title to its own component #474
 - 🐛(frontend) Fix hidden menu on Firefox #468
 - ⚡️(backend) optimize number of queries on document list view #411
--- a/2
+++ b/2
@@ -122,8 +122,8 @@ logs: ## display app-dev logs (follow mode)

 run: ## start the wsgi (production) and development server
 	@$(COMPOSE) up --force-recreate -d celery-dev
-	@$(COMPOSE) up --force-recreate -d nginx
 	@$(COMPOSE) up --force-recreate -d y-provider
+	@$(COMPOSE) up --force-recreate -d nginx
 	@echo "Wait for postgresql to be up..."
 	@$(WAIT_DB)
 .PHONY: run
--- a/docker-compose.yml
+++ b/docker-compose.yml
@@ -118,6 +118,7 @@ services:
    depends_on:
      - keycloak
      - app-dev
+      - y-provider

  frontend-dev:
    user: "${DOCKER_USER:-1000}"
--- a/docker/files/etc/nginx/conf.d/default.conf
+++ b/docker/files/etc/nginx/conf.d/default.conf
@@ -4,6 +4,49 @@ server {
    server_name localhost;
    charset utf-8;

+    # Proxy auth for collaboration server
+    location /collaboration/ws/ {
+        # Collaboration Auth request configuration
+        auth_request /collaboration-auth;
+        auth_request_set $authHeader $upstream_http_authorization;
+        auth_request_set $canEdit $upstream_http_x_can_edit;
+        auth_request_set $userId $upstream_http_x_user_id;
+
+        # Pass specific headers from the auth response
+        proxy_set_header Authorization $authHeader;
+        proxy_set_header X-Can-Edit $canEdit;
+        proxy_set_header X-User-Id $userId;
+
+        # Ensure WebSocket upgrade
+        proxy_http_version 1.1;
+        proxy_set_header Upgrade $http_upgrade;
+        proxy_set_header Connection "Upgrade";
+
+        # Collaboration server
+        proxy_pass http://y-provider:4444;
+
+        # Set appropriate timeout for WebSocket
+        proxy_read_timeout 86400;
+        proxy_send_timeout 86400;
+
+        # Preserve original host and additional headers
+        proxy_set_header Host $host;
+    }
+
+    location /collaboration-auth {
+        proxy_pass http://app-dev:8000/api/v1.0/documents/collaboration-auth/;
+        proxy_set_header Host $host;
+        proxy_set_header X-Real-IP $remote_addr;
+        proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
+        proxy_set_header X-Original-URL $request_uri;
+        
+        # Prevent the body from being passed
+        proxy_pass_request_body off;
+        proxy_set_header Content-Length "";
+        proxy_set_header X-Original-Method $request_method;
+    }
+
+    # Proxy auth for media
    location /media/ {
        # Auth request configuration
        auth_request /media-auth;
--- a/env.d/development/common.dist
+++ b/env.d/development/common.dist
@@ -53,7 +53,8 @@ AI_API_KEY=password
 AI_MODEL=llama

 # Collaboration
-COLLABORATION_WS_URL=ws://localhost:4444
+COLLABORATION_SERVER_SECRET=my-secret
+COLLABORATION_WS_URL=ws://localhost:8083/collaboration/ws

 # Frontend
 FRONTEND_THEME=dsfr
--- a/src/backend/core/api/viewsets.py
+++ b/src/backend/core/api/viewsets.py
@@ -45,6 +45,7 @@ MEDIA_STORAGE_URL_PATTERN = re.compile(
    f"{settings.MEDIA_URL:s}(?P<pk>{UUID_REGEX:s})/"
    f"(?P<key>{ATTACHMENTS_FOLDER:s}/{UUID_REGEX:s}{FILE_EXT_REGEX:s})$"
 )
+COLLABORATION_WS_URL_PATTERN = re.compile(rf"(?:^|&)room=(?P<pk>{UUID_REGEX})(?:&|$)")

 # pylint: disable=too-many-ancestors

@@ -620,6 +621,10 @@ class DocumentViewSet(
        parsed_url = urlparse(original_url)
        match = pattern.search(parsed_url.path)

+        # If the path does not match the pattern, try to extract the parameters from the query
+        if not match:
+            match = pattern.search(parsed_url.query)
+
        if not match:
            logger.debug(
                "Subrequest URL '%s' did not match pattern '%s'",
@@ -645,17 +650,19 @@ class DocumentViewSet(
        except models.Document.DoesNotExist as exc:
            logger.debug("Document with ID '%s' does not exist", pk)
            raise drf.exceptions.PermissionDenied() from exc
-        print(document)
-        if not document.get_abilities(request.user).get(self.action, False):
+
+        user_abilities = document.get_abilities(request.user)
+
+        if not user_abilities.get(self.action, False):
            logger.debug(
                "User '%s' lacks permission for document '%s'", request.user, pk
            )
-            # raise drf.exceptions.PermissionDenied()
+            raise drf.exceptions.PermissionDenied()

        logger.debug(
            "Subrequest authorization successful. Extracted parameters: %s", url_params
        )
-        return url_params
+        return url_params, user_abilities, request.user.id

    @drf.decorators.action(detail=False, methods=["get"], url_path="media-auth")
    def media_auth(self, request, *args, **kwargs):
@@ -668,7 +675,9 @@ class DocumentViewSet(
        annotation. The request will then be proxied to the object storage backend who will
        respond with the file after checking the signature included in headers.
        """
-        url_params = self._authorize_subrequest(request, MEDIA_STORAGE_URL_PATTERN)
+        url_params, _, _ = self._authorize_subrequest(
+            request, MEDIA_STORAGE_URL_PATTERN
+        )
        pk, key = url_params.values()

        # Generate S3 authorization headers using the extracted URL parameters
@@ -676,6 +685,26 @@ class DocumentViewSet(

        return drf.response.Response("authorized", headers=request.headers, status=200)

+    @drf.decorators.action(detail=False, methods=["get"], url_path="collaboration-auth")
+    def collaboration_auth(self, request, *args, **kwargs):
+        """
+        This view is used by an Nginx subrequest to control access to a document's
+        collaboration server.
+        """
+        _, user_abilities, user_id = self._authorize_subrequest(
+            request, COLLABORATION_WS_URL_PATTERN
+        )
+        can_edit = user_abilities["partial_update"]
+
+        # Add the collaboration server secret token to the headers
+        headers = {
+            "Authorization": settings.COLLABORATION_SERVER_SECRET,
+            "X-Can-Edit": str(can_edit),
+            "X-User-Id": str(user_id),
+        }
+
+        return drf.response.Response("authorized", headers=headers, status=200)
+
    @drf.decorators.action(
        detail=True,
        methods=["post"],
--- a/src/backend/core/models.py
+++ b/src/backend/core/models.py
@@ -520,6 +520,7 @@ class Document(BaseModel):
            "ai_transform": can_update,
            "ai_translate": can_update,
            "attachment_upload": can_update,
+            "collaboration_auth": can_get,
            "destroy": RoleChoices.OWNER in roles,
            "favorite": can_get and user.is_authenticated,
            "link_configuration": is_owner_or_admin,
--- a/src/backend/core/tests/documents/test_api_documents_retrieve.py
+++ b/src/backend/core/tests/documents/test_api_documents_retrieve.py
@@ -26,6 +26,7 @@ def test_api_documents_retrieve_anonymous_public():
            "ai_transform": document.link_role == "editor",
            "ai_translate": document.link_role == "editor",
            "attachment_upload": document.link_role == "editor",
+            "collaboration_auth": True,
            "destroy": False,
            # Anonymous user can't favorite a document even with read access
            "favorite": False,
@@ -89,6 +90,7 @@ def test_api_documents_retrieve_authenticated_unrelated_public_or_authenticated(
            "ai_transform": document.link_role == "editor",
            "ai_translate": document.link_role == "editor",
            "attachment_upload": document.link_role == "editor",
+            "collaboration_auth": True,
            "destroy": False,
            "favorite": True,
            "invite_owner": False,
--- a/src/backend/core/tests/test_models_documents.py
+++ b/src/backend/core/tests/test_models_documents.py
@@ -98,6 +98,7 @@ def test_models_documents_get_abilities_forbidden(is_authenticated, reach, role)
        "ai_transform": False,
        "ai_translate": False,
        "attachment_upload": False,
+        "collaboration_auth": False,
        "destroy": False,
        "favorite": False,
        "invite_owner": False,
@@ -134,6 +135,7 @@ def test_models_documents_get_abilities_reader(is_authenticated, reach):
        "ai_transform": False,
        "ai_translate": False,
        "attachment_upload": False,
+        "collaboration_auth": True,
        "destroy": False,
        "favorite": is_authenticated,
        "invite_owner": False,
@@ -170,6 +172,7 @@ def test_models_documents_get_abilities_editor(is_authenticated, reach):
        "ai_transform": True,
        "ai_translate": True,
        "attachment_upload": True,
+        "collaboration_auth": True,
        "destroy": False,
        "favorite": is_authenticated,
        "invite_owner": False,
@@ -195,6 +198,7 @@ def test_models_documents_get_abilities_owner():
        "ai_transform": True,
        "ai_translate": True,
        "attachment_upload": True,
+        "collaboration_auth": True,
        "destroy": True,
        "favorite": True,
        "invite_owner": True,
@@ -219,6 +223,7 @@ def test_models_documents_get_abilities_administrator():
        "ai_transform": True,
        "ai_translate": True,
        "attachment_upload": True,
+        "collaboration_auth": True,
        "destroy": False,
        "favorite": True,
        "invite_owner": False,
@@ -246,6 +251,7 @@ def test_models_documents_get_abilities_editor_user(django_assert_num_queries):
        "ai_transform": True,
        "ai_translate": True,
        "attachment_upload": True,
+        "collaboration_auth": True,
        "destroy": False,
        "favorite": True,
        "invite_owner": False,
@@ -275,6 +281,7 @@ def test_models_documents_get_abilities_reader_user(django_assert_num_queries):
        "ai_transform": False,
        "ai_translate": False,
        "attachment_upload": False,
+        "collaboration_auth": True,
        "destroy": False,
        "favorite": True,
        "invite_owner": False,
@@ -305,6 +312,7 @@ def test_models_documents_get_abilities_preset_role(django_assert_num_queries):
        "ai_transform": False,
        "ai_translate": False,
        "attachment_upload": False,
+        "collaboration_auth": True,
        "destroy": False,
        "favorite": True,
        "invite_owner": False,
--- a/src/backend/impress/settings.py
+++ b/src/backend/impress/settings.py
@@ -372,6 +372,9 @@ class Base(Configuration):
    SENTRY_DSN = values.Value(None, environ_name="SENTRY_DSN", environ_prefix=None)

    # Collaboration
+    COLLABORATION_SERVER_SECRET = values.Value(
+        None, environ_name="COLLABORATION_SERVER_SECRET", environ_prefix=None
+    )
    COLLABORATION_WS_URL = values.Value(
        None, environ_name="COLLABORATION_WS_URL", environ_prefix=None
    )
@@ -465,9 +468,22 @@ class Base(Configuration):
        environ_prefix=None,
    )

+    USER_OIDC_FIELDS_TO_FULLNAME = values.ListValue(
+        default=["first_name", "last_name"],
+        environ_name="USER_OIDC_FIELDS_TO_FULLNAME",
+        environ_prefix=None,
+    )
+    USER_OIDC_FIELD_TO_SHORTNAME = values.Value(
+        default="first_name",
+        environ_name="USER_OIDC_FIELD_TO_SHORTNAME",
+        environ_prefix=None,
+    )
+
    ALLOW_LOGOUT_GET_METHOD = values.BooleanValue(
        default=True, environ_name="ALLOW_LOGOUT_GET_METHOD", environ_prefix=None
    )
+
+    # AI service
    AI_API_KEY = values.Value(None, environ_name="AI_API_KEY", environ_prefix=None)
    AI_BASE_URL = values.Value(None, environ_name="AI_BASE_URL", environ_prefix=None)
    AI_MODEL = values.Value(None, environ_name="AI_MODEL", environ_prefix=None)
@@ -483,17 +499,6 @@ class Base(Configuration):
        "day": 200,
    }

-    USER_OIDC_FIELDS_TO_FULLNAME = values.ListValue(
-        default=["first_name", "last_name"],
-        environ_name="USER_OIDC_FIELDS_TO_FULLNAME",
-        environ_prefix=None,
-    )
-    USER_OIDC_FIELD_TO_SHORTNAME = values.Value(
-        default="first_name",
-        environ_name="USER_OIDC_FIELD_TO_SHORTNAME",
-        environ_prefix=None,
-    )
-
    # Logging
    # We want to make it easy to log to console but by default we log production
    # to Sentry and don't want to log to console.
--- a/src/helm/env.d/dev/values.impress.yaml.gotmpl
+++ b/src/helm/env.d/dev/values.impress.yaml.gotmpl
@@ -6,6 +6,7 @@ image:
 backend:
  replicas: 1
  envVars:
+    COLLABORATION_SERVER_SECRET: my-secret
    DJANGO_CSRF_TRUSTED_ORIGINS: https://impress.127.0.0.1.nip.io,http://impress.127.0.0.1.nip.io
    DJANGO_CONFIGURATION: Feature
    DJANGO_ALLOWED_HOSTS: impress.127.0.0.1.nip.io
@@ -104,6 +105,12 @@ ingressWS:
  enabled: true
  host: impress.127.0.0.1.nip.io

+  annotations:
+    nginx.ingress.kubernetes.io/auth-response-headers: "Authorization, Can-Edit, User-Id"
+    nginx.ingress.kubernetes.io/auth-url: https://impress.127.0.0.1.nip.io/api/v1.0/documents/collaboration-auth/
+    nginx.ingress.kubernetes.io/enable-websocket: "true"
+    nginx.ingress.kubernetes.io/upstream-hash-by: "$request_uri"
+
 ingressAdmin:
  enabled: true
  host: impress.127.0.0.1.nip.io
--- a/src/helm/impress/values.yaml
+++ b/src/helm/impress/values.yaml
@@ -72,6 +72,8 @@ ingressWS:
  customBackends: []

  annotations: 
+    nginx.ingress.kubernetes.io/auth-url: https://impress.example.com/api/v1.0/documents/collaboration-auth/
+    nginx.ingress.kubernetes.io/auth-response-headers: "Authorization, Can-Edit, User-Id"
    nginx.ingress.kubernetes.io/enable-websocket: "true" 
    nginx.ingress.kubernetes.io/upstream-hash-by: "$request_uri"