feat: Replace JClouds with AWS SDK + File NIO [DHIS2-20648] (#23918)

* feat: Replace JClouds with AWS SDK and NIO [DHIS2-20648]

* feat: Align impl contracts for consistency. Add comments [DHIS2-20648]

* feat: Remove remaining JClouds refs [DHIS2-20648]

* feat: Harden security and add more tests [DHIS2-20648]

* feat: Clean logs [DHIS2-20648]

* Revert get content length

* feat: Enable dir create/exists to keep existing behaviour

* clean up tests

* feat: Use static of

* feat: Craete bucket first without HEAD call

* feat: Address PR comments

* remove redundant tests

* feat: Remove AppStorageSource, no longer required

* feat: Ensure streams are resetable when passing to s3 [DHIS2-20648]

* resolve conflicts

* remove test util method

---------

Co-authored-by: Stian Sandvold <stian@dhis2.org>

Author: david-mackessy

dhis-2/dhis-api/src/main/java/org/hisp/dhis/appmanager/App.java

@@ -82,8 +82,6 @@ public class App implements Serializable {
 
   private String defaultLocale;
 
-  private AppStorageSource appStorageSource;
-
   private String folderName;
 
   /** Optional. */
@@ -414,22 +412,12 @@ public void setBaseUrl(String baseUrl) {
     this.baseUrl = baseUrl;
   }
 
-  @JsonProperty
-  @JacksonXmlProperty(namespace = DxfNamespaces.DXF_2_0)
-  public AppStorageSource getAppStorageSource() {
-    return appStorageSource;
-  }
-
   @JsonProperty
   @JacksonXmlProperty(namespace = DxfNamespaces.DXF_2_0)
   public List<AppShortcut> getShortcuts() {
     return shortcuts;
   }
 
-  public void setAppStorageSource(AppStorageSource appStorageSource) {
-    this.appStorageSource = appStorageSource;
-  }
-
   @JsonProperty
   @JacksonXmlProperty(namespace = DxfNamespaces.DXF_2_0)
   public Set<String> getAuthorities() {

dhis-2/dhis-api/src/main/java/org/hisp/dhis/appmanager/AppFolderName.java

@@ -100,7 +100,7 @@ public BlobKey resolve(String resource) {
    * org.hisp.dhis.storage.BlobStoreService#deleteDirectory}.
    */
   public BlobKeyPrefix asPrefix() {
-    return new BlobKeyPrefix(path);
+    return BlobKeyPrefix.of(path);
   }
 
   @Nonnull

dhis-2/dhis-api/src/main/java/org/hisp/dhis/appmanager/AppStorageSource.java

@@ -201,7 +201,7 @@ public String getStorageKey() {
    * rather than {@link String}.
    */
   public BlobKey asBlobKey() {
-    return new BlobKey(storageKey);
+    return BlobKey.of(storageKey);
   }
 
   public void setStorageKey(String storageKey) {

dhis-2/dhis-api/src/main/java/org/hisp/dhis/fileresource/FileResource.java

@@ -38,7 +38,8 @@
  * #resolve(BlobKey)} always produces a clean path without any slash-cleaning.
  *
  * <p>Configured via {@link org.hisp.dhis.external.conf.ConfigurationKey#FILESTORE_CONTAINER} and
- * resolved once at startup by {@link org.hisp.dhis.jclouds.JCloudsStore}.
+ * resolved once at startup by the {@link org.hisp.dhis.storage.BlobStoreService} implementation
+ * selected for the configured {@code filestore.provider}.
  */
 public record BlobContainerName(String value) {
 

dhis-2/dhis-api/src/main/java/org/hisp/dhis/storage/BlobContainerName.java

@@ -59,6 +59,16 @@ public record BlobKey(String value) {
       throw new IllegalArgumentException(
           "BlobKey value must not exceed " + MAX_KEY_LENGTH + " characters: " + value);
     }
+    // Reject `..` as a path segment so filesystem-backed stores can't be tricked into writing
+    // outside the container directory. Substrings like "a..b" are fine — only segments split by
+    // `/` count. No DHIS2 caller produces `..` segments today (zip uploads sanitise via
+    // ZipFileUtils, file-resource storage keys are UUID-shaped) so this is defense-in-depth.
+    for (String segment : value.split("/", -1)) {
+      if ("..".equals(segment)) {
+        throw new IllegalArgumentException(
+            "BlobKey value must not contain '..' as a path segment: " + value);
+      }
+    }
   }
 
   /**

dhis-2/dhis-api/src/main/java/org/hisp/dhis/storage/BlobKey.java

@@ -61,7 +61,8 @@ public record BlobKeyPrefix(@Nonnull String value) {
   /**
    * Creates a {@link BlobKeyPrefix} from {@code value}, stripping any leading or trailing {@code /}
    * before construction. Prefer this over the canonical constructor when the input may carry
-   * store-native separators (e.g. JClouds appends a trailing {@code /} to folder names).
+   * store-native separators (e.g. S3's {@code CommonPrefix} entries arrive with a trailing {@code
+   * /}).
    *
    * <p>Examples:
    *

dhis-2/dhis-api/src/main/java/org/hisp/dhis/storage/BlobKeyPrefix.java

@@ -66,4 +66,26 @@ void ofSingleSegment() {
   void ofJoinsSegmentsWithSlash() {
     assertEquals("apps/my-app/index.html", BlobKey.of("apps", "my-app", "index.html").value());
   }
+
+  @Test
+  void leadingDotDotSegmentIsRejected() {
+    assertThrows(IllegalArgumentException.class, () -> new BlobKey("../escape.txt"));
+  }
+
+  @Test
+  void interiorDotDotSegmentIsRejected() {
+    assertThrows(IllegalArgumentException.class, () -> new BlobKey("a/../b/c"));
+  }
+
+  @Test
+  void trailingDotDotSegmentIsRejected() {
+    assertThrows(IllegalArgumentException.class, () -> new BlobKey("a/b/.."));
+  }
+
+  @Test
+  void dotDotAsSubstringWithinSegmentIsAccepted() {
+    // "a..b" is a perfectly legitimate filename — only `..` as a *segment* is rejected.
+    BlobKey key = new BlobKey("apps/my..app/index.html");
+    assertEquals("apps/my..app/index.html", key.value());
+  }
 }

dhis-2/dhis-api/src/test/java/org/hisp/dhis/storage/BlobKeyTest.java

@@ -251,32 +251,11 @@
       <artifactId>annotations</artifactId>
     </dependency>
 
-    <!-- Apache JClouds -->
-
-    <dependency>
-      <groupId>org.apache.jclouds</groupId>
-      <artifactId>jclouds-core</artifactId>
-    </dependency>
+    <!-- AWS SDK for Java v2 -->
     <dependency>
-      <groupId>org.apache.jclouds</groupId>
-      <artifactId>jclouds-blobstore</artifactId>
-    </dependency>
-    <dependency>
-      <groupId>org.apache.jclouds.api</groupId>
-      <artifactId>filesystem</artifactId>
-    </dependency>
-    <dependency>
-      <groupId>org.apache.jclouds.api</groupId>
+      <groupId>software.amazon.awssdk</groupId>
       <artifactId>s3</artifactId>
     </dependency>
-    <dependency>
-      <groupId>org.apache.jclouds.provider</groupId>
-      <artifactId>aws-s3</artifactId>
-    </dependency>
-    <dependency>
-      <groupId>org.apache.jclouds.driver</groupId>
-      <artifactId>jclouds-slf4j</artifactId>
-    </dependency>
 
     <!-- Image Processing -->
 

dhis-2/dhis-services/dhis-service-core/pom.xml

@@ -37,7 +37,6 @@
 import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
-import java.net.MalformedURLException;
 import java.net.URI;
 import java.util.Collections;
 import java.util.Enumeration;
@@ -69,6 +68,7 @@
 import org.hisp.dhis.util.ZipBombException;
 import org.hisp.dhis.util.ZipFileUtils;
 import org.hisp.dhis.util.ZipSlipException;
+import org.springframework.core.io.ByteArrayResource;
 import org.springframework.core.io.FileSystemResource;
 import org.springframework.core.io.Resource;
 import org.springframework.core.io.UrlResource;
@@ -79,8 +79,8 @@
  */
 @Slf4j
 @RequiredArgsConstructor
-@Service("org.hisp.dhis.appmanager.JCloudsAppStorageService")
-public class JCloudsAppStorageService implements AppStorageService {
+@Service
+public class BlobStoreAppStorageService implements AppStorageService {
 
   private static final String BUNDLED_APP_INFO_FILENAME = "bundled-app-info.json";
   public static final String MANIFEST_WEBAPP_FILENAME = "manifest.webapp";
@@ -118,7 +118,6 @@ private void handleAppManifest(
       BiConsumer<App, BundledAppInfo> handler, BlobKeyPrefix folder, InputStream manifestStream) {
     try (InputStream inputStream = manifestStream) {
       App app = App.MAPPER.readValue(inputStream, App.class);
-      app.setAppStorageSource(AppStorageSource.JCLOUDS);
       app.setFolderName(folder.value());
       app.setManifestTranslations(
           readAppManifestTranslations(
@@ -234,7 +233,6 @@ public App installApp(
       app = AppManager.readAppManifest(file, this.jsonMapper, topLevelFolder);
       folder = AppFolderName.ofKey(app.getKey());
       app.setFolderName(folder.path());
-      app.setAppStorageSource(AppStorageSource.JCLOUDS);
     } catch (IOException e) {
       log.error("Failed to install app: Failure during reading manifest from zip file", e);
       app = new App();
@@ -294,9 +292,15 @@ private void unzipFile(File file, AppFolderName folder, String topLevelFolder)
         String filePath = getFilePath(folder.path(), topLevelFolder, zipEntry);
         // If it's the root folder, skip
         if (filePath == null) continue;
+        if (zipEntry.isDirectory()) {
+          // Materialise empty directories so the /api/apps/<app>/<dir> → /<dir>/ redirect
+          // contract holds even when the zip contains a directory with no files inside.
+          blobStore.createDirectory(BlobKeyPrefix.of(filePath));
+          continue;
+        }
         try (InputStream zipInputStream = zipFile.getInputStream(zipEntry)) {
           blobStore.putBlob(
-              new BlobKey(filePath), zipInputStream, zipEntry.getSize(), null, null, null);
+              BlobKey.of(filePath), zipInputStream, zipEntry.getSize(), null, null, null);
         }
       }
     }
@@ -338,30 +342,17 @@ public void deleteApp(@Nonnull App app) {
     AppFolderName folder = app.appFolder();
     blobStore.deleteBlob(folder.resolve(MANIFEST_WEBAPP_FILENAME));
 
-    // TODO(DHIS2-20648) Once the replacement BlobStoreService implementation does recursive
-    // deleteDirectory on every backend (see contract test), this branch can collapse to a
-    // single blobStore.deleteDirectory(folder.asPrefix()) call.
-    if (blobStore.isFilesystem()) {
-      // Delete all files related to app (works for local filestore)
-      blobStore.deleteDirectory(folder.asPrefix());
-    } else {
-      // S3: deleteDirectory is not recursive, so enumerate and delete individually
-      for (BlobKey key : blobStore.listKeys(folder.asPrefix())) {
-        log.debug("Deleting app file: {}", key);
-        blobStore.deleteBlob(key);
-      }
-    }
+    blobStore.deleteDirectory(folder.asPrefix());
     log.info("Deleted app {}", app.getName());
   }
 
   @Override
   @Nonnull
   public ResourceResult getAppResource(@CheckForNull App app, @Nonnull String resource)
       throws IOException {
-    if (app == null || !app.getAppStorageSource().equals(AppStorageSource.JCLOUDS)) {
+    if (app == null) {
       log.warn(
-          "Can't look up resource {}. The specified app was not found in JClouds storage.",
-          resource);
+          "Can't look up resource {}. The specified app was not found in blob storage.", resource);
       return new ResourceNotFound(resource);
     }
     if (resource.isBlank()) {
@@ -374,25 +365,28 @@ public ResourceResult getAppResource(@CheckForNull App app, @Nonnull String reso
     if (blobStore.blobExists(key)) {
       return new ResourceFound(getResource(key));
     }
-    if (keyExistsAsDirectory(key)) {
+    if (blobStore.directoryExists(BlobKeyPrefix.of(key.value()))) {
       return new Redirect(resource + "/");
     }
     log.debug("ResourceNotFound {} for App {}", key, app.getName());
     return new ResourceNotFound(resource);
   }
 
-  private boolean keyExistsAsDirectory(BlobKey key) {
-    return blobStore.listKeys(BlobKeyPrefix.of(key.value())).iterator().hasNext();
-  }
-
-  private Resource getResource(@Nonnull BlobKey key) throws MalformedURLException {
+  private Resource getResource(@Nonnull BlobKey key) throws IOException {
     if (blobStore.isFilesystem()) {
       return new FileSystemResource(
           locationManager.getFileForReading(blobStore.container().resolve(key)));
-    } else {
-      URI uri = fileResourceContentStore.getSignedGetContentUri(key);
+    }
+    URI uri = fileResourceContentStore.getSignedGetContentUri(key);
+    if (uri != null) {
       return new UrlResource(uri);
     }
+    // When backend supports neither filesystem access nor signed URLs (e.g. transient): buffer
+    // the blob bytes so Spring can serve them directly.
+    try (InputStream in = blobStore.openStream(key)) {
+      if (in == null) throw new IOException("Blob not found: " + key);
+      return new ByteArrayResource(in.readAllBytes());
+    }
   }
 
   /**
@@ -429,12 +423,11 @@ private static void logInstallSuccess(App app, String appFolder) {
 
   private static void logDiscoveredApps(Map<String, Pair<App, BundledAppInfo>> apps) {
     if (apps.isEmpty()) {
-      log.info("No apps found during JClouds discovery.");
+      log.info("No apps found during blob store discovery");
     } else {
       apps.values()
           .forEach(
-              pair ->
-                  log.info("Discovered app '{}' from JClouds storage ", pair.getLeft().getName()));
+              pair -> log.info("Discovered app '{}' from blob storage ", pair.getLeft().getName()));
     }
   }
 }