plug type system hole where functions returning views can cause inval…

…id refs
supabase · Jan 3, 2024 · 7d3b205 · 7d3b205
1 parent 2bb35d5
commit 7d3b205
Show file tree

Hide file tree

Showing 5 changed files with 203 additions and 28 deletions.
diff --git a/src/graphql.rs b/src/graphql.rs
@@ -1289,9 +1289,11 @@ fn function_fields(schema: &Arc<__Schema>, volatilities: &[FunctionVolatility])
 
                     // If the return type is a table type, it must be selectable
                     if !match &return_type {
-                        __Type::Node(table_type) => table_type.table.permissions.is_selectable,
+                        __Type::Node(table_type) => {
+                            schema.graphql_table_select_types_are_valid(&table_type.table)
+                        }
                         __Type::Connection(table_type) => {
-                            table_type.table.permissions.is_selectable
+                            schema.graphql_table_select_types_are_valid(&table_type.table)
                         }
                         _ => true,
                     } {

diff --git a/test/expected/function_calls.out b/test/expected/function_calls.out
@@ -2601,7 +2601,7 @@ begin;
 
     rollback to savepoint a;
     create table account(
-        id int,
+        id int primary key,
         email varchar(255),
         name text null
     );
@@ -2620,8 +2620,8 @@ begin;
     insert into account(id, email, name)
     values
         (1, 'aardvark@x.com', 'aardvark'),--all columns non-null
-        (2, 'bat@x.com', null),--mixed: some null, some non-null
-        (null, null, null);--all columns null
+        (2, 'bat@x.com', null);--mixed: some null, some non-null
+        --(null, null, null);--all columns null
     -- comment on table account is e'@graphql({"totalCount": {"enabled": true}})';
     select jsonb_pretty(graphql.resolve($$
         query {

diff --git a/test/expected/function_return_view_has_pkey.out b/test/expected/function_return_view_has_pkey.out
@@ -0,0 +1,122 @@
+begin;
+    create view account as
+    select
+      1 as foo,
+      2 as bar;
+    create function returns_account()
+        returns account language sql stable
+    as $$ select foo, bar from account; $$;
+    -- Account should not be visible because the view has no primary key
+    select jsonb_pretty(
+        graphql.resolve($$
+        {
+          __type(name: "Account") {
+            __typename
+          }
+        }
+        $$)
+    );
+      jsonb_pretty      
+------------------------
+ {                     +
+     "data": {         +
+         "__type": null+
+     }                 +
+ }
+(1 row)
+
+    -- returnsAccount should also not be visible because account has no primary key
+    select jsonb_pretty(
+        graphql.resolve($$
+            query IntrospectionQuery {
+              __schema {
+                queryType {
+                  fields {
+                    name
+                  }
+                }
+              }
+            }
+        $$)
+    );
+              jsonb_pretty              
+----------------------------------------
+ {                                     +
+     "data": {                         +
+         "__schema": {                 +
+             "queryType": {            +
+                 "fields": [           +
+                     {                 +
+                         "name": "node"+
+                     }                 +
+                 ]                     +
+             }                         +
+         }                             +
+     }                                 +
+ }
+(1 row)
+
+    comment on view account is e'
+    @graphql({
+        "primary_key_columns": ["foo"]
+    })';
+    -- Account should be visible because the view is selectable and has a primary key
+    select jsonb_pretty(
+        graphql.resolve($$
+        {
+          __type(name: "Account") {
+            __typename
+          }
+        }
+        $$)
+    );
+            jsonb_pretty             
+-------------------------------------
+ {                                  +
+     "data": {                      +
+         "__type": {                +
+             "__typename": "Account"+
+         }                          +
+     }                              +
+ }
+(1 row)
+
+    -- returnsAccount should also be visible because account has a primary key and is selectable
+    select jsonb_pretty(
+        graphql.resolve($$
+            query IntrospectionQuery {
+              __schema {
+                queryType {
+                  fields {
+                    name
+                  }
+                }
+              }
+            }
+        $$)
+    );
+                    jsonb_pretty                     
+-----------------------------------------------------
+ {                                                  +
+     "data": {                                      +
+         "__schema": {                              +
+             "queryType": {                         +
+                 "fields": [                        +
+                     {                              +
+                         "name": "accountCollection"+
+                     },                             +
+                     {                              +
+                         "name": "node"             +
+                     },                             +
+                     {                              +
+                         "name": "returnsAccount"   +
+                     }                              +
+                 ]                                  +
+             }                                      +
+         }                                          +
+     }                                              +
+ }
+(1 row)
+
+
+rollback;
diff --git a/test/sql/function_calls.sql b/test/sql/function_calls.sql
@@ -807,7 +807,7 @@ begin;
     rollback to savepoint a;
 
     create table account(
-        id int,
+        id int primary key,
         email varchar(255),
         name text null
     );
@@ -820,21 +820,15 @@ begin;
         returns account language sql stable
     as $$ select id, email, name from account where id = 2; $$;
 
-    create function returns_all_columns_null_account()
-        returns account language sql stable
-    as $$ select id, email, name from account where id is null; $$;
-
     create function returns_null_account()
         returns account language sql stable
     as $$ select id, email, name from account where id = 9; $$;
 
     insert into account(id, email, name)
     values
         (1, 'aardvark@x.com', 'aardvark'),--all columns non-null
-        (2, 'bat@x.com', null),--mixed: some null, some non-null
-        (null, null, null);--all columns null
+        (2, 'bat@x.com', null);--mixed: some null, some non-null
 
-    -- comment on table account is e'@graphql({"totalCount": {"enabled": true}})';
 
     select jsonb_pretty(graphql.resolve($$
         query {
@@ -858,21 +852,7 @@ begin;
         }
     $$));
 
-    -- With current implementation we can't distinguish between
-    -- when all columns of a composite type are null and when
-    -- the composite type itself is null. In both these cases
-    -- the result will be null for the top-level field.
-    select jsonb_pretty(graphql.resolve($$
-        query {
-            returnsAllColumnsNullAccount {
-                id
-                email
-                name
-                __typename
-            }
-        }
-    $$));
-
+    -- When no record is found, the top level field becomes null
     select jsonb_pretty(graphql.resolve($$
         query {
             returnsNullAccount {

diff --git a/test/sql/function_return_view_has_pkey.sql b/test/sql/function_return_view_has_pkey.sql
@@ -0,0 +1,71 @@
+begin;
+
+    create view account as
+    select
+      1 as foo,
+      2 as bar;
+
+    create function returns_account()
+        returns account language sql stable
+    as $$ select foo, bar from account; $$;
+
+    -- Account should not be visible because the view has no primary key
+    select jsonb_pretty(
+        graphql.resolve($$
+        {
+          __type(name: "Account") {
+            __typename
+          }
+        }
+        $$)
+    );
+
+    -- returnsAccount should also not be visible because account has no primary key
+    select jsonb_pretty(
+        graphql.resolve($$
+            query IntrospectionQuery {
+              __schema {
+                queryType {
+                  fields {
+                    name
+                  }
+                }
+              }
+            }
+        $$)
+    );
+
+    comment on view account is e'
+    @graphql({
+        "primary_key_columns": ["foo"]
+    })';
+
+    -- Account should be visible because the view is selectable and has a primary key
+    select jsonb_pretty(
+        graphql.resolve($$
+        {
+          __type(name: "Account") {
+            __typename
+          }
+        }
+        $$)
+    );
+
+    -- returnsAccount should also be visible because account has a primary key and is selectable
+    select jsonb_pretty(
+        graphql.resolve($$
+            query IntrospectionQuery {
+              __schema {
+                queryType {
+                  fields {
+                    name
+                  }
+                }
+              }
+            }
+        $$)
+    );
+
+
+
+rollback;