usebasejump / supabase-test-helpers

Test helpers for pgTAP and Supabase
https://usebasejump.com/blog/testing-on-supabase-with-pgtap
MIT License
98 stars 12 forks source link

tests.rls_enabled(schema_name) fails on views #16

Open inslayn opened 10 months ago

inslayn commented 10 months ago

I have a public schema that has only views in it (code and log output is an illustration created from the actual code involved).

create view public.public_view as select * from private.private_table;
create view public.secure_view with (security_barrier = true) as select * from private.private_table;

If I use the following:

select plan(1);
select tests.rls_enabled('public');
select * from finish();
rollback;

It fails with output similar to the following:

supabase test db
supabase/tests/20231107180900-app-schema.test.sql ...................... 
# Failed test 1: "All tables in thepublic schema should have row level security enabled"
#         have: 2
#         want: 0
# Looks like you failed 1 test of 1
Failed 1/1 subtests 

Now, since they are views, I can't enable row level security, so I tried to set security_barrier = true on the views which didn't work either.

Looking at the function code, there is no special handling for views, so I added the following after creating the extension at the start of the test plan: Key parts are:

  1. only applying the row security check if the subject is not a view.
  2. enforcing that views in the test schema have security_barrier enabled.
    
    CREATE OR REPLACE FUNCTION tests.rls_enabled (testing_schema text)
    RETURNS text AS $$
    select is(
        coalesce((select count(pc.relname)::integer
            from pg_class pc
                join pg_namespace pn on pn.oid = pc.relnamespace and pn.nspname = rls_enabled.testing_schema
                join pg_type pt on pt.oid = pc.reltype
            where (pc.relkind != 'v' and pc.relrowsecurity = false) 
            or (pc.relkind = 'v' and (reloptions is null or 'security_barrier=true' != any(reloptions)))
            group by pc.relname, reloptions), 0) 
        ,
        0,
        'All tables in the ' || testing_schema || ' schema should have row level security enabled');
    $$ LANGUAGE sql;

CREATE OR REPLACE FUNCTION tests.rls_enabled (testing_schema text, testing_table text) RETURNS TEXT AS $$ select is( coalesce((select count(*)::integer from pg_class pc join pg_namespace pn on pn.oid = pc.relnamespace and pn.nspname = rls_enabled.testing_schema and pc.relname = rls_enabled.testing_table join pg_type pt on pt.oid = pc.reltype where (pc.relkind != 'v' and pc.relrowsecurity = true) or (pc.relkind = 'v' and 'security_barrier=true' = any(reloptions)) group by pc.relname, reloptions), 0), 1, testing_table || 'table in the' || testing_schema || ' schema should have row level security enabled' ); $$ LANGUAGE sql;



I honestly don't know the implications of this change, which is why I didn't make a PR instead. I would have thought someone must have come across this before, though it's more likely that I'm missing something 😅  
tiniscule commented 9 months ago

Thanks for raising this - I've been doing a bit of research to determine best practices to make sure we test for that here.

From here, it looks like the recommended way to handle this is to use the new security_invoker option in postgres 15, but I'm wondering if we actually want to encourage both. https://github.com/orgs/supabase/discussions/1501

Goal here is to check all tables/views for the recommended best practice, so you're definitely on the right path by checking views explicitly. I'll do a bit more digging and see what's recommended here.

MaximusMcCann commented 7 months ago

Another option: temporarily drop all the views.


BEGIN TRANSACTION;

  CREATE EXTENSION "basejump-supabase_test_helpers";

  DO
  $$
  DECLARE
      row record;
  BEGIN

      FOR row IN SELECT viewname FROM pg_views AS t WHERE t.schemaname = 'public'
      LOOP
          EXECUTE format('DROP VIEW IF EXISTS public.%I CASCADE;', row.viewname);
      END LOOP;

  END;
  $$;

  SELECT plan(1);
  SELECT tests.rls_enabled('public');
  SELECT * FROM finish();

ROLLBACK TRANSACTION;
pghalliday commented 4 weeks ago

Not sure what the status of this is but I have been using the drop views workaround above and the following function to check if views have security_invoker enabled

-- test that security_invoker has been enabled for all views in the given schema
create function app_tests.security_invoker_enabled_on_views(p_schema text)
returns text
language sql
as $_func_$
  select is_empty(
    format($$
      select
          relname
        from pg_class
        join pg_catalog.pg_namespace n on n.oid = pg_class.relnamespace
        where n.nspname = %L -- filter on the schema
          and relkind='v' -- only select views
          and (
            -- if reloptions is null then the array check does
            -- not work as you might expect :s (I think it might resolve
            -- to null and then not null becomes false or something)
            reloptions is null or
            not (
              -- lower case the options text and extract array, then check if any
              -- elements match elements in our array of possibilities for the
              -- security_invoker option being enabled
              lower(reloptions::text)::text[] &&
              array['security_invoker=1','security_invoker=true','security_invoker=on']
            )
          )
    $$, p_schema),
    format('The security_invoker option should be enabled for all views in the %L schema', p_schema)
  );
$_func_$;

it prints the list of views without security_invoker on failure and is adapted from this answer from stack overflow: https://stackoverflow.com/a/75909368

elyobo commented 2 weeks ago

Keeping this as the same issue rather than opening another, but this rls_enabled also fails on composite types (relkind = c)

My workaround is to copy the rls_enabled code from the repo and amend it to exclude composite types.

-- All tables must have RLS enabled
-- Fails to handle composite types, see
-- https://github.com/usebasejump/supabase-test-helpers/issues/16
-- select tests.rls_enabled('public');
select
    is (
        (
            select count(pc.relname)::integer
            from pg_class pc
            join pg_namespace pn on pn.oid = pc.relnamespace and pn.nspname = 'public'
            join pg_type pt on pt.oid = pc.reltype
            where not relrowsecurity and pc.relkind != 'c'
        ),
        0,
        'All tables in the public schema should have row level security enabled'
    )
;