fix: enforce inherited read-only access for pages in a view-only shared space (#386)

appflowy · claude · web-flow · commit a5ba5b2cb38c · 2026-06-01T15:15:56.000+08:00
* fix: enforce inherited read-only access for pages in a view-only shared space

Pages inside a private space shared with View-only access carried no
explicit access_level of their own, so getViewReadOnlyStatus treated them
as editable. Resolve the effective access level by walking the ancestor
chain inside the hidden "Shared with me" space and inheriting from the
nearest ancestor that declares an access_level (an explicit child re-share
still overrides the inherited space level).

- Add findSharedAccessLevel() to resolve inherited shared access
- Consolidate getViewReadOnlyStatus traversal: findView already resolves
  views inside the shared space, so drop the redundant findViewInShareWithMe
  walk and its duplicate space lookup
- Cover the inheritance logic with unit tests and assert the read-only
  editor gate end-to-end in the public-to-private BDD scenario

Co-Authored-By: Claude Opus 4.8 (1M context) &lt;noreply@anthropic.com&gt;

* fix: show "in Trash" for embedded database whose container is trashed

When an embedded database's container page is moved to trash, the block
showed "permanently deleted" instead of "in Trash". The block's viewId is
a child of the container, and once the container is trashed
ViewService.get(viewId) returns 404, so viewMeta becomes null and the
parent id is lost. The trash list only contains the container id (never
the embedded child view id), so the trash match failed and the block was
misclassified as permanently deleted.

Cache the parent id (keyed by viewId) while the view is still reachable
and fall back to it in the trash check when viewMeta is null. Fixes the
consistently-failing database-sidebar-deletion Scenario 1 and Scenario 6.

Co-Authored-By: Claude Opus 4.8 (1M context) &lt;noreply@anthropic.com&gt;

---------

Co-authored-by: Claude Opus 4.8 (1M context) &lt;noreply@anthropic.com&gt;
diff --git a/playwright/bdd/features/page/public-to-private-move-access.feature b/playwright/bdd/features/page/public-to-private-move-access.feature
@@ -32,6 +32,8 @@ Feature: Public page moved under private page access
     When I sign in as seeded public-to-private "member 1"
     And I open the temporary public-to-private movable page
     Then the temporary public-to-private movable page title is visible
+    And the temporary public-to-private movable page editor is read-only
+    And typing in the temporary public-to-private movable page editor has no effect
     When I sign in as seeded public-to-private "member 2"
     And I open the temporary public-to-private movable page
     Then the public-to-private no access page is shown
diff --git a/playwright/bdd/steps/public-to-private-move-access.steps.ts b/playwright/bdd/steps/public-to-private-move-access.steps.ts
@@ -2,7 +2,7 @@ import { APIRequestContext, expect, Page } from '@playwright/test';
 import { createBdd } from 'playwright-bdd';
 
 import { signInWithPasswordViaUi } from '../../support/auth-flow-helpers';
-import { PageSelectors, ShareSelectors, SidebarSelectors } from '../../support/selectors';
+import { EditorSelectors, PageSelectors, ShareSelectors, SidebarSelectors } from '../../support/selectors';
 import { setupPageErrorHandling, TestConfig } from '../../support/test-config';
 
 const { Given, When, Then, Before, After } = createBdd();
@@ -73,7 +73,7 @@ After(async ({ page, request }) => {
 
 Given('the seeded ptp0527 public-to-private fixture exists', async () => {
   // Created by AppFlowy-Cloud-Premium:
-  // PUBLIC_TO_PRIVATE_WEB_PREFIX=ptp0527 cargo test --test public_to_private_access_seed ...
+  // cargo test --test public_to_private_access_seed seed_public_to_private_move_web_suite -- --ignored
 });
 
 Given('I sign in as seeded public-to-private {string}', async ({ page, request }, accountAliasValue: string) => {
@@ -235,6 +235,34 @@ Then('the temporary public-to-private movable page title is visible', async ({ p
   await expect(page.getByText(movablePage.pageTitle, { exact: true }).first()).toBeVisible({ timeout: 30000 });
 });
 
+Then('the temporary public-to-private movable page editor is read-only', async ({ page }) => {
+  // A View-only member viewing a page inside a private space must not be able
+  // to edit it. Slate renders the editor with contenteditable="false" when the
+  // page resolves to read-only, so assert the inherited access is enforced.
+  const editor = EditorSelectors.firstEditor(page);
+
+  await expect(editor).toBeVisible({ timeout: 30000 });
+  await expect(editor).toHaveAttribute('contenteditable', 'false');
+});
+
+Then('typing in the temporary public-to-private movable page editor has no effect', async ({ page }) => {
+  // Prove the read-only gate actually rejects edits: focus the editor, type a
+  // sentinel, and assert it never lands in the document content.
+  const sentinel = 'PTP_READONLY_EDIT_ATTEMPT';
+  const editor = EditorSelectors.firstEditor(page);
+
+  await expect(editor).toBeVisible({ timeout: 30000 });
+
+  const before = (await editor.textContent()) ?? '';
+
+  await editor.click({ position: { x: 40, y: 20 } }).catch(() => undefined);
+  await page.keyboard.type(sentinel);
+  await page.waitForTimeout(500);
+
+  await expect(editor).not.toContainText(sentinel);
+  await expect(editor).toHaveText(before);
+});
+
 Then('the public-to-private share panel only shows {string}', async ({ page }, aliasesValue: string) => {
   const expectedEmails = parseAccountAliases(aliasesValue).map(accountEmail);
   const popover = ShareSelectors.sharePopover(page);
diff --git a/src/components/_shared/outline/utils.ts b/src/components/_shared/outline/utils.ts
@@ -1,4 +1,4 @@
-import { View, ViewLayout } from '@/application/types';
+import { AccessLevel, View, ViewLayout } from '@/application/types';
 
 export function filterViews (views: View[], keyword: string): View[] {
   const filterAndFlatten = (views: View[]): View[] => {
@@ -217,4 +217,41 @@ export function findViewInShareWithMe (views: View[], targetViewId: string): Vie
   }
 
   return findView(shareWithMeSpace.children, targetViewId);
-}
+}
+
+/**
+ * Resolve the effective access level for a view shared with the current user.
+ *
+ * A shared private space carries its `access_level` on the space node; child
+ * pages inside it have no explicit level of their own. Walk the ancestor chain
+ * inside the hidden "Shared with me" space and return the nearest ancestor that
+ * declares an `access_level` (the target view itself first), so an explicit
+ * child re-share overrides the inherited space access.
+ *
+ * Returns `undefined` when the view is not part of the "Shared with me" space
+ * (e.g. a view the user owns), in which case no shared access restriction
+ * applies.
+ */
+export function findSharedAccessLevel (views: View[], targetViewId: string): AccessLevel | undefined {
+  const shareWithMeSpace = findShareWithMeSpace(views);
+
+  if (!shareWithMeSpace?.children) {
+    return undefined;
+  }
+
+  const path = findAncestors(shareWithMeSpace.children, targetViewId);
+
+  if (!path) {
+    return undefined;
+  }
+
+  for (let i = path.length - 1; i >= 0; i--) {
+    const accessLevel = path[i].access_level;
+
+    if (accessLevel !== undefined) {
+      return accessLevel;
+    }
+  }
+
+  return undefined;
+}
diff --git a/src/components/app/hooks/__tests__/useViewOperations.privateSpaceAccess.test.ts b/src/components/app/hooks/__tests__/useViewOperations.privateSpaceAccess.test.ts
@@ -0,0 +1,122 @@
+import { AccessLevel, View, ViewExtra, ViewLayout } from '@/application/types';
+import { getPlatform } from '@/utils/platform';
+
+import { getViewReadOnlyStatus } from '../useViewOperations';
+
+// useViewOperations pulls in service/loader modules at import time; stub the
+// heavy ones so the pure getViewReadOnlyStatus function can be tested in isolation.
+jest.mock('@/application/services/domains', () => ({
+  CollabService: {},
+  ViewService: {},
+  WorkspaceService: {},
+}));
+jest.mock('@/application/view-loader', () => ({ openView: jest.fn() }));
+jest.mock('@/utils/platform', () => ({ getPlatform: jest.fn(() => ({ isMobile: false })) }));
+
+const mockGetPlatform = getPlatform as jest.MockedFunction<typeof getPlatform>;
+
+function createView(overrides: Partial<View>): View {
+  return {
+    view_id: 'view-id',
+    name: 'View',
+    icon: null,
+    layout: ViewLayout.Document,
+    extra: null,
+    children: [],
+    is_published: false,
+    is_private: false,
+    ...overrides,
+  };
+}
+
+/**
+ * Build the hidden "Shared with me" space holding a shared private space that
+ * itself contains a child page.
+ *
+ * This mirrors the user-reported scenario: an owner creates a private space,
+ * invites a member with View-only (ReadOnly) access, and the member opens a
+ * page *inside* that space. The server reports the access level on the shared
+ * space node; the child page does not carry its own `access_level`.
+ */
+function shareWithMePrivateSpaceOutline(spaceAccessLevel: AccessLevel, childPage: View): View[] {
+  return [
+    createView({
+      view_id: 'share-with-me-space',
+      extra: { is_space: true, is_hidden_space: true } as ViewExtra,
+      children: [
+        createView({
+          view_id: 'shared-private-space',
+          is_private: true,
+          extra: { is_space: true } as ViewExtra,
+          access_level: spaceAccessLevel,
+          children: [childPage],
+        }),
+      ],
+    }),
+  ];
+}
+
+describe('getViewReadOnlyStatus (private space inherited access)', () => {
+  beforeEach(() => {
+    mockGetPlatform.mockReturnValue({ isMobile: false } as ReturnType<typeof getPlatform>);
+  });
+
+  it('treats the shared private space root as read-only for a View-only member', () => {
+    // Sanity check: the shared node itself carries the access level, so this
+    // already works today and pins the boundary of the inheritance gap below.
+    const outline = shareWithMePrivateSpaceOutline(
+      AccessLevel.ReadOnly,
+      createView({ view_id: 'child-page' })
+    );
+
+    expect(getViewReadOnlyStatus('shared-private-space', outline)).toBe(true);
+  });
+
+  it('treats a child page of a View-only private space as read-only', () => {
+    // User-reported bug: a member invited to a private space with View-only
+    // access can still edit pages *inside* the space. The child page inherits
+    // the space's ReadOnly access and must therefore be read-only, even though
+    // the page itself has no explicit `access_level`.
+    const outline = shareWithMePrivateSpaceOutline(
+      AccessLevel.ReadOnly,
+      createView({ view_id: 'child-page' })
+    );
+
+    expect(getViewReadOnlyStatus('child-page', outline)).toBe(true);
+  });
+
+  it('treats a nested page of a View-only private space as read-only', () => {
+    // Deeper nesting must still inherit the space's restricted access.
+    const outline = shareWithMePrivateSpaceOutline(
+      AccessLevel.ReadOnly,
+      createView({
+        view_id: 'child-page',
+        children: [createView({ view_id: 'grandchild-page' })],
+      })
+    );
+
+    expect(getViewReadOnlyStatus('grandchild-page', outline)).toBe(true);
+  });
+
+  it('keeps a child page editable when the private space grants write access', () => {
+    // Counter-case: a member shared into the private space with write access
+    // must be able to edit pages inside it.
+    const outline = shareWithMePrivateSpaceOutline(
+      AccessLevel.ReadAndWrite,
+      createView({ view_id: 'child-page' })
+    );
+
+    expect(getViewReadOnlyStatus('child-page', outline)).toBe(false);
+  });
+
+  it('lets an explicit child access level override the inherited space access', () => {
+    // A child re-shared with write access inside an otherwise View-only space
+    // should follow its own (more permissive) access level.
+    const outline = shareWithMePrivateSpaceOutline(
+      AccessLevel.ReadOnly,
+      createView({ view_id: 'child-page', access_level: AccessLevel.ReadAndWrite })
+    );
+
+    expect(getViewReadOnlyStatus('child-page', outline)).toBe(false);
+  });
+});
diff --git a/src/components/app/hooks/useViewOperations.ts b/src/components/app/hooks/useViewOperations.ts
@@ -16,7 +16,7 @@ import {
 } from '@/application/types';
 import { openView } from '@/application/view-loader';
 import { getDatabaseIdFromExtra, getFirstChildView, isDatabaseContainer, isDatabaseLayout } from '@/application/view-utils';
-import { findView, findViewInShareWithMe } from '@/components/_shared/outline/utils';
+import { findSharedAccessLevel, findView } from '@/components/_shared/outline/utils';
 import { CollabDocResetPayload } from '@/components/ws/sync/types';
 import { Log } from '@/utils/log';
 import { getPlatform } from '@/utils/platform';
@@ -47,20 +47,24 @@ export function getViewReadOnlyStatus(viewId: string, outline?: View[], fallback
 
   if (!outline) return false;
 
-  // Check if view exists in shareWithMe
-  const shareWithMeView = findViewInShareWithMe(outline, viewId);
-
-  // A locked page is read-only for everyone until it is unlocked.
-  const view = findView(outline, viewId) ?? shareWithMeView;
+  // A locked page is read-only for everyone until it is unlocked. The outline
+  // includes the hidden "Shared with me" space, so findView also resolves views
+  // shared with the current user.
+  const view = findView(outline, viewId);
 
   if (view?.is_locked) return true;
 
-  if (shareWithMeView?.access_level !== undefined) {
-    // If found in shareWithMe, check access level
-    return shareWithMeView.access_level <= AccessLevel.ReadAndComment;
+  // Resolve the effective shared access level, inheriting from the nearest
+  // ancestor inside the "Shared with me" space. This makes pages inside a
+  // View-only private space read-only even though the page itself carries no
+  // explicit access level.
+  const sharedAccessLevel = findSharedAccessLevel(outline, viewId);
+
+  if (sharedAccessLevel !== undefined) {
+    return sharedAccessLevel <= AccessLevel.ReadAndComment;
   }
 
-  // If not found in shareWithMe, default is false (editable)
+  // If not part of the shared-with-me space, default is false (editable)
   return false;
 }
 
diff --git a/src/components/editor/components/blocks/database/DatabaseBlock.tsx b/src/components/editor/components/blocks/database/DatabaseBlock.tsx
@@ -79,6 +79,14 @@ export const DatabaseBlock = memo(
 
     notFoundRef.current = notFound;
 
+    // Remember the database container (parent) id while the view is still reachable.
+    // Once the container is moved to trash, ViewService.get(viewId) returns 404 and
+    // viewMeta becomes null, so the response can no longer tell us the parent id. We
+    // still need it to recognize the deletion as "in trash" rather than "permanently
+    // deleted": the trash list only contains the container id, never the embedded
+    // child view id. Keyed by viewId so a stale parent is ignored if the block swaps views.
+    const lastKnownParentRef = useRef<{ viewId: string; parentId: string } | null>(null);
+
     useEffect(() => {
       const eventEmitter = context.eventEmitter;
 
@@ -112,13 +120,23 @@ export const DatabaseBlock = memo(
             return;
           }
 
-          // Build the set of IDs to check: the view itself, its parent (database container),
-          // and the document page. When a database container is trashed, only the container
-          // ID appears in trash — not its child view IDs.
+          // Cache the parent id whenever the view is reachable, so we can still resolve
+          // the container after it is trashed (at which point viewMeta is null).
+          if (viewMeta?.parent_view_id) {
+            lastKnownParentRef.current = { viewId, parentId: viewMeta.parent_view_id };
+          }
+
+          // Build the set of IDs to check: the view itself and its parent (database
+          // container). When a database container is trashed, only the container ID
+          // appears in trash — not its child view IDs — so fall back to the last known
+          // parent id when the trashed view no longer reports its metadata.
+          const cachedParentId =
+            lastKnownParentRef.current?.viewId === viewId ? lastKnownParentRef.current.parentId : null;
+          const parentId = viewMeta?.parent_view_id ?? cachedParentId;
           const idsToCheck = new Set<string>([viewId]);
 
-          if (viewMeta?.parent_view_id) {
-            idsToCheck.add(viewMeta.parent_view_id);
+          if (parentId) {
+            idsToCheck.add(parentId);
           }
 
           const isInTrash = trashItems?.some((item) => idsToCheck.has(item.view_id));
