From 35d3b0de73cc6f86f817e2c29687411ca3a123ae Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Fri, 16 Jan 2026 03:50:13 +0000 Subject: [PATCH] Implement storage connection testing Added `testConnection` to `StorageService` and `testS3Connection` to `r2-s3.js`. Updated `ProfileModalController` to use the new `storageService.testAccess` method for verifying credentials before saving. The test logic supports both specific bucket checks (via `HeadBucket`) and general credential checks (via `ListBuckets`). --- js/services/storageService.js | 46 ++++++++++++++++++++ js/storage/r2-s3.js | 48 +++++++++++++++++++++ js/ui/profileModalController.js | 75 +++++++++------------------------ 3 files changed, 114 insertions(+), 55 deletions(-) diff --git a/js/services/storageService.js b/js/services/storageService.js index 00c95ad6..03e4cc7e 100644 --- a/js/services/storageService.js +++ b/js/services/storageService.js @@ -1,5 +1,6 @@ import { userLogger } from "../utils/logger.js"; import { bytesToHex } from "../../vendor/crypto-helpers.bundle.min.js"; +import { testS3Connection } from "../storage/r2-s3.js"; const DB_NAME = "bitvid-storage"; const DB_VERSION = 1; @@ -11,6 +12,12 @@ export const PROVIDERS = Object.freeze({ GENERIC: "generic_s3", }); +const PROVIDER_TESTS = { + [PROVIDERS.R2]: testS3Connection, + [PROVIDERS.S3]: testS3Connection, + [PROVIDERS.GENERIC]: testS3Connection, +}; + function hexToBytes(hex) { if (typeof hex !== "string") return new Uint8Array(0); const bytes = new Uint8Array(hex.length / 2); @@ -390,6 +397,45 @@ export class StorageService { await this._saveAccount(account); } + + /** + * Tests a connection access by delegating to the provider's test handler. + * + * @param {string} provider - The provider ID (e.g. "cloudflare_r2") + * @param {object} config - The decrypted configuration object + * @returns {Promise<{success: boolean, message?: string, error?: string}>} + */ + async testAccess(provider, config) { + const handler = PROVIDER_TESTS[provider]; + if (!handler) { + return { success: false, error: `No test handler available for provider: ${provider}` }; + } + + try { + const result = await handler(config); + return { success: true, message: result.message || "Connection verified." }; + } catch (err) { + const cleanError = err.message || "Unknown connection error"; + userLogger.warn(`[StorageService] Test failed for ${provider}:`, err); + return { success: false, error: cleanError }; + } + } + + /** + * Tests a saved connection by ID. + * Requires unlock() to be called first. + * + * @param {string} pubkey + * @param {string} connectionId + */ + async testConnection(pubkey, connectionId) { + const connection = await this.getConnection(pubkey, connectionId); + if (!connection) { + throw new Error("Connection not found."); + } + + return this.testAccess(connection.provider, connection); + } } const storageService = new StorageService(); diff --git a/js/storage/r2-s3.js b/js/storage/r2-s3.js index 9ab51be3..cf73b2c5 100644 --- a/js/storage/r2-s3.js +++ b/js/storage/r2-s3.js @@ -22,6 +22,7 @@ function assignAwsSdk(module) { UploadPartCommand: module.UploadPartCommand, CompleteMultipartUploadCommand: module.CompleteMultipartUploadCommand, AbortMultipartUploadCommand: module.AbortMultipartUploadCommand, + ListBucketsCommand: module.ListBucketsCommand, HeadBucketCommand: module.HeadBucketCommand, PutBucketCorsCommand: module.PutBucketCorsCommand, DeleteObjectCommand: module.DeleteObjectCommand, @@ -60,6 +61,53 @@ async function loadAwsSdkModule() { } } +export async function testS3Connection(config) { + if (!config) { + throw new Error("Configuration required for testing S3 connection."); + } + + await ensureR2SdkLoaded(); + + const { ListBucketsCommand, HeadBucketCommand } = requireAwsSdk(); + + const s3 = makeR2Client({ + accountId: config.accountId, + accessKeyId: config.accessKeyId, + secretAccessKey: config.secretAccessKey, + endpoint: config.endpoint, + region: config.region, + }); + + // If bucket is specified, we check that specific bucket (permissions + existence). + if (config.bucket) { + try { + await s3.send(new HeadBucketCommand({ Bucket: config.bucket })); + return { success: true, message: `Successfully connected to bucket "${config.bucket}".` }; + } catch (error) { + const status = error?.$metadata?.httpStatusCode || 0; + if (status === 403) { + throw new Error(`Access denied to bucket "${config.bucket}". Check your permissions.`); + } else if (status === 404) { + throw new Error(`Bucket "${config.bucket}" does not exist.`); + } + throw error; + } + } + + // Otherwise, we just list buckets to verify credentials. + try { + const response = await s3.send(new ListBucketsCommand({})); + const count = response.Buckets ? response.Buckets.length : 0; + return { success: true, message: `Connection successful. Found ${count} buckets.` }; + } catch (error) { + const status = error?.$metadata?.httpStatusCode || 0; + if (status === 403) { + throw new Error("Access denied. Check your Access Key ID and Secret Access Key."); + } + throw error; + } +} + function requireAwsSdk() { if (awsSdk) { return awsSdk; diff --git a/js/ui/profileModalController.js b/js/ui/profileModalController.js index d6694916..e53309e4 100644 --- a/js/ui/profileModalController.js +++ b/js/ui/profileModalController.js @@ -6108,79 +6108,44 @@ export class ProfileModalController { const pubkey = this.normalizeHexPubkey(this.getActivePubkey()); if (!pubkey) return; - const provider = this.storageProviderInput?.value || "cloudflare_r2"; - // We only support testing R2 for now via r2Service - if (provider !== "cloudflare_r2") { - this.setStorageFormStatus("Test only supported for R2 currently.", "warning"); + const storageService = this.services.storageService; + if (!storageService) { + this.setStorageFormStatus("Storage service unavailable.", "error"); return; } - const accountId = this.storageEndpointInput?.value?.trim() || ""; + const provider = this.storageProviderInput?.value || "cloudflare_r2"; + const endpointOrAccount = this.storageEndpointInput?.value?.trim() || ""; + const region = this.storageRegionInput?.value?.trim() || "auto"; const accessKeyId = this.storageAccessKeyInput?.value?.trim() || ""; const secretAccessKey = this.storageSecretKeyInput?.value?.trim() || ""; const bucket = this.storageBucketInput?.value?.trim() || ""; - const baseDomain = this.storagePrefixInput?.value?.trim() || ""; - if (!accountId || !accessKeyId || !secretAccessKey || !baseDomain) { + if (!accessKeyId || !secretAccessKey || !endpointOrAccount) { this.setStorageFormStatus("Missing credentials for test.", "error"); return; } this.setStorageFormStatus("Testing connection...", "info"); - // Construct settings object expected by r2Service - const settings = { - accountId, - accessKeyId, - secretAccessKey, - baseDomain + const config = { + provider, + accessKeyId, + secretAccessKey, + region, + bucket, }; - // We also need to ensure the bucket name matches what verifyPublicAccess expects? - // r2Service.verifyPublicAccess derives bucket name from npub usually. - // But here we allow custom bucket? - // r2Service.verifyPublicAccess uses `sanitizeBucketName(npub)`. - // If the user entered a custom bucket, r2Service might not support testing it easily - // unless we modify r2Service to accept explicit bucket name. - // r2Service.js: `const bucketName = sanitizeBucketName(npub);` inside verifyPublicAccess. - // This implies r2Service enforces a specific bucket naming convention. - // However, for this generic storage tab, the user might want *any* bucket. - // Ideally r2Service.verifyPublicAccess should accept an optional bucket override. - // For now, I will warn if the user entered a bucket that doesn't match the expectation, - // OR I will trust r2Service to do its thing. - // Actually, r2Service is tightly coupled to the specific bitvid bucket naming scheme. - // The prompt asked for "Bucket" field. - // If I cannot change r2Service right now, I might have to skip the full test - // or rely on r2Service's logic which might fail if bucket doesn't match. - // Let's call verifyPublicAccess and see. It uses `npub` to derive bucket name. - // If I pass the user's manual bucket name, I need `r2Service` to use it. - // Since I can't modify r2Service in this step easily without reading it again (I did read it), - // I recall `verifyPublicAccess` takes `{ settings, npub }`. - // I will try to implement a lightweight test here or just call it. - // But wait, the user provided `bucket`. - // `r2Service.verifyPublicAccess` implementation: - // `const bucketName = sanitizeBucketName(npub);` - // It ignores any bucket in settings? No, it uses it to verify. - // I should probably implement a custom test function or update r2Service later. - // For now, I'll simulate a test or just try to use r2Service and warn about bucket name if it fails. - - // Actually, I can check if `r2Service` has a method to test generic settings? No. - // I'll try to use `verifyPublicAccess` but warn the user that it tests the *default* bucket for their npub. - // Or better, I won't call it if the bucket doesn't match? - // Let's just try calling it. - - // Wait, the prompt said "Implement a Storage tab... Wire UI handlers". - // It didn't force me to rewrite R2Service. - // I'll invoke verifyPublicAccess. + if (provider === "cloudflare_r2") { + config.accountId = endpointOrAccount; + } else { + config.endpoint = endpointOrAccount; + } try { - const result = await this.services.r2Service.verifyPublicAccess({ - settings, - npub: this.services.safeEncodeNpub(pubkey) - }); - + const result = await storageService.testAccess(provider, config); if (result.success) { - this.setStorageFormStatus("Connection Verified!", "success"); + this.setStorageFormStatus(result.message || "Connection Verified!", "success"); } else { this.setStorageFormStatus(`Test Failed: ${result.error}`, "error"); }