From 7ad1a080094128b6ae48db3a262d8bd20a686ee1 Mon Sep 17 00:00:00 2001 From: Todd Burry Date: Fri, 12 Jun 2020 13:52:12 -0400 Subject: [PATCH 1/3] Add support for nonce re-use This PR allows `JsConnectServer::generateRequest()` to take a `FIELD_COOKIE` field in order to re-use a cookie from a previous call. This is useful when you have a site that can make several calls to `generateRequest()` and thus create race conditions with nonces. --- src/JsConnectServer.php | 22 ++++++++++++++++++---- tests/JsConnectServerTest.php | 23 +++++++++++++++++++++++ 2 files changed, 41 insertions(+), 4 deletions(-) diff --git a/src/JsConnectServer.php b/src/JsConnectServer.php index 7ff7c5e..a3e4327 100644 --- a/src/JsConnectServer.php +++ b/src/JsConnectServer.php @@ -7,11 +7,13 @@ namespace Vanilla\JsConnect; +use Firebase\JWT\ExpiredException; use Firebase\JWT\JWT; use Vanilla\JsConnect\Exceptions\InvalidValueException; class JsConnectServer extends JsConnect { const FIELD_NONCE = 'n'; + const FIELD_COOKIE = 'cookie'; /** * @var string @@ -41,13 +43,25 @@ public function setKeyStore(\ArrayAccess $keystore) { * @return array Returns an array in the format `[$requestUrl, $cookie]`. */ public function generateRequest(array $state = []): array { - $nonce = JWT::urlsafeB64Encode(openssl_random_pseudo_bytes(15)); - $cookie = $this->jwtEncode([self::FIELD_NONCE => $nonce]); + if (isset($state[self::FIELD_COOKIE])) { + // The cookie was already set. Make sure it's one of ours. + try { + $decodedCookie = $this->jwtDecode($state[self::FIELD_COOKIE]); + } catch (\Exception $ex) { + throw new InvalidValueException('Could not use supplied jsConnect SSO token: '.$ex->getMessage()); + } + $nonce = self::validateFieldExists(self::FIELD_NONCE, $decodedCookie, 'ssoToken'); + $cookie = $state[self::FIELD_COOKIE]; + unset($state[self::FIELD_COOKIE]); + } else { + $nonce = JWT::urlsafeB64Encode(openssl_random_pseudo_bytes(15)); + $cookie = $this->jwtEncode([self::FIELD_NONCE => $nonce]); + } $requestJWT = $this->jwtEncode([ - 'st' => $state + [ + 'st' => [ self::FIELD_NONCE => $nonce, - ], + ] + $state, 'rurl' => $this->getRedirectUrl(), ]); $requestUrl = $this->getAuthenticateUrlWithSeparator().http_build_query(['jwt' => $requestJWT]); diff --git a/tests/JsConnectServerTest.php b/tests/JsConnectServerTest.php index 3cff702..acc4662 100644 --- a/tests/JsConnectServerTest.php +++ b/tests/JsConnectServerTest.php @@ -109,4 +109,27 @@ public function testMissingCookie() { '' ); } + + /** + * I should be able to re-use my cookie if I pass it back to another generate request. + */ + public function testCookieReuse(): void { + list($_, $cookie) = $this->jsc->generateRequest(); + list($_, $cookie2) = $this->jsc->generateRequest([JsConnectServer::FIELD_COOKIE => $cookie]); + + $decoded = $this->jsc->jwtDecode($cookie); + $decoded2 = $this->jsc->jwtDecode($cookie2); + + $this->assertArrayNotHasKey(JsConnectServer::FIELD_COOKIE, $decoded2, "The cookie should be double encoded."); + $this->assertSame($decoded[JsConnectServer::FIELD_NONCE], $decoded2[JsConnectServer::FIELD_NONCE]); + } + + /** + * I should not be able to re-use any old cookie value for JsConnect. + */ + public function testCookieReuseError(): void { + $this->expectException(InvalidValueException::class); + $this->expectExceptionMessage("Could not use supplied jsConnect SSO token"); + list($_, $cookie) = $this->jsc->generateRequest([JsConnectServer::FIELD_COOKIE => 'cook']); + } } From 8833c725f1ef8c5be89c02fa9d00b8d43c0eca74 Mon Sep 17 00:00:00 2001 From: Todd Burry Date: Fri, 12 Jun 2020 13:53:49 -0400 Subject: [PATCH 2/3] Remove unnecessary use statement --- src/JsConnectServer.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/JsConnectServer.php b/src/JsConnectServer.php index a3e4327..b2dcba3 100644 --- a/src/JsConnectServer.php +++ b/src/JsConnectServer.php @@ -7,7 +7,6 @@ namespace Vanilla\JsConnect; -use Firebase\JWT\ExpiredException; use Firebase\JWT\JWT; use Vanilla\JsConnect\Exceptions\InvalidValueException; From 8d77a08f612ddfd7e414eb032338f185bb740ccb Mon Sep 17 00:00:00 2001 From: Todd Burry Date: Fri, 12 Jun 2020 14:20:16 -0400 Subject: [PATCH 3/3] =?UTF-8?q?Make=20sure=20a=20re-used=20cookie=20isn?= =?UTF-8?q?=E2=80=99t=20re-issued?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- tests/JsConnectServerTest.php | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/JsConnectServerTest.php b/tests/JsConnectServerTest.php index acc4662..fb9b353 100644 --- a/tests/JsConnectServerTest.php +++ b/tests/JsConnectServerTest.php @@ -115,13 +115,12 @@ public function testMissingCookie() { */ public function testCookieReuse(): void { list($_, $cookie) = $this->jsc->generateRequest(); + sleep(1); list($_, $cookie2) = $this->jsc->generateRequest([JsConnectServer::FIELD_COOKIE => $cookie]); + $this->assertSame($cookie, $cookie2); - $decoded = $this->jsc->jwtDecode($cookie); $decoded2 = $this->jsc->jwtDecode($cookie2); - $this->assertArrayNotHasKey(JsConnectServer::FIELD_COOKIE, $decoded2, "The cookie should be double encoded."); - $this->assertSame($decoded[JsConnectServer::FIELD_NONCE], $decoded2[JsConnectServer::FIELD_NONCE]); } /**