Skip to content

Commit c2dc607

Browse files
hamza221hamza221
committed
feat(delegation): add unit tests
Signed-off-by: Hamza <hamzamahjoubi221@gmail.com>
1 parent d11d808 commit c2dc607

8 files changed

Lines changed: 635 additions & 23 deletions

File tree

‎lib/Controller/AccountsController.php‎

Lines changed: 40 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
use OCA\Mail\Model\NewMessageData;
2626
use OCA\Mail\Service\AccountService;
2727
use OCA\Mail\Service\AliasesService;
28+
use OCA\Mail\Service\DelegationService;
2829
use OCA\Mail\Service\SetupService;
2930
use OCA\Mail\Service\Sync\SyncService;
3031
use OCP\AppFramework\Controller;
@@ -52,6 +53,7 @@ class AccountsController extends Controller {
5253
private IConfig $config;
5354
private IRemoteHostValidator $hostValidator;
5455
private MailboxSync $mailboxSync;
56+
private DelegationService $delegationService;
5557

5658
public function __construct(
5759
string $appName,
@@ -69,6 +71,7 @@ public function __construct(
6971
IRemoteHostValidator $hostValidator,
7072
MailboxSync $mailboxSync,
7173
private ITimeFactory $timeFactory,
74+
DelegationService $delegationService,
7275
) {
7376
parent::__construct($appName, $request);
7477
$this->accountService = $accountService;
@@ -83,6 +86,7 @@ public function __construct(
8386
$this->config = $config;
8487
$this->hostValidator = $hostValidator;
8588
$this->mailboxSync = $mailboxSync;
89+
$this->delegationService = $delegationService;
8690
}
8791

8892
/**
@@ -122,7 +126,8 @@ public function index(): JSONResponse {
122126
*/
123127
#[TrapError]
124128
public function show(int $id): JSONResponse {
125-
return new JSONResponse($this->accountService->find($this->currentUserId, $id));
129+
$effectiveUserId = $this->delegationService->resolveAccountUserId($id, $this->currentUserId);
130+
return new JSONResponse($this->accountService->find($effectiveUserId, $id));
126131
}
127132

128133
/**
@@ -145,9 +150,10 @@ public function update(int $id,
145150
?string $imapPassword = null,
146151
?string $smtpPassword = null,
147152
string $authMethod = 'password'): JSONResponse {
153+
$effectiveUserId = $this->delegationService->resolveAccountUserId($id, $this->currentUserId);
148154
try {
149155
// Make sure the account actually exists
150-
$this->accountService->find($this->currentUserId, $id);
156+
$this->accountService->find($effectiveUserId, $id);
151157
} catch (ClientException $e) {
152158
return new JSONResponse([], Http::STATUS_BAD_REQUEST);
153159
}
@@ -173,9 +179,11 @@ public function update(int $id,
173179
}
174180

175181
try {
176-
return MailJsonResponse::success(
177-
$this->setup->createNewAccount($accountName, $emailAddress, $imapHost, $imapPort, $imapSslMode, $imapUser, $imapPassword, $smtpHost, $smtpPort, $smtpSslMode, $smtpUser, $smtpPassword, $this->currentUserId, $authMethod, $id)
182+
$result = MailJsonResponse::success(
183+
$this->setup->createNewAccount($accountName, $emailAddress, $imapHost, $imapPort, $imapSslMode, $imapUser, $imapPassword, $smtpHost, $smtpPort, $smtpSslMode, $smtpUser, $smtpPassword, $effectiveUserId, $authMethod, $id)
178184
);
185+
$this->delegationService->logDelegatedAction("$this->currentUserId updated account <$id> on behalf of $effectiveUserId");
186+
return $result;
179187
} catch (CouldNotConnectException $e) {
180188
$data = [
181189
'error' => $e->getReason(),
@@ -230,28 +238,29 @@ public function patchAccount(int $id,
230238
?bool $classificationEnabled = null,
231239
?bool $imipCreate = null,
232240
): JSONResponse {
233-
$account = $this->accountService->find($this->currentUserId, $id);
241+
$effectiveUserId = $this->delegationService->resolveAccountUserId($id, $this->currentUserId);
242+
$account = $this->accountService->find($effectiveUserId, $id);
234243

235244
$dbAccount = $account->getMailAccount();
236245

237246
if ($draftsMailboxId !== null) {
238-
$this->mailManager->getMailbox($this->currentUserId, $draftsMailboxId);
247+
$this->mailManager->getMailbox($effectiveUserId, $draftsMailboxId);
239248
$dbAccount->setDraftsMailboxId($draftsMailboxId);
240249
}
241250
if ($sentMailboxId !== null) {
242-
$this->mailManager->getMailbox($this->currentUserId, $sentMailboxId);
251+
$this->mailManager->getMailbox($effectiveUserId, $sentMailboxId);
243252
$dbAccount->setSentMailboxId($sentMailboxId);
244253
}
245254
if ($trashMailboxId !== null) {
246-
$this->mailManager->getMailbox($this->currentUserId, $trashMailboxId);
255+
$this->mailManager->getMailbox($effectiveUserId, $trashMailboxId);
247256
$dbAccount->setTrashMailboxId($trashMailboxId);
248257
}
249258
if ($archiveMailboxId !== null) {
250-
$this->mailManager->getMailbox($this->currentUserId, $archiveMailboxId);
259+
$this->mailManager->getMailbox($effectiveUserId, $archiveMailboxId);
251260
$dbAccount->setarchiveMailboxId($archiveMailboxId);
252261
}
253262
if ($snoozeMailboxId !== null) {
254-
$this->mailManager->getMailbox($this->currentUserId, $snoozeMailboxId);
263+
$this->mailManager->getMailbox($effectiveUserId, $snoozeMailboxId);
255264
$dbAccount->setSnoozeMailboxId($snoozeMailboxId);
256265
}
257266
if ($editorMode !== null) {
@@ -271,7 +280,7 @@ public function patchAccount(int $id,
271280
$dbAccount->setTrashRetentionDays($trashRetentionDays <= 0 ? null : $trashRetentionDays);
272281
}
273282
if ($junkMailboxId !== null) {
274-
$this->mailManager->getMailbox($this->currentUserId, $junkMailboxId);
283+
$this->mailManager->getMailbox($effectiveUserId, $junkMailboxId);
275284
$dbAccount->setJunkMailboxId($junkMailboxId);
276285
}
277286
if ($searchBody !== null) {
@@ -283,9 +292,11 @@ public function patchAccount(int $id,
283292
if ($imipCreate !== null) {
284293
$dbAccount->setImipCreate($imipCreate);
285294
}
286-
return new JSONResponse(
295+
$result = new JSONResponse(
287296
new Account($this->accountService->save($dbAccount))
288297
);
298+
$this->delegationService->logDelegatedAction("$this->currentUserId patched account <$id> on behalf of $effectiveUserId");
299+
return $result;
289300
}
290301

291302
/**
@@ -301,7 +312,9 @@ public function patchAccount(int $id,
301312
*/
302313
#[TrapError]
303314
public function updateSignature(int $id, ?string $signature = null): JSONResponse {
304-
$this->accountService->updateSignature($id, $this->currentUserId, $signature);
315+
$effectiveUserId = $this->delegationService->resolveAccountUserId($id, $this->currentUserId);
316+
$this->accountService->updateSignature($id, $effectiveUserId, $signature);
317+
$this->delegationService->logDelegatedAction("$this->currentUserId updated signature for account <$id> on behalf of $effectiveUserId");
305318
return new JSONResponse();
306319
}
307320

@@ -316,7 +329,9 @@ public function updateSignature(int $id, ?string $signature = null): JSONRespons
316329
*/
317330
#[TrapError]
318331
public function destroy(int $id): JSONResponse {
319-
$this->accountService->delete($this->currentUserId, $id);
332+
$effectiveUserId = $this->delegationService->resolveAccountUserId($id, $this->currentUserId);
333+
$this->accountService->delete($effectiveUserId, $id);
334+
$this->delegationService->logDelegatedAction("$this->currentUserId deleted account <$id> on behalf of $effectiveUserId");
320335
return new JSONResponse();
321336
}
322337

@@ -429,11 +444,12 @@ public function draft(int $id,
429444
$this->logger->info("Updating draft <$draftId> in account <$id>");
430445
}
431446

432-
$account = $this->accountService->find($this->currentUserId, $id);
447+
$effectiveUserId = $this->delegationService->resolveAccountUserId($id, $this->currentUserId);
448+
$account = $this->accountService->find($effectiveUserId, $id);
433449
$previousDraft = null;
434450
if ($draftId !== null) {
435451
try {
436-
$previousDraft = $this->mailManager->getMessage($this->currentUserId, $draftId);
452+
$previousDraft = $this->mailManager->getMessage($effectiveUserId, $draftId);
437453
} catch (ClientException $e) {
438454
$this->logger->info("Draft {$draftId} could not be loaded: {$e->getMessage()}");
439455
}
@@ -451,6 +467,7 @@ public function draft(int $id,
451467
null,
452468
[]
453469
);
470+
$this->delegationService->logDelegatedAction("$this->currentUserId saved draft in account <$id> on behalf of $effectiveUserId");
454471
return new JSONResponse([
455472
'id' => $this->mailManager->getMessageIdForUid($draftsMailbox, $newUID)
456473
]);
@@ -469,7 +486,8 @@ public function draft(int $id,
469486
* @throws ClientException
470487
*/
471488
public function getQuota(int $id): JSONResponse {
472-
$account = $this->accountService->find($this->currentUserId, $id);
489+
$effectiveUserId = $this->delegationService->resolveAccountUserId($id, $this->currentUserId);
490+
$account = $this->accountService->find($effectiveUserId, $id);
473491

474492
$quota = $this->mailManager->getQuota($account);
475493
if ($quota === null) {
@@ -488,9 +506,11 @@ public function getQuota(int $id): JSONResponse {
488506
* @throws ClientException
489507
*/
490508
public function updateSmimeCertificate(int $id, ?int $smimeCertificateId = null) {
491-
$account = $this->accountService->find($this->currentUserId, $id)->getMailAccount();
509+
$effectiveUserId = $this->delegationService->resolveAccountUserId($id, $this->currentUserId);
510+
$account = $this->accountService->find($effectiveUserId, $id)->getMailAccount();
492511
$account->setSmimeCertificateId($smimeCertificateId);
493512
$this->accountService->update($account);
513+
$this->delegationService->logDelegatedAction("$this->currentUserId updated S/MIME certificate for account <$id> on behalf of $effectiveUserId");
494514
return MailJsonResponse::success();
495515
}
496516

@@ -503,8 +523,9 @@ public function updateSmimeCertificate(int $id, ?int $smimeCertificateId = null)
503523
* @throws ClientException
504524
*/
505525
public function testAccountConnection(int $id) {
526+
$effectiveUserId = $this->delegationService->resolveAccountUserId($id, $this->currentUserId);
506527
return new JSONResponse([
507-
'data' => $this->accountService->testAccountConnection($this->currentUserId, $id),
528+
'data' => $this->accountService->testAccountConnection($effectiveUserId, $id),
508529
]);
509530
}
510531

‎lib/Controller/DelegationController.php‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,10 @@ public function getDelegatedUsers(int $accountId): JSONResponse {
6565
public function delegate(int $accountId, string $userId): JSONResponse {
6666

6767
$account = $this->accountService->findById($accountId);
68+
if ($this->currentUserId === null) {
69+
return new JSONResponse([], Http::STATUS_UNAUTHORIZED);
70+
}
71+
6872
if ($account->getUserId() !== $this->currentUserId) {
6973
return new JSONResponse([], Http::STATUS_UNAUTHORIZED);
7074
}
@@ -96,6 +100,11 @@ public function delegate(int $accountId, string $userId): JSONResponse {
96100
#[TrapError]
97101
public function unDelegate(int $accountId, string $userId): JSONResponse {
98102
$account = $this->accountService->findById($accountId);
103+
104+
if ($this->currentUserId === null) {
105+
return new JSONResponse([], Http::STATUS_UNAUTHORIZED);
106+
}
107+
99108
if ($account->getUserId() !== $this->currentUserId) {
100109
return new JSONResponse([], Http::STATUS_UNAUTHORIZED);
101110
}

‎lib/Controller/DraftsController.php‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ public function create(
126126

127127
$this->service->saveMessage($account, $message, $to, $cc, $bcc, $attachments);
128128
$id = $message->getId();
129-
$this->delegationService->logDelegatedAction("$this->currentUserId created draft: $id on behalf of $effectiveUserId");
129+
$this->delegationService->logDelegatedAction("$this->userId created draft: $id on behalf of $effectiveUserId");
130130
return JsonResponse::success($message, Http::STATUS_CREATED);
131131
}
132132

@@ -214,7 +214,7 @@ public function destroy(int $id): JsonResponse {
214214
$this->accountService->find($effectiveUserId, $message->getAccountId());
215215

216216
$this->service->deleteMessage($effectiveUserId, $message);
217-
$this->delegationService->logDelegatedAction("$this->currentUserId deleted draft: $id on behalf of $effectiveUserId");
217+
$this->delegationService->logDelegatedAction("$this->userId deleted draft: $id on behalf of $effectiveUserId");
218218
return JsonResponse::success('Message deleted', Http::STATUS_ACCEPTED);
219219
}
220220

@@ -231,7 +231,7 @@ public function move(int $id): JsonResponse {
231231
$account = $this->accountService->find($effectiveUserId, $message->getAccountId());
232232

233233
$this->service->sendMessage($message, $account);
234-
$this->delegationService->logDelegatedAction("$this->currentUserId moved draft: $id to the IMAP server on behalf of $effectiveUserId");
234+
$this->delegationService->logDelegatedAction("$this->userId moved draft: $id to the IMAP server on behalf of $effectiveUserId");
235235
return JsonResponse::success(
236236
'Message moved to IMAP', Http::STATUS_ACCEPTED
237237
);

‎lib/Service/DelegationService.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ public function logDelegatedAction(string $logMessage) {
134134
*/
135135
private function notify(string $userId, string $currentUserId, Account $account, bool $delegated) {
136136
$notification = $this->notificationManager->createNotification();
137-
$displayName = $this->userManager->get($currentUserId)->getDisplayName();
137+
$displayName = $this->userManager->get($currentUserId)?->getDisplayName() ?? $currentUserId;
138138
$time = $this->time->getDateTime('now');
139139
$notification
140140
->setApp('mail')

‎tests/Integration/MailboxSynchronizationTest.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
use OCA\Mail\Controller\MailboxesController;
1717
use OCA\Mail\Db\MessageMapper as DbMessageMapper;
1818
use OCA\Mail\Service\AccountService;
19+
use OCA\Mail\Service\DelegationService;
1920
use OCA\Mail\Service\Sync\ImapToDbSynchronizer;
2021
use OCA\Mail\Service\Sync\SyncService;
2122
use OCA\Mail\Tests\Integration\Framework\ImapTest;
@@ -51,6 +52,7 @@ protected function setUp(): void {
5152
Server::get(SyncService::class),
5253
Server::get(IConfig::class),
5354
Server::get(ITimeFactory::class),
55+
Server::get(DelegationService::class),
5456
);
5557

5658
$this->account = $this->createTestAccount('user12345');

‎tests/Unit/Controller/AccountsControllerTest.php‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
use OCA\Mail\IMAP\Sync\Response;
2222
use OCA\Mail\Service\AccountService;
2323
use OCA\Mail\Service\AliasesService;
24+
use OCA\Mail\Service\DelegationService;
2425
use OCA\Mail\Service\SetupService;
2526
use OCA\Mail\Service\Sync\SyncService;
2627
use OCP\AppFramework\Db\DoesNotExistException;
@@ -85,6 +86,9 @@ class AccountsControllerTest extends TestCase {
8586

8687
/** @var IConfig|(IConfig&MockObject)|MockObject */
8788
private IConfig|MockObject $config;
89+
90+
/** @var DelegationService|MockObject */
91+
private $delegationService;
8892
/** @var IRemoteHostValidator|MockObject */
8993
private $hostValidator;
9094

@@ -107,6 +111,9 @@ protected function setUp(): void {
107111
$this->hostValidator = $this->createMock(IRemoteHostValidator::class);
108112
$this->hostValidator->method('isValid')->willReturn(true);
109113
$this->timeFactory = $this->createMock(ITimeFactory::class);
114+
$this->delegationService = $this->createMock(DelegationService::class);
115+
$this->delegationService->method('resolveAccountUserId')
116+
->willReturn($this->userId);
110117

111118
$this->controller = new AccountsController(
112119
$this->appName,
@@ -124,6 +131,7 @@ protected function setUp(): void {
124131
$this->hostValidator,
125132
$this->mailboxSync,
126133
$this->timeFactory,
134+
$this->delegationService,
127135
);
128136
$this->account = $this->createMock(Account::class);
129137
$this->accountId = 123;

0 commit comments

Comments
 (0)