Merge pull request #62 from ing-bank/feature/multidelete-v4

add patch to handle multidelete using V4
ing-bank · Mar 20, 2019 · 7d3a117 · 7d3a117
2 parents 7df23b2 + 2cce98d
commit 7d3a117
Show file tree

Hide file tree

Showing 6 changed files with 29 additions and 6 deletions.
diff --git a/build.sbt b/build.sbt
@@ -3,7 +3,7 @@ import com.typesafe.sbt.packager.docker.ExecCmd
 import scalariform.formatter.preferences._
 
 name := "airlock"
-version := "0.1.22"
+version := "0.1.23"
 scalaVersion := "2.12.8"
 
 scalacOptions += "-unchecked"

diff --git a/src/main/scala/com/ing/wbaa/airlock/proxy/api/ProxyService.scala b/src/main/scala/com/ing/wbaa/airlock/proxy/api/ProxyService.scala
@@ -103,9 +103,11 @@ trait ProxyService {
       logger.debug(s"Request authenticated: $httpRequest")
       if (isUserAuthorizedForRequest(s3Request, userSTS)) {
         val rawQueryString = httpRequest.uri.rawQueryString.getOrElse("")
-        val isMultideletePost = httpRequest.entity.contentType.mediaType == MediaTypes.`application/xml` && httpRequest.method == HttpMethods.POST
+        val isMultideletePost =
+          (httpRequest.entity.contentType.mediaType == MediaTypes.`application/xml` || httpRequest.entity.contentType.mediaType == MediaTypes.`application/octet-stream`) &&
+            httpRequest.method == HttpMethods.POST && rawQueryString == "delete"
 
-        if (isMultideletePost && rawQueryString == "delete") {
+        if (isMultideletePost) {
           checkExtractedPostContents(
             httpRequest,
             s3Request.copy(mediaType = MediaTypes.`application/xml`), userSTS)

diff --git a/src/main/scala/com/ing/wbaa/airlock/proxy/handler/FilterRecursiveMultiDelete.scala b/src/main/scala/com/ing/wbaa/airlock/proxy/handler/FilterRecursiveMultiDelete.scala
@@ -19,16 +19,22 @@ object FilterRecursiveMultiDelete {
       .via(XmlParsing.parser)
       .statefulMapConcat(() => {
         val keys = new ListBuffer[String]
+        isKeyTag = false
+
         parseEvent =>
           parseEvent match {
-            case e: StartElement if e.localName == "Delete" =>
+            case e: StartElement if e.localName.startsWith("Delete") =>
               keys.clear()
               immutable.Seq.empty
 
             case e: StartElement if e.localName == "Key" =>
               isKeyTag = true
               immutable.Seq.empty
 
+            case e: EndElement if e.localName == "Key" =>
+              isKeyTag = false
+              immutable.Seq.empty
+
             case e: TextEvent =>
               if (isKeyTag) keys.append(e.text)
               immutable.Seq.empty

diff --git a/src/main/scala/com/ing/wbaa/airlock/proxy/provider/AuthorizationProviderRanger.scala b/src/main/scala/com/ing/wbaa/airlock/proxy/provider/AuthorizationProviderRanger.scala
@@ -92,8 +92,9 @@ trait AuthorizationProviderRanger {
       case S3Request(_, Some(s3path), None, accessType, _, _, _) if accessType == Read || accessType == Head =>
         isAuthorisedByRanger(s3path)
 
-      // multidelete with xml list of objects in post. Will catch also regular application/xml post requests
-      case S3Request(_, Some(s3path), None, accessType, _, _, mediaType) if accessType == Write && mediaType == MediaTypes.`application/xml` =>
+      // multidelete with xml list of objects in post
+      case S3Request(_, Some(s3path), None, accessType, _, _, mediaType) if accessType == Write &&
+        (mediaType == MediaTypes.`application/xml` || mediaType == MediaTypes.`application/octet-stream`) =>
         logger.debug(s"Passing ranger check for multi object deletion to check method")
         true
 

diff --git a/src/test/resources/multiDeleteRequestV4.xml b/src/test/resources/multiDeleteRequestV4.xml
@@ -0,0 +1,6 @@
+<Delete xmlns="http://s3.amazonaws.com/doc/2006-03-01/">
+  <Object>
+    <Key>testuser/issue</Key>
+  </Object>
+  <Quiet>true</Quiet>
+</Delete>
diff --git a/src/test/scala/com/ing/wbaa/airlock/proxy/handler/FilterRecursiveMultiDeleteSpec.scala b/src/test/scala/com/ing/wbaa/airlock/proxy/handler/FilterRecursiveMultiDeleteSpec.scala
@@ -18,8 +18,10 @@ class FilterRecursiveMultiDeleteSpec extends AsyncWordSpec with DiagrammedAssert
   implicit def materializer: ActorMaterializer = ActorMaterializer()(system)
 
   val multiDeleteRequestXml: String = scala.io.Source.fromResource("multiDeleteRequest.xml").mkString.stripMargin.trim
+  val multiDeleteRequestV4Xml: String = scala.io.Source.fromResource("multiDeleteRequestV4.xml").mkString.stripMargin.trim
   val multiPartComplete: String = scala.io.Source.fromResource("multipartUploadComplete.xml").mkString.stripMargin.trim
   val data: Source[ByteString, NotUsed] = Source.single(ByteString.fromString(multiDeleteRequestXml))
+  val dataV4: Source[ByteString, NotUsed] = Source.single(ByteString.fromString(multiDeleteRequestV4Xml))
   val otherData: Source[ByteString, NotUsed] = Source.single(ByteString.fromString(multiPartComplete))
 
   "multiDelete request" should {
@@ -30,6 +32,12 @@ class FilterRecursiveMultiDeleteSpec extends AsyncWordSpec with DiagrammedAssert
         assert(r.contains("testuser/file3"))
       }
     }
+    "v4 should be parsed to objects list" in {
+      exctractMultideleteObjectsFlow(dataV4).map { r =>
+        assert(r.contains("testuser/issue"))
+        assert(!r.contains("true"))
+      }
+    }
 
     "should return empty list" in {
       exctractMultideleteObjectsFlow(otherData).map(r => assert(r == Vector()))