fix(files_sharing): fix share creation error handling

Signed-off-by: skjnldsv <skjnldsv@protonmail.com>
This commit is contained in:
skjnldsv 2025-04-09 16:12:18 +02:00 committed by backportbot[bot]
parent bd5002958f
commit f81ef274dd
5 changed files with 26 additions and 6 deletions

View file

@ -52,6 +52,7 @@ use OCP\Lock\ILockingProvider;
use OCP\Lock\LockedException;
use OCP\Mail\IMailer;
use OCP\Server;
use OCP\Share\Exceptions\GenericShareException;
use OCP\Share\Exceptions\ShareNotFound;
use OCP\Share\Exceptions\ShareTokenException;
use OCP\Share\IManager;
@ -800,6 +801,9 @@ class ShareAPIController extends OCSController {
} catch (HintException $e) {
$code = $e->getCode() === 0 ? 403 : $e->getCode();
throw new OCSException($e->getHint(), $code);
} catch (GenericShareException|\InvalidArgumentException $e) {
$this->logger->error($e->getMessage(), ['exception' => $e]);
throw new OCSForbiddenException($e->getMessage(), $e);
} catch (\Exception $e) {
$this->logger->error($e->getMessage(), ['exception' => $e]);
throw new OCSForbiddenException('Failed to create share.', $e);

View file

@ -6,10 +6,12 @@
// TODO: remove when ie not supported
import 'url-search-params-polyfill'
import { emit } from '@nextcloud/event-bus'
import { showError } from '@nextcloud/dialogs'
import { generateOcsUrl } from '@nextcloud/router'
import axios from '@nextcloud/axios'
import Share from '../models/Share.ts'
import { emit } from '@nextcloud/event-bus'
const shareUrl = generateOcsUrl('apps/files_sharing/api/v1/shares')
@ -45,7 +47,7 @@ export default {
} catch (error) {
console.error('Error while creating share', error)
const errorMessage = error?.response?.data?.ocs?.meta?.message
OC.Notification.showTemporary(
showError(
errorMessage ? t('files_sharing', 'Error creating the share: {errorMessage}', { errorMessage }) : t('files_sharing', 'Error creating the share'),
{ type: 'error' },
)

View file

@ -251,6 +251,7 @@
</NcButton>
<NcButton type="primary"
data-cy-files-sharing-share-editor-action="save"
:disabled="creating"
@click="saveShare">
{{ shareButtonText }}
<template v-if="creating" #icon>
@ -1003,8 +1004,16 @@ export default {
incomingShare.password = this.share.password
}
this.creating = true
const share = await this.addShare(incomingShare)
let share
try {
this.creating = true
share = await this.addShare(incomingShare)
} catch (error) {
this.creating = false
// Error is already handled by ShareRequests mixin
return
}
// ugly hack to make code work - we need the id to be set but at the same time we need to keep values we want to update
this.share._share.id = share.id
await this.queueUpdate(...permissionsAndAttributes)
@ -1018,6 +1027,7 @@ export default {
}
}
}
this.share = share
this.creating = false
this.$emit('add:share', this.share)

View file

@ -597,7 +597,11 @@ class Manager implements IManager {
$mounts = $this->mountManager->findIn($path->getPath());
foreach ($mounts as $mount) {
if ($mount->getStorage()->instanceOfStorage('\OCA\Files_Sharing\ISharedStorage')) {
throw new \InvalidArgumentException($this->l->t('Path contains files shared with you'));
// Using a flat sharing model ensures the file owner can always see who has access.
// Allowing parent folder sharing would require tracking inherited access, which adds complexity
// and hurts performance/scalability.
// So we forbid sharing a parent folder of a share you received.
throw new \InvalidArgumentException($this->l->t('You cannot share a folder that contains other shares'));
}
}
}

View file

@ -2264,7 +2264,7 @@ class ManagerTest extends \Test\TestCase {
public function testPathCreateChecksContainsSharedMount(): void {
$this->expectException(\InvalidArgumentException::class);
$this->expectExceptionMessage('Path contains files shared with you');
$this->expectExceptionMessage('You cannot share a folder that contains other shares');
$path = $this->createMock(Folder::class);
$path->method('getPath')->willReturn('path');