From 0fb77712e334670530d544d21395179ac5ed9465 Mon Sep 17 00:00:00 2001 From: Brent Moran Date: Fri, 1 Nov 2024 15:05:54 +0800 Subject: [PATCH 1/8] change format for record lister when no privilege --- db/sql/00_msar.sql | 6 +++--- db/sql/test_00_msar.sql | 14 ++++++++++++++ 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/db/sql/00_msar.sql b/db/sql/00_msar.sql index 5159c9d6e6..718eb71cf0 100644 --- a/db/sql/00_msar.sql +++ b/db/sql/00_msar.sql @@ -5123,15 +5123,15 @@ BEGIN LEFT JOIN groups_cte ON enriched_results_cte.__mathesar_gid = groups_cte.id %14$s CROSS JOIN count_cte $q$, - /* %1 */ msar.build_selectable_column_expr(tab_id), + /* %1 */ COALESCE(msar.build_selectable_column_expr(tab_id), 'NULL'), /* %2 */ msar.get_relation_schema_name(tab_id), /* %3 */ msar.get_relation_name(tab_id), /* %4 */ limit_, /* %5 */ offset_, /* %6 */ msar.build_order_by_expr(tab_id, order_), /* %7 */ msar.build_where_clause(tab_id, filter_), - /* %8 */ msar.build_grouping_expr(tab_id, group_), - /* %9 */ msar.build_results_jsonb_expr(tab_id, 'enriched_results_cte', order_), + /* %8 */ COALESCE(msar.build_grouping_expr(tab_id, group_), 'NULL'), + /* %9 */ COALESCE(msar.build_results_jsonb_expr(tab_id, 'enriched_results_cte', order_), 'NULL'), /* %10 */ COALESCE( msar.build_grouping_results_jsonb_expr(tab_id, 'groups_cte', group_), 'NULL' diff --git a/db/sql/test_00_msar.sql b/db/sql/test_00_msar.sql index b86d694d42..f59cdb71b0 100644 --- a/db/sql/test_00_msar.sql +++ b/db/sql/test_00_msar.sql @@ -3249,6 +3249,20 @@ BEGIN ) ) ); + SET ROLE NONE; + CREATE ROLE intern_no_access; + GRANT USAGE ON SCHEMA msar, __msar TO intern_no_access; + GRANT EXECUTE ON ALL FUNCTIONS IN SCHEMA msar, __msar TO intern_no_access; + SET ROLE intern_no_access; + RETURN NEXT throws_ok( + format( + 'SELECT msar.list_records_from_table(%s, null, null, null, null, null);', + rel_id + ), + '42501', + 'permission denied for table atable', + 'Records lister throws permission error' + ); END; $$ LANGUAGE plpgsql; From bcdd03991d3fd2a4c9c3e1d70e50bcaa810ae00a Mon Sep 17 00:00:00 2001 From: Brent Moran Date: Mon, 4 Nov 2024 09:14:12 +0800 Subject: [PATCH 2/8] add some tests specifying current record lister behavior --- db/sql/test_00_msar.sql | 221 +++++++++++++++++++++++++++------------- 1 file changed, 148 insertions(+), 73 deletions(-) diff --git a/db/sql/test_00_msar.sql b/db/sql/test_00_msar.sql index f59cdb71b0..10698a8dad 100644 --- a/db/sql/test_00_msar.sql +++ b/db/sql/test_00_msar.sql @@ -3190,79 +3190,6 @@ BEGIN ) ) ); - CREATE ROLE intern_no_pkey; - GRANT USAGE ON SCHEMA msar, __msar TO intern_no_pkey; - GRANT EXECUTE ON ALL FUNCTIONS IN SCHEMA msar, __msar TO intern_no_pkey; - GRANT SELECT (col1, col2, col3, col4) ON TABLE atable TO intern_no_pkey; - SET ROLE intern_no_pkey; - RETURN NEXT is( - msar.list_records_from_table( - tab_id => rel_id, - limit_ => null, - offset_ => null, - order_ => null, - filter_ => null, - group_ => null - ), - $j${ - "count": 3, - "results": [ - {"2": 2, "3": "abcde", "4": {"k": 3242348}, "5": true}, - {"2": 5, "3": "sdflkj", "4": "s", "5": {"a": "val"}}, - {"2": 34, "3": "sdflfflsk", "4": null, "5": [1, 2, 3, 4]} - ], - "grouping": null, - "linked_record_summaries": null, - "record_summaries": null - }$j$ || jsonb_build_object( - 'query', concat( - 'SELECT msar.format_data(col1) AS "2", msar.format_data(col2) AS "3",', - ' msar.format_data(col3) AS "4", msar.format_data(col4) AS "5" FROM public.atable', - ' ORDER BY "2" ASC, "3" ASC, "5" ASC LIMIT NULL OFFSET NULL' - ) - ) - ); - RETURN NEXT is( - msar.list_records_from_table( - tab_id => rel_id, - limit_ => null, - offset_ => null, - order_ => '[{"attnum": 3, "direction": "desc"}]', - filter_ => null, - group_ => null - ), - $j${ - "count": 3, - "results": [ - {"2": 5, "3": "sdflkj", "4": "s", "5": {"a": "val"}}, - {"2": 34, "3": "sdflfflsk", "4": null, "5": [1, 2, 3, 4]}, - {"2": 2, "3": "abcde", "4": {"k": 3242348}, "5": true} - ], - "grouping": null, - "linked_record_summaries": null, - "record_summaries": null - }$j$ || jsonb_build_object( - 'query', concat( - 'SELECT msar.format_data(col1) AS "2", msar.format_data(col2) AS "3",', - ' msar.format_data(col3) AS "4", msar.format_data(col4) AS "5" FROM public.atable', - ' ORDER BY "3" DESC, "2" ASC, "3" ASC, "5" ASC LIMIT NULL OFFSET NULL' - ) - ) - ); - SET ROLE NONE; - CREATE ROLE intern_no_access; - GRANT USAGE ON SCHEMA msar, __msar TO intern_no_access; - GRANT EXECUTE ON ALL FUNCTIONS IN SCHEMA msar, __msar TO intern_no_access; - SET ROLE intern_no_access; - RETURN NEXT throws_ok( - format( - 'SELECT msar.list_records_from_table(%s, null, null, null, null, null);', - rel_id - ), - '42501', - 'permission denied for table atable', - 'Records lister throws permission error' - ); END; $$ LANGUAGE plpgsql; @@ -5598,3 +5525,151 @@ BEGIN ); END; $$ LANGUAGE plpgsql; + + +CREATE OR REPLACE FUNCTION test_table_select_permissions() RETURNS SETOF TEXT AS $$/* +This test is to check behavior whenever we're selecting from a user table where the calling user has +SELECT permissions on some proper subset. + +- when a user has SELECT on some, but not all, columns we should return results only for the + columns for which they have access. +- when a user doesn't have SELECT on any columns of a table, we should raise a permissions error. +*/ +DECLARE + rel_id oid; +BEGIN + PERFORM __setup_list_records_table(); + rel_id := 'atable'::regclass::oid; + + CREATE ROLE intern_no_pkey; + GRANT USAGE ON SCHEMA msar, __msar TO intern_no_pkey; + GRANT EXECUTE ON ALL FUNCTIONS IN SCHEMA msar, __msar TO intern_no_pkey; + GRANT SELECT ON ALL TABLES IN SCHEMA msar, __msar TO intern_no_pkey; + GRANT SELECT (col1, col2, col3, col4) ON TABLE atable TO intern_no_pkey; + SET ROLE intern_no_pkey; + RETURN NEXT is( + msar.list_records_from_table( + tab_id => rel_id, + limit_ => null, + offset_ => null, + order_ => null, + filter_ => null, + group_ => null + ) -> 'results', + $j$[ + {"2": 2, "3": "abcde", "4": {"k": 3242348}, "5": true}, + {"2": 5, "3": "sdflkj", "4": "s", "5": {"a": "val"}}, + {"2": 34, "3": "sdflfflsk", "4": null, "5": [1, 2, 3, 4]} + ]$j$, + 'Results should not have column 1, and should be ordered by remaining columns' + ); + RETURN NEXT is( + msar.list_records_from_table( + tab_id => rel_id, + limit_ => null, + offset_ => null, + order_ => '[{"attnum": 3, "direction": "desc"}]', + filter_ => null, + group_ => null + ) -> 'results', + $j$[ + {"2": 5, "3": "sdflkj", "4": "s", "5": {"a": "val"}}, + {"2": 34, "3": "sdflfflsk", "4": null, "5": [1, 2, 3, 4]}, + {"2": 2, "3": "abcde", "4": {"k": 3242348}, "5": true} + ]$j$, + 'Results should not have a column 1, and ordering spec should work' + ); + RETURN NEXT is( + msar.list_records_from_table( + tab_id => rel_id, + limit_ => null, + offset_ => null, + order_ => '[{"attnum": 1, "direction": "asc"}]', + filter_ => null, + group_ => null + ) -> 'results', + $j$[ + {"2": 2, "3": "abcde", "4": {"k": 3242348}, "5": true}, + {"2": 5, "3": "sdflkj", "4": "s", "5": {"a": "val"}}, + {"2": 34, "3": "sdflfflsk", "4": null, "5": [1, 2, 3, 4]} + ]$j$, + 'specifying that you want to order by a column without permissions is ignored' + ); + RETURN NEXT is( + msar.list_records_from_table( + tab_id => rel_id, + limit_ => null, + offset_ => null, + order_ => '[{"attnum": 1, "direction": "asc", "attnum": 3, "direction": "desc"}]', + filter_ => null, + group_ => null + ) -> 'results', + $j$[ + {"2": 5, "3": "sdflkj", "4": "s", "5": {"a": "val"}}, + {"2": 34, "3": "sdflfflsk", "4": null, "5": [1, 2, 3, 4]}, + {"2": 2, "3": "abcde", "4": {"k": 3242348}, "5": true} + ]$j$, + 'ignore order by column without permissions, use one with permissions' + ); + RETURN NEXT is( + msar.list_records_from_table( + tab_id => rel_id, + limit_ => null, + offset_ => null, + order_ => null, + filter_ => jsonb_build_object( + 'type', 'equal', 'args', jsonb_build_array( + jsonb_build_object('type', 'attnum', 'value', 2), + jsonb_build_object('type', 'literal', 'value', 2) + ) + ), + group_ => null + ) -> 'results', + $j$[ + {"2": 2, "3": "abcde", "4": {"k": 3242348}, "5": true} + ]$j$, + 'filtering without specifying column without permissions works' + ); + + + RETURN NEXT throws_ok( + format( + $s$SELECT + msar.list_records_from_table( + tab_id => %s, + limit_ => null, + offset_ => null, + order_ => null, + filter_ => %L, + group_ => null + ); + $s$, + rel_id, + jsonb_build_object( + 'type', 'equal', 'args', jsonb_build_array( + jsonb_build_object('type', 'attnum', 'value', 1), + jsonb_build_object('type', 'literal', 'value', 2) + ) + ) + ), + '42501', + 'permission denied for table atable', + 'Records lister throws permission error when filtering on column without privilege' + ); + + SET ROLE NONE; + CREATE ROLE intern_no_access; + GRANT USAGE ON SCHEMA msar, __msar TO intern_no_access; + GRANT EXECUTE ON ALL FUNCTIONS IN SCHEMA msar, __msar TO intern_no_access; + SET ROLE intern_no_access; + RETURN NEXT throws_ok( + format( + 'SELECT msar.list_records_from_table(%s, null, null, null, null, null);', + rel_id + ), + '42501', + 'permission denied for table atable', + 'Records lister throws permission error' + ); +END; +$$ LANGUAGE plpgsql; From 27eeed33e7a32a5f8f7b60374e0e21e7ae5b716f Mon Sep 17 00:00:00 2001 From: Brent Moran Date: Mon, 4 Nov 2024 17:35:30 +0800 Subject: [PATCH 3/8] add tests for privilege behavior for grouping, summaries --- db/sql/test_00_msar.sql | 102 ++++++++++++++++++++++++++++------------ 1 file changed, 72 insertions(+), 30 deletions(-) diff --git a/db/sql/test_00_msar.sql b/db/sql/test_00_msar.sql index 10698a8dad..6b23168c4c 100644 --- a/db/sql/test_00_msar.sql +++ b/db/sql/test_00_msar.sql @@ -4109,34 +4109,6 @@ END; $$ LANGUAGE plpgsql; -CREATE OR REPLACE FUNCTION test_delete_records_from_table_no_pkey() RETURNS SETOF TEXT AS $$ -DECLARE - rel_id oid; - delete_result integer; -BEGIN - PERFORM __setup_list_records_table(); - rel_id := 'atable'::regclass::oid; - CREATE ROLE intern_no_pkey; - GRANT USAGE ON SCHEMA msar, __msar TO intern_no_pkey; - GRANT EXECUTE ON ALL FUNCTIONS IN SCHEMA msar, __msar TO intern_no_pkey; - GRANT SELECT ON ALL TABLES IN SCHEMA msar, __msar TO INTERN_no_pkey; - GRANT SELECT (col1, col2, col3, col4) ON TABLE atable TO intern_no_pkey; - SET ROLE intern_no_pkey; - RETURN NEXT throws_ok( - format('SELECT msar.delete_records_from_table(%s, ''[2, 3]'')', rel_id), - '42501', - 'permission denied for table atable', - 'Throw error when trying to delete without permission' - ); - SET ROLE NONE; - RETURN NEXT results_eq( - 'SELECT id FROM atable ORDER BY id', - $v$VALUES ('1'::integer), ('2'::integer), ('3'::integer)$v$ - ); -END; -$$ LANGUAGE plpgsql; - - CREATE OR REPLACE FUNCTION test_delete_records_from_table_stringy_pkey() RETURNS SETOF TEXT AS $$ DECLARE rel_id oid; @@ -5537,8 +5509,10 @@ SELECT permissions on some proper subset. */ DECLARE rel_id oid; + jsonb_result jsonb; BEGIN PERFORM __setup_list_records_table(); + PERFORM __setup_preview_fkey_cols(); rel_id := 'atable'::regclass::oid; CREATE ROLE intern_no_pkey; @@ -5630,8 +5604,6 @@ BEGIN ]$j$, 'filtering without specifying column without permissions works' ); - - RETURN NEXT throws_ok( format( $s$SELECT @@ -5656,10 +5628,71 @@ BEGIN 'permission denied for table atable', 'Records lister throws permission error when filtering on column without privilege' ); + RETURN NEXT is( + msar.list_records_from_table( + tab_id => rel_id, + limit_ => null, + offset_ => null, + order_ => '[{"attnum": 3, "direction": "asc"}, {"attnum": 1, "direction": "asc"}]', + filter_ => null, + group_ => '{"columns": [3, 1]}' + ) -> 'grouping', + $j${ + "groups": [ + {"id": 1, "count": 1, "results_eq": {"3": "abcde"}, "result_indices": [0]}, + {"id": 2, "count": 1, "results_eq": {"3": "sdflfflsk"}, "result_indices": [1]}, + {"id": 3, "count": 1, "results_eq": {"3": "sdflkj"}, "result_indices": [2]} + ], + "columns": [3, 1], + "preproc": null + }$j$, + 'ignore group column without permissions, use one with permissions' + ); + RETURN NEXT throws_ok( + format('SELECT msar.delete_records_from_table(%s, ''[2, 3]'')', rel_id), + '42501', + 'permission denied for table atable', + 'Throw error when trying to delete without permission' + ); + + SET ROLE NONE; + CREATE ROLE intern_students_only; + GRANT USAGE ON SCHEMA msar, __msar TO intern_students_only; + GRANT EXECUTE ON ALL FUNCTIONS IN SCHEMA msar, __msar TO intern_students_only; + GRANT SELECT ON ALL TABLES IN SCHEMA msar, __msar TO intern_students_only; + GRANT SELECT ON TABLE "Students" TO intern_students_only; + SET ROLE intern_students_only; + jsonb_result = msar.get_record_from_table( + tab_id => '"Students"'::regclass::oid, + rec_id => 4 + ); + RETURN NEXT is( + jsonb_result -> 'results', + '[{"1": 4, "2": 2.345, "3": 1, "4": "Ida Idalia", "5": 90, "6": "iidalia@example.edu"}]', + 'Record results work when no access to linked table' + ); + RETURN NEXT is( + jsonb_result -> 'linked_record_summaries', + 'null', + 'Record summaries are ignored when no access to linked tables' + ); + RETURN NEXT is( + msar.get_record_from_table( + tab_id => '"Students"'::regclass::oid, + rec_id => 4, + table_record_summary_templates => jsonb_build_object( + '"Teachers"'::regclass::oid, + '[[3], " / ", [2, 2]]'::jsonb + ) + ) -> 'results', + '[{"1": 4, "2": 2.345, "3": 1, "4": "Ida Idalia", "5": 90, "6": "iidalia@example.edu"}]', + 'Record results work when no access to linked table having custom summary' + ); SET ROLE NONE; CREATE ROLE intern_no_access; GRANT USAGE ON SCHEMA msar, __msar TO intern_no_access; + GRANT SELECT ON ALL TABLES IN SCHEMA msar, __msar TO intern_no_access; GRANT EXECUTE ON ALL FUNCTIONS IN SCHEMA msar, __msar TO intern_no_access; SET ROLE intern_no_access; RETURN NEXT throws_ok( @@ -5671,5 +5704,14 @@ BEGIN 'permission denied for table atable', 'Records lister throws permission error' ); + RETURN NEXT throws_ok( + format( + 'SELECT msar.get_record_from_table(%s, 1, true);', + rel_id + ), + '42501', + 'permission denied for table atable', + 'Records getter throws permission error' + ); END; $$ LANGUAGE plpgsql; From 8e51f71c8a21b87c2b422a3a9508c4c13f57bcfd Mon Sep 17 00:00:00 2001 From: Brent Moran Date: Mon, 4 Nov 2024 17:37:02 +0800 Subject: [PATCH 4/8] add test showing weird summary privilege behavior --- db/sql/test_00_msar.sql | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/db/sql/test_00_msar.sql b/db/sql/test_00_msar.sql index 6b23168c4c..fac85eff0b 100644 --- a/db/sql/test_00_msar.sql +++ b/db/sql/test_00_msar.sql @@ -5688,6 +5688,23 @@ BEGIN '[{"1": 4, "2": 2.345, "3": 1, "4": "Ida Idalia", "5": 90, "6": "iidalia@example.edu"}]', 'Record results work when no access to linked table having custom summary' ); + RETURN NEXT throws_ok( + format( + $s$SELECT msar.get_record_from_table( + tab_id => %1$s, + rec_id => 4, + return_record_summaries => true, + table_record_summary_templates => jsonb_build_object( + %1$s, + '[[4], " ", [5], "%% - (", [3, 3], " / ", [3, 2, 2], ")"]'::jsonb + ) + );$s$, + '"Students"'::regclass::oid + ), + '42501', + 'permission denied for table Teachers', + 'Record summary throws permission error when linked table missing privileges -- TODO' + ); SET ROLE NONE; CREATE ROLE intern_no_access; From 9595f616ade0e6b36ee3c0d995513a7caad43f92 Mon Sep 17 00:00:00 2001 From: Brent Moran Date: Mon, 4 Nov 2024 23:38:03 +0800 Subject: [PATCH 5/8] modify table patcher to return correct permission error --- db/sql/00_msar.sql | 7 +++---- db/sql/test_00_msar.sql | 15 ++++++++++++++- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/db/sql/00_msar.sql b/db/sql/00_msar.sql index 718eb71cf0..24d1e83363 100644 --- a/db/sql/00_msar.sql +++ b/db/sql/00_msar.sql @@ -5443,8 +5443,8 @@ DECLARE BEGIN EXECUTE format( $p$ - WITH update_cte AS (%1$s %2$s RETURNING %3$s) - SELECT msar.format_data(%4$I)::text FROM update_cte + WITH update_cte AS (%1$s %2$s RETURNING %3$I) + SELECT * FROM update_cte $p$, msar.build_update_expr(tab_id, rec_def), msar.build_where_clause( @@ -5455,8 +5455,7 @@ BEGIN ) ) ), - msar.build_selectable_column_expr(tab_id), - msar.get_pk_column(tab_id) + msar.get_column_name(tab_id, msar.get_pk_column(tab_id)) ) INTO rec_modified_id; rec_modified := msar.get_record_from_table( tab_id, diff --git a/db/sql/test_00_msar.sql b/db/sql/test_00_msar.sql index fac85eff0b..4e8fcd5cf0 100644 --- a/db/sql/test_00_msar.sql +++ b/db/sql/test_00_msar.sql @@ -5652,7 +5652,20 @@ BEGIN format('SELECT msar.delete_records_from_table(%s, ''[2, 3]'')', rel_id), '42501', 'permission denied for table atable', - 'Throw error when trying to delete without permission' + 'Throw error when trying to delete without SELECT on id' + ); + RETURN NEXT throws_ok( + format( + $s$SELECT msar.patch_record_in_table( + tab_id => %s, + rec_id => 1, + rec_def => '{"2": 10}'::jsonb + );$s$, + rel_id + ), + '42501', + 'permission denied for table atable', + 'Records patcher throws permission error when trying to patch without SELECT on id' ); SET ROLE NONE; From 0abf434be19da53e8cddd8fb6b4cca7ead6ed4c8 Mon Sep 17 00:00:00 2001 From: Brent Moran Date: Tue, 5 Nov 2024 13:35:27 +0800 Subject: [PATCH 6/8] modify record adder to return correct permission error --- db/sql/00_msar.sql | 5 ++--- db/sql/test_00_msar.sql | 16 +++++++++++++++- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/db/sql/00_msar.sql b/db/sql/00_msar.sql index 24d1e83363..fa2acd91ad 100644 --- a/db/sql/00_msar.sql +++ b/db/sql/00_msar.sql @@ -5380,12 +5380,11 @@ BEGIN EXECUTE format( $q$ WITH insert_cte AS (%1$s RETURNING %2$s) - SELECT msar.format_data(%3$I)::text + SELECT * FROM insert_cte $q$, /* %1 */ msar.build_single_insert_expr(tab_id, rec_def), - /* %2 */ msar.build_selectable_column_expr(tab_id), - /* %3 */ msar.get_pk_column(tab_id) + /* %2 */ msar.get_column_name(tab_id, msar.get_pk_column(tab_id)) ) INTO rec_created_id; rec_created := msar.get_record_from_table( tab_id, diff --git a/db/sql/test_00_msar.sql b/db/sql/test_00_msar.sql index 4e8fcd5cf0..cf80a88b2b 100644 --- a/db/sql/test_00_msar.sql +++ b/db/sql/test_00_msar.sql @@ -5665,9 +5665,23 @@ BEGIN ), '42501', 'permission denied for table atable', - 'Records patcher throws permission error when trying to patch without SELECT on id' + 'Records patcher throws permission error when trying to patch without SELECT on pkey' ); + RETURN NEXT throws_ok( + format( + $s$SELECT msar.add_record_to_table( + tab_id => %s, + rec_def => '{"2": 234, "3": "ab234", "4": {"key": "val"}, "5": {"key2": "val2"}}'::jsonb + );$s$, + rel_id + ), + '42501', + 'permission denied for table atable', + 'Record adder throws permission error when adding a record without SELECT on pkey' + ); + + SET ROLE NONE; CREATE ROLE intern_students_only; GRANT USAGE ON SCHEMA msar, __msar TO intern_students_only; From 30747c55afa6d68dad97ec68347d5bafbdb46bb8 Mon Sep 17 00:00:00 2001 From: Brent Moran Date: Tue, 5 Nov 2024 15:14:24 +0800 Subject: [PATCH 7/8] modify record search to return correct permission error --- db/sql/00_msar.sql | 15 ++++++---- db/sql/test_00_msar.sql | 61 ++++++++++++++++++++++++++++++++++++++--- 2 files changed, 66 insertions(+), 10 deletions(-) diff --git a/db/sql/00_msar.sql b/db/sql/00_msar.sql index fa2acd91ad..f5384ab839 100644 --- a/db/sql/00_msar.sql +++ b/db/sql/00_msar.sql @@ -5220,7 +5220,7 @@ BEGIN SELECT count(1) AS count FROM %2$I.%3$I %4$s ), results_cte AS ( - SELECT %1$s FROM %2$I.%3$I %4$s ORDER BY %6$s LIMIT %5$L + SELECT %1$s FROM %2$I.%3$I %4$s %6$s LIMIT %5$L ), summary_cte_self AS (%7$s) %8$s SELECT jsonb_build_object( @@ -5228,19 +5228,22 @@ BEGIN 'count', coalesce(max(count_cte.count), 0), 'linked_record_summaries', %10$s, 'record_summaries', %11$s, - 'query', $iq$SELECT %1$s FROM %2$I.%3$I %4$s ORDER BY %6$s LIMIT %5$L$iq$ + 'query', $iq$SELECT %1$s FROM %2$I.%3$I %4$s %6$s LIMIT %5$L$iq$ ) FROM results_cte %9$s CROSS JOIN count_cte $q$, - /* %1 */ msar.build_selectable_column_expr(tab_id), + /* %1 */ COALESCE(msar.build_selectable_column_expr(tab_id), 'NULL'), /* %2 */ msar.get_relation_schema_name(tab_id), /* %3 */ msar.get_relation_name(tab_id), /* %4 */ 'WHERE ' || msar.get_score_expr(tab_id, search_) || ' > 0', /* %5 */ limit_, - /* %6 */ concat( - msar.get_score_expr(tab_id, search_) || ' DESC, ', - msar.build_total_order_expr(tab_id, null) + /* %6 */ 'ORDER BY ' || NULLIF( + concat( + msar.get_score_expr(tab_id, search_) || ' DESC, ', + msar.build_total_order_expr(tab_id, null) + ), + '' ), /* %7 */ msar.build_record_summary_query_for_table( tab_id, diff --git a/db/sql/test_00_msar.sql b/db/sql/test_00_msar.sql index cf80a88b2b..98e7877c14 100644 --- a/db/sql/test_00_msar.sql +++ b/db/sql/test_00_msar.sql @@ -3828,14 +3828,14 @@ $$ LANGUAGE plpgsql; CREATE OR REPLACE FUNCTION __setup_search_records_table() RETURNS SETOF TEXT AS $$ BEGIN - CREATE TABLE atable ( + CREATE TABLE search_table ( id integer PRIMARY KEY GENERATED ALWAYS AS IDENTITY, col1 integer, col2 varchar, coltodrop integer ); - ALTER TABLE atable DROP COLUMN coltodrop; - INSERT INTO atable (col1, col2) VALUES + ALTER TABLE search_table DROP COLUMN coltodrop; + INSERT INTO search_table (col1, col2) VALUES (1, 'bcdea'), (12, 'vwxyz'), (1, 'edcba'), @@ -3850,7 +3850,7 @@ DECLARE search_result jsonb; BEGIN PERFORM __setup_search_records_table(); - rel_id := 'atable'::regclass::oid; + rel_id := 'search_table'::regclass::oid; search_result := msar.search_records_from_table( rel_id, jsonb_build_array( @@ -5513,6 +5513,7 @@ DECLARE BEGIN PERFORM __setup_list_records_table(); PERFORM __setup_preview_fkey_cols(); + PERFORM __setup_search_records_table(); rel_id := 'atable'::regclass::oid; CREATE ROLE intern_no_pkey; @@ -5520,6 +5521,7 @@ BEGIN GRANT EXECUTE ON ALL FUNCTIONS IN SCHEMA msar, __msar TO intern_no_pkey; GRANT SELECT ON ALL TABLES IN SCHEMA msar, __msar TO intern_no_pkey; GRANT SELECT (col1, col2, col3, col4) ON TABLE atable TO intern_no_pkey; + GRANT SELECT (col1, col2) ON TABLE search_table TO intern_no_pkey; SET ROLE intern_no_pkey; RETURN NEXT is( msar.list_records_from_table( @@ -5648,12 +5650,44 @@ BEGIN }$j$, 'ignore group column without permissions, use one with permissions' ); + + RETURN NEXT is( + msar.search_records_from_table( + 'search_table'::regclass::oid, + jsonb_build_array( + jsonb_build_object('attnum', 3, 'literal', 'bc') + ), + null + ) -> 'results', + jsonb_build_array( + jsonb_build_object('2', 1, '3', 'bcdea'), + jsonb_build_object('2', 2, '3', 'abcde') + ), + 'search ignores unspecified columns without permissions' + ); + RETURN NEXT is( + msar.search_records_from_table( + 'search_table'::regclass::oid, + jsonb_build_array( + jsonb_build_object('attnum', 1, 'literal', 2), + jsonb_build_object('attnum', 3, 'literal', 'bc') + ), + null + ) -> 'results', + jsonb_build_array( + jsonb_build_object('2', 1, '3', 'bcdea'), + jsonb_build_object('2', 2, '3', 'abcde') + ), + 'search ignores specified columns without permissions, uses other' + ); + RETURN NEXT throws_ok( format('SELECT msar.delete_records_from_table(%s, ''[2, 3]'')', rel_id), '42501', 'permission denied for table atable', 'Throw error when trying to delete without SELECT on id' ); + RETURN NEXT throws_ok( format( $s$SELECT msar.patch_record_in_table( @@ -5757,5 +5791,24 @@ BEGIN 'permission denied for table atable', 'Records getter throws permission error' ); + + RETURN NEXT throws_ok( + format( + 'SELECT msar.search_records_from_table(%s, ''[{"attnum": 3, "literal": "bc"}]'', null);', + 'search_table'::regclass::oid + ), + '42501', + 'permission denied for table search_table', + 'Records search throws permission error with nonempty search terms' + ); + RETURN NEXT throws_ok( + format( + 'SELECT msar.search_records_from_table(%s, ''[]'', null);', + 'search_table'::regclass::oid + ), + '42501', + 'permission denied for table search_table', + 'Records search throws permission error with empty search terms' + ); END; $$ LANGUAGE plpgsql; From 7b29fe3efa91e38ea44e2c5343d016297c2f3bfb Mon Sep 17 00:00:00 2001 From: Brent Moran Date: Wed, 6 Nov 2024 16:58:25 +0800 Subject: [PATCH 8/8] remove defunct SQL test after merge --- db/sql/test_00_msar.sql | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/db/sql/test_00_msar.sql b/db/sql/test_00_msar.sql index 811ec5acbc..2287d39fad 100644 --- a/db/sql/test_00_msar.sql +++ b/db/sql/test_00_msar.sql @@ -5861,23 +5861,6 @@ BEGIN '[{"1": 4, "2": 2.345, "3": 1, "4": "Ida Idalia", "5": 90, "6": "iidalia@example.edu"}]', 'Record results work when no access to linked table having custom summary' ); - RETURN NEXT throws_ok( - format( - $s$SELECT msar.get_record_from_table( - tab_id => %1$s, - rec_id => 4, - return_record_summaries => true, - table_record_summary_templates => jsonb_build_object( - %1$s, - '[[4], " ", [5], "%% - (", [3, 3], " / ", [3, 2, 2], ")"]'::jsonb - ) - );$s$, - '"Students"'::regclass::oid - ), - '42501', - 'permission denied for table Teachers', - 'Record summary throws permission error when linked table missing privileges -- TODO' - ); SET ROLE NONE; CREATE ROLE intern_no_access;