Skip to content

Commit e70df82

Browse files
max-nextcloudmax-nextcloud
committed
fix(attachments): use User and Share from controller
For direct editing pass the user to the attachment service. For the sake of consistency pass IShare|IUser from the controllers everywhere. This way we can also build on the permission checks and getters in the controllers. Signed-off-by: Max <max@nextcloud.com>
1 parent 6f03bc6 commit e70df82

12 files changed

Lines changed: 145 additions & 268 deletions

‎lib/Context/ContextManager.php‎

Lines changed: 5 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,8 @@
1010
use OCA\Text\Event\RegisterContextEvent;
1111
use OCP\EventDispatcher\IEventDispatcher;
1212
use OCP\Files\NotFoundException;
13-
use OCP\Files\NotPermittedException;
14-
use OCP\IUserSession;
15-
use OCP\Share\Exceptions\ShareNotFound;
16-
use OCP\Share\IManager as ShareManager;
13+
use OCP\IUser;
14+
use OCP\Share\IShare;
1715
use Psr\Container\ContainerInterface;
1816
use Psr\Log\LoggerInterface;
1917

@@ -24,8 +22,6 @@ public function __construct(
2422
private readonly ContainerInterface $c,
2523
private readonly IEventDispatcher $eventDispatcher,
2624
private readonly LoggerInterface $logger,
27-
private readonly IUserSession $userSession,
28-
private readonly ShareManager $shareManager,
2925
) {
3026
}
3127

@@ -50,7 +46,7 @@ public function registerContext(string $type, string $factoryClassName): void {
5046
$this->contexts[$type] = $factoryClassName;
5147
}
5248

53-
public function getContext(string $type, int $id, ?string $shareToken): IContext {
49+
public function getContext(string $type, int $id, IShare|IUser $auth): IContext {
5450
$factoryClassName = $this->getContexts()[$type];
5551
if ($factoryClassName === null) {
5652
throw new NotFoundException('Context of type "' . $type . '" was not registered!');
@@ -59,19 +55,7 @@ public function getContext(string $type, int $id, ?string $shareToken): IContext
5955
if (!$factory instanceof IContextFactory) {
6056
throw new NotFoundException('Context factory of type "' . $type . '" is not an IContextFactory.');
6157
}
62-
if ($shareToken === null) {
63-
$user = $this->userSession->getUser();
64-
if ($user === null) {
65-
throw new NotPermittedException();
66-
}
67-
return $factory->build($user, $type, $id);
68-
} else {
69-
try {
70-
$share = $this->shareManager->getShareByToken($shareToken);
71-
} catch (ShareNotFound) {
72-
throw new NotFoundException();
73-
}
74-
return $factory->build($share, $type, $id);
75-
}
58+
return $factory->build($auth, $type, $id);
7659
}
60+
7761
}

‎lib/Controller/AttachmentController.php‎

Lines changed: 52 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,16 @@
2222
use OCP\AppFramework\Http\DataDownloadResponse;
2323
use OCP\AppFramework\Http\DataResponse;
2424
use OCP\AppFramework\Http\RedirectResponse;
25+
use OCP\Constants;
2526
use OCP\Files\IMimeTypeDetector;
2627
use OCP\Files\InvalidPathException;
28+
use OCP\Files\NotPermittedException;
2729
use OCP\IL10N;
2830
use OCP\IRequest;
31+
use OCP\IUser;
32+
use OCP\Share\Exceptions\ShareNotFound;
33+
use OCP\Share\IManager as ShareManager;
34+
use OCP\Share\IShare;
2935
use OCP\Util;
3036
use Psr\Log\LoggerInterface;
3137

@@ -63,6 +69,7 @@ public function __construct(
6369
private LoggerInterface $logger,
6470
private IMimeTypeDetector $mimeTypeDetector,
6571
private AttachmentService $attachmentService,
72+
private ShareManager $shareManager,
6673
) {
6774
parent::__construct($appName, $request);
6875
}
@@ -77,25 +84,19 @@ public function getAttachmentList(string $shareToken = ''): DataResponse {
7784
} catch (InvalidSessionException) {
7885
$session = null;
7986
}
80-
81-
if ($shareToken) {
82-
$attachments = $this->attachmentService->getAttachmentList($documentId, null, $session, $shareToken);
83-
} else {
84-
$userId = $this->getUserId();
85-
$attachments = $this->attachmentService->getAttachmentList($documentId, $userId, $session);
86-
}
87-
87+
$auth = $this->getAuth($shareToken, false);
88+
$attachments = $this->attachmentService->getAttachmentList($documentId, $auth, $session);
8889
return new DataResponse($attachments);
8990
}
9091

9192
#[NoAdminRequired]
9293
#[PublicPage]
9394
#[RequireDocumentSession]
9495
public function insertAttachmentFile(string $filePath): DataResponse {
95-
$userId = $this->getSession()->getUserId();
96+
$user = $this->getUser();
9697

9798
try {
98-
$insertResult = $this->attachmentService->insertAttachmentFile($this->getSession()->getDocumentId(), $filePath, $userId);
99+
$insertResult = $this->attachmentService->insertAttachmentFile($this->getSession()->getDocumentId(), $filePath, $user);
99100
if (isset($insertResult['error'])) {
100101
return new DataResponse($insertResult, Http::STATUS_BAD_REQUEST);
101102
} else {
@@ -121,12 +122,8 @@ public function uploadAttachment(string $token = ''): DataResponse {
121122
throw new Exception('Could not read file');
122123
}
123124
$newFileName = $file['name'];
124-
if ($token) {
125-
$uploadResult = $this->attachmentService->uploadAttachmentPublic($documentId, $newFileName, $newFileResource, $token);
126-
} else {
127-
$userId = $this->getSession()->getUserId();
128-
$uploadResult = $this->attachmentService->uploadAttachment($documentId, $newFileName, $newFileResource, $userId);
129-
}
125+
$auth = $this->getAuth($token);
126+
$uploadResult = $this->attachmentService->uploadAttachment($documentId, $newFileName, $newFileResource, $auth);
130127
if (isset($uploadResult['error'])) {
131128
return new DataResponse($uploadResult, Http::STATUS_BAD_REQUEST);
132129
} else {
@@ -147,12 +144,12 @@ public function uploadAttachment(string $token = ''): DataResponse {
147144
#[NoAdminRequired]
148145
#[PublicPage]
149146
#[RequireDocumentSession]
150-
public function createAttachment(string $token = ''): DataResponse {
147+
public function createAttachment(): DataResponse {
151148
$documentId = $this->getSession()->getDocumentId();
152149
try {
153-
$userId = $this->getSession()->getUserId();
150+
$user = $this->getUser();
154151
$newFileName = $this->request->getParam('fileName', 'text.md');
155-
$createResult = $this->attachmentService->createAttachmentFile($documentId, $newFileName, $userId);
152+
$createResult = $this->attachmentService->createAttachmentFile($documentId, $newFileName, $user);
156153
if (isset($createResult['error'])) {
157154
return new DataResponse($createResult, Http::STATUS_BAD_REQUEST);
158155
} else {
@@ -208,13 +205,8 @@ public function getImageFile(string $imageFileName, string $shareToken = '',
208205
$documentId = $this->getDocumentId();
209206

210207
try {
211-
if ($shareToken) {
212-
$imageFile = $this->attachmentService->getImageFilePublic($documentId, $imageFileName, $shareToken, $preferRawImage === 1);
213-
} else {
214-
$userId = $this->getUserId();
215-
$imageFile = $this->attachmentService->getImageFile($documentId, $imageFileName, $userId, $preferRawImage === 1);
216-
}
217-
208+
$auth = $this->getAuth($shareToken, false);
209+
$imageFile = $this->attachmentService->getImageFile($documentId, $imageFileName, $auth, $preferRawImage === 1);
218210
if ($imageFile !== null) {
219211
$response = new DataDownloadResponse(
220212
$imageFile->getContent(),
@@ -247,12 +239,8 @@ public function getMediaFile(string $mediaFileName, string $shareToken = ''): Da
247239
$documentId = $this->getDocumentId();
248240

249241
try {
250-
if ($shareToken) {
251-
$mediaFile = $this->attachmentService->getMediaFilePublic($documentId, $mediaFileName, $shareToken);
252-
} else {
253-
$userId = $this->getUserId();
254-
$mediaFile = $this->attachmentService->getMediaFile($documentId, $mediaFileName, $userId);
255-
}
242+
$auth = $this->getAuth($shareToken, false);
243+
$mediaFile = $this->attachmentService->getMediaFile($documentId, $mediaFileName, $auth);
256244
return $mediaFile !== null
257245
? new DataDownloadResponse(
258246
$mediaFile->getContent(),
@@ -278,12 +266,8 @@ public function getMediaFilePreview(string $mediaFileName, string $shareToken =
278266
$documentId = $this->getDocumentId();
279267

280268
try {
281-
if ($shareToken) {
282-
$preview = $this->attachmentService->getMediaFilePreviewPublic($documentId, $mediaFileName, $shareToken);
283-
} else {
284-
$userId = $this->getUserId();
285-
$preview = $this->attachmentService->getMediaFilePreview($documentId, $mediaFileName, $userId);
286-
}
269+
$auth = $this->getAuth($shareToken, false);
270+
$preview = $this->attachmentService->getMediaFilePreview($documentId, $mediaFileName, $auth);
287271
if ($preview === null) {
288272
return new DataResponse('', Http::STATUS_NOT_FOUND);
289273
}
@@ -302,6 +286,36 @@ public function getMediaFilePreview(string $mediaFileName, string $shareToken =
302286
return new DataResponse('', Http::STATUS_NOT_FOUND);
303287
}
304288

289+
private function getAuth(string $shareToken, bool $updatePermissionRequired = true): IShare|IUser {
290+
if ($shareToken !== '') {
291+
try {
292+
$share = $this->shareManager->getShareByToken($shareToken);
293+
if (!$updatePermissionRequired && !$this->hasUpdatePermissions($share)) {
294+
throw new NotPermittedException('No write permissions');
295+
}
296+
return $share;
297+
} catch (ShareNotFound) {
298+
throw new InvalidSessionException();
299+
}
300+
} else {
301+
return $this->getUser();
302+
}
303+
}
304+
305+
/**
306+
* Check if the shared access has write permissions
307+
*/
308+
private function hasUpdatePermissions(IShare $share): bool {
309+
return (
310+
in_array(
311+
$share->getShareType(),
312+
[IShare::TYPE_LINK, IShare::TYPE_EMAIL, IShare::TYPE_ROOM],
313+
true
314+
)
315+
&& $share->getPermissions() & Constants::PERMISSION_UPDATE
316+
&& $share->getNode()->getPermissions() & Constants::PERMISSION_UPDATE);
317+
}
318+
305319
/**
306320
* Allow all supported mimetypes
307321
* Use mimetype detector for the other ones

‎lib/Controller/ISessionAwareController.php‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99

1010
use OCA\Text\Db\Document;
1111
use OCA\Text\Db\Session;
12+
use OCP\IUser;
1213

1314
interface ISessionAwareController {
1415
public function getSession(): Session;
@@ -17,6 +18,6 @@ public function getDocumentId(): int;
1718
public function setDocumentId(int $documentId): void;
1819
public function getDocument(): Document;
1920
public function setDocument(Document $document): void;
20-
public function getUserId(): string;
21-
public function setUserId(string $userId): void;
21+
public function getUser(): IUser;
22+
public function setUser(IUser $user): void;
2223
}

‎lib/Controller/PublicSessionController.php‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -84,32 +84,32 @@ public function create(string $token, ?string $filePath = null, ?string $baseVer
8484

8585
#[NoAdminRequired]
8686
#[PublicPage]
87-
public function close(int $documentId, int $sessionId, string $sessionToken, string $token): DataResponse {
88-
return $this->apiService->close($documentId, $sessionId, $sessionToken, $token);
87+
public function close(int $documentId, int $sessionId, string $sessionToken): DataResponse {
88+
return $this->apiService->close($documentId, $sessionId, $sessionToken, $this->getShare());
8989
}
9090

9191
#[NoAdminRequired]
9292
#[PublicPage]
9393
#[RequireDocumentBaseVersionEtag]
9494
#[RequireDocumentSession]
95-
public function push(int $version, array $steps, string $awareness, string $token, ?int $recoveryAttempt = null): DataResponse {
96-
return $this->apiService->push($this->getSession(), $this->getDocument(), $version, $steps, $awareness, $recoveryAttempt, $token);
95+
public function push(int $version, array $steps, string $awareness, ?int $recoveryAttempt = null): DataResponse {
96+
return $this->apiService->push($this->getSession(), $this->getDocument(), $version, $steps, $awareness, $recoveryAttempt, $this->getShare());
9797
}
9898

9999
#[NoAdminRequired]
100100
#[PublicPage]
101101
#[RequireDocumentBaseVersionEtag]
102102
#[RequireDocumentSession]
103-
public function sync(string $token, int $version = 0): DataResponse {
104-
return $this->apiService->sync($this->getSession(), $this->getDocument(), $version, $token);
103+
public function sync(int $version = 0): DataResponse {
104+
return $this->apiService->sync($this->getDocument(), $this->getShare(), $version);
105105
}
106106

107107
#[NoAdminRequired]
108108
#[PublicPage]
109109
#[RequireDocumentBaseVersionEtag]
110110
#[RequireDocumentSession]
111-
public function save(string $token, int $version, string $autosaveContent, string $documentState, bool $force = false, bool $manualSave = false): DataResponse {
112-
return $this->apiService->save($this->getSession(), $this->getDocument(), $version, $autosaveContent, $documentState, $force, $manualSave, $token);
111+
public function save(int $version, string $autosaveContent, string $documentState, bool $force = false, bool $manualSave = false): DataResponse {
112+
return $this->apiService->save($this->getDocument(), $this->getShare(), $version, $autosaveContent, $documentState, $force, $manualSave);
113113
}
114114

115115
#[NoAdminRequired]
@@ -118,4 +118,5 @@ public function save(string $token, int $version, string $autosaveContent, strin
118118
public function updateSession(string $guestName): DataResponse {
119119
return $this->apiService->updateSession($this->getSession(), $guestName);
120120
}
121+
121122
}

‎lib/Controller/SessionController.php‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,11 @@ public function __construct(
5656
#[NoAdminRequired]
5757
public function create(string $type, int $id, ?string $baseVersionEtag = null): DataResponse {
5858
try {
59-
$context = $this->contextManager->getContext($type, $id, null);
59+
$user = $this->userSession->getUser();
60+
if ($user === null) {
61+
throw new NotFoundException('No user found.');
62+
}
63+
$context = $this->contextManager->getContext($type, $id, $user);
6064
} catch (NotFoundException|NotPermittedException $e) {
6165
$this->logger->error('No context for ' . $type . ' (' . $id . ') ', [ 'exception' => $e ]);
6266
return new DataResponse([
@@ -74,7 +78,7 @@ public function close(int $documentId, int $sessionId, string $sessionToken): Da
7478
if ($userId === null) {
7579
throw new InvalidSessionException();
7680
}
77-
return $this->apiService->close($documentId, $sessionId, $sessionToken, null);
81+
return $this->apiService->close($documentId, $sessionId, $sessionToken, $this->getUser());
7882
}
7983

8084
#[NoAdminRequired]
@@ -84,7 +88,7 @@ public function close(int $documentId, int $sessionId, string $sessionToken): Da
8488
public function push(int $version, array $steps, string $awareness, ?int $recoveryAttempt = null): DataResponse {
8589
try {
8690
$this->loginSessionUser();
87-
return $this->apiService->push($this->getSession(), $this->getDocument(), $version, $steps, $awareness, $recoveryAttempt);
91+
return $this->apiService->push($this->getSession(), $this->getDocument(), $version, $steps, $awareness, $recoveryAttempt, $this->getUser());
8892
} finally {
8993
$this->restoreSessionUser();
9094
}
@@ -97,7 +101,7 @@ public function push(int $version, array $steps, string $awareness, ?int $recove
97101
public function sync(int $version = 0): DataResponse {
98102
try {
99103
$this->loginSessionUser();
100-
return $this->apiService->sync($this->getSession(), $this->getDocument(), $version);
104+
return $this->apiService->sync($this->getDocument(), $this->getUser(), $version);
101105
} finally {
102106
$this->restoreSessionUser();
103107
}
@@ -110,7 +114,7 @@ public function sync(int $version = 0): DataResponse {
110114
public function save(int $version, string $autosaveContent, string $documentState, bool $force = false, bool $manualSave = false): DataResponse {
111115
try {
112116
$this->loginSessionUser();
113-
return $this->apiService->save($this->getSession(), $this->getDocument(), $version, $autosaveContent, $documentState, $force, $manualSave);
117+
return $this->apiService->save($this->getDocument(), $this->getUser(), $version, $autosaveContent, $documentState, $force, $manualSave);
114118
} finally {
115119
$this->restoreSessionUser();
116120
}

‎lib/Controller/TSessionAwareController.php‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,12 +12,13 @@
1212
use OCA\Text\Db\Document;
1313
use OCA\Text\Db\Session;
1414
use OCA\Text\Exception\InvalidSessionException;
15+
use OCP\IUser;
1516

1617
trait TSessionAwareController {
1718
private ?Session $textSession = null;
1819
private ?int $documentId = null;
1920
private ?Document $document = null;
20-
private ?string $userId = null;
21+
private ?IUser $user = null;
2122

2223
public function setSession(?Session $session): void {
2324
$this->textSession = $session;
@@ -31,8 +32,8 @@ public function setDocument(?Document $document): void {
3132
$this->document = $document;
3233
}
3334

34-
public function setUserId(?string $userId): void {
35-
$this->userId = $userId;
35+
public function setUser(IUser $user): void {
36+
$this->user = $user;
3637
}
3738

3839
/**
@@ -71,11 +72,11 @@ public function getDocument(): Document {
7172
/**
7273
* @throws InvalidSessionException
7374
*/
74-
public function getUserId(): string {
75-
if ($this->userId === null) {
75+
public function getUser(): IUser {
76+
if ($this->user === null) {
7677
throw new InvalidSessionException();
7778
}
7879

79-
return $this->userId;
80+
return $this->user;
8081
}
8182
}

0 commit comments

Comments
 (0)