✨(back) check on document update if user can save it
When a document is updated, users not connected to the collaboration server can override work made by other people connected to the collaboration server. To avoid this, the priority is given to user connected to the collaboration server. If the websocket property in the request payload is missing or set to False, the backend fetch the collaboration server to now if the user can save or not. If users are already connected, the user can't save. Also, only one user without websocket can save a connect, the first user saving acquire a lock and all other users can't save. To implement this behavior, we need to track all users, connected and not, so a session is created for every user in the ForceSessionMiddleware.
This commit is contained in:
@@ -239,6 +239,7 @@ class DocumentSerializer(ListDocumentSerializer):
|
||||
"""Serialize documents with all fields for display in detail views."""
|
||||
|
||||
content = serializers.CharField(required=False)
|
||||
websocket = serializers.BooleanField(required=False, write_only=True)
|
||||
|
||||
class Meta:
|
||||
model = models.Document
|
||||
@@ -260,6 +261,7 @@ class DocumentSerializer(ListDocumentSerializer):
|
||||
"title",
|
||||
"updated_at",
|
||||
"user_roles",
|
||||
"websocket",
|
||||
]
|
||||
read_only_fields = [
|
||||
"id",
|
||||
|
||||
@@ -32,6 +32,7 @@ from rest_framework import filters, status, viewsets
|
||||
from rest_framework import response as drf_response
|
||||
from rest_framework.permissions import AllowAny
|
||||
from rest_framework.throttling import UserRateThrottle
|
||||
from sentry_sdk import capture_exception
|
||||
|
||||
from core import authentication, enums, models
|
||||
from core.services.ai_services import AIService
|
||||
@@ -634,6 +635,54 @@ class DocumentViewSet(
|
||||
"""Override to implement a soft delete instead of dumping the record in database."""
|
||||
instance.soft_delete()
|
||||
|
||||
def perform_update(self, serializer):
|
||||
"""Check rules about collaboration."""
|
||||
if serializer.validated_data.get("websocket"):
|
||||
return super().perform_update(serializer)
|
||||
|
||||
try:
|
||||
connection_info = CollaborationService().get_document_connection_info(
|
||||
serializer.instance.id,
|
||||
self.request.session.session_key,
|
||||
)
|
||||
except requests.HTTPError as e:
|
||||
capture_exception(e)
|
||||
connection_info = {
|
||||
"count": 0,
|
||||
"exists": False,
|
||||
}
|
||||
|
||||
if connection_info["count"] == 0:
|
||||
# No websocket mode
|
||||
logger.debug("update without connection found in the websocket server")
|
||||
cache_key = f"docs:no-websocket:{serializer.instance.id}"
|
||||
current_editor = cache.get(cache_key)
|
||||
if not current_editor:
|
||||
cache.set(
|
||||
cache_key,
|
||||
self.request.session.session_key,
|
||||
settings.NO_WEBSOCKET_CACHE_TIMEOUT,
|
||||
)
|
||||
elif current_editor != self.request.session.session_key:
|
||||
raise drf.exceptions.PermissionDenied(
|
||||
"You are not allowed to edit this document."
|
||||
)
|
||||
cache.touch(cache_key, settings.NO_WEBSOCKET_CACHE_TIMEOUT)
|
||||
return super().perform_update(serializer)
|
||||
|
||||
if connection_info["exists"]:
|
||||
# Websocket mode
|
||||
logger.debug("session key found in the websocket server")
|
||||
return super().perform_update(serializer)
|
||||
|
||||
logger.debug(
|
||||
"Users connected to the websocket but current editor not connected to it. Can not edit."
|
||||
)
|
||||
|
||||
raise drf.exceptions.PermissionDenied(
|
||||
"You are not allowed to edit this document."
|
||||
)
|
||||
|
||||
@drf.decorators.action(
|
||||
detail=False,
|
||||
methods=["get"],
|
||||
|
||||
21
src/backend/core/middleware.py
Normal file
21
src/backend/core/middleware.py
Normal file
@@ -0,0 +1,21 @@
|
||||
"""Force session creation for all requests."""
|
||||
|
||||
|
||||
class ForceSessionMiddleware:
|
||||
"""
|
||||
Force session creation for unauthenticated users.
|
||||
Must be used after Authentication middleware.
|
||||
"""
|
||||
|
||||
def __init__(self, get_response):
|
||||
"""Initialize the middleware."""
|
||||
self.get_response = get_response
|
||||
|
||||
def __call__(self, request):
|
||||
"""Force session creation for unauthenticated users."""
|
||||
|
||||
if not request.user.is_authenticated and request.session.session_key is None:
|
||||
request.session.create()
|
||||
|
||||
response = self.get_response(request)
|
||||
return response
|
||||
@@ -41,3 +41,31 @@ class CollaborationService:
|
||||
f"Failed to notify WebSocket server. Status code: {response.status_code}, "
|
||||
f"Response: {response.text}"
|
||||
)
|
||||
|
||||
def get_document_connection_info(self, room, session_key):
|
||||
"""
|
||||
Get the connection info for a document.
|
||||
"""
|
||||
endpoint = "get-connections"
|
||||
querystring = {
|
||||
"room": room,
|
||||
"sessionKey": session_key,
|
||||
}
|
||||
endpoint_url = f"{settings.COLLABORATION_API_URL}{endpoint}/"
|
||||
|
||||
headers = {"Authorization": settings.COLLABORATION_SERVER_SECRET}
|
||||
|
||||
try:
|
||||
response = requests.get(
|
||||
endpoint_url, headers=headers, params=querystring, timeout=10
|
||||
)
|
||||
except requests.RequestException as e:
|
||||
raise requests.HTTPError("Failed to get document connection info.") from e
|
||||
|
||||
if response.status_code != 200:
|
||||
raise requests.HTTPError(
|
||||
f"Failed to get document connection info. Status code: {response.status_code}, "
|
||||
f"Response: {response.text}"
|
||||
)
|
||||
|
||||
return response.json()
|
||||
|
||||
@@ -5,8 +5,10 @@ Tests for Documents API endpoint in impress's core app: update
|
||||
import random
|
||||
|
||||
from django.contrib.auth.models import AnonymousUser
|
||||
from django.core.cache import cache
|
||||
|
||||
import pytest
|
||||
import responses
|
||||
from rest_framework.test import APIClient
|
||||
|
||||
from core import factories, models
|
||||
@@ -44,6 +46,7 @@ def test_api_documents_update_anonymous_forbidden(reach, role, via_parent):
|
||||
new_document_values = serializers.DocumentSerializer(
|
||||
instance=factories.DocumentFactory()
|
||||
).data
|
||||
new_document_values["websocket"] = True
|
||||
response = APIClient().put(
|
||||
f"/api/v1.0/documents/{document.id!s}/",
|
||||
new_document_values,
|
||||
@@ -90,8 +93,9 @@ def test_api_documents_update_authenticated_unrelated_forbidden(
|
||||
|
||||
old_document_values = serializers.DocumentSerializer(instance=document).data
|
||||
new_document_values = serializers.DocumentSerializer(
|
||||
instance=factories.DocumentFactory()
|
||||
instance=factories.DocumentFactory(),
|
||||
).data
|
||||
new_document_values["websocket"] = True
|
||||
response = client.put(
|
||||
f"/api/v1.0/documents/{document.id!s}/",
|
||||
new_document_values,
|
||||
@@ -141,8 +145,9 @@ def test_api_documents_update_anonymous_or_authenticated_unrelated(
|
||||
|
||||
old_document_values = serializers.DocumentSerializer(instance=document).data
|
||||
new_document_values = serializers.DocumentSerializer(
|
||||
instance=factories.DocumentFactory()
|
||||
instance=factories.DocumentFactory(),
|
||||
).data
|
||||
new_document_values["websocket"] = True
|
||||
response = client.put(
|
||||
f"/api/v1.0/documents/{document.id!s}/",
|
||||
new_document_values,
|
||||
@@ -206,6 +211,7 @@ def test_api_documents_update_authenticated_reader(via, via_parent, mock_user_te
|
||||
new_document_values = serializers.DocumentSerializer(
|
||||
instance=factories.DocumentFactory()
|
||||
).data
|
||||
new_document_values["websocket"] = True
|
||||
response = client.put(
|
||||
f"/api/v1.0/documents/{document.id!s}/",
|
||||
new_document_values,
|
||||
@@ -258,6 +264,7 @@ def test_api_documents_update_authenticated_editor_administrator_or_owner(
|
||||
new_document_values = serializers.DocumentSerializer(
|
||||
instance=factories.DocumentFactory()
|
||||
).data
|
||||
new_document_values["websocket"] = True
|
||||
response = client.put(
|
||||
f"/api/v1.0/documents/{document.id!s}/",
|
||||
new_document_values,
|
||||
@@ -287,6 +294,274 @@ def test_api_documents_update_authenticated_editor_administrator_or_owner(
|
||||
assert value == new_document_values[key]
|
||||
|
||||
|
||||
@responses.activate
|
||||
def test_api_documents_update_authenticated_no_websocket(settings):
|
||||
"""
|
||||
When a user updates the document, not connected to the websocket and is the first to update,
|
||||
the document should be updated.
|
||||
"""
|
||||
user = factories.UserFactory(with_owned_document=True)
|
||||
client = APIClient()
|
||||
client.force_login(user)
|
||||
session_key = client.session.session_key
|
||||
|
||||
document = factories.DocumentFactory(users=[(user, "editor")])
|
||||
|
||||
new_document_values = serializers.DocumentSerializer(
|
||||
instance=factories.DocumentFactory()
|
||||
).data
|
||||
new_document_values["websocket"] = False
|
||||
settings.COLLABORATION_API_URL = "http://example.com/"
|
||||
settings.COLLABORATION_SERVER_SECRET = "secret-token"
|
||||
endpoint_url = (
|
||||
f"{settings.COLLABORATION_API_URL}get-connections/"
|
||||
f"?room={document.id}&sessionKey={session_key}"
|
||||
)
|
||||
|
||||
ws_resp = responses.get(endpoint_url, json={"count": 0, "exists": False})
|
||||
|
||||
assert cache.get(f"docs:no-websocket:{document.id}") is None
|
||||
|
||||
response = client.put(
|
||||
f"/api/v1.0/documents/{document.id!s}/",
|
||||
new_document_values,
|
||||
format="json",
|
||||
)
|
||||
assert response.status_code == 200
|
||||
|
||||
assert cache.get(f"docs:no-websocket:{document.id}") == session_key
|
||||
assert ws_resp.call_count == 1
|
||||
|
||||
|
||||
@responses.activate
|
||||
def test_api_documents_update_authenticated_no_websocket_user_already_editing(settings):
|
||||
"""
|
||||
When a user updates the document, not connected to the websocket and is not the first to update,
|
||||
the document should not be updated.
|
||||
"""
|
||||
user = factories.UserFactory(with_owned_document=True)
|
||||
client = APIClient()
|
||||
client.force_login(user)
|
||||
session_key = client.session.session_key
|
||||
|
||||
document = factories.DocumentFactory(users=[(user, "editor")])
|
||||
|
||||
new_document_values = serializers.DocumentSerializer(
|
||||
instance=factories.DocumentFactory()
|
||||
).data
|
||||
new_document_values["websocket"] = False
|
||||
settings.COLLABORATION_API_URL = "http://example.com/"
|
||||
settings.COLLABORATION_SERVER_SECRET = "secret-token"
|
||||
endpoint_url = (
|
||||
f"{settings.COLLABORATION_API_URL}get-connections/"
|
||||
f"?room={document.id}&sessionKey={session_key}"
|
||||
)
|
||||
ws_resp = responses.get(endpoint_url, json={"count": 0, "exists": False})
|
||||
|
||||
cache.set(f"docs:no-websocket:{document.id}", "other_session_key")
|
||||
|
||||
response = client.put(
|
||||
f"/api/v1.0/documents/{document.id!s}/",
|
||||
new_document_values,
|
||||
format="json",
|
||||
)
|
||||
assert response.status_code == 403
|
||||
assert response.json() == {"detail": "You are not allowed to edit this document."}
|
||||
|
||||
assert ws_resp.call_count == 1
|
||||
|
||||
|
||||
@responses.activate
|
||||
def test_api_documents_update_no_websocket_other_user_connected_to_websocket(settings):
|
||||
"""
|
||||
When a user updates the document, not connected to the websocket and another user is connected
|
||||
to the websocket, the document should not be updated.
|
||||
"""
|
||||
user = factories.UserFactory(with_owned_document=True)
|
||||
client = APIClient()
|
||||
client.force_login(user)
|
||||
session_key = client.session.session_key
|
||||
|
||||
document = factories.DocumentFactory(users=[(user, "editor")])
|
||||
|
||||
new_document_values = serializers.DocumentSerializer(
|
||||
instance=factories.DocumentFactory()
|
||||
).data
|
||||
new_document_values["websocket"] = False
|
||||
settings.COLLABORATION_API_URL = "http://example.com/"
|
||||
settings.COLLABORATION_SERVER_SECRET = "secret-token"
|
||||
endpoint_url = (
|
||||
f"{settings.COLLABORATION_API_URL}get-connections/"
|
||||
f"?room={document.id}&sessionKey={session_key}"
|
||||
)
|
||||
ws_resp = responses.get(endpoint_url, json={"count": 3, "exists": False})
|
||||
|
||||
assert cache.get(f"docs:no-websocket:{document.id}") is None
|
||||
|
||||
response = client.put(
|
||||
f"/api/v1.0/documents/{document.id!s}/",
|
||||
new_document_values,
|
||||
format="json",
|
||||
)
|
||||
assert response.status_code == 403
|
||||
assert response.json() == {"detail": "You are not allowed to edit this document."}
|
||||
assert cache.get(f"docs:no-websocket:{document.id}") is None
|
||||
assert ws_resp.call_count == 1
|
||||
|
||||
|
||||
@responses.activate
|
||||
def test_api_documents_update_user_connected_to_websocket(settings):
|
||||
"""
|
||||
When a user updates the document, connected to the websocket, the document should be updated.
|
||||
"""
|
||||
user = factories.UserFactory(with_owned_document=True)
|
||||
client = APIClient()
|
||||
client.force_login(user)
|
||||
session_key = client.session.session_key
|
||||
|
||||
document = factories.DocumentFactory(users=[(user, "editor")])
|
||||
|
||||
new_document_values = serializers.DocumentSerializer(
|
||||
instance=factories.DocumentFactory()
|
||||
).data
|
||||
new_document_values["websocket"] = False
|
||||
settings.COLLABORATION_API_URL = "http://example.com/"
|
||||
settings.COLLABORATION_SERVER_SECRET = "secret-token"
|
||||
endpoint_url = (
|
||||
f"{settings.COLLABORATION_API_URL}get-connections/"
|
||||
f"?room={document.id}&sessionKey={session_key}"
|
||||
)
|
||||
ws_resp = responses.get(endpoint_url, json={"count": 3, "exists": True})
|
||||
|
||||
assert cache.get(f"docs:no-websocket:{document.id}") is None
|
||||
|
||||
response = client.put(
|
||||
f"/api/v1.0/documents/{document.id!s}/",
|
||||
new_document_values,
|
||||
format="json",
|
||||
)
|
||||
assert response.status_code == 200
|
||||
assert cache.get(f"docs:no-websocket:{document.id}") is None
|
||||
assert ws_resp.call_count == 1
|
||||
|
||||
|
||||
@responses.activate
|
||||
def test_api_documents_update_websocket_server_unreachable_fallback_to_no_websocket(
|
||||
settings,
|
||||
):
|
||||
"""
|
||||
When the websocket server is unreachable, the document should be updated like if the user was
|
||||
not connected to the websocket.
|
||||
"""
|
||||
user = factories.UserFactory(with_owned_document=True)
|
||||
client = APIClient()
|
||||
client.force_login(user)
|
||||
session_key = client.session.session_key
|
||||
|
||||
document = factories.DocumentFactory(users=[(user, "editor")])
|
||||
|
||||
new_document_values = serializers.DocumentSerializer(
|
||||
instance=factories.DocumentFactory()
|
||||
).data
|
||||
new_document_values["websocket"] = False
|
||||
settings.COLLABORATION_API_URL = "http://example.com/"
|
||||
settings.COLLABORATION_SERVER_SECRET = "secret-token"
|
||||
endpoint_url = (
|
||||
f"{settings.COLLABORATION_API_URL}get-connections/"
|
||||
f"?room={document.id}&sessionKey={session_key}"
|
||||
)
|
||||
ws_resp = responses.get(endpoint_url, status=500)
|
||||
|
||||
assert cache.get(f"docs:no-websocket:{document.id}") is None
|
||||
|
||||
response = client.put(
|
||||
f"/api/v1.0/documents/{document.id!s}/",
|
||||
new_document_values,
|
||||
format="json",
|
||||
)
|
||||
assert response.status_code == 200
|
||||
|
||||
assert cache.get(f"docs:no-websocket:{document.id}") == session_key
|
||||
assert ws_resp.call_count == 1
|
||||
|
||||
|
||||
@responses.activate
|
||||
def test_api_documents_update_websocket_server_unreachable_fallback_to_no_websocket_other_users(
|
||||
settings,
|
||||
):
|
||||
"""
|
||||
When the websocket server is unreachable, the behavior fallback to the no websocket one.
|
||||
If an other user is already editing, the document should not be updated.
|
||||
"""
|
||||
user = factories.UserFactory(with_owned_document=True)
|
||||
client = APIClient()
|
||||
client.force_login(user)
|
||||
session_key = client.session.session_key
|
||||
|
||||
document = factories.DocumentFactory(users=[(user, "editor")])
|
||||
|
||||
new_document_values = serializers.DocumentSerializer(
|
||||
instance=factories.DocumentFactory()
|
||||
).data
|
||||
new_document_values["websocket"] = False
|
||||
settings.COLLABORATION_API_URL = "http://example.com/"
|
||||
settings.COLLABORATION_SERVER_SECRET = "secret-token"
|
||||
endpoint_url = (
|
||||
f"{settings.COLLABORATION_API_URL}get-connections/"
|
||||
f"?room={document.id}&sessionKey={session_key}"
|
||||
)
|
||||
ws_resp = responses.get(endpoint_url, status=500)
|
||||
|
||||
cache.set(f"docs:no-websocket:{document.id}", "other_session_key")
|
||||
|
||||
response = client.put(
|
||||
f"/api/v1.0/documents/{document.id!s}/",
|
||||
new_document_values,
|
||||
format="json",
|
||||
)
|
||||
assert response.status_code == 403
|
||||
|
||||
assert cache.get(f"docs:no-websocket:{document.id}") == "other_session_key"
|
||||
assert ws_resp.call_count == 1
|
||||
|
||||
|
||||
@responses.activate
|
||||
def test_api_documents_update_force_websocket_param_to_true(settings):
|
||||
"""
|
||||
When the websocket parameter is set to true, the document should be updated without any check.
|
||||
"""
|
||||
user = factories.UserFactory(with_owned_document=True)
|
||||
client = APIClient()
|
||||
client.force_login(user)
|
||||
session_key = client.session.session_key
|
||||
|
||||
document = factories.DocumentFactory(users=[(user, "editor")])
|
||||
|
||||
new_document_values = serializers.DocumentSerializer(
|
||||
instance=factories.DocumentFactory()
|
||||
).data
|
||||
new_document_values["websocket"] = True
|
||||
settings.COLLABORATION_API_URL = "http://example.com/"
|
||||
settings.COLLABORATION_SERVER_SECRET = "secret-token"
|
||||
endpoint_url = (
|
||||
f"{settings.COLLABORATION_API_URL}get-connections/"
|
||||
f"?room={document.id}&sessionKey={session_key}"
|
||||
)
|
||||
ws_resp = responses.get(endpoint_url, status=500)
|
||||
|
||||
assert cache.get(f"docs:no-websocket:{document.id}") is None
|
||||
|
||||
response = client.put(
|
||||
f"/api/v1.0/documents/{document.id!s}/",
|
||||
new_document_values,
|
||||
format="json",
|
||||
)
|
||||
assert response.status_code == 200
|
||||
|
||||
assert cache.get(f"docs:no-websocket:{document.id}") is None
|
||||
assert ws_resp.call_count == 0
|
||||
|
||||
|
||||
@pytest.mark.parametrize("via", VIA)
|
||||
def test_api_documents_update_administrator_or_owner_of_another(via, mock_user_teams):
|
||||
"""
|
||||
@@ -317,6 +592,7 @@ def test_api_documents_update_administrator_or_owner_of_another(via, mock_user_t
|
||||
new_document_values = serializers.DocumentSerializer(
|
||||
instance=factories.DocumentFactory()
|
||||
).data
|
||||
new_document_values["websocket"] = True
|
||||
response = client.put(
|
||||
f"/api/v1.0/documents/{other_document.id!s}/",
|
||||
new_document_values,
|
||||
|
||||
@@ -50,7 +50,7 @@ def test_api_documents_update_new_attachment_keys_anonymous(django_assert_num_qu
|
||||
with django_assert_num_queries(11):
|
||||
response = APIClient().put(
|
||||
f"/api/v1.0/documents/{document.id!s}/",
|
||||
{"content": get_ydoc_with_mages(image_keys)},
|
||||
{"content": get_ydoc_with_mages(image_keys), "websocket": True},
|
||||
format="json",
|
||||
)
|
||||
assert response.status_code == 200
|
||||
@@ -63,7 +63,7 @@ def test_api_documents_update_new_attachment_keys_anonymous(django_assert_num_qu
|
||||
with django_assert_num_queries(7):
|
||||
response = APIClient().put(
|
||||
f"/api/v1.0/documents/{document.id!s}/",
|
||||
{"content": get_ydoc_with_mages(image_keys[:2])},
|
||||
{"content": get_ydoc_with_mages(image_keys[:2]), "websocket": True},
|
||||
format="json",
|
||||
)
|
||||
assert response.status_code == 200
|
||||
|
||||
Reference in New Issue
Block a user