From f4bbb90fa73e53842d41fee7ce2d99a9927865d8 Mon Sep 17 00:00:00 2001 From: Artem Niehrieiev Date: Thu, 6 Aug 2026 10:24:09 +0000 Subject: [PATCH] feat: implement column-level read permissions for table queries and responses --- .../export-csv-from-table.use.case.ts | 7 +- .../get-row-by-primary-key.use.case.ts | 37 ++-- .../use-cases/get-table-rows.use.case.ts | 57 +++--- .../non-saas-cedar-save-policy-e2e.test.ts | 170 ++++++++++++++++++ 4 files changed, 239 insertions(+), 32 deletions(-) diff --git a/backend/src/entities/table/use-cases/export-csv-from-table.use.case.ts b/backend/src/entities/table/use-cases/export-csv-from-table.use.case.ts index 0e0c809d2..9e11ac42d 100644 --- a/backend/src/entities/table/use-cases/export-csv-from-table.use.case.ts +++ b/backend/src/entities/table/use-cases/export-csv-from-table.use.case.ts @@ -111,9 +111,14 @@ export class ExportCSVFromTableUseCase if (isHexString(searchingFieldValue)) { searchingFieldValue = hexToBinary(searchingFieldValue) as any; // Readable columns only — a binary search must not reach a withheld column either. - tableSettings.search_fields = queryableStructure + // This must land on the settings object the DAO actually receives; assigning it to + // `tableSettings` (which was already consumed above) had no effect on the query. + const binarySearchFields = queryableStructure .filter((field) => isBinary(field.data_type)) .map((field) => field.column_name); + if (binarySearchFields.length > 0) { + builtDAOsTableSettings.search_fields = binarySearchFields; + } } const rowsStream = await dao.getTableRowsStream( diff --git a/backend/src/entities/table/use-cases/get-row-by-primary-key.use.case.ts b/backend/src/entities/table/use-cases/get-row-by-primary-key.use.case.ts index 032f1d908..c72be407f 100644 --- a/backend/src/entities/table/use-cases/get-row-by-primary-key.use.case.ts +++ b/backend/src/entities/table/use-cases/get-row-by-primary-key.use.case.ts @@ -36,6 +36,10 @@ import { filterReferencedTablesByPermission, } from '../utils/process-referenced-tables.util.js'; import { removePasswordsFromRowsUtil } from '../utils/remove-password-from-row.util.js'; +import { + assertSomeColumnReadable, + restrictTableSettingsToReadableColumns, +} from '../utils/restrict-query-to-readable-columns.util.js'; import { getUserEmailForAgent, validateConnection } from '../utils/validate-connection.util.js'; import { IGetRowByPrimaryKey } from './table-use-cases.interface.js'; @@ -133,13 +137,31 @@ export class GetRowByPrimaryKeyUseCase ), ); } + // Column-level read permission (the ColumnRead half of table:read), resolved BEFORE the query + // so a withheld column is never selected — and so a caller who may read no column at all gets + // a 403 instead of a row-existence answer (plan 13 P0-3; same rule as + // `pure-read-row-from-table.use.case.ts`). + const allColumnNames = tableStructure.map((column) => column.column_name); + const readableColumns = await this.cedarPermissions.getReadableColumns( + userId, + connectionId, + tableName, + allColumnNames, + ); + assertSomeColumnReadable(readableColumns); + let rowData: Record; const builtDAOsTableSettings = buildDAOsTableSettingsDs( buildCommonTableSettingsInput(tableSettings), personalTableSettings, ); + // The DAO's copy carries the withheld columns in `excluded_fields`, which bounds its + // `select()` list. The response keeps the unrestricted copy so the withheld column NAMES are + // not disclosed through `table_settings`. + const daoTableSettings = { ...builtDAOsTableSettings }; + restrictTableSettingsToReadableColumns(daoTableSettings, readableColumns, allColumnNames); try { - rowData = await dao.getRowByPrimaryKey(tableName, primaryKey, builtDAOsTableSettings, userEmail); + rowData = await dao.getRowByPrimaryKey(tableName, primaryKey, daoTableSettings, userEmail); } catch (e) { throw new UnknownSQLException(getErrorMessage(e), ExceptionOperations.FAILED_TO_GET_ROW_BY_PRIMARY_KEY); } @@ -155,16 +177,9 @@ export class GetRowByPrimaryKeyUseCase rowData = removePasswordsFromRowsUtil(rowData, tableWidgets); let formedTableStructure = formFullTableStructure(tableStructure, tableSettings); - // Column-level read permission (the ColumnRead half of table:read): strip columns the - // user may not read from the row and metadata. - const allColumnNames = tableStructure.map((column) => column.column_name); - const readableColumns = await this.cedarPermissions.getReadableColumns( - userId, - connectionId, - tableName, - allColumnNames, - ); - let listFields = findAvailableFields(builtDAOsTableSettings, tableStructure); + // Response-side projection, on top of the query-level restriction above (defense in depth: a + // widget or a DAO that ignores `excluded_fields` must not put a withheld column in the row). + let listFields = findAvailableFields(daoTableSettings, tableStructure); if (!isAllColumnsReadable(readableColumns, allColumnNames)) { rowData = filterRowByReadableColumns(rowData, readableColumns); formedTableStructure = filterStructureByReadableColumns(formedTableStructure, readableColumns); diff --git a/backend/src/entities/table/use-cases/get-table-rows.use.case.ts b/backend/src/entities/table/use-cases/get-table-rows.use.case.ts index 021c85b40..48e1e86d9 100644 --- a/backend/src/entities/table/use-cases/get-table-rows.use.case.ts +++ b/backend/src/entities/table/use-cases/get-table-rows.use.case.ts @@ -51,6 +51,11 @@ import { findOrderingFieldUtil } from '../utils/find-ordering-field.util.js'; import { formFullTableStructure } from '../utils/form-full-table-structure.js'; import { isHexString } from '../utils/is-hex-string.js'; import { processRowsUtil } from '../utils/process-found-rows-util.js'; +import { + assertSomeColumnReadable, + readableTableStructure, + restrictTableSettingsToReadableColumns, +} from '../utils/restrict-query-to-readable-columns.util.js'; import { getUserEmailForAgent, validateConnection } from '../utils/validate-connection.util.js'; import { IGetTableRows } from './table-use-cases.interface.js'; @@ -113,11 +118,28 @@ export class GetTableRowsUseCase extends AbstractUseCase column.column_name); + const readableColumns = await this.cedarPermissions.getReadableColumns( + userId, + connectionId, + tableName, + allColumnNames, + ); + assertSomeColumnReadable(readableColumns); + const restrictColumns = !isAllColumnsReadable(readableColumns, allColumnNames); + const queryableStructure = readableTableStructure(tableStructure, readableColumns); + const filteringFields: Array = isObjectEmpty(filters) - ? findFilteringFieldsUtil(query, tableStructure) - : parseFilteringFieldsFromBodyData(filters ?? {}, tableStructure); + ? findFilteringFieldsUtil(query, queryableStructure) + : parseFilteringFieldsFromBodyData(filters ?? {}, queryableStructure); - const orderingField = findOrderingFieldUtil(query, tableStructure, tableSettings); + const orderingField = findOrderingFieldUtil(query, queryableStructure, tableSettings); const configured = !!tableSettings; @@ -135,7 +157,7 @@ export class GetTableRowsUseCase extends AbstractUseCase isBinary(field.data_type)) || + (queryableStructure.some((field) => isBinary(field.data_type)) || connection.type === ConnectionTypesEnum.mongodb || connection.type === ConnectionTypesEnum.agent_mongodb) ) { searchingFieldValue = hexToBinary(searchingFieldValue) as any; - builtDAOsTableSettings.search_fields = tableStructure + // Readable columns only — a binary search must not reach a withheld column either. + builtDAOsTableSettings.search_fields = queryableStructure .filter((field) => isBinary(field.data_type)) .map((field) => field.column_name); if (connection.type === 'mongodb' || connection.type === 'agent_mongodb') { @@ -161,11 +184,17 @@ export class GetTableRowsUseCase extends AbstractUseCase Constants.LARGE_DATASET_ROW_LIMIT; - const listFields = findAvailableFields(builtDAOsTableSettings, tableStructure); + const listFields = findAvailableFields(daoTableSettings, tableStructure); const actionEventsDtos = customActionEvents.map((el) => buildActionEventDto(el)); const savedFiltersRO = savedTableFilters.map((el) => buildCreatedTableFilterRO(el)); - // Column-level read permission (the ColumnRead half of table:read). Computed once; - // when the user lacks read access to some columns we strip them from the rows and - // metadata below, after foreign-key identity enrichment has run. - const allColumnNames = tableStructure.map((column) => column.column_name); - const readableColumns = await this.cedarPermissions.getReadableColumns( - userId, - connectionId, - tableName, - allColumnNames, - ); - const restrictColumns = !isAllColumnsReadable(readableColumns, allColumnNames); - const rowsRO = { rows: rows.data, primaryColumns: tablePrimaryColumns, diff --git a/backend/test/ava-tests/non-saas-tests/non-saas-cedar-save-policy-e2e.test.ts b/backend/test/ava-tests/non-saas-tests/non-saas-cedar-save-policy-e2e.test.ts index cfc4c701a..ef67c023e 100644 --- a/backend/test/ava-tests/non-saas-tests/non-saas-cedar-save-policy-e2e.test.ts +++ b/backend/test/ava-tests/non-saas-tests/non-saas-cedar-save-policy-e2e.test.ts @@ -252,6 +252,176 @@ test.serial( }, ); +// Plan 13 P0-3 on the AUTHENTICATED read path: stripping a withheld column from the response still +// let a filter/search on it run in SQL, so `pagination.total` answered "does this column start with +// X?" one character at a time. The readable set now bounds the query itself, so a filter on a +// withheld column is ignored exactly like one naming a column that does not exist. +// The seeded table has 42 rows, 3 of them carrying 'Vasia' in `testTableColumnName`; the counts below +// are the oracle (42 = the filter was dropped, 3 = it ran). +function readOnlyCedarPolicy(connectionId: string, tableName: string, readableColumns: Array): string { + return [ + `permit(\n principal,\n action == RocketAdmin::Action::"connection:read",\n resource == RocketAdmin::Connection::"${connectionId}"\n);`, + `permit(\n principal,\n action == RocketAdmin::Action::"table:query",\n resource == RocketAdmin::Table::"${connectionId}/${tableName}"\n);`, + ...readableColumns.map( + (columnName) => + `permit(\n principal,\n action == RocketAdmin::Action::"column:read",\n resource == RocketAdmin::Column::"${connectionId}/${tableName}/${columnName}"\n);`, + ), + ].join('\n\n'); +} + +test.serial( + `${currentTest} authenticated rows: a filter on a withheld column is ignored in both filter forms`, + async (t) => { + try { + const testData = await createConnectionsAndInviteNewUserInNewGroupWithGroupPermissions(app); + const connectionId = testData.connections.firstId; + const groupId = testData.groups.createdGroupId; + const tableName = testData.firstTableInfo.testTableName; + // The 'Vasia' column is WITHHELD; only id + the email column are readable. + const hiddenColumn = testData.firstTableInfo.testTableColumnName; + const allowedColumn = testData.firstTableInfo.testTableSecondColumnName; + + const savePolicyResponse = await request(app.getHttpServer()) + .post(`/connection/cedar-policy/${connectionId}`) + .send({ cedarPolicy: readOnlyCedarPolicy(connectionId, tableName, ['id', allowedColumn]), groupId }) + .set('Cookie', testData.users.adminUserToken) + .set('Content-Type', 'application/json') + .set('Accept', 'application/json'); + t.is(savePolicyResponse.status, 201); + + // Query-string filter form (`f___eq`), used by GET /table/rows. + const queryFiltered = await request(app.getHttpServer()) + .get(`/table/rows/${connectionId}?tableName=${tableName}&page=1&perPage=10&f_${hiddenColumn}__eq=Vasia`) + .set('Cookie', testData.users.simpleUserToken) + .set('Content-Type', 'application/json') + .set('Accept', 'application/json'); + t.is(queryFiltered.status, 200); + t.is(queryFiltered.body.pagination.total, 42); + t.false(Object.keys(queryFiltered.body.rows[0]).includes(hiddenColumn)); + + // Body filter form, used by POST /table/rows/find. + const bodyFiltered = await request(app.getHttpServer()) + .post(`/table/rows/find/${connectionId}?tableName=${tableName}&page=1&perPage=10`) + .send({ filters: { [hiddenColumn]: { eq: 'Vasia' } } }) + .set('Cookie', testData.users.simpleUserToken) + .set('Content-Type', 'application/json') + .set('Accept', 'application/json'); + t.is(bodyFiltered.status, 200); + t.is(bodyFiltered.body.pagination.total, 42); + } catch (error) { + console.error(error); + throw error; + } + }, +); + +test.serial(`${currentTest} authenticated rows: search never reaches a withheld column`, async (t) => { + try { + const testData = await createConnectionsAndInviteNewUserInNewGroupWithGroupPermissions(app); + const connectionId = testData.connections.firstId; + const groupId = testData.groups.createdGroupId; + const tableName = testData.firstTableInfo.testTableName; + const hiddenColumn = testData.firstTableInfo.testTableColumnName; + const allowedColumn = testData.firstTableInfo.testTableSecondColumnName; + + const savePolicyResponse = await request(app.getHttpServer()) + .post(`/connection/cedar-policy/${connectionId}`) + .send({ cedarPolicy: readOnlyCedarPolicy(connectionId, tableName, ['id', allowedColumn]), groupId }) + .set('Cookie', testData.users.adminUserToken) + .set('Content-Type', 'application/json') + .set('Accept', 'application/json'); + t.is(savePolicyResponse.status, 201); + + // 'Vasia' exists only in the withheld column: the search must match nothing rather than + // returning those 3 rows (before the fix, search ILIKEd every text column of the table). + const searched = await request(app.getHttpServer()) + .get(`/table/rows/${connectionId}?tableName=${tableName}&page=1&perPage=10&search=Vasia`) + .set('Cookie', testData.users.simpleUserToken) + .set('Content-Type', 'application/json') + .set('Accept', 'application/json'); + t.is(searched.status, 200); + t.is(searched.body.rows.length, 0); + t.not(searched.body.pagination.total, 3); + } catch (error) { + console.error(error); + throw error; + } +}); + +test.serial(`${currentTest} authenticated rows: filter and search on a READABLE column still work`, async (t) => { + try { + const testData = await createConnectionsAndInviteNewUserInNewGroupWithGroupPermissions(app); + const connectionId = testData.connections.firstId; + const groupId = testData.groups.createdGroupId; + const tableName = testData.firstTableInfo.testTableName; + const allowedColumn = testData.firstTableInfo.testTableColumnName; + + const savePolicyResponse = await request(app.getHttpServer()) + .post(`/connection/cedar-policy/${connectionId}`) + .send({ cedarPolicy: readOnlyCedarPolicy(connectionId, tableName, ['id', allowedColumn]), groupId }) + .set('Cookie', testData.users.adminUserToken) + .set('Content-Type', 'application/json') + .set('Accept', 'application/json'); + t.is(savePolicyResponse.status, 201); + + const filtered = await request(app.getHttpServer()) + .get(`/table/rows/${connectionId}?tableName=${tableName}&page=1&perPage=10&f_${allowedColumn}__eq=Vasia`) + .set('Cookie', testData.users.simpleUserToken) + .set('Content-Type', 'application/json') + .set('Accept', 'application/json'); + t.is(filtered.status, 200); + t.is(filtered.body.pagination.total, 3); + t.is(filtered.body.rows.length, 3); + + const searched = await request(app.getHttpServer()) + .get(`/table/rows/${connectionId}?tableName=${tableName}&page=1&perPage=10&search=Vasia`) + .set('Cookie', testData.users.simpleUserToken) + .set('Content-Type', 'application/json') + .set('Accept', 'application/json'); + t.is(searched.status, 200); + t.is(searched.body.rows.length, 3); + } catch (error) { + console.error(error); + throw error; + } +}); + +test.serial(`${currentTest} authenticated reads fail CLOSED when no column is readable at all`, async (t) => { + try { + const testData = await createConnectionsAndInviteNewUserInNewGroupWithGroupPermissions(app); + const connectionId = testData.connections.firstId; + const groupId = testData.groups.createdGroupId; + const tableName = testData.firstTableInfo.testTableName; + + // table:query but NOT a single column:read. Answering "rows with every column stripped" + // would still hand back a real pagination.total to mine, so this is a 403. + const savePolicyResponse = await request(app.getHttpServer()) + .post(`/connection/cedar-policy/${connectionId}`) + .send({ cedarPolicy: readOnlyCedarPolicy(connectionId, tableName, []), groupId }) + .set('Cookie', testData.users.adminUserToken) + .set('Content-Type', 'application/json') + .set('Accept', 'application/json'); + t.is(savePolicyResponse.status, 201); + + const getRows = await request(app.getHttpServer()) + .get(`/table/rows/${connectionId}?tableName=${tableName}&page=1&perPage=10`) + .set('Cookie', testData.users.simpleUserToken) + .set('Content-Type', 'application/json') + .set('Accept', 'application/json'); + t.is(getRows.status, 403); + + const getRow = await request(app.getHttpServer()) + .get(`/table/row/${connectionId}?tableName=${tableName}&id=1`) + .set('Cookie', testData.users.simpleUserToken) + .set('Content-Type', 'application/json') + .set('Accept', 'application/json'); + t.is(getRow.status, 403); + } catch (error) { + console.error(error); + throw error; + } +}); + test.serial( `${currentTest} should enforce QueryTable - user without table:query is denied before the query`, async (t) => {