-
Notifications
You must be signed in to change notification settings - Fork 12
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Merge pull request #152 from guardian/update-cookie-generation-and-pa…
…rsing-code Refactor Cookie generation-&-parsing
- Loading branch information
Showing
8 changed files
with
154 additions
and
67 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
4 changes: 3 additions & 1 deletion
4
...in-auth-verification/src/main/scala/com/gu/pandomainauth/model/AuthenticationStatus.scala
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 |
56 changes: 56 additions & 0 deletions
56
pan-domain-auth-verification/src/main/scala/com/gu/pandomainauth/service/CookiePayload.scala
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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)) | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
37 changes: 37 additions & 0 deletions
37
...ain-auth-verification/src/test/scala/com/gu/pandomainauth/service/CookiePayloadTest.scala
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 | ||
| } | ||
| } | ||
| } |
37 changes: 17 additions & 20 deletions
37
...omain-auth-verification/src/test/scala/com/gu/pandomainauth/service/CookieUtilsTest.scala
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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", "test.user@example.com", 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 | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters