Validate records before storing them and compare ckeys after normalisation

README.md

@@ -68,6 +68,53 @@ Verifier-Server is a web server designed to handle BYOND user verification.
 
 - `GET /verified`: Retrieve the list of verified users.
 - `POST /verified`: Add or delete a verified user. Requires a valid token.
+- `GET /ss14verified` and `POST /ss14verified`: the same contract for Space Station 14
+  accounts, with an `ss14` field instead of `ckey`.
+
+A trailing slash is accepted (`/verified/`), and `/` is an alias of `/verified`.
+
+## HTTP contract
+
+This is the contract [Civilizationbot](https://github.com/Valgorithms/Civilizationbot)
+relies on for `approveme`; keep it backward compatible.
+
+### Records
+
+```json
+[{"discord": "123456789012345678", "ss13": "alice", "create_time": "2026-01-01 12:00:00"}]
+```
+
+Each record links exactly one Discord user id to one BYOND ckey. A `ckey` is
+stored in its normalised form: lowercased, with `.`, `_`, `-`, `+` and spaces
+removed, which is how BYOND and the bot compare keys.
+
+### `POST /verified`
+
+Body: `application/x-www-form-urlencoded` (the legacy `Field: value` header
+form is still accepted for in-process callers). Fields:
+
+| Field     | Required | Meaning |
+| --------- | -------- | ------- |
+| `token`   | yes      | Must equal the configured `TOKEN`. |
+| `discord` | insert   | Discord user id, 17 to 20 digits. |
+| `ckey`    | insert   | BYOND key, normalised before storage. |
+| `method`  | no       | `delete` removes the record matching `ckey`, else `discord`. Anything else inserts. |
+
+Status codes:
+
+| Status | Meaning |
+| ------ | ------- |
+| 200    | Inserted (body: the full list) or deleted (body: the removed records). |
+| 400    | `discord` or `ckey` is malformed. Nothing was stored. |
+| 401    | Missing or wrong `token`, or the server still uses the default token. |
+| 403    | The Discord id or the (normalised) ckey is already registered. |
+| 404    | `delete` found no matching record. |
+| 405    | Unsupported HTTP method. |
+
+### `GET /verified`
+
+Returns the full list as JSON. There is no read token; do not host the
+server on a public interface unless that is intended.
 
 ## Running Tests
 

src/VerifierServer/Endpoints/SS14VerifiedEndpoint.php

@@ -50,6 +50,24 @@ class SS14VerifiedEndpoint extends VerifiedEndpoint
     public function __construct(protected &$state)
     {}
 
+    /** SS14 user names keep their spelling; they only need to be printable and bounded. */
+    protected function validateRecord(string $discord, string &$account): ?string
+    {
+        if (preg_match(self::DISCORD_ID_PATTERN, $discord) !== 1) {
+            return 'discord must be a Discord user id (17 to 20 digits)';
+        }
+        $account = trim($account);
+        if ($account === '' || strlen($account) > 256 || preg_match('/[\x00-\x1F\x7F]/', $account) === 1) {
+            return 'ss14 must be a printable user name of at most 256 bytes';
+        }
+        return null;
+    }
+
+    protected function findAccount(string $account, array $list): int|string|false
+    {
+        return $this->findIndex('ss14', $account, $list);
+    }
+
     /**
      * Handles POST requests by parsing the request data and performing actions based on the method type.
      *
@@ -132,7 +150,13 @@ protected function post(ServerRequestInterface|string $request, int|string &$res
      */
     protected function __post(array &$list, string $ss14, string $discord, int|string &$response, array &$headers, string &$body): void
     {
-        if ($this->findIndex('discord', $discord, $list) !== false || $this->findIndex('ss14', $ss14, $list) !== false) {
+        if ($problem = $this->validateRecord($discord, $ss14)) {
+            $response = Response::STATUS_BAD_REQUEST;
+            $headers = ['Content-Type' => 'text/plain'];
+            $body = "Bad Request: $problem";
+            return;
+        }
+        if ($this->findIndex('discord', $discord, $list) !== false || $this->findAccount($ss14, $list) !== false) {
             $response = Response::STATUS_FORBIDDEN;
             $headers = ['Content-Type' => 'text/plain'];
             $body = 'Forbidden';

src/VerifierServer/Endpoints/VerifiedEndpoint.php

@@ -48,25 +48,66 @@
  */
 class VerifiedEndpoint extends Endpoint
 {
+    /** Discord user ids are snowflakes: 17 to 20 decimal digits. */
+    public const DISCORD_ID_PATTERN = '/^[0-9]{17,20}$/D';
+    /** A normalised BYOND ckey: lowercase letters and digits only. */
+    public const CKEY_PATTERN = '/^[a-z0-9]{1,256}$/D';
+
     private ?array $lookupSnapshot = null;
     private array $lookupColumns = [];
 
     /** Snapshot comparison also detects changes through SS14's reference accessor. */
-    protected function findIndex(string $column, string $value, ?array $list = null): int|string|false
+    protected function findIndex(string $column, string $value, ?array $list = null, ?callable $normalize = null): int|string|false
     {
         $list = $list ?? $this->state->getVerifyList();
         if ($this->lookupSnapshot !== $list) {
             $this->lookupSnapshot = $list;
             $this->lookupColumns = [];
         }
-        if (!array_key_exists($column, $this->lookupColumns)) {
+        $cacheKey = $normalize ? "$column#normalized" : $column;
+        if (!array_key_exists($cacheKey, $this->lookupColumns)) {
             $values = [];
             foreach ($list as $index => $item) {
-                if (array_key_exists($column, $item)) $values[$index] = $item[$column];
+                if (!array_key_exists($column, $item)) continue;
+                $values[$index] = $normalize && is_string($item[$column]) ? $normalize($item[$column]) : $item[$column];
             }
-            $this->lookupColumns[$column] = $values;
+            $this->lookupColumns[$cacheKey] = $values;
+        }
+        return array_search($normalize ? $normalize($value) : $value, $this->lookupColumns[$cacheKey]);
+    }
+
+    /**
+     * Reduces a BYOND key to its ckey form the same way Civilizationbot's
+     * sanitizeInput() and HostedVerificationSource::validate() do, so that
+     * two spellings of one account can never both be registered.
+     */
+    public static function normalizeCkey(string $key): string
+    {
+        return str_replace(['.', '_', '-', '+', ' '], '', strtolower(trim($key)));
+    }
+
+    /**
+     * Validates and canonicalises a record before it is stored.
+     *
+     * @return string|null A human-readable problem, or null when the record is acceptable.
+     *                     `$account` is replaced with its canonical spelling on success.
+     */
+    protected function validateRecord(string $discord, string &$account): ?string
+    {
+        if (preg_match(self::DISCORD_ID_PATTERN, $discord) !== 1) {
+            return 'discord must be a Discord user id (17 to 20 digits)';
         }
-        return array_search($value, $this->lookupColumns[$column]);
+        $account = self::normalizeCkey($account);
+        if (preg_match(self::CKEY_PATTERN, $account) !== 1) {
+            return 'ckey must be a BYOND key (letters and digits after normalisation)';
+        }
+        return null;
+    }
+
+    /** Index of a record whose account matches `$account` after normalisation, or false. */
+    protected function findAccount(string $account, array $list): int|string|false
+    {
+        return $this->findIndex('ss13', $account, $list, [self::class, 'normalizeCkey']);
     }
 
     public function __construct(protected &$state)
@@ -210,10 +251,11 @@ protected function post(ServerRequestInterface|string $request, int|string &$res
     /**
      * Handles the default verification process.
      *
-     * This method checks if the provided `ckey` or `discord` already exists in the list.
-     * If either exists, it sets the response to a 403 Forbidden status.
-     * If neither exists, it adds the new entry to the list, writes the updated list to a JSON file,
-     * updates the state, and sets the response to a 200 OK status.
+     * This method first validates the record (400 Bad Request when the Discord id or ckey
+     * is malformed), then checks if the provided `ckey` or `discord` already exists in the list,
+     * comparing ckeys after normalisation. If either exists, it sets the response to a
+     * 403 Forbidden status. If neither exists, it adds the new entry to the list, writes the
+     * updated list to a JSON file, updates the state, and sets the response to a 200 OK status.
      *
      * @param array  $list          The list of existing entries.
      * @param string $ckey          The ckey to be verified.
@@ -224,7 +266,13 @@ protected function post(ServerRequestInterface|string $request, int|string &$res
      */
     protected function __post(array &$list, string $ckey, string $discord, int|string &$response, array &$headers, string &$body): void
     {
-        if ($this->findIndex('discord', $discord, $list) !== false || $this->findIndex('ss13', $ckey, $list) !== false) {
+        if ($problem = $this->validateRecord($discord, $ckey)) {
+            $response = Response::STATUS_BAD_REQUEST;
+            $headers = ['Content-Type' => 'text/plain'];
+            $body = "Bad Request: $problem";
+            return;
+        }
+        if ($this->findIndex('discord', $discord, $list) !== false || $this->findAccount($ckey, $list) !== false) {
             $response = Response::STATUS_FORBIDDEN;
             $headers = ['Content-Type' => 'text/plain'];
             $body = 'Forbidden';

tests/isolated/RecordValidationTest.php

@@ -0,0 +1,118 @@
+<?php declare(strict_types=1);
+
+use PHPUnit\Framework\TestCase;
+use React\Http\Message\ServerRequest;
+use VerifierServer\Endpoints\SS14VerifiedEndpoint;
+use VerifierServer\Endpoints\VerifiedEndpoint;
+use VerifierServer\PersistentState;
+use VerifierServer\SS14PersistentState;
+
+final class ValidationMemoryState extends PersistentState
+{
+    public static function writeJson(string $file, array $data): void {}
+}
+final class ValidationMemorySS14State extends SS14PersistentState
+{
+    public static function writeJson(string $file, array $data): void {}
+}
+
+final class RecordValidationTest extends TestCase
+{
+    private function post(VerifiedEndpoint $endpoint, array $fields): array
+    {
+        $request = new ServerRequest('POST', '/verified', ['Content-Type' => 'application/x-www-form-urlencoded'], http_build_query($fields + ['token' => 'test-token']));
+        $status = 404; $headers = []; $body = '';
+        $endpoint->handle('POST', $request, $status, $headers, $body);
+        return [$status, $body];
+    }
+
+    private function endpoint(array $list = []): VerifiedEndpoint
+    {
+        $state = new ValidationMemoryState('test-token');
+        $state->setVerifyList($list, false);
+        return new VerifiedEndpoint($state);
+    }
+
+    /** @dataProvider malformedRecords */
+    public function testMalformedRecordsAreRejectedWith400(array $fields, string $problem): void
+    {
+        $endpoint = $this->endpoint();
+        [$status, $body] = $this->post($endpoint, $fields);
+        self::assertSame(400, $status);
+        self::assertStringContainsString($problem, $body);
+        self::assertSame([], $endpoint->getState()->getVerifyList());
+    }
+
+    public function malformedRecords(): array
+    {
+        return [
+            'no fields' => [[], 'discord'],
+            'short discord' => [['discord' => '123', 'ckey' => 'alice'], 'discord'],
+            'non-numeric discord' => [['discord' => '12345678901234567x', 'ckey' => 'alice'], 'discord'],
+            'mention' => [['discord' => '<@123456789012345678>', 'ckey' => 'alice'], 'discord'],
+            'empty ckey' => [['discord' => '123456789012345678', 'ckey' => ''], 'ckey'],
+            'ckey of separators only' => [['discord' => '123456789012345678', 'ckey' => '_ -.'], 'ckey'],
+            'control characters' => [['discord' => '123456789012345678', 'ckey' => "al\x01ice"], 'ckey'],
+            'unicode' => [['discord' => '123456789012345678', 'ckey' => 'alíce'], 'ckey'],
+        ];
+    }
+
+    public function testKeysAreStoredAsNormalisedCkeys(): void
+    {
+        $endpoint = $this->endpoint();
+        [$status, $body] = $this->post($endpoint, ['discord' => '123456789012345678', 'ckey' => ' Al_ice-Smith.Jr ']);
+        self::assertSame(200, $status);
+        self::assertSame('alicesmithjr', json_decode($body, true)[0]['ss13']);
+        self::assertSame('alicesmithjr', $endpoint->getState()->getVerifyList()[0]['ss13']);
+        self::assertSame(0, $endpoint->getIndex('', 'alicesmithjr'));
+    }
+
+    public function testAnotherSpellingOfARegisteredKeyIsADuplicate(): void
+    {
+        $endpoint = $this->endpoint([['discord' => '123456789012345678', 'ss13' => 'alice']]);
+        [$status] = $this->post($endpoint, ['discord' => '234567890123456789', 'ckey' => 'Al_ice']);
+        self::assertSame(403, $status);
+        $endpoint = $this->endpoint([['discord' => '123456789012345678', 'ss13' => 'Al Ice']]);
+        [$status] = $this->post($endpoint, ['discord' => '234567890123456789', 'ckey' => 'alice']);
+        self::assertSame(403, $status, 'legacy records are compared after normalisation too');
+        self::assertCount(1, $endpoint->getState()->getVerifyList());
+        [$status] = $this->post($endpoint, ['discord' => '234567890123456789', 'ckey' => 'bob']);
+        self::assertSame(200, $status);
+        self::assertCount(2, $endpoint->getState()->getVerifyList());
+    }
+
+    public function testLegacyHeaderRequestsAreValidatedToo(): void
+    {
+        $endpoint = $this->endpoint();
+        $status = 404; $headers = []; $body = '';
+        $endpoint->handle('POST', "token: test-token".PHP_EOL."discord: 42".PHP_EOL."ckey: alice", $status, $headers, $body);
+        self::assertSame(400, $status);
+        $endpoint->handle('POST', "token: test-token".PHP_EOL."discord: 123456789012345678".PHP_EOL."ckey: Alice", $status, $headers, $body);
+        self::assertSame(200, $status);
+        self::assertSame('alice', $endpoint->getState()->getVerifyList()[0]['ss13']);
+    }
+
+    public function testDeletionDoesNotRequireAValidRecord(): void
+    {
+        $endpoint = $this->endpoint([['discord' => '123456789012345678', 'ss13' => 'alice']]);
+        [$status] = $this->post($endpoint, ['method' => 'delete', 'discord' => 'alice', 'ckey' => 'alice']);
+        self::assertSame(200, $status);
+        self::assertSame([], $endpoint->getState()->getVerifyList());
+    }
+
+    public function testSs14NamesKeepTheirSpellingButMustBePrintable(): void
+    {
+        $state = new ValidationMemorySS14State('test-token');
+        $state->setVerifyList([], false);
+        $endpoint = new SS14VerifiedEndpoint($state);
+        [$status] = $this->post($endpoint, ['discord' => '123456789012345678', 'ss14' => 'Space Captain_42']);
+        self::assertSame(200, $status);
+        self::assertSame('Space Captain_42', $state->getVerifyList()[0]['ss14']);
+        [$status] = $this->post($endpoint, ['discord' => '234567890123456789', 'ss14' => "bad\x00name"]);
+        self::assertSame(400, $status);
+        [$status] = $this->post($endpoint, ['discord' => '123', 'ss14' => 'Someone']);
+        self::assertSame(400, $status);
+        [$status] = $this->post($endpoint, ['discord' => '234567890123456789', 'ss14' => 'Space Captain_42']);
+        self::assertSame(403, $status);
+    }
+}