(core) Fix owner view access to snapshots

Summary:
Owners weren't able to access snapshots if access rules
that denied access to non-owners existed. The backend
was lowering snapshot document access to "viewers" as
part of implementing read-only behavior; this is now done
in the client, with document access for snapshots now
accurately reflecting the user's trunk access.

Additionally, sandboxes are no longer created for snapshots,
and background intervals aren't started for snapshots.

Test Plan: Browser test.

Reviewers: jarek, paulfitz

Reviewed By: jarek, paulfitz

Differential Revision: https://phab.getgrist.com/D3849
This commit is contained in:
George Gevoian 2023-04-11 01:56:26 -04:00
parent 40ea6bb2bc
commit 36f3fd0120
6 changed files with 80 additions and 53 deletions

View File

@ -39,6 +39,7 @@ export interface DocInfo extends Document {
userOverride: UserOverride|null;
isBareFork: boolean; // a document created without logging in, which is treated as a
// fork without an original.
isSnapshot: boolean;
isTutorialTrunk: boolean;
isTutorialFork: boolean;
idParts: UrlIdParts;
@ -72,6 +73,7 @@ export interface DocPageModel {
isRecoveryMode: Observable<boolean>;
userOverride: Observable<UserOverride|null>;
isBareFork: Observable<boolean>;
isSnapshot: Observable<boolean>;
isTutorialTrunk: Observable<boolean>;
isTutorialFork: Observable<boolean>;
@ -124,6 +126,7 @@ export class DocPageModelImpl extends Disposable implements DocPageModel {
(use, doc) => doc ? doc.isRecoveryMode : false);
public readonly userOverride = Computed.create(this, this.currentDoc, (use, doc) => doc ? doc.userOverride : null);
public readonly isBareFork = Computed.create(this, this.currentDoc, (use, doc) => doc ? doc.isBareFork : false);
public readonly isSnapshot = Computed.create(this, this.currentDoc, (use, doc) => doc ? doc.isSnapshot : false);
public readonly isTutorialTrunk = Computed.create(this, this.currentDoc,
(use, doc) => doc ? doc.isTutorialTrunk : false);
public readonly isTutorialFork = Computed.create(this, this.currentDoc,
@ -441,9 +444,10 @@ function buildDocInfo(doc: Document, mode: OpenDocMode | undefined): DocInfo {
const isPreFork = (openMode === 'fork');
const isBareFork = isFork && idParts.trunkId === NEW_DOCUMENT_CODE;
const isSnapshot = Boolean(idParts.snapshotId);
const isTutorialTrunk = !isFork && doc.type === 'tutorial' && mode !== 'default';
const isTutorialFork = isFork && doc.type === 'tutorial';
const isEditable = canEdit(doc.access) || isPreFork;
const isEditable = !isSnapshot && (canEdit(doc.access) || isPreFork);
return {
...doc,
isFork,
@ -451,6 +455,7 @@ function buildDocInfo(doc: Document, mode: OpenDocMode | undefined): DocInfo {
userOverride: null, // ditto.
isPreFork,
isBareFork,
isSnapshot,
isTutorialTrunk,
isTutorialFork,
isReadonly: !isEditable,

View File

@ -36,7 +36,7 @@ export function buildShareMenuButton(pageModel: DocPageModel): DomContents {
return dom.maybe(pageModel.currentDoc, (doc) => {
const appModel = pageModel.appModel;
const saveCopy = () => makeCopy(doc, appModel, t("Save Document")).catch(reportError);
if (doc.idParts.snapshotId) {
if (doc.isSnapshot) {
const backToCurrent = () => urlState().pushUrl({doc: buildOriginalUrlId(doc.id, true)});
return shareButton(t("Back to Current"), () => [
menuManageUsers(doc, pageModel),

View File

@ -86,7 +86,7 @@ export function createTopBarDoc(owner: MultiHolder, appModel: AppModel, pageMode
isRecoveryMode: pageModel.isRecoveryMode,
isTutorialFork: pageModel.isTutorialFork,
isFiddle: Computed.create(owner, (use) => use(pageModel.isPrefork)),
isSnapshot: Computed.create(owner, doc, (use, _doc) => Boolean(_doc && _doc.idParts.snapshotId)),
isSnapshot: pageModel.isSnapshot,
isPublic: Computed.create(owner, doc, (use, _doc) => Boolean(_doc && _doc.public)),
})
)

View File

@ -1249,7 +1249,7 @@ export class HomeDBManager extends EventEmitter {
doc.trunkAccess = doc.access;
// Update access for fork.
this._setForkAccess(doc, {userId, forkUserId, snapshotId}, doc);
if (forkId) { this._setForkAccess(doc, {userId, forkUserId}, doc); }
if (!doc.access) {
throw new ApiError('access denied', 403);
}
@ -2484,9 +2484,9 @@ export class HomeDBManager extends EventEmitter {
// If we are on a fork, make any access changes needed. Assumes results
// have been flattened.
if (forkId || snapshotId) {
if (forkId) {
for (const user of users) {
this._setForkAccess(doc, {userId: user.id, forkUserId, snapshotId}, user);
this._setForkAccess(doc, {userId: user.id, forkUserId}, user);
}
}
@ -3259,12 +3259,12 @@ export class HomeDBManager extends EventEmitter {
* their own in the db).
* - If fork is a tutorial:
* - User ~USERID from the fork id is owner, all others have no access.
* - If fork is a snapshot, all users are at most viewers. Else:
* - If fork is not a tutorial:
* - If there is no ~USERID in fork id, then all viewers of trunk are owners of the fork.
* - If there is a ~USERID in fork id, that user is owner, all others are at most viewers.
*/
private _setForkAccess(doc: Document,
ids: {userId: number, forkUserId?: number, snapshotId?: string},
ids: {userId: number, forkUserId?: number},
res: {access: roles.Role|null}) {
if (doc.type === 'tutorial') {
if (ids.userId === this.getPreviewerUserId()) {
@ -3286,10 +3286,6 @@ export class HomeDBManager extends EventEmitter {
res.access = roles.getWeakestRole('viewers', res.access);
}
}
// Finally, if we are viewing a snapshot, we can't edit it.
if (ids.snapshotId) {
res.access = roles.getWeakestRole('viewers', res.access);
}
}
}

View File

@ -96,6 +96,7 @@ import {GRIST_DOC_SQL, GRIST_DOC_WITH_TABLE1_SQL} from 'app/server/lib/initialDo
import {ISandbox} from 'app/server/lib/ISandbox';
import log from 'app/server/lib/log';
import {LogMethods} from "app/server/lib/LogMethods";
import {NullSandbox} from 'app/server/lib/NullSandbox';
import {DocRequests} from 'app/server/lib/Requests';
import {shortDesc} from 'app/server/lib/shortDesc';
import {TableMetadataLoader} from 'app/server/lib/TableMetadataLoader';
@ -226,7 +227,8 @@ export class ActiveDoc extends EventEmitter {
private _docUsage: DocumentUsage|null = null;
private _product?: Product;
private _gracePeriodStart: Date|null = null;
private _isForkOrSnapshot: boolean = false;
private _isSnapshot: boolean;
private _isForkOrSnapshot: boolean;
private _onlyAllowMetaDataActionsOnDb: boolean = false;
// Cache of which columns are attachment columns.
private _attachmentColumns?: AttachmentColumns;
@ -243,7 +245,14 @@ export class ActiveDoc extends EventEmitter {
private _shuttingDown: boolean = false;
private _afterShutdownCallback?: () => Promise<void>;
private _doShutdown?: Promise<void>;
private _intervals: Interval[] = [];
constructor(docManager: DocManager, docName: string, private _options?: ICreateActiveDocOptions) {
super();
const {forkId, snapshotId} = parseUrlId(docName);
this._isSnapshot = Boolean(snapshotId);
this._isForkOrSnapshot = Boolean(forkId || snapshotId);
if (!this._isSnapshot) {
/**
* In cases where large numbers of documents are restarted simultaneously
* (like during deployments), there's a tendency for scheduled intervals to
@ -252,7 +261,7 @@ export class ActiveDoc extends EventEmitter {
* To mitigate this, we use randomized intervals that re-compute their delay
* in-between calls, with a variance of 30 seconds.
*/
private _intervals = [
this._intervals.push(
// Cleanup expired attachments every hour (also happens when shutting down).
new Interval(
() => this.removeUnusedAttachments(true),
@ -277,12 +286,8 @@ export class ActiveDoc extends EventEmitter {
LOG_DOCUMENT_METRICS_DELAY,
{onError: (e) => this._log.error(null, 'failed to log document metrics', e)},
),
];
constructor(docManager: DocManager, docName: string, private _options?: ICreateActiveDocOptions) {
super();
const {forkId, snapshotId} = parseUrlId(docName);
this._isForkOrSnapshot = Boolean(forkId || snapshotId);
);
}
if (_options?.safeMode) { this._recoveryMode = true; }
if (_options?.doc) {
this._doc = _options.doc;
@ -630,13 +635,17 @@ export class ActiveDoc extends EventEmitter {
* DocManager.
*/
public async replace(docSession: OptDocSession, source: DocReplacementOptions) {
// During replacement, it is important for all hands to be off the document. So we
// ask the shutdown method to do the replacement when the ActiveDoc is shutdown but
// before a new one could be opened.
if (parseUrlId(this._docName).snapshotId) {
throw new ApiError('Snapshots cannot be replaced.', 400);
}
if (!await this._granularAccess.isOwner(docSession)) {
throw new ApiError('Only owners can replace a document.', 403);
}
this._log.debug(docSession, 'ActiveDoc.replace starting shutdown');
// During replacement, it is important for all hands to be off the document. So we
// ask the shutdown method to do the replacement when the ActiveDoc is shutdown but
// before a new one could be opened.
return this.shutdown({
afterShutdown: () => this._docManager.storageManager.replace(this.docName, source)
});
@ -959,7 +968,7 @@ export class ActiveDoc extends EventEmitter {
this._log.info(docSession, "fetchQuery %s %s", JSON.stringify(query),
onDemand ? "(onDemand)" : "(regular)");
let data: TableDataAction;
if (onDemand) {
if (onDemand || this._isSnapshot) {
data = await this._fetchQueryFromDB(query, onDemand);
} else if (wantFull) {
await this.waitForInitialization();
@ -2542,6 +2551,8 @@ export class ActiveDoc extends EventEmitter {
}
private async _makeEngine(): Promise<ISandbox> {
if (this._isSnapshot) { return new NullSandbox(); }
// Figure out what kind of engine we need for this document.
let preferredPythonVersion: '2' | '3' = process.env.PYTHON_VERSION === '3' ? '3' : '2';

View File

@ -0,0 +1,15 @@
import {ISandbox} from 'app/server/lib/ISandbox';
export class NullSandbox implements ISandbox {
public async shutdown(): Promise<unknown> {
return undefined;
}
public async pyCall(_funcName: string, ..._varArgs: unknown[]) {
return undefined;
}
public async reportMemoryUsage() {
return undefined;
}
}