(core) make AccessRules and FullCopies effective

Summary:
This allows `*SPECIAL:AccessRules` to give read access to the access rules to more users, and `*SPECIAL:FullCopies` to grant download/copy rights to more users.

This diff also changes forks to be owned by the user who forked them (previously they were an editor), since that feels more natural.

Test Plan: Added and updated tests.

Reviewers: dsagal

Reviewed By: dsagal

Differential Revision: https://phab.getgrist.com/D2760
This commit is contained in:
Paul Fitzpatrick 2021-03-25 13:37:09 -04:00
parent e14488bcc8
commit 9d1bc5a518
10 changed files with 86 additions and 36 deletions

View File

@ -57,7 +57,11 @@ export class ACLUsersPopup extends Disposable {
public async init(pageModel: DocPageModel) {
this._currentUser = pageModel.userOverride.get()?.user || pageModel.appModel.currentValidUser;
const doc = pageModel.currentDoc.get();
if (doc) {
// Disabling "View as user" for forks for the moment. The getDocAccess endpoint
// only succeeds for documents that exist in the DB currently.
// TODO: modify the getDocAccess endpoint to accept forks, through the kind of
// manipulation that getDoc does. Then we can enable this button for forks.
if (doc && !doc.isFork) {
const permissionData = await pageModel.appModel.api.getDocAccess(doc.id);
if (this.isDisposed()) { return; }
this._usersInDoc = permissionData.users.map(user => ({

View File

@ -318,9 +318,10 @@ export class AccessRules extends Disposable {
),
),
bigBasicButton('Add User Attributes', dom.on('click', () => this._addUserAttributes())),
bigBasicButton('Users', cssDropdownIcon('Dropdown'), elem => this._aclUsersPopup.attachPopup(elem),
dom.style('visibility', use => use(this._aclUsersPopup.isInitialized) ? '' : 'hidden'),
),
!this._gristDoc.docPageModel.isFork.get() ?
bigBasicButton('Users', cssDropdownIcon('Dropdown'), elem => this._aclUsersPopup.attachPopup(elem),
dom.style('visibility', use => use(this._aclUsersPopup.isInitialized) ? '' : 'hidden'),
) : null,
),
cssConditionError(dom.text(this._errorMessage), {style: 'margin-left: 16px'},
testId('access-rules-error')

View File

@ -24,6 +24,7 @@ import * as rowset from 'app/client/models/rowset';
import {RowId} from 'app/client/models/rowset';
import {schema, SchemaTypes} from 'app/common/schema';
import {ACLRuleRec, createACLRuleRec} from 'app/client/models/entities/ACLRuleRec';
import {ColumnRec, createColumnRec} from 'app/client/models/entities/ColumnRec';
import {createDocInfoRec, DocInfoRec} from 'app/client/models/entities/DocInfoRec';
import {createPageRec, PageRec} from 'app/client/models/entities/PageRec';
@ -111,6 +112,7 @@ export class DocModel {
public validations: MTM<ValidationRec> = this._metaTableModel("_grist_Validations", createValidationRec);
public replHist: MTM<REPLRec> = this._metaTableModel("_grist_REPL_Hist", createREPLRec);
public pages: MTM<PageRec> = this._metaTableModel("_grist_Pages", createPageRec);
public rules: MTM<ACLRuleRec> = this._metaTableModel("_grist_ACLRules", createACLRuleRec);
public allTables: KoArray<TableRec>;
public allTableIds: KoArray<string>;

View File

@ -0,0 +1,7 @@
import {DocModel, IRowModel} from 'app/client/models/DocModel';
export type ACLRuleRec = IRowModel<"_grist_ACLRules">;
export function createACLRuleRec(this: ACLRuleRec, docModel: DocModel): void {
// currently don't care much about content.
}

View File

@ -123,7 +123,7 @@ function shareButton(buttonText: string|null, menuCreateFunc: MenuCreateFunc,
function menuManageUsers(doc: DocInfo, pageModel: DocPageModel) {
return [
menuItem(() => manageUsers(doc, pageModel), 'Manage Users',
dom.cls('disabled', !roles.canEditAccess(doc.access)),
dom.cls('disabled', !roles.canEditAccess(doc.access) || doc.isFork),
testId('tb-share-option')
),
menuDivider(),

View File

@ -9,7 +9,7 @@ import { colors } from 'app/client/ui2018/cssVars';
import { icon } from 'app/client/ui2018/icons';
import { cssLink } from 'app/client/ui2018/links';
import { userOverrideParams } from 'app/common/gristUrls';
import { Disposable, dom, makeTestId, Observable, styled } from "grainjs";
import { Disposable, dom, makeTestId, Observable, observable, styled } from "grainjs";
const testId = makeTestId('test-tools-');
@ -17,7 +17,13 @@ export function tools(owner: Disposable, gristDoc: GristDoc, leftPanelOpen: Obse
const aclUIEnabled = Boolean(urlState().state.get().params?.aclUI);
const isOwner = gristDoc.docPageModel.currentDoc.get()?.access === 'owners';
const isOverridden = Boolean(gristDoc.docPageModel.userOverride.get());
const canUseAccessRules = isOwner && !isOverridden;
const canViewAccessRules = observable(false);
function updateCanViewAccessRules() {
canViewAccessRules.set((isOwner && !isOverridden) ||
gristDoc.docModel.rules.getNumRows() > 0);
}
owner.autoDispose(gristDoc.docModel.rules.tableData.tableActionEmitter.addListener(updateCanViewAccessRules));
updateCanViewAccessRules();
return cssTools(
cssTools.cls('-collapsed', (use) => !use(leftPanelOpen)),
cssSectionHeader("TOOLS"),
@ -25,12 +31,15 @@ export function tools(owner: Disposable, gristDoc: GristDoc, leftPanelOpen: Obse
(aclUIEnabled ?
cssPageEntry(
cssPageEntry.cls('-selected', (use) => use(gristDoc.activeViewId) === 'acl'),
cssPageEntry.cls('-disabled', !isOwner),
cssPageLink(cssPageIcon('EyeShow'),
cssLinkText('Access Rules'),
canUseAccessRules ? urlState().setLinkUrl({docPage: 'acl'}) : null,
isOverridden ? addRevertViewAsUI() : null,
),
cssPageEntry.cls('-disabled', (use) => !use(canViewAccessRules)),
dom.domComputed(canViewAccessRules, (_canViewAccessRules) => {
return cssPageLink(
cssPageIcon('EyeShow'),
cssLinkText('Access Rules'),
_canViewAccessRules ? urlState().setLinkUrl({docPage: 'acl'}) : null,
isOverridden ? addRevertViewAsUI() : null,
);
}),
testId('access-rules'),
) :
null

View File

@ -44,8 +44,8 @@ const SPECIAL_RULE_SETS: Record<string, RuleSet> = {
}, {
aclFormula: "",
matchFunc: defaultMatchFunc,
permissions: parsePermissions('none'),
permissionsText: 'none',
permissions: parsePermissions('-R'),
permissionsText: '-R',
}],
},
FullCopies: {
@ -59,8 +59,8 @@ const SPECIAL_RULE_SETS: Record<string, RuleSet> = {
}, {
aclFormula: "",
matchFunc: defaultMatchFunc,
permissions: parsePermissions('none'),
permissionsText: 'none',
permissions: parsePermissions('-R'),
permissionsText: '-R',
}],
}
};
@ -100,6 +100,9 @@ export class ACLRuleCollection {
// Maps 'tableId:colId' to one of the RuleSets in the list _columnRuleSets.get(tableId).
private _tableColumnMap = new Map<string, RuleSet>();
// Rules for SPECIAL_RULES_TABLE_ID "columns".
private _specialRuleSets = new Map<string, RuleSet>();
// Map of tableId to the single default RuleSet for the table (colIds of '*')
private _tableRuleSets = new Map<string, RuleSet>();
@ -119,6 +122,7 @@ export class ACLRuleCollection {
// Return the RuleSet for "tableId:colId", or undefined if there isn't one for this column.
public getColumnRuleSet(tableId: string, colId: string): RuleSet|undefined {
if (tableId === SPECIAL_RULES_TABLE_ID) { return this._specialRuleSets.get(colId); }
return this._tableColumnMap.get(`${tableId}:${colId}`);
}
@ -220,6 +224,7 @@ export class ACLRuleCollection {
this._defaultRuleSet = defaultRuleSet;
this._tableIds = [...tableIds];
this._userAttributeRules = userAttributeMap;
this._specialRuleSets = specialRuleSets;
}
/**

View File

@ -983,19 +983,16 @@ export class HomeDBManager extends EventEmitter {
doc.trunkAccess = doc.access;
// Forks without a user id are editable by anyone with view access to the trunk.
if (forkUserId === undefined && doc.access === 'viewers') { doc.access = 'editors'; }
if (forkUserId === undefined && roles.canView(doc.access)) { doc.access = 'owners'; }
if (forkUserId !== undefined) {
// A fork user id is known, so only that user should get to edit the fork.
if (userId === forkUserId) {
// Promote to editor if just a viewer of the trunk.
if (doc.access === 'viewers') { doc.access = 'editors'; }
if (roles.canView(doc.access)) { doc.access = 'owners'; }
} else {
// reduce to viewer if not already viewer
doc.access = roles.getWeakestRole('viewers', doc.access);
}
}
// No-one may be an owner of a fork, since there's no way to set up ACLs for it.
if (doc.access === 'owners') { doc.access = 'editors'; }
// Finally, if we are viewing a snapshot, we can't edit it.
if (snapshotId) {

View File

@ -697,9 +697,8 @@ export class ActiveDoc extends EventEmitter {
*/
public async findColFromValues(docSession: DocSession, values: any[], n: number,
optTableId?: string): Promise<number[]> {
// This could leak information about private tables, so if user cannot read entire
// document, do nothing.
if (!await this._granularAccess.canReadEverything(docSession)) { return []; }
// This could leak information about private tables, so check for permission.
if (!await this._granularAccess.canScanData(docSession)) { return []; }
this.logInfo(docSession, "findColFromValues(%s, %s, %s)", docSession, values, n);
await this.waitForInitialization();
return this._pyCall('find_col_from_values', values, n, optTableId);
@ -843,7 +842,7 @@ export class ActiveDoc extends EventEmitter {
public async autocomplete(docSession: DocSession, txt: string, tableId: string): Promise<string[]> {
// Autocompletion can leak names of tables and columns.
if (!await this._granularAccess.canReadEverything(docSession)) { return []; }
if (!await this._granularAccess.canScanData(docSession)) { return []; }
await this.waitForInitialization();
return this._pyCall('autocomplete', txt, tableId);
}
@ -895,7 +894,7 @@ export class ActiveDoc extends EventEmitter {
const user = getDocSessionUser(docSession);
// For now, fork only if user can read everything (or is owner).
// TODO: allow forks with partial content.
if (!user || !await this._granularAccess.canCopyEverything(docSession)) {
if (!user || !await this.canDownload(docSession)) {
throw new ApiError('Insufficient access to document to copy it entirely', 403);
}
const userId = user.id;
@ -960,7 +959,7 @@ export class ActiveDoc extends EventEmitter {
* regardless of rules that may block access to them.
*/
public async getAclResources(docSession: DocSession): Promise<{[tableId: string]: string[]}> {
if (await this._granularAccess.hasNuancedAccess(docSession) || !this.docData) {
if (!this.docData || !await this._granularAccess.hasAccessRulesPermission(docSession)) {
throw new Error('Cannot list ACL resources');
}
const result: {[tableId: string]: string[]} = {};

View File

@ -1,5 +1,5 @@
import { ALL_PERMISSION_PROPS } from 'app/common/ACLPermissions';
import { ACLRuleCollection } from 'app/common/ACLRuleCollection';
import { ACLRuleCollection, SPECIAL_RULES_TABLE_ID } from 'app/common/ACLRuleCollection';
import { ActionGroup } from 'app/common/ActionGroup';
import { createEmptyActionSummary } from 'app/common/ActionSummary';
import { Query } from 'app/common/ActiveDocAPI';
@ -400,6 +400,23 @@ export class GranularAccess implements GranularAccessForBundle {
return !await this.hasFullAccess(docSession);
}
/**
* Check if user is explicitly permitted to download/copy document.
* They may be allowed to download in any case, see canCopyEverything.
*/
public async hasFullCopiesPermission(docSession: OptDocSession): Promise<boolean> {
const permInfo = await this._getAccess(docSession);
return permInfo.getColumnAccess(SPECIAL_RULES_TABLE_ID, 'FullCopies').perms.read === 'allow';
}
/**
* Check if user may view Access Rules.
*/
public async hasAccessRulesPermission(docSession: OptDocSession): Promise<boolean> {
const permInfo = await this._getAccess(docSession);
return permInfo.getColumnAccess(SPECIAL_RULES_TABLE_ID, 'AccessRules').perms.read === 'allow';
}
/**
* Check whether user can read everything in document. Checks both home-level and doc-level
* permissions.
@ -411,6 +428,14 @@ export class GranularAccess implements GranularAccessForBundle {
return this.getReadPermission(permInfo.getFullAccess()) === 'allow';
}
/**
* An odd little right for findColFromValues and autocomplete. Allow if user can read
* all data, or is an owner. Might be worth making a special permission.
*/
public async canScanData(docSession: OptDocSession): Promise<boolean> {
return await this.isOwner(docSession) || await this.canReadEverything(docSession);
}
/**
* Check whether user can copy everything in document. Owners can always copy
* everything, even if there are rules that specify they cannot.
@ -422,7 +447,8 @@ export class GranularAccess implements GranularAccessForBundle {
* just a bit inconsistent.
*/
public async canCopyEverything(docSession: OptDocSession): Promise<boolean> {
return (await this.isOwner(docSession)) || (await this.canReadEverything(docSession));
return await this.hasFullCopiesPermission(docSession) ||
await this.canReadEverything(docSession);
}
/**
@ -469,7 +495,7 @@ export class GranularAccess implements GranularAccessForBundle {
const permInfo = await this._getAccess(docSession);
const censor = new CensorshipInfo(permInfo, this._ruler.ruleCollection, tables,
await this.isOwner(docSession));
await this.hasAccessRulesPermission(docSession));
for (const tableId of STRUCTURAL_TABLES) {
censor.apply(tables[tableId]);
@ -1181,7 +1207,7 @@ export class GranularAccess implements GranularAccessForBundle {
const censor = new CensorshipInfo(permissionInfo,
ruler.ruleCollection,
step.metaAfter,
await this.isOwner(cursor.docSession));
await this.hasAccessRulesPermission(cursor.docSession));
if (censor.apply(act)) {
results.push(act);
}
@ -1193,7 +1219,7 @@ export class GranularAccess implements GranularAccessForBundle {
const censorBefore = new CensorshipInfo(permissionInfo,
ruler.ruleCollection,
step.metaBefore,
await this.isOwner(cursor.docSession));
await this.hasAccessRulesPermission(cursor.docSession));
// For all views previously censored, if they are now uncensored,
// add an UpdateRecord to expose them.
for (const v of censorBefore.censoredViews) {
@ -1478,7 +1504,7 @@ export class CensorshipInfo {
public constructor(permInfo: PermissionInfo,
ruleCollection: ACLRuleCollection,
tables: {[key: string]: TableDataAction},
private _isOwner: boolean) {
private _canViewACLs: boolean) {
// Collect a list of censored columns (by "<tableRef> <colId>").
const columnCode = (tableRef: number, colId: string) => `${tableRef} ${colId}`;
const censoredColumnCodes: Set<string> = new Set();
@ -1550,11 +1576,11 @@ export class CensorshipInfo {
const ids = getRowIdsFromDocAction(a);
if (!STRUCTURAL_TABLES.has(tableId)) { return true; }
if (!(tableId in this.censored)) {
if (!this._isOwner && a[0] === 'TableData') {
if (!this._canViewACLs && a[0] === 'TableData') {
a[2] = [];
a[3] = {};
}
return this._isOwner;
return this._canViewACLs;
}
const rec = new RecordEditor(a, undefined, true);
const method = getCensorMethod(getTableId(a));