Skip to content

Commit 901859e

Browse files
provokateurinprovokateurin
committed
fix(FilesDropPlugin): Ensure all request for file request have a nickname
Signed-off-by: provokateurin <[email protected]>
1 parent ff1772f commit 901859e

File tree

4 files changed

+107
-24
lines changed

4 files changed

+107
-24
lines changed

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

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,23 @@ public function beforeMethod(RequestInterface $request, ResponseInterface $respo
8787
throw new MethodNotAllowed('Only PUT, MKCOL and MOVE are allowed on files drop');
8888
}
8989

90+
// Extract the attributes for the file request
91+
$isFileRequest = false;
92+
$attributes = $this->share->getAttributes();
93+
if ($attributes !== null) {
94+
$isFileRequest = $attributes->getAttribute('fileRequest', 'enabled') === true;
95+
}
96+
97+
// Retrieve the nickname from the request
98+
$nickname = $request->hasHeader('X-NC-Nickname')
99+
? trim(urldecode($request->getHeader('X-NC-Nickname')))
100+
: null;
101+
102+
// We need a valid nickname for file requests
103+
if ($isFileRequest && !$nickname) {
104+
throw new BadRequest('A nickname header is required for file requests');
105+
}
106+
90107
// If this is a folder creation request
91108
// let's stop there and let the onMkcol handle it
92109
if ($request->getMethod() === 'MKCOL') {
@@ -114,23 +131,6 @@ public function beforeMethod(RequestInterface $request, ResponseInterface $respo
114131
// e.g /Folder/image.jpg
115132
$relativePath = substr($path, strlen($rootPath));
116133

117-
// Extract the attributes for the file request
118-
$isFileRequest = false;
119-
$attributes = $this->share->getAttributes();
120-
if ($attributes !== null) {
121-
$isFileRequest = $attributes->getAttribute('fileRequest', 'enabled') === true;
122-
}
123-
124-
// Retrieve the nickname from the request
125-
$nickname = $request->hasHeader('X-NC-Nickname')
126-
? trim(urldecode($request->getHeader('X-NC-Nickname')))
127-
: null;
128-
129-
// We need a valid nickname for file requests
130-
if ($isFileRequest && !$nickname) {
131-
throw new BadRequest('A nickname header is required for file requests');
132-
}
133-
134134
if ($nickname) {
135135
try {
136136
$node->verifyPath($nickname);

‎apps/dav/tests/unit/Files/Sharing/FilesDropPluginTest.php‎

Lines changed: 44 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
use OCP\Share\IAttributes;
1414
use OCP\Share\IShare;
1515
use PHPUnit\Framework\MockObject\MockObject;
16+
use Sabre\DAV\Exception\BadRequest;
1617
use Sabre\DAV\Server;
1718
use Sabre\HTTP\RequestInterface;
1819
use Sabre\HTTP\ResponseInterface;
@@ -23,6 +24,7 @@ class FilesDropPluginTest extends TestCase {
2324
private FilesDropPlugin $plugin;
2425

2526
private Folder&MockObject $node;
27+
private IAttributes&MockObject $attributes;
2628
private IShare&MockObject $share;
2729
private Server&MockObject $server;
2830
private RequestInterface&MockObject $request;
@@ -45,10 +47,10 @@ protected function setUp(): void {
4547
$this->request = $this->createMock(RequestInterface::class);
4648
$this->response = $this->createMock(ResponseInterface::class);
4749

48-
$attributes = $this->createMock(IAttributes::class);
50+
$this->attributes = $this->createMock(IAttributes::class);
4951
$this->share->expects($this->any())
5052
->method('getAttributes')
51-
->willReturn($attributes);
53+
->willReturn($this->attributes);
5254

5355
$this->share
5456
->method('getToken')
@@ -104,7 +106,7 @@ public function testFileAlreadyExistsValid(): void {
104106
$this->plugin->beforeMethod($this->request, $this->response);
105107
}
106108

107-
public function testMKCOL(): void {
109+
public function testFileDropMKCOLWithoutNickname(): void {
108110
$this->plugin->enable();
109111
$this->plugin->setShare($this->share);
110112

@@ -116,6 +118,45 @@ public function testMKCOL(): void {
116118
$this->plugin->beforeMethod($this->request, $this->response);
117119
}
118120

121+
public function testFileRequestNoMKCOLWithoutNickname(): void {
122+
$this->plugin->enable();
123+
$this->plugin->setShare($this->share);
124+
125+
$this->request->method('getMethod')
126+
->willReturn('MKCOL');
127+
128+
$this->attributes
129+
->method('getAttribute')
130+
->with('fileRequest', 'enabled')
131+
->willReturn(true);
132+
133+
$this->expectException(BadRequest::class);
134+
135+
$this->plugin->beforeMethod($this->request, $this->response);
136+
}
137+
138+
public function testFileRequestMKCOLWithNickname(): void {
139+
$this->plugin->enable();
140+
$this->plugin->setShare($this->share);
141+
142+
$this->request->method('getMethod')
143+
->willReturn('MKCOL');
144+
145+
$this->attributes
146+
->method('getAttribute')
147+
->with('fileRequest', 'enabled')
148+
->willReturn(true);
149+
150+
$this->request->method('hasHeader')
151+
->with('X-NC-Nickname')
152+
->willReturn(true);
153+
$this->request->method('getHeader')
154+
->with('X-NC-Nickname')
155+
->willReturn('nickname');
156+
157+
$this->plugin->beforeMethod($this->request, $this->response);
158+
}
159+
119160
public function testSubdirPut(): void {
120161
$this->plugin->enable();
121162
$this->plugin->setShare($this->share);

‎build/integration/features/bootstrap/FilesDropContext.php‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ public function droppingFileWithAs($path, $content, $nickname) {
5757
/**
5858
* @When Creating folder :folder in drop
5959
*/
60-
public function creatingFolderInDrop($folder) {
60+
public function creatingFolderInDrop($folder, $nickname = null) {
6161
$client = new Client();
6262
$options = [];
6363
if (count($this->lastShareData->data->element) > 0) {
@@ -73,10 +73,22 @@ public function creatingFolderInDrop($folder) {
7373
'X-REQUESTED-WITH' => 'XMLHttpRequest',
7474
];
7575

76+
if ($nickname) {
77+
$options['headers']['X-NC-NICKNAME'] = $nickname;
78+
}
79+
7680
try {
7781
$this->response = $client->request('MKCOL', $fullUrl, $options);
7882
} catch (\GuzzleHttp\Exception\ClientException $e) {
7983
$this->response = $e->getResponse();
8084
}
8185
}
86+
87+
88+
/**
89+
* @When Creating folder :folder in drop as :nickName
90+
*/
91+
public function creatingFolderInDropWithNickname($folder, $nickname) {
92+
return $this->creatingFolderInDrop($folder, $nickname);
93+
}
8294
}

‎build/integration/filesdrop_features/filesdrop.feature‎

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,40 +33,70 @@ Feature: FilesDrop
3333
And Downloading file "/drop/a (2).txt"
3434
Then Downloaded content should be "def"
3535

36-
Scenario: Files drop forbid directory without a nickname
36+
Scenario: Files request forbid directory without a nickname
3737
Given user "user0" exists
3838
And As an "user0"
3939
And user "user0" created a folder "/drop"
4040
And as "user0" creating a share with
4141
| path | drop |
4242
| shareType | 3 |
4343
| publicUpload | true |
44+
| attributes | [{"scope":"fileRequest","key":"enabled","value":true}] |
4445
And Updating last share with
4546
| permissions | 4 |
4647
When Dropping file "/folder/a.txt" with "abc"
4748
Then the HTTP status code should be "400"
4849

49-
Scenario: Files drop allows MKCOL
50+
Scenario: Files drop allow MKCOL without a nickname
51+
Given user "user0" exists
52+
And As an "user0"
53+
And user "user0" created a folder "/drop"
54+
And as "user0" creating a share with
55+
| path | drop |
56+
| shareType | 3 |
57+
| publicUpload | true |
58+
And Updating last share with
59+
| permissions | 4 |
60+
When Creating folder "folder" in drop
61+
Then the HTTP status code should be "201"
62+
63+
Scenario: Files request forbid MKCOL without a nickname
5064
Given user "user0" exists
5165
And As an "user0"
5266
And user "user0" created a folder "/drop"
5367
And as "user0" creating a share with
5468
| path | drop |
5569
| shareType | 3 |
5670
| publicUpload | true |
71+
| attributes | [{"scope":"fileRequest","key":"enabled","value":true}] |
5772
And Updating last share with
5873
| permissions | 4 |
5974
When Creating folder "folder" in drop
75+
Then the HTTP status code should be "400"
76+
77+
Scenario: Files request allows MKCOL with a nickname
78+
Given user "user0" exists
79+
And As an "user0"
80+
And user "user0" created a folder "/drop"
81+
And as "user0" creating a share with
82+
| path | drop |
83+
| shareType | 3 |
84+
| publicUpload | true |
85+
| attributes | [{"scope":"fileRequest","key":"enabled","value":true}] |
86+
And Updating last share with
87+
| permissions | 4 |
88+
When Creating folder "folder" in drop as "nickname"
6089
Then the HTTP status code should be "201"
6190

62-
Scenario: Files drop forbid subfolder creation without a nickname
91+
Scenario: Files request forbid subfolder creation without a nickname
6392
Given user "user0" exists
6493
And As an "user0"
6594
And user "user0" created a folder "/drop"
6695
And as "user0" creating a share with
6796
| path | drop |
6897
| shareType | 3 |
6998
| publicUpload | true |
99+
| attributes | [{"scope":"fileRequest","key":"enabled","value":true}] |
70100
And Updating last share with
71101
| permissions | 4 |
72102
When dropping file "/folder/a.txt" with "abc"

0 commit comments

Comments
 (0)