(core) freshen "view as user" behavior

Summary:
Now as the user an owner might choose to view their document as
is likely to not have access to rules, it is better to start
viewing on the default document page rather than /p/acl.

The "Access Rules" link is grayed out when in "view as" mode for
now (improvements are planned).

Test Plan: updated test

Reviewers: dsagal

Reviewed By: dsagal

Differential Revision: https://phab.getgrist.com/D2743
This commit is contained in:
Paul Fitzpatrick 2021-03-02 16:11:37 -05:00
parent 937214d927
commit c37a04c578
5 changed files with 39 additions and 24 deletions

View File

@ -40,7 +40,7 @@ function buildUserRow(user: UserAccessData, currentUser: FullUser|null, ctl: IOp
testId('acl-user-view-as'), testId('acl-user-view-as'),
icon('FieldLink'), 'View As', { icon('FieldLink'), 'View As', {
href: urlState().makeUrl( href: urlState().makeUrl(
merge({}, urlState().state.get(), {params: {linkParameters: {aclAsUser: user.email}}})), merge({}, urlState().state.get(), {docPage: '', params: {linkParameters: {aclAsUser: user.email}}})),
}), }),
testId('acl-user-item'), testId('acl-user-item'),
); );

View File

@ -13,6 +13,8 @@ const testId = makeTestId('test-tools-');
export function tools(owner: Disposable, gristDoc: GristDoc, leftPanelOpen: Observable<boolean>): Element { export function tools(owner: Disposable, gristDoc: GristDoc, leftPanelOpen: Observable<boolean>): Element {
const aclUIEnabled = Boolean(urlState().state.get().params?.aclUI); const aclUIEnabled = Boolean(urlState().state.get().params?.aclUI);
const isOwner = gristDoc.docPageModel.currentDoc.get()?.access === 'owners'; const isOwner = gristDoc.docPageModel.currentDoc.get()?.access === 'owners';
const isOverridden = Boolean(gristDoc.docPageModel.userOverride.get());
const canUseAccessRules = isOwner && !isOverridden;
return cssTools( return cssTools(
cssTools.cls('-collapsed', (use) => !use(leftPanelOpen)), cssTools.cls('-collapsed', (use) => !use(leftPanelOpen)),
cssSectionHeader("TOOLS"), cssSectionHeader("TOOLS"),
@ -20,10 +22,10 @@ export function tools(owner: Disposable, gristDoc: GristDoc, leftPanelOpen: Obse
(aclUIEnabled ? (aclUIEnabled ?
cssPageEntry( cssPageEntry(
cssPageEntry.cls('-selected', (use) => use(gristDoc.activeViewId) === 'acl'), cssPageEntry.cls('-selected', (use) => use(gristDoc.activeViewId) === 'acl'),
cssPageEntry.cls('-disabled', !isOwner), cssPageEntry.cls('-disabled', !canUseAccessRules),
(isOwner ? cssPageLink : cssPageDisabledLink)(cssPageIcon('EyeShow'), (canUseAccessRules ? cssPageLink : cssPageDisabledLink)(cssPageIcon('EyeShow'),
cssLinkText('Access Rules'), cssLinkText('Access Rules'),
isOwner ? urlState().setLinkUrl({docPage: 'acl'}) : null canUseAccessRules ? urlState().setLinkUrl({docPage: 'acl'}) : null
), ),
testId('access-rules'), testId('access-rules'),
) : ) :

View File

@ -845,8 +845,8 @@ export class ActiveDoc extends EventEmitter {
return fetchURL(url, this.makeAccessId(docSession.authorizer.getUserId())); return fetchURL(url, this.makeAccessId(docSession.authorizer.getUserId()));
} }
public forwardPluginRpc(docSession: DocSession, pluginId: string, msg: IMessage): Promise<any> { public async forwardPluginRpc(docSession: DocSession, pluginId: string, msg: IMessage): Promise<any> {
if (this._granularAccess.hasNuancedAccess(docSession)) { if (await this._granularAccess.hasNuancedAccess(docSession)) {
throw new Error('cannot confirm access to plugin'); throw new Error('cannot confirm access to plugin');
} }
const pluginRpc = this.docPluginManager.plugins[pluginId].rpc; const pluginRpc = this.docPluginManager.plugins[pluginId].rpc;
@ -876,7 +876,7 @@ export class ActiveDoc extends EventEmitter {
return this.shutdown(); return this.shutdown();
} }
public isOwner(docSession: OptDocSession): boolean { public isOwner(docSession: OptDocSession): Promise<boolean> {
return this._granularAccess.isOwner(docSession); return this._granularAccess.isOwner(docSession);
} }
@ -1036,14 +1036,14 @@ export class ActiveDoc extends EventEmitter {
} }
public async removeSnapshots(docSession: OptDocSession, snapshotIds: string[]): Promise<void> { public async removeSnapshots(docSession: OptDocSession, snapshotIds: string[]): Promise<void> {
if (!this.isOwner(docSession)) { if (!await this.isOwner(docSession)) {
throw new Error('cannot remove snapshots, access denied'); throw new Error('cannot remove snapshots, access denied');
} }
return this._docManager.storageManager.removeSnapshots(this.docName, snapshotIds); return this._docManager.storageManager.removeSnapshots(this.docName, snapshotIds);
} }
public async deleteActions(docSession: OptDocSession, keepN: number): Promise<void> { public async deleteActions(docSession: OptDocSession, keepN: number): Promise<void> {
if (!this.isOwner(docSession)) { if (!await this.isOwner(docSession)) {
throw new Error('cannot delete actions, access denied'); throw new Error('cannot delete actions, access denied');
} }
await this._actionHistory.deleteActions(keepN); await this._actionHistory.deleteActions(keepN);

View File

@ -382,7 +382,7 @@ export class DocManager extends EventEmitter {
for (;;) { for (;;) {
if (this._activeDocs.has(docName) && wantRecoveryMode !== undefined) { if (this._activeDocs.has(docName) && wantRecoveryMode !== undefined) {
const activeDoc = await this._activeDocs.get(docName); const activeDoc = await this._activeDocs.get(docName);
if (activeDoc && activeDoc.recoveryMode !== wantRecoveryMode && activeDoc.isOwner(docSession)) { if (activeDoc && activeDoc.recoveryMode !== wantRecoveryMode && await activeDoc.isOwner(docSession)) {
// shutting doc down to have a chance to re-open in the correct mode. // shutting doc down to have a chance to re-open in the correct mode.
// TODO: there could be a battle with other users opening it in a different mode. // TODO: there could be a battle with other users opening it in a different mode.
await activeDoc.shutdown(); await activeDoc.shutdown();

View File

@ -206,7 +206,7 @@ export class GranularAccess implements GranularAccessForBundle {
public async canApplyBundle() { public async canApplyBundle() {
if (!this._activeBundle) { throw new Error('no active bundle'); } if (!this._activeBundle) { throw new Error('no active bundle'); }
const {docActions, docSession} = this._activeBundle; const {docActions, docSession} = this._activeBundle;
if (this._activeBundle.hasDeliberateRuleChange && !this.isOwner(docSession)) { if (this._activeBundle.hasDeliberateRuleChange && !await this.isOwner(docSession)) {
throw new ErrorWithCode('ACL_DENY', 'Only owners can modify access rules'); throw new ErrorWithCode('ACL_DENY', 'Only owners can modify access rules');
} }
if (this._ruler.haveRules()) { if (this._ruler.haveRules()) {
@ -360,13 +360,13 @@ export class GranularAccess implements GranularAccessForBundle {
const name = a[0] as string; const name = a[0] as string;
if (OK_ACTIONS.has(name)) { return true; } if (OK_ACTIONS.has(name)) { return true; }
if (SPECIAL_ACTIONS.has(name)) { if (SPECIAL_ACTIONS.has(name)) {
if (this.hasNuancedAccess(docSession)) { if (await this.hasNuancedAccess(docSession)) {
throw new ErrorWithCode('ACL_DENY', `Blocked by access rules: '${name}' actions need uncomplicated access`); throw new ErrorWithCode('ACL_DENY', `Blocked by access rules: '${name}' actions need uncomplicated access`);
} }
return true; return true;
} }
if (SURPRISING_ACTIONS.has(name)) { if (SURPRISING_ACTIONS.has(name)) {
if (!this.hasFullAccess(docSession)) { if (!await this.hasFullAccess(docSession)) {
throw new ErrorWithCode('ACL_DENY', `Blocked by access rules: '${name}' actions need full access`); throw new ErrorWithCode('ACL_DENY', `Blocked by access rules: '${name}' actions need full access`);
} }
return true; return true;
@ -394,9 +394,9 @@ export class GranularAccess implements GranularAccessForBundle {
* worked through. Currently if there are no owner-only tables, then everyone's * worked through. Currently if there are no owner-only tables, then everyone's
* access is simple and without nuance. * access is simple and without nuance.
*/ */
public hasNuancedAccess(docSession: OptDocSession): boolean { public async hasNuancedAccess(docSession: OptDocSession): Promise<boolean> {
if (!this._ruler.haveRules()) { return false; } if (!this._ruler.haveRules()) { return false; }
return !this.hasFullAccess(docSession); return !await this.hasFullAccess(docSession);
} }
/** /**
@ -404,7 +404,7 @@ export class GranularAccess implements GranularAccessForBundle {
* permissions. * permissions.
*/ */
public async canReadEverything(docSession: OptDocSession): Promise<boolean> { public async canReadEverything(docSession: OptDocSession): Promise<boolean> {
const access = getDocSessionAccess(docSession); const access = await this._getNominalAccess(docSession);
if (!canView(access)) { return false; } if (!canView(access)) { return false; }
const permInfo = await this._getAccess(docSession); const permInfo = await this._getAccess(docSession);
return this.getReadPermission(permInfo.getFullAccess()) === 'allow'; return this.getReadPermission(permInfo.getFullAccess()) === 'allow';
@ -421,7 +421,7 @@ export class GranularAccess implements GranularAccessForBundle {
* just a bit inconsistent. * just a bit inconsistent.
*/ */
public async canCopyEverything(docSession: OptDocSession): Promise<boolean> { public async canCopyEverything(docSession: OptDocSession): Promise<boolean> {
return this.isOwner(docSession) || this.canReadEverything(docSession); return (await this.isOwner(docSession)) || (await this.canReadEverything(docSession));
} }
/** /**
@ -430,15 +430,15 @@ export class GranularAccess implements GranularAccessForBundle {
* TODO: uses of this method should be checked to see if they can be fleshed out * TODO: uses of this method should be checked to see if they can be fleshed out
* now we have more of the ACL implementation done. * now we have more of the ACL implementation done.
*/ */
public hasFullAccess(docSession: OptDocSession): boolean { public hasFullAccess(docSession: OptDocSession): Promise<boolean> {
return this.isOwner(docSession); return this.isOwner(docSession);
} }
/** /**
* Check whether user has owner-level access to the document. * Check whether user has owner-level access to the document.
*/ */
public isOwner(docSession: OptDocSession): boolean { public async isOwner(docSession: OptDocSession): Promise<boolean> {
const access = getDocSessionAccess(docSession); const access = await this._getNominalAccess(docSession);
return access === 'owners'; return access === 'owners';
} }
@ -468,7 +468,7 @@ export class GranularAccess implements GranularAccessForBundle {
const permInfo = await this._getAccess(docSession); const permInfo = await this._getAccess(docSession);
const censor = new CensorshipInfo(permInfo, this._ruler.ruleCollection, tables, const censor = new CensorshipInfo(permInfo, this._ruler.ruleCollection, tables,
this.isOwner(docSession)); await this.isOwner(docSession));
for (const tableId of STRUCTURAL_TABLES) { for (const tableId of STRUCTURAL_TABLES) {
censor.apply(tables[tableId]); censor.apply(tables[tableId]);
@ -528,6 +528,19 @@ export class GranularAccess implements GranularAccessForBundle {
(docSession) => this._filterDocUpdate(docSession, message)); (docSession) => this._filterDocUpdate(docSession, message));
} }
/**
* Get the role the session user has for this document. User may be overridden,
* in which case the role of the override is returned.
*/
private async _getNominalAccess(docSession: OptDocSession): Promise<Role> {
const linkParameters = docSession.authorizer?.getLinkParameters() || {};
if (linkParameters.aclAsUserId || linkParameters.aclAsUser) {
const info = await this._getUser(docSession);
return info.Access as Role;
}
return getDocSessionAccess(docSession);
}
/** /**
* This filters a message being broadcast to all clients to be appropriate for one * This filters a message being broadcast to all clients to be appropriate for one
* particular client, if that client may need some material filtered out. * particular client, if that client may need some material filtered out.
@ -896,7 +909,7 @@ export class GranularAccess implements GranularAccessForBundle {
// If aclAsUserId/aclAsUser is set, then override user for acl purposes. // If aclAsUserId/aclAsUser is set, then override user for acl purposes.
if (linkParameters.aclAsUserId || linkParameters.aclAsUser) { if (linkParameters.aclAsUserId || linkParameters.aclAsUser) {
if (!this.isOwner(docSession)) { throw new Error('only an owner can override user'); } if (access !== 'owners') { throw new Error('only an owner can override user'); }
if (attrs.override) { if (attrs.override) {
// Used cached properties. // Used cached properties.
access = attrs.override.access; access = attrs.override.access;
@ -1167,7 +1180,7 @@ export class GranularAccess implements GranularAccessForBundle {
const censor = new CensorshipInfo(permissionInfo, const censor = new CensorshipInfo(permissionInfo,
ruler.ruleCollection, ruler.ruleCollection,
step.metaAfter, step.metaAfter,
this.isOwner(cursor.docSession)); await this.isOwner(cursor.docSession));
if (censor.apply(act)) { if (censor.apply(act)) {
results.push(act); results.push(act);
} }
@ -1179,7 +1192,7 @@ export class GranularAccess implements GranularAccessForBundle {
const censorBefore = new CensorshipInfo(permissionInfo, const censorBefore = new CensorshipInfo(permissionInfo,
ruler.ruleCollection, ruler.ruleCollection,
step.metaBefore, step.metaBefore,
this.isOwner(cursor.docSession)); await this.isOwner(cursor.docSession));
// For all views previously censored, if they are now uncensored, // For all views previously censored, if they are now uncensored,
// add an UpdateRecord to expose them. // add an UpdateRecord to expose them.
for (const v of censorBefore.censoredViews) { for (const v of censorBefore.censoredViews) {