From df75df08ece7d8db12e7834fe8eb9573fd791a26 Mon Sep 17 00:00:00 2001
From: David Goodwin <david@codepoets.co.uk>
Date: Thu, 14 Jan 2021 17:05:50 +0000
Subject: [PATCH] update changelog; try and improve tests

---
 CHANGELOG.TXT         |  5 +++++
 model/PFAHandler.php  |  2 +-
 tests/LoginTest.php   | 30 ++++++++++++++++++++++++++++++
 tests/PacryptTest.php | 11 +++++++++--
 4 files changed, 45 insertions(+), 3 deletions(-)

diff --git a/CHANGELOG.TXT b/CHANGELOG.TXT
index 4fd502ff..e811364a 100644
--- a/CHANGELOG.TXT
+++ b/CHANGELOG.TXT
@@ -6,6 +6,11 @@
 #
 # Further details on the project are available at https://github.com/postfixadmin/postfixadmin
 
+Version 3.3.3 - 2021/01/14
+-------------------------------------------------
+   - Improve error handling around login (require non-empty password; cope with pacrypt() throwing an exception; see https://github.com/postfixadmin/postfixadmin/issues/420)
+   - Improve setup.php (show error messages in admin creation form, fix unable to create admin - see https://github.com/postfixadmin/postfixadmin/issues/418)
+
 Version 3.3.2 - 2021/01/13
 -------------------------------------------------
    - Add in the ability to specify a hash prefix with php_crypt password format, useful for Dovecot replacement. ( https://github.com/postfixadmin/postfixadmin/issues/344 )
diff --git a/model/PFAHandler.php b/model/PFAHandler.php
index d11f0cb0..1254e849 100644
--- a/model/PFAHandler.php
+++ b/model/PFAHandler.php
@@ -560,7 +560,7 @@ abstract class PFAHandler {
                     break;
                 case 'pass':
                     $val = (string) $val;
-                    $db_values[$key] = pacrypt($val);
+                    $db_values[$key] = pacrypt($val); // throws Exception
                     break;
                 case 'b64p':
                     $db_values[$key] = base64_encode($val);
diff --git a/tests/LoginTest.php b/tests/LoginTest.php
index 985c5655..68f950d9 100644
--- a/tests/LoginTest.php
+++ b/tests/LoginTest.php
@@ -2,8 +2,12 @@
 
 class LoginTest extends \PHPUnit\Framework\TestCase {
     public function setUp(): void {
+        global $CONF;
+
         $this->cleanUp();
 
+        $CONF['pacrypt'] = 'md5'; // crap
+
         db_execute("INSERT INTO domain(`domain`, description, transport) values ('example.com', 'test', 'foo')", [], true);
 
         db_execute(
@@ -40,6 +44,32 @@ VALUES(:username, :password, :name, :maildir, :local_part, :domain)",
     }
 
 
+    public function testEmptyStringWithDovecot() {
+        global $CONF;
+
+        if (!file_exists('/usr/bin/doveadm')) {
+            $this->markTestSkipped("/usr/bin/doveadm doesn't exist.");
+        }
+
+        $CONF['encrypt'] = 'dovecot:sha512';
+
+
+        db_execute(
+            "UPDATE mailbox SET password = :password WHERE username = :username",
+            [
+                'username' => 'test@example.com',
+                'password' => '{SHA512}ClAmHr0aOQ/tK/Mm8mc8FFWCpjQtUjIElz0CGTN/gWFqgGmwElh89WNfaSXxtWw2AjDBmyc1AO4BPgMGAb8kJQ==', // pacrypt('foobar'),
+            ]
+        );
+
+        $l = new Login('mailbox');
+        $this->assertFalse($l->login('test@example.com', ''));
+
+        $this->assertTrue($l->login('test@example.com', 'foobar'));
+
+        $this->assertFalse($l->login('test@fails.com', 'foobar'));
+    }
+
     public function testValidLogin() {
         $login = new Login('mailbox');
 
diff --git a/tests/PacryptTest.php b/tests/PacryptTest.php
index 614fbf17..ccf89579 100644
--- a/tests/PacryptTest.php
+++ b/tests/PacryptTest.php
@@ -11,7 +11,6 @@ class PaCryptTest extends \PHPUnit\Framework\TestCase {
     }
 
     public function testCrypt() {
-
         // E_NOTICE if we pass in '' for the salt
         $hash = _pacrypt_crypt('test', 'sa');
 
@@ -55,7 +54,8 @@ class PaCryptTest extends \PHPUnit\Framework\TestCase {
                 'md5' => 'CY9rzUYh03PK3k6DJie09g==',
                 // crypt requires salt ...
                 'SHA' => 'qUqP5cyxm6YcTAhz05Hph5gvu9M='
-            ] as $flavour => $hash) {
+            ] as $flavour => $hash
+        ) {
             $CONF['authlib_default_flavour'] = $flavour;
 
             $stored = "{" . $flavour . "}$hash";
@@ -80,6 +80,13 @@ class PaCryptTest extends \PHPUnit\Framework\TestCase {
         $this->assertEquals($expected_hash, _pacrypt_dovecot('test', ''));
 
         $this->assertEquals($expected_hash, _pacrypt_dovecot('test', $expected_hash));
+
+        // This should also work.
+        $sha512 = '{SHA512}ClAmHr0aOQ/tK/Mm8mc8FFWCpjQtUjIElz0CGTN/gWFqgGmwElh89WNfaSXxtWw2AjDBmyc1AO4BPgMGAb8kJQ=='; // foobar
+        $this->assertEquals($sha512, _pacrypt_dovecot('foobar', $sha512));
+
+        $sha512 = '{SHA512}ClAmHr0aOQ/tK/Mm8mc8FFWCpjQtUjIElz0CGTN/gWFqgGmwElh89WNfaSXxtWw2AjDBmyc1AO4BPgMGAb8kJQ=='; // foobar
+        $this->assertNotEquals($sha512, _pacrypt_dovecot('foobarbaz', $sha512));
     }
 
     public function testPhpCrypt() {