Refactor Cookie generation-&-parsing

pan-domain-auth-play/src/main/scala/com/gu/pandomainauth/action/Actions.scala

@@ -197,8 +197,8 @@ trait AuthActions {
     flushCookie(showUnauthedMessage("logged out"))
   }
 
-  def readAuthenticatedUser(request: RequestHeader): Option[AuthenticatedUser] = readCookie(request) map { cookie =>
-    CookieUtils.parseCookieData(cookie.cookie.value, settings.signingKeyPair.publicKey)
+  def readAuthenticatedUser(request: RequestHeader): Option[AuthenticatedUser] = readCookie(request) flatMap { cookie =>
+    CookieUtils.parseCookieData(cookie.cookie.value, settings.signingKeyPair.publicKey).toOption
   }
 
   def readCookie(request: RequestHeader): Option[PandomainCookie] = {

pan-domain-auth-verification/src/main/scala/com/gu/pandomainauth/PanDomain.scala

@@ -12,13 +12,9 @@ object PanDomain {
    */
   def authStatus(cookieData: String, publicKey: PublicKey, validateUser: AuthenticatedUser => Boolean,
                  apiGracePeriod: Long, system: String, cacheValidation: Boolean, forceExpiry: Boolean): AuthenticationStatus = {
-    try {
-      val authedUser = CookieUtils.parseCookieData(cookieData, publicKey)
+    CookieUtils.parseCookieData(cookieData, publicKey).fold(InvalidCookie, { authedUser =>
       checkStatus(authedUser, validateUser, apiGracePeriod, system, cacheValidation, forceExpiry)
-    } catch {
-      case e: Exception =>
-        InvalidCookie(e)
-    }
+    })
   }
 
   private def checkStatus(authedUser: AuthenticatedUser, validateUser: AuthenticatedUser => Boolean,

pan-domain-auth-verification/src/main/scala/com/gu/pandomainauth/model/AuthenticationStatus.scala

@@ -1,9 +1,11 @@
 package com.gu.pandomainauth.model
 
+import com.gu.pandomainauth.service.CookieUtils.CookieIntegrityFailure
+
 sealed trait AuthenticationStatus
 case class Expired(authedUser: AuthenticatedUser) extends AuthenticationStatus
 case class GracePeriod(authedUser: AuthenticatedUser) extends AuthenticationStatus
 case class Authenticated(authedUser: AuthenticatedUser) extends AuthenticationStatus
 case class NotAuthorized(authedUser: AuthenticatedUser) extends AuthenticationStatus
-case class InvalidCookie(exception: Exception) extends AuthenticationStatus
+case class InvalidCookie(e: CookieIntegrityFailure) extends AuthenticationStatus
 case object NotAuthenticated extends AuthenticationStatus

pan-domain-auth-verification/src/main/scala/com/gu/pandomainauth/service/CookiePayload.scala

@@ -0,0 +1,56 @@
+package com.gu.pandomainauth.service
+
+import com.gu.pandomainauth.service.CookiePayload.encodeBase64
+import org.apache.commons.codec.binary.Base64
+import org.apache.commons.codec.binary.Base64.isBase64
+
+import java.nio.charset.StandardCharsets.UTF_8
+import java.security.{PrivateKey, PublicKey}
+import scala.util.matching.Regex
+
+/**
+ * A representation of the underlying binary data (both payload & signature) in a Panda cookie.
+ *
+ * If an instance has been parsed from a cookie's text value, the existence of the instance
+ * *does not* imply that the signature has been verified. It only means that the cookie text was
+ * correctly formatted (two Base64 strings separated by '.').
+ *
+ * `CookiePayload` is designed to be the optimal representation of cookie data for checking
+ * signature-validity against *multiple* possible accepted public keys. It's a bridge between
+ * these two contexts:
+ *
+ * * cookie text: the raw cookie value - two Base64-encoded strings (payload & signature), separated by '.'
+ * * payload text: in Panda, a string representation of `AuthenticatedUser`
+ *
+ * To make those transformations, you need either a public or private key:
+ *
+ * * payload text -> cookie text: uses a *private* key to generate the signature
+ * * cookie text -> payload text: uses a *public* key to verify the signature
+ */
+case class CookiePayload(payloadBytes: Array[Byte], sig: Array[Byte]) {
+  def payloadTextVerifiedSignedWith(publicKey: PublicKey): Option[String] =
+    if (Crypto.verifySignature(payloadBytes, sig, publicKey)) Some(new String(payloadBytes, UTF_8)) else None
+
+  lazy val asCookieText: String = s"${encodeBase64(payloadBytes)}.${encodeBase64(sig)}"
+}
+
+object CookiePayload {
+  private val CookieRegEx: Regex = "^([\\w\\W]*)\\.([\\w\\W]*)$".r
+
+  private def encodeBase64(data: Array[Byte]): String = new String(Base64.encodeBase64(data), UTF_8)
+  private def decodeBase64(text: String): Array[Byte] = Base64.decodeBase64(text.getBytes(UTF_8))
+
+  /**
+   * @return `None` if the cookie text is incorrectly formatted (ie not "abc.xyz", with a '.' separator)
+   */
+  def parse(cookieText: String): Option[CookiePayload] = cookieText match {
+    case CookieRegEx(data, sig) if isBase64(data) && isBase64(sig) =>
+      Some(CookiePayload(decodeBase64(data), decodeBase64(sig)))
+    case _ => None
+  }
+
+  def generateForPayloadText(payloadText: String, privateKey: PrivateKey): CookiePayload = {
+    val data = payloadText.getBytes(UTF_8)
+    CookiePayload(data, Crypto.signData(data, privateKey))
+  }
+}

pan-domain-auth-verification/src/main/scala/com/gu/pandomainauth/service/CookieUtils.scala

@@ -1,10 +1,20 @@
 package com.gu.pandomainauth.service
 
-import java.security.{PrivateKey, PublicKey, SignatureException}
-import com.gu.pandomainauth.model.{AuthenticatedUser, CookieParseException, CookieSignatureInvalidException, User}
-import org.apache.commons.codec.binary.Base64
+import com.gu.pandomainauth.model.{AuthenticatedUser, User}
+import com.gu.pandomainauth.service.CookieUtils.CookieIntegrityFailure.{MalformedCookieText, MissingOrMalformedUserData, SignatureNotValid}
+
+import java.security.{PrivateKey, PublicKey}
+import scala.util.Try
 
 object CookieUtils {
+  sealed trait CookieIntegrityFailure
+  object CookieIntegrityFailure {
+    case object MalformedCookieText extends CookieIntegrityFailure
+    case object SignatureNotValid extends CookieIntegrityFailure
+    case object MissingOrMalformedUserData extends CookieIntegrityFailure
+  }
+
+  type CookieResult[A] = Either[CookieIntegrityFailure, A]
 
   private[service] def serializeAuthenticatedUser(authUser: AuthenticatedUser): String =
       s"firstName=${authUser.user.firstName}" +
@@ -16,48 +26,37 @@ object CookieUtils {
       s"&expires=${authUser.expires}" +
       s"&multifactor=${authUser.multiFactor}"
 
-  private[service] def deserializeAuthenticatedUser(serializedForm: String): AuthenticatedUser = {
+  private[service] def deserializeAuthenticatedUser(serializedForm: String): Option[AuthenticatedUser] = {
     val data = serializedForm
       .split("&")
       .map(_.split("=", 2))
       .map{p => p(0) -> p(1)}
       .toMap
 
-    AuthenticatedUser(
-      user = User(data("firstName"), data("lastName"), data("email"), data.get("avatarUrl")),
-      authenticatingSystem = data("system"),
-      authenticatedIn = Set(data("authedIn").split(",").toSeq :_*),
-      expires = data("expires").toLong,
-      multiFactor = data("multifactor").toBoolean
+    for {
+      firstName <- data.get("firstName")
+      lastName <- data.get("lastName")
+      email <- data.get("email")
+      system <- data.get("system")
+      authedIn <- data.get("authedIn")
+      expires <- data.get("expires").flatMap(text => Try(text.toLong).toOption)
+      multiFactor <- data.get("multifactor").flatMap(text => Try(text.toBoolean).toOption)
+    } yield AuthenticatedUser(
+      user = User(firstName, lastName, email, data.get("avatarUrl")),
+      authenticatingSystem = system,
+      authenticatedIn = Set(authedIn.split(",").toSeq :_*),
+      expires = expires,
+      multiFactor = multiFactor
     )
   }
 
-  def generateCookieData(authUser: AuthenticatedUser, prvKey: PrivateKey): String = {
-    val data = serializeAuthenticatedUser(authUser)
-    val encodedData = new String(Base64.encodeBase64(data.getBytes("UTF-8")))
-    val signature = Crypto.signData(data.getBytes("UTF-8"), prvKey)
-    val encodedSignature = new String(Base64.encodeBase64(signature))
+  def generateCookieData(authUser: AuthenticatedUser, prvKey: PrivateKey): String =
+    CookiePayload.generateForPayloadText(serializeAuthenticatedUser(authUser), prvKey).asCookieText
 
-    s"$encodedData.$encodedSignature"
-  }
-
-  lazy val CookieRegEx = "^^([\\w\\W]*)\\.([\\w\\W]*)$".r
-
-  def parseCookieData(cookieString: String, pubKey: PublicKey): AuthenticatedUser = {
-
-    cookieString match {
-      case CookieRegEx(data, sig) =>
-        try {
-          if (Crypto.verifySignature(Base64.decodeBase64(data.getBytes("UTF-8")), Base64.decodeBase64(sig.getBytes("UTF-8")), pubKey)) {
-            deserializeAuthenticatedUser(new String(Base64.decodeBase64(data.getBytes("UTF-8"))))
-          } else {
-            throw new CookieSignatureInvalidException
-          }
-        } catch {
-          case e: SignatureException =>
-            throw new CookieSignatureInvalidException
-        }
-      case _ => throw new CookieParseException
-    }
-  }
+  def parseCookieData(cookieString: String, publicKey: PublicKey): CookieResult[AuthenticatedUser] = for {
+    cookiePayload <- CookiePayload.parse(cookieString).toRight(MalformedCookieText)
+    cookiePayloadText <- cookiePayload.payloadTextVerifiedSignedWith(publicKey).toRight(SignatureNotValid)
+    authUser <- deserializeAuthenticatedUser(cookiePayloadText).toRight(MissingOrMalformedUserData)
+  } yield authUser
 }
+

pan-domain-auth-verification/src/test/scala/com/gu/pandomainauth/service/CookiePayloadTest.scala

@@ -0,0 +1,37 @@
+package com.gu.pandomainauth.service
+
+import org.scalatest.OptionValues
+import org.scalatest.freespec.AnyFreeSpec
+import org.scalatest.matchers.should.Matchers
+import TestKeys._
+
+class CookiePayloadTest extends AnyFreeSpec with Matchers with OptionValues {
+
+  "CookiePayload" - {
+    "round-trip from payload text to CookiePayload if matching public-private keys are used" in {
+      val payloadText = "Boom"
+      val payload = CookiePayload.generateForPayloadText(payloadText, testPrivateKey.key)
+      payload.payloadTextVerifiedSignedWith(testPublicKey.key).value shouldBe payloadText
+    }
+
+    "not return payload text if a rogue key was used" in {
+      val payload = CookiePayload.generateForPayloadText("Boom", testINCORRECTPrivateKey.key)
+      payload.payloadTextVerifiedSignedWith(testPublicKey.key) shouldBe None
+    }
+
+    "reject cookie text that does not contain a ." in {
+      CookiePayload.parse("AQIDBAUG") shouldBe None
+    }
+
+    "reject cookie text that does not contain valid BASE64 text" in {
+      CookiePayload.parse("AQ!D.BAUG") shouldBe None
+      CookiePayload.parse("AQID.BA!G") shouldBe None
+    }
+
+    "round-trip from correctly-formatted cookie-text to CookiePayload instance and back again" in {
+      val correctlyFormattedCookieText = "AQID.BAUG"
+      val cookiePayload = CookiePayload.parse(correctlyFormattedCookieText).value
+      cookiePayload.asCookieText shouldBe correctlyFormattedCookieText
+    }
+  }
+}

pan-domain-auth-verification/src/test/scala/com/gu/pandomainauth/service/CookieUtilsTest.scala

@@ -1,51 +1,48 @@
 package com.gu.pandomainauth.service
 
-import java.util.Date
-
-import com.gu.pandomainauth.model.{AuthenticatedUser, CookieParseException, CookieSignatureInvalidException, User}
+import com.gu.pandomainauth.model.{AuthenticatedUser, User}
+import com.gu.pandomainauth.service.CookieUtils.CookieIntegrityFailure.{MalformedCookieText, SignatureNotValid}
+import com.gu.pandomainauth.service.CookieUtils.{deserializeAuthenticatedUser, parseCookieData, serializeAuthenticatedUser}
 import org.scalatest.freespec.AnyFreeSpec
 import org.scalatest.matchers.should.Matchers
+import org.scalatest.{EitherValues, OptionValues}
+
+import java.util.Date
 
 
-class CookieUtilsTest extends AnyFreeSpec with Matchers {
+class CookieUtilsTest extends AnyFreeSpec with Matchers with EitherValues with OptionValues {
   import TestKeys._
 
   val authUser = AuthenticatedUser(User("test", "üsér", "[email protected]", None), "testsuite", Set("testsuite", "another"), new Date().getTime + 86400, multiFactor = true)
 
   "generateCookieData" - {
-    "generates a a base64-encoded 'data.signature' cookie value" in {
+    "generates a base64-encoded 'data.signature' cookie value" in {
       CookieUtils.generateCookieData(authUser, testPrivateKey.key) should fullyMatch regex "[\\w+/]+=*\\.[\\w+/]+=*".r
     }
   }
 
   "parseCookieData" - {
     "can extract an authenticatedUser from real cookie data" in {
       val cookieData = CookieUtils.generateCookieData(authUser, testPrivateKey.key)
-      CookieUtils.parseCookieData(cookieData, testPublicKey.key) should equal(authUser)
+
+      parseCookieData(cookieData, testPublicKey.key).value should equal(authUser)
     }
 
-    "fails to extract invalid data with a CookieSignatureInvalidException" in {
-      val cookieData = CookieUtils.generateCookieData(authUser, testINCORRECTPrivateKey.key)
-      intercept[CookieSignatureInvalidException] {
-        CookieUtils.parseCookieData("data.invalidSignature", testPublicKey.key)
-      }
+    "fails to extract invalid data with a SignatureNotValid" in {
+      parseCookieData("data.invalidSignature", testPublicKey.key).left.value shouldBe SignatureNotValid
     }
 
-    "fails to extract incorrectly signed data with a CookieSignatureInvalidException" - {
+    "fails to extract incorrectly signed data with a CookieSignatureInvalidException" in {
       val cookieData = CookieUtils.generateCookieData(authUser, testINCORRECTPrivateKey.key)
-      intercept[CookieSignatureInvalidException] {
-        CookieUtils.parseCookieData(cookieData, testPublicKey.key)
-      }
+      parseCookieData(cookieData, testPublicKey.key).left.value should equal(SignatureNotValid)
     }
 
-    "fails to extract completely incorrect cookie data with a CookieParseException" - {
-      intercept[CookieParseException] {
-        CookieUtils.parseCookieData("Completely incorrect cookie data", testPublicKey.key)
-      }
+    "fails to extract completely incorrect cookie data with a CookieParseException" in {
+      parseCookieData("Completely incorrect cookie data", testPublicKey.key).left.value shouldBe MalformedCookieText
     }
   }
 
   "serialize/deserialize preserves data" in {
-    CookieUtils.deserializeAuthenticatedUser(CookieUtils.serializeAuthenticatedUser(authUser)) should equal(authUser)
+    deserializeAuthenticatedUser(serializeAuthenticatedUser(authUser)).value shouldEqual authUser
   }
 }

project/Dependencies.scala

@@ -43,7 +43,7 @@ object Dependencies {
     "commons-codec" % "commons-codec" % "1.14"
   )
 
-  val testDependencies = Seq("org.scalatest" %% "scalatest" % "3.2.0" % "test")
+  val testDependencies = Seq("org.scalatest" %% "scalatest" % "3.2.19" % Test)
 
   val loggingDependencies = Seq("org.slf4j" % "slf4j-api" % "1.7.30")