fixs

docjyJ · Oct 5, 2024 · 76e0387 · 76e0387
1 parent 300acbe
commit 76e0387
Show file tree

Hide file tree

Showing 8 changed files with 54 additions and 26 deletions.
diff --git a/compose.mysql.yml b/compose.mysql.yml
@@ -11,6 +11,12 @@ services:
       - MYSQL_DATABASE=nextcloud
       - MARIADB_AUTO_UPGRADE=1
       - MARIADB_DISABLE_UPGRADE_BACKUP=1
+    healthcheck:
+      test: [ "CMD", "healthcheck.sh", "--connect", "--innodb_initialized" ]
+      start_period: 10s
+      interval: 10s
+      timeout: 5s
+      retries: 3
 
   nextcloud:
     image: nextcloud:apache
@@ -27,8 +33,10 @@ services:
       - NEXTCLOUD_ADMIN_USER=admin
       - NEXTCLOUD_ADMIN_PASSWORD=admin
     depends_on:
-      - mysql
-      - stalwart
+      mysql:
+        condition: service_healthy
+      stalwart:
+        condition: service_started
 
   phpmyadmin:
     image: phpmyadmin
@@ -39,7 +47,8 @@ services:
       - PMA_USER=root
       - PMA_PASSWORD=root
     depends_on:
-      - mysql
+      mysql:
+        condition: service_healthy
 
   stalwart:
     image: stalwartlabs/mail-server:latest

diff --git a/lib/Controller/ApiController.php b/lib/Controller/ApiController.php
@@ -18,6 +18,7 @@
 use OCP\AppFramework\OCSController;
 use OCP\DB\Exception;
 use OCP\IRequest;
+use Psr\Log\LoggerInterface;
 
 /**
  * @psalm-api
@@ -30,6 +31,7 @@ public function __construct(
 		IRequest                       $request,
 		private readonly ConfigService $configService,
 		private readonly UsersService  $usersService,
+        private readonly LoggerInterface $logger,
 	) {
 		parent::__construct($appName, $request);
 	}
@@ -49,7 +51,8 @@ public function list(): DataResponse {
 		try {
 			$result = $this->configService->findMany();
 		} catch (Exception $e) {
-			throw new OCSException(previous: $e);
+            $this->logger->error($e->getMessage(), ['exception' => $e]);
+            throw new OCSException($e->getMessage(), Http::STATUS_INTERNAL_SERVER_ERROR);
 		}
 		return new DataResponse($result);
 	}
@@ -71,7 +74,8 @@ public function post(string $endpoint, string $username, string $password): Data
 		try {
 			$result = $this->configService->create($endpoint, $username, $password);
 		} catch (Exception $e) {
-			throw new OCSException(previous: $e);
+            $this->logger->error($e->getMessage(), ['exception' => $e]);
+			throw new OCSException($e->getMessage(), Http::STATUS_INTERNAL_SERVER_ERROR);
 		}
 		unset($result['password']);
 		unset($result['health_expires']);
@@ -94,7 +98,8 @@ public function get(int $id): DataResponse {
 		try {
 			$result = $this->configService->findId($id);
 		} catch (Exception $e) {
-			throw new OCSException(previous: $e);
+            $this->logger->error($e->getMessage(), ['exception' => $e]);
+            throw new OCSException($e->getMessage(), Http::STATUS_INTERNAL_SERVER_ERROR);
 		}
 		if ($result === null) {
 			throw new OCSNotFoundException();
@@ -123,7 +128,8 @@ public function put(int $id, string $endpoint, string $username, string $passwor
 		try {
 			$result = $this->configService->updateCredentials($id, $endpoint, $username, $password);
 		} catch (Exception $e) {
-			throw new OCSException(previous: $e);
+            $this->logger->error($e->getMessage(), ['exception' => $e]);
+            throw new OCSException($e->getMessage(), Http::STATUS_INTERNAL_SERVER_ERROR);
 		}
 		if ($result === null) {
 			throw new OCSNotFoundException();
@@ -149,7 +155,8 @@ public function delete(int $id): DataResponse {
 		try {
 			$result = $this->configService->delete($id);
 		} catch (Exception $e) {
-			throw new OCSException(previous: $e);
+            $this->logger->error($e->getMessage(), ['exception' => $e]);
+            throw new OCSException($e->getMessage(), Http::STATUS_INTERNAL_SERVER_ERROR);
 		}
 		if ($result === null) {
 			throw new OCSNotFoundException();
@@ -175,7 +182,8 @@ public function getUsers(int $id): DataResponse {
 		try {
 			$result = $this->usersService->findMany($id);
 		} catch (Exception $e) {
-			throw new OCSException(previous: $e);
+            $this->logger->error($e->getMessage(), ['exception' => $e]);
+            throw new OCSException($e->getMessage(), Http::STATUS_INTERNAL_SERVER_ERROR);
 		}
 		return new DataResponse($result);
 	}
@@ -197,7 +205,8 @@ public function postUsers(int $id, string $uid): DataResponse {
 		try {
 			$user = $this->usersService->addUser($id, $uid);
 		} catch (Exception $e) {
-			throw new OCSException(previous: $e);
+            $this->logger->error($e->getMessage(), ['exception' => $e]);
+            throw new OCSException($e->getMessage(), Http::STATUS_INTERNAL_SERVER_ERROR);
 		}
 		if ($user === null) {
 			throw new OCSNotFoundException();
@@ -222,7 +231,8 @@ public function deleteUsers(int $id, string $uid): DataResponse {
 		try {
 			$user = $this->usersService->removeUser($id, $uid);
 		} catch (Exception $e) {
-			throw new OCSException(previous: $e);
+            $this->logger->error($e->getMessage(), ['exception' => $e]);
+            throw new OCSException($e->getMessage(), Http::STATUS_INTERNAL_SERVER_ERROR);
 		}
 		if ($user === null) {
 			throw new OCSNotFoundException();

diff --git a/lib/Migration/Version000100Date20240914153000.php b/lib/Migration/Version000100Date20240914153000.php
@@ -67,9 +67,11 @@ public function changeSchema(IOutput $output, Closure $schemaClosure, array $opt
 				'length' => 128,
 			]);
 			$table->addColumn('password', Types::STRING, [
+                'notnull' => false,
 				'length' => 300,
 			]);
 			$table->addColumn('quota', Types::BIGINT, [
+                'notnull' => false,
 				'length' => 4,
 			]);
 			$table->setPrimaryKey(['cid', 'uid']);

diff --git a/lib/Services/ConfigService.php b/lib/Services/ConfigService.php
@@ -21,7 +21,7 @@
  * @psalm-import-type StalwartServerConfig from ResponseDefinitions
  */
 class ConfigService {
-	private const TABLE_CONFIG = 'stalwart_config';
+	private const TABLE_CONFIG = 'stalwart_configs';
 
 	/** @psalm-suppress PossiblyUnusedMethod */
 	public function __construct(

diff --git a/lib/Services/ISqlService.php b/lib/Services/ISqlService.php
@@ -6,5 +6,5 @@
 
 interface ISqlService {
 	public function __construct(IConfig $config);
-	public function getStalwartConfig(int $config_id): array;
+	public function getStalwartConfig(int $config_id): string;
 }
diff --git a/lib/Services/MysqlService.php b/lib/Services/MysqlService.php
@@ -69,7 +69,7 @@ private static function parseQuery(string $query, int $cid, string $tableUsers,
 	/**
 	 * @throws Exception
 	 */
-	public function getStalwartConfig(int $config_id): array {
+	public function getStalwartConfig(int $config_id): string {
 		$dbType = $this->config->getSystemValueString('dbtype');
 		if ($dbType !== 'mysql') {
 			throw new Exception('This app only supports MySQL');
@@ -100,7 +100,7 @@ public function getStalwartConfig(int $config_id): array {
 		$tableAlias = $dbTablePrefix . UsersService::TABLE_EMAILS;
 		$config_name = $dbName . '_' . $config_id;
 
-		return [
+		return json_encode([
 			[
 				'prefix' => 'directory.' . $config_name,
 				'type' => 'Clear',
@@ -161,6 +161,6 @@ public function getStalwartConfig(int $config_id): array {
 					[ 'storage.directory.', $config_name ],
 				],
 			],
-		];
+		], JSON_THROW_ON_ERROR);
 	}
 }
diff --git a/lib/Services/StalwartAPIService.php b/lib/Services/StalwartAPIService.php
@@ -8,6 +8,7 @@
 use OCP\Http\Client\IClientService;
 use OCP\Http\Client\IResponse;
 use OCP\IConfig;
+use Psr\Log\LoggerInterface;
 use Throwable;
 
 class StalwartAPIService {
@@ -21,6 +22,7 @@ class StalwartAPIService {
 	 */
 	public function __construct(
 		private readonly IClientService $clientService,
+		private readonly LoggerInterface $logger,
 		IConfig $config,
 	) {
 		/** @psalm-suppress MixedAssignment */
@@ -74,31 +76,36 @@ private static function getExpiration(bool $success): DateTime {
 	 */
 	public function challenge(string $endpoint, string $username, string $password): array {
 		if ($username === '' || $password === '' || preg_match(self::URL_PATTERN, $endpoint) !== 1) {
+			$this->logger->warning('A Stalwart server configuration is invalid');
 			return [ServerStatus::InvalidConfig, self::getExpiration(false)];
 		}
 
 		$client = $this->clientService->newClient();
 		try {
 			$response = $client->post($endpoint . '/oauth', [
-				'body' => [
+				'body' => json_encode([
 					'type' => 'Code',
 					'client_id' => 'nextcloud',
 					'redirect_uri' => null
-				],
+				], JSON_THROW_ON_ERROR),
 				'headers' => ['Authorization' => self::genCred($username, $password)]
 			]);
-			return [
-				str_contains(self::readBody($response), '"is_admin":true') ? ServerStatus::Success : ServerStatus::NoAdmin,
-				self::getExpiration(true)
-			];
+			if (str_contains(self::readBody($response), '"is_admin":true')) {
+				return [ ServerStatus::Success, self::getExpiration(true) ];
+			} else {
+				$this->logger->warning('The user of a Stalwart server is not an admin');
+				return [ ServerStatus::NoAdmin, self::getExpiration(true) ];
+			}
 		} catch (Throwable $e) {
 			try {
 				$response = $client->getResponseFromThrowable($e);
+                $this->logger->warning($e->getMessage(), ['exception' => $e]);
 				return [
 					$response->getStatusCode() === 401 ? ServerStatus::Unauthorized : ServerStatus::ErrorServer,
 					self::getExpiration(false)
 				];
-			} catch (Throwable) {
+			} catch (Throwable $e) {
+                $this->logger->warning($e->getMessage(), ['exception' => $e]);
 				return [ServerStatus::ErrorNetwork, self::getExpiration(false)];
 			}
 		}

diff --git a/lib/Services/UsersService.php b/lib/Services/UsersService.php
@@ -48,7 +48,7 @@ public function findMany(int $id): array {
 		$q = $this->db->getQueryBuilder();
 		$result = $q->select('uid')
 			->from(self::TABLE_USERS)
-			->where($q->expr()->eq('config_id', $q->createNamedParameter($id, IQueryBuilder::PARAM_INT)))
+			->where($q->expr()->eq('cid', $q->createNamedParameter($id, IQueryBuilder::PARAM_INT)))
 			->executeQuery();
 		$entities = [];
 		while (true) {
@@ -85,7 +85,7 @@ public function addUser(int $cid, string $uid): ?array {
 					->setValue('uid', $q->createNamedParameter($uid))
 					->setValue('type', $q->createNamedParameter(self::TYPE_USER))
 					->setValue('display_name', $q->createNamedParameter($user['displayName']))
-					->setValue('password', $q->createNamedParameter(null, IQueryBuilder::PARAM_NULL))
+					->setValue('password', $q->createNamedParameter(''))
 					->setValue('quota', $q->createNamedParameter(null, IQueryBuilder::PARAM_NULL))
 					->executeStatement();
 
@@ -120,7 +120,7 @@ public function removeUser(int $id, string $uid): ?array {
 			try {
 				$q = $this->db->getQueryBuilder();
 				$q->delete(self::TABLE_USERS)
-					->where($q->expr()->eq('config_id', $q->createNamedParameter($id, IQueryBuilder::PARAM_INT)))
+					->where($q->expr()->eq('cid', $q->createNamedParameter($id, IQueryBuilder::PARAM_INT)))
 					->andWhere($q->expr()->eq('uid', $q->createNamedParameter($uid)))
 					->executeStatement();
 				$this->db->commit();