diff --git a/apps/docs/content/guides/api/custom-claims-and-role-based-access-control-rbac.mdx b/apps/docs/content/guides/api/custom-claims-and-role-based-access-control-rbac.mdx index cea9bc511670d..d1cf1d7e794e8 100644 --- a/apps/docs/content/guides/api/custom-claims-and-role-based-access-control-rbac.mdx +++ b/apps/docs/content/guides/api/custom-claims-and-role-based-access-control-rbac.mdx @@ -181,7 +181,7 @@ $$ language plpgsql stable security definer set search_path = ''; -You can read more about using functions in RLS policies in the [RLS guide](/docs/guides/database/postgres/row-level-security#using-functions). +You can read more about using functions in RLS policies in the [RLS guide](/docs/guides/database/postgres/row-level-security#use-security-definer-functions). @@ -219,5 +219,5 @@ You now have a robust system in place to manage user roles and permissions withi - [Auth Hooks](/docs/guides/auth/auth-hooks) - [Row Level Security](/docs/guides/database/postgres/row-level-security) -- [RLS Functions](/docs/guides/database/postgres/row-level-security#using-functions) +- [RLS helper functions](/docs/guides/database/postgres/row-level-security#helper-functions) - [Next.js Slack Clone Example](https://github.com/supabase/supabase/tree/master/examples/slack-clone/nextjs-slack-clone) diff --git a/apps/docs/content/guides/database/postgres/row-level-security.mdx b/apps/docs/content/guides/database/postgres/row-level-security.mdx index 460e2aa761191..7943aebcc910f 100644 --- a/apps/docs/content/guides/database/postgres/row-level-security.mdx +++ b/apps/docs/content/guides/database/postgres/row-level-security.mdx @@ -5,7 +5,7 @@ description: 'Secure your data using Postgres Row Level Security.' subtitle: 'Secure your data using Postgres Row Level Security.' --- -When you need granular authorization rules, nothing beats Postgres's [Row Level Security (RLS)](https://www.postgresql.org/docs/current/ddl-rowsecurity.html). +Postgres [Row Level Security (RLS)](https://www.postgresql.org/docs/current/ddl-rowsecurity.html) gives you granular authorization rules that run inside the database. ## Row Level Security in Supabase @@ -31,15 +31,15 @@ Policies alone don't do this. See [Grants and policies](#grants-and-policies). -RLS is incredibly powerful and flexible, allowing you to write complex SQL rules that fit your unique business needs. RLS can be combined with [Supabase Auth](/docs/guides/auth) for end-to-end user security from the browser to the database. +You write RLS rules in SQL, so a rule can express whatever access logic your app needs. Combine RLS with [Supabase Auth](/docs/guides/auth) for end-to-end user security from the browser to the database. RLS is a Postgres primitive and can provide "[defense in depth]()" to protect your data from malicious actors even when accessed through third-party tooling. ## Policies -[Policies](https://www.postgresql.org/docs/current/sql-createpolicy.html) are Postgres's rule engine. Policies are easy to understand once you get the hang of them. Each policy is attached to a table, and the policy is executed every time a table is accessed. +[Policies](https://www.postgresql.org/docs/current/sql-createpolicy.html) are Postgres's rule engine. Each policy is attached to a table, and the policy is executed every time a table is accessed. -You can just think of them as adding a `WHERE` clause to every query. For example a policy like this ... +Think of a policy as adding a `WHERE` clause to every query. For example, a policy like this: ```sql create policy "Individuals can view their own todos." @@ -47,7 +47,7 @@ on todos for select using ( (select auth.uid()) = user_id ); ``` -.. would translate to this whenever a user tries to select from the todos table: +That policy translates to this whenever a user tries to select from the todos table: ```sql select * @@ -217,7 +217,7 @@ Using the `anon` Postgres role is different from an [anonymous user](/docs/guide Policies are SQL logic that you attach to a Postgres table. You can attach as many policies as you want to each table. -Supabase provides some [helpers](#helper-functions) that simplify RLS if you're using Supabase Auth. We'll use these helpers to illustrate some basic policies: +Supabase provides some [helpers](#helper-functions) that simplify RLS if you're using Supabase Auth. The examples below use these helpers. ### SELECT policies @@ -247,15 +247,16 @@ Alternatively, if you only wanted users to be able to see their own profiles: ```sql create policy "User can see their own profile only." -on profiles -for select using ( (select auth.uid()) = user_id ); +on profiles for select +to authenticated +using ( (select auth.uid()) = user_id ); ``` ### INSERT policies You can specify insert policies with the `with check` clause. The `with check` expression ensures that any new row data adheres to the policy constraints. -Say you have a table called `profiles` in the public schema and you only want users to create a profile for themselves. In that case, we want to check their User ID matches the value that they are trying to insert: +Say you have a table called `profiles` in the public schema and you only want users to create a profile for themselves. In that case, check that their user ID matches the value they are trying to insert: ```sql -- 1. Create table @@ -408,7 +409,7 @@ Supabase provides special "Service" keys, which can be used to bypass RLS. These -Supabase will adhere to the RLS policy of the signed-in user, even if the client library is initialized with a Service Key. +A Service Key bypasses RLS only when the request carries no user access token. If the request carries one, it runs under the RLS policies of that signed-in user, even when the client library was initialized with a Service Key. @@ -418,7 +419,143 @@ You can also create new [Postgres Roles](/docs/guides/database/postgres/roles) w alter role "role_name" with bypassrls; ``` -This can be useful for system-level access. You should _never_ share login credentials for any Postgres Role with this privilege. +This can be useful for system-level access. **Never** share login credentials for any Postgres Role with this privilege. + +## Test your policies + +We recommend writing tests for every policy, in the same change that sets the grants and creates the policies. Tests are a fundamental part of a secure setup, and they give you a repeatable way to prove a policy behaves the way you intended. + +A wrong policy fails quietly. Too permissive, and a query returns rows it shouldn't. Too strict, and it returns nothing and raises no error. Neither case surfaces as an error, so tests are how you find out. + +Supabase runs database tests with [pgTAP](/docs/guides/database/extensions/pgtap) through the CLI. Test files are `.sql` files under `supabase/tests/`. + +### Anatomy of a policy test + +Each case sets an identity, runs one statement as that identity, and asserts the outcome. Three things decide whether the assertion means anything. + +**Identity.** Switch role and identity between cases with `set local role` and `set local request.jwt.claim.sub`, so each assertion runs as the user it describes. Without the switch, every case runs as the same role and proves nothing about access. + +**Denials.** A denied request doesn't always raise an error, so match the assertion to the way the denial happens: + +- A missing grant raises `42501`. Assert it with `throws_ok`. +- A `with check` violation raises `42501`. Assert it with `throws_ok`. +- A `using` clause that filters the target row out raises nothing. The update or delete matches zero rows instead. Assert that no row changed. + +**Allowed writes.** The absence of an error doesn't prove that anything changed. Add `returning` to the statement so one assertion covers both directions. An allowed write returns the changed row, and a write the policy filters out returns nothing. + +### Write and run the tests + +1. Create the tests directory and a test file: + + ```bash + mkdir -p supabase/tests + touch supabase/tests/profiles_rls.test.sql + ``` + +2. Write the tests. Cover `select`, `insert`, `update`, and `delete` twice each, once for a request the policy allows and once for a request it denies. Cover `anon` as well as `authenticated`. + +3. Run the suite: + + ```bash + supabase test db + ``` + +This example tests a `profiles` table where `authenticated` holds every privilege, `anon` holds none, and each user reads and writes only their own row: + +```sql supabase/tests/profiles_rls.test.sql +begin; +select plan(11); + +-- Seed two users. The rows come later, through the policies under test. +insert into auth.users (id, email) +values + ('11111111-1111-1111-1111-111111111111', 'owner@example.com'), + ('22222222-2222-2222-2222-222222222222', 'other@example.com'); + +-- Signed-out visitors hold no grant, so the request stops before any policy runs. +set local role anon; +select throws_ok( + $$select * from profiles$$, + '42501', + null, + 'anon cannot read profiles' +); +select throws_ok( + $$insert into profiles (id, user_id) + values (gen_random_uuid(), '11111111-1111-1111-1111-111111111111')$$, + '42501', + null, + 'anon cannot insert a profile' +); + +-- The owner reads and writes their own row. +set local role authenticated; +set local request.jwt.claim.sub = '11111111-1111-1111-1111-111111111111'; +select results_eq( + $$insert into profiles (id, user_id, avatar_url) + values ( + gen_random_uuid(), + '11111111-1111-1111-1111-111111111111', + 'owner.png' + ) + returning avatar_url$$, + array['owner.png'], + 'the owner creates their own profile' +); +select results_eq( + $$select avatar_url from profiles$$, + array['owner.png'], + 'the owner reads their own profile' +); +select results_eq( + $$update profiles set avatar_url = 'updated.png' returning avatar_url$$, + array['updated.png'], + 'the owner updates their own profile' +); + +-- The with check clause rejects the row, which raises. +select throws_ok( + $$insert into profiles (id, user_id) + values (gen_random_uuid(), '22222222-2222-2222-2222-222222222222')$$, + '42501', + null, + 'the owner cannot create a profile for someone else' +); + +-- A signed-in stranger holds the grant, so the policy is what stops them. The +-- using clause filters the row out, so these match nothing and raise nothing. +set local request.jwt.claim.sub = '22222222-2222-2222-2222-222222222222'; +select is_empty( + $$select * from profiles$$, + 'another user reads no profiles' +); +select is_empty( + $$update profiles set avatar_url = 'stolen.png' returning avatar_url$$, + 'another user updates no profiles' +); +select is_empty( + $$delete from profiles returning id$$, + 'another user deletes no profiles' +); + +-- The row is still there, still holding the owner's value. +set local request.jwt.claim.sub = '11111111-1111-1111-1111-111111111111'; +select results_eq( + $$select avatar_url from profiles$$, + array['updated.png'], + 'the other user changed nothing' +); +select results_eq( + $$delete from profiles returning avatar_url$$, + array['updated.png'], + 'the owner deletes their own profile' +); + +select * from finish(); +rollback; +``` + +For CLI setup and more pgTAP helpers, see [Testing your database](/docs/guides/database/testing). ## Test your policies @@ -558,9 +695,9 @@ For CLI setup and more pgTAP helpers, see [Testing your database](/docs/guides/d ## RLS performance recommendations -Every authorization system has an impact on performance. While row level security is powerful, the performance impact is important to keep in mind. This is especially true for queries that scan every row in a table - like many `select` operations, including those using limit, offset, and ordering. +Every authorization system has an impact on performance. Postgres evaluates a policy expression against each candidate row, so the cost scales with the rows a query scans. This matters most for queries that scan every row in a table, like many `select` operations, including those using limit, offset, and ordering. -Based on a series of [tests](https://github.com/GaryAustin1/RLS-Performance), we have a few recommendations for RLS: +Based on a series of [tests](https://github.com/GaryAustin1/RLS-Performance), these are the recommendations for RLS: ### Add indexes @@ -688,10 +825,11 @@ create function private.has_good_role() returns boolean language plpgsql security definer -- will run as the creator +set search_path = '' -- every name inside must be schema-qualified as $$ begin return exists ( - select 1 from roles_table + select 1 from public.roles_table where (select auth.uid()) = user_id and role = 'good_role' ); end; @@ -704,9 +842,11 @@ to authenticated using ( (select private.has_good_role()) ); ``` +Set `search_path = ''` on every `security definer` function and schema-qualify the names inside it. Without a pinned `search_path`, a caller can point an unqualified name at their own object and run it with the function owner's privileges. + -Security-definer functions should never be created in a schema in the "Exposed schemas" inside your [API settings](/dashboard/project/_/settings/api)`. +A `security definer` function in an exposed schema is callable over the Data API with the creator's privileges. Never create one in a schema listed under "Exposed schemas" in your [API settings](/dashboard/project/_/settings/api). diff --git a/apps/studio/components/interfaces/Account/AccessTokens/Scoped/NewScopedTokenSheet.test.tsx b/apps/studio/components/interfaces/Account/AccessTokens/Scoped/NewScopedTokenSheet.test.tsx index dce13235404ed..a1c401f7170b7 100644 --- a/apps/studio/components/interfaces/Account/AccessTokens/Scoped/NewScopedTokenSheet.test.tsx +++ b/apps/studio/components/interfaces/Account/AccessTokens/Scoped/NewScopedTokenSheet.test.tsx @@ -204,7 +204,7 @@ describe('NewScopedTokenSheet', () => { fireEvent.click(await screen.findByRole('button', { name: 'Done' })) // Dialog has been closed await waitFor(() => expect(screen.queryByRole('dialog')).toBeNull()) - }) + }, 10_000) // Organization scope tests test('requires an organization when scope is Organization', async () => { @@ -254,7 +254,7 @@ describe('NewScopedTokenSheet', () => { fireEvent.click(await screen.findByRole('button', { name: 'Done' })) // Dialog has been closed await waitFor(() => expect(screen.queryByRole('dialog')).toBeNull()) - }) + }, 10_000) test('opens the experimental API dialog from the dropdown', async () => { renderSheet() diff --git a/apps/studio/components/interfaces/Explorer/QueryTab.test.tsx b/apps/studio/components/interfaces/Explorer/QueryTab.test.tsx index 61c303017ada8..21da1d7857472 100644 --- a/apps/studio/components/interfaces/Explorer/QueryTab.test.tsx +++ b/apps/studio/components/interfaces/Explorer/QueryTab.test.tsx @@ -32,7 +32,19 @@ vi.mock('@/components/ui/CodeEditor/CodeEditor', () => ({ ), })) -vi.mock('./ExplorerQuerySourceMenu', () => ({ ExplorerQuerySourceMenu: () => null })) +vi.mock('./ExplorerQuerySourceMenu', () => ({ + ExplorerQuerySourceMenu: ({ + roleImpersonationState, + }: { + roleImpersonationState?: { role?: { type: string; role?: string } } + }) => ( +
+ {roleImpersonationState?.role?.type === 'postgrest' + ? roleImpersonationState.role.role + : 'none'} +
+ ), +})) const renderQueryTab = () => customRender( @@ -168,4 +180,30 @@ describe('QueryTab execution', () => { new Date(bodies[0].iso_timestamp_start).getTime() ).toBe(2 * 60 * 60 * 1000) }) + + it('isolates the impersonated role selection per query tab', async () => { + createDraft({ _tag: 'database' }) + explorerQueryState.removeDraft({ id: 'query-test-2', projectRef: 'default' }) + explorerQueryState.createDraft({ id: 'query-test-2', projectRef: 'default', sql: 'select 2' }) + explorerQueryState.setRole({ + id: 'query-test', + role: { type: 'postgrest', role: 'service_role' }, + }) + + const { rerender } = renderQueryTab() + expect(await screen.findByTestId('impersonated-role')).toHaveTextContent('service_role') + + testContext.params = { ref: 'default', id: 'query-test-2' } + rerender( + + + + ) + + expect(await screen.findByTestId('impersonated-role')).toHaveTextContent('none') + + await act(async () => { + explorerQueryState.removeDraft({ id: 'query-test-2', projectRef: 'default' }) + }) + }) }) diff --git a/apps/studio/components/interfaces/Explorer/QueryTab.tsx b/apps/studio/components/interfaces/Explorer/QueryTab.tsx index ee94abf4ea894..306b03b41712b 100644 --- a/apps/studio/components/interfaces/Explorer/QueryTab.tsx +++ b/apps/studio/components/interfaces/Explorer/QueryTab.tsx @@ -1,14 +1,14 @@ import { useParams } from 'common' import { Loader2, SquareCode } from 'lucide-react' import { useRouter } from 'next/router' -import { useContext, useEffect, useState } from 'react' +import { useCallback, useContext, useEffect, useState } from 'react' import { Button } from 'ui' import { QueryEditor, type ExplorerQueryModel } from './QueryEditor' import { type QueryResult } from './types' import { toQuerySourceBinding } from '@/data/query-sources/query-source-registry' import { explorerQueryState, useExplorerQueryStateSnapshot } from '@/state/explorer-query' -import { useLocalRoleImpersonationState } from '@/state/role-impersonation-state' +import { useControlledRoleImpersonationState } from '@/state/role-impersonation-state' import { createTabId, TabsStateContext } from '@/state/tabs' /** Query-tab lifecycle adapter around the shared QueryEditor. */ @@ -17,9 +17,7 @@ export const QueryTab = () => { const router = useRouter() const tabs = useContext(TabsStateContext) const querySnap = useExplorerQueryStateSnapshot() - const roleImpersonationState = useLocalRoleImpersonationState() - const [rowLimit, setRowLimit] = useState(100) const [restoredQueryKey, setRestoredQueryKey] = useState() const stateDraft = id ? querySnap.drafts[id] : undefined @@ -27,6 +25,16 @@ export const QueryTab = () => { const result = draft && id ? querySnap.results[id] : undefined const queryKey = id && ref ? `${ref}:${id}` : undefined + const roleImpersonationState = useControlledRoleImpersonationState( + draft?._tag === 'database' ? draft.role : undefined, + useCallback( + (role) => { + if (id) explorerQueryState.setRole({ id, role }) + }, + [id] + ) + ) + useEffect(() => { if (!id || !ref) return @@ -84,7 +92,7 @@ export const QueryTab = () => { : { ...toQuerySourceBinding(draft), uncheckedSql: draft.uncheckedSql, - rowLimit, + rowLimit: draft.rowLimit, } return ( @@ -103,7 +111,7 @@ export const QueryTab = () => { onSqlChange={(sql) => explorerQueryState.updateDraft({ id, sql })} onSourceChange={(source) => explorerQueryState.updateDraft({ id, source })} onResultChange={handleResultChange} - onRowLimitChange={setRowLimit} + onRowLimitChange={(rowLimit) => explorerQueryState.updateDraft({ id, rowLimit })} /> ) } diff --git a/apps/studio/lib/role-impersonation.ts b/apps/studio/lib/role-impersonation.ts index 0f6353f12b70e..96fd694deaf29 100644 --- a/apps/studio/lib/role-impersonation.ts +++ b/apps/studio/lib/role-impersonation.ts @@ -1,4 +1,5 @@ import { getImpersonationSQL, type SafeSqlFragment } from '@supabase/pg-meta' +import { z } from 'zod' import { uuidv4 } from './helpers' import type { User } from '@/data/auth/users-infinite-query' @@ -40,6 +41,58 @@ type CustomImpersonationRole = { export type ImpersonationRole = PostgrestImpersonationRole | CustomImpersonationRole +/** + * The impersonated `user` is the same generated `User` shape already persisted verbatim to + * localStorage elsewhere (see `USER_IMPERSONATION_SELECTOR_PREVIOUS_SEARCHES`) — trusted + * as-is rather than re-validated field-by-field, since it only ever round-trips our own + * writes and its shape tracks a generated API type this schema shouldn't have to mirror. + */ +const impersonatedUserSchema = z + .record(z.string(), z.unknown()) + .transform((value) => value as unknown as User) + +const aalSchema = z.enum(['aal1', 'aal2']) + +const postgrestImpersonationRoleSchema = z.union([ + z.object({ type: z.literal('postgrest'), role: z.literal('anon') }).strict(), + z.object({ type: z.literal('postgrest'), role: z.literal('service_role') }).strict(), + z + .object({ + type: z.literal('postgrest'), + role: z.literal('authenticated'), + userType: z.literal('native'), + user: impersonatedUserSchema.optional(), + aal: aalSchema.optional(), + }) + .strict(), + z + .object({ + type: z.literal('postgrest'), + role: z.literal('authenticated'), + userType: z.literal('external'), + externalAuth: z + .object({ + sub: z.string(), + additionalClaims: z.record(z.string(), z.unknown()).optional(), + }) + .optional(), + aal: aalSchema.optional(), + }) + .strict(), +]) + +const customImpersonationRoleSchema = z + .object({ type: z.literal('custom'), role: z.string() }) + .strict() + +/** Parses to `ImpersonationRole` — verified at the `role` field assignment in `toDraft` + * (`state/explorer-query.ts`), since annotating the schema type directly here would also + * constrain its *input* type, which is narrower than `ImpersonationRole` pre-transform. */ +export const impersonationRoleSchema = z.union([ + postgrestImpersonationRoleSchema, + customImpersonationRoleSchema, +]) + export function getExp1HourFromNow() { return Math.floor((Date.now() + 60 * 60 * 1000) / 1000) } diff --git a/apps/studio/state/explorer-query.test.ts b/apps/studio/state/explorer-query.test.ts index e23f7f9fbdb56..d3268a2606f42 100644 --- a/apps/studio/state/explorer-query.test.ts +++ b/apps/studio/state/explorer-query.test.ts @@ -202,6 +202,161 @@ describe('explorer query drafts', () => { expect(persisted[`query-${MAX_PERSISTED_EXPLORER_QUERY_DRAFTS}`]).toBeDefined() }) + it('persists and restores a per-draft row limit independently of other drafts', () => { + const storage = createMemoryStorage() + const state = createExplorerQueryState(storage) + + state.createDraft({ id: 'query-1', projectRef: 'project-a' }) + state.createDraft({ id: 'query-2', projectRef: 'project-a' }) + state.updateDraft({ id: 'query-1', rowLimit: 500 }) + + expect(state.drafts['query-1']).toMatchObject({ rowLimit: 500 }) + expect(state.drafts['query-2']).toMatchObject({ rowLimit: 100 }) + + const restored = createExplorerQueryState(storage) + expect(restored.restoreDraft({ id: 'query-1', projectRef: 'project-a' })).toBe(true) + expect(restored.restoreDraft({ id: 'query-2', projectRef: 'project-a' })).toBe(true) + expect(restored.drafts['query-1']).toMatchObject({ rowLimit: 500 }) + expect(restored.drafts['query-2']).toMatchObject({ rowLimit: 100 }) + }) + + it('defaults the row limit for drafts persisted before row limits existed', () => { + const storage = createMemoryStorage() + storage.setItem( + LOCAL_STORAGE_KEYS.EXPLORER_QUERY_DRAFTS('project-a'), + JSON.stringify({ + 'query-1': { name: 'Legacy query', sql: 'select 1', updatedAt: 1 }, + }) + ) + + const state = createExplorerQueryState(storage) + expect(state.restoreDraft({ id: 'query-1', projectRef: 'project-a' })).toBe(true) + expect(state.drafts['query-1']).toMatchObject({ rowLimit: 100 }) + }) + + it('accepts every row limit the row limit menu can produce', () => { + const storage = createMemoryStorage() + + for (const rowLimit of [-1, 100, 500, 1000]) { + storage.setItem( + LOCAL_STORAGE_KEYS.EXPLORER_QUERY_DRAFTS('project-a'), + JSON.stringify({ + 'query-1': { name: 'Query', sql: 'select 1', updatedAt: 1, rowLimit }, + }) + ) + + const state = createExplorerQueryState(storage) + expect(state.restoreDraft({ id: 'query-1', projectRef: 'project-a' })).toBe(true) + expect(state.drafts['query-1']).toMatchObject({ rowLimit }) + } + }) + + it('normalizes a fractional persisted row limit to the default', () => { + const storage = createMemoryStorage() + storage.setItem( + LOCAL_STORAGE_KEYS.EXPLORER_QUERY_DRAFTS('project-a'), + JSON.stringify({ + 'query-1': { name: 'Query', sql: 'select 1', updatedAt: 1, rowLimit: 100.5 }, + }) + ) + + const state = createExplorerQueryState(storage) + expect(state.restoreDraft({ id: 'query-1', projectRef: 'project-a' })).toBe(true) + expect(state.drafts['query-1']).toMatchObject({ rowLimit: 100 }) + }) + + it('normalizes an out-of-range persisted row limit to the default', () => { + const storage = createMemoryStorage() + storage.setItem( + LOCAL_STORAGE_KEYS.EXPLORER_QUERY_DRAFTS('project-a'), + JSON.stringify({ + 'query-1': { name: 'Query', sql: 'select 1', updatedAt: 1, rowLimit: 999999 }, + }) + ) + + const state = createExplorerQueryState(storage) + expect(state.restoreDraft({ id: 'query-1', projectRef: 'project-a' })).toBe(true) + expect(state.drafts['query-1']).toMatchObject({ rowLimit: 100 }) + }) + + it('normalizes a non-numeric persisted row limit to the default without dropping the draft', () => { + const storage = createMemoryStorage() + storage.setItem( + LOCAL_STORAGE_KEYS.EXPLORER_QUERY_DRAFTS('project-a'), + JSON.stringify({ + 'query-1': { name: 'Query', sql: 'select 1', updatedAt: 1, rowLimit: 'unlimited' }, + }) + ) + + const state = createExplorerQueryState(storage) + expect(state.restoreDraft({ id: 'query-1', projectRef: 'project-a' })).toBe(true) + expect(state.drafts['query-1']).toMatchObject({ rowLimit: 100, uncheckedSql: 'select 1' }) + }) + + it('persists and restores a per-draft impersonated role independently of other drafts', () => { + const storage = createMemoryStorage() + const state = createExplorerQueryState(storage) + + state.createDraft({ id: 'query-1', projectRef: 'project-a' }) + state.createDraft({ id: 'query-2', projectRef: 'project-a' }) + state.setRole({ id: 'query-1', role: { type: 'postgrest', role: 'anon' } }) + + expect(state.drafts['query-1']).toMatchObject({ role: { type: 'postgrest', role: 'anon' } }) + expect(state.drafts['query-2']).toMatchObject({ role: undefined }) + + const restored = createExplorerQueryState(storage) + expect(restored.restoreDraft({ id: 'query-1', projectRef: 'project-a' })).toBe(true) + expect(restored.restoreDraft({ id: 'query-2', projectRef: 'project-a' })).toBe(true) + expect(restored.drafts['query-1']).toMatchObject({ + role: { type: 'postgrest', role: 'anon' }, + }) + expect(restored.drafts['query-2']).toMatchObject({ role: undefined }) + }) + + it('clears a persisted role when set back to undefined', () => { + const storage = createMemoryStorage() + const state = createExplorerQueryState(storage) + + state.createDraft({ id: 'query-1', projectRef: 'project-a' }) + state.setRole({ id: 'query-1', role: { type: 'postgrest', role: 'service_role' } }) + state.setRole({ id: 'query-1', role: undefined }) + + expect(state.drafts['query-1']).toMatchObject({ role: undefined }) + + const restored = createExplorerQueryState(storage) + expect(restored.restoreDraft({ id: 'query-1', projectRef: 'project-a' })).toBe(true) + expect(restored.drafts['query-1']).toMatchObject({ role: undefined }) + }) + + it('drops a malformed persisted role rather than failing to restore the draft', () => { + const storage = createMemoryStorage() + storage.setItem( + LOCAL_STORAGE_KEYS.EXPLORER_QUERY_DRAFTS('project-a'), + JSON.stringify({ + 'query-1': { + name: 'Query with bad role', + sql: 'select 1', + updatedAt: 1, + role: { type: 'postgrest', role: 'not-a-real-role' }, + }, + }) + ) + + const state = createExplorerQueryState(storage) + expect(state.restoreDraft({ id: 'query-1', projectRef: 'project-a' })).toBe(true) + expect(state.drafts['query-1']).toMatchObject({ role: undefined }) + }) + + it('ignores role updates for logs drafts', () => { + const storage = createMemoryStorage() + const state = createExplorerQueryState(storage) + + state.createDraft({ id: 'query-1', projectRef: 'project-a', source: LOGS_SOURCE }) + state.setRole({ id: 'query-1', role: { type: 'postgrest', role: 'anon' } }) + + expect(state.drafts['query-1']).not.toHaveProperty('role') + }) + it('removes the persisted draft and its session result when its tab closes', () => { const storage = createMemoryStorage() const state = createExplorerQueryState(storage) diff --git a/apps/studio/state/explorer-query.ts b/apps/studio/state/explorer-query.ts index ae1f1972c6177..eb15cd9f3bfea 100644 --- a/apps/studio/state/explorer-query.ts +++ b/apps/studio/state/explorer-query.ts @@ -3,7 +3,9 @@ import { LOCAL_STORAGE_KEYS, safeLocalStorage } from 'common' import { proxy, ref, snapshot, useSnapshot } from 'valtio' import { z } from 'zod' +import { DEFAULT_CELL_ROW_LIMIT } from '@/components/interfaces/Explorer/QueryCell/QueryCell.utils' import { type QueryResult } from '@/components/interfaces/Explorer/types' +import { ROWS_PER_PAGE_OPTIONS } from '@/components/interfaces/SQLEditor/SQLEditor.constants' import { type DatabaseSourceParameters, type LogsSourceParameters, @@ -15,6 +17,7 @@ import { toQuerySourceBinding, type QuerySourceBinding, } from '@/data/query-sources/query-source-registry' +import { impersonationRoleSchema, type ImpersonationRole } from '@/lib/role-impersonation' type ExplorerQueryDraftBase = { id: string @@ -33,6 +36,8 @@ export type DatabaseQueryDraft = ExplorerQueryDraftBase & DatabaseSourceParameters & { _tag: 'database' uncheckedSql: UntrustedSqlFragment + rowLimit: number + role?: ImpersonationRole } export type LogsQueryDraft = ExplorerQueryDraftBase & @@ -57,6 +62,8 @@ type PersistedExplorerQueryDraft = { source: QuerySourceBinding sql: string updatedAt: number + rowLimit?: number + role?: ImpersonationRole } type PersistedExplorerQueryDrafts = Record @@ -72,8 +79,24 @@ const persistedDraftSchema = z.object({ sql: z.string(), updatedAt: z.number(), source: z.unknown().optional(), + rowLimit: z.unknown().optional(), + role: z.unknown().optional(), }) +const VALID_ROW_LIMITS = ROWS_PER_PAGE_OPTIONS.map((option) => option.value) + +/** + * Falls back to the default whenever a persisted row limit isn't one of the values the row + * limit menu can actually produce — e.g. a fractional or out-of-range number from corrupted + * or hand-edited storage. `undefined` (never persisted) passes through unchanged; `toDraft` + * applies the default for that case. + */ +const rowLimitSchema = z + .number() + .refine((value) => VALID_ROW_LIMITS.includes(value)) + .catch(DEFAULT_CELL_ROW_LIMIT) + .optional() + /** * Rebuilds a draft from its persisted form, branding the SQL for the backend the binding * names. The single place a stored string re-enters the type system as untrusted SQL, which @@ -104,6 +127,8 @@ const toDraft = ({ _tag: 'database', database_identifier: persisted.source.database_identifier, uncheckedSql: untrustedSql(persisted.sql), + rowLimit: persisted.rowLimit ?? DEFAULT_CELL_ROW_LIMIT, + role: persisted.role, } } @@ -125,6 +150,10 @@ const readPersistedDrafts = (storage: StorageLike, projectRef: string) => { ? parsedSource.data : createDefaultSourceBinding('database') + const parsedRole = impersonationRoleSchema.safeParse(draft.data.role) + const role = parsedRole.success ? parsedRole.data : undefined + const rowLimit = rowLimitSchema.parse(draft.data.rowLimit) + return [ [ id, @@ -133,6 +162,8 @@ const readPersistedDrafts = (storage: StorageLike, projectRef: string) => { source, sql: draft.data.sql, updatedAt: draft.data.updatedAt, + role, + rowLimit, }, ], ] @@ -172,6 +203,8 @@ export const createExplorerQueryState = (storage: StorageLike = safeLocalStorage source: toQuerySourceBinding(draft), sql: draft.uncheckedSql, updatedAt: draft.updatedAt, + rowLimit: draft._tag === 'database' ? draft.rowLimit : undefined, + role: draft._tag === 'database' ? draft.role : undefined, } writePersistedDrafts(storage, draft.projectRef, persisted) } @@ -186,12 +219,14 @@ export const createExplorerQueryState = (storage: StorageLike = safeLocalStorage name = 'Untitled query', sql = '', source = createDefaultSourceBinding('database'), + rowLimit = DEFAULT_CELL_ROW_LIMIT, }: { id: string projectRef: string name?: string sql?: string source?: QuerySourceBinding + rowLimit?: number }) => { const draft = toDraft({ id, @@ -201,6 +236,7 @@ export const createExplorerQueryState = (storage: StorageLike = safeLocalStorage source: querySourceBindingSchema.parse(source), sql, updatedAt: Date.now(), + rowLimit, }, }) @@ -237,11 +273,13 @@ export const createExplorerQueryState = (storage: StorageLike = safeLocalStorage name, source, sql, + rowLimit, }: { id: string name?: string source?: QuerySourceBinding sql?: string + rowLimit?: number }) => { const draft = state.drafts[id] if (!draft) return @@ -249,6 +287,9 @@ export const createExplorerQueryState = (storage: StorageLike = safeLocalStorage const nextSource = source === undefined ? undefined : querySourceBindingSchema.parse(source) if (nextSource !== undefined && nextSource._tag !== draft._tag) delete state.results[id] + const currentRowLimit = draft._tag === 'database' ? draft.rowLimit : undefined + const currentRole = draft._tag === 'database' ? draft.role : undefined + state.drafts[id] = toDraft({ id, projectRef: draft.projectRef, @@ -257,6 +298,8 @@ export const createExplorerQueryState = (storage: StorageLike = safeLocalStorage source: nextSource ?? toQuerySourceBinding(draft), sql: sql ?? draft.uncheckedSql, updatedAt: Date.now(), + rowLimit: rowLimit ?? currentRowLimit, + role: currentRole, }, }) @@ -273,7 +316,7 @@ export const createExplorerQueryState = (storage: StorageLike = safeLocalStorage const pending = pendingPersistence.get(id) if (pending) clearTimeout(pending.timeout) - if (name !== undefined || source !== undefined) persist() + if (name !== undefined || source !== undefined || rowLimit !== undefined) persist() else { const timeout = setTimeout(persist, EXPLORER_QUERY_PERSIST_DELAY) pendingPersistence.set(id, { timeout, persist }) @@ -305,6 +348,21 @@ export const createExplorerQueryState = (storage: StorageLike = safeLocalStorage setResult: ({ id, result }: { id: string; result: ExplorerQueryResult }) => { state.results[id] = ref(result) }, + + /** + * Separate from `updateDraft` because `undefined` is a meaningful value here (clearing + * the impersonated role), whereas `updateDraft`'s optional fields all use `undefined` + * to mean "leave unchanged." Logs drafts have no impersonation concept, so this is a + * no-op for them. + */ + setRole: ({ id, role }: { id: string; role: ImpersonationRole | undefined }) => { + const draft = state.drafts[id] + if (!draft || draft._tag !== 'database') return + + const updatedDraft: DatabaseQueryDraft = { ...draft, role, updatedAt: Date.now() } + state.drafts[id] = updatedDraft + persistDraft(updatedDraft) + }, }) return state diff --git a/apps/studio/state/role-impersonation-state.tsx b/apps/studio/state/role-impersonation-state.tsx index e1ba37494ffdb..4142c61d6966f 100644 --- a/apps/studio/state/role-impersonation-state.tsx +++ b/apps/studio/state/role-impersonation-state.tsx @@ -6,11 +6,15 @@ import { useCallback, useContext, useEffect, + useRef, useState, } from 'react' import { proxy, snapshot, subscribe, useSnapshot } from 'valtio' -import { CustomAccessTokenHookDetails } from '../hooks/misc/useCustomAccessTokenHookDetails' +import { + CustomAccessTokenHookDetails, + useCustomAccessTokenHookDetails, +} from '../hooks/misc/useCustomAccessTokenHookDetails' import { executeSql } from '@/data/sql/execute-sql-mutation' import { useLatest } from '@/hooks/misc/useLatest' import { useSelectedProjectQuery } from '@/hooks/misc/useSelectedProject' @@ -100,12 +104,6 @@ export function createRoleImpersonationState( export type RoleImpersonationState = ReturnType -/** - * The subset of `RoleImpersonationState` a role-picker UI needs: the current selection, its - * resolved claims, and the setter. Satisfied by both the shared project-wide context (via - * `useRoleImpersonationStateSnapshot`) and `useLocalRoleImpersonationState`, so role-picking - * components can work against either without knowing which one they got. - */ export type RoleImpersonationController = Pick< RoleImpersonationState, 'role' | 'claims' | 'setRole' @@ -141,6 +139,9 @@ export function useRoleImpersonationStateSnapshot(options?: Parameters void +): RoleImpersonationController { + const { data: project } = useSelectedProjectQuery() + const customizeAccessToken = useCustomizeAccessToken(project?.ref, project?.connectionString) + + const projectRef = project?.ref ?? '' + const customizeAccessTokenRef = useLatest(customizeAccessToken) + const customAccessTokenHookDetails = useCustomAccessTokenHookDetails(project?.ref) + const customAccessTokenHookDetailsRef = useLatest(customAccessTokenHookDetails) + const onRoleChangeRef = useLatest(onRoleChange) + + const [claims, setClaims] = useState(undefined) + + // Guards against re-resolving claims for a role change that `setRole` below just resolved + // itself — without it, every selection would re-run the (possibly RPC-backed) resolution + // twice: once eagerly in `setRole`, once again here once `role` updates on the next render. + const skipNextResolveRef = useRef(false) + + useEffect(() => { + if (skipNextResolveRef.current) { + skipNextResolveRef.current = false + return + } + + let cancelled = false + + resolveRoleClaims( + projectRef, + role, + customAccessTokenHookDetailsRef.current, + customizeAccessTokenRef.current + ).then((nextClaims) => { + if (!cancelled) setClaims(nextClaims) + }) + + return () => { + cancelled = true + } + // Resolves only when the controlled role identity changes (e.g. switching tabs) + }, [customAccessTokenHookDetailsRef, customizeAccessTokenRef, projectRef, role]) + + const setRole = useCallback( + async ( + nextRole: ImpersonationRole | undefined, + customAccessTokenHookDetails?: CustomAccessTokenHookDetails + ) => { + // Captured before the await: if the controlling tab changes while this resolution is + // in flight, `onRoleChangeRef.current` will point at a different tab's callback by the + // time we get here. Comparing against the captured reference lets us detect that and + // discard the result instead of writing this role/claims into the wrong tab. + const onRoleChangeAtCallTime = onRoleChangeRef.current + + const nextClaims = await resolveRoleClaims( + projectRef, + nextRole, + customAccessTokenHookDetails ?? customAccessTokenHookDetailsRef.current, + customizeAccessTokenRef.current + ) + + if (onRoleChangeRef.current !== onRoleChangeAtCallTime) return + + skipNextResolveRef.current = true + onRoleChangeAtCallTime(nextRole) + setClaims(nextClaims) + }, + [projectRef, customizeAccessTokenRef, customAccessTokenHookDetailsRef, onRoleChangeRef] + ) + + return { role, claims, setRole } +} + export function useGetImpersonatedRoleState() { const roleImpersonationState = useContext(RoleImpersonationStateContext)