From 007e3747ef5e8ca5195286d9fd5f104187d4a933 Mon Sep 17 00:00:00 2001
From: "J. King" <jking@jkingweb.ca>
Date: Thu, 2 Mar 2017 20:47:00 -0500
Subject: [PATCH] Various database fixes

Authentication appears to be broken with a real database
---
 lib/Database.php                              |   8 +-
 lib/Db/{Common.php => AbstractDriver.php}     |   2 +-
 lib/Db/DriverSQLite3.php                      |   6 +-
 lib/User/InternalFunctions.php                |   4 +-
 tests/User/TestUserExternal.php               | 135 ------------------
 tests/User/TestUserInternalDriver.php         |  26 ++++
 tests/User/TestUserMockExternal.php           |  26 ++++
 tests/User/TestUserMockInternal.php           |  25 ++++
 tests/lib/Db/BindingTests.php                 |   1 -
 .../TestUser.php => lib/User/CommonTests.php} |  40 ++----
 tests/lib/User/Database.php                   |   8 ++
 tests/phpunit.xml                             |   5 +-
 12 files changed, 109 insertions(+), 177 deletions(-)
 rename lib/Db/{Common.php => AbstractDriver.php} (97%)
 delete mode 100644 tests/User/TestUserExternal.php
 create mode 100644 tests/User/TestUserInternalDriver.php
 create mode 100644 tests/User/TestUserMockExternal.php
 create mode 100644 tests/User/TestUserMockInternal.php
 rename tests/{User/TestUser.php => lib/User/CommonTests.php} (80%)

diff --git a/lib/Database.php b/lib/Database.php
index 8236958c..bb2ebba7 100644
--- a/lib/Database.php
+++ b/lib/Database.php
@@ -2,10 +2,10 @@
 declare(strict_types=1);
 namespace JKingWeb\NewsSync;
 use PasswordGenerator\Generator as PassGen;
+use PicoFeed\Reader\Reader;
+use PicoFeed\PicoFeedException;
 
 class Database {
-    use PicoFeed\Reader\Reader;
-    use PicoFeed\PicoFeedException;
 
     const SCHEMA_VERSION = 1;
     const FORMAT_TS      = "Y-m-d h:i:s";
@@ -26,7 +26,7 @@ class Database {
         $this->db = new $driver($data, INSTALL);
         $ver = $this->db->schemaVersion();
         if(!INSTALL && $ver < self::SCHEMA_VERSION) {
-            $this->db->update(self::SCHEMA_VERSION);
+            $this->db->schemaUpdate(self::SCHEMA_VERSION);
         }
     }
 
@@ -50,7 +50,7 @@ class Database {
         return $this->db->schemaVersion();
     }
 
-    public function dbUpdate(): bool {
+    public function schemaUpdate(): bool {
         if($this->db->schemaVersion() < self::SCHEMA_VERSION) return $this->db->update(self::SCHEMA_VERSION);
         return false;
     }
diff --git a/lib/Db/Common.php b/lib/Db/AbstractDriver.php
similarity index 97%
rename from lib/Db/Common.php
rename to lib/Db/AbstractDriver.php
index 169cbff0..fc2ac4ce 100644
--- a/lib/Db/Common.php
+++ b/lib/Db/AbstractDriver.php
@@ -3,7 +3,7 @@ declare(strict_types=1);
 namespace JKingWeb\NewsSync\Db;
 use JKingWeb\DrUUID\UUID as UUID;
 
-Trait Common {
+abstract class AbstractDriver implements Driver {
     protected $transDepth = 0;
 
     public function schemaVersion(): int {
diff --git a/lib/Db/DriverSQLite3.php b/lib/Db/DriverSQLite3.php
index 74c969de..84069260 100644
--- a/lib/Db/DriverSQLite3.php
+++ b/lib/Db/DriverSQLite3.php
@@ -2,9 +2,7 @@
 declare(strict_types=1);
 namespace JKingWeb\NewsSync\Db;
 
-class DriverSQLite3 implements Driver {
-    use Common;
-    
+class DriverSQLite3 extends AbstractDriver {    
     protected $db;
     protected $data;
     
@@ -47,7 +45,7 @@ class DriverSQLite3 implements Driver {
         return $this->query("PRAGMA user_version")->getSingle();
     }
 
-    public function update(int $to): bool {
+    public function schemaUpdate(int $to): bool {
         $ver = $this->schemaVersion();
         if(!$this->data->conf->dbSQLite3AutoUpd)  throw new Update\Exception("manual", ['version' => $ver, 'driver_name' => $this->driverName()]);
         if($ver >= $to) throw new Update\Exception("tooNew", ['difference' => ($ver - $to), 'current' => $ver, 'target' => $to, 'driver_name' => $this->driverName()]);
diff --git a/lib/User/InternalFunctions.php b/lib/User/InternalFunctions.php
index 5db1e6bf..6ea37cd4 100644
--- a/lib/User/InternalFunctions.php
+++ b/lib/User/InternalFunctions.php
@@ -13,7 +13,7 @@ trait InternalFunctions {
     function auth(string $user, string $password): bool {
         if(!$this->data->user->exists($user)) return false;
         $hash = $this->db->userPasswordGet($user);
-        if(!$hash) return false;
+        if($password==="" && $hash==="") return true;
         return password_verify($password, $hash);
     }
 
@@ -33,7 +33,7 @@ trait InternalFunctions {
         return $this->db->userList($domain);
     }
     
-    function userPasswordSet(string $user, string $newPassword = null, string $oldPassword = null): bool {
+    function userPasswordSet(string $user, string $newPassword = null, string $oldPassword = null): string {
         return $this->db->userPasswordSet($user, $newPassword);
     }
 
diff --git a/tests/User/TestUserExternal.php b/tests/User/TestUserExternal.php
deleted file mode 100644
index e7e28407..00000000
--- a/tests/User/TestUserExternal.php
+++ /dev/null
@@ -1,135 +0,0 @@
-<?php
-declare(strict_types=1);
-namespace JKingWeb\NewsSync;
-
-
-class TestUserExternal extends \PHPUnit\Framework\TestCase {
-    use Test\Tools;
-    
-	const USER1 = "john.doe@example.com";
-	const USER2 = "jane.doe@example.com";
-
-	protected $data;
-
-    function setUp() {
-		$drv = Test\User\DriverExternalMock::class;
-		$conf = new Conf();
-		$conf->userDriver = $drv;
-		$conf->userAuthPreferHTTP = true;
-		$this->data = new Test\RuntimeData($conf);
-		$this->data->db = new Test\User\Database($this->data);
-		$this->data->user = new User($this->data);
-		$this->data->user->authorizationEnabled(false);
-		$_SERVER['PHP_AUTH_USER'] = self::USER1;
-		$_SERVER['PHP_AUTH_PW'] = "secret";
-	}
-
-	function testListUsers() {
-		$this->assertCount(0,$this->data->user->list());
-	}
-	
-	function testCheckIfAUserDoesNotExist() {
-		$this->assertFalse($this->data->user->exists(self::USER1));
-	}
-
-	function testAddAUser() {
-		$this->data->user->add(self::USER1, "");
-		$this->assertCount(1,$this->data->user->list());
-	}
-
-	function testCheckIfAUserDoesExist() {
-		$this->data->user->add(self::USER1, "");
-		$this->assertTrue($this->data->user->exists(self::USER1));
-	}
-
-	function testAddADuplicateUser() {
-		$this->data->user->add(self::USER1, "");
-		$this->assertException("alreadyExists", "User");
-		$this->data->user->add(self::USER1, "");
-	}
-
-	function testAddMultipleUsers() {
-		$this->data->user->add(self::USER1, "");
-		$this->data->user->add(self::USER2, "");
-		$this->assertCount(2,$this->data->user->list());
-	}
-	
-	function testRemoveAUser() {
-		$this->data->user->add(self::USER1, "");
-		$this->assertCount(1,$this->data->user->list());
-		$this->data->user->remove(self::USER1);
-		$this->assertCount(0,$this->data->user->list());
-	}
-
-	function testRemoveAMissingUser() {
-		$this->assertException("doesNotExist", "User");
-		$this->data->user->remove(self::USER1);
-	}
-
-	function testAuthenticateAUser() {
-		$this->data->user->add(self::USER1, "secret");
-		$this->data->user->add(self::USER2, "");
-		$this->assertTrue($this->data->user->auth(self::USER1, "secret"));
-		$this->assertFalse($this->data->user->auth(self::USER1, "superman"));
-		$this->assertTrue($this->data->user->auth(self::USER2, ""));
-	}
-
-	function testChangeAPassword() {
-		$this->data->user->add(self::USER1, "secret");
-		$this->assertEquals("superman", $this->data->user->passwordSet(self::USER1, "superman"));
-		$this->assertTrue($this->data->user->auth(self::USER1, "superman"));
-		$this->assertFalse($this->data->user->auth(self::USER1, "secret"));
-		$this->assertEquals("", $this->data->user->passwordSet(self::USER1, ""));
-		$this->assertTrue($this->data->user->auth(self::USER1, ""));
-		$this->assertEquals($this->data->conf->userTempPasswordLength, strlen($this->data->user->passwordSet(self::USER1)));
-	}
-
-	function testChangeAPasswordForAMissingUser() {
-		$this->assertException("doesNotExist", "User");
-		$this->data->user->passwordSet(self::USER1, "superman");
-	}
-
-	function testGetThePropertiesOfAUser() {
-		$this->data->user->add(self::USER1, "secret");
-		$p = $this->data->user->propertiesGet(self::USER1);
-		$this->assertArrayHasKey('id', $p);
-		$this->assertArrayHasKey('name', $p);
-		$this->assertArrayHasKey('domain', $p);
-		$this->assertArrayHasKey('rights', $p);
-		$this->assertArrayNotHasKey('password', $p);
-		$this->assertEquals(self::USER1, $p['name']);
-	}
-
-	function testSetThePropertiesOfAUser() {
-		$pSet = [
-			'name'     => 'John Doe',
-			'id'       => 'invalid',
-			'domain'   => 'localhost',
-			'rights'   => User\Driver::RIGHTS_GLOBAL_ADMIN,
-			'password' => 'superman',
-		];
-		$pGet = [
-			'name'   => 'John Doe',
-			'id'     => self::USER1,
-			'domain' => 'example.com',
-			'rights' => User\Driver::RIGHTS_NONE,
-		];
-		$this->data->user->add(self::USER1, "secret");
-		$this->data->user->propertiesSet(self::USER1, $pSet);
-		$p = $this->data->user->propertiesGet(self::USER1);
-		$this->assertArraySubset($pGet, $p);
-		$this->assertArrayNotHasKey('password', $p);
-		$this->assertFalse($this->data->user->auth(self::USER1, "superman"));
-	}
-
-	function testGetTheRightsOfAUser() {
-		$this->data->user->add(self::USER1, "");
-		$this->assertEquals(User\Driver::RIGHTS_NONE, $this->data->user->rightsGet(self::USER1));
-	}
-
-	function testSetTheRightsOfAUser() {
-		$this->data->user->add(self::USER1, "");
-		$this->data->user->rightsSet(self::USER1, User\Driver::RIGHTS_GLOBAL_ADMIN);
-		$this->assertEquals(User\Driver::RIGHTS_GLOBAL_ADMIN, $this->data->user->rightsGet(self::USER1));
-	}
-}
\ No newline at end of file
diff --git a/tests/User/TestUserInternalDriver.php b/tests/User/TestUserInternalDriver.php
new file mode 100644
index 00000000..e47bd7fe
--- /dev/null
+++ b/tests/User/TestUserInternalDriver.php
@@ -0,0 +1,26 @@
+<?php
+declare(strict_types=1);
+namespace JKingWeb\NewsSync;
+
+
+class TestUserInternalDriver extends \PHPUnit\Framework\TestCase {
+    use Test\Tools, Test\User\CommonTests;
+    
+	const USER1 = "john.doe@example.com";
+	const USER2 = "jane.doe@example.com";
+
+	protected $data;
+
+    function setUp() {
+		$drv = User\DriverInternal::class;
+		$conf = new Conf();
+		$conf->userDriver = $drv;
+		$conf->userAuthPreferHTTP = true;
+		$this->data = new Test\RuntimeData($conf);
+		$this->data->db = new Test\User\Database($this->data);
+		$this->data->user = new User($this->data);
+		$this->data->user->authorizationEnabled(false);
+		$_SERVER['PHP_AUTH_USER'] = self::USER1;
+		$_SERVER['PHP_AUTH_PW'] = "secret";
+	}
+}
diff --git a/tests/User/TestUserMockExternal.php b/tests/User/TestUserMockExternal.php
new file mode 100644
index 00000000..296fddde
--- /dev/null
+++ b/tests/User/TestUserMockExternal.php
@@ -0,0 +1,26 @@
+<?php
+declare(strict_types=1);
+namespace JKingWeb\NewsSync;
+
+
+class TestUserMockExternal extends \PHPUnit\Framework\TestCase {
+    use Test\Tools, Test\User\CommonTests;
+    
+	const USER1 = "john.doe@example.com";
+	const USER2 = "jane.doe@example.com";
+
+	protected $data;
+
+    function setUp() {
+		$drv = Test\User\DriverExternalMock::class;
+		$conf = new Conf();
+		$conf->userDriver = $drv;
+		$conf->userAuthPreferHTTP = true;
+		$this->data = new Test\RuntimeData($conf);
+		$this->data->db = new Test\User\Database($this->data);
+		$this->data->user = new User($this->data);
+		$this->data->user->authorizationEnabled(false);
+		$_SERVER['PHP_AUTH_USER'] = self::USER1;
+		$_SERVER['PHP_AUTH_PW'] = "secret";
+	}
+}
\ No newline at end of file
diff --git a/tests/User/TestUserMockInternal.php b/tests/User/TestUserMockInternal.php
new file mode 100644
index 00000000..5dd3312e
--- /dev/null
+++ b/tests/User/TestUserMockInternal.php
@@ -0,0 +1,25 @@
+<?php
+declare(strict_types=1);
+namespace JKingWeb\NewsSync;
+
+
+class TestUserMockInternal extends \PHPUnit\Framework\TestCase {
+    use Test\Tools, Test\User\CommonTests;
+    
+	const USER1 = "john.doe@example.com";
+	const USER2 = "jane.doe@example.com";
+
+	protected $data;
+
+    function setUp() {
+		$drv = Test\User\DriverInternalMock::class;
+		$conf = new Conf();
+		$conf->userDriver = $drv;
+		$conf->userAuthPreferHTTP = true;
+		$this->data = new Test\RuntimeData($conf);
+		$this->data->user = new User($this->data);
+		$this->data->user->authorizationEnabled(false);
+		$_SERVER['PHP_AUTH_USER'] = self::USER1;
+		$_SERVER['PHP_AUTH_PW'] = "secret";
+	}
+}
diff --git a/tests/lib/Db/BindingTests.php b/tests/lib/Db/BindingTests.php
index a589ce1a..c09cee4a 100644
--- a/tests/lib/Db/BindingTests.php
+++ b/tests/lib/Db/BindingTests.php
@@ -2,7 +2,6 @@
 declare(strict_types=1);
 namespace JKingWeb\NewsSync\Test\Db;
 use JKingWeb\NewsSync\Db\Statement;
-use JKingWeb\NewsSync\Db\Driver;
 
 trait BindingTests {
 	
diff --git a/tests/User/TestUser.php b/tests/lib/User/CommonTests.php
similarity index 80%
rename from tests/User/TestUser.php
rename to tests/lib/User/CommonTests.php
index 60cfb4f5..5ec11e3b 100644
--- a/tests/User/TestUser.php
+++ b/tests/lib/User/CommonTests.php
@@ -1,28 +1,9 @@
 <?php
 declare(strict_types=1);
-namespace JKingWeb\NewsSync;
-
-
-class TestUser extends \PHPUnit\Framework\TestCase {
-    use Test\Tools;
-    
-	const USER1 = "john.doe@example.com";
-	const USER2 = "jane.doe@example.com";
-
-	protected $data;
-
-    function setUp() {
-		$drv = Test\User\DriverInternalMock::class;
-		$conf = new Conf();
-		$conf->userDriver = $drv;
-		$conf->userAuthPreferHTTP = true;
-		$this->data = new Test\RuntimeData($conf);
-		$this->data->user = new User($this->data);
-		$this->data->user->authorizationEnabled(false);
-		$_SERVER['PHP_AUTH_USER'] = self::USER1;
-		$_SERVER['PHP_AUTH_PW'] = "secret";
-	}
+namespace JKingWeb\NewsSync\Test\User;
+use JKingWeb\NewsSync\User\Driver;
 
+trait CommonTests {
 	function testListUsers() {
 		$this->assertCount(0,$this->data->user->list());
 	}
@@ -66,8 +47,11 @@ class TestUser extends \PHPUnit\Framework\TestCase {
 	}
 
 	function testAuthenticateAUser() {
+		$_SERVER['PHP_AUTH_USER'] = self::USER1;
+		$_SERVER['PHP_AUTH_PW'] = "secret";
 		$this->data->user->add(self::USER1, "secret");
 		$this->data->user->add(self::USER2, "");
+		$this->assertTrue($this->data->user->auth());
 		$this->assertTrue($this->data->user->auth(self::USER1, "secret"));
 		$this->assertFalse($this->data->user->auth(self::USER1, "superman"));
 		$this->assertTrue($this->data->user->auth(self::USER2, ""));
@@ -104,14 +88,14 @@ class TestUser extends \PHPUnit\Framework\TestCase {
 			'name'     => 'John Doe',
 			'id'       => 'invalid',
 			'domain'   => 'localhost',
-			'rights'   => User\Driver::RIGHTS_GLOBAL_ADMIN,
+			'rights'   => Driver::RIGHTS_GLOBAL_ADMIN,
 			'password' => 'superman',
 		];
 		$pGet = [
 			'name'   => 'John Doe',
 			'id'     => self::USER1,
 			'domain' => 'example.com',
-			'rights' => User\Driver::RIGHTS_NONE,
+			'rights' => Driver::RIGHTS_NONE,
 		];
 		$this->data->user->add(self::USER1, "secret");
 		$this->data->user->propertiesSet(self::USER1, $pSet);
@@ -123,12 +107,12 @@ class TestUser extends \PHPUnit\Framework\TestCase {
 
 	function testGetTheRightsOfAUser() {
 		$this->data->user->add(self::USER1, "");
-		$this->assertEquals(User\Driver::RIGHTS_NONE, $this->data->user->rightsGet(self::USER1));
+		$this->assertEquals(Driver::RIGHTS_NONE, $this->data->user->rightsGet(self::USER1));
 	}
 
 	function testSetTheRightsOfAUser() {
 		$this->data->user->add(self::USER1, "");
-		$this->data->user->rightsSet(self::USER1, User\Driver::RIGHTS_GLOBAL_ADMIN);
-		$this->assertEquals(User\Driver::RIGHTS_GLOBAL_ADMIN, $this->data->user->rightsGet(self::USER1));
+		$this->data->user->rightsSet(self::USER1, Driver::RIGHTS_GLOBAL_ADMIN);
+		$this->assertEquals(Driver::RIGHTS_GLOBAL_ADMIN, $this->data->user->rightsGet(self::USER1));
 	}
-}
+}
\ No newline at end of file
diff --git a/tests/lib/User/Database.php b/tests/lib/User/Database.php
index c241fbcd..7664cadd 100644
--- a/tests/lib/User/Database.php
+++ b/tests/lib/User/Database.php
@@ -76,4 +76,12 @@ class Database extends DriverSkeleton {
         if(!$this->userExists($user)) throw new Exception("doesNotExist", ["action" => __FUNCTION__, "user" => $user]);
         return parent::userRightsSet($user, $level);
     }
+
+    // specific to mock database
+
+    function userPasswordGet(string $user): string {
+        if(!$this->data->user->authorize($user, __FUNCTION__)) throw new ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]);
+        if(!$this->userExists($user)) throw new Exception("doesNotExist", ["action" => __FUNCTION__, "user" => $user]);
+        return $this->db[$user]['password'];
+    }
 }
\ No newline at end of file
diff --git a/tests/phpunit.xml b/tests/phpunit.xml
index b4b94ebc..27c9c01a 100644
--- a/tests/phpunit.xml
+++ b/tests/phpunit.xml
@@ -22,8 +22,9 @@
 </testsuite>
 
 <testsuite name="User management">
-    <file>User/TestUser.php</file>
-    <file>User/TestUserExternal.php</file>
+    <file>User/TestUserMockInternal.php</file>
+    <file>User/TestUserMockExternal.php</file>
+    <file>User/TestUserInternalDriver.php</file>
     <file>User/TestAuthorization.php</file>
 </testsuite>