Merge pull request #1255 from PR0M3TH3AN/feature/storage-connection-test-16098191007604609215

Implement Storage Connection Testing

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();

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;

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");
         }