Adding signed url decoding logic in deleteHandler (#3155)

Co-authored-by: Sophie Guo <[email protected]>

Author: SophieGuo410

ambry-frontend/src/main/java/com/github/ambry/frontend/DeleteBlobHandler.java

@@ -22,7 +22,6 @@
 import com.github.ambry.clustermap.ClusterMap;
 import com.github.ambry.commons.BlobId;
 import com.github.ambry.commons.Callback;
-import com.github.ambry.named.NamedBlobRecord;
 import com.github.ambry.quota.QuotaManager;
 import com.github.ambry.quota.QuotaUtils;
 import com.github.ambry.rest.RequestPath;
@@ -35,7 +34,10 @@
 import com.github.ambry.rest.RestServiceException;
 import com.github.ambry.rest.RestUtils;
 import com.github.ambry.router.Router;
+import com.github.ambry.utils.Pair;
+import java.io.IOException;
 import java.util.GregorianCalendar;
+import java.util.Map;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -60,10 +62,11 @@ public class DeleteBlobHandler {
   private final ClusterMap clusterMap;
   private final QuotaManager quotaManager;
   private final AccountService accountService;
+  private final IdSigningService idSigningService;
 
   DeleteBlobHandler(Router router, SecurityService securityService, IdConverter idConverter,
       AccountAndContainerInjector accountAndContainerInjector, FrontendMetrics metrics, ClusterMap clusterMap,
-      QuotaManager quotaManager, AccountService accountService) {
+      QuotaManager quotaManager, AccountService accountService, IdSigningService idSigningService) {
     this.router = router;
     this.securityService = securityService;
     this.idConverter = idConverter;
@@ -72,6 +75,7 @@ public class DeleteBlobHandler {
     this.clusterMap = clusterMap;
     this.quotaManager = quotaManager;
     this.accountService = accountService;
+    this.idSigningService = idSigningService;
   }
 
   public void handle(RestRequest restRequest, RestResponseChannel restResponseChannel, Callback<Void> callback)
@@ -122,7 +126,9 @@ private Callback<Void> securityProcessRequestCallback() {
         String blobIdStr = getRequestPath(restRequest).getOperationOrBlobId(false);
         if (restRequest.getArgs().get(InternalKeys.TARGET_ACCOUNT_KEY) == null)  {
           // Inject account and container when they are missing from the rest request.
-          blobIdStr = RestUtils.stripSlashAndExtensionFromId(blobIdStr);
+          String decryptedInput =
+              parseSignedIdIfRequired(blobIdStr.startsWith("/") ? blobIdStr.substring(1) : blobIdStr);
+          blobIdStr = RestUtils.stripSlashAndExtensionFromId(decryptedInput);
           BlobId convertedBlobId = FrontendUtils.getBlobIdFromString(blobIdStr, clusterMap);
           accountAndContainerInjector.injectTargetAccountAndContainerFromBlobId(convertedBlobId, restRequest,
               metrics.deleteBlobMetricsGroup);
@@ -242,5 +248,18 @@ private void deleteDatasetVersion(RestRequest restRequest) throws RestServiceExc
             RestServiceErrorCode.getRestServiceErrorCode(ex.getErrorCode()));
       }
     }
+
+    private String parseSignedIdIfRequired(String incomingId) throws RestServiceException {
+      String blobId;
+      String sanitizedIncomingId = stripSlashAndExtensionFromId(incomingId);
+      if (idSigningService.isIdSigned(sanitizedIncomingId)) {
+        Pair<String, Map<String, String>> idAndMetadata = idSigningService.parseSignedId(sanitizedIncomingId);
+        restRequest.setArg(RestUtils.InternalKeys.SIGNED_ID_METADATA_KEY, idAndMetadata.getSecond());
+        blobId = conditionalCopySlashAndExtension(incomingId, idAndMetadata.getFirst());
+      } else {
+        blobId = incomingId;
+      }
+      return blobId;
+    }
   }
 }

ambry-frontend/src/main/java/com/github/ambry/frontend/FrontendRestRequestService.java

@@ -211,7 +211,7 @@ public void start() throws InstantiationException {
         new CopyDatasetVersionHandler(securityService, accountService, frontendMetrics, accountAndContainerInjector);
     deleteBlobHandler =
         new DeleteBlobHandler(router, securityService, idConverter, accountAndContainerInjector, frontendMetrics,
-            clusterMap, quotaManager, accountService);
+            clusterMap, quotaManager, accountService, idSigningService);
     deleteDatasetHandler =
         new DeleteDatasetHandler(securityService, accountService, frontendMetrics, accountAndContainerInjector,
             deleteBlobHandler);

ambry-frontend/src/main/java/com/github/ambry/frontend/FrontendUtils.java

@@ -84,6 +84,42 @@ static BlobId getBlobIdFromString(String blobIdStr, ClusterMap clusterMap) throw
     }
   }
 
+  /**
+   * Copies leading slash and/or extension from {@code src} to {@code dest} if present in {@code src} and not present
+   * in {@code dest}
+   * <p/>
+   * Does not overwrite if already present in {@code dest}. Does not create if not present in {@code src}.
+   * @param src the source string
+   * @param dest the destination string
+   * @return {@code dest} with leading slash and/or extension (depending on presence in {@code src} and {@code dest}).
+   */
+  public static String conditionalCopySlashAndExtension(String src, String dest) {
+    if (src.startsWith("/")) {
+      dest = addLeadingSlashIfAbsent(dest);
+    }
+    int destExtensionIndex = dest.indexOf(".");
+    if (destExtensionIndex == -1) {
+      int srcExtensionIndex = src.indexOf(".");
+      if (srcExtensionIndex != -1) {
+        String extension = src.substring(srcExtensionIndex);
+        dest = dest + extension;
+      }
+    }
+    return dest;
+  }
+
+  /**
+   * Adds a leading slash if {@code input} doesn't already have one.
+   * @param input the input string
+   * @return input string with a leading slash.
+   */
+  public static String addLeadingSlashIfAbsent(String input) {
+    if (!input.startsWith("/")) {
+      return "/" + input;
+    }
+    return input;
+  }
+
   /**
    * @param metrics the {@link AsyncOperationTracker.Metrics} instance to update.
    * @param successAction the action to take if the callback was called successfully.

ambry-frontend/src/test/java/com/github/ambry/frontend/NamedBlobDeleteHandlerTest.java

@@ -138,7 +138,7 @@ public NamedBlobDeleteHandlerTest() throws Exception {
             ACCOUNT_SERVICE, null);
     deleteBlobHandler =
         new DeleteBlobHandler(router, securityServiceFactory.getSecurityService(), idConverterFactory.getIdConverter(),
-            injector, metrics, CLUSTER_MAP, QuotaTestUtils.createDummyQuotaManager(), ACCOUNT_SERVICE);
+            injector, metrics, CLUSTER_MAP, QuotaTestUtils.createDummyQuotaManager(), ACCOUNT_SERVICE, null);
   }
 
   @Test

ambry-frontend/src/test/java/com/github/ambry/frontend/PostBlobHandlerTest.java

@@ -67,6 +67,7 @@
 import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Base64;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.LinkedList;
@@ -78,6 +79,7 @@
 import java.util.concurrent.TimeUnit;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
+import org.apache.commons.lang3.tuple.Pair;
 import org.json.JSONObject;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -87,6 +89,7 @@
 import static com.github.ambry.frontend.FrontendRestRequestServiceTest.*;
 import static com.github.ambry.rest.RestUtils.InternalKeys.*;
 import static org.junit.Assert.*;
+import static org.mockito.Mockito.*;
 
 
 /**
@@ -146,6 +149,7 @@ public class PostBlobHandlerTest {
   private FrontendConfig frontendConfig;
   private PostBlobHandler postBlobHandler;
   private GetBlobHandler getBlobHandler;
+  private DeleteBlobHandler deleteBlobHandler;
 
   private String reservedMetadataId;
 
@@ -170,6 +174,7 @@ public PostBlobHandlerTest(boolean isReservedMetadataEnabled) {
     }
     initPostBlobHandler(props);
     initGetBlobHandler(props);
+    initDeleteBlobHandler(props);
   }
 
   /**
@@ -399,6 +404,14 @@ private void initGetBlobHandler(Properties properties) {
         idConverterFactory.getIdConverter(), injector, metrics, CLUSTER_MAP, QUOTA_MANAGER, ACCOUNT_SERVICE);
   }
 
+  private void initDeleteBlobHandler(Properties props) {
+    VerifiableProperties verifiableProperties = new VerifiableProperties(props);
+    frontendConfig = new FrontendConfig(verifiableProperties);
+    deleteBlobHandler =
+        new DeleteBlobHandler(router, securityServiceFactory.getSecurityService(), idConverterFactory.getIdConverter(),
+            injector, metrics, CLUSTER_MAP, QUOTA_MANAGER, ACCOUNT_SERVICE, idSigningService);
+  }
+
   // ttlRequiredEnforcementTest() helpers
 
   /**

ambry-frontend/src/test/java/com/github/ambry/frontend/S3BatchDeleteHandlerTest.java

@@ -248,7 +248,7 @@ private void setup() throws Exception {
             QuotaTestUtils.createDummyQuotaManager(), ACCOUNT_SERVICE, null);
     DeleteBlobHandler deleteBlobHandler =
         new DeleteBlobHandler(router, securityService, ambryIdConverterFactory.getIdConverter(), injector, metrics,
-            new MockClusterMap(), QuotaTestUtils.createDummyQuotaManager(), ACCOUNT_SERVICE);
+            new MockClusterMap(), QuotaTestUtils.createDummyQuotaManager(), ACCOUNT_SERVICE, null);
     s3BatchDeleteHandler = new S3BatchDeleteHandler(deleteBlobHandler, metrics);
   }
 

ambry-frontend/src/test/java/com/github/ambry/frontend/S3DeleteHandlerTest.java

@@ -134,7 +134,7 @@ private void setup() throws Exception {
             QuotaTestUtils.createDummyQuotaManager(), ACCOUNT_SERVICE, null);
     DeleteBlobHandler deleteBlobHandler =
         new DeleteBlobHandler(router, securityService, ambryIdConverterFactory.getIdConverter(), injector, metrics,
-            new MockClusterMap(), QuotaTestUtils.createDummyQuotaManager(), ACCOUNT_SERVICE);
+            new MockClusterMap(), QuotaTestUtils.createDummyQuotaManager(), ACCOUNT_SERVICE, null);
     s3DeleteHandler = new S3DeleteHandler(deleteBlobHandler, null, metrics);
   }
 

ambry-frontend/src/test/java/com/github/ambry/frontend/S3MultipartUploadTest.java

@@ -361,7 +361,8 @@ private void setup() throws Exception {
         new GetBlobHandler(frontendConfig, router, securityService, idConverter, injector, metrics, clusterMap,
             quotaManager, ACCOUNT_SERVICE);
     deleteBlobHandler =
-        new DeleteBlobHandler(router, securityService, idConverter, injector, metrics, clusterMap, quotaManager, ACCOUNT_SERVICE);
+        new DeleteBlobHandler(router, securityService, idConverter, injector, metrics, clusterMap, quotaManager, ACCOUNT_SERVICE,
+            null);
     s3MultipartUploadHandler = new S3MultipartUploadHandler(securityService, metrics, injector, frontendConfig,
             namedBlobDb, idConverter, router, quotaManager);