diff --git a/app/gen-server/lib/HomeDBManager.ts b/app/gen-server/lib/HomeDBManager.ts index 9ae645ee..d2a64a94 100644 --- a/app/gen-server/lib/HomeDBManager.ts +++ b/app/gen-server/lib/HomeDBManager.ts @@ -24,7 +24,7 @@ import {Workspace} from "app/gen-server/entity/Workspace"; import {Permissions} from 'app/gen-server/lib/Permissions'; import {scrubUserFromOrg} from "app/gen-server/lib/scrubUserFromOrg"; import {applyPatch} from 'app/gen-server/lib/TypeORMPatches'; -import {bitOr, now, readJson} from 'app/gen-server/sqlUtils'; +import {bitOr, getRawAndEntities, now, readJson} from 'app/gen-server/sqlUtils'; import {makeId} from 'app/server/lib/idUtils'; import * as log from 'app/server/lib/log'; import {Permit} from 'app/server/lib/Permit'; @@ -725,6 +725,7 @@ export class HomeDBManager extends EventEmitter { .leftJoin('orgs.billingAccount', 'account') .leftJoin('account.product', 'product') .addSelect('product.features') + .addSelect('product.id') .addSelect('account.id') // order the support org (aka Samples/Examples) after other ones. .orderBy('coalesce(orgs.owner_id = :supportId, false)') @@ -751,7 +752,21 @@ 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}, ['orgs', 'workspaces', 'docs']); - const result = await this._verifyAclPermissions(queryBuilder); + + // 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}); // Return the workspaces, not the org(s). if (result.status === 200) { // Place ownership information in workspaces, available for the merged org. @@ -819,7 +834,7 @@ export class HomeDBManager extends EventEmitter { let qb = this._orgs(); qb = this._whereOrg(qb, org); qb = this._withAccess(qb, users, 'orgs'); - const result = await this._verifyAclPermissions(qb, true); + const result = await this._verifyAclPermissions(qb, {emptyAllowed: true}); if (!result.data) { throw new ApiError(result.errMessage || 'failed to select user', result.status); } @@ -869,7 +884,7 @@ export class HomeDBManager extends EventEmitter { return {status: 200, data: []}; } } - return this._verifyAclPermissions(queryBuilder, true); + return this._verifyAclPermissions(queryBuilder, {emptyAllowed: true}); } // As for getOrgs, but all personal orgs are merged into a single entry. @@ -2998,6 +3013,9 @@ export class HomeDBManager extends EventEmitter { } // Return a QueryResult reflecting the output of a query builder. + // If a rawQueryBuilder is supplied, it is used to make the query, + // but then the original queryBuilder is used to interpret the results + // as entities (make sure the two queries give results in the same format!) // Checks on all "permissions" fields which select queries set on // resources to indicate whether the user has access. // If the output is empty, and `emptyAllowed` is not set, we signal that the desired @@ -3008,16 +3026,21 @@ export class HomeDBManager extends EventEmitter { // Returns the resource fetched by the queryBuilder. private async _verifyAclPermissions( queryBuilder: SelectQueryBuilder, - emptyAllowed: boolean = false + options: { + rawQueryBuilder?: SelectQueryBuilder, + emptyAllowed?: boolean + } = {} ): Promise> { - const results = await queryBuilder.getRawAndEntities(); + const results = await (options.rawQueryBuilder ? + getRawAndEntities(options.rawQueryBuilder, queryBuilder) : + queryBuilder.getRawAndEntities()); if (results.entities.length === 0 || (results.entities.length === 1 && results.entities[0].filteredOut)) { - if (emptyAllowed) { return {status: 200, data: []}; } + if (options.emptyAllowed) { return {status: 200, data: []}; } return {errMessage: `${getFrom(queryBuilder)} not found`, status: 404}; } const resources = this._normalizeQueryResults(results.entities); - if (resources.length === 0 && !emptyAllowed) { + if (resources.length === 0 && !options.emptyAllowed) { return {errMessage: "access denied", status: 403}; } else { return { diff --git a/app/gen-server/sqlUtils.ts b/app/gen-server/sqlUtils.ts index 14242ee1..6a1e9818 100644 --- a/app/gen-server/sqlUtils.ts +++ b/app/gen-server/sqlUtils.ts @@ -1,4 +1,7 @@ -import {DatabaseType} from 'typeorm'; +import {DatabaseType, QueryRunner, SelectQueryBuilder} from 'typeorm'; +import {RelationCountLoader} from 'typeorm/query-builder/relation-count/RelationCountLoader'; +import {RelationIdLoader} from 'typeorm/query-builder/relation-id/RelationIdLoader'; +import {RawSqlResultsToEntityTransformer} from "typeorm/query-builder/transformer/RawSqlResultsToEntityTransformer"; /** * @@ -88,3 +91,68 @@ export function datetime(dbType: DatabaseType) { throw new Error(`now not implemented for ${dbType}`); } } + +/** + * + * Generate SQL code from one QueryBuilder, get the "raw" results, and then decode + * them as entities using a different QueryBuilder. + * + * This is useful for example if we have a query Q and we wish to add + * a where clause referring to one of the query's selected columns by + * its alias. This isn't supported by Postgres (since the SQL + * standard says not to). A simple solution is to wrap Q in a query + * like "SELECT * FROM (Q) WHERE ...". But if we do that in TypeORM, + * it loses track of metadata and isn't able to decode the results, + * even though nothing has changed structurally. Hence this method. + * + * (An alternate solution to this scenario is to simply duplicate the + * SQL code for the selected column in the where clause. But our SQL + * queries are getting awkwardly long.) + * + * The results are returned in the same format as SelectQueryBuilder's + * getRawAndEntities. + */ +export async function getRawAndEntities(rawQueryBuilder: SelectQueryBuilder, + nominalQueryBuilder: SelectQueryBuilder): Promise<{ + entities: T[], + raw: any[], +}> { + const raw = await rawQueryBuilder.getRawMany(); + + // The following code is based on SelectQueryBuilder's + // executeEntitiesAndRawResults. To extract and use it here, we + // need to access the QueryBuilder's QueryRunner, which is + // protected, so we break abstraction a little bit. + const runnerSource = nominalQueryBuilder as any as QueryRunnerSource; + const queryRunner = runnerSource.obtainQueryRunner(); + try { + const expressionMap = nominalQueryBuilder.expressionMap; + const connection = nominalQueryBuilder.connection; + const relationIdLoader = new RelationIdLoader(connection, queryRunner, expressionMap.relationIdAttributes); + const relationCountLoader = new RelationCountLoader(connection, queryRunner, expressionMap.relationCountAttributes); + const rawRelationIdResults = await relationIdLoader.load(raw); + const rawRelationCountResults = await relationCountLoader.load(raw); + const transformer = new RawSqlResultsToEntityTransformer(expressionMap, connection.driver, + rawRelationIdResults, rawRelationCountResults, + queryRunner); + const entities = transformer.transform(raw, expressionMap.mainAlias!); + return { + entities, + raw, + }; + } finally { + // This is how the QueryBuilder <-> QueryRunner relationship is managed in TypeORM code. + if (queryRunner !== runnerSource.queryRunner) { + await queryRunner.release(); + } + } +} + +/** + * QueryBuilders keep track of a runner that we need for getRawAndEntities, + * but access is protected. This interface declared the fields we expect. + */ +interface QueryRunnerSource { + queryRunner: QueryRunner; + obtainQueryRunner(): QueryRunner; +}