Skip to content

Commit a431c10

Browse files
staaldraadsamrosesoedirgo
authored
fix: search path and migration grants (#1939)
* fix: search path and migration grants search_path not set on pgbouncer.get_auth and later migrations don't apply permissions correctly. * tests: update pgbouncer tests * fix: update files used for schema tests * chore: revoke from postgres role * chore: update schema dumps * chore: bump versions * chore: bump versions --------- Co-authored-by: Sam Rose <samuel@supabase.io> Co-authored-by: Bobbie Soedirgo <bobbie@soedirgo.dev> Co-authored-by: Bobbie Soedirgo <31685197+soedirgo@users.noreply.github.com>
1 parent 2bf0696 commit a431c10

File tree

8 files changed

+61
-6
lines changed

8 files changed

+61
-6
lines changed

ansible/files/pgbouncer_config/pgbouncer_auth_schema.sql

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,9 @@ BEGIN
1414
SELECT usename::TEXT, passwd::TEXT FROM pg_catalog.pg_shadow
1515
WHERE usename = p_usename;
1616
END;
17-
$$ LANGUAGE plpgsql SECURITY DEFINER;
17+
$$ LANGUAGE plpgsql
18+
SET search_path = ''
19+
SECURITY DEFINER;
1820

1921
REVOKE ALL ON FUNCTION pgbouncer.get_auth(p_usename TEXT) FROM PUBLIC;
2022
GRANT EXECUTE ON FUNCTION pgbouncer.get_auth(p_usename TEXT) TO pgbouncer;

ansible/vars.yml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,9 @@ postgres_major:
1010

1111
# Full version strings for each major version
1212
postgres_release:
13-
postgresorioledb-17: "17.6.0.019-orioledb"
14-
postgres17: "17.6.1.062"
15-
postgres15: "15.14.1.062"
13+
postgresorioledb-17: "17.6.0.020-orioledb"
14+
postgres17: "17.6.1.063"
15+
postgres15: "15.14.1.063"
1616

1717
# Non Postgres Extensions
1818
pgbouncer_release: 1.19.0
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
-- migrate:up
2+
3+
create or replace function pgbouncer.get_auth(p_usename text) returns table (username text, password text)
4+
language plpgsql
5+
set search_path = ''
6+
security definer
7+
as $$
8+
begin
9+
raise debug 'PgBouncer auth request: %', p_usename;
10+
11+
return query
12+
select
13+
rolname::text,
14+
case when rolvaliduntil < now()
15+
then null
16+
else rolpassword::text
17+
end
18+
from pg_authid
19+
where rolname=$1 and rolcanlogin;
20+
end;
21+
$$;
22+
23+
revoke all on function pgbouncer.get_auth(text) from public;
24+
revoke execute on function pgbouncer.get_auth(text) from postgres;
25+
grant execute on function pgbouncer.get_auth(text) to pgbouncer;
26+
-- migrate:down
27+

migrations/schema-15.sql

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -478,6 +478,7 @@ COMMENT ON FUNCTION extensions.set_graphql_placeholder() IS 'Reintroduces placeh
478478

479479
CREATE FUNCTION pgbouncer.get_auth(p_usename text) RETURNS TABLE(username text, password text)
480480
LANGUAGE plpgsql SECURITY DEFINER
481+
SET search_path TO ''
481482
AS $_$
482483
begin
483484
raise debug 'PgBouncer auth request: %', p_usename;

migrations/schema-17.sql

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -479,6 +479,7 @@ COMMENT ON FUNCTION extensions.set_graphql_placeholder() IS 'Reintroduces placeh
479479

480480
CREATE FUNCTION pgbouncer.get_auth(p_usename text) RETURNS TABLE(username text, password text)
481481
LANGUAGE plpgsql SECURITY DEFINER
482+
SET search_path TO ''
482483
AS $_$
483484
begin
484485
raise debug 'PgBouncer auth request: %', p_usename;

migrations/schema-orioledb-17.sql

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -493,6 +493,7 @@ COMMENT ON FUNCTION extensions.set_graphql_placeholder() IS 'Reintroduces placeh
493493

494494
CREATE FUNCTION pgbouncer.get_auth(p_usename text) RETURNS TABLE(username text, password text)
495495
LANGUAGE plpgsql SECURITY DEFINER
496+
SET search_path TO ''
496497
AS $_$
497498
begin
498499
raise debug 'PgBouncer auth request: %', p_usename;

nix/tests/expected/pgbouncer.out

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,9 +62,8 @@ ORDER BY object_name, grantee, privilege_type;
6262
schema | object_name | grantee | privilege_type
6363
-----------+-------------+----------------+----------------
6464
pgbouncer | get_auth | pgbouncer | EXECUTE
65-
pgbouncer | get_auth | postgres | EXECUTE
6665
pgbouncer | get_auth | supabase_admin | EXECUTE
67-
(3 rows)
66+
(2 rows)
6867

6968
-- Ensure that pgbouncer.get_auth() function does not return an expired password
7069
create role test_expired_user_password with login password 'expired_password' valid until '2000-01-01 00:00:00+00';
@@ -83,5 +82,19 @@ select pgbouncer.get_auth('test_valid_user_password');
8382
(test_valid_user_password,SCRAM-SHA-256$4096:testsaltbase64$storedkeybase64$serverkeybase64)
8483
(1 row)
8584

85+
-- Test pgbouncer.get_auth is executable by the pgbouncer user
86+
set role pgbouncer;
87+
select pgbouncer.get_auth('test_valid_user_password');
88+
get_auth
89+
----------------------------------------------------------------------------------------------
90+
(test_valid_user_password,SCRAM-SHA-256$4096:testsaltbase64$storedkeybase64$serverkeybase64)
91+
(1 row)
92+
93+
reset role;
94+
-- and not other non-superusers
95+
set role postgres;
96+
select pgbouncer.get_auth('test_valid_user_password');
97+
ERROR: permission denied for function get_auth
98+
reset role;
8699
drop role test_expired_user_password;
87100
drop role test_valid_user_password;

nix/tests/sql/pgbouncer.sql

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,5 +62,15 @@ select pgbouncer.get_auth('test_expired_user_password');
6262

6363
select pgbouncer.get_auth('test_valid_user_password');
6464

65+
-- Test pgbouncer.get_auth is executable by the pgbouncer user
66+
set role pgbouncer;
67+
select pgbouncer.get_auth('test_valid_user_password');
68+
reset role;
69+
70+
-- and not other non-superusers
71+
set role postgres;
72+
select pgbouncer.get_auth('test_valid_user_password');
73+
reset role;
74+
6575
drop role test_expired_user_password;
6676
drop role test_valid_user_password;

0 commit comments

Comments
 (0)