Merge pull request #219 from advancedtelematic/feat/OTA-2367

Throw explicit error when adding a target to signed delegations file

cli/README.adoc

@@ -52,7 +52,12 @@ The basic use case for delegations works as follows.
 
 7. The supplier signs the delegations metadata and sends it to the OEM:
 
-    garage-sign delegations sign --inplace --input delegations01.json --key-name supplier01-key
+     garage-sign delegations sign --inplace --input delegations01.json --key-name supplier01-key
+
++
+After signing the delegations the file will need to be converted back to an unsigned file to be modified, for example to add another target. This can be done with:
+
+     jq .signed delegations01.json  > delegations01-unsigned.json
 
 8. The OEM pushes the signed role to the remote repository:
 

cli/src/main/scala/com/advancedtelematic/tuf/cli/Delegations.scala

@@ -11,12 +11,13 @@ import com.advancedtelematic.libtuf.data.ClientDataType.{ClientTargetItem, Deleg
 import com.advancedtelematic.libtuf.data.TufCodecs._
 import com.advancedtelematic.libtuf.data.TufDataType.{SignedPayload, TargetFilename, TufKey, TufPrivateKey}
 import com.advancedtelematic.libtuf.http.ReposerverClient
+import com.advancedtelematic.tuf.cli.Errors.DelegationsAlreadySigned
 import com.advancedtelematic.tuf.cli.TryToFuture._
-import io.circe.{Json, jawn}
+import io.circe.{DecodingFailure, Json, jawn}
 import io.circe.syntax._
 
 import scala.concurrent.{ExecutionContext, Future}
-import scala.util.Try
+import scala.util.{Failure, Success, Try}
 
 object Delegations {
   private val defaultExpirePeriod = Period.ofDays(365)
@@ -27,11 +28,19 @@ object Delegations {
     output.write(empty.asJson.spaces2.getBytes)
   }
 
-  private def read(input: Path): Try[Json] =
-    io.circe.jawn.parseFile(input.toFile).toTry
+  private def readUnsigned(input: Path): Try[Json] = {
+    io.circe.jawn.parseFile(input.toFile).toTry.flatMap { json =>
+      json.as[SignedPayload[Json]] match {
+        case Right(_) =>
+          Failure(DelegationsAlreadySigned(input))
+        case Left(_) =>
+          Success(json)
+      }
+    }
+  }
 
   def signPayload(keys: List[(TufKey, TufPrivateKey)], input : Path, output: WriteOutput): Try[Unit] = {
-    read(input).map { inJson =>
+    readUnsigned(input).map { inJson =>
       val sigs = keys.map { case (pub, priv) =>
         TufCrypto.signPayload(priv, inJson).toClient(pub.id)
       }
@@ -58,7 +67,7 @@ object Delegations {
   }
 
   def addTarget(input: Path, output: WriteOutput, targetFilename: TargetFilename, targetItem: ClientTargetItem): Try[Unit] = {
-    read(input).flatMap { json =>
+    readUnsigned(input).flatMap { json =>
       json.as[TargetsRole].toTry.map(json -> _)
     }.map { case (rawJson, targets) =>
       val newTargets = targets.copy(targets = Map(targetFilename -> targetItem))

cli/src/main/scala/com/advancedtelematic/tuf/cli/Errors.scala

@@ -1,5 +1,7 @@
 package com.advancedtelematic.tuf.cli
 
+import java.nio.file.Path
+
 import com.advancedtelematic.tuf.cli.DataType.TufServerType
 
 import scala.util.control.NoStackTrace
@@ -8,4 +10,5 @@ object Errors {
   case class CliArgumentsException(msg: String) extends Exception(msg) with NoStackTrace
   case class CliArgumentMissing(msg: String) extends Exception(msg)
   case class CommandNotSupportedByRepositoryType(repoType: TufServerType, msg: String) extends Exception(msg) with NoStackTrace
+  case class DelegationsAlreadySigned(path: Path) extends Exception(s"Delegations file $path is already signed, convert it to an unsigned file or provide an unsigned file")
 }

cli/src/test/scala/com/advancedtelematic/tuf/cli/DelegationsSpec.scala

@@ -12,8 +12,11 @@ import com.advancedtelematic.tuf.cli.util.CliSpec
 import io.circe.{Json, jawn}
 import cats.syntax.either._
 import com.advancedtelematic.libats.data.DataType.{HashMethod, ValidChecksum}
+import com.advancedtelematic.tuf.cli.Errors.DelegationsAlreadySigned
 import eu.timepit.refined._
 
+import scala.util.Failure
+
 class DelegationsSpec extends CliSpec {
   test("generates an empty delegations role to a given Path") {
     val p = new ByteArrayOutputStream()
@@ -43,6 +46,28 @@ class DelegationsSpec extends CliSpec {
     signedPayload.signed shouldBe Json.obj()
   }
 
+  test("returns descriptive error when adding a target to a signed delegations file") {
+    val in = Files.createTempFile("delegations", ".json")
+    Delegations.writeNew(new FileOutputStream(in.toFile)).get
+    val pair = KeyType.default.crypto.generateKeyPair()
+    val out = Files.createTempFile("delegations-signed", ".json")
+    val outOutput = WriteOutput.fromFile(out.toFile)
+
+    Delegations.signPayload(List(pair.pubkey -> pair.privkey), in, outOutput).get
+
+    val filename = refineV[ValidTargetFilename]("test-0.0.1").right.get
+
+    val item = ClientTargetItem(
+      Map(HashMethod.SHA256 -> refineV[ValidChecksum]("4c89194898360ebba34059bb8a5dd47a0650ca66b37c0143c32f7e70416e29e0").right.get),
+      1024,
+      custom = None
+    )
+
+    val result = Delegations.addTarget(out, WriteOutput.fromOutputStream(new ByteArrayOutputStream()), filename, item)
+
+    result.failed.get shouldBe DelegationsAlreadySigned(out)
+  }
+
   test("overwrites existing target") {
     val in = Files.createTempFile("payload", ".json")
     Delegations.writeNew(new FileOutputStream(in.toFile)).get