Skip to content

Commit d64c8bf

Browse files
provokateurinprovokateurin
authored
Merge pull request #62550 from nextcloud/feat/sharing/search-recipiets-omit-existing
feat(Sharing): Allow omitting existing recipients when searching for recipients
2 parents c50f1e3 + 9ea9500 commit d64c8bf

9 files changed

Lines changed: 212 additions & 92 deletions

File tree

‎apps/sharing/lib/Controller/ApiV1Controller.php‎

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -75,18 +75,20 @@ public function __construct(
7575
/**
7676
* Search for recipients that can be added to a share.
7777
*
78-
* @param ?list<class-string<IShareRecipientType>> $recipientTypeClasses Type class of recipients to filter by
78+
* @param ?list<class-string<IShareRecipientType>> $filterRecipientTypeClasses Type classes of recipients to filter by
7979
* @param string $query The query to search for
8080
* @param int<1, 100> $limit The maximum number of participants
8181
* @param non-negative-int $offset The offset of the participants
82-
* @return DataResponse<Http::STATUS_OK, list<SharingRecipient>, array{}>|DataResponse<Http::STATUS_BAD_REQUEST, string, array{}>
82+
* @param ?string $id If provided, recipients that are already part of the share will not be returned.
83+
* @return DataResponse<Http::STATUS_OK, list<SharingRecipient>, array{}>|DataResponse<Http::STATUS_BAD_REQUEST|Http::STATUS_NOT_FOUND, string, array{}>
8384
*
8485
* 200: Recipients returned
8586
* 400: Invalid recipient search parameters
87+
* 404: Share used for filtering existing recipients does not exist
8688
*/
8789
#[NoAdminRequired]
8890
#[ApiRoute(verb: 'GET', url: '/api/v1/recipients')]
89-
public function searchRecipients(?array $recipientTypeClasses, string $query, int $limit = 10, int $offset = 0): DataResponse {
91+
public function searchRecipients(?array $filterRecipientTypeClasses, string $query, int $limit = 10, int $offset = 0, ?string $id = null): DataResponse {
9092
/** @psalm-suppress DocblockTypeContradiction */
9193
if ($limit < 1) {
9294
return new DataResponse('The limit is too low.', Http::STATUS_BAD_REQUEST);
@@ -103,10 +105,17 @@ public function searchRecipients(?array $recipientTypeClasses, string $query, in
103105
}
104106

105107
try {
106-
$recipients = $this->manager->searchRecipients($this->accessContext, $recipientTypeClasses, $query, $limit, $offset);
107-
return new DataResponse(ShareRecipient::formatMultiple($this->registry, $this->l10nFactory, $this->urlGenerator, $this->userManager, $recipients));
108-
} catch (ShareInvalidException $shareInvalidException) {
109-
return new DataResponse($shareInvalidException->getHint(), Http::STATUS_BAD_REQUEST);
108+
try {
109+
$this->dbConnection->beginTransaction();
110+
$recipients = $this->manager->searchRecipients($this->accessContext, $filterRecipientTypeClasses, $query, $limit, $offset, $id);
111+
$this->dbConnection->commit();
112+
return new DataResponse(ShareRecipient::formatMultiple($this->registry, $this->l10nFactory, $this->urlGenerator, $this->userManager, $recipients));
113+
} catch (Exception $exception) {
114+
$this->dbConnection->rollBack();
115+
throw $exception;
116+
}
117+
} catch (ShareNotFoundException $shareNotFoundException) {
118+
return new DataResponse($shareNotFoundException->getHint(), Http::STATUS_NOT_FOUND);
110119
}
111120
}
112121

@@ -519,10 +528,9 @@ public function deleteShare(string $id): DataResponse {
519528
* @param string $id ID of the share
520529
* @param ?string $secret Secret of the share
521530
* @param array<class-string<IShareRecipientType|ISharePropertyTypeFilter>, mixed> $arguments Arguments for accessing the share
522-
* @return DataResponse<Http::STATUS_OK, SharingShare, array{}>|DataResponse<Http::STATUS_BAD_REQUEST|Http::STATUS_NOT_FOUND, string, array{}>
531+
* @return DataResponse<Http::STATUS_OK, SharingShare, array{}>|DataResponse<Http::STATUS_NOT_FOUND, string, array{}>
523532
*
524533
* 200: Share returned
525-
* 400: Invalid arguments
526534
* 404: Share not found
527535
*/
528536
#[PublicPage]
@@ -540,8 +548,6 @@ public function getShare(string $id, ?string $secret = null, array $arguments =
540548
$this->dbConnection->rollBack();
541549
throw $exception;
542550
}
543-
} catch (ShareInvalidException $shareInvalidException) {
544-
return new DataResponse($shareInvalidException->getHint(), Http::STATUS_BAD_REQUEST);
545551
} catch (ShareNotFoundException $shareNotFoundException) {
546552
return new DataResponse($shareNotFoundException->getHint(), Http::STATUS_NOT_FOUND);
547553
}

‎apps/sharing/openapi.json‎

Lines changed: 42 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -618,9 +618,9 @@
618618
],
619619
"parameters": [
620620
{
621-
"name": "recipientTypeClasses[]",
621+
"name": "filterRecipientTypeClasses[]",
622622
"in": "query",
623-
"description": "Type class of recipients to filter by",
623+
"description": "Type classes of recipients to filter by",
624624
"schema": {
625625
"type": "array",
626626
"nullable": true,
@@ -662,6 +662,16 @@
662662
"minimum": 0
663663
}
664664
},
665+
{
666+
"name": "id",
667+
"in": "query",
668+
"description": "If provided, recipients that are already part of the share will not be returned.",
669+
"schema": {
670+
"type": "string",
671+
"nullable": true,
672+
"default": null
673+
}
674+
},
665675
{
666676
"name": "OCS-APIRequest",
667677
"in": "header",
@@ -737,6 +747,36 @@
737747
}
738748
}
739749
},
750+
"404": {
751+
"description": "Share used for filtering existing recipients does not exist",
752+
"content": {
753+
"application/json": {
754+
"schema": {
755+
"type": "object",
756+
"required": [
757+
"ocs"
758+
],
759+
"properties": {
760+
"ocs": {
761+
"type": "object",
762+
"required": [
763+
"meta",
764+
"data"
765+
],
766+
"properties": {
767+
"meta": {
768+
"$ref": "#/components/schemas/OCSMeta"
769+
},
770+
"data": {
771+
"type": "string"
772+
}
773+
}
774+
}
775+
}
776+
}
777+
}
778+
}
779+
},
740780
"401": {
741781
"description": "Current user is not logged in",
742782
"content": {
@@ -3005,36 +3045,6 @@
30053045
}
30063046
}
30073047
},
3008-
"400": {
3009-
"description": "Invalid arguments",
3010-
"content": {
3011-
"application/json": {
3012-
"schema": {
3013-
"type": "object",
3014-
"required": [
3015-
"ocs"
3016-
],
3017-
"properties": {
3018-
"ocs": {
3019-
"type": "object",
3020-
"required": [
3021-
"meta",
3022-
"data"
3023-
],
3024-
"properties": {
3025-
"meta": {
3026-
"$ref": "#/components/schemas/OCSMeta"
3027-
},
3028-
"data": {
3029-
"type": "string"
3030-
}
3031-
}
3032-
}
3033-
}
3034-
}
3035-
}
3036-
}
3037-
},
30383048
"404": {
30393049
"description": "Share not found",
30403050
"content": {

‎apps/sharing/tests/Command/CommandTest.php‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -145,10 +145,18 @@ private function runCommand(ShareAccessContext $accessContext, string $class, ar
145145
}
146146

147147
#[Override]
148-
protected function searchRecipients(ShareAccessContext $accessContext, ?array $recipientTypeClasses, string $query, int $limit, int $offset): array {
148+
protected function searchRecipients(ShareAccessContext $accessContext, ?array $filterRecipientTypeClasses, string $query, int $limit, int $offset, ?string $id = null): array {
149149
// We don't have a command for this, so we just call the real manager to make the test pass.
150-
/** @psalm-suppress ArgumentTypeCoercion */
151-
return ShareRecipient::formatMultiple(Server::get(ISharingRegistry::class), Server::get(IFactory::class), Server::get(IURLGenerator::class), Server::get(IUserManager::class), $this->manager->searchRecipients($accessContext, $recipientTypeClasses, $query, $limit, $offset));
150+
try {
151+
$this->dbConnection->beginTransaction();
152+
/** @psalm-suppress ArgumentTypeCoercion */
153+
$shares = ShareRecipient::formatMultiple($this->registry, Server::get(IFactory::class), Server::get(IURLGenerator::class), Server::get(IUserManager::class), $this->manager->searchRecipients($accessContext, $filterRecipientTypeClasses, $query, $limit, $offset, $id));
154+
$this->dbConnection->commit();
155+
return $shares;
156+
} catch (Exception $exception) {
157+
$this->dbConnection->rollBack();
158+
throw $exception;
159+
}
152160
}
153161

154162
/**

‎apps/sharing/tests/Controller/ApiV1ControllerTest.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -84,9 +84,9 @@ private function executeRequest(ShareAccessContext $accessContext, Closure $clos
8484
}
8585

8686
#[Override]
87-
protected function searchRecipients(ShareAccessContext $accessContext, ?array $recipientTypeClasses, string $query, int $limit, int $offset): array {
87+
protected function searchRecipients(ShareAccessContext $accessContext, ?array $filterRecipientTypeClasses, string $query, int $limit, int $offset, ?string $id = null): array {
8888
/** @psalm-suppress ArgumentTypeCoercion */
89-
return $this->executeRequest($accessContext, fn (ApiV1Controller $controller): DataResponse => $controller->searchRecipients($recipientTypeClasses, $query, $limit, $offset));
89+
return $this->executeRequest($accessContext, fn (ApiV1Controller $controller): DataResponse => $controller->searchRecipients($filterRecipientTypeClasses, $query, $limit, $offset, $id));
9090
}
9191

9292
#[Override]

‎lib/private/Sharing/SharingManager.php‎

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -87,12 +87,12 @@ public function __construct(
8787
}
8888

8989
#[\Override]
90-
public function searchRecipients(ShareAccessContext $accessContext, ?array $recipientTypeClasses, string $query, int $limit, int $offset): array {
90+
public function searchRecipients(ShareAccessContext $accessContext, ?array $filterRecipientTypeClasses, string $query, int $limit, int $offset, ?string $id = null): array {
9191
$recipientTypes = $this->registry->getRecipientTypes();
9292

93-
if ($recipientTypeClasses !== null) {
93+
if ($filterRecipientTypeClasses !== null) {
9494
$filteredRecipientTypes = [];
95-
foreach (array_unique($recipientTypeClasses) as $recipientTypeClass) {
95+
foreach (array_unique($filterRecipientTypeClasses) as $recipientTypeClass) {
9696
if (($recipientType = $recipientTypes[$recipientTypeClass] ?? null) === null) {
9797
throw new RuntimeException('The recipient type is not registered: ' . $recipientTypeClass);
9898
}
@@ -112,10 +112,25 @@ public function searchRecipients(ShareAccessContext $accessContext, ?array $reci
112112
));
113113
}
114114

115-
return array_merge(...array_map(
115+
$results = array_merge(...array_map(
116116
static fn (IShareRecipientTypeSearch $recipientType): array => $recipientType->searchRecipients($accessContext, $query, $limit, $offset),
117117
$recipientTypes,
118118
));
119+
120+
if ($id !== null) {
121+
// Do not create a new access context with overridden checks, because it could leak the existence of shares and share recipients.
122+
$share = $this->getShare($accessContext, $id);
123+
$recipients = [];
124+
foreach ($share->recipients as $recipient) {
125+
$recipients[$recipient->class] ??= [];
126+
$recipients[$recipient->class][$recipient->instance ?? ''] ??= [];
127+
$recipients[$recipient->class][$recipient->instance ?? ''][$recipient->value] = true;
128+
}
129+
130+
$results = array_values(array_filter($results, static fn (ShareRecipient $recipient): bool => !isset($recipients[$recipient->class][$recipient->instance ?? ''][$recipient->value])));
131+
}
132+
133+
return $results;
119134
}
120135

121136
#[\Override]

‎lib/public/Sharing/ISharingManager.php‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,13 +30,15 @@ interface ISharingManager {
3030
/**
3131
* Search for recipients that can be added to a share.
3232
*
33-
* @param ?list<class-string<IShareRecipientType>> $recipientTypeClasses
33+
* @param ?list<class-string<IShareRecipientType>> $filterRecipientTypeClasses
3434
* @param positive-int $limit
3535
* @param non-negative-int $offset
36+
* @param ?string $id If provided, recipients that are already part of the share will not be returned.
3637
* @return list<ShareRecipient>
38+
* @throws ShareNotFoundException
3739
* @since 35.0.0
3840
*/
39-
public function searchRecipients(ShareAccessContext $accessContext, ?array $recipientTypeClasses, string $query, int $limit, int $offset): array;
41+
public function searchRecipients(ShareAccessContext $accessContext, ?array $filterRecipientTypeClasses, string $query, int $limit, int $offset, ?string $id = null): array;
4042

4143
/**
4244
* Generate a new secret.

‎openapi.json‎

Lines changed: 42 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -36514,9 +36514,9 @@
3651436514
],
3651536515
"parameters": [
3651636516
{
36517-
"name": "recipientTypeClasses[]",
36517+
"name": "filterRecipientTypeClasses[]",
3651836518
"in": "query",
36519-
"description": "Type class of recipients to filter by",
36519+
"description": "Type classes of recipients to filter by",
3652036520
"schema": {
3652136521
"type": "array",
3652236522
"nullable": true,
@@ -36558,6 +36558,16 @@
3655836558
"minimum": 0
3655936559
}
3656036560
},
36561+
{
36562+
"name": "id",
36563+
"in": "query",
36564+
"description": "If provided, recipients that are already part of the share will not be returned.",
36565+
"schema": {
36566+
"type": "string",
36567+
"nullable": true,
36568+
"default": null
36569+
}
36570+
},
3656136571
{
3656236572
"name": "OCS-APIRequest",
3656336573
"in": "header",
@@ -36633,6 +36643,36 @@
3663336643
}
3663436644
}
3663536645
},
36646+
"404": {
36647+
"description": "Share used for filtering existing recipients does not exist",
36648+
"content": {
36649+
"application/json": {
36650+
"schema": {
36651+
"type": "object",
36652+
"required": [
36653+
"ocs"
36654+
],
36655+
"properties": {
36656+
"ocs": {
36657+
"type": "object",
36658+
"required": [
36659+
"meta",
36660+
"data"
36661+
],
36662+
"properties": {
36663+
"meta": {
36664+
"$ref": "#/components/schemas/OCSMeta"
36665+
},
36666+
"data": {
36667+
"type": "string"
36668+
}
36669+
}
36670+
}
36671+
}
36672+
}
36673+
}
36674+
}
36675+
},
3663636676
"401": {
3663736677
"description": "Current user is not logged in",
3663836678
"content": {
@@ -38901,36 +38941,6 @@
3890138941
}
3890238942
}
3890338943
},
38904-
"400": {
38905-
"description": "Invalid arguments",
38906-
"content": {
38907-
"application/json": {
38908-
"schema": {
38909-
"type": "object",
38910-
"required": [
38911-
"ocs"
38912-
],
38913-
"properties": {
38914-
"ocs": {
38915-
"type": "object",
38916-
"required": [
38917-
"meta",
38918-
"data"
38919-
],
38920-
"properties": {
38921-
"meta": {
38922-
"$ref": "#/components/schemas/OCSMeta"
38923-
},
38924-
"data": {
38925-
"type": "string"
38926-
}
38927-
}
38928-
}
38929-
}
38930-
}
38931-
}
38932-
}
38933-
},
3893438944
"404": {
3893538945
"description": "Share not found",
3893638946
"content": {

0 commit comments

Comments
 (0)