Skip to content

Commit 14b0e5d

Browse files
authored
Merge pull request #53278 from nextcloud/fix/dav-nickname-stable31
2 parents cea3178 + ccea5df commit 14b0e5d

File tree

13 files changed

+183
-15
lines changed

13 files changed

+183
-15
lines changed

apps/dav/lib/Files/Sharing/FilesDropPlugin.php

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
use OC\Files\View;
99
use OCP\Share\IShare;
10+
use Sabre\DAV\Exception\BadRequest;
1011
use Sabre\DAV\Exception\MethodNotAllowed;
1112
use Sabre\DAV\ServerPlugin;
1213
use Sabre\HTTP\RequestInterface;
@@ -64,14 +65,28 @@ public function beforeMethod(RequestInterface $request, ResponseInterface $respo
6465
// Extract the attributes for the file request
6566
$isFileRequest = false;
6667
$attributes = $this->share->getAttributes();
67-
$nickName = $request->hasHeader('X-NC-Nickname') ? urldecode($request->getHeader('X-NC-Nickname')) : null;
68+
$nickName = $request->hasHeader('X-NC-Nickname') ? trim(urldecode($request->getHeader('X-NC-Nickname'))) : null;
6869
if ($attributes !== null) {
6970
$isFileRequest = $attributes->getAttribute('fileRequest', 'enabled') === true;
7071
}
7172

7273
// We need a valid nickname for file requests
73-
if ($isFileRequest && ($nickName == null || trim($nickName) === '')) {
74-
throw new MethodNotAllowed('Nickname is required for file requests');
74+
if ($isFileRequest && !$nickName) {
75+
throw new BadRequest('Nickname is required for file requests');
76+
}
77+
78+
if ($nickName !== null) {
79+
try {
80+
$this->view->verifyPath($path, $nickName);
81+
} catch (\Exception $e) {
82+
// If the path is not valid, we throw an exception
83+
throw new BadRequest('Invalid nickname: ' . $nickName);
84+
}
85+
86+
// Forbid nicknames starting with a dot
87+
if (str_starts_with($nickName, '.')) {
88+
throw new BadRequest('Invalid nickname: ' . $nickName);
89+
}
7590
}
7691

7792
// If this is a file request we need to create a folder for the user
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
/*!
2+
* SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
3+
* SPDX-License-Identifier: AGPL-3.0-or-later
4+
*/
5+
import { InvalidFilenameError, InvalidFilenameErrorReason, validateFilename } from '@nextcloud/files'
6+
import { t } from '@nextcloud/l10n'
7+
8+
/**
9+
* Get the validity of a filename (empty if valid).
10+
* This can be used for `setCustomValidity` on input elements
11+
* @param name The filename
12+
* @param escape Escape the matched string in the error (only set when used in HTML)
13+
*/
14+
export function getGuestNameValidity(name: string, escape = false): string {
15+
if (name.trim() === '') {
16+
return t('files', 'Filename must not be empty.')
17+
}
18+
19+
if (name.startsWith('.')) {
20+
return t('files', 'Names must not start with a dot.')
21+
}
22+
23+
try {
24+
validateFilename(name)
25+
return ''
26+
} catch (error) {
27+
if (!(error instanceof InvalidFilenameError)) {
28+
throw error
29+
}
30+
31+
switch (error.reason) {
32+
case InvalidFilenameErrorReason.Character:
33+
return t('files', '"{char}" is not allowed inside a name.', { char: error.segment }, undefined, { escape })
34+
case InvalidFilenameErrorReason.ReservedName:
35+
return t('files', '"{segment}" is a reserved name and not allowed.', { segment: error.segment }, undefined, { escape: false })
36+
case InvalidFilenameErrorReason.Extension:
37+
if (error.segment.match(/\.[a-z]/i)) {
38+
return t('files', '"{extension}" is not an allowed name.', { extension: error.segment }, undefined, { escape: false })
39+
}
40+
return t('files', 'Names must not end with "{extension}".', { extension: error.segment }, undefined, { escape: false })
41+
default:
42+
return t('files', 'Invalid name.')
43+
}
44+
}
45+
}

apps/files_sharing/src/views/PublicAuthPrompt.vue

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,12 +35,14 @@
3535

3636
<script lang="ts">
3737
import { defineComponent } from 'vue'
38+
import { loadState } from '@nextcloud/initial-state'
3839
import { t } from '@nextcloud/l10n'
3940
40-
import NcDialog from '@nextcloud/vue/dist/Components/NcDialog.js'
41-
import NcNoteCard from '@nextcloud/vue/dist/Components/NcNoteCard.js'
42-
import NcTextField from '@nextcloud/vue/dist/Components/NcTextField.js'
43-
import { loadState } from '@nextcloud/initial-state'
41+
import NcDialog from '@nextcloud/vue/components/NcDialog'
42+
import NcNoteCard from '@nextcloud/vue/components/NcNoteCard'
43+
import NcTextField from '@nextcloud/vue/components/NcTextField'
44+
45+
import { getGuestNameValidity } from '../services/GuestNameValidity'
4446
4547
export default defineComponent({
4648
name: 'PublicAuthPrompt',
@@ -101,6 +103,19 @@ export default defineComponent({
101103
},
102104
immediate: true,
103105
},
106+
107+
name() {
108+
// Check validity of the new name
109+
const newName = this.name.trim?.() || ''
110+
const input = (this.$refs.input as Vue|undefined)?.$el.querySelector('input')
111+
if (!input) {
112+
return
113+
}
114+
115+
const validity = getGuestNameValidity(newName)
116+
input.setCustomValidity(validity)
117+
input.reportValidity()
118+
},
104119
},
105120
})
106121
</script>

build/integration/filesdrop_features/filesdrop.feature

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ Feature: FilesDrop
4747
And Downloading file "/drop/a.txt"
4848
Then Downloaded content should be "abc"
4949

50-
Scenario: Files drop forbis MKCOL
50+
Scenario: Files drop forbid MKCOL
5151
Given user "user0" exists
5252
And As an "user0"
5353
And user "user0" created a folder "/drop"
@@ -90,3 +90,42 @@ Feature: FilesDrop
9090
Then Downloaded content should be "abc"
9191
And Downloading file "/drop/Mallory/a (2).txt"
9292
Then Downloaded content should be "def"
93+
94+
Scenario: Files request drop with invalid nickname with slashes
95+
Given user "user0" exists
96+
And As an "user0"
97+
And user "user0" created a folder "/drop"
98+
And as "user0" creating a share with
99+
| path | drop |
100+
| shareType | 4 |
101+
| permissions | 4 |
102+
| attributes | [{"scope":"fileRequest","key":"enabled","value":true}] |
103+
| shareWith | |
104+
When Dropping file "/folder/a.txt" with "abc" as "Alice/Bob/Mallory"
105+
Then the HTTP status code should be "400"
106+
107+
Scenario: Files request drop with invalid nickname with forbidden characters
108+
Given user "user0" exists
109+
And As an "user0"
110+
And user "user0" created a folder "/drop"
111+
And as "user0" creating a share with
112+
| path | drop |
113+
| shareType | 4 |
114+
| permissions | 4 |
115+
| attributes | [{"scope":"fileRequest","key":"enabled","value":true}] |
116+
| shareWith | |
117+
When Dropping file "/folder/a.txt" with "abc" as ".htaccess"
118+
Then the HTTP status code should be "400"
119+
120+
Scenario: Files request drop with invalid nickname with forbidden characters
121+
Given user "user0" exists
122+
And As an "user0"
123+
And user "user0" created a folder "/drop"
124+
And as "user0" creating a share with
125+
| path | drop |
126+
| shareType | 4 |
127+
| permissions | 4 |
128+
| attributes | [{"scope":"fileRequest","key":"enabled","value":true}] |
129+
| shareWith | |
130+
When Dropping file "/folder/a.txt" with "abc" as ".Mallory"
131+
Then the HTTP status code should be "400"

dist/4926-4926.js

Lines changed: 0 additions & 2 deletions
This file was deleted.

dist/4926-4926.js.map

Lines changed: 0 additions & 1 deletion
This file was deleted.

dist/4926-4926.js.map.license

Lines changed: 0 additions & 1 deletion
This file was deleted.

0 commit comments

Comments
 (0)