(core) Fix exports to CSV/XLSX/etc when data is restricted by access rules

Summary:
- The issue manifested as error "Cannot read property '0' of undefined" in some
  cases, and as "Blocked by table read access rules" in others (instead of
  limiting output to what's not blocked)
- Goes deeper: exports weren't respecting metadata censoring.
- The fix changes exports to use censored metadata, which addresses both errors above.
- Includes an improvement to column ordering in XLSX exports.

Test Plan: Add a server test for CSV and XLSX exports with access rules

Reviewers: paulfitz, georgegevoian

Reviewed By: paulfitz, georgegevoian

Subscribers: paulfitz

Differential Revision: https://phab.getgrist.com/D3615

app/common/isHiddenTable.ts

@@ -7,6 +7,7 @@ import {UIRowId} from 'app/common/UIRowId';
  */
 export function isHiddenTable(tablesData: TableData, tableRef: UIRowId): boolean {
   const tableId = tablesData.getValue(tableRef, 'tableId') as string|undefined;
+  // The `!tableId` check covers the case of censored tables (see isTableCensored() below).
   return !tableId || isSummaryTable(tablesData, tableRef) || tableId.startsWith('GristHidden_');
 }
 
@@ -17,3 +18,11 @@ export function isHiddenTable(tablesData: TableData, tableRef: UIRowId): boolean
 export function isSummaryTable(tablesData: TableData, tableRef: UIRowId): boolean {
   return tablesData.getValue(tableRef, 'summarySourceTable') !== 0;
 }
+
+// Check if a table record (from _grist_Tables) is censored.
+// Metadata records get censored by clearing certain of their fields, so it's expected that a
+// record may exist even though various code should consider it as hidden.
+export function isTableCensored(tablesData: TableData, tableRef: UIRowId): boolean {
+  const tableId = tablesData.getValue(tableRef, 'tableId');
+  return !tableId;
+}

app/server/lib/Export.ts

@@ -1,11 +1,13 @@
 import {ApiError} from 'app/common/ApiError';
 import {buildColFilter} from 'app/common/ColumnFilterFunc';
-import {DocData} from 'app/common/DocData';
+import {TableDataAction} from 'app/common/DocActions';
 import {DocumentSettings} from 'app/common/DocumentSettings';
 import * as gristTypes from 'app/common/gristTypes';
 import * as gutil from 'app/common/gutil';
+import {nativeCompare} from 'app/common/gutil';
+import {isTableCensored} from 'app/common/isHiddenTable';
 import {buildRowFilter} from 'app/common/RowFilterFunc';
-import {SchemaTypes} from 'app/common/schema';
+import {schema, SchemaTypes} from 'app/common/schema';
 import {SortFunc} from 'app/common/SortFunc';
 import {Sort} from 'app/common/SortSpec';
 import {MetaRowRecord, MetaTableData} from 'app/common/TableData';
@@ -78,6 +80,10 @@ export interface ExportParameters {
   filters: Filter[];
 }
 
+interface FilteredMetaTables {
+  [tableId: string]: TableDataAction;
+}
+
 /**
  * Gets export parameters from a request.
  */
@@ -95,22 +101,37 @@ export function parseExportParameters(req: express.Request): ExportParameters {
   };
 }
 
-// Makes assertion that value does exists or throws an error
+// Helper for getting filtered metadata tables.
+async function getMetaTables(activeDoc: ActiveDoc, req: express.Request): Promise<FilteredMetaTables> {
+  const docSession = docSessionFromRequest(req as RequestWithLogin);
+  return safe(await activeDoc.fetchMetaTables(docSession), "No metadata available in active document");
+}
+
+// Makes assertion that value does exist or throws an error
 function safe<T>(value: T, msg: string) {
   if (!value) { throw new ApiError(msg, 404); }
   return value as NonNullable<T>;
 }
 
-// Helper to for getting table from docData.
+// Helper for getting table from filtered metadata.
-function safeTable<TableId extends keyof SchemaTypes>(docData: DocData, name: TableId) {
+function safeTable<TableId extends keyof SchemaTypes>(metaTables: FilteredMetaTables, tableId: TableId) {
-  return safe(docData.getMetaTable(name), `No table '${name}' in document with id ${docData}`);
+  const table = safe(metaTables[tableId], `No table '${tableId}' in document`);
+  const colTypes = safe(schema[tableId], `No table '${tableId}' in document schema`);
+  return new MetaTableData<TableId>(tableId, table, colTypes);
 }
 
-// Helper for getting record safe
+// Helper for getting record safely: it throws if the record is missing.
 function safeRecord<TableId extends keyof SchemaTypes>(table: MetaTableData<TableId>, id: number) {
   return safe(table.getRecord(id), `No record ${id} in table ${table.tableId}`);
 }
 
+// Check that tableRef points to an uncensored table, or throw otherwise.
+function checkTableAccess(tables: MetaTableData<"_grist_Tables">, tableRef: number): void {
+  if (isTableCensored(tables, tableRef)) {
+    throw new ApiError(`Cannot find or access table`, 404);
+  }
+}
+
 /**
  * Builds export for all raw tables that are in doc.
  * @param activeDoc Active document
@@ -119,13 +140,14 @@ function safeRecord<TableId extends keyof SchemaTypes>(table: MetaTableData<Tabl
 export async function exportDoc(
   activeDoc: ActiveDoc,
   req: express.Request) {
-  const docData = safe(activeDoc.docData, "No docData in active document");
+  const metaTables = await getMetaTables(activeDoc, req);
-  const tables = safeTable(docData, '_grist_Tables');
+  const tables = safeTable(metaTables, '_grist_Tables');
   // select raw tables
-  const tableIds = tables.filterRowIds({ summarySourceTable: 0 });
+  const tableRefs = tables.filterRowIds({ summarySourceTable: 0 });
   const tableExports = await Promise.all(
-    tableIds
+    tableRefs
-      .map(tId => exportTable(activeDoc, tId, req))
+      .filter(tId => !isTableCensored(tables, tId))     // Omit censored tables
+      .map(tId => exportTable(activeDoc, tId, req, {metaTables}))
   );
   return tableExports;
 }
@@ -135,20 +157,25 @@ export async function exportDoc(
  */
 export async function exportTable(
   activeDoc: ActiveDoc,
-  tableId: number,
+  tableRef: number,
-  req: express.Request): Promise<ExportData> {
+  req: express.Request,
-  const docData = safe(activeDoc.docData, "No docData in active document");
+  {metaTables}: {metaTables?: FilteredMetaTables} = {},
-  const tables = safeTable(docData, '_grist_Tables');
+): Promise<ExportData> {
-  const table = safeRecord(tables, tableId);
+  metaTables = metaTables || await getMetaTables(activeDoc, req);
-  const tableColumns = safeTable(docData, '_grist_Tables_column')
+  const tables = safeTable(metaTables, '_grist_Tables');
+  checkTableAccess(tables, tableRef);
+  const table = safeRecord(tables, tableRef);
+  const tableColumns = safeTable(metaTables, '_grist_Tables_column')
     .getRecords()
+    // sort by parentPos and id, which should be the same order as in raw data
+    .sort((c1, c2) => nativeCompare(c1.parentPos, c2.parentPos) || nativeCompare(c1.id, c2.id))
     // remove manual sort column
     .filter(col => col.colId !== gristTypes.MANUALSORT);
   // Produce a column description matching what user will see / expect to export
   const tableColsById = _.indexBy(tableColumns, 'id');
   const columns = tableColumns.map(tc => {
     // remove all columns that don't belong to this table
-    if (tc.parentId !== tableId) {
+    if (tc.parentId !== tableRef) {
       return emptyCol;
     }
     // remove all helpers
@@ -183,12 +210,12 @@ export async function exportTable(
   // since tables ids are not very friendly, borrow name from a primary view
   if (table.primaryViewId) {
     const viewId = table.primaryViewId;
-    const views = safeTable(docData, '_grist_Views');
+    const views = safeTable(metaTables, '_grist_Views');
     const view = safeRecord(views, viewId);
     tableName = view.name;
   }
 
-  const docInfo = safeRecord(safeTable(docData, '_grist_DocInfo'), 1);
+  const docInfo = safeRecord(safeTable(metaTables, '_grist_DocInfo'), 1);
   const docSettings = gutil.safeJsonParse(docInfo.documentSettings, {});
   return {
     tableName,
@@ -208,18 +235,21 @@ export async function exportSection(
   viewSectionId: number,
   sortSpec: Sort.SortSpec | null,
   filters: Filter[] | null,
-  req: express.Request): Promise<ExportData> {
+  req: express.Request,
-
+  {metaTables}: {metaTables?: FilteredMetaTables} = {},
-  const docData = safe(activeDoc.docData, "No docData in active document");
+): Promise<ExportData> {
-  const viewSections = safeTable(docData, '_grist_Views_section');
+  metaTables = metaTables || await getMetaTables(activeDoc, req);
+  const viewSections = safeTable(metaTables, '_grist_Views_section');
   const viewSection = safeRecord(viewSections, viewSectionId);
-  const tables = safeTable(docData, '_grist_Tables');
+  safe(viewSection.tableRef, `Cannot find or access table`);
+  const tables = safeTable(metaTables, '_grist_Tables');
+  checkTableAccess(tables, viewSection.tableRef);
   const table = safeRecord(tables, viewSection.tableRef);
-  const columns = safeTable(docData, '_grist_Tables_column')
+  const columns = safeTable(metaTables, '_grist_Tables_column')
     .filterRecords({parentId: table.id});
-  const viewSectionFields = safeTable(docData, '_grist_Views_section_field');
+  const viewSectionFields = safeTable(metaTables, '_grist_Views_section_field');
   const fields = viewSectionFields.filterRecords({parentId: viewSection.id});
-  const savedFilters = safeTable(docData, '_grist_Filters')
+  const savedFilters = safeTable(metaTables, '_grist_Filters')
     .filterRecords({viewSectionRef: viewSection.id});
 
   const tableColsById = _.indexBy(columns, 'id');
@@ -280,7 +310,7 @@ export async function exportSection(
   // filter rows numbers
   rowIds = rowIds.filter(rowFilter);
 
-  const docInfo = safeRecord(safeTable(docData, '_grist_DocInfo'), 1);
+  const docInfo = safeRecord(safeTable(metaTables, '_grist_DocInfo'), 1);
   const docSettings = gutil.safeJsonParse(docInfo.documentSettings, {});
 
   return {

app/server/lib/GranularAccess.ts

@@ -2170,6 +2170,13 @@ const dummyAccessCheck: IAccessCheck = {
 
 /**
  * Manage censoring metadata.
+ *
+ * For most metadata, censoring means blanking out certain fields, rather than removing rows,
+ * (because the latter was too big of a change). In particular, these changes are relied on by
+ * other code:
+ *
+ *  - Censored tables (from _grist_Tables) have cleared tableId field. To check for it, use the
+ *    isTableCensored() helper in app/common/isHiddenTable.ts. This is used by exports to Excel.
  */
 export class CensorshipInfo {
   public censoredTables = new Set<number>();