(core) Limiting doc remove permission to owners.

Summary:
Guest editors added to a document were able to remove it. This limits this permission
by allowing only owners of a doc to delete it.

Test Plan: Updated

Reviewers: paulfitz

Reviewed By: paulfitz

Subscribers: dsagal, anaisconce

Differential Revision: https://phab.getgrist.com/D3708
This commit is contained in:
Jarosław Sadziński 2022-11-30 23:10:07 +01:00
parent 601ba58a2e
commit 59942a23b6
4 changed files with 38 additions and 9 deletions

View File

@ -488,7 +488,7 @@ export function makeDocOptionsMenu(home: HomeModel, doc: Document, renaming: Obs
testId('move-doc') testId('move-doc')
), ),
menuItem(deleteDoc, t('Remove'), menuItem(deleteDoc, t('Remove'),
dom.cls('disabled', !roles.canDelete(doc.access)), dom.cls('disabled', !roles.isOwner(doc)),
testId('delete-doc') testId('delete-doc')
), ),
menuItem(() => home.pinUnpinDoc(doc.id, !doc.isPinned).catch(reportError), menuItem(() => home.pinUnpinDoc(doc.id, !doc.isPinned).catch(reportError),
@ -511,11 +511,11 @@ export function makeRemovedDocOptionsMenu(home: HomeModel, doc: Document, worksp
return [ return [
menuItem(() => home.restoreDoc(doc), t('Restore'), menuItem(() => home.restoreDoc(doc), t('Restore'),
dom.cls('disabled', !roles.canDelete(doc.access) || !!workspace.removedAt), dom.cls('disabled', !roles.isOwner(doc) || !!workspace.removedAt),
testId('doc-restore') testId('doc-restore')
), ),
menuItem(hardDeleteDoc, t('DeleteForever'), menuItem(hardDeleteDoc, t('DeleteForever'),
dom.cls('disabled', !roles.canDelete(doc.access)), dom.cls('disabled', !roles.isOwner(doc)),
testId('doc-delete-forever') testId('doc-delete-forever')
), ),
(workspace.removedAt ? (workspace.removedAt ?

View File

@ -1885,7 +1885,7 @@ export class HomeDBManager extends EventEmitter {
return await this._connection.transaction(async manager => { return await this._connection.transaction(async manager => {
const docQuery = this._doc(scope, { const docQuery = this._doc(scope, {
manager, manager,
markPermissions: Permissions.REMOVE, markPermissions: Permissions.REMOVE | Permissions.SCHEMA_EDIT,
allowSpecialPermit: true allowSpecialPermit: true
}) })
// Join the docs's ACLs and groups so we can remove them. // Join the docs's ACLs and groups so we can remove them.
@ -3396,7 +3396,7 @@ export class HomeDBManager extends EventEmitter {
effectiveUserId = this.getPreviewerUserId(); effectiveUserId = this.getPreviewerUserId();
threshold = Permissions.VIEW; threshold = Permissions.VIEW;
} }
// Compute whether we have access to the doc // Compute whether we have access to the ws
query = query.addSelect( query = query.addSelect(
this._markIsPermitted('workspaces', effectiveUserId, 'open', threshold), this._markIsPermitted('workspaces', effectiveUserId, 'open', threshold),
'is_permitted' 'is_permitted'
@ -4298,7 +4298,7 @@ export class HomeDBManager extends EventEmitter {
return this._connection.transaction(async manager => { return this._connection.transaction(async manager => {
let docQuery = this._doc({...scope, showAll: true}, { let docQuery = this._doc({...scope, showAll: true}, {
manager, manager,
markPermissions: Permissions.REMOVE, markPermissions: Permissions.SCHEMA_EDIT | Permissions.REMOVE,
allowSpecialPermit: true allowSpecialPermit: true
}); });
if (!removedAt) { if (!removedAt) {

View File

@ -26,6 +26,19 @@ export function configForUser(username: string): AxiosRequestConfig {
return config; return config;
} }
/**
* Appends a permit key to the given config. Creates a new config object.
*/
export function configWithPermit(config: AxiosRequestConfig, permitKey: string): AxiosRequestConfig {
return {
...config,
headers: {
...config.headers,
Permit: permitKey
}
};
}
/** /**
* Create a new user and return their personal org. * Create a new user and return their personal org.
*/ */

View File

@ -198,6 +198,21 @@ describe('DocApi', function() {
// Contains the tests. This is where you want to add more test. // Contains the tests. This is where you want to add more test.
function testDocApi() { function testDocApi() {
it("should allow only owners to remove a document", async () => {
const ws1 = (await userApi.getOrgWorkspaces('current'))[0].id;
const doc1 = await userApi.newDoc({name: 'testdeleteme1'}, ws1);
const kiwiApi = makeUserApi(ORG_NAME, 'kiwi');
// Kiwi is editor of the document, so he can't delete it.
await userApi.updateDocPermissions(doc1, {users: {'kiwi@getgrist.com': 'editors'}});
await assert.isRejected(kiwiApi.softDeleteDoc(doc1), /Forbidden/);
await assert.isRejected(kiwiApi.deleteDoc(doc1), /Forbidden/);
// Kiwi is owner of the document - now he can delete it.
await userApi.updateDocPermissions(doc1, {users: {'kiwi@getgrist.com': 'owners'}});
await assert.isFulfilled(kiwiApi.softDeleteDoc(doc1));
await assert.isFulfilled(kiwiApi.deleteDoc(doc1));
});
it("guesses types of new columns", async () => { it("guesses types of new columns", async () => {
const userActions = [ const userActions = [
@ -3151,6 +3166,7 @@ interface WebhookRequests {
"add,update": object[][]; "add,update": object[][];
} }
const ORG_NAME = 'docs-1';
function setup(name: string, cb: () => Promise<void>) { function setup(name: string, cb: () => Promise<void>) {
let api: UserAPIImpl; let api: UserAPIImpl;
@ -3162,7 +3178,7 @@ function setup(name: string, cb: () => Promise<void>) {
await cb(); await cb();
// create TestDoc as an empty doc into Private workspace // create TestDoc as an empty doc into Private workspace
userApi = api = makeUserApi('docs-1'); userApi = api = makeUserApi(ORG_NAME);
const wid = await getWorkspaceId(api, 'Private'); const wid = await getWorkspaceId(api, 'Private');
docIds.TestDoc = await api.newDoc({name: 'TestDoc'}, wid); docIds.TestDoc = await api.newDoc({name: 'TestDoc'}, wid);
}); });
@ -3178,9 +3194,9 @@ function setup(name: string, cb: () => Promise<void>) {
}); });
} }
function makeUserApi(org: string) { function makeUserApi(org: string, user?: string) {
return new UserAPIImpl(`${home.serverUrl}/o/${org}`, { return new UserAPIImpl(`${home.serverUrl}/o/${org}`, {
headers: {Authorization: 'Bearer api_key_for_chimpy'}, headers: {Authorization: `Bearer api_key_for_${user || 'chimpy'}`},
fetch: fetch as any, fetch: fetch as any,
newFormData: () => new FormData() as any, newFormData: () => new FormData() as any,
logger: log logger: log