Skip to content

Commit 3514ae7

Browse files
CarlSchwanCarlSchwan
committed
refactor(external-share): Port more code to string as type for the id
Signed-off-by: Carl Schwan <carl.schwan@nextcloud.com>
1 parent a1a3d76 commit 3514ae7

File tree

17 files changed

+294
-640
lines changed

17 files changed

+294
-640
lines changed

‎apps/federatedfilesharing/lib/Controller/RequestHandlerController.php‎

Lines changed: 25 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,7 @@
77
*/
88
namespace OCA\FederatedFileSharing\Controller;
99

10-
use OCA\FederatedFileSharing\AddressHandler;
1110
use OCA\FederatedFileSharing\FederatedShareProvider;
12-
use OCA\FederatedFileSharing\Notifications;
1311
use OCP\App\IAppManager;
1412
use OCP\AppFramework\Http;
1513
use OCP\AppFramework\Http\Attribute\NoCSRFRequired;
@@ -29,7 +27,6 @@
2927
use OCP\HintException;
3028
use OCP\IDBConnection;
3129
use OCP\IRequest;
32-
use OCP\IUserManager;
3330
use OCP\Log\Audit\CriticalActionPerformedEvent;
3431
use OCP\Server;
3532
use OCP\Share;
@@ -44,10 +41,6 @@ public function __construct(
4441
IRequest $request,
4542
private FederatedShareProvider $federatedShareProvider,
4643
private IDBConnection $connection,
47-
private Share\IManager $shareManager,
48-
private Notifications $notifications,
49-
private AddressHandler $addressHandler,
50-
private IUserManager $userManager,
5144
private ICloudIdManager $cloudIdManager,
5245
private LoggerInterface $logger,
5346
private ICloudFederationFactory $cloudFederationFactory,
@@ -66,10 +59,10 @@ public function __construct(
6659
* @param string|null $owner Display name of the receiver
6760
* @param string|null $sharedBy Display name of the sender
6861
* @param string|null $shareWith ID of the user that receives the share
69-
* @param int|null $remoteId ID of the remote
62+
* @param string|null $remoteId ID of the remote
7063
* @param string|null $sharedByFederatedId Federated ID of the sender
7164
* @param string|null $ownerFederatedId Federated ID of the receiver
72-
* @return Http\DataResponse<Http::STATUS_OK, list<empty>, array{}>
65+
* @return DataResponse<Http::STATUS_OK, list<empty>, array{}>
7366
* @throws OCSException
7467
*
7568
* 200: Share created successfully
@@ -83,10 +76,10 @@ public function createShare(
8376
?string $owner = null,
8477
?string $sharedBy = null,
8578
?string $shareWith = null,
86-
?int $remoteId = null,
79+
?string $remoteId = null,
8780
?string $sharedByFederatedId = null,
8881
?string $ownerFederatedId = null,
89-
) {
82+
): DataResponse {
9083
if ($ownerFederatedId === null) {
9184
$ownerFederatedId = $this->cloudIdManager->getCloudId($owner, $this->cleanupRemote($remote))->getId();
9285
}
@@ -132,19 +125,19 @@ public function createShare(
132125
/**
133126
* create re-share on behalf of another user
134127
*
135-
* @param int $id ID of the share
128+
* @param string $id ID of the share
136129
* @param string|null $token Shared secret between servers
137130
* @param string|null $shareWith ID of the user that receives the share
138131
* @param int|null $remoteId ID of the remote
139-
* @return Http\DataResponse<Http::STATUS_OK, array{token: string, remoteId: string}, array{}>
132+
* @return DataResponse<Http::STATUS_OK, array{token: string, remoteId: string}, array{}>
140133
* @throws OCSBadRequestException Re-sharing is not possible
141134
* @throws OCSException
142135
*
143136
* 200: Remote share returned
144137
*/
145138
#[NoCSRFRequired]
146139
#[PublicPage]
147-
public function reShare(int $id, ?string $token = null, ?string $shareWith = null, ?int $remoteId = 0) {
140+
public function reShare(string $id, ?string $token = null, ?string $shareWith = null, ?int $remoteId = 0): DataResponse {
148141
if ($token === null
149142
|| $shareWith === null
150143
|| $remoteId === null
@@ -181,9 +174,9 @@ public function reShare(int $id, ?string $token = null, ?string $shareWith = nul
181174
/**
182175
* accept server-to-server share
183176
*
184-
* @param int $id ID of the remote share
177+
* @param string $id ID of the remote share
185178
* @param string|null $token Shared secret between servers
186-
* @return Http\DataResponse<Http::STATUS_OK, list<empty>, array{}>
179+
* @return DataResponse<Http::STATUS_OK, list<empty>, array{}>
187180
* @throws OCSException
188181
* @throws ShareNotFound
189182
* @throws HintException
@@ -192,7 +185,7 @@ public function reShare(int $id, ?string $token = null, ?string $shareWith = nul
192185
*/
193186
#[NoCSRFRequired]
194187
#[PublicPage]
195-
public function acceptShare(int $id, ?string $token = null) {
188+
public function acceptShare(string $id, ?string $token = null): DataResponse {
196189
$notification = [
197190
'sharedSecret' => $token,
198191
'message' => 'Recipient accept the share'
@@ -216,16 +209,16 @@ public function acceptShare(int $id, ?string $token = null) {
216209
/**
217210
* decline server-to-server share
218211
*
219-
* @param int $id ID of the remote share
212+
* @param string $id ID of the remote share
220213
* @param string|null $token Shared secret between servers
221-
* @return Http\DataResponse<Http::STATUS_OK, list<empty>, array{}>
214+
* @return DataResponse<Http::STATUS_OK, list<empty>, array{}>
222215
* @throws OCSException
223216
*
224217
* 200: Share declined successfully
225218
*/
226219
#[NoCSRFRequired]
227220
#[PublicPage]
228-
public function declineShare(int $id, ?string $token = null) {
221+
public function declineShare(string $id, ?string $token = null) {
229222
$notification = [
230223
'sharedSecret' => $token,
231224
'message' => 'Recipient declined the share'
@@ -249,16 +242,16 @@ public function declineShare(int $id, ?string $token = null) {
249242
/**
250243
* remove server-to-server share if it was unshared by the owner
251244
*
252-
* @param int $id ID of the share
245+
* @param string $id ID of the share
253246
* @param string|null $token Shared secret between servers
254-
* @return Http\DataResponse<Http::STATUS_OK, list<empty>, array{}>
247+
* @return DataResponse<Http::STATUS_OK, list<empty>, array{}>
255248
* @throws OCSException
256249
*
257250
* 200: Share unshared successfully
258251
*/
259252
#[NoCSRFRequired]
260253
#[PublicPage]
261-
public function unshare(int $id, ?string $token = null) {
254+
public function unshare(string $id, ?string $token = null) {
262255
if (!$this->isS2SEnabled()) {
263256
throw new OCSException('Server does not support federated cloud sharing', 503);
264257
}
@@ -275,7 +268,7 @@ public function unshare(int $id, ?string $token = null) {
275268
return new DataResponse();
276269
}
277270

278-
private function cleanupRemote($remote) {
271+
private function cleanupRemote(string $remote): string {
279272
$remote = substr($remote, strpos($remote, '://') + 3);
280273

281274
return rtrim($remote, '/');
@@ -285,16 +278,16 @@ private function cleanupRemote($remote) {
285278
/**
286279
* federated share was revoked, either by the owner or the re-sharer
287280
*
288-
* @param int $id ID of the share
281+
* @param string $id ID of the share
289282
* @param string|null $token Shared secret between servers
290-
* @return Http\DataResponse<Http::STATUS_OK, list<empty>, array{}>
283+
* @return DataResponse<Http::STATUS_OK, list<empty>, array{}>
291284
* @throws OCSBadRequestException Revoking the share is not possible
292285
*
293286
* 200: Share revoked successfully
294287
*/
295288
#[NoCSRFRequired]
296289
#[PublicPage]
297-
public function revoke(int $id, ?string $token = null) {
290+
public function revoke(string $id, ?string $token = null) {
298291
try {
299292
$provider = $this->cloudFederationProviderManager->getCloudFederationProvider('file');
300293
$notification = ['sharedSecret' => $token];
@@ -324,19 +317,19 @@ private function isS2SEnabled($incoming = false) {
324317
}
325318

326319
/**
327-
* update share information to keep federated re-shares in sync
320+
* Update share information to keep federated re-shares in sync.
328321
*
329-
* @param int $id ID of the share
322+
* @param string $id ID of the share
330323
* @param string|null $token Shared secret between servers
331324
* @param int|null $permissions New permissions
332-
* @return Http\DataResponse<Http::STATUS_OK, list<empty>, array{}>
325+
* @return DataResponse<Http::STATUS_OK, list<empty>, array{}>
333326
* @throws OCSBadRequestException Updating permissions is not possible
334327
*
335328
* 200: Permissions updated successfully
336329
*/
337330
#[NoCSRFRequired]
338331
#[PublicPage]
339-
public function updatePermissions(int $id, ?string $token = null, ?int $permissions = null) {
332+
public function updatePermissions(string $id, ?string $token = null, ?int $permissions = null) {
340333
$ncPermissions = $permissions;
341334

342335
try {
@@ -385,7 +378,7 @@ protected function ncPermissions2ocmPermissions($ncPermissions) {
385378
* @param string|null $token Shared secret between servers
386379
* @param string|null $remote Address of the remote
387380
* @param string|null $remote_id ID of the remote
388-
* @return Http\DataResponse<Http::STATUS_OK, array{remote: string, owner: string}, array{}>
381+
* @return DataResponse<Http::STATUS_OK, array{remote: string, owner: string}, array{}>
389382
* @throws OCSBadRequestException Moving share is not possible
390383
*
391384
* 200: Share moved successfully

‎apps/federatedfilesharing/lib/FederatedShareProvider.php‎

Lines changed: 18 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
use OCP\Share\IShare;
2828
use OCP\Share\IShareProvider;
2929
use OCP\Share\IShareProviderSupportsAllSharesInFolder;
30+
use Override;
3031
use Psr\Log\LoggerInterface;
3132

3233
/**
@@ -140,7 +141,7 @@ public function create(IShare $share) {
140141
if ($remoteShare) {
141142
try {
142143
$ownerCloudId = $this->cloudIdManager->getCloudId($remoteShare['owner'], $remoteShare['remote']);
143-
$shareId = $this->addShareToDB($itemSource, $itemType, $shareWith, $sharedBy, $ownerCloudId->getId(), $permissions, 'tmp_token_' . time(), $shareType, $expirationDate);
144+
$shareId = (string)$this->addShareToDB($itemSource, $itemType, $shareWith, $sharedBy, $ownerCloudId->getId(), $permissions, 'tmp_token_' . time(), $shareType, $expirationDate);
144145
$share->setId($shareId);
145146
[$token, $remoteId] = $this->askOwnerToReShare($shareWith, $share, $shareId);
146147
// remote share was create successfully if we get a valid token as return
@@ -168,16 +169,14 @@ public function create(IShare $share) {
168169
}
169170

170171
/**
171-
* create federated share and inform the recipient
172+
* Create federated share and inform the recipient.
172173
*
173-
* @param IShare $share
174-
* @return int
175174
* @throws ShareNotFound
176175
* @throws \Exception
177176
*/
178-
protected function createFederatedShare(IShare $share) {
177+
protected function createFederatedShare(IShare $share): string {
179178
$token = $this->tokenHandler->generateToken();
180-
$shareId = $this->addShareToDB(
179+
$shareId = (string)$this->addShareToDB(
181180
$share->getNodeId(),
182181
$share->getNodeType(),
183182
$share->getSharedWith(),
@@ -321,12 +320,9 @@ private function addShareToDB($itemSource, $itemType, $shareWith, $sharedBy, $ui
321320
}
322321

323322
/**
324-
* Update a share
325-
*
326-
* @param IShare $share
327-
* @return IShare The share object
323+
* Update a share.
328324
*/
329-
public function update(IShare $share) {
325+
public function update(IShare $share): IShare {
330326
/*
331327
* We allow updating the permissions of federated shares
332328
*/
@@ -348,13 +344,12 @@ public function update(IShare $share) {
348344
}
349345

350346
/**
351-
* send the updated permission to the owner/initiator, if they are not the same
347+
* Send the updated permission to the owner/initiator, if they are not the same.
352348
*
353-
* @param IShare $share
354349
* @throws ShareNotFound
355350
* @throws HintException
356351
*/
357-
protected function sendPermissionUpdate(IShare $share) {
352+
protected function sendPermissionUpdate(IShare $share): void {
358353
$remoteId = $this->getRemoteId($share);
359354
// if the local user is the owner we send the permission change to the initiator
360355
if ($this->userManager->userExists($share->getShareOwner())) {
@@ -367,12 +362,9 @@ protected function sendPermissionUpdate(IShare $share) {
367362

368363

369364
/**
370-
* update successful reShare with the correct token
371-
*
372-
* @param int $shareId
373-
* @param string $token
365+
* Update successful reShare with the correct token.
374366
*/
375-
protected function updateSuccessfulReShare($shareId, $token) {
367+
protected function updateSuccessfulReShare(string $shareId, string $token): void {
376368
$query = $this->dbConnection->getQueryBuilder();
377369
$query->update('share')
378370
->where($query->expr()->eq('id', $query->createNamedParameter($shareId)))
@@ -381,12 +373,9 @@ protected function updateSuccessfulReShare($shareId, $token) {
381373
}
382374

383375
/**
384-
* store remote ID in federated reShare table
385-
*
386-
* @param $shareId
387-
* @param $remoteId
376+
* Store remote ID in federated reShare table.
388377
*/
389-
public function storeRemoteId(int $shareId, string $remoteId): void {
378+
public function storeRemoteId(string $shareId, string $remoteId): void {
390379
$query = $this->dbConnection->getQueryBuilder();
391380
$query->insert('federated_reshares')
392381
->values(
@@ -399,10 +388,8 @@ public function storeRemoteId(int $shareId, string $remoteId): void {
399388
}
400389

401390
/**
402-
* get share ID on remote server for federated re-shares
391+
* Get share ID on remote server for federated re-shares.
403392
*
404-
* @param IShare $share
405-
* @return string
406393
* @throws ShareNotFound
407394
*/
408395
public function getRemoteId(IShare $share): string {
@@ -512,11 +499,9 @@ public function removeShareFromTable(IShare $share) {
512499
}
513500

514501
/**
515-
* remove share from table
516-
*
517-
* @param string $shareId
502+
* Remove share from table.
518503
*/
519-
private function removeShareFromTableById($shareId) {
504+
private function removeShareFromTableById(string $shareId): void {
520505
$qb = $this->dbConnection->getQueryBuilder();
521506
$qb->delete('share')
522507
->where($qb->expr()->eq('id', $qb->createNamedParameter($shareId)))
@@ -748,13 +733,8 @@ public function getSharedWith($userId, $shareType, $node, $limit, $offset) {
748733
return $shares;
749734
}
750735

751-
/**
752-
* Get a share by token
753-
*
754-
* @param string $token
755-
* @throws ShareNotFound
756-
*/
757-
public function getShareByToken($token): IShare {
736+
#[Override]
737+
public function getShareByToken(string $token): IShare {
758738
$qb = $this->dbConnection->getQueryBuilder();
759739

760740
$cursor = $qb->select('*')

‎apps/federatedfilesharing/lib/OCM/CloudFederationProviderFiles.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -618,7 +618,7 @@ protected function reshareRequested($id, array $notification) {
618618
$share->setSharedBy($share->getSharedWith());
619619
$share->setSharedWith($shareWith);
620620
$result = $this->federatedShareProvider->create($share);
621-
$this->federatedShareProvider->storeRemoteId((int)$result->getId(), $senderId);
621+
$this->federatedShareProvider->storeRemoteId($result->getId(), $senderId);
622622
return ['token' => $result->getToken(), 'providerId' => $result->getId()];
623623
} else {
624624
throw new ProviderCouldNotAddShareException('resharing not allowed for share: ' . $id);

0 commit comments

Comments
 (0)