diff --git a/frontend/src/lib/components/workspaceSettings/AddDataTableWizard.svelte b/frontend/src/lib/components/workspaceSettings/AddDataTableWizard.svelte index f8a22207e2..c73db25f8c 100644 --- a/frontend/src/lib/components/workspaceSettings/AddDataTableWizard.svelte +++ b/frontend/src/lib/components/workspaceSettings/AddDataTableWizard.svelte @@ -353,13 +353,13 @@ }) }) - function reset() { + function reset(from: WizardResume | undefined) { wiz = newWizardState({ - name: resume?.name || defaultTableName(), - projectName: resume?.projectName || defaultProjectName(), + name: from?.name || defaultTableName(), + projectName: from?.projectName || defaultProjectName(), folder: defaultFolder() }) - wiz.supabase.region = resume?.region ?? DEFAULT_SUPABASE_REGION + wiz.supabase.region = from?.region ?? DEFAULT_SUPABASE_REGION run = { steps: [], running: false } maxStep = 1 // What the last run claimed belongs to the data table it created; a fresh one has to @@ -372,38 +372,39 @@ lastFailure = '' pathTakenError = '' poolerUnavailable = undefined - if (resume) { + if (from) { wiz.provider = 'supabase' // The clears above are for a fresh run. This one is the same run coming back from the // redirect, so what it had already created is still its own to write over. - claims = claimsFromJSON(resume.claims) - createdProjects = resume.createdProjects ?? [] + claims = claimsFromJSON(from.claims) + createdProjects = from.createdProjects ?? [] leftBehind = anythingClaimed(claims) || createdProjects.length > 0 // Which side of the toggle it was on, and the organization it was pointed at. Left to // default, a run that died mid-create comes back asking for the password it generated. - if (resume.mode) wiz.supabase.mode = resume.mode - if (resume.org) wiz.supabase.org = resume.org + if (from.mode) wiz.supabase.mode = from.mode + if (from.org) wiz.supabase.org = from.org // The password is deliberately not parked -- it is a secret and sessionStorage is not // the place for one. Carrying the project is what stops the resume landing on a // different database with an empty password field and no sign anything moved. - if (resume.project) wiz.supabase.project = resume.project - if (resume.connectionMode) wiz.supabase.connectionMode = resume.connectionMode - const cut = resume.resourcePath?.lastIndexOf('/') ?? -1 - if (resume.resourcePath && cut > 0) { - wiz.review.folder = resume.resourcePath.slice(0, cut) - wiz.review.resourceName = resume.resourcePath.slice(cut + 1) + if (from.project) wiz.supabase.project = from.project + if (from.connectionMode) wiz.supabase.connectionMode = from.connectionMode + const cut = from.resourcePath?.lastIndexOf('/') ?? -1 + if (from.resourcePath && cut > 0) { + wiz.review.folder = from.resourcePath.slice(0, cut) + wiz.review.resourceName = from.resourcePath.slice(cut + 1) } enterStep(2) } } /** - * Opened by the settings page. Reopening after the Supabase redirect drops the user back on - * the setup step with what they had already chosen, so authorizing does not feel like - * starting over. + * Opened by the settings page. A run coming back from the Supabase redirect is handed in + * rather than read off the `resume` prop: the caller has it, and taking it as an argument is + * what keeps the restore independent of when the prop it was assigned to reaches this + * component. */ - export function open() { - reset() + export function open(parked?: WizardResume) { + reset(parked ?? resume) opened = true } diff --git a/frontend/src/lib/components/workspaceSettings/DataTableSettings.svelte b/frontend/src/lib/components/workspaceSettings/DataTableSettings.svelte index 8ce088beea..bb97e434f6 100644 --- a/frontend/src/lib/components/workspaceSettings/DataTableSettings.svelte +++ b/frontend/src/lib/components/workspaceSettings/DataTableSettings.svelte @@ -224,7 +224,7 @@ const wizardEnabled = isDataTableWizardEnabled() let wizardOpen = $state(false) /** Opened through the wizard's own `open()`, which is what sets a fresh run up. */ - let wizard: { open: () => void } | undefined = $state(undefined) + let wizard: { open: (parked?: WizardResume) => void } | undefined = $state(undefined) let wizardResume: WizardResume | undefined = $state(undefined) // Supabase sends the user back here after authorizing; pick the wizard back up where it @@ -234,7 +234,9 @@ const parked = takeParkedWizard() if (parked) { wizardResume = parked - wizard?.open() + // Handed in, not left to the `resume` prop: the wizard rebuilds the run synchronously + // inside this call, and a parked run that arrived late would come back as a fresh one. + wizard?.open(parked) } }) diff --git a/frontend/src/lib/components/workspaceSettings/addDataTableModel.ts b/frontend/src/lib/components/workspaceSettings/addDataTableModel.ts index 6017d0e66b..e01987c3ad 100644 --- a/frontend/src/lib/components/workspaceSettings/addDataTableModel.ts +++ b/frontend/src/lib/components/workspaceSettings/addDataTableModel.ts @@ -368,7 +368,7 @@ export type RunDeps = { * name or the path in between. */ claims: Claims - /** The mark written onto a secret or resource this run creates: `edited_by` / `created_by`. */ + /** Stands in as the mark where the object was written but its timestamp could not be read back. */ username: string } diff --git a/frontend/src/lib/components/workspaceSettings/setupClaims.ts b/frontend/src/lib/components/workspaceSettings/setupClaims.ts index 7520833f66..48f844d6f9 100644 --- a/frontend/src/lib/components/workspaceSettings/setupClaims.ts +++ b/frontend/src/lib/components/workspaceSettings/setupClaims.ts @@ -66,7 +66,7 @@ export function anythingClaimed(claims: Claims): boolean { /** * Carried across the full-page redirect the blocked-popup Supabase leg falls back to. No secret - * travels: a mark is a username or a resource path. + * travels: a mark is a timestamp or a resource path. */ export function claimsToJSON(claims: Claims): Claim[] { return [...claims] diff --git a/frontend/src/lib/utils/postgresConnectionString.test.ts b/frontend/src/lib/utils/postgresConnectionString.test.ts index 56d1211102..33efc16797 100644 --- a/frontend/src/lib/utils/postgresConnectionString.test.ts +++ b/frontend/src/lib/utils/postgresConnectionString.test.ts @@ -138,6 +138,26 @@ describe('unsupportedConnectionParam', () => { expect(parsePostgresConnectionString(disguised)?.sslmode).toBeUndefined() }) + // libpq matches parameter names case-insensitively. Folding in one reader and not the other + // is what lets a name through the allowlist and past the parser, so the string is saved as + // whatever the default happens to be rather than what it asked for. + it('reads a parameter whatever its case', () => { + const shouted = 'postgres://u:p@h/db?SslMode=verify-full' + expect(unsupportedConnectionParam(shouted)).toBeUndefined() + expect(parsePostgresConnectionString(shouted)?.sslmode).toBe('verify-full') + expect(unsupportedConnectionParam('postgres://u:p@h/db?Connect_Timeout=1')).toBe( + 'connect_timeout' + ) + }) + + // libpq takes the last of a repeated parameter. Taking the first reads a weaker mode than + // the string actually asks for. + it('takes the last value of a repeated parameter', () => { + expect( + parsePostgresConnectionString('postgres://u:p@h/db?sslmode=disable&sslmode=require')?.sslmode + ).toBe('require') + }) + it('ignores the one it can store, and the ones that cost nothing', () => { expect(unsupportedConnectionParam('postgres://u:p@h/db?sslmode=require')).toBeUndefined() expect(unsupportedConnectionParam('postgres://u:p@h/db?application_name=wm')).toBeUndefined() diff --git a/frontend/src/lib/utils/postgresConnectionString.ts b/frontend/src/lib/utils/postgresConnectionString.ts index 035ff20c1f..e4da0825c9 100644 --- a/frontend/src/lib/utils/postgresConnectionString.ts +++ b/frontend/src/lib/utils/postgresConnectionString.ts @@ -24,9 +24,18 @@ const CONNECTION_STRING = /postgres(?:ql)?:\/\/(?[^:@]+)(?::(?[^@]+))?@(?\[[^\]]+\]|[^:\/?]+)(?::(?\d+))?\/(?[^\?]+)?/ -/** Everything after the first `?`, which is where every query parameter lives. */ -function queryOf(connectionString: string): string { - return connectionString.split('?').slice(1).join('?') +/** + * The query parameters, read the way libpq reads them: names are case-insensitive, and a name + * repeated takes its last value. One reader for both the parser and the allowlist below, or + * they disagree about what a string says — a name only one of them folds is refused by neither + * and honoured by neither. + */ +function paramsOf(connectionString: string): Map { + const query = connectionString.split('?').slice(1).join('?') + const params = new Map() + if (!query) return params + new URLSearchParams(query).forEach((value, name) => params.set(name.toLowerCase(), value)) + return params } /** @@ -63,8 +72,7 @@ export function parsePostgresConnectionString( // By parameter name, never by searching the query text: `sslmode=` also occurs inside // another parameter's *value*, and a substring match there reads someone's // `application_name=sslmode=disable` as a request to turn TLS off. - const query = queryOf(connectionString) - const sslmode = query ? new URLSearchParams(query).get('sslmode') : null + const sslmode = paramsOf(connectionString).get('sslmode') return { user: decode(user), password: password ? decode(password) : undefined, @@ -92,15 +100,10 @@ const COSMETIC_PARAMS = ['application_name'] * other than the one pasted, behind a probe that reports success. */ export function unsupportedConnectionParam(connectionString: string): string | undefined { - const query = queryOf(connectionString) - if (!query) return undefined - let found: string | undefined = undefined - new URLSearchParams(query).forEach((_value, name) => { - const known = REPRESENTABLE_PARAMS.includes(name.toLowerCase()) - const harmless = COSMETIC_PARAMS.includes(name.toLowerCase()) - if (!found && !known && !harmless) found = name - }) - return found + for (const name of paramsOf(connectionString).keys()) { + if (!REPRESENTABLE_PARAMS.includes(name) && !COSMETIC_PARAMS.includes(name)) return name + } + return undefined } /**