From 227bb54853ee04afca979fe8858d8851032fa072 Mon Sep 17 00:00:00 2001 From: Joshen Lim Date: Wed, 14 May 2025 18:02:07 +0800 Subject: [PATCH] Improve auth policies RLS warnings granularity (#35579) --- .../Auth/Policies/PolicyTableRow/index.tsx | 66 +++++++++++++---- apps/studio/data/tables/keys.ts | 6 ++ .../data/tables/table-roles-access-query.ts | 70 +++++++++++++++++++ 3 files changed, 129 insertions(+), 13 deletions(-) create mode 100644 apps/studio/data/tables/table-roles-access-query.ts diff --git a/apps/studio/components/interfaces/Auth/Policies/PolicyTableRow/index.tsx b/apps/studio/components/interfaces/Auth/Policies/PolicyTableRow/index.tsx index 38a369f6275..4b115b87f96 100644 --- a/apps/studio/components/interfaces/Auth/Policies/PolicyTableRow/index.tsx +++ b/apps/studio/components/interfaces/Auth/Policies/PolicyTableRow/index.tsx @@ -5,7 +5,9 @@ import { Info } from 'lucide-react' import { useProjectContext } from 'components/layouts/ProjectLayout/ProjectContext' import AlertError from 'components/ui/AlertError' import Panel from 'components/ui/Panel' +import { useProjectPostgrestConfigQuery } from 'data/config/project-postgrest-config-query' import { useDatabasePoliciesQuery } from 'data/database-policies/database-policies-query' +import { useTableRolesAccessQuery } from 'data/tables/table-roles-access-query' import { cn, Tooltip, TooltipContent, TooltipTrigger } from 'ui' import ShimmeringLoader from 'ui-patterns/ShimmeringLoader' import PolicyRow from './PolicyRow' @@ -39,6 +41,30 @@ export const PolicyTableRow = ({ onSelectDeletePolicy = noop, }: PolicyTableRowProps) => { const { project } = useProjectContext() + + // [Joshen] Changes here are so that warnings are more accurate and granular instead of purely relying if RLS is disabled or enabled + // The following scenarios are technically okay if the table has RLS disabled, in which it won't be publicly readable / writable + // - If the schema is not exposed through the API via Postgrest + // - If the anon and authenticated roles do not have access to the table + // Ideally we should just rely on the security lints as the source of truth, but the security lints currently have limitations + // - They only consider the public schema + // - They do not consider roles + // Eventually if the security lints are able to cover those, we can look to using them as the source of truth instead then + const { data: config } = useProjectPostgrestConfigQuery({ projectRef: project?.ref }) + const exposedSchemas = config?.db_schema ? config?.db_schema.replace(/ /g, '').split(',') : [] + const isRLSEnabled = table.rls_enabled + const isTableExposedThroughAPI = exposedSchemas.includes(table.schema) + + const { data: roles = [] } = useTableRolesAccessQuery({ + projectRef: project?.ref, + connectionString: project?.connectionString, + schema: table.schema, + table: table.name, + }) + const hasAnonAuthenticatedRolesAccess = roles.length !== 0 + const isPubliclyReadableWritable = + !isRLSEnabled && isTableExposedThroughAPI && hasAnonAuthenticatedRolesAccess + const { data, error, isLoading, isError, isSuccess } = useDatabasePoliciesQuery({ projectRef: project?.ref, connectionString: project?.connectionString, @@ -46,6 +72,7 @@ export const PolicyTableRow = ({ const policies = (data ?? []) .filter((policy) => policy.schema === table.schema && policy.table === table.name) .sort((a, b) => a.name.localeCompare(b.name)) + const rlsEnabledNoPolicies = isRLSEnabled && policies.length === 0 return ( } > - {!table.rls_enabled && !isLocked && ( + {(isPubliclyReadableWritable || rlsEnabledNoPolicies) && (
-
- Warning:{' '} +
+ + {isPubliclyReadableWritable ? 'Warning' : 'Note'}: + {' '} - Row Level Security is disabled. Your table is publicly readable and writable. + {isPubliclyReadableWritable + ? 'Row Level Security is disabled. Your table is publicly readable and writable.' + : 'Row Level Security is enabled, but no policies exist. No data will be selectable via Supabase APIs.'} - - - - - - Anyone with the project's anonymous key can modify or delete your data. Enable RLS and - create access policies to keep your data secure. - - + {isPubliclyReadableWritable && ( + + + + + + Anyone with the project's anonymous key can modify or delete your data. Enable RLS + and create access policies to keep your data secure. + + + )}
)} {isLoading && ( diff --git a/apps/studio/data/tables/keys.ts b/apps/studio/data/tables/keys.ts index 688655a6078..2d5ccc776a1 100644 --- a/apps/studio/data/tables/keys.ts +++ b/apps/studio/data/tables/keys.ts @@ -1,4 +1,10 @@ export const tableKeys = { list: (projectRef: string | undefined, schema?: string, includeColumns?: boolean) => ['projects', projectRef, 'tables', schema, includeColumns].filter(Boolean), + rolesAccess: (projectRef: string | undefined, schema: string, table: string) => [ + 'projects', + projectRef, + 'roles-access', + { schema, table }, + ], } diff --git a/apps/studio/data/tables/table-roles-access-query.ts b/apps/studio/data/tables/table-roles-access-query.ts new file mode 100644 index 00000000000..d47f2a04877 --- /dev/null +++ b/apps/studio/data/tables/table-roles-access-query.ts @@ -0,0 +1,70 @@ +import { useQuery, UseQueryOptions } from '@tanstack/react-query' +import { executeSql, ExecuteSqlError } from '../sql/execute-sql-query' +import { tableKeys } from './keys' + +type TableRolesAccessArgs = { + schema: string + table: string +} + +/** + * [Joshen] Specifically just checking for anon and authenticated roles since this is + * just to verify if the table is exposed via the Supabase API + */ +export const getTableRolesAccessSql = ({ schema, table }: TableRolesAccessArgs) => { + const sql = /* SQL */ ` +SELECT grantee, privilege_type +FROM information_schema.role_table_grants +WHERE table_schema = '${schema}' + AND table_name = '${table}' + AND grantee IN ('anon', 'authenticated'); +`.trim() + + return sql +} + +export type TableRolesAccessVariables = TableRolesAccessArgs & { + projectRef?: string + connectionString?: string | null +} + +export async function getTableRolesAccess( + { schema, table, projectRef, connectionString }: TableRolesAccessVariables, + signal?: AbortSignal +) { + if (!schema) { + throw new Error('schema is required') + } + + const sql = getTableRolesAccessSql({ schema, table }) + + const { result } = (await executeSql( + { projectRef, connectionString, sql, queryKey: ['TableRolesAccess', schema] }, + signal + )) as { result: { grantee: string; privilege_type: string }[] } + + const res = [] + if (result.some((x) => x.grantee === 'anon')) res.push('anon') + if (result.some((x) => x.grantee === 'authenticated')) res.push('authenticated') + + return res +} + +export type TableRolesAccessData = Awaited> +export type TableRolesAccessError = ExecuteSqlError + +export const useTableRolesAccessQuery = ( + { projectRef, connectionString, schema, table }: TableRolesAccessVariables, + { + enabled = true, + ...options + }: UseQueryOptions = {} +) => + useQuery( + tableKeys.rolesAccess(projectRef, schema, table), + ({ signal }) => getTableRolesAccess({ projectRef, connectionString, schema, table }, signal), + { + enabled: enabled && typeof projectRef !== 'undefined' && typeof schema !== 'undefined', + ...options, + } + )