diff --git a/CHANGELOG.md b/CHANGELOG.md index 774f03ab5..fe8f9651b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,14 +20,25 @@ and this project adheres to ### Added -- ✨(collaboration) grant the browser only the two routes it uses. The +- ✨(collaboration) let a user read the document's editing history from the + moment they were given access to it. The collaboration server's `activity` and + `changeset` routes are opened to the browser, bounded per user to the earliest + access they hold on the document or on one of its ancestors — the same cut-off + the version endpoints have always applied, now computed once in the backend + (`user_access_since`) and applied by the collaboration server as well. The + bound is enforced server-side and silently: a client asks for whatever range it + likes and receives only its own share. A reader who reaches a document through + its link alone holds no access and so has no date to bound a history with, and + gets none — as they never did + +- ✨(collaboration) grant the browser only the routes it uses. The collaboration server now answers what a caller may do with a document as a permission object, facet by facet, and enforces every facet itself. A browser - is granted the websocket and the document route the http fallback polls, and - nothing else — the history, activity, changeset, rollback and prune routes, - every backend-internal endpoint, and any endpoint a future release adds are - refused to it. `create-ydoc` in particular was reachable by any signed-in - editor and is now the backend's alone. The backend's admin token keeps full + is granted the websocket, the document route the http fallback polls and the + two history routes, and nothing else — rollback, prune, every backend-internal + endpoint, and any endpoint a future release adds are refused to it. + `create-ydoc` in particular was reachable by any signed-in editor and is now + the backend's alone. The backend's admin token keeps full access, minus the irreversible content erasure that the new version exposes over http for the first time diff --git a/documentation/collaboration.md b/documentation/collaboration.md index 3a99d4955..35e24de71 100644 --- a/documentation/collaboration.md +++ b/documentation/collaboration.md @@ -22,7 +22,7 @@ The Django backend reads and writes document content there too, so point it at t YHUB_API_BASE_URL: http://{yhub-service}:443 ``` -Prefer the internal service url: the routes the backend calls are not meant to be reachable from the outside. Route `/collaboration/ws/` to the service publicly — that is the one the browsers open — plus the document routes (`/collaboration/ydoc/`, `rollback`, `prune`, `changeset`, `activity`) and `/collaboration/jwks/`, which carries public keys and nothing else. Keep `reset-connections`, `migrate`, `restore-ydoc`, `reset-ydoc` and `create-ydoc` in-cluster. +Prefer the internal service url: the routes the backend calls are not meant to be reachable from the outside. Route `/collaboration/ws/` to the service publicly — that is the one the browsers open — plus `/collaboration/ydoc/` for the http fallback, `/collaboration/activity/` and `/collaboration/changeset/` for the editing history, and `/collaboration/jwks/`, which carries public keys and nothing else. Keep everything else in-cluster: `rollback`, `prune`, `reset-connections`, `migrate`, `restore-ydoc`, `reset-ydoc` and `create-ydoc` are refused to a browser by the permission tables anyway, and an endpoint that cannot be reached cannot be probed. Both directions are authenticated with short-lived RS256 JWTs rather than a shared secret, and each side verifies the other against the JWKS it publishes — so both need a signing key of their own, and neither needs a copy of the other's: @@ -109,3 +109,25 @@ fallback request, so a modified or stale client changes nothing. See the access- Note this is deliberately stricter than the collaboration server's own default, which lets read-only connections broadcast cursors. + +## How much history a user may see + +The editing history is bounded per user: **you see the document's history from the moment you were +given access to it, and no further back.** Joining a document that has been written for a year does +not hand you the year — it hands you what happened since you arrived. + +This is not a new rule. It is the one the version endpoints have always applied ("only those +created after the user got access to the document"); it now also bounds the collaboration server's +`activity` and `changeset` routes, which are what a history view is built on. + +The date is the earliest access you hold on the document **or on one of its ancestors** — share a +folder with someone and they get its whole subtree from that moment, including documents created in +it later. The backend computes it (`user_access_since` on the document endpoint) and the +collaboration server turns it into the start of the history it will serve. The bound is applied +server-side and silently: a client asks for whatever range it likes and receives only its own +share, so there is no bound for it to get wrong and none it can widen. + +A reader who reaches a document through its link alone — a public or authenticated-reach document +they hold no access on — gets **no history at all**, not a bounded one. There is no access record +and therefore no date to bound it with, which is the same reason the version endpoints have always +refused them. diff --git a/src/backend/core/api/serializers.py b/src/backend/core/api/serializers.py index eb8f27261..8c291cbbc 100644 --- a/src/backend/core/api/serializers.py +++ b/src/backend/core/api/serializers.py @@ -183,6 +183,16 @@ class DocumentSerializer(ListDocumentSerializer): file = serializers.FileField( required=False, write_only=True, allow_null=True, max_length=255 ) + # When the current user gained access to this document — the earliest access they hold on + # it or on one of its ancestors, `null` when they reach it through its link reach alone. + # It bounds the history they may read: the collaboration server fetches this endpoint to + # authorize a connection and turns this into `history.from`. + # + # Read from the `user_access_since` annotation, which `filter_queryset` applies — so it is + # answered on the retrieve endpoint, the one that is read for it. It falls back to `null` + # on the write responses (create/update), which serialize an instance that never came from + # that queryset; nothing consumes it there. + user_access_since = serializers.DateTimeField(read_only=True, default=None) class Meta: model = models.Document @@ -208,6 +218,7 @@ class DocumentSerializer(ListDocumentSerializer): "path", "title", "updated_at", + "user_access_since", "user_role", ] read_only_fields = [ @@ -229,6 +240,7 @@ class DocumentSerializer(ListDocumentSerializer): "numchild", "path", "updated_at", + "user_access_since", "user_role", ] diff --git a/src/backend/core/api/viewsets.py b/src/backend/core/api/viewsets.py index b8c78c0ba..aa2e664a1 100644 --- a/src/backend/core/api/viewsets.py +++ b/src/backend/core/api/viewsets.py @@ -21,7 +21,7 @@ from django.core.validators import URLValidator from django.db import DatabaseError, transaction from django.db import models as db from django.db.models.expressions import RawSQL -from django.db.models.functions import Greatest, Left, Length +from django.db.models.functions import Greatest from django.http import Http404, StreamingHttpResponse from django.urls import reverse from django.utils import timezone @@ -561,6 +561,7 @@ class DocumentViewSet( all_serializer_class = serializers.ListDocumentSerializer children_serializer_class = serializers.ListDocumentSerializer descendants_serializer_class = serializers.ListDocumentSerializer + favorite_list_serializer_class = serializers.ListDocumentSerializer list_serializer_class = serializers.ListDocumentSerializer trashbin_serializer_class = serializers.ListDocumentSerializer tree_serializer_class = serializers.ListDocumentSerializer @@ -605,6 +606,9 @@ class DocumentViewSet( queryset = queryset.annotate_is_favorite(user) queryset = queryset.annotate_user_roles(user) queryset = queryset.annotate_user_has_link_trace(user) + # Detail views only — `list` builds its own annotation chain below and does not need + # this one, so the list endpoint keeps its current query cost + queryset = queryset.annotate_user_access_since(user) return queryset @@ -1805,16 +1809,14 @@ class DocumentViewSet( document = self.get_object() # Users should not see version history dating from before they gained access to the - # document. Filter to get the minimum access date for the logged-in user - access_queryset = models.DocumentAccess.objects.filter( - db.Q(user=user) | db.Q(team__in=user.teams), - document__path=Left(db.Value(document.path), Length("document__path")), - ).aggregate(min_date=db.Min("created_at")) - - # Handle the case where the user has no accesses - min_datetime = access_queryset["min_date"] - if not min_datetime: - return drf.exceptions.PermissionDenied( + # document. `user_access_since` is annotated onto the queryset (see + # `DocumentQuerySet.annotate_user_access_since`) and is the one definition of that + # date — the collaboration server is handed the same value to bound the history it + # serves. It is None for a user who reaches the document through its link reach + # alone: no access, no date, and so no history. + min_datetime = document.user_access_since + if min_datetime is None: + raise drf.exceptions.PermissionDenied( "Only users with specific access can see version history" ) @@ -1836,22 +1838,21 @@ class DocumentViewSet( """Custom action to retrieve a specific version of a document""" document = self.get_object() + # Don't let users access versions that were created before they were given access to + # the document — the same cut-off as `versions_list`, from the same annotation. + # Checked before the object is fetched: a caller who may see no version at all should + # not learn from a 404 whether this one exists. + min_datetime = document.user_access_since + if min_datetime is None: + raise drf.exceptions.PermissionDenied( + "Only users with specific access can see version history" + ) + try: response = document.get_content_response(version_id=version_id) except (FileNotFoundError, ClientError) as err: raise Http404 from err - # Don't let users access versions that were created before they were given access - # to the document - user = request.user - min_datetime = min( - access.created_at - for access in models.DocumentAccess.objects.filter( - db.Q(user=user) | db.Q(team__in=user.teams), - document__path=Left(db.Value(document.path), Length("document__path")), - ) - ) - if response["LastModified"] < min_datetime: raise Http404 diff --git a/src/backend/core/models.py b/src/backend/core/models.py index 34e54c9cc..99bc53493 100644 --- a/src/backend/core/models.py +++ b/src/backend/core/models.py @@ -924,6 +924,46 @@ class DocumentQuerySet(MP_NodeQuerySet): user_roles=models.Value([], output_field=output_field), ) + def annotate_user_access_since(self, user): + """ + Annotate document queryset with the moment the current user gained access to the + document — the earliest access they hold on it or on one of its ancestors. + + This is the point from which they may see the document's history: the collaboration + server turns it into a `history.from` permission, and the version endpoints filter on + it. `None` when the user reaches the document through its link reach alone, which is + not an access and carries no date — those users get no history at all, deliberately + (see the comment in `get_abilities`). + """ + if user.is_authenticated: + # the same ancestor-aware subquery as `annotate_user_roles`: the access's document + # path is a prefix of this one's, so it matches the document and every ancestor + user_access_since_subquery = ( + DocumentAccess.objects.filter( + models.Q(user=user) | models.Q(team__in=user.teams), + document__path=Left( + models.OuterRef("path"), Length("document__path") + ), + ) + .order_by() + .values("user") + .annotate(min_created_at=models.Min("created_at")) + .values("min_created_at") + ) + + return self.annotate( + user_access_since=models.Subquery( + user_access_since_subquery, + output_field=models.DateTimeField(), + ) + ) + + return self.annotate( + user_access_since=models.Value( + None, output_field=models.DateTimeField() + ), + ) + def annotate_user_has_link_trace(self, user): """ Annotate document queryset with a boolean to know if the current user 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 c4cd90b3a..fc7625e07 100644 --- a/src/backend/core/tests/documents/test_api_documents_retrieve.py +++ b/src/backend/core/tests/documents/test_api_documents_retrieve.py @@ -83,6 +83,7 @@ def test_api_documents_retrieve_anonymous_public_standalone(): "path": document.path, "title": document.title, "updated_at": document.updated_at.isoformat().replace("+00:00", "Z"), + "user_access_since": None, "user_role": None, } @@ -159,6 +160,7 @@ def test_api_documents_retrieve_anonymous_public_parent(): "path": document.path, "title": document.title, "updated_at": document.updated_at.isoformat().replace("+00:00", "Z"), + "user_access_since": None, "user_role": None, } @@ -268,6 +270,7 @@ def test_api_documents_retrieve_authenticated_unrelated_public_or_authenticated( "path": document.path, "title": document.title, "updated_at": document.updated_at.isoformat().replace("+00:00", "Z"), + "user_access_since": None, "user_role": None, } assert ( @@ -351,6 +354,7 @@ def test_api_documents_retrieve_authenticated_public_or_authenticated_parent(rea "path": document.path, "title": document.title, "updated_at": document.updated_at.isoformat().replace("+00:00", "Z"), + "user_access_since": None, "user_role": None, } @@ -465,6 +469,9 @@ def test_api_documents_retrieve_authenticated_related_direct(): "path": document.path, "title": document.title, "updated_at": document.updated_at.isoformat().replace("+00:00", "Z"), + "user_access_since": access.created_at.isoformat().replace( + "+00:00", "Z" + ), "user_role": access.role, } @@ -548,6 +555,9 @@ def test_api_documents_retrieve_authenticated_related_parent(): "path": document.path, "title": document.title, "updated_at": document.updated_at.isoformat().replace("+00:00", "Z"), + "user_access_since": access.created_at.isoformat().replace( + "+00:00", "Z" + ), "user_role": access.role, } @@ -678,6 +688,14 @@ def test_api_documents_retrieve_authenticated_related_team_members( factories.TeamDocumentAccessFactory(document=document, team="owners", role="owner") factories.TeamDocumentAccessFactory(document=document) factories.TeamDocumentAccessFactory() + # the history this user may read starts at the earliest access they hold — + # here through one of their teams + expected_access_since = ( + models.DocumentAccess.objects.filter(document=document, team__in=teams) + .earliest("created_at") + .created_at.isoformat() + .replace("+00:00", "Z") + ) response = client.get(f"/api/v1.0/documents/{document.id!s}/") @@ -704,6 +722,7 @@ def test_api_documents_retrieve_authenticated_related_team_members( "path": document.path, "title": document.title, "updated_at": document.updated_at.isoformat().replace("+00:00", "Z"), + "user_access_since": expected_access_since, "user_role": role, } @@ -744,6 +763,14 @@ def test_api_documents_retrieve_authenticated_related_team_administrators( factories.TeamDocumentAccessFactory(document=document, team="owners", role="owner") factories.TeamDocumentAccessFactory(document=document) factories.TeamDocumentAccessFactory() + # the history this user may read starts at the earliest access they hold — + # here through one of their teams + expected_access_since = ( + models.DocumentAccess.objects.filter(document=document, team__in=teams) + .earliest("created_at") + .created_at.isoformat() + .replace("+00:00", "Z") + ) response = client.get(f"/api/v1.0/documents/{document.id!s}/") @@ -770,6 +797,7 @@ def test_api_documents_retrieve_authenticated_related_team_administrators( "path": document.path, "title": document.title, "updated_at": document.updated_at.isoformat().replace("+00:00", "Z"), + "user_access_since": expected_access_since, "user_role": role, } @@ -810,6 +838,14 @@ def test_api_documents_retrieve_authenticated_related_team_owners( factories.TeamDocumentAccessFactory(document=document, team="owners", role="owner") factories.TeamDocumentAccessFactory(document=document) factories.TeamDocumentAccessFactory() + # the history this user may read starts at the earliest access they hold — + # here through one of their teams + expected_access_since = ( + models.DocumentAccess.objects.filter(document=document, team__in=teams) + .earliest("created_at") + .created_at.isoformat() + .replace("+00:00", "Z") + ) response = client.get(f"/api/v1.0/documents/{document.id!s}/") @@ -836,6 +872,7 @@ def test_api_documents_retrieve_authenticated_related_team_owners( "path": document.path, "title": document.title, "updated_at": document.updated_at.isoformat().replace("+00:00", "Z"), + "user_access_since": expected_access_since, "user_role": role, } @@ -1053,3 +1090,58 @@ def test_api_documents_retrieve_permanently_deleted_related(role, depth): assert response.status_code == 404 assert response.json() == {"detail": "Not found."} + + +def test_api_documents_retrieve_user_access_since_is_ancestor_aware(): + """ + `user_access_since` is the earliest access the user holds on the document or on any of + its ancestors. It is what bounds the history they may read — the collaboration server + turns it into `history.from` — so a later access on the document itself must not shorten + what an earlier one on a parent already gave them. + """ + user = factories.UserFactory() + client = APIClient() + client.force_login(user) + + parent = factories.DocumentFactory(link_reach="restricted") + child = factories.DocumentFactory(parent=parent, link_reach="restricted") + + ten_days_ago = timezone.now() - timedelta(days=10) + with mock.patch("django.utils.timezone.now", return_value=ten_days_ago): + parent_access = factories.UserDocumentAccessFactory( + document=parent, user=user, role="reader" + ) + # granted later, and deliberately the stronger role: recency must not win + factories.UserDocumentAccessFactory(document=child, user=user, role="editor") + + response = client.get(f"/api/v1.0/documents/{child.id!s}/") + + assert response.status_code == 200 + expected = parent_access.created_at.isoformat().replace("+00:00", "Z") + assert response.json()["user_access_since"] == expected + + +def test_api_documents_retrieve_user_access_since_is_null_without_an_access(): + """ + A user who reaches a document through its link reach alone holds no access, so there is + no date to bound their history with — and they get none at all. This is the reason the + version endpoints have always refused them, and the collaboration server withholds the + `history` facet on the same grounds. + """ + document = factories.DocumentFactory(link_reach="public", link_role="editor") + + # anonymous + assert ( + APIClient().get(f"/api/v1.0/documents/{document.id!s}/").json()[ + "user_access_since" + ] + is None + ) + + # signed in, but still reaching it only through the link + client = APIClient() + client.force_login(factories.UserFactory()) + response = client.get(f"/api/v1.0/documents/{document.id!s}/") + + assert response.status_code == 200 + assert response.json()["user_access_since"] is None diff --git a/src/helm/impress/values.yaml b/src/helm/impress/values.yaml index eacc127ff..e51c69c16 100644 --- a/src/helm/impress/values.yaml +++ b/src/helm/impress/values.yaml @@ -127,8 +127,14 @@ ingressCollaborationApi: ## ## `jwks` is public on purpose: it carries the public halves of the keys ## yhub signs with, and nothing else. + ## + ## `activity` and `changeset` carry the editing history, bounded per user to + ## the moment they were given access to the document — a user who holds no + ## access on it, only its link, is refused both. paths: - /collaboration/ydoc/ + - /collaboration/activity/ + - /collaboration/changeset/ - /collaboration/jwks/ ## @param ingressCollaborationApi.hosts Additional host to configure for the Ingress hosts: [] diff --git a/src/yhub-server/README.md b/src/yhub-server/README.md index 3e63bfc2a..f5ab27427 100644 --- a/src/yhub-server/README.md +++ b/src/yhub-server/README.md @@ -80,14 +80,14 @@ It is not a fork of yhub — it is a thin wrapper: - mirrors the environment conventions used elsewhere in this repository (`*_FILE` secret indirection, `COLLABORATION_SERVER_ORIGIN` allowlist, …). -Public exposure: the browser needs two routes, the websocket -`/collaboration/ws/` and `/collaboration/ydoc/` for the http fallback, plus +Public exposure: the browser needs the websocket `/collaboration/ws/`, +`/collaboration/ydoc/` for the http fallback, `/collaboration/activity/` and +`/collaboration/changeset/` for the editing history, plus `/collaboration/jwks/v1`, which carries public keys and nothing else. Every -other route this server serves — `rollback`, `prune`, `changeset`, `activity`, -`reset-connections`, `migrate`, `create-ydoc`, `restore-ydoc`, `reset-ydoc` — -is now refused to a browser by the permission tables themselves (see "Access -control" below), so publishing one is no longer the security boundary it was -under yhub 0.7. Keep them off the +other route this server serves — `rollback`, `prune`, `reset-connections`, +`migrate`, `create-ydoc`, `restore-ydoc`, `reset-ydoc` — is refused to a browser +by the permission tables themselves (see "Access control" below), so publishing +one is no longer the security boundary it was under yhub 0.7. Keep them off the public ingress all the same: an endpoint that cannot be reached cannot be probed. The two probes are not worth publishing either — kubelet calls them from inside — and the helm chart's ingress lists what it routes rather than what it @@ -105,21 +105,25 @@ out of `server.js` so they can be read and tested without redis and postgres. Masks are positional `crud` strings where `-` denies, so `'-r--'` is read-only. -| Facet | Reader | Editor | Admin token | -|---|---|---|---| -| `ydoc` | `-r--` | `-ru-` | `cru-` | -| `awareness` | `-r--` | `-ru-` | `-ru-` | -| `history` | — | — | `from: 0` | -| `delete` | — | — | `['soft']` | -| `endpoint.ws` | `-r--` | `-ru-` | `crud` (`'*'`) | -| `endpoint.ydoc` | `-r--` | `-ru-` | `crud` (`'*'`) | -| every other endpoint | — | — | `crud` (`'*'`) | +| Facet | Reader | Editor | Link-only reader | Admin token | +|---|---|---|---|---| +| `ydoc` | `-r--` | `-ru-` | as reader/editor | `cru-` | +| `awareness` | `-r--` | `-ru-` | as reader/editor | `-ru-` | +| `history` | `from: ` | `from: ` | — | `from: 0` | +| `delete` | — | — | — | `['soft']` | +| `endpoint.ws` | `-r--` | `-ru-` | as reader/editor | `crud` (`'*'`) | +| `endpoint.ydoc` | `-r--` | `-ru-` | as reader/editor | `crud` (`'*'`) | +| `endpoint.activity` | `-r--` | `-r--` | — | `crud` (`'*'`) | +| `endpoint.changeset` | `-r--` | `-r--` | — | `crud` (`'*'`) | +| every other endpoint | — | — | — | `crud` (`'*'`) | -Reader and editor are the same document permission, `browserDocumentPermissions`, -switched on the backend's `abilities.update`; `abilities.retrieve` decided -whether there is any access at all before that. +All three browser columns are the same document permission, +`browserDocumentPermissions`, switched on two things the backend sends: +`abilities.update` for reader-vs-editor, and `user_access_since` for whether +there is a history to read. `abilities.retrieve` decided whether there is any +access at all before either. -Four of those cells are decisions rather than transcriptions: +Five of those cells are decisions rather than transcriptions: - **`awareness: '-r--'` for a reader.** A reader receives presence and never publishes it — [suitenumerique/docs#2544](https://github.com/suitenumerique/docs/pull/2544), @@ -131,14 +135,32 @@ Four of those cells are decisions rather than transcriptions: a feature. The frontend has to know it too: the http fallback provider has no receive-only setting, so a reader's `HttpProvider` is built with no awareness instance at all, or its first `PATCH` would take a 403 and close it for good. -- **No `'*'` endpoint fallback for the browser.** Only `ws` and `ydoc` are named, - so everything else is denied — including any endpoint a future yhub release - adds. Under 0.7 this fence was a `purpose != null` check, which `create-ydoc` - slipped through by declaring no purpose. -- **No `history` facet for the browser.** This is what makes yhub refuse a - `gc=false` connection with a 403; Docs users are served the garbage-collected - document, and the full history is the backend's business. It also keeps - `activity` and `changeset` closed even if their endpoints were ever granted. +- **No `'*'` endpoint fallback for the browser.** Only the four routes above are + named, so everything else is denied — including any endpoint a future yhub + release adds. Under 0.7 this fence was a `purpose != null` check, which + `create-ydoc` slipped through by declaring no purpose. +- **`history.from` is the moment the user got access**, not the beginning of the + document. It is the backend's `user_access_since` — the earliest access they + hold on the document or on one of its ancestors — and it is the same rule the + version endpoints have always applied ("only those created after the user got + access to the document"). yhub clamps `from` up to it on every + `activity`/`changeset` read, so a client asks for whatever range it likes and + gets back only its own share: the bound is silent, enforced server-side, and a + stale client cannot widen it. Two properties fall out of it and are worth + keeping true: + - a **`gc=false` connection stays refused**, because that requires + `from === 0` exactly and a real access date never is; + - the ray is a **stored** bound (`DocumentAccess.created_at`), not a + wall-clock-relative one, which is what yhub's determinism contract asks for — + it re-derives identically on every websocket recheck instead of flapping the + connection. +- **A reader who holds no access, only the link, gets no history.** There is no + access row and so no date; the backend has always refused those users their + version history for exactly that reason ("we wouldn't know from which date to + allow them anyway"). `activity` and `changeset` are withheld together with the + ray rather than granted alone, which would open a route that answers 403 by + itself. `rollback` and `prune` are withheld from everyone: they are + destructive and are granted by name. - **`delete: ['soft']` and not `'hard'` for the admin.** yhub 0.8 made `DELETE /ydoc?hard=true` reachable over REST for the first time. Docs keeps irreversible erasure programmatic, behind `reset-ydoc` (see "Deletion"). diff --git a/src/yhub-server/permissions.js b/src/yhub-server/permissions.js index 9c819f3ea..0f556b3a0 100644 --- a/src/yhub-server/permissions.js +++ b/src/yhub-server/permissions.js @@ -29,27 +29,51 @@ * `create-ydoc` with the admin token. `u` alone already creates the document on * first write. * - * No `history` facet, and that is load-bearing rather than an omission: it is - * what makes yhub refuse a `gc=false` connection with a 403. Docs users are - * served the garbage-collected document; the full history is the backend's. + * `historyFrom` is the moment this user gained access to the document, in unix + * milliseconds — the backend's `user_access_since`, which is the earliest access + * they hold on the document or on one of its ancestors. It becomes the start of + * the history they may read, which is the rule Docs has always had rather than a + * new one: the version endpoints have always shown "only those created after the + * user got access to the document". yhub clamps `from` up to this on every + * changeset/activity read, so a client asks for whatever range it likes and gets + * back only its own share — it never has to know the bound, and a stale or + * modified one cannot widen it. + * + * `null` for a reader who reaches the document by link alone. There is no access + * row and so no date, and the backend has always refused those users their + * history for exactly that reason: "we wouldn't know from which date to allow + * them anyway" (`Document.get_abilities`). Without the facet, `activity` and + * `changeset` answer 403 on their own, so their endpoint entries are withheld + * together with it rather than granting a route that opens nothing. + * + * A bounded ray is not a wall-clock-relative grant: it comes from a stored + * `created_at`, so it re-derives identically on every websocket recheck, which is + * what yhub's determinism contract asks for. And it never unlocks a `gc=false` + * connection, which requires `from === 0` exactly — see the guard in server.js. * * No `delete` facet: deleting a document is Django's, through the admin token. + * Deliberately absent too: `rollback` and `prune`, which are destructive and are + * granted by name — restoring a version is not something a reader, or an editor, + * does through this grant today. * - * No `'*'` endpoint fallback, so everything not named here is denied. The browser - * calls exactly two routes — the websocket, and `ydoc` for the http fallback. - * `activity`, `changeset`, `rollback`, `prune` and every custom endpoint are - * closed to it. Under 0.7 this fence was a `purpose != null` check in - * `getAccessType`, which `create-ydoc` slipped through by declaring no purpose. + * No `'*'` endpoint fallback, so everything not named here is denied — including + * any endpoint a future yhub release adds. Under 0.7 this fence was a + * `purpose != null` check in `getAccessType`, which `create-ydoc` slipped through + * by declaring no purpose. */ -export const browserDocumentPermissions = (canEdit) => ({ +export const browserDocumentPermissions = (canEdit, historyFrom = null) => ({ type: 'permissions:document:v1', ydoc: canEdit ? '-ru-' : '-r--', awareness: canEdit ? '-ru-' : '-r--', + ...(historyFrom ? { history: { from: historyFrom } } : null), endpoint: { // `r` opens the socket, `u` admits document updates over it ws: canEdit ? '-ru-' : '-r--', // GET is `r` and PATCH is `u`; DELETE (`d`) stays out — see `delete` above ydoc: canEdit ? '-ru-' : '-r--', + // the editing timeline, and one point in it — both GET-only, both clamped to + // the ray above + ...(historyFrom ? { activity: '-r--', changeset: '-r--' } : null), }, }); diff --git a/src/yhub-server/permissions.test.js b/src/yhub-server/permissions.test.js index 072a42f97..9a4e796c7 100644 --- a/src/yhub-server/permissions.test.js +++ b/src/yhub-server/permissions.test.js @@ -26,8 +26,22 @@ import { * not as a snapshot of the objects: a table may be respelled freely, but it may * not start answering a question differently. */ -const reader = normalizePermissions(browserDocumentPermissions(false)); -const editor = normalizePermissions(browserDocumentPermissions(true)); +/** + * When these two were given access to the document. Any positive number would + * do — what the assertions care about is that it is the value the ray starts at, + * and that it is not zero. + */ +const ACCESS_SINCE = 1_700_000_000_000; + +const reader = normalizePermissions( + browserDocumentPermissions(false, ACCESS_SINCE), +); +const editor = normalizePermissions( + browserDocumentPermissions(true, ACCESS_SINCE), +); +// the same two reaching the document by link alone: no access row, so no date +const linkReader = normalizePermissions(browserDocumentPermissions(false)); +const linkEditor = normalizePermissions(browserDocumentPermissions(true)); const admin = normalizePermissions(adminDocumentPermissions); const grants = (permissions, required) => @@ -87,13 +101,104 @@ describe('the http fallback route', () => { }); }); +describe('the history a user may read', () => { + /** + * The rule Docs has always had, now expressed as a permission: a user sees the + * document's history from the moment they were given access to it, and no + * further back. yhub clamps every changeset/activity read up to this, so the + * bound is enforced on the server and the client never has to know it. + */ + it('starts the ray where the user got access', () => { + for (const who of [reader, editor]) { + assert.equal(grants(who, { history: { from: ACCESS_SINCE } }), true); + } + }); + + it('does not reach back before that', () => { + // a requirement asking for a wider ray than the grant is not contained in it + for (const who of [reader, editor]) { + assert.equal(grants(who, { history: { from: ACCESS_SINCE - 1 } }), false); + assert.equal(grants(who, { history: { from: 0 } }), false); + } + }); + + it('opens the timeline and one point in it, read only', () => { + for (const who of [reader, editor]) { + for (const name of ['activity', 'changeset']) { + assert.equal(grants(who, { endpoint: { [name]: '-r--' } }), true); + // GET only: neither endpoint has another verb, and granting one would + // be granting a route that does not exist + assert.equal(grants(who, { endpoint: { [name]: '--u-' } }), false); + } + } + }); + + it('never grants the full ray, which is what would unlock gc=false', () => { + // `gc=false` demands `history.from === 0` exactly. A real access date is + // ~1.8e12, so this can only regress through a bug — assert on the object + // rather than through `hasPermissions`, because it is the literal value + // that matters here + for (const who of [reader, editor]) { + assert.notEqual(who.history, false); + assert.ok(who.history.from > 0); + } + }); + + it('never grants rollback or prune', () => { + // destructive, granted by name, and restoring a version is not something + // this grant does + for (const who of [reader, editor]) { + assert.equal( + grants(who, { history: { from: ACCESS_SINCE, rollback: true } }), + false, + ); + assert.equal( + grants(who, { history: { from: ACCESS_SINCE, prune: true } }), + false, + ); + assert.equal(grants(who, { endpoint: { rollback: 'c---' } }), false); + assert.equal(grants(who, { endpoint: { prune: 'c---' } }), false); + } + }); +}); + +describe('a reader who holds no access, only the link', () => { + /** + * There is no access row and so no date. The backend has always refused these + * users their version history for exactly that reason — "we wouldn't know from + * which date to allow them anyway" — and the grant says the same thing by + * withholding the facet. + */ + it('gets no history at all', () => { + for (const who of [linkReader, linkEditor]) { + assert.equal(who.history, false); + assert.equal(grants(who, { history: { from: 0 } }), false); + assert.equal(grants(who, { history: { from: ACCESS_SINCE } }), false); + } + }); + + it('cannot reach the timeline either', () => { + // withheld together with the ray: without it these two answer 403 on their + // own, so granting them would open nothing and only muddy the grant + for (const who of [linkReader, linkEditor]) { + for (const name of ['activity', 'changeset']) { + assert.equal(grants(who, { endpoint: { [name]: '-r--' } }), false); + } + } + }); + + it('still reads and syncs the document like anyone else', () => { + assert.equal(grants(linkReader, { ydoc: '-r--' }), true); + assert.equal(grants(linkEditor, { ydoc: '--u-' }), true); + assert.equal(grants(linkEditor, { endpoint: { ws: '--u-' } }), true); + }); +}); + describe('everything the browser must not reach', () => { // there is no '*' fallback in the browser grant, so an endpoint yhub adds in // a future release is denied until it is named — this is the property that // replaced 0.7's `purpose != null` check for (const name of [ - 'activity', - 'changeset', 'rollback', 'prune', 'create-ydoc', @@ -108,10 +213,6 @@ describe('everything the browser must not reach', () => { assert.equal(grants(editor, { endpoint: { [name]: 'c---' } }), false); }); } - - it('withholds history, which is what refuses a gc=false connection', () => { - assert.equal(grants(editor, { history: { from: 0 } }), false); - }); }); describe('the admin token', () => { diff --git a/src/yhub-server/server.js b/src/yhub-server/server.js index cc068cb02..84d2c1626 100644 --- a/src/yhub-server/server.js +++ b/src/yhub-server/server.js @@ -450,7 +450,19 @@ const auth = createAuthPlugin({ if (SOFT_MIGRATION) { await seedFromLegacyStore({ org, docid, branch }); } - return browserDocumentPermissions(doc.abilities.update === true); + // When this caller was given access, which is where the history they may + // read starts. The backend sends ISO-8601 (null for a link-reach reader, + // who holds no access and so has no date); `history.from` is unix ms. + // + // Anything unparseable is *no* history rather than full history, and zero + // is refused with it: `from: 0` is the one value that also unlocks a + // `gc=false` websocket, and no real access date is ever zero, so a zero + // here could only ever be a bug upstream. + const accessSince = Date.parse(doc.user_access_since ?? ''); + return browserDocumentPermissions( + doc.abilities.update === true, + Number.isFinite(accessSince) && accessSince > 0 ? accessSince : null, + ); }, async global() { return publicGlobalPermissions;