Archives
/
gristlabs_grist-core
mirror of https://github.com/gristlabs/grist-core
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

(core) list inaccessible docs for editors/owners of workspaces

Browse Source 
Summary:
This modifies the material listed in workspaces.  Previously,
material the user did not have access to was omitted.  Now, it
is included if the user has the right to delete the workspace.
This is to avoid scenarios where a user might try to delete a
workspace without being aware of the full consequences.

Test Plan: added tests; existing tests should pass

Reviewers: dsagal

Reviewed By: dsagal

Differential Revision: https://phab.getgrist.com/D2568
pull/4/head
Paul Fitzpatrick 4 years ago
parent b82eec714a
commit 156b75133c
1 changed files with 37 additions and 25 deletions
Show Stats Download Patch File Download Diff File

62
app/gen-server/lib/HomeDBManager.ts
Unescape View File

@ -753,20 +753,7 @@ export class HomeDBManager extends EventEmitter {
// TODO: allow generic org limit once sample/support workspace is done differently
queryBuilder = this._applyLimit(queryBuilder, {...scope, org: undefined}, ['orgs', 'workspaces', 'docs']);
// We'd like to filter results to exclude docs for which the user doesn't have permissions.
// We filter by permissions in Javascript anyway, but doing it in SQL simplifies
// edge-cases about which workspaces to include (and is parsimonious).
// But permissions is a computed column, so not available for WHERE clauses (in postgres).
// So we use a subquery to get a hold of it.
const rawQueryBuilder = (options.manager || this._connection).createQueryBuilder()
.select('*')
.from('(' + queryBuilder.getQuery() + ')', 'orgs')
// Keep rows that only mention workspaces or orgs, or docs we have access to.
// docs_permissions and docs_id are docs.permissions and docs.id in the subquery.
.where('(docs_permissions > 0 or docs_id is null)')
.setParameters(queryBuilder.getParameters());
const result = await this._verifyAclPermissions(queryBuilder, {rawQueryBuilder});
const result = await this._verifyAclPermissions(queryBuilder, { scope });
// Return the workspaces, not the org(s).
if (result.status === 200) {
// Place ownership information in workspaces, available for the merged org.
@ -802,7 +789,7 @@ export class HomeDBManager extends EventEmitter {
// Add access information and query limits
// TODO: allow generic org limit once sample/support workspace is done differently
queryBuilder = this._applyLimit(queryBuilder, {...scope, org: undefined}, ['workspaces', 'docs']);
const result = await this._verifyAclPermissions(queryBuilder);
const result = await this._verifyAclPermissions(queryBuilder, { scope });
// Return a single workspace.
if (result.status === 200) {
result.data = result.data[0];
@ -3032,7 +3019,8 @@ export class HomeDBManager extends EventEmitter {
queryBuilder: SelectQueryBuilder<Resource>,
options: {
rawQueryBuilder?: SelectQueryBuilder<any>,
emptyAllowed?: boolean
emptyAllowed?: boolean,
scope?: Scope,
} = {}
): Promise<QueryResult<any>> {
const results = await (options.rawQueryBuilder ?
@ -3043,7 +3031,9 @@ export class HomeDBManager extends EventEmitter {
if (options.emptyAllowed) { return {status: 200, data: []}; }
return {errMessage: `${getFrom(queryBuilder)} not found`, status: 404};
}
const resources = this._normalizeQueryResults(results.entities);
const resources = this._normalizeQueryResults(results.entities, {
scope: options.scope,
});
if (resources.length === 0 && !options.emptyAllowed) {
return {errMessage: "access denied", status: 403};
} else {
@ -3065,7 +3055,10 @@ export class HomeDBManager extends EventEmitter {
// Find any nested entities with a "permissions" field, and add to them an
// "access" field (if the permission is a simple number) or an "accessOptions"
// field (if the permission is json). Entities in a list that the user doesn't
// have the right to access are removed.
// have the right to access may be removed.
// * They are removed for workspaces in orgs.
// * They are not removed for docs in workspaces, if user has right to delete
// the workspace.
//
// When returning organizations, set the domain to docs-${userId} for personal orgs.
// We could also have simply stored that domain in the database, but have kept
@ -3076,13 +3069,22 @@ export class HomeDBManager extends EventEmitter {
// in ugly o-NNNN form.
private _normalizeQueryResults(value: any,
options: {
suppressDomain?: boolean
suppressDomain?: boolean,
scope?: Scope,
parentPermissions?: number,
} = {}): any {
// We only need to examine objects, excluding null.
if (typeof value !== 'object' || value === null) { return value; }
// For arrays, add access information and remove anything user doesn't have access to.
// For arrays, add access information and remove anything user should not see.
if (Array.isArray(value)) {
return value.map(v => this._normalizeQueryResults(v, options)).filter(v => !this._isForbidden(v));
const items = value.map(v => this._normalizeQueryResults(v, options));
// If the items are not workspaces, and the user can delete their parent, then
// ignore the user's access level when deciding whether to filter them out or
// to keep them.
const ignoreAccess = options.parentPermissions &&
(options.parentPermissions & Permissions.REMOVE) && // tslint:disable-line:no-bitwise
items.length > 0 && !items[0].docs;
return items.filter(v => !this._isForbidden(v, Boolean(ignoreAccess), options.scope));
}
// For hashes, iterate through key/values, adding access info if 'permissions' field is found.
if (value.billingAccount) {
@ -3094,6 +3096,8 @@ export class HomeDBManager extends EventEmitter {
options = {...options, suppressDomain: true};
}
}
const permissions = (typeof value.permissions === 'number') ? value.permissions : undefined;
const childOptions = { ...options, parentPermissions: permissions };
for (const key of Object.keys(value)) {
const subValue = value[key];
// When returning organizations, set the domain to docs-${userId} for personal orgs.
@ -3123,7 +3127,7 @@ export class HomeDBManager extends EventEmitter {
continue;
}
if (key === 'managers') {
const managers = this._normalizeQueryResults(subValue, options);
const managers = this._normalizeQueryResults(subValue, childOptions);
for (const manager of managers) {
if (manager.user) {
Object.assign(manager, manager.user);
@ -3134,7 +3138,7 @@ export class HomeDBManager extends EventEmitter {
continue;
}
if (key !== 'permissions') {
value[key] = this._normalizeQueryResults(subValue, options);
value[key] = this._normalizeQueryResults(subValue, childOptions);
continue;
}
if (typeof subValue === 'number' || !subValue) {
@ -3157,10 +3161,18 @@ export class HomeDBManager extends EventEmitter {
// entity is forbidden if it contains an access field set to null, or an accessOptions field
// that is the empty list.
private _isForbidden(entity: any): boolean {
private _isForbidden(entity: any, ignoreAccess: boolean, scope?: Scope): boolean {
if (!entity) { return false; }
if (entity.access === null) { return true; }
if (entity.filteredOut) { return true; }
// Specifically for workspaces (as determined by having a "docs" field):
// if showing trash, and the workspace looks empty, and the workspace is itself
// not marked as trash, then filter it out. This situation can arise when there is
// a trash doc in a workspace that the user does not have access to, and also a
// doc that the user does have access to.
if (entity.docs && scope?.showRemoved && entity.docs.length === 0 &&
!entity.removedAt) { return true; }
if (ignoreAccess) { return false; }
if (entity.access === null) { return true; }
if (!entity.accessOptions) { return false; }
return entity.accessOptions.length === 0;
}

Write Preview
Loading…
Cancel
Save