(core) limit access to list of snapshots for documents with granular access

Summary:
Snapshots can now only be listed for users with non-nuanced access
(no access rules, or owners on docs with rules). If a snapshot URL
leaks, or is shared by a user who can list snapshots, that URL
behaves as before -- it gives access to the snapshot according
to access rules in that snapshot.

Test Plan: added test

Reviewers: georgegevoian, dsagal

Reviewed By: georgegevoian, dsagal

Subscribers: jarek

Differential Revision: https://phab.getgrist.com/D3698
This commit is contained in:
Paul Fitzpatrick 2022-11-15 10:58:25 -05:00
parent ea71312d0e
commit 7b7b26c983
4 changed files with 24 additions and 8 deletions

View File

@ -68,11 +68,19 @@ export class DocHistory extends Disposable implements IDomComponent {
}
const snapshots = Observable.create<DocSnapshot[]>(owner, []);
const snapshotsDenied = Observable.create<boolean>(owner, false);
const userApi = this._docPageModel.appModel.api;
const docApi = userApi.getDocAPI(origUrlId);
docApi.getSnapshots().then(result =>
snapshots.isDisposed() || snapshots.set(result.snapshots)).catch(reportError);
return dom('div',
snapshots.isDisposed() || snapshots.set(result.snapshots)).catch(err => {
snapshotsDenied.set(true);
reportError(err);
});
return dom(
'div',
dom.maybe(snapshotsDenied, () => cssSnapshotDenied(
t('SnapshotsUnavailable'),
testId('doc-history-error'))),
// Note that most recent snapshots are first.
dom.domComputed(snapshots, (snapshotList) => snapshotList.map((snapshot, index) => {
const modified = moment(snapshot.lastModified);
@ -118,6 +126,10 @@ const cssSnapshot = styled('div', `
margin: 8px 16px;
`);
const cssSnapshotDenied = styled('div', `
margin: 8px 16px;
`);
const cssSnapshotTime = styled('div', `
text-align: right;
color: ${theme.lightText};

View File

@ -1624,8 +1624,10 @@ export class ActiveDoc extends EventEmitter {
this._inactivityTimer.ping();
}
public async getSnapshots(skipMetadataCache?: boolean): Promise<DocSnapshots> {
// Assume any viewer can access this list.
public async getSnapshots(docSession: OptDocSession, skipMetadataCache?: boolean): Promise<DocSnapshots> {
if (await this._granularAccess.hasNuancedAccess(docSession)) {
throw new Error('cannot confirm access to snapshots');
}
return this._docManager.storageManager.getSnapshots(this.docName, skipMetadataCache);
}

View File

@ -651,7 +651,8 @@ export class DocWorkerApi {
}));
this._app.get('/api/docs/:docId/snapshots', canView, withDoc(async (activeDoc, req, res) => {
const {snapshots} = await activeDoc.getSnapshots(isAffirmative(req.query.raw));
const docSession = docSessionFromRequest(req);
const {snapshots} = await activeDoc.getSnapshots(docSession, isAffirmative(req.query.raw));
res.json({snapshots});
}));
@ -666,8 +667,8 @@ export class DocWorkerApi {
if (req.body.select === 'unlisted') {
// Remove any snapshots not listed in inventory. Ideally, there should be no
// snapshots, and this undocumented feature is just for fixing up problems.
const full = (await activeDoc.getSnapshots(true)).snapshots.map(s => s.snapshotId);
const listed = new Set((await activeDoc.getSnapshots()).snapshots.map(s => s.snapshotId));
const full = (await activeDoc.getSnapshots(docSession, true)).snapshots.map(s => s.snapshotId);
const listed = new Set((await activeDoc.getSnapshots(docSession)).snapshots.map(s => s.snapshotId));
const unlisted = full.filter(snapshotId => !listed.has(snapshotId));
await activeDoc.removeSnapshots(docSession, unlisted);
res.json({snapshotIds: unlisted});
@ -676,7 +677,7 @@ export class DocWorkerApi {
if (req.body.select === 'past') {
// Remove all but the latest snapshot. Useful for sanitizing history if something
// bad snuck into previous snapshots and they are not valuable to preserve.
const past = (await activeDoc.getSnapshots(true)).snapshots.map(s => s.snapshotId);
const past = (await activeDoc.getSnapshots(docSession, true)).snapshots.map(s => s.snapshotId);
past.shift(); // remove current version.
await activeDoc.removeSnapshots(docSession, past);
res.json({snapshotIds: past});

View File

@ -111,6 +111,7 @@
"DocHistory": {
"Activity": "Activity",
"Snapshots": "Snapshots",
"SnapshotsUnavailable": "Snapshots are unavailable.",
"OpenSnapshot": "Open Snapshot",
"CompareToCurrent": "Compare to Current",
"CompareToPrevious": "Compare to Previous",