Merge pull request #1637 from PR0M3TH3AN/codex/add-s3-service-validation-and-configuration

Introduce generic S3 helper and wire non‑R2 uploads to standard S3 client

js/services/r2Service.js

@@ -40,6 +40,7 @@ import {
   ensureBucketExists,
   deleteObject,
 } from "../storage/s3-multipart.js";
+import { ensureS3SdkLoaded, makeS3Client } from "../storage/s3-client.js";
 import { truncateMiddle } from "../utils/formatters.js";
 import { userLogger } from "../utils/logger.js";
 import {
@@ -406,16 +407,31 @@ class R2Service {
                 target.id
               );
               if (details) {
+                const provider =
+                  details.provider || details.meta?.provider || "";
+                const publicBaseUrl =
+                  details.meta?.publicBaseUrl ||
+                  details.meta?.baseDomain ||
+                  details.meta?.publicUrl ||
+                  "";
+                const forcePathStyle =
+                  typeof details.forcePathStyle === "boolean"
+                    ? details.forcePathStyle
+                    : typeof details.meta?.forcePathStyle === "boolean"
+                    ? details.meta.forcePathStyle
+                    : undefined;
                 // Map to R2 settings format
                 return {
+                  provider,
                   accountId: details.accountId || details.meta?.accountId || "",
                   endpoint: details.endpoint || details.meta?.endpoint || "",
                   bucket: details.bucket || details.meta?.bucket || "",
                   region: details.region || details.meta?.region || "auto",
                   accessKeyId: details.accessKeyId || "",
                   secretAccessKey: details.secretAccessKey || "",
-                  baseDomain:
-                    details.meta?.baseDomain || details.meta?.publicUrl || "",
+                  baseDomain: publicBaseUrl,
+                  publicBaseUrl,
+                  forcePathStyle,
                   isLegacy: false,
                 };
               }
@@ -1069,11 +1085,13 @@ class R2Service {
 
   async uploadFile({
     file,
+    provider,
     accountId,
     accessKeyId,
     secretAccessKey,
     endpoint,
     region,
+    forcePathStyle,
     bucket,
     key,
     onProgress,
@@ -1089,13 +1107,27 @@ class R2Service {
       throw new Error("Missing required parameters for uploadFile");
     }
 
-    const s3 = makeR2Client({
-      accountId,
-      accessKeyId,
-      secretAccessKey,
-      endpoint,
-      region,
-    });
+    let s3 = null;
+    const useR2 = provider === "cloudflare_r2" || Boolean(accountId);
+
+    if (useR2) {
+      s3 = makeR2Client({
+        accountId,
+        accessKeyId,
+        secretAccessKey,
+        endpoint,
+        region,
+      });
+    } else {
+      await ensureS3SdkLoaded();
+      s3 = makeS3Client({
+        endpoint,
+        region,
+        accessKeyId,
+        secretAccessKey,
+        forcePathStyle: Boolean(forcePathStyle),
+      });
+    }
 
     await multipartUpload({
       s3,

js/services/s3Service.js

@@ -0,0 +1,172 @@
+import { ensureS3SdkLoaded, makeS3Client } from "../storage/s3-client.js";
+import { ensureBucketCors, ensureBucketExists } from "../storage/s3-multipart.js";
+import { userLogger } from "../utils/logger.js";
+
+function normalizeEndpoint(endpoint) {
+  const trimmed = String(endpoint || "").trim();
+  if (!trimmed) {
+    return "";
+  }
+  if (/^https?:\/\//i.test(trimmed)) {
+    return trimmed.replace(/\/+$/, "");
+  }
+  return `https://${trimmed.replace(/\/+$/, "")}`;
+}
+
+function normalizeRegion(region) {
+  const trimmed = String(region || "").trim();
+  return trimmed || "auto";
+}
+
+function normalizeBucket(bucket) {
+  return String(bucket || "").trim();
+}
+
+export function derivePublicBaseUrl({
+  endpoint,
+  bucket,
+  forcePathStyle,
+} = {}) {
+  const normalizedEndpoint = normalizeEndpoint(endpoint);
+  const normalizedBucket = normalizeBucket(bucket);
+  if (!normalizedEndpoint || !normalizedBucket) {
+    return "";
+  }
+
+  const url = new URL(normalizedEndpoint);
+  url.hash = "";
+  url.search = "";
+
+  if (forcePathStyle) {
+    const basePath = url.pathname.replace(/\/+$/, "");
+    const prefix = basePath === "/" ? "" : basePath;
+    url.pathname = `${prefix}/${normalizedBucket}`;
+  } else if (!url.hostname.startsWith(`${normalizedBucket}.`)) {
+    url.hostname = `${normalizedBucket}.${url.hostname}`;
+  }
+
+  return url.toString().replace(/\/+$/, "");
+}
+
+export function validateS3Connection({
+  endpoint,
+  region,
+  accessKeyId,
+  secretAccessKey,
+  bucket,
+  forcePathStyle,
+} = {}) {
+  const normalizedEndpoint = normalizeEndpoint(endpoint);
+  const normalizedBucket = normalizeBucket(bucket);
+  const normalizedRegion = normalizeRegion(region);
+  const normalizedAccessKeyId = String(accessKeyId || "").trim();
+  const normalizedSecretAccessKey = String(secretAccessKey || "").trim();
+
+  if (!normalizedEndpoint) {
+    throw new Error("S3 endpoint is required.");
+  }
+  if (!normalizedRegion) {
+    throw new Error("S3 region is required.");
+  }
+  if (!normalizedAccessKeyId) {
+    throw new Error("S3 access key ID is required.");
+  }
+  if (!normalizedSecretAccessKey) {
+    throw new Error("S3 secret access key is required.");
+  }
+  if (!normalizedBucket) {
+    throw new Error("S3 bucket name is required.");
+  }
+  if (typeof forcePathStyle !== "boolean") {
+    throw new Error("S3 forcePathStyle setting must be true or false.");
+  }
+
+  const publicBaseUrl = derivePublicBaseUrl({
+    endpoint: normalizedEndpoint,
+    bucket: normalizedBucket,
+    forcePathStyle,
+  });
+
+  if (!publicBaseUrl) {
+    throw new Error("Unable to derive a public base URL for the S3 bucket.");
+  }
+
+  return {
+    endpoint: normalizedEndpoint,
+    region: normalizedRegion,
+    accessKeyId: normalizedAccessKeyId,
+    secretAccessKey: normalizedSecretAccessKey,
+    bucket: normalizedBucket,
+    forcePathStyle,
+    publicBaseUrl,
+  };
+}
+
+export function getCorsOrigins() {
+  const origins = new Set();
+  if (typeof window !== "undefined" && window.location) {
+    const origin = window.location.origin;
+    if (origin && origin !== "null") {
+      origins.add(origin);
+    }
+    if (origin && origin.startsWith("http://localhost")) {
+      origins.add(origin.replace("http://", "https://"));
+    }
+  }
+  return Array.from(origins);
+}
+
+export async function prepareS3Connection({
+  endpoint,
+  region,
+  accessKeyId,
+  secretAccessKey,
+  bucket,
+  forcePathStyle,
+  origins,
+} = {}) {
+  const normalized = validateS3Connection({
+    endpoint,
+    region,
+    accessKeyId,
+    secretAccessKey,
+    bucket,
+    forcePathStyle,
+  });
+
+  await ensureS3SdkLoaded();
+
+  const s3 = makeS3Client({
+    endpoint: normalized.endpoint,
+    region: normalized.region,
+    accessKeyId: normalized.accessKeyId,
+    secretAccessKey: normalized.secretAccessKey,
+    forcePathStyle: normalized.forcePathStyle,
+  });
+
+  try {
+    await ensureBucketExists({
+      s3,
+      bucket: normalized.bucket,
+      region: normalized.region,
+    });
+  } catch (error) {
+    userLogger.warn("Failed to ensure S3 bucket exists:", error);
+  }
+
+  const allowedOrigins = Array.isArray(origins) ? origins : [];
+  if (allowedOrigins.length > 0) {
+    try {
+      await ensureBucketCors({
+        s3,
+        bucket: normalized.bucket,
+        origins: allowedOrigins,
+        region: normalized.region,
+      });
+    } catch (error) {
+      userLogger.warn("Failed to ensure S3 bucket CORS:", error);
+    }
+  }
+
+  return normalized;
+}

js/ui/profileModalController.js

@@ -31,6 +31,7 @@ import {
   extractAttachmentsFromMessage,
   formatAttachmentSize,
 } from "../attachments/attachmentUtils.js";
+import { getCorsOrigins, prepareS3Connection } from "../services/s3Service.js";
 import {
   clearAttachmentCache,
   downloadAttachment,
@@ -9131,7 +9132,7 @@ export class ProfileModalController {
     if (!storageService) return;
 
     const provider = this.storageProviderInput?.value || "cloudflare_r2";
-    const endpointOrAccount = this.storageEndpointInput?.value?.trim() || "";
+    let endpointOrAccount = this.storageEndpointInput?.value?.trim() || "";
     const region = this.storageRegionInput?.value?.trim() || "auto";
     const accessKeyId = this.storageAccessKeyInput?.value?.trim() || "";
     const secretAccessKey = this.storageSecretKeyInput?.value?.trim() || "";
@@ -9144,6 +9145,9 @@ export class ProfileModalController {
         return;
     }
 
+    let publicBaseUrl = "";
+    let forcePathStyle = provider === PROVIDERS.GENERIC;
+
     const payload = {
         provider,
         accessKeyId,
@@ -9155,7 +9159,30 @@ export class ProfileModalController {
     if (provider === "cloudflare_r2") {
         payload.accountId = endpointOrAccount;
     } else {
+        try {
+          const normalized = await prepareS3Connection({
+            endpoint: endpointOrAccount,
+            region,
+            accessKeyId,
+            secretAccessKey,
+            bucket,
+            forcePathStyle,
+            origins: getCorsOrigins(),
+          });
+          endpointOrAccount = normalized.endpoint;
+          publicBaseUrl = normalized.publicBaseUrl;
+          forcePathStyle = normalized.forcePathStyle;
+        } catch (error) {
+          devLogger.error("Failed to validate S3 connection:", error);
+          this.setStorageFormStatus(
+            error?.message || "Invalid S3 configuration.",
+            "error"
+          );
+          return;
+        }
+
         payload.endpoint = endpointOrAccount;
+        payload.forcePathStyle = forcePathStyle;
     }
 
     const meta = {
@@ -9174,6 +9201,10 @@ export class ProfileModalController {
     // We can store it in meta if we want to show it in UI without decrypting.
     if (provider === "cloudflare_r2") {
         meta.accountId = endpointOrAccount;
+    } else {
+        meta.publicBaseUrl = publicBaseUrl;
+        meta.baseDomain = publicBaseUrl;
+        meta.forcePathStyle = forcePathStyle;
     }
 
     this.setStorageFormStatus("Saving...", "info");
@@ -9204,6 +9235,7 @@ export class ProfileModalController {
     const accessKeyId = this.storageAccessKeyInput?.value?.trim() || "";
     const secretAccessKey = this.storageSecretKeyInput?.value?.trim() || "";
     const bucket = this.storageBucketInput?.value?.trim() || "";
+    const forcePathStyle = provider === PROVIDERS.GENERIC;
 
     if (!accessKeyId || !secretAccessKey || !endpointOrAccount) {
         this.setStorageFormStatus("Missing credentials for test.", "error");
@@ -9224,6 +9256,7 @@ export class ProfileModalController {
       config.accountId = endpointOrAccount;
     } else {
       config.endpoint = endpointOrAccount;
+      config.forcePathStyle = forcePathStyle;
     }
 
     try {