Skip to content

Commit

Permalink
feat: per account imap and smtp debugging
Browse files Browse the repository at this point in the history
Signed-off-by: SebastianKrupinski <[email protected]>
  • Loading branch information
SebastianKrupinski committed Oct 29, 2024
1 parent 43cfb5a commit 421a8dc
Show file tree
Hide file tree
Showing 10 changed files with 153 additions and 6 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ jobs:
run: echo "SELECT * FROM mysql.slow_log WHERE sql_text LIKE '%oc_mail%' AND sql_text NOT LIKE '%information_schema%'" | mysql -h 127.0.0.1 -u root -pmy-secret-pw
- name: Print debug logs
if: ${{ always() }}
run: cat nextcloud/data/horde_*.log
run: cat nextcloud/data/mail-*-*-imap.log
- name: Report coverage
uses: codecov/codecov-action@b9fd7d16f6d7d1b5d2bec1a2887e65ceed900238 # v4.6.0
if: ${{ always() && matrix.db == 'mysql' }}
Expand Down
1 change: 1 addition & 0 deletions appinfo/info.xml
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ Learn more about the Nextcloud Ethical AI Rating [in our blog](https://nextcloud
<command>OCA\Mail\Command\CleanUp</command>
<command>OCA\Mail\Command\CreateAccount</command>
<command>OCA\Mail\Command\CreateTagMigrationJobEntry</command>
<command>OCA\Mail\Command\DebugAccount</command>
<command>OCA\Mail\Command\DeleteAccount</command>
<command>OCA\Mail\Command\DiagnoseAccount</command>
<command>OCA\Mail\Command\ExportAccount</command>
Expand Down
7 changes: 7 additions & 0 deletions lib/Account.php
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,13 @@ public function getUserId() {
return $this->account->getUserId();
}

/**
* @return int
*/
public function getDebug(): int {
return $this->account->getDebug();
}

/**
* Set the quota percentage
* @param Quota $quota
Expand Down
78 changes: 78 additions & 0 deletions lib/Command/DebugAccount.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
<?php

declare(strict_types=1);

/**
* SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/

namespace OCA\Mail\Command;

use OCA\Mail\Service\AccountService;
use OCP\AppFramework\Db\DoesNotExistException;
use Psr\Log\LoggerInterface;
use Symfony\Component\Console\Command\Command;
use Symfony\Component\Console\Input\InputArgument;
use Symfony\Component\Console\Input\InputInterface;
use Symfony\Component\Console\Input\InputOption;
use Symfony\Component\Console\Output\OutputInterface;

class DebugAccount extends Command {
protected const ARGUMENT_ACCOUNT_ID = 'account-id';
protected const OPTION_IMAP_DEFAULT = 'imap';
protected const OPTION_IMAP_FULL = 'imap-full';
protected const OPTION_SMTP_DEFAULT = 'smtp';

public function __construct(

Check warning on line 27 in lib/Command/DebugAccount.php

View check run for this annotation

Codecov / codecov/patch

lib/Command/DebugAccount.php#L27

Added line #L27 was not covered by tests
private AccountService $accountService,
private LoggerInterface $logger,
) {
parent::__construct();

Check warning on line 31 in lib/Command/DebugAccount.php

View check run for this annotation

Codecov / codecov/patch

lib/Command/DebugAccount.php#L31

Added line #L31 was not covered by tests
}

/**
* @return void
*/
protected function configure() {
$this->setName('mail:account:debug');
$this->setDescription('Enable or Disable IMAP/SMTP debugging on a account');
$this->addArgument(self::ARGUMENT_ACCOUNT_ID, InputArgument::REQUIRED);
$this->addOption(self::OPTION_IMAP_DEFAULT, null, InputOption::VALUE_NONE);
$this->addOption(self::OPTION_IMAP_FULL, null, InputOption::VALUE_NONE);
$this->addOption(self::OPTION_SMTP_DEFAULT, null, InputOption::VALUE_NONE);

Check warning on line 43 in lib/Command/DebugAccount.php

View check run for this annotation

Codecov / codecov/patch

lib/Command/DebugAccount.php#L37-L43

Added lines #L37 - L43 were not covered by tests
}

protected function execute(InputInterface $input, OutputInterface $output): int {
$accountId = (int)$input->getArgument(self::ARGUMENT_ACCOUNT_ID);
$imapDefault = $input->getOption(self::OPTION_IMAP_DEFAULT);
$imapFull = $input->getOption(self::OPTION_IMAP_FULL);
$smtpDefault = $input->getOption(self::OPTION_SMTP_DEFAULT);
$debug = 0;
$debugImapDefault = 1 << 0; // 1 (0000 0001)
$debugImapFull = 1 << 1; // 2 (0000 0010)
$debugSmtpDefault = 1 << 4; // 16 (0001 0000)

Check warning on line 54 in lib/Command/DebugAccount.php

View check run for this annotation

Codecov / codecov/patch

lib/Command/DebugAccount.php#L46-L54

Added lines #L46 - L54 were not covered by tests

try {
$account = $this->accountService->findById($accountId)->getMailAccount();
} catch (DoesNotExistException $e) {
$output->writeln("<error>Account $accountId does not exist</error>");
return 1;

Check warning on line 60 in lib/Command/DebugAccount.php

View check run for this annotation

Codecov / codecov/patch

lib/Command/DebugAccount.php#L57-L60

Added lines #L57 - L60 were not covered by tests
}

if ($imapDefault) {
$debug += $debugImapDefault;
} elseif ($imapFull) {
$debug += $debugImapFull;

Check warning on line 66 in lib/Command/DebugAccount.php

View check run for this annotation

Codecov / codecov/patch

lib/Command/DebugAccount.php#L63-L66

Added lines #L63 - L66 were not covered by tests
}

if ($smtpDefault) {
$debug += $debugSmtpDefault;

Check warning on line 70 in lib/Command/DebugAccount.php

View check run for this annotation

Codecov / codecov/patch

lib/Command/DebugAccount.php#L69-L70

Added lines #L69 - L70 were not covered by tests
}

$account->setDebug($debug);
$this->accountService->save($account);

Check warning on line 74 in lib/Command/DebugAccount.php

View check run for this annotation

Codecov / codecov/patch

lib/Command/DebugAccount.php#L73-L74

Added lines #L73 - L74 were not covered by tests

return 0;

Check warning on line 76 in lib/Command/DebugAccount.php

View check run for this annotation

Codecov / codecov/patch

lib/Command/DebugAccount.php#L76

Added line #L76 was not covered by tests
}
}
9 changes: 9 additions & 0 deletions lib/Db/MailAccount.php
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,8 @@
* @method void setSearchBody(bool $searchBody)
* @method bool|null getOooFollowsSystem()
* @method void setOooFollowsSystem(bool $oooFollowsSystem)
* @method int getDebug()
* @method void setDebug(int $debug)
*/
class MailAccount extends Entity {
public const SIGNATURE_MODE_PLAIN = 0;
Expand Down Expand Up @@ -183,6 +185,8 @@ class MailAccount extends Entity {
/** @var bool|null */
protected $oooFollowsSystem;

protected int $debug = 0;

/**
* @param array $params
*/
Expand Down Expand Up @@ -240,6 +244,9 @@ public function __construct(array $params = []) {
if (isset($params['outOfOfficeFollowsSystem'])) {
$this->setOutOfOfficeFollowsSystem($params['outOfOfficeFollowsSystem']);
}
if (isset($params['debug'])) {
$this->setDebug($params['debug']);
}

$this->addType('inboundPort', 'integer');
$this->addType('outboundPort', 'integer');
Expand All @@ -263,6 +270,7 @@ public function __construct(array $params = []) {
$this->addType('junkMailboxId', 'integer');
$this->addType('searchBody', 'boolean');
$this->addType('oooFollowsSystem', 'boolean');
$this->addType('debug', 'integer');
}

public function getOutOfOfficeFollowsSystem(): bool {
Expand Down Expand Up @@ -310,6 +318,7 @@ public function toJson() {
'junkMailboxId' => $this->getJunkMailboxId(),
'searchBody' => $this->getSearchBody(),
'outOfOfficeFollowsSystem' => $this->getOutOfOfficeFollowsSystem(),
'debug' => $this->getDebug(),
];

if (!is_null($this->getOutboundHost())) {
Expand Down
12 changes: 10 additions & 2 deletions lib/IMAP/IMAPClientFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,9 @@ class IMAPClientFactory {
private ITimeFactory $timeFactory;
private HordeCacheFactory $hordeCacheFactory;

private int $debugDefault = 1 << 0; // 1 (0000 0001)
private int $debugFull = 1 << 1; // 2 (0000 0010)

public function __construct(ICrypto $crypto,
IConfig $config,
ICacheFactory $cacheFactory,
Expand Down Expand Up @@ -117,8 +120,13 @@ public function getClient(Account $account, bool $useCache = true): Horde_Imap_C
'backend' => $this->hordeCacheFactory->newCache($account),
];
}
if ($this->config->getSystemValue('debug', false)) {
$params['debug'] = $this->config->getSystemValue('datadirectory') . '/horde_imap.log';
$debug = $account->getDebug();
if ($debug & $this->debugDefault || $debug & $this->debugFull) {
$fn = 'mail-' . $account->getUserId() . '-' . $account->getId() . '-imap.log';
$params['debug'] = $this->config->getSystemValue('datadirectory') . '/' . $fn;
if ($debug & $this->debugFull) {
$params['debug_literal'] = true;

Check warning on line 128 in lib/IMAP/IMAPClientFactory.php

View check run for this annotation

Codecov / codecov/patch

lib/IMAP/IMAPClientFactory.php#L128

Added line #L128 was not covered by tests
}
}

$client = new HordeImapClient($params);
Expand Down
38 changes: 38 additions & 0 deletions lib/Migration/Version4100Date20241028000000.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
<?php

declare(strict_types=1);

/**
* SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/

namespace OCA\Mail\Migration;

use Closure;
use OCP\DB\ISchemaWrapper;
use OCP\DB\Types;
use OCP\Migration\IOutput;
use OCP\Migration\SimpleMigrationStep;

class Version4100Date20241028000000 extends SimpleMigrationStep {

/**
* @param IOutput $output
* @param Closure(): ISchemaWrapper $schemaClosure
* @param array $options
* @return null|ISchemaWrapper
*/
public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper {
$schema = $schemaClosure();

Check warning on line 27 in lib/Migration/Version4100Date20241028000000.php

View check run for this annotation

Codecov / codecov/patch

lib/Migration/Version4100Date20241028000000.php#L26-L27

Added lines #L26 - L27 were not covered by tests

$accountsTable = $schema->getTable('mail_accounts');
if (!$accountsTable->hasColumn('debug')) {
$accountsTable->addColumn('debug', Types::SMALLINT, [
'notnull' => false,
'default' => 0,
]);

Check warning on line 34 in lib/Migration/Version4100Date20241028000000.php

View check run for this annotation

Codecov / codecov/patch

lib/Migration/Version4100Date20241028000000.php#L29-L34

Added lines #L29 - L34 were not covered by tests
}
return $schema;

Check warning on line 36 in lib/Migration/Version4100Date20241028000000.php

View check run for this annotation

Codecov / codecov/patch

lib/Migration/Version4100Date20241028000000.php#L36

Added line #L36 was not covered by tests
}
}
7 changes: 5 additions & 2 deletions lib/SMTP/SmtpClientFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ class SmtpClientFactory {
/** @var HostNameFactory */
private $hostNameFactory;

private int $debugDefault = 1 << 4; // 16 (0001 0000)

public function __construct(IConfig $config,
ICrypto $crypto,
HostNameFactory $hostNameFactory) {
Expand Down Expand Up @@ -77,8 +79,9 @@ public function create(Account $account): Horde_Mail_Transport {
$decryptedAccessToken,
);
}
if ($this->config->getSystemValue('debug', false)) {
$params['debug'] = $this->config->getSystemValue('datadirectory') . '/horde_smtp.log';
if ($account->getDebug() & $this->debugDefault) {
$fn = 'mail-' . $account->getUserId() . '-' . $account->getId() . '-smtp.log';
$params['debug'] = $this->config->getSystemValue('datadirectory') . '/' . $fn;

Check warning on line 84 in lib/SMTP/SmtpClientFactory.php

View check run for this annotation

Codecov / codecov/patch

lib/SMTP/SmtpClientFactory.php#L83-L84

Added lines #L83 - L84 were not covered by tests
}
return new Horde_Mail_Transport_Smtphorde($params);
}
Expand Down
4 changes: 3 additions & 1 deletion tests/Integration/Db/MailAccountTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ public function testToAPI() {
'editorMode' => 'html',
'provisioningId' => null,
'order' => 13,
'showSubscribedOnly' => null,
'showSubscribedOnly' => false,
'personalNamespace' => null,
'draftsMailboxId' => null,
'sentMailboxId' => null,
Expand All @@ -70,6 +70,7 @@ public function testToAPI() {
'snoozeMailboxId' => null,
'searchBody' => false,
'outOfOfficeFollowsSystem' => true,
'debug' => 0,
], $a->toJson());
}

Expand Down Expand Up @@ -107,6 +108,7 @@ public function testMailAccountConstruct() {
'snoozeMailboxId' => null,
'searchBody' => false,
'outOfOfficeFollowsSystem' => false,
'debug' => 0,
];
$a = new MailAccount($expected);
// TODO: fix inconsistency
Expand Down
1 change: 1 addition & 0 deletions tests/Integration/Framework/ImapTestAccount.php
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ public function createTestAccount(?string $userId = null) {
$mailAccount->setOutboundUser('[email protected]');
$mailAccount->setOutboundPassword(OC::$server->getCrypto()->encrypt('mypassword'));
$mailAccount->setOutboundSslMode('none');
$mailAccount->setDebug(1);
$acc = $accountService->save($mailAccount);

/** @var MailboxSync $mbSync */
Expand Down

0 comments on commit 421a8dc

Please sign in to comment.