8000 fix: Fix logic when returning default roles by ruggi99 · Pull Request #490 · supabase/postgres-meta · GitHub
[go: up one dir, main page]
Skip to content

fix: Fix logic when returning default roles #490

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jan 30, 2023
Merged

fix: Fix logic when returning default roles #490

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5c2fa90
Bug fix returning default roles
ruggi99 Jan 28, 2023
fc59610
refactor: replace DEFAULT_ROLES w/ prefix test
soedirgo Jan 30, 2023
d5ae678
test: includeDefaultRoles
soedirgo Jan 30, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions src/lib/PostgresMetaRoles.ts
View file
Open in desktop
8000
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { ident, literal } from 'pg-format'
import { DEFAULT_ROLES } from './constants.js'
import { rolesSql } from './sql/index.js'
import {
PostgresMetaResult,
Expand Down Expand Up @@ -42,8 +41,16 @@ FROM
roles
WHERE
true`
if (includeDefaultRoles) {
sql += ` AND name NOT IN (${DEFAULT_ROLES.map(literal).join(',')})`
if (!includeDefaultRoles) {
// All default/predefined roles start with pg_: https://www.postgresql.org/docs/15/predefined-roles.html
// The pg_ prefix is also reserved:
//
// ```
// postgres=# create role pg_mytmp;
// ERROR: role name "pg_mytmp" is reserved
// DETAIL: Role names starting with "pg_" are reserved.
// ```
sql += ` AND NOT pg_catalog.starts_with(name, 'pg_')`
}
if (limit) {
sql += ` LIMIT ${limit}`
Expand Down
11 changes: 0 additions & 11 deletions src/lib/constants.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,3 @@
export const DEFAULT_ROLES = [
'pg_execute_server_program',
'pg_monitor',
'pg_read_all_settings',
'pg_read_all_stats',
'pg_read_server_files',
'pg_signal_backend',
'pg_stat_scan_tables',
'pg_write_server_files',
]

export const DEFAULT_SYSTEM_SCHEMAS = [
'information_schema',
'pg_catalog',
Expand Down
39 changes: 38 additions & 1 deletion test/lib/roles.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { pgMeta } from './utils'
test('list', async () => {
const res = await pgMeta.roles.list()

const role: any = res.data?.find(({ name }) => name === 'postgres')
let role = res.data?.find(({ name }) => name === 'postgres')

expect(role).toMatchInlineSnapshot(
{ active_connections: expect.any(Number), id: expect.any(Number) },
Expand All @@ -26,6 +26,43 @@ test('list', async () => {
}
`
)

// pg_monitor is a predefined role. `includeDefaultRoles` defaults to false,
// so it shouldn't be included in the result.
role = res.data?.find(({ name }) => name === 'pg_monitor')

expect(role).toMatchInlineSnapshot(`undefined`)
})

test('list w/ default roles', async () => {
const res = await pgMeta.roles.list({ includeDefaultRoles: true })

const role = res.data?.find(({ name }) => name === 'pg_monitor')

expect(role).toMatchInlineSnapshot(
{
active_connections: expect.any(Number),
id: expect.any(Number),
},
`
{
"active_connections": Any<Number>,
"can_bypass_rls": false,
"can_create_db": false,
"can_create_role": false,
"can_login": false,
"config": null,
"connection_limit": 100,
"id": Any<Number>,
"inherit_role": true,
"is_replication_role": false,
"is_superuser": false,
"name": "pg_monitor",
"password": "********",
"valid_until": null,
}
`
)
})

test('retrieve, create, update, delete', async () => {
Expand Down
0