(core) update column access logic to respect wildcards

Summary:
Previously, if some columns are allowed and the rest are denied,
the client could see unnecessary blank columns.  This diff cleans
up column metadata censorship.  It also adds a small tweak to
retain the `manualSort` column when filtering columns for a mixed
access table.

Test Plan: added tests

Reviewers: dsagal

Reviewed By: dsagal

Differential Revision: https://phab.getgrist.com/D2742
This commit is contained in:
Paul Fitzpatrick 2021-03-02 16:17:53 -05:00
parent 1995a96178
commit 937214d927

View File

@ -835,10 +835,11 @@ export class GranularAccess implements GranularAccessForBundle {
/** /**
* Remove columns from a ColumnValues parameter of certain DocActions, using a predicate for * Remove columns from a ColumnValues parameter of certain DocActions, using a predicate for
* which columns to keep. * which columns to keep.
* Will retain manualSort columns regardless of wildcards.
*/ */
private _filterColumns(data: BulkColValues|ColValues, shouldInclude: (colId: string) => boolean) { private _filterColumns(data: BulkColValues|ColValues, shouldInclude: (colId: string) => boolean) {
for (const colId of Object.keys(data)) { for (const colId of Object.keys(data)) {
if (!shouldInclude(colId)) { if (colId !== 'manualSort' && !shouldInclude(colId)) {
delete data[colId]; delete data[colId];
} }
} }
@ -1467,26 +1468,35 @@ export class CensorshipInfo {
// Collect a list of censored columns (by "<tableRef> <colId>"). // Collect a list of censored columns (by "<tableRef> <colId>").
const columnCode = (tableRef: number, colId: string) => `${tableRef} ${colId}`; const columnCode = (tableRef: number, colId: string) => `${tableRef} ${colId}`;
const censoredColumnCodes: Set<string> = new Set(); const censoredColumnCodes: Set<string> = new Set();
const tableRefToTableId: Map<number, string> = new Map();
const uncensoredTables: Set<number> = new Set();
// Scan for forbidden tables.
let rec = new RecordView(tables._grist_Tables, undefined); let rec = new RecordView(tables._grist_Tables, undefined);
let ids = getRowIdsFromDocAction(tables._grist_Tables); let ids = getRowIdsFromDocAction(tables._grist_Tables);
for (let idx = 0; idx < ids.length; idx++) { for (let idx = 0; idx < ids.length; idx++) {
rec.index = idx; rec.index = idx;
const tableId = rec.get('tableId') as string; const tableId = rec.get('tableId') as string;
const tableRef = ids[idx]; const tableRef = ids[idx];
tableRefToTableId.set(tableRef, tableId);
const tableAccess = permInfo.getTableAccess(tableId); const tableAccess = permInfo.getTableAccess(tableId);
if (tableAccess.perms.read === 'deny') { if (tableAccess.perms.read === 'deny') {
this.censoredTables.add(tableRef); this.censoredTables.add(tableRef);
} else if (tableAccess.perms.read === 'allow') {
uncensoredTables.add(tableRef);
} }
// TODO If some columns are allowed and the rest (*) are denied, we need to be able to }
// censor all columns outside a set. // Scan for forbidden columns.
for (const ruleSet of ruleCollection.getAllColumnRuleSets(tableId)) { ids = getRowIdsFromDocAction(tables._grist_Tables_column);
if (Array.isArray(ruleSet.colIds)) { rec = new RecordView(tables._grist_Tables_column, undefined);
for (const colId of ruleSet.colIds) { for (let idx = 0; idx < ids.length; idx++) {
if (permInfo.getColumnAccess(tableId, colId).perms.read === 'deny') { rec.index = idx;
censoredColumnCodes.add(columnCode(tableRef, colId)); const tableRef = rec.get('parentId') as number;
} if (uncensoredTables.has(tableRef)) { continue; }
} const tableId = tableRefToTableId.get(tableRef);
} if (!tableId) { throw new Error('table not found'); }
const colId = rec.get('colId') as string;
if (this.censoredTables.has(tableRef) || (colId !== 'manualSort' && permInfo.getColumnAccess(tableId, colId).perms.read === 'deny')) {
censoredColumnCodes.add(columnCode(tableRef, colId));
} }
} }
// Collect a list of all sections and views containing a table to which the user has no access. // Collect a list of all sections and views containing a table to which the user has no access.