diff --git a/app/client/aclui/ACLUsers.ts b/app/client/aclui/ACLUsers.ts index c6803a76..a342cec1 100644 --- a/app/client/aclui/ACLUsers.ts +++ b/app/client/aclui/ACLUsers.ts @@ -10,7 +10,8 @@ import {menuCssClass} from 'app/client/ui2018/menus'; import {userOverrideParams} from 'app/common/gristUrls'; import {FullUser} from 'app/common/LoginSessionAPI'; import * as roles from 'app/common/roles'; -import {ANONYMOUS_USER_EMAIL, EVERYONE_EMAIL, getRealAccess, UserAccessData} from 'app/common/UserAPI'; +import {ANONYMOUS_USER_EMAIL, EVERYONE_EMAIL} from 'app/common/UserAPI'; +import {getRealAccess, PermissionData, UserAccessData} from 'app/common/UserAPI'; import {Disposable, dom, Observable, styled} from 'grainjs'; import {cssMenu, cssMenuWrap, defaultMenuOptions, IOpenController, setPopupToCreateDom} from 'popweasel'; @@ -54,16 +55,9 @@ export class ACLUsersPopup extends Disposable { private _usersInDoc: UserAccessData[] = []; private _currentUser: FullUser|null = null; - public async init(pageModel: DocPageModel) { + public init(pageModel: DocPageModel, permissionData: PermissionData|null) { this._currentUser = pageModel.userOverride.get()?.user || pageModel.appModel.currentValidUser; - const doc = pageModel.currentDoc.get(); - // 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; } + if (permissionData) { this._usersInDoc = permissionData.users.map(user => ({ ...user, access: getRealAccess(user, permissionData), diff --git a/app/client/aclui/AccessRules.ts b/app/client/aclui/AccessRules.ts index 34b5452f..6d9c6d96 100644 --- a/app/client/aclui/AccessRules.ts +++ b/app/client/aclui/AccessRules.ts @@ -26,7 +26,9 @@ import {FormulaProperties, RulePart, RuleSet, UserAttributeRule} from 'app/commo import {getFormulaProperties} from 'app/common/GranularAccessClause'; import {isHiddenCol} from 'app/common/gristTypes'; import {isObject} from 'app/common/gutil'; +import * as roles from 'app/common/roles'; import {SchemaTypes} from 'app/common/schema'; +import {ANONYMOUS_USER_EMAIL, EVERYONE_EMAIL, getRealAccess} from 'app/common/UserAPI'; import {BaseObservable, Computed, Disposable, MutableObsArray, obsArray, Observable} from 'grainjs'; import {dom, DomElementArg, IDisposableOwner, styled} from 'grainjs'; import isEqual = require('lodash/isEqual'); @@ -91,6 +93,8 @@ export class AccessRules extends Disposable { private _aclUsersPopup = ACLUsersPopup.create(this); + private _publicEditAccess = Observable.create(this, false); + constructor(private _gristDoc: GristDoc) { super(); this._ruleStatus = Computed.create(this, (use) => { @@ -110,7 +114,8 @@ export class AccessRules extends Disposable { ); }); - this._savingEnabled = Computed.create(this, this._ruleStatus, (use, s) => (s === RuleStatus.ChangedValid)); + this._savingEnabled = Computed.create(this, this._ruleStatus, (use, s) => + (s === RuleStatus.ChangedValid) && !use(this._publicEditAccess)); this._userAttrChoices = Computed.create(this, this._userAttrRules, (use, rules) => { const result: IAttrOption[] = [ @@ -137,22 +142,12 @@ export class AccessRules extends Disposable { for (const tableId of ['_grist_ACLResources', '_grist_ACLRules']) { const tableData = this._gristDoc.docData.getTable(tableId)!; this.autoDispose(tableData.tableActionEmitter.addListener(this._onChange, this)); + this.autoDispose(this._gristDoc.docPageModel.currentDoc.addListener(this._updateDocAccessData, this)); } this.update().catch((e) => this._errorMessage.set(e.message)); } - public _onChange() { - if (this._ruleStatus.get() === RuleStatus.Unchanged) { - // If no changes, it's safe to just reload the rules from docData. - this.update().catch((e) => this._errorMessage.set(e.message)); - } else { - this._errorMessage.set( - 'Access rules have changed. Click Reset to revert your changes and refresh the rules.' - ); - } - } - public get allTableIds() { return Object.keys(this._aclResources).sort(); } public get userAttrRules() { return this._userAttrRules; } public get userAttrChoices() { return this._userAttrChoices; } @@ -164,11 +159,13 @@ export class AccessRules extends Disposable { if (this.isDisposed()) { return; } this._errorMessage.set(''); const rules = this._ruleCollection; + [ , , this._aclResources] = await Promise.all([ rules.update(this._gristDoc.docData, {log: console}), - this._aclUsersPopup.init(this._gristDoc.docPageModel), + this._updateDocAccessData(), this._gristDoc.docComm.getAclResources(), ]); + if (this.isDisposed()) { return; } this._tableRules.set( rules.getAllTableIds() @@ -292,7 +289,7 @@ export class AccessRules extends Disposable { dom.text((use) => { const s = use(this._ruleStatus); return s === RuleStatus.CheckPending ? 'Checking...' : - s === RuleStatus.Invalid ? 'Invalid' : 'Saved'; + s === RuleStatus.Unchanged ? 'Saved' : 'Invalid'; }), testId('rules-non-save') ), @@ -318,12 +315,19 @@ export class AccessRules extends Disposable { ), ), bigBasicButton('Add User Attributes', dom.on('click', () => this._addUserAttributes())), + // Disabling "View as user" for forks for the moment. TODO Modify getDocAccess endpoint + // to accept forks, through the kind of manipulation that getDoc does; then can enable. !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'}, + cssConditionError({style: 'margin-left: 16px'}, + dom.maybe(this._publicEditAccess, () => dom('div', + 'Public "Editor" access is incompatible with Access Rules. ' + + 'To set rules, remove it or reduce to "Viewer".' + )), + dom.text(this._errorMessage), testId('access-rules-error') ), shadowScroll( @@ -427,6 +431,38 @@ export class AccessRules extends Disposable { private _addUserAttributes() { this._userAttrRules.push(ObsUserAttributeRule.create(this._userAttrRules, this, undefined, {focus: true})); } + + private _onChange() { + if (this._ruleStatus.get() === RuleStatus.Unchanged) { + // If no changes, it's safe to just reload the rules from docData. + this.update().catch((e) => this._errorMessage.set(e.message)); + } else { + this._errorMessage.set( + 'Access rules have changed. Click Reset to revert your changes and refresh the rules.' + ); + } + } + + private async _updateDocAccessData() { + const pageModel = this._gristDoc.docPageModel; + const doc = pageModel.currentDoc.get(); + + // Note that the getDocAccess endpoint does not succeed for forks currently. + const permissionData = doc && !doc.isFork ? await pageModel.appModel.api.getDocAccess(doc.id) : null; + if (this.isDisposed()) { return; } + + this._aclUsersPopup.init(pageModel, permissionData); + + // We do not allow Public Editor access in combination with Granular ACL rules. When + // _publicEditAccess is on, we show a warning and prevent saving rules. + if (permissionData) { + const publicEditAccess = permissionData.users.some(user => ( + (user.email === EVERYONE_EMAIL || user.email === ANONYMOUS_USER_EMAIL) && + roles.canEdit(getRealAccess(user, permissionData)) + )); + this._publicEditAccess.set(publicEditAccess); + } + } } // Represents all rules for a table. diff --git a/app/client/components/GristDoc.ts b/app/client/components/GristDoc.ts index b8d6d085..d733466e 100644 --- a/app/client/components/GristDoc.ts +++ b/app/client/components/GristDoc.ts @@ -522,6 +522,14 @@ export class GristDoc extends DisposableWithEvents { }); } + public hasGranularAccessRules(): boolean { + const rulesTable = this.docData.getTable('_grist_ACLRules')!; + // To check if there are rules, ignore the default no-op rule created for an older incarnation + // of ACLs. It exists in older documents, and is still created for new ones. We detect it by + // the use of the deprecated 'permissions' field, and not the new 'permissionsText' field. + return rulesTable.numRecords() > rulesTable.filterRowIds({permissionsText: '', permissions: 63}).length; + } + private _getToolContent(tool: typeof RightPanelTool.type): IExtraTool|null { switch (tool) { case 'docHistory': { diff --git a/app/client/models/NotifyModel.ts b/app/client/models/NotifyModel.ts index 8451a83a..6033ad53 100644 --- a/app/client/models/NotifyModel.ts +++ b/app/client/models/NotifyModel.ts @@ -96,7 +96,7 @@ export class Notification extends Expirable implements INotification { constructor(_opts: INotifyOptions) { super(); - this.options = defaults({}, _opts, this.options) + this.options = defaults({}, _opts, this.options); if (this.options.expireSec > 0) { const expireTimer = setTimeout(() => this.expire(), 1000 * this.options.expireSec); diff --git a/app/client/models/UserManagerModel.ts b/app/client/models/UserManagerModel.ts index 204e3618..d7b9de92 100644 --- a/app/client/models/UserManagerModel.ts +++ b/app/client/models/UserManagerModel.ts @@ -1,3 +1,5 @@ +import {DocPageModel} from 'app/client/models/DocPageModel'; +import {reportError, UserError} from 'app/client/models/errors'; import {normalizeEmail} from 'app/common/emails'; import {GristLoadConfig} from 'app/common/gristUrls'; import * as roles from 'app/common/roles'; @@ -120,7 +122,8 @@ export class UserManagerModelImpl extends Disposable implements UserManagerModel constructor( public initData: PermissionData, public resourceType: ResourceType, - private _activeUserEmail: string|null + private _activeUserEmail: string|null, + private _docPageModel?: DocPageModel, ) { super(); } @@ -194,13 +197,20 @@ export class UserManagerModelImpl extends Disposable implements UserManagerModel delta.maxInheritedRole = maxInheritedRole; } } - // Looping through the members has the side effect of updating the delta. const members = [...this.membersEdited.get()]; if (this.publicMember) { members.push(this.publicMember); } - members.forEach((m, i) => { - const access = m.access.get(); + // Loop through the members and update the delta. + for (const m of members) { + let access = m.access.get(); + if (m === this.publicMember && access === roles.EDITOR && + this._docPageModel?.gristDoc.get()?.hasGranularAccessRules()) { + access = roles.VIEWER; + reportError(new UserError( + 'Public "Editor" access is incompatible with Access Rules. Reduced to "Viewer".' + )); + } if (!roles.isValidRole(access)) { throw new Error(`Cannot update user to invalid role ${access}`); } @@ -208,7 +218,7 @@ export class UserManagerModelImpl extends Disposable implements UserManagerModel // Add users whose access changed. delta.users![m.email] = m.isRemoved ? null : access as roles.NonGuestRole; } - }); + } return delta; } diff --git a/app/client/ui/UserManager.ts b/app/client/ui/UserManager.ts index 6f731ae1..87bf2e4c 100644 --- a/app/client/ui/UserManager.ts +++ b/app/client/ui/UserManager.ts @@ -45,7 +45,8 @@ export interface IUserManagerOptions { // required properties of the options. async function getModel(options: IUserManagerOptions): Promise { const permissionData = await options.permissionData; - return new UserManagerModelImpl(permissionData, options.resourceType, options.activeEmail); + return new UserManagerModelImpl(permissionData, options.resourceType, options.activeEmail, + options.docPageModel); } /**