diff --git a/src/components/hosts/HostsPage.tsx b/src/components/hosts/HostsPage.tsx index 2339a2bbd..303a09891 100644 --- a/src/components/hosts/HostsPage.tsx +++ b/src/components/hosts/HostsPage.tsx @@ -589,8 +589,7 @@ export default function HostsPage() { const handleSubmit = async (data: ConnectionFormData, secrets: HostFormSecrets) => { try { - const saved = await saveHostFromForm(editing, data, secrets, selectedVaultIds[0] ?? "personal"); - if (!editing && saved) setEditingId(saved.id); + await saveHostFromForm(editing, data, secrets, selectedVaultIds[0] ?? "personal", (created) => setEditingId(created.id)); } catch (err) { reportSaveError(err); } diff --git a/src/components/keychain/KeychainPage.tsx b/src/components/keychain/KeychainPage.tsx index 6ab390f7b..cb9bcd06d 100644 --- a/src/components/keychain/KeychainPage.tsx +++ b/src/components/keychain/KeychainPage.tsx @@ -461,8 +461,9 @@ export default function KeychainPage() { const handleKeySubmit = async (data: SshKeyFormData, privateKey: string | null, publicKey: string | null, passphrase: string | null) => { try { - const key = await saveKeyFromForm(editingKey, data, privateKey, publicKey, passphrase, selectedVaultIds[0] ?? "personal"); - if (!editingKey) setEditingKeyId(key.id); + await saveKeyFromForm( + editingKey, data, privateKey, publicKey, passphrase, selectedVaultIds[0] ?? "personal", (created) => setEditingKeyId(created.id), + ); } catch (err) { reportSaveError(err); } @@ -474,10 +475,10 @@ export default function KeychainPage() { inlineKeyMaterial?: { label?: string; privateKey: string; publicKey: string }, ) => { try { - const identity = await saveIdentityFromForm( + await saveIdentityFromForm( editingIdentity, data, password, inlineKeyMaterial, inlineKeyIdRef, selectedVaultIds[0] ?? "personal", + (created) => setEditingIdentityId(created.id), ); - if (!editingIdentity) setEditingIdentityId(identity.id); } catch (err) { reportSaveError(err); } diff --git a/src/components/mobile/screens/MobileHostEditScreen.tsx b/src/components/mobile/screens/MobileHostEditScreen.tsx index cec7adf51..4d9100836 100644 --- a/src/components/mobile/screens/MobileHostEditScreen.tsx +++ b/src/components/mobile/screens/MobileHostEditScreen.tsx @@ -41,10 +41,7 @@ export default function MobileHostEditScreen({ hostId }: { hostId?: string }) { ref={formRef} hideChrome initial={editing ?? undefined} - onSubmit={async (data, secrets) => { - const saved = await saveHostFromForm(editing, data, secrets, createVaultId); - if (!editing && saved) setEditingId(saved.id); - }} + onSubmit={(data, secrets) => saveHostFromForm(editing, data, secrets, createVaultId, (created) => setEditingId(created.id))} onClose={pop} vaults={[]} /> diff --git a/src/components/mobile/screens/MobileIdentityEditScreen.tsx b/src/components/mobile/screens/MobileIdentityEditScreen.tsx index d0ff08f76..420992722 100644 --- a/src/components/mobile/screens/MobileIdentityEditScreen.tsx +++ b/src/components/mobile/screens/MobileIdentityEditScreen.tsx @@ -37,12 +37,10 @@ export default function MobileIdentityEditScreen({ identityId }: { identityId?: hideChrome initial={editing ?? undefined} flushRef={flushRef} - onSubmit={async (data, password, inlineKeyMaterial) => { - const saved = await saveIdentityFromForm( - editing, data, password, inlineKeyMaterial, inlineKeyIdRef, createVaultId, - ); - if (!editing) setEditingId(saved.id); - }} + onSubmit={(data, password, inlineKeyMaterial) => + saveIdentityFromForm( + editing, data, password, inlineKeyMaterial, inlineKeyIdRef, createVaultId, (created) => setEditingId(created.id), + )} onClose={pop} /> diff --git a/src/components/mobile/screens/MobileKeyEditScreen.tsx b/src/components/mobile/screens/MobileKeyEditScreen.tsx index 29bf953e2..84910834c 100644 --- a/src/components/mobile/screens/MobileKeyEditScreen.tsx +++ b/src/components/mobile/screens/MobileKeyEditScreen.tsx @@ -39,10 +39,8 @@ export default function MobileKeyEditScreen({ keyId, mode }: { keyId?: string; m initial={editing ?? undefined} initialMode={mode} flushRef={flushRef} - onSubmit={async (data, privateKey, publicKey, passphrase) => { - const saved = await saveKeyFromForm(editing, data, privateKey, publicKey, passphrase, createVaultId); - if (!editing) setEditingId(saved.id); - }} + onSubmit={(data, privateKey, publicKey, passphrase) => + saveKeyFromForm(editing, data, privateKey, publicKey, passphrase, createVaultId, (created) => setEditingId(created.id))} onClose={pop} /> diff --git a/src/services/editorMoveSecrets.test.ts b/src/services/editorMoveSecrets.test.ts index a31e7d085..c481acfa3 100644 --- a/src/services/editorMoveSecrets.test.ts +++ b/src/services/editorMoveSecrets.test.ts @@ -70,17 +70,17 @@ const editors = [ { kind: "host", key: "password:c1", save: (vault_id: string, value: string, from = "personal") => - saveHostFromForm({ id: "c1", vault_id: from } as Connection, { tags: [], vault_id }, { ...none, password: value }, "personal"), + saveHostFromForm({ id: "c1", vault_id: from } as Connection, { tags: [], vault_id }, { ...none, password: value }, "personal", () => {}), }, { kind: "key", key: "key:k1:private", save: (vault_id: string, value: string, from = "personal") => - saveKeyFromForm({ id: "k1", vault_id: from } as SshKey, { tags: [], vault_id }, value, null, null, "personal"), + saveKeyFromForm({ id: "k1", vault_id: from } as SshKey, { tags: [], vault_id }, value, null, null, "personal", () => {}), }, { kind: "identity", key: "identity:i1:password", save: (vault_id: string, value: string, from = "personal") => - saveIdentityFromForm({ id: "i1", vault_id: from } as Identity, { vault_id } as never, value, undefined, { current: null }, "personal"), + saveIdentityFromForm({ id: "i1", vault_id: from } as Identity, { vault_id } as never, value, undefined, { current: null }, "personal", () => {}), }, ]; diff --git a/src/services/hostForm.test.ts b/src/services/hostForm.test.ts index b6ce9f026..9aa627f1a 100644 --- a/src/services/hostForm.test.ts +++ b/src/services/hostForm.test.ts @@ -39,7 +39,7 @@ describe("saveHostFromForm", () => { storeSecret.mockImplementation(async (k: string) => { calls.push(`store ${k}`); }); const personal = { id: "c1", vault_id: "personal" } as never; - await saveHostFromForm(personal, { tags: [], vault_id: "v-other" }, { ...none, password: "new" }, "personal"); + await saveHostFromForm(personal, { tags: [], vault_id: "v-other" }, { ...none, password: "new" }, "personal", () => {}); expect(moveWithSecrets).toHaveBeenCalledWith("connection", personal, "v-other", expect.any(Function), [ ["password:c1", "new"], ["key:c1", null], ["passphrase:c1", null], ["proxy_password:c1", null], ["knock_sequence:c1", null], @@ -49,31 +49,49 @@ describe("saveHostFromForm", () => { }); it("stores the proxy password locally", async () => { - await saveHostFromForm(editing, { tags: [] }, { ...none, proxy_password: "pp" }, "personal"); + await saveHostFromForm(editing, { tags: [] }, { ...none, proxy_password: "pp" }, "personal", () => {}); expect(storeSecret).toHaveBeenCalledWith("proxy_password:c1", "pp"); }); it("stores the knock sequence locally", async () => { - await saveHostFromForm(editing, { tags: [] }, { ...none, knock_sequence: "666/tcp" }, "personal"); + await saveHostFromForm(editing, { tags: [] }, { ...none, knock_sequence: "666/tcp" }, "personal", () => {}); expect(storeSecret).toHaveBeenCalledWith("knock_sequence:c1", "666/tcp"); }); it("clears the proxy password when emptied", async () => { - await saveHostFromForm(editing, { tags: [] }, { ...none, proxy_password: "" }, "personal"); + await saveHostFromForm(editing, { tags: [] }, { ...none, proxy_password: "" }, "personal", () => {}); expect(deleteSecret).toHaveBeenCalledWith("proxy_password:c1"); }); it("an edit that clears a password rejects when deleteSecret rejects", async () => { deleteSecret.mockRejectedValueOnce(new Error("withdraw failed")); await expect( - saveHostFromForm(editing, { tags: [] }, { ...none, password: "" }, "personal"), + saveHostFromForm(editing, { tags: [] }, { ...none, password: "" }, "personal", () => {}), ).rejects.toThrow("withdraw failed"); }); it("an upload failure while saving a host form reaches the caller", async () => { storeSecret.mockRejectedValue(new TeamSecretUploadError("password:c1", new Error("403"))); await expect( - saveHostFromForm(editing, { tags: [] }, { ...none, password: "pw" }, "v1"), + saveHostFromForm(editing, { tags: [] }, { ...none, password: "pw" }, "v1", () => {}), ).rejects.toBeInstanceOf(TeamSecretUploadError); }); + + it("a new host whose secret save fails is announced first, so the next save edits it instead of creating another", async () => { + saveConnection.mockResolvedValue({ id: "n1" }); + storeSecret.mockRejectedValue(new TeamSecretUploadError("password:n1", new Error("403"))); + const created = vi.fn(); + + await expect( + saveHostFromForm(null, { tags: [] }, { ...none, password: "pw" }, "team-1", created), + ).rejects.toBeInstanceOf(TeamSecretUploadError); + + expect(created).toHaveBeenCalledWith({ id: "n1" }); + }); + + it("an edit announces nothing", async () => { + const created = vi.fn(); + await saveHostFromForm(editing, { tags: [] }, none, "personal", created); + expect(created).not.toHaveBeenCalled(); + }); }); diff --git a/src/services/hostForm.ts b/src/services/hostForm.ts index 1f20b219c..776259e65 100644 --- a/src/services/hostForm.ts +++ b/src/services/hostForm.ts @@ -1,7 +1,7 @@ import type { Connection, ConnectionFormData } from "@/types"; import { useConnectionStore } from "@/stores/connectionStore"; import { CONNECTION_SECRET_FIELDS, CONNECTION_SECRET_KEYS, type ConnectionSecretField } from "@/services/teamVaultSecretKeys"; -import { moveWithSecrets, storeNewSecrets, type SecretEdit } from "@/services/vaultObjectSecrets"; +import { moveWithSecrets, createWithSecrets, type SecretEdit } from "@/services/vaultObjectSecrets"; export type HostFormSecrets = Record; @@ -17,13 +17,15 @@ export async function saveHostFromForm( data: ConnectionFormData, secrets: HostFormSecrets, fallbackVaultId: string, -): Promise { + onCreated: (created: Connection) => void, +): Promise { const { updateConnection, saveConnection } = useConnectionStore.getState(); if (editing) { - await moveWithSecrets("connection", editing, data.vault_id, () => updateConnection(editing.id, data), secretEdits(editing.id, secrets)); - return editing; + return moveWithSecrets("connection", editing, data.vault_id, () => updateConnection(editing.id, data), secretEdits(editing.id, secrets)); } - const conn = await saveConnection({ ...data, vault_id: data.vault_id ?? fallbackVaultId }); - if (conn) await storeNewSecrets(secretEdits(conn.id, secrets)); - return conn ?? null; + await createWithSecrets( + () => saveConnection({ ...data, vault_id: data.vault_id ?? fallbackVaultId }), + (id) => secretEdits(id, secrets), + onCreated, + ); } diff --git a/src/services/keychainForm.test.ts b/src/services/keychainForm.test.ts index 882f683dd..87cf78f02 100644 --- a/src/services/keychainForm.test.ts +++ b/src/services/keychainForm.test.ts @@ -47,7 +47,7 @@ beforeEach(() => { describe("editing hands the edited fields to the move", () => { test("a key passes its halves, written after a same-location update", async () => { - await saveKeyFromForm(existing, { tags: [], vault_id: "v-other" }, null, null, "pass", "personal"); + await saveKeyFromForm(existing, { tags: [], vault_id: "v-other" }, null, null, "pass", "personal", () => {}); expect(h.moveWithSecrets).toHaveBeenCalledWith("key", existing, "v-other", expect.any(Function), [ ["key:k1:private", null], ["key:k1:public", null], ["key:k1:passphrase", "pass"], ]); @@ -57,7 +57,7 @@ describe("editing hands the edited fields to the move", () => { test("an identity passes its password, written after a same-location update", async () => { const identity = { id: "i1", vault_id: "v-other" } as Identity; - await saveIdentityFromForm(identity, { vault_id: "personal" } as never, "pw", undefined, { current: null }, "personal"); + await saveIdentityFromForm(identity, { vault_id: "personal" } as never, "pw", undefined, { current: null }, "personal", () => {}); expect(h.moveWithSecrets).toHaveBeenCalledWith("identity", identity, "personal", expect.any(Function), [ ["identity:i1:password", "pw"], ]); @@ -68,7 +68,7 @@ describe("editing hands the edited fields to the move", () => { describe("saveKeyFromForm", () => { test("creates the key, then stores each half under its own secret name", async () => { - await saveKeyFromForm(null, { tags: [] }, "PRIV", "PUB", "pass", "personal"); + await saveKeyFromForm(null, { tags: [] }, "PRIV", "PUB", "pass", "personal", () => {}); expect(saveKey).toHaveBeenCalledWith({ tags: [], vault_id: "personal" }); expect(storeSecret.mock.calls).toEqual([ ["key:new:private", "PRIV"], @@ -79,14 +79,14 @@ describe("saveKeyFromForm", () => { }); test("a null half is left untouched, an emptied one is deleted on edit", async () => { - await saveKeyFromForm(existing, { tags: [] }, null, "", "pass", "personal"); + await saveKeyFromForm(existing, { tags: [] }, null, "", "pass", "personal", () => {}); expect(updateKey).toHaveBeenCalledWith("k1", { tags: [] }); expect(storeSecret.mock.calls).toEqual([["key:k1:passphrase", "pass"]]); expect(deleteSecret.mock.calls).toEqual([["key:k1:public"]]); }); test("an empty half on create writes nothing rather than deleting", async () => { - await saveKeyFromForm(null, { tags: [] }, "PRIV", "", "", "personal"); + await saveKeyFromForm(null, { tags: [] }, "PRIV", "", "", "personal", () => {}); expect(storeSecret.mock.calls).toEqual([["key:new:private", "PRIV"]]); expect(deleteSecret).not.toHaveBeenCalled(); }); @@ -95,11 +95,42 @@ describe("saveKeyFromForm", () => { describe("saveIdentityFromForm", () => { test("an inline key is filed in the identity's vault, not the personal one", async () => { h.saveIdentity.mockResolvedValue({ id: "i-new" }); - await saveIdentityFromForm(null, { username: "root", tags: [] }, null, { privateKey: "PRIV", publicKey: "" }, { current: null }, "team-1"); + await saveIdentityFromForm(null, { username: "root", tags: [] }, null, { privateKey: "PRIV", publicKey: "" }, { current: null }, "team-1", () => {}); expect(saveKey).toHaveBeenCalledWith(expect.objectContaining({ vault_id: "team-1" })); }); }); +describe("a new object whose secret save fails", () => { + const refused = new TeamSecretUploadError("k", new Error("403")); + + test("a key is announced first, so the next save edits it instead of creating another", async () => { + h.storeSecret.mockRejectedValue(refused); + const created = vi.fn(); + + await expect(saveKeyFromForm(null, { tags: [] }, "PRIV", null, null, "team-1", created)).rejects.toBe(refused); + + expect(created).toHaveBeenCalledWith({ id: "new", vault_id: "personal" }); + }); + + test("an identity is announced first too", async () => { + h.saveIdentity.mockResolvedValue({ id: "i-new" }); + h.storeSecret.mockRejectedValue(refused); + const created = vi.fn(); + + await expect( + saveIdentityFromForm(null, { username: "root", tags: [] }, "pw", undefined, { current: null }, "team-1", created), + ).rejects.toBe(refused); + + expect(created).toHaveBeenCalledWith({ id: "i-new" }); + }); + + test("an edit announces nothing", async () => { + const created = vi.fn(); + await saveKeyFromForm(existing, { tags: [] }, null, null, null, "personal", created); + expect(created).not.toHaveBeenCalled(); + }); +}); + describe("unlinkIdentityFromHost", () => { const identity = { id: "i1", username: "root", key_id: "k1" } as Identity; const host = { id: "c1", name: "web", host: "h", port: 22, tags: [] } as never; diff --git a/src/services/keychainForm.ts b/src/services/keychainForm.ts index d80b02826..60147a09d 100644 --- a/src/services/keychainForm.ts +++ b/src/services/keychainForm.ts @@ -3,7 +3,7 @@ import { useIdentityStore } from "@/stores/identityStore"; import { connectionToFormData } from "@/stores/connectionStore"; import { storeSecret, getSecret } from "@/services/vault"; import { reportUploadFailure } from "@/services/secretRouting"; -import { moveWithSecrets, storeNewSecrets, type SecretEdit } from "@/services/vaultObjectSecrets"; +import { moveWithSecrets, createWithSecrets, type SecretEdit } from "@/services/vaultObjectSecrets"; import type { AuthType, Connection, ConnectionFormData, Identity, IdentityFormData, SshKey, SshKeyFormData } from "@/types"; type InlineKeyMaterial = { label?: string; privateKey: string; publicKey: string }; @@ -15,20 +15,16 @@ export async function saveKeyFromForm( publicKey: string | null, passphrase: string | null, fallbackVaultId: string, -): Promise { + onCreated: (created: SshKey) => void, +): Promise { const { saveKey, updateKey } = useKeyStore.getState(); const edits = (id: string): SecretEdit[] => [ [`key:${id}:private`, privateKey], [`key:${id}:public`, publicKey], [`key:${id}:passphrase`, passphrase], ]; - if (editing) { - await moveWithSecrets("key", editing, data.vault_id, () => updateKey(editing.id, data), edits(editing.id)); - return editing; - } - const key = await saveKey({ ...data, vault_id: data.vault_id ?? fallbackVaultId }); - await storeNewSecrets(edits(key.id)); - return key; + if (editing) return moveWithSecrets("key", editing, data.vault_id, () => updateKey(editing.id, data), edits(editing.id)); + await createWithSecrets(() => saveKey({ ...data, vault_id: data.vault_id ?? fallbackVaultId }), edits, onCreated); } /** `inlineKeyId` carries the key a previous autosave pass created, so repeated @@ -40,7 +36,8 @@ export async function saveIdentityFromForm( inlineKeyMaterial: InlineKeyMaterial | undefined, inlineKeyId: { current: string | null }, fallbackVaultId: string, -): Promise { + onCreated: (created: Identity) => void, +): Promise { const { saveIdentity, updateIdentity } = useIdentityStore.getState(); let resolvedData = data; @@ -57,12 +54,13 @@ export async function saveIdentityFromForm( const edits = (id: string): SecretEdit[] => [[`identity:${id}:password`, password]]; if (editing) { - await moveWithSecrets("identity", editing, resolvedData.vault_id, () => updateIdentity(editing.id, resolvedData), edits(editing.id)); - return editing; + return moveWithSecrets("identity", editing, resolvedData.vault_id, () => updateIdentity(editing.id, resolvedData), edits(editing.id)); } - const identity = await saveIdentity({ ...resolvedData, vault_id: resolvedData.vault_id ?? fallbackVaultId }); - await storeNewSecrets(edits(identity.id)); - return identity; + await createWithSecrets( + () => saveIdentity({ ...resolvedData, vault_id: resolvedData.vault_id ?? fallbackVaultId }), + edits, + onCreated, + ); } export async function unlinkIdentityFromHost( diff --git a/src/services/vaultObjectSecrets.ts b/src/services/vaultObjectSecrets.ts index 5c6f10781..6057294e6 100644 --- a/src/services/vaultObjectSecrets.ts +++ b/src/services/vaultObjectSecrets.ts @@ -96,7 +96,17 @@ async function applyEdits( } } -export const storeNewSecrets = (edits: readonly SecretEdit[]) => applyEdits(edits, storeSecret, async () => {}); +export async function createWithSecrets( + create: () => Promise, + edits: (id: string) => readonly SecretEdit[], + onCreated: (created: T) => void, +): Promise { + const created = await create(); + if (!created) return; + // Announced before its secrets: a caller that never learns the id creates the object again on its next save. + onCreated(created); + await applyEdits(edits(created.id), storeSecret, async () => {}); +} export async function moveWithSecrets( kind: SecretObjectKind,