Merge pull request #10 from portier/feat-testing

Test with client-tester, fix spec incompatibilities

Author: stephank

.editorconfig

@@ -0,0 +1,13 @@
+root = true
+
+[*]
+charset = utf-8
+trim_trailing_whitespace = true
+insert_final_newline = true
+indent_style = space
+indent_size = 2
+max_line_length = 80
+
+[*.php]
+indent_size = 4
+max_line_length = 120

.github/workflows/check.yml

@@ -51,3 +51,22 @@ jobs:
 
       - name: PHPUnit
         run: vendor/bin/phpunit --teamcity
+
+      - name: Set up Go
+        uses: actions/setup-go@v2
+        with:
+          go-version: ^1.16
+
+      - name: Go cache
+        uses: actions/cache@v2
+        with:
+          path: ~/go/pkg/mod
+          key: ${{ runner.os }}-go-${{ hashFiles('**/go.sum') }}
+          restore-keys: |
+            ${{ runner.os }}-go-
+
+      - name: Build tester
+        run: go get -v github.com/portier/client-tester
+
+      - name: Run test suite
+        run: ~/go/bin/client-tester -bin ./client-tester.php

client-tester.php

@@ -0,0 +1,50 @@
+#!/usr/bin/env php
+<?php
+
+$argv = $_SERVER['argv'];
+if (count($argv) !== 2) {
+    error_log('Broker required');
+    exit(1);
+}
+
+require_once __DIR__ . '/vendor/autoload.php';
+
+$client = new \Portier\Client\Client(
+    new \Portier\Client\MemoryStore(),
+    'http://imaginary-client.test/fake-verify-route'
+);
+$client->broker = $argv[1];
+
+$stdin = fopen('php://stdin', 'r');
+while (($line = fgets($stdin, 4096)) !== false) {
+    $cmd = explode("\t", trim($line));
+    switch ($cmd[0]) {
+    case 'echo':
+        echo "ok\t{$cmd[1]}\n";
+        break;
+    case 'auth':
+        try {
+            $authUrl = $client->authenticate($cmd[1]);
+            echo "ok\t{$authUrl}\n";
+        } catch (Throwable $err) {
+            $msg = implode("  ", explode("\n", $err->getMessage()));
+            echo "err\t{$msg}\n";
+        }
+        break;
+    case 'verify':
+        try {
+            $email = $client->verify($cmd[1]);
+            echo "ok\t{$email}\n";
+        } catch (Throwable $err) {
+            $msg = implode("  ", explode("\n", $err->getMessage()));
+            echo "err\t{$msg}\n";
+        }
+        break;
+    default:
+        error_log("invalid command: {$cmd[0]}");
+        exit(1);
+    }
+}
+if (!feof($stdin)) {
+    exit(1);
+}

src/Client.php

@@ -94,6 +94,11 @@ public static function normalize(string $email): string
      */
     public function authenticate(string $email): string
     {
+        $authEndpoint = $this->fetchDiscovery()->authorization_endpoint ?? null;
+        if (!is_string($authEndpoint)) {
+            throw new \Exception('No authorization_endpoint in discovery document');
+        }
+
         $nonce = $this->store->createNonce($email);
         $query = http_build_query([
             'login_hint' => $email,
@@ -104,7 +109,7 @@ public function authenticate(string $email): string
             'client_id' => $this->clientId,
             'redirect_uri' => $this->redirectUri,
         ]);
-        return $this->broker . '/auth?' . $query;
+        return $authEndpoint . '?' . $query;
     }
 
     /**
@@ -126,13 +131,12 @@ public function verify(string $token): string
         }
 
         // Fetch broker keys.
-        $discoveryUrl = $this->broker . '/.well-known/openid-configuration';
-        $discoveryDoc = $this->store->fetchCached('discovery', $discoveryUrl);
-        if (!isset($discoveryDoc->jwks_uri) || !is_string($discoveryDoc->jwks_uri)) {
-            throw new \Exception('Discovery document incorrectly formatted');
+        $jwksUri = $this->fetchDiscovery()->jwks_uri ?? null;
+        if (!is_string($jwksUri)) {
+            throw new \Exception('No jwks_uri in discovery document');
         }
 
-        $keysDoc = $this->store->fetchCached('keys', $discoveryDoc->jwks_uri);
+        $keysDoc = $this->store->fetchCached('keys', $jwksUri);
         if (!isset($keysDoc->keys) || !is_array($keysDoc->keys)) {
             throw new \Exception('Keys document incorrectly formatted');
         }
@@ -153,11 +157,13 @@ public function verify(string $token): string
         }
 
         // Validate the token claims.
+        $clock = \Lcobucci\Clock\SystemClock::fromUTC();
+        $leeway = new \DateInterval('PT' . $this->leeway . 'S');
         $constraints = [
             new JwtConstraint\SignedWith(new JwtSigner\Rsa\Sha256(), $publicKey),
             new JwtConstraint\IssuedBy($this->broker),
             new JwtConstraint\PermittedFor($this->clientId),
-            new JwtConstraint\LooseValidAt(\Lcobucci\Clock\SystemClock::fromUTC()),
+            new JwtConstraint\LooseValidAt($clock, $leeway),
         ];
         $jwt->validator()->assert($token, ...$constraints);
 
@@ -180,6 +186,15 @@ public function verify(string $token): string
         return $email;
     }
 
+    /**
+     * Fetches the OpenID discovery document from the broker.
+     */
+    private function fetchDiscovery(): \stdClass
+    {
+        $discoveryUrl = $this->broker . '/.well-known/openid-configuration';
+        return $this->store->fetchCached('discovery', $discoveryUrl);
+    }
+
     /**
      * Parse a JWK into a PEM public key.
      */

src/MemoryStore.php

@@ -0,0 +1,80 @@
+<?php
+
+namespace Portier\Client;
+
+/**
+ * A store implementation that keeps everything in-memory.
+ *
+ * This will often not work as expected, because PHP clears everything between
+ * requests. It only exists for testing purposes.
+ */
+class MemoryStore extends AbstractStore
+{
+    /** @var \stdClass[] */
+    private $cache;
+    /** @var \stdClass[] */
+    private $nonces;
+
+    /**
+     * Constructor
+     */
+    public function __construct()
+    {
+        parent::__construct();
+
+        $this->cache = [];
+        $this->nonces = [];
+    }
+
+    /**
+     * {@inheritDoc}
+     */
+    public function fetchCached(string $cacheId, string $url): \stdClass
+    {
+        $item = $this->cache[$cacheId] ?? null;
+        if ($item !== null && time() < $item->expires) {
+            return $item->data;
+        }
+
+        $res = $this->fetch($url);
+
+        $this->cache[$cacheId] = (object) [
+            'data' => $res->data,
+            'expires' => time() + $res->ttl,
+        ];
+
+        return $res->data;
+    }
+
+    /**
+     * {@inheritDoc}
+     */
+    public function createNonce(string $email): string
+    {
+        $nonce = $this->generateNonce($email);
+
+        $this->nonces[$nonce] = (object) [
+            'email' => $email,
+            'expires' => time() + (int) $this->nonceTtl,
+        ];
+
+        return $nonce;
+    }
+
+    /**
+     * {@inheritDoc}
+     */
+    public function consumeNonce(string $nonce, string $email): void
+    {
+        $item = $this->nonces[$nonce] ?? null;
+        if ($item !== null) {
+            unset($this->nonces[$nonce]);
+
+            if ($item->email === $email && time() < $item->expires) {
+                return;
+            }
+        }
+
+        throw new \Exception('Invalid or expired nonce');
+    }
+}

tests/ClientTest.php

@@ -2,7 +2,8 @@
 
 namespace Tests;
 
-use \Portier\Client;
+use Portier\Client;
+use Prophecy\Argument;
 
 class ClientTest extends \PHPUnit\Framework\TestCase
 {
@@ -38,25 +39,30 @@ public function testNormalize()
     public function testAuthenticate()
     {
         $store = $this->prophesize(Client\StoreInterface::class);
+        $store->fetchCached('discovery', Argument::type('string'))
+            ->willReturn((object) [
+                'authorization_endpoint' => 'http://imaginary-server.test/auth',
+            ])
+            ->shouldBeCalled();
         $store->createNonce('johndoe@example.com')
             ->willReturn('foobar')
             ->shouldBeCalled();
 
         $client = new Client\Client(
             $store->reveal(),
-            'https://example.com/callback'
+            'https://imaginary-client.test/callback'
         );
 
         $this->assertEquals(
             $client->authenticate('johndoe@example.com'),
-            'https://broker.portier.io/auth?' . http_build_query([
+            'http://imaginary-server.test/auth?' . http_build_query([
                 'login_hint' => 'johndoe@example.com',
                 'scope' => 'openid email',
                 'nonce' => 'foobar',
                 'response_type' => 'id_token',
                 'response_mode' => 'form_post',
-                'client_id' => 'https://example.com',
-                'redirect_uri' => 'https://example.com/callback',
+                'client_id' => 'https://imaginary-client.test',
+                'redirect_uri' => 'https://imaginary-client.test/callback',
             ])
         );
     }