Reject ambiguous organization DELETE requests (#4503)

delta/app/src/main/scala/ch/epfl/bluebrain/nexus/delta/routes/OrganizationsRoutes.scala

@@ -25,6 +25,7 @@ import ch.epfl.bluebrain.nexus.delta.sdk.organizations.model.OrganizationRejecti
 import ch.epfl.bluebrain.nexus.delta.sdk.organizations.model.{Organization, OrganizationRejection}
 import ch.epfl.bluebrain.nexus.delta.sdk.organizations.{OrganizationDeleter, Organizations}
 import ch.epfl.bluebrain.nexus.delta.sdk.permissions.Permissions._
+import ch.epfl.bluebrain.nexus.delta.sourcing.model.Label
 import io.circe.Decoder
 import io.circe.generic.extras.Configuration
 import io.circe.generic.extras.semiauto.deriveConfiguredDecoder
@@ -132,20 +133,15 @@ final class OrganizationsRoutes(
                   },
                   // Deprecate or delete organization
                   delete {
-                    concat(
-                      parameter("rev".as[Int]) { rev =>
-                        authorizeFor(id, orgs.write).apply {
-                          emitMetadata(
-                            organizations.deprecate(id, rev)
-                          )
-                        }
-                      },
-                      parameter("prune".requiredValue(true)) { _ =>
+                    parameters("rev".as[Int].?, "prune".as[Boolean].?) {
+                      case (Some(rev), None)        => deprecate(id, rev)
+                      case (Some(rev), Some(false)) => deprecate(id, rev)
+                      case (None, Some(true))       =>
                         authorizeFor(id, orgs.delete).apply {
                           emit(orgDeleter.delete(id).attemptNarrow[OrganizationRejection])
                         }
-                      }
-                    )
+                      case (_, _)                   => emit((InvalidDeleteRequest(id): OrganizationRejection).asLeft[Unit].pure[IO])
+                    }
                   }
                 )
               }
@@ -169,6 +165,11 @@ final class OrganizationsRoutes(
         }
       }
     }
+
+  private def deprecate(id: Label, rev: Int)(implicit c: Caller) =
+    authorizeFor(id, orgs.write).apply {
+      emitMetadata(organizations.deprecate(id, rev))
+    }
 }
 
 object OrganizationsRoutes {

delta/app/src/test/scala/ch/epfl/bluebrain/nexus/delta/routes/OrganizationsRoutesSpec.scala

@@ -222,13 +222,25 @@ class OrganizationsRoutesSpec extends BaseRouteSpec with IOFromMap {
       }
     }
 
+    "fail to deprecate an organization if 'prune' is specified for deletion" in {
+      Delete("/v1/orgs/org2?rev=1&prune=true") ~> addCredentials(OAuth2BearerToken("alice")) ~> routes ~> check {
+        status shouldEqual StatusCodes.BadRequest
+      }
+    }
+
     "delete an organization" in {
       aclChecker.append(AclAddress.fromOrg(org2.label), caller.subject -> Set(orgsPermissions.delete)).accepted
       Delete("/v1/orgs/org2?prune=true") ~> addCredentials(OAuth2BearerToken("alice")) ~> routes ~> check {
         status shouldEqual StatusCodes.OK
       }
     }
 
+    "fail to delete an organization if 'prune' is false but no revision is specified for deprecation" in {
+      Delete("/v1/orgs/org2?prune=false") ~> addCredentials(OAuth2BearerToken("alice")) ~> routes ~> check {
+        status shouldEqual StatusCodes.BadRequest
+      }
+    }
+
     "fail when trying to delete a non-empty organization" in {
       aclChecker.append(AclAddress.fromOrg(org1.label), caller.subject -> Set(orgsPermissions.delete)).accepted
       Delete("/v1/orgs/org1?prune=true") ~> addCredentials(OAuth2BearerToken("alice")) ~> routes ~> check {

delta/sdk/src/main/scala/ch/epfl/bluebrain/nexus/delta/sdk/organizations/model/OrganizationRejection.scala

@@ -86,6 +86,11 @@ object OrganizationRejection {
   final case class OrganizationNonEmpty(label: Label)
       extends OrganizationRejection(s"Organization '$label' cannot be deleted since it contains at least one project.")
 
+  final case class InvalidDeleteRequest(label: Label)
+      extends OrganizationRejection(
+        s"Invalid organization '$label' DELETE request. To deprecate, 'rev' must be provided and 'prune' must be 'false' or omitted."
+      )
+
   /**
     * Rejection returned when the organization initialization could not be performed.
     *

docs/src/main/paradox/docs/delta/api/orgs-api.md

@@ -91,7 +91,9 @@ Response
 
 ## Delete
 
-Delete a organization containing no projects. If there is a project, returns 409 Conflict. 
+Permanently delete an organization containing no projects. If there is a project, returns 409 Conflict.
+
+The caller must have `organizations/delete` permissions.
 
 ```
 DELETE /v1/orgs/{label}?prune=true
@@ -100,11 +102,12 @@ DELETE /v1/orgs/{label}?prune=true
 ... where
 
 - `{label}`: String - is the user friendly name that identifies this organization.
+- `prune`: a flag to make permanent deletion explicit. To avoid mistakes, if `prune=true` then `rev` cannot be present and a 400 Bad Request will be returned.
 
 **Example**
 
 Request
-:   @@snip [delete.sh](assets/organizations/deprecate.sh)
+:   @@snip [delete.sh](assets/organizations/delete.sh)
 
 ## Fetch (current version)
 

tests/src/test/resources/admin/errors/invalid-delete-request.json

@@ -0,0 +1,5 @@
+{
+  "@context": "https://bluebrain.github.io/nexus/contexts/error.json",
+  "@type": "InvalidDeleteRequest",
+  "reason": "Invalid organization '{{orgId}}' DELETE request. To deprecate, 'rev' must be provided and 'prune' must be 'false' or omitted."
+}

tests/src/test/scala/ch/epfl/bluebrain/nexus/tests/admin/OrgsSpec.scala

@@ -250,7 +250,7 @@ class OrgsSpec extends BaseIntegrationSpec {
     "fail when revision is not provided" in {
       deltaClient.delete[Json](s"/orgs/$id", Leela) { (json, response) =>
         response.status shouldEqual StatusCodes.BadRequest
-        json shouldEqual jsonContentOf("/admin/errors/rev-not-provided.json")
+        json shouldEqual jsonContentOf("/admin/errors/invalid-delete-request.json", "orgId" -> id)
       }
     }