diff --git a/CHANGELOG.md b/CHANGELOG.md index 2a6657a7..fb7ab7cf 100644 --- 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 diff --git a/Makefile b/Makefile index f682bd08..0e81087b 100644 --- a/Makefile +++ b/Makefile @@ -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 diff --git a/docker-compose.yml b/docker-compose.yml index 5c7e4d1a..dd2a1b9e 100644 --- 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}" diff --git a/docker/files/etc/nginx/conf.d/default.conf b/docker/files/etc/nginx/conf.d/default.conf index 9b5cf8db..b984787f 100644 --- 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; diff --git a/env.d/development/common.dist b/env.d/development/common.dist index 6833f09b..ce683107 100644 --- 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 diff --git a/src/backend/core/api/viewsets.py b/src/backend/core/api/viewsets.py index fe163af3..969d1861 100644 --- 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{UUID_REGEX:s})/" f"(?P{ATTACHMENTS_FOLDER:s}/{UUID_REGEX:s}{FILE_EXT_REGEX:s})$" ) +COLLABORATION_WS_URL_PATTERN = re.compile(rf"(?:^|&)room=(?P{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"], diff --git a/src/backend/core/models.py b/src/backend/core/models.py index 02f56073..2f52b2fe 100644 --- 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, diff --git a/src/backend/core/tests/documents/test_api_documents_retrieve.py b/src/backend/core/tests/documents/test_api_documents_retrieve.py index deb06038..4307ed69 100644 --- 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, diff --git a/src/backend/core/tests/test_models_documents.py b/src/backend/core/tests/test_models_documents.py index 91c17265..17cab6cd 100644 --- 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, diff --git a/src/backend/impress/settings.py b/src/backend/impress/settings.py index 50140620..4f65a60f 100755 --- 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. diff --git a/src/helm/env.d/dev/values.impress.yaml.gotmpl b/src/helm/env.d/dev/values.impress.yaml.gotmpl index 1b21ebc6..00af4575 100644 --- 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 diff --git a/src/helm/impress/values.yaml b/src/helm/impress/values.yaml index 7a994d87..2e3d6c81 100644 --- 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"