diff --git a/src/rls/index.test.ts b/src/rls/index.test.ts index cf00156..67d7585 100644 --- a/src/rls/index.test.ts +++ b/src/rls/index.test.ts @@ -98,6 +98,17 @@ describe('applyRLS - Query Modification', () => { expect(modifiedSql).toContain("WHERE `user_id` = 'user123'") }) it('should modify DELETE queries by adding policy-based WHERE clause', async () => { + vi.mocked(mockDataSource.rpc.executeQuery).mockResolvedValueOnce([ + { + actions: 'DELETE', + schema: 'public', + table: 'users', + column: 'user_id', + value: 'context.id()', + value_type: 'string', + operator: '=', + }, + ]) const sql = "DELETE FROM users WHERE name = 'Alice'" const modifiedSql = await applyRLS({ sql, @@ -107,9 +118,21 @@ describe('applyRLS - Query Modification', () => { }) expect(modifiedSql).toContain("WHERE `name` = 'Alice'") + expect(modifiedSql).toContain("`user_id` = 'user123'") }) it('should modify UPDATE queries with additional WHERE clause', async () => { + vi.mocked(mockDataSource.rpc.executeQuery).mockResolvedValueOnce([ + { + actions: 'UPDATE', + schema: 'public', + table: 'users', + column: 'user_id', + value: 'context.id()', + value_type: 'string', + operator: '=', + }, + ]) const sql = "UPDATE users SET name = 'Bob' WHERE age = 25" const modifiedSql = await applyRLS({ sql, @@ -119,9 +142,21 @@ describe('applyRLS - Query Modification', () => { }) expect(modifiedSql).toContain("`name` = 'Bob' WHERE `age` = 25") + expect(modifiedSql).toContain("`user_id` = 'user123'") }) it('should modify INSERT queries to enforce column values', async () => { + vi.mocked(mockDataSource.rpc.executeQuery).mockResolvedValueOnce([ + { + actions: 'INSERT', + schema: 'public', + table: 'users', + column: 'user_id', + value: 'context.id()', + value_type: 'string', + operator: '=', + }, + ]) const sql = "INSERT INTO users (user_id, name) VALUES (1, 'Alice')" const modifiedSql = await applyRLS({ sql, @@ -130,7 +165,7 @@ describe('applyRLS - Query Modification', () => { config: mockConfig, }) - expect(modifiedSql).toContain("VALUES (1,'Alice')") + expect(modifiedSql).toContain("VALUES ('user123','Alice')") }) }) @@ -148,14 +183,17 @@ describe('applyRLS - Edge Cases', () => { }) it('should not modify SQL if user is admin', async () => { - mockConfig.role = 'admin' + const adminConfig: StarbaseDBConfiguration = { + ...mockConfig, + role: 'admin', + } const sql = 'SELECT * FROM users' const modifiedSql = await applyRLS({ sql, isEnabled: true, dataSource: mockDataSource, - config: mockConfig, + config: adminConfig, }) expect(modifiedSql).toBe(sql) @@ -200,8 +238,8 @@ describe('applyRLS - Multi-Table Queries', () => { config: mockConfig, }) - expect(modifiedSql).toContain("WHERE `users.user_id` = 'user123'") - expect(modifiedSql).toContain("AND `orders.user_id` = 'user123'") + expect(modifiedSql).toContain("`users`.`user_id` = 'user123'") + expect(modifiedSql).toContain("`orders`.`user_id` = 'user123'") }) it('should apply RLS policies to multiple tables in a JOIN', async () => { @@ -218,8 +256,8 @@ describe('applyRLS - Multi-Table Queries', () => { config: mockConfig, }) - expect(modifiedSql).toContain("WHERE (users.user_id = 'user123')") - expect(modifiedSql).toContain("AND (orders.user_id = 'user123')") + expect(modifiedSql).toContain("`users`.`user_id` = 'user123'") + expect(modifiedSql).toContain("`orders`.`user_id` = 'user123'") }) it('should apply RLS policies to subqueries inside FROM clause', async () => { @@ -236,6 +274,6 @@ describe('applyRLS - Multi-Table Queries', () => { config: mockConfig, }) - expect(modifiedSql).toContain("WHERE `users.user_id` = 'user123'") + expect(modifiedSql).toContain("`user_id` = 'user123'") }) }) diff --git a/src/rls/index.ts b/src/rls/index.ts index 68abb4e..0dc0942 100644 --- a/src/rls/index.ts +++ b/src/rls/index.ts @@ -235,10 +235,15 @@ function applyRLSToAst(ast: any): void { const tablesWithRules: Record = {} policies.forEach((policy) => { const tbl = normalizeIdentifier(policy.condition.left.table) + const baseTbl = tbl.includes('.') ? tbl.split('.')[1] : tbl if (!tablesWithRules[tbl]) { tablesWithRules[tbl] = [] } + if (!tablesWithRules[baseTbl]) { + tablesWithRules[baseTbl] = [] + } tablesWithRules[tbl].push(policy.action) + tablesWithRules[baseTbl].push(policy.action) }) const statementType = ast.type?.toUpperCase() @@ -266,11 +271,11 @@ function applyRLSToAst(ast: any): void { tables = ast.from?.map((fromTable: any) => { let tableName = normalizeIdentifier(fromTable.table) - if (tableName.includes('.')) { + if (tableName && tableName.includes('.')) { tableName = tableName.split('.')[1] } return tableName - }) || [] + }).filter(Boolean) || [] } const restrictedTables = Object.keys(tablesWithRules) @@ -292,30 +297,38 @@ function applyRLSToAst(ast: any): void { ) .forEach(({ action, condition }) => { const targetTable = normalizeIdentifier(condition.left.table) - const isTargetTable = tables.includes(targetTable) + const baseTargetTable = targetTable.includes('.') + ? targetTable.split('.')[1] + : targetTable + const isTargetTable = + tables.includes(targetTable) || tables.includes(baseTargetTable) if (!isTargetTable) return + const resolvedCondition = { + ...condition, + left: { + ...condition.left, + table: tables.length > 1 ? baseTargetTable : null, + }, + } + if (action !== 'INSERT') { - // Add condition to WHERE with parentheses + // Add condition to WHERE if (ast.where) { ast.where = { type: 'binary_expr', operator: 'AND', - parentheses: true, left: { ...ast.where, - parentheses: true, }, right: { - ...condition, - parentheses: true, + ...resolvedCondition, }, } } else { ast.where = { - ...condition, - parentheses: true, + ...resolvedCondition, } } } else { @@ -349,8 +362,12 @@ function applyRLSToAst(ast: any): void { }) ast.from?.forEach((fromItem: any) => { - if (fromItem.expr && fromItem.expr.type === 'select') { - applyRLSToAst(fromItem.expr) + if (fromItem.expr) { + if (fromItem.expr.ast) { + applyRLSToAst(fromItem.expr.ast) + } else if (fromItem.expr.type === 'select') { + applyRLSToAst(fromItem.expr) + } } // Handle both single join and array of joins