Skip to content

Commit f4e0035

Browse files
SebastianKrupinskiSebastianKrupinski
committed
fix: automatically disable sab
Signed-off-by: SebastianKrupinski <krupinskis05@gmail.com>
1 parent 3f8f1b3 commit f4e0035

File tree

2 files changed

+128
-9
lines changed

2 files changed

+128
-9
lines changed

‎apps/dav/lib/CardDAV/SyncService.php‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010

1111
use OCP\AppFramework\Db\TTransactional;
1212
use OCP\AppFramework\Http;
13+
use OCP\AppFramework\Services\IAppConfig;
1314
use OCP\DB\Exception;
1415
use OCP\Http\Client\IClient;
1516
use OCP\Http\Client\IClientService;
@@ -38,6 +39,7 @@ public function __construct(
3839
private Converter $converter,
3940
private IClientService $clientService,
4041
private IConfig $config,
42+
private IAppConfig $appConfig,
4143
) {
4244
$this->certPath = '';
4345
}
@@ -239,6 +241,15 @@ private function parseMultiStatus($body) {
239241
* @param IUser $user
240242
*/
241243
public function updateUser(IUser $user): void {
244+
// automatically disable the system addressbook if the limit is reached
245+
if ($this->appConfig->getAppValueBool('system_addressbook_exposed', true)) {
246+
$limit = (int)$this->appConfig->getAppValueInt('system_addressbook_limit', 5000);
247+
// we use
248+
if ($this->userManager->countSeenUsers() >= $limit) {
249+
$this->appConfig->setAppValueBool('system_addressbook_exposed', false);
250+
}
251+
}
252+
242253
$systemAddressBook = $this->getLocalSystemAddressBook();
243254
$addressBookId = $systemAddressBook['id'];
244255

‎apps/dav/tests/unit/CardDAV/SyncServiceTest.php‎

Lines changed: 117 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
use OCA\DAV\CardDAV\CardDavBackend;
1515
use OCA\DAV\CardDAV\Converter;
1616
use OCA\DAV\CardDAV\SyncService;
17+
use OCP\AppFramework\Services\IAppConfig;
1718
use OCP\Http\Client\IClient;
1819
use OCP\Http\Client\IClientService;
1920
use OCP\IConfig;
@@ -36,30 +37,35 @@ class SyncServiceTest extends TestCase {
3637
protected Converter&MockObject $converter;
3738
protected IClient&MockObject $client;
3839
protected IConfig&MockObject $config;
40+
protected IAppConfig&MockObject $appConfig;
3941
protected SyncService $service;
4042

4143
public function setUp(): void {
4244
parent::setUp();
4345

46+
$this->backend = $this->createMock(CardDavBackend::class);
4447
$addressBook = [
4548
'id' => 1,
4649
'uri' => 'system',
4750
'principaluri' => 'principals/system/system',
4851
'{DAV:}displayname' => 'system',
4952
// watch out, incomplete address book mock.
5053
];
51-
52-
$this->backend = $this->createMock(CardDavBackend::class);
5354
$this->backend->method('getAddressBooksByUri')
54-
->with('principals/system/system', 1)
55-
->willReturn($addressBook);
55+
->willReturnCallback(function ($principal, $idOrUri) use ($addressBook) {
56+
if ($principal === 'principals/system/system' && ($idOrUri === 'system' || $idOrUri === 1 || $idOrUri === '1')) {
57+
return $addressBook;
58+
}
59+
return null;
60+
});
5661

5762
$this->userManager = $this->createMock(IUserManager::class);
5863
$this->dbConnection = $this->createMock(IDBConnection::class);
5964
$this->logger = new NullLogger();
6065
$this->converter = $this->createMock(Converter::class);
6166
$this->client = $this->createMock(IClient::class);
6267
$this->config = $this->createMock(IConfig::class);
68+
$this->appConfig = $this->createMock(IAppConfig::class);
6369

6470
$clientService = $this->createMock(IClientService::class);
6571
$clientService->method('newClient')
@@ -72,7 +78,8 @@ public function setUp(): void {
7278
$this->logger,
7379
$this->converter,
7480
$clientService,
75-
$this->config
81+
$this->config,
82+
$this->appConfig
7683
);
7784
}
7885

@@ -313,8 +320,9 @@ public function testEnsureSystemAddressBookExists(): void {
313320
$converter = $this->createMock(Converter::class);
314321
$clientService = $this->createMock(IClientService::class);
315322
$config = $this->createMock(IConfig::class);
323+
$appConfig = $this->createMock(IAppConfig::class);
316324

317-
$ss = new SyncService($backend, $userManager, $dbConnection, $logger, $converter, $clientService, $config);
325+
$ss = new SyncService($backend, $userManager, $dbConnection, $logger, $converter, $clientService, $config, $appConfig);
318326
$ss->ensureSystemAddressBookExists('principals/users/adam', 'contacts', []);
319327
}
320328

@@ -360,8 +368,9 @@ public function testUpdateAndDeleteUser(bool $activated, int $createCalls, int $
360368

361369
$clientService = $this->createMock(IClientService::class);
362370
$config = $this->createMock(IConfig::class);
371+
$appConfig = $this->createMock(IAppConfig::class);
363372

364-
$ss = new SyncService($backend, $userManager, $dbConnection, $logger, $converter, $clientService, $config);
373+
$ss = new SyncService($backend, $userManager, $dbConnection, $logger, $converter, $clientService, $config, $appConfig);
365374
$ss->updateUser($user);
366375

367376
$ss->updateUser($user);
@@ -402,8 +411,9 @@ public function testDeleteAddressbookWhenAccessRevoked(): void {
402411
$message = 'Client error: `REPORT https://server2.internal/cloud/remote.php/dav/addressbooks/system/system/system` resulted in a `401 Unauthorized` response:
403412
<?xml version="1.0" encoding="utf-8"?>
404413
<d:error xmlns:d="DAV:" xmlns:s="http://sabredav.org/ns">
405-
<s:exception>Sabre\DA (truncated...)
406-
';
414+
<s:exception>Sabre\DAV\Exception\NotAuthenticated</s:exception>
415+
<s:message>No public access to this resource., Username or password was incorrect, No \'Authorization: Bearer\' header found. Either the client didn\'t send one, or the server is mis-configured, Username or password was incorrect</s:message>
416+
</d:error>';
407417

408418
$reportException = new ClientException(
409419
$message,
@@ -481,4 +491,102 @@ public static function providerUseAbsoluteUriReport(): array {
481491
['https://server.internal:8080/nextcloud/', 'https://server.internal:8080/nextcloud/remote.php/dav/addressbooks/system/system/system'],
482492
];
483493
}
494+
495+
public function testUpdateUserDisablesSystemAddressBookWhenLimitReached(): void {
496+
$this->appConfig->method('getAppValueBool')
497+
->with('system_addressbook_exposed', true)
498+
->willReturn(true);
499+
$this->appConfig->method('getAppValueInt')
500+
->with('system_addressbook_limit', 5000)
501+
->willReturn(2);
502+
$this->userManager->method('countSeenUsers')->willReturn(2);
503+
$this->appConfig->expects($this->once())
504+
->method('setAppValueBool')
505+
->with('system_addressbook_exposed', false);
506+
$user = $this->createMock(IUser::class);
507+
$user->method('isEnabled')->willReturn(false);
508+
$user->method('getBackendClassName')->willReturn('unittest');
509+
$user->method('getUID')->willReturn('test-user');
510+
$this->backend->method('getAddressBooksByUri')
511+
->with('principals/system/system', 'system')
512+
->willReturn(['id' => 1]);
513+
$this->backend->expects($this->once())->method('deleteCard');
514+
$this->service->updateUser($user);
515+
}
516+
517+
public function testUpdateUserCreatesCardIfNotExistsAndEnabled(): void {
518+
$this->appConfig->method('getAppValueBool')->willReturn(true);
519+
$this->appConfig->method('getAppValueInt')->willReturn(5000);
520+
$user = $this->createMock(IUser::class);
521+
$user->method('isEnabled')->willReturn(true);
522+
$user->method('getBackendClassName')->willReturn('unittest');
523+
$user->method('getUID')->willReturn('test-user');
524+
$this->backend->method('getAddressBooksByUri')
525+
->with('principals/system/system', 'system')
526+
->willReturn(['id' => 1]);
527+
$this->backend->method('getCard')->willReturn(false);
528+
$vCard = $this->createMock(VCard::class);
529+
$vCard->method('serialize')->willReturn('VCARD-DATA');
530+
$this->converter->method('createCardFromUser')->willReturn($vCard);
531+
$this->backend->expects($this->once())
532+
->method('createCard')
533+
->with(1, 'unittest:test-user.vcf', 'VCARD-DATA', false);
534+
$this->service->updateUser($user);
535+
}
536+
537+
public function testUpdateUserUpdatesCardIfExistsAndEnabled(): void {
538+
$this->appConfig->method('getAppValueBool')->willReturn(true);
539+
$this->appConfig->method('getAppValueInt')->willReturn(5000);
540+
$user = $this->createMock(IUser::class);
541+
$user->method('isEnabled')->willReturn(true);
542+
$user->method('getBackendClassName')->willReturn('unittest');
543+
$user->method('getUID')->willReturn('test-user');
544+
$this->backend->method('getAddressBooksByUri')
545+
->with('principals/system/system', 'system')
546+
->willReturn(['id' => 1]);
547+
$this->backend->method('getCard')->willReturn(['carddata' => 'foo']);
548+
$vCard = $this->createMock(VCard::class);
549+
$vCard->method('serialize')->willReturn('VCARD-DATA');
550+
$this->converter->method('createCardFromUser')->willReturn($vCard);
551+
$this->backend->expects($this->once())
552+
->method('updateCard')
553+
->with(1, 'unittest:test-user.vcf', 'VCARD-DATA');
554+
$this->service->updateUser($user);
555+
}
556+
557+
public function testUpdateUserDeletesCardIfConverterReturnsNull(): void {
558+
$this->appConfig->method('getAppValueBool')->willReturn(true);
559+
$this->appConfig->method('getAppValueInt')->willReturn(5000);
560+
$user = $this->createMock(IUser::class);
561+
$user->method('isEnabled')->willReturn(true);
562+
$user->method('getBackendClassName')->willReturn('unittest');
563+
$user->method('getUID')->willReturn('test-user');
564+
$this->backend->method('getAddressBooksByUri')
565+
->with('principals/system/system', 'system')
566+
->willReturn(['id' => 1]);
567+
$this->backend->method('getCard')->willReturn(['carddata' => 'foo']);
568+
$this->converter->method('createCardFromUser')->willReturn(null);
569+
$this->backend->expects($this->once())
570+
->method('deleteCard')
571+
->with(1, 'unittest:test-user.vcf');
572+
$this->service->updateUser($user);
573+
}
574+
575+
public function testUpdateUserDeletesCardIfUserDisabled(): void {
576+
$this->appConfig->method('getAppValueBool')->willReturn(true);
577+
$this->appConfig->method('getAppValueInt')->willReturn(5000);
578+
$user = $this->createMock(IUser::class);
579+
$user->method('isEnabled')->willReturn(false);
580+
$user->method('getBackendClassName')->willReturn('unittest');
581+
$user->method('getUID')->willReturn('test-user');
582+
$this->backend->method('getAddressBooksByUri')
583+
->with('principals/system/system', 'system')
584+
->willReturn(['id' => 1]);
585+
$this->backend->expects($this->once())
586+
->method('deleteCard')
587+
->with(1, 'unittest:test-user.vcf');
588+
$this->service->updateUser($user);
589+
}
590+
591+
484592
}

0 commit comments

Comments
 (0)