From 2946d950f2559dea004465866681c00725adc60c Mon Sep 17 00:00:00 2001
From: "J. King" <jking@jkingweb.ca>
Date: Sun, 27 Dec 2020 10:08:00 -0500
Subject: [PATCH] Forbid more user names

- Control characters are now forbidden
- Controls and colons are now also forbidden when renaming
---
 CHANGELOG                     |  9 +++++----
 lib/User.php                  | 13 ++++++++++---
 tests/cases/User/TestUser.php | 27 +++++++++++++++++++++------
 3 files changed, 36 insertions(+), 13 deletions(-)

diff --git a/CHANGELOG b/CHANGELOG
index a3847c4f..8580d40b 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,13 +1,14 @@
 Version 0.9.0 (????-??-??)
 ==========================
 
+New features:
+- Support for the Miniflux protocol (see manual for details)
+
 Bug fixes:
 - Use icons specified in Atom feeds when available
 - Do not return null as subscription unread count
-
-Changes:
-- Explicitly forbid U+003A COLON in usernames, for compatibility with HTTP
-  Basic authentication
+- Explicitly forbid U+003A COLON and control characters in usernames, for 
+  compatibility with RFC 7617
 
 Version 0.8.5 (2020-10-27)
 ==========================
diff --git a/lib/User.php b/lib/User.php
index d0bbbf80..accec103 100644
--- a/lib/User.php
+++ b/lib/User.php
@@ -84,10 +84,11 @@ class User {
     }
 
     public function add(string $user, ?string $password = null): string {
-        // ensure the user name does not contain any U+003A COLON characters, as
+        // ensure the user name does not contain any U+003A COLON or control characters, as
         // this is incompatible with HTTP Basic authentication
-        if (strpos($user, ":") !== false) {
-            throw new User\ExceptionInput("invalidUsername", "U+003A COLON");
+        if (preg_match("/[\x{00}-\x{1F}\x{7F}:]/", $user, $m)) {
+            $c = ord($m[0]);
+            throw new User\ExceptionInput("invalidUsername", "U+".str_pad((string) $c, 4, "0", \STR_PAD_LEFT)." ".\IntlChar::charName($c, \IntlChar::EXTENDED_CHAR_NAME));
         }
         try {
             $out = $this->u->userAdd($user, $password) ?? $this->u->userAdd($user, $this->generatePassword());
@@ -105,6 +106,12 @@ class User {
     }
 
     public function rename(string $user, string $newName): bool {
+        // ensure the new user name does not contain any U+003A COLON or 
+        // control characters, as this is incompatible with HTTP Basic authentication
+        if (preg_match("/[\x{00}-\x{1F}\x{7F}:]/", $newName, $m)) {
+            $c = ord($m[0]);
+            throw new User\ExceptionInput("invalidUsername", "U+".str_pad((string) $c, 4, "0", \STR_PAD_LEFT)." ".\IntlChar::charName($c, \IntlChar::EXTENDED_CHAR_NAME));
+        }
         if ($this->u->userRename($user, $newName)) {
             $tr = Arsse::$db->begin();
             if (!Arsse::$db->userExists($user)) {
diff --git a/tests/cases/User/TestUser.php b/tests/cases/User/TestUser.php
index c2a2645d..7c87e0c3 100644
--- a/tests/cases/User/TestUser.php
+++ b/tests/cases/User/TestUser.php
@@ -160,13 +160,22 @@ class TestUser extends \JKingWeb\Arsse\Test\AbstractTest {
         }
     }
 
-    public function testAddAnInvalidUser(): void {
-        $user = "john:doe@example.com";
-        $pass = "secret";
+    /** @dataProvider provideInvalidUserNames */
+    public function testAddAnInvalidUser(string $user): void {
         $u = new User($this->drv);
-        \Phake::when($this->drv)->userAdd->thenThrow(new ExceptionInput("invalidUsername"));
         $this->assertException("invalidUsername", "User", "ExceptionInput");
-        $u->add($user, $pass);
+        $u->add($user, "secret");
+    }
+
+    public function provideInvalidUserNames(): iterable {
+        // output names with control characters
+        foreach (array_merge(range(0x00, 0x1F), [0x7F]) as $ord) {
+            yield [chr($ord)];
+            yield ["john".chr($ord)."doe@example.com"];
+        }
+        // also handle colons
+        yield [":"];
+        yield ["john:doe@example.com"];
     }
 
     public function testAddAUserWithARandomPassword(): void {
@@ -231,11 +240,17 @@ class TestUser extends \JKingWeb\Arsse\Test\AbstractTest {
         \Phake::when($this->drv)->userRename->thenReturn(false);
         $u = new User($this->drv);
         $old = "john.doe@example.com";
-        $new  = "jane.doe@example.com";
         $this->assertFalse($u->rename($old, $old));
         \Phake::verify($this->drv)->userRename($old, $old);
     }
 
+    /** @dataProvider provideInvalidUserNames */
+    public function testRenameAUserToAnInvalidName(string $new): void {
+        $u = new User($this->drv);
+        $this->assertException("invalidUsername", "User", "ExceptionInput");
+        $u->rename("john.doe@example.com", $new);
+    }
+
     public function testRemoveAUser(): void {
         $user = "john.doe@example.com";
         $u = new User($this->drv);