Merge pull request #6800 from ChurchCRM/slim-fix-system-upgrade

fix system updater to prevent TypeError
ChurchCRM · Jan 4, 2024 · 5faef0b · 5faef0b
2 parents dfe7226 + c130487
commit 5faef0b
Show file tree

Hide file tree

Showing 8 changed files with 66 additions and 47 deletions.
diff --git a/src/ChurchCRM/dto/ChurchCRMReleaseManager.php b/src/ChurchCRM/dto/ChurchCRMReleaseManager.php
@@ -233,7 +233,7 @@ public static function preShutdown(): void
         }
     }
 
-    public static function doUpgrade(string $zipFilename, string $sha1)
+    public static function doUpgrade(string $zipFilename, string $sha1): void
     {
         self::$isUpgradeInProgress = true;
         // temporarily disable PHP's error display so that
@@ -249,43 +249,53 @@ public static function doUpgrade(string $zipFilename, string $sha1)
         $logger->info('Beginning upgrade process');
         $logger->info('PHP max_execution_time is now: ' . ini_get('max_execution_time'));
         $logger->info('Beginning hash validation on ' . $zipFilename);
-        if ($sha1 === sha1_file($zipFilename)) {
-            $logger->info('Hash validation succeeded on ' . $zipFilename . ' Got: ' . sha1_file($zipFilename));
 
-            $zip = new \ZipArchive();
-            if ($zip->open($zipFilename) === true) {
-                $logger->info('Extracting ' . $zipFilename . ' to: ' . SystemURLs::getDocumentRoot() . '/Upgrade');
-
-                $executionTime = new ExecutionTime();
-                $isSuccessful = $zip->extractTo(SystemURLs::getDocumentRoot() . '/Upgrade');
-                MiscUtils::throwIfFailed($isSuccessful);
+        $actualSha1 = sha1_file($zipFilename);
+        if ($sha1 !== $actualSha1) {
+            self::$isUpgradeInProgress = false;
+            ini_set('display_errors', $displayErrors);
+            $message = 'hash validation failure';
+            $logger->error(
+                $message,
+                [
+                    'zipFilename' => $zipFilename,
+                    'expectedHash' => $sha1,
+                    'actualHash' => $actualSha1,
+                ]
+            );
+
+            throw new \Exception($message);
+        }
 
-                $zip->close();
+        $logger->info('Hash validation succeeded on ' . $zipFilename . ' Got: ' . $actualSha1);
 
-                $logger->info('Extraction completed.  Took:' . $executionTime->getMilliseconds());
-                $logger->info('Moving extracted zip into place');
+        $zip = new \ZipArchive();
+        if ($zip->open($zipFilename) === true) {
+            $logger->info('Extracting ' . $zipFilename . ' to: ' . SystemURLs::getDocumentRoot() . '/Upgrade');
 
-                $executionTime = new ExecutionTime();
+            $executionTime = new ExecutionTime();
+            $isSuccessful = $zip->extractTo(SystemURLs::getDocumentRoot() . '/Upgrade');
+            MiscUtils::throwIfFailed($isSuccessful);
 
-                FileSystemUtils::moveDir(SystemURLs::getDocumentRoot() . '/Upgrade/churchcrm', SystemURLs::getDocumentRoot());
-                $logger->info('Move completed.  Took:' . $executionTime->getMilliseconds());
-            }
-            $logger->info('Deleting zip archive: ' . $zipFilename);
-            unlink($zipFilename);
+            $zip->close();
 
-            SystemConfig::setValue('sLastIntegrityCheckTimeStamp', null);
-            $logger->debug('Set sLastIntegrityCheckTimeStamp to null');
-            $logger->info('Upgrade process complete');
-            ini_set('display_errors', $displayErrors);
-            self::$isUpgradeInProgress = false;
+            $logger->info('Extraction completed.  Took:' . $executionTime->getMilliseconds());
+            $logger->info('Moving extracted zip into place');
 
-            return 'success';
-        } else {
-            self::$isUpgradeInProgress = false;
-            ini_set('display_errors', $displayErrors);
-            $logger->error('Hash validation failed on ' . $zipFilename . '. Expected: ' . $sha1 . '. Got: ' . sha1_file($zipFilename));
+            $executionTime = new ExecutionTime();
 
-            return 'hash validation failure';
+            FileSystemUtils::moveDir(SystemURLs::getDocumentRoot() . '/Upgrade/churchcrm', SystemURLs::getDocumentRoot());
+            $logger->info('Move completed.  Took:' . $executionTime->getMilliseconds());
         }
+        $logger->info('Deleting zip archive: ' . $zipFilename);
+        unlink($zipFilename);
+
+        SystemConfig::setValue('sLastIntegrityCheckTimeStamp', null);
+        $logger->debug('Set sLastIntegrityCheckTimeStamp to null');
+        $logger->info('Upgrade process complete');
+        ini_set('display_errors', $displayErrors);
+        self::$isUpgradeInProgress = false;
+
+        return;
     }
 }
diff --git a/src/api/dependencies.php b/src/api/dependencies.php
@@ -4,7 +4,7 @@
 use ChurchCRM\Service\GroupService;
 use ChurchCRM\Service\PersonService;
 use ChurchCRM\Service\SystemService;
-use Psr\Container\ContainerInterface;
+use Symfony\Component\DependencyInjection\ContainerInterface;
 
 // DIC configuration
 /** @var ContainerInterface $container */

diff --git a/src/api/routes/people/people-properties.php b/src/api/routes/people/people-properties.php
@@ -12,6 +12,7 @@
 use ChurchCRM\Utils\LoggerUtils;
 use Psr\Http\Message\ResponseInterface as Response;
 use Psr\Http\Message\ServerRequestInterface as Request;
+use Slim\Exception\HttpNotFoundException;
 use Slim\Routing\RouteCollectorProxy;
 
 $app->group('/people/properties', function (RouteCollectorProxy $group): void {

diff --git a/src/api/routes/public/public-user.php b/src/api/routes/public/public-user.php
@@ -15,18 +15,20 @@
 
 function userLogin(Request $request, Response $response, array $args): Response
 {
-    $body = json_decode($request->getBody(), null, 512, JSON_THROW_ON_ERROR);
-    if (!empty($body->userName)) {
-        $user = UserQuery::create()->findOneByUserName($body->userName);
-        if (!empty($user)) {
-            $password = $body->password;
-            if (!$user->isPasswordValid($password)) {
-                throw new HttpUnauthorizedException($request, gettext('Invalid User/Password'));
-            }
+    $body = json_decode($request->getBody(), true, 512, JSON_THROW_ON_ERROR);
+    if (empty($body['userName'])) {
+        throw new HttpNotFoundException($request);
+    }
+
+    $user = UserQuery::create()->findOneByUserName($body['userName']);
+    if (empty($user)) {
+        throw new HttpNotFoundException($request);
+    }
 
-            return SlimUtils::renderJSON($response, ['apiKey' => $user->getApiKey()]);
-        }
+    $password = $body['password'];
+    if (!$user->isPasswordValid($password)) {
+        throw new HttpUnauthorizedException($request, gettext('Invalid User/Password'));
     }
 
-    throw new HttpNotFoundException($request);
+    return SlimUtils::renderJSON($response, ['apiKey' => $user->getApiKey()]);
 }
diff --git a/src/api/routes/system/system-database.php b/src/api/routes/system/system-database.php
@@ -168,7 +168,7 @@ function exportChMeetings(Request $request, Response $response, array $args): Re
             $person->getCellPhone(),
             $person->getHomePhone(),
             $person->getEmail(),
-            $person->getFacebookID(),
+            $person->getFacebook(),
             '',
             '',
             '',

diff --git a/src/api/routes/system/system-upgrade.php b/src/api/routes/system/system-upgrade.php
@@ -16,8 +16,8 @@
 
     $group->post('/doupgrade', function (Request $request, Response $response, array $args): Response {
         $input = $request->getParsedBody();
-        $upgradeResult = ChurchCRMReleaseManager::doUpgrade($input['fullPath'], $input['sha1']);
+        ChurchCRMReleaseManager::doUpgrade($input['fullPath'], $input['sha1']);
 
-        return SlimUtils::renderJSON($response, $upgradeResult);
+        return SlimUtils::renderSuccessJSON($response);
     });
 })->add(AdminRoleAuthMiddleware::class);
diff --git a/src/api/routes/users/user.php b/src/api/routes/users/user.php
@@ -30,7 +30,10 @@ function updateUserConfig(Request $request, Response $response, array $args): Re
     $newValue = $parsedBody['value'];
     $user->setUserConfigString($userConfigName, $newValue);
     $user->save();
-    if ($user->getUserConfigString($userConfigName) == $newValue) {
-        return SlimUtils::renderJSON($response, [$userConfigName => $newValue]);
+
+    if ($user->getUserConfigString($userConfigName) !== $newValue) {
+        throw new \Exception('user config string does not match provided value');
     }
+
+    return SlimUtils::renderJSON($response, [$userConfigName => $newValue]);
 }
diff --git a/src/v2/routes/user-current.php b/src/v2/routes/user-current.php
@@ -64,6 +64,9 @@ function changepassword(Request $request, Response $response, array $args): Resp
         // i.e. SSO will usually be a password change "portal," so we would redirect here.
         // but this will come later when we add more AuthenticationProviders
         RedirectUtils::absoluteRedirect($authenticationProvider->getPasswordChangeURL());
+
+        // unused but provided to complete the function signature
+        return $response;
     } else {
         // we're not using LocalAuth, and the AuthProvider does not specify a password change url
         // so tell the user we can't help them