diff --git a/js/ui/profileModal/ProfileStorageController.js b/js/ui/profileModal/ProfileStorageController.js index 000c6b9c..6b8f388e 100644 --- a/js/ui/profileModal/ProfileStorageController.js +++ b/js/ui/profileModal/ProfileStorageController.js @@ -9,6 +9,12 @@ import { getActiveSigner } from "../../nostr/client.js"; import { DEFAULT_NIP07_ENCRYPTION_METHODS } from "../../nostr/nip07Permissions.js"; import { storageSyncService } from "../../services/storageSyncService.js"; import { StorageCorsHelp } from "./storageCorsHelp.js"; +import { + fillStorageForm, + clearCredentialFields, + findProviderConnection, + saveProviderConnection, +} from "./storageConnections.js"; export class ProfileStorageController { constructor(mainController) { @@ -111,7 +117,7 @@ export class ProfileStorageController { if (this.storageProviderInput instanceof HTMLElement) { this.storageProviderInput.addEventListener("change", () => { - this.updateStorageFormVisibility(); + void this.handleProviderChange(); }); } @@ -212,7 +218,7 @@ export class ProfileStorageController { targetConn.id ); if (conn) { - this.fillStorageForm(conn); + fillStorageForm(this, conn); if (this.storageStatusText) { const label = conn.meta?.label || conn.provider || "S3"; this.storageStatusText.textContent = `Unlocked (${label})`; @@ -355,57 +361,32 @@ export class ProfileStorageController { } } - fillStorageForm(conn) { - if (!conn) return; - const { - provider, - accessKeyId, - secretAccessKey, - accountId: payloadAccountId, - endpoint: payloadEndpoint, - forcePathStyle: payloadForcePathStyle, - } = conn; - const { - endpoint, - region, - bucket, - prefix, - defaultForUploads, - accountId, - forcePathStyle: metaForcePathStyle, - } = conn.meta || {}; - - if (this.storageProviderInput) - this.storageProviderInput.value = provider || "cloudflare_r2"; - - const resolvedEndpoint = - endpoint || accountId || payloadAccountId || payloadEndpoint || ""; - - if (this.storageEndpointInput) - this.storageEndpointInput.value = resolvedEndpoint; - if (this.storageRegionInput) - this.storageRegionInput.value = region || "auto"; - if (this.storageAccessKeyInput) - this.storageAccessKeyInput.value = accessKeyId || ""; - if (this.storageSecretKeyInput) - this.storageSecretKeyInput.value = secretAccessKey || ""; - if (this.storageBucketInput) this.storageBucketInput.value = bucket || ""; - if (this.storagePrefixInput) this.storagePrefixInput.value = prefix || ""; - if (this.storageDefaultInput) - this.storageDefaultInput.checked = !!defaultForUploads; - - if (this.storageForcePathStyleInput) { - if (typeof payloadForcePathStyle === "boolean") { - this.storageForcePathStyleInput.checked = payloadForcePathStyle; - } else if (typeof metaForcePathStyle === "boolean") { - this.storageForcePathStyleInput.checked = metaForcePathStyle; - } else { - this.storageForcePathStyleInput.checked = true; - } - } - + // Load the saved connection for the newly-selected provider so each provider keeps + // its own credentials; clear the credential fields when that provider has none yet. + async handleProviderChange() { this.updateStorageFormVisibility(); - this.handlePublicUrlInput(); + const pubkey = this.mainController.normalizeHexPubkey( + this.mainController.getActivePubkey(), + ); + const storageService = this.mainController.services?.storageService; + if (!pubkey || !storageService || !storageService.isUnlocked?.(pubkey)) { + return; + } + const provider = this.storageProviderInput?.value || "cloudflare_r2"; + try { + const connections = await storageService.listConnections(pubkey); + const match = findProviderConnection(connections, provider); + if (match) { + const conn = await storageService.getConnection(pubkey, match.id); + if (conn) { + fillStorageForm(this, conn); + return; + } + } + clearCredentialFields(this); + } catch (error) { + devLogger.warn("[ProfileModal] Failed to load connection for provider:", error); + } } handlePublicUrlInput() { @@ -858,7 +839,13 @@ export class ProfileStorageController { this.setStorageFormStatus("Saving...", "info"); try { - await storageService.saveConnection(pubkey, "default", payload, meta); + // One slot per provider type so providers don't overwrite or clash. + await saveProviderConnection(storageService, pubkey, { + provider, + payload, + meta, + isDefault, + }); this.setStorageFormStatus("Connection saved.", "success"); this.mainController.showSuccess("Storage connection saved."); diff --git a/js/ui/profileModal/storageConnections.js b/js/ui/profileModal/storageConnections.js new file mode 100644 index 00000000..b3aa62ba --- /dev/null +++ b/js/ui/profileModal/storageConnections.js @@ -0,0 +1,126 @@ +// Per-provider connection model for the Storage pane. +// +// Each provider type (Cloudflare R2 / Backblaze B2 / Custom S3) gets its OWN connection +// slot, keyed by the provider id, so their credentials don't overwrite or clash. The +// underlying storageService already stores a map of connections + syncs the whole +// account record, so this just stops the UI from collapsing everything into one slot. + +export function connectionProvider(conn) { + return (conn && (conn.provider || conn.meta?.provider)) || ""; +} + +export function findProviderConnection(connections, provider) { + return ( + (Array.isArray(connections) ? connections : []).find( + (c) => connectionProvider(c) === provider, + ) || null + ); +} + +// Keep a sane active upload target: default when the user asked, when it's the first +// connection saved, or when re-saving the provider that was already the default. +export function computeDefaultForUploads({ isDefault, connections, provider }) { + if (isDefault) { + return true; + } + if (!Array.isArray(connections) || connections.length === 0) { + return true; + } + return ( + findProviderConnection(connections, provider)?.meta?.defaultForUploads === true + ); +} + +// Other-id connections of the same provider (e.g. the legacy shared "default" slot) +// that should be removed after saving so they can't clash with the per-provider slot. +export function legacyDuplicateIds(connections, provider, keepId) { + return (Array.isArray(connections) ? connections : []) + .filter((c) => c.id !== keepId && connectionProvider(c) === provider) + .map((c) => c.id); +} + +// Persist a connection under its per-provider slot: pick a sane default-for-uploads, +// save, then remove any legacy/duplicate same-provider connection so nothing clashes. +// Returns the connection id used. Shared by the controller + tests (one source of truth). +export async function saveProviderConnection( + storageService, + pubkey, + { provider, payload, meta = {}, isDefault = false } = {}, +) { + const existing = await storageService.listConnections(pubkey); + meta.defaultForUploads = computeDefaultForUploads({ + isDefault, + connections: existing, + provider, + }); + await storageService.saveConnection(pubkey, provider, payload, meta); + for (const id of legacyDuplicateIds(existing, provider, provider)) { + await storageService.deleteConnection(pubkey, id); + } + return provider; +} + +// Fill the Storage form inputs from a saved connection. `c` is the controller (used for +// its cached input refs + the visibility refresh) — keeps all the element wiring in one +// place without threading a dozen refs through. +export function fillStorageForm(c, conn) { + if (!conn) { + return; + } + const { + provider, + accessKeyId, + secretAccessKey, + accountId: payloadAccountId, + endpoint: payloadEndpoint, + forcePathStyle: payloadForcePathStyle, + } = conn; + const { + endpoint, + region, + bucket, + prefix, + defaultForUploads, + accountId, + forcePathStyle: metaForcePathStyle, + } = conn.meta || {}; + + if (c.storageProviderInput) { + c.storageProviderInput.value = provider || "cloudflare_r2"; + } + const resolvedEndpoint = + endpoint || accountId || payloadAccountId || payloadEndpoint || ""; + if (c.storageEndpointInput) c.storageEndpointInput.value = resolvedEndpoint; + if (c.storageRegionInput) c.storageRegionInput.value = region || "auto"; + if (c.storageAccessKeyInput) c.storageAccessKeyInput.value = accessKeyId || ""; + if (c.storageSecretKeyInput) c.storageSecretKeyInput.value = secretAccessKey || ""; + if (c.storageBucketInput) c.storageBucketInput.value = bucket || ""; + if (c.storagePrefixInput) c.storagePrefixInput.value = prefix || ""; + if (c.storageDefaultInput) c.storageDefaultInput.checked = !!defaultForUploads; + + if (c.storageForcePathStyleInput) { + if (typeof payloadForcePathStyle === "boolean") { + c.storageForcePathStyleInput.checked = payloadForcePathStyle; + } else if (typeof metaForcePathStyle === "boolean") { + c.storageForcePathStyleInput.checked = metaForcePathStyle; + } else { + c.storageForcePathStyleInput.checked = true; + } + } + + c.updateStorageFormVisibility(); + c.handlePublicUrlInput(); +} + +// Clear the credential inputs but keep the selected provider (used when switching to a +// provider that has no saved connection yet). +export function clearCredentialFields(c) { + if (c.storageEndpointInput) c.storageEndpointInput.value = ""; + if (c.storageRegionInput) c.storageRegionInput.value = "auto"; + if (c.storageAccessKeyInput) c.storageAccessKeyInput.value = ""; + if (c.storageSecretKeyInput) c.storageSecretKeyInput.value = ""; + if (c.storageBucketInput) c.storageBucketInput.value = ""; + if (c.storagePrefixInput) c.storagePrefixInput.value = ""; + if (c.storageDefaultInput) c.storageDefaultInput.checked = false; + c.updateStorageFormVisibility(); +} diff --git a/tests/storage-connections.test.mjs b/tests/storage-connections.test.mjs new file mode 100644 index 00000000..8ccb3934 --- /dev/null +++ b/tests/storage-connections.test.mjs @@ -0,0 +1,185 @@ +// Per-provider storage connections: each provider type (R2 / B2 / Custom S3) must keep +// its OWN credentials so saving one never overwrites another, and the saved set syncs +// as a whole. The UI used to collapse everything into one shared "default" slot. + +import { test, describe, beforeEach } from "node:test"; +import assert from "node:assert"; +import * as fakeIndexedDB from "fake-indexeddb"; + +if (!globalThis.indexedDB) { + globalThis.indexedDB = fakeIndexedDB.indexedDB; + globalThis.IDBKeyRange = fakeIndexedDB.IDBKeyRange; +} + +import storageService from "../js/services/storageService.js"; +import { + connectionProvider, + findProviderConnection, + computeDefaultForUploads, + legacyDuplicateIds, + saveProviderConnection, +} from "../js/ui/profileModal/storageConnections.js"; + +// --- Pure decision logic --- + +describe("per-provider connection helpers", () => { + test("computeDefaultForUploads: explicit choice, first-ever, and 'was already default'", () => { + // Explicit checkbox always wins. + assert.equal( + computeDefaultForUploads({ isDefault: true, connections: [{ id: "x" }], provider: "backblaze_b2" }), + true, + ); + // The very first connection becomes the default automatically. + assert.equal( + computeDefaultForUploads({ isDefault: false, connections: [], provider: "backblaze_b2" }), + true, + ); + // Re-saving the provider that was already the default keeps it default... + assert.equal( + computeDefaultForUploads({ + isDefault: false, + connections: [{ id: "cloudflare_r2", provider: "cloudflare_r2", meta: { defaultForUploads: true } }], + provider: "cloudflare_r2", + }), + true, + ); + // ...but a non-default provider stays non-default (won't steal it). + assert.equal( + computeDefaultForUploads({ + isDefault: false, + connections: [ + { id: "cloudflare_r2", provider: "cloudflare_r2", meta: { defaultForUploads: true } }, + { id: "backblaze_b2", provider: "backblaze_b2", meta: { defaultForUploads: false } }, + ], + provider: "backblaze_b2", + }), + false, + ); + }); + + test("findProviderConnection matches on provider (payload or meta)", () => { + const conns = [ + { id: "a", provider: "cloudflare_r2" }, + { id: "b", meta: { provider: "backblaze_b2" } }, + ]; + assert.equal(findProviderConnection(conns, "backblaze_b2").id, "b"); + assert.equal(findProviderConnection(conns, "generic_s3"), null); + assert.equal(connectionProvider(conns[1]), "backblaze_b2"); + }); + + test("legacyDuplicateIds flags same-provider connections stored under a different id", () => { + const conns = [ + { id: "default", provider: "cloudflare_r2" }, // the legacy shared slot + { id: "cloudflare_r2", provider: "cloudflare_r2" }, // the new per-provider slot + { id: "backblaze_b2", provider: "backblaze_b2" }, + ]; + assert.deepEqual(legacyDuplicateIds(conns, "cloudflare_r2", "cloudflare_r2"), ["default"]); + assert.deepEqual(legacyDuplicateIds(conns, "backblaze_b2", "backblaze_b2"), []); + }); +}); + +// --- Integration against the real storageService (fake-indexeddb) --- + +describe("saving multiple providers does not clash", () => { + const pubkey = "00".repeat(31) + "07"; + const signer = { + nip44Encrypt: async (_pk, pt) => `enc_${pt}`, + nip44Decrypt: async (_pk, ct) => ct.replace(/^enc_/, ""), + }; + + beforeEach(async () => { + storageService.masterKeys.clear(); + if (storageService.db) { + storageService.db.close(); + storageService.db = null; + storageService.dbPromise = null; + } + await new Promise((resolve) => { + const req = globalThis.indexedDB.deleteDatabase("bitvid-storage"); + req.onsuccess = resolve; + req.onerror = resolve; + req.onblocked = resolve; + }); + await storageService.unlock(pubkey, { signer }); + }); + + const r2Payload = { provider: "cloudflare_r2", accessKeyId: "r2k", secretAccessKey: "r2s", accountId: "acct" }; + const b2Payload = { provider: "backblaze_b2", accessKeyId: "b2k", secretAccessKey: "b2s" }; + + test("R2 then B2 both persist under their own provider ids; only one is default", async () => { + await saveProviderConnection(storageService, pubkey, { + provider: "cloudflare_r2", + payload: r2Payload, + meta: { provider: "cloudflare_r2", bucket: "r2-bucket" }, + isDefault: true, + }); + await saveProviderConnection(storageService, pubkey, { + provider: "backblaze_b2", + payload: b2Payload, + meta: { provider: "backblaze_b2", bucket: "b2-bucket", region: "us-west-004" }, + isDefault: false, + }); + + const conns = await storageService.listConnections(pubkey); + const ids = conns.map((c) => c.id).sort(); + assert.deepEqual(ids, ["backblaze_b2", "cloudflare_r2"], "both providers stored, no overwrite"); + + const r2 = await storageService.getConnection(pubkey, "cloudflare_r2"); + const b2 = await storageService.getConnection(pubkey, "backblaze_b2"); + assert.equal(r2.meta.bucket, "r2-bucket"); + assert.equal(b2.meta.bucket, "b2-bucket"); + // Credentials are independent (B2 did not clobber R2). + assert.equal(r2.accessKeyId, "r2k"); + assert.equal(b2.accessKeyId, "b2k"); + // Exactly one default-for-uploads, and it's the one the user chose. + assert.equal(conns.filter((c) => c.meta?.defaultForUploads).length, 1); + assert.equal(r2.meta.defaultForUploads, true); + assert.equal(b2.meta.defaultForUploads, false); + }); + + test("re-saving B2 as the default moves the active target off R2 (still only one default)", async () => { + await saveProviderConnection(storageService, pubkey, { provider: "cloudflare_r2", payload: r2Payload, meta: { provider: "cloudflare_r2" }, isDefault: true }); + await saveProviderConnection(storageService, pubkey, { provider: "backblaze_b2", payload: b2Payload, meta: { provider: "backblaze_b2" }, isDefault: false }); + await saveProviderConnection(storageService, pubkey, { provider: "backblaze_b2", payload: b2Payload, meta: { provider: "backblaze_b2" }, isDefault: true }); + + const conns = await storageService.listConnections(pubkey); + assert.equal(conns.filter((c) => c.meta?.defaultForUploads).length, 1); + assert.equal((await storageService.getConnection(pubkey, "backblaze_b2")).meta.defaultForUploads, true); + assert.equal((await storageService.getConnection(pubkey, "cloudflare_r2")).meta.defaultForUploads, false); + }); + + test("a legacy shared 'default' slot is migrated to its per-provider id (no duplicate)", async () => { + // Simulate the old UI: an R2 connection under the shared "default" id. + await storageService.saveConnection(pubkey, "default", r2Payload, { + provider: "cloudflare_r2", + bucket: "legacy", + defaultForUploads: true, + }); + + // Re-saving R2 through the new path should re-key it and remove the legacy slot. + await saveProviderConnection(storageService, pubkey, { + provider: "cloudflare_r2", + payload: r2Payload, + meta: { provider: "cloudflare_r2", bucket: "legacy" }, + isDefault: false, + }); + + const ids = (await storageService.listConnections(pubkey)).map((c) => c.id); + assert.ok(ids.includes("cloudflare_r2")); + assert.ok(!ids.includes("default"), "legacy 'default' slot removed"); + // Default-for-uploads carried over from the migrated connection. + assert.equal((await storageService.getConnection(pubkey, "cloudflare_r2")).meta.defaultForUploads, true); + }); + + test("both connections are present in the exported account record (so sync carries them)", async () => { + await saveProviderConnection(storageService, pubkey, { provider: "cloudflare_r2", payload: r2Payload, meta: { provider: "cloudflare_r2" }, isDefault: true }); + await saveProviderConnection(storageService, pubkey, { provider: "backblaze_b2", payload: b2Payload, meta: { provider: "backblaze_b2" }, isDefault: false }); + + const record = await storageService.exportAccountRecord(pubkey); + assert.deepEqual( + Object.keys(record.connections).sort(), + ["backblaze_b2", "cloudflare_r2"], + "the synced record contains every provider's connection", + ); + }); +}); diff --git a/todo/TODO_2026-06-20_pre-launch.md b/todo/TODO_2026-06-20_pre-launch.md index e2e52fcc..66d37b13 100644 --- a/todo/TODO_2026-06-20_pre-launch.md +++ b/todo/TODO_2026-06-20_pre-launch.md @@ -886,6 +886,21 @@ Reported 2026-06-25. Relates to #17 (NIP-71 interop) / the bitvid→NIP-71 mirro - [ ] **VERIFY live** on unstable: pick Backblaze B2, enter region (`us-west-004`) + bucket + key/secret, Test → passes; apply the CORS helper's rules; upload a video → plays from `https://.s3..backblazeb2.com/...`. +- [x] **Per-provider connections (no overwrite/clash) — DONE 2026-06-30.** The UI saved + every provider into a single shared `"default"` connection id, so configuring B2 + overwrote R2 (etc.). The service layer + upload path + sync already supported a + MAP of connections keyed by id (`listConnections`/`defaultForUploads`/ + `exportAccountRecord`) — only the controller collapsed them. Now each provider type + gets its own slot keyed by provider id (`cloudflare_r2`/`backblaze_b2`/`generic_s3`): + save writes that slot (preserving a single active `defaultForUploads`, and lazily + migrating the legacy `"default"` slot to its provider id), and switching the provider + dropdown loads that provider's saved credentials (or clears the fields for a fresh + entry). Sync already carries the whole record, so all providers sync together. + Logic extracted to `js/ui/profileModal/storageConnections.js` (`saveProviderConnection` + + pure helpers; keeps the controller under the 1000-line cap). Tests: + `tests/storage-connections.test.mjs` (7 — pure default/dup logic + integration: + R2+B2 coexist with one default, re-default switch, legacy-`default` migration, and + the export carries both). Storage + profile-modal suites 69/69; build + lint clean. ### 39. Image uploads should prefer uploading to configured storage (UX) - [ ] Anywhere the user adds an image, prefer letting them **upload an image file to