Skip to content

Commit 10dd884

Browse files
theCalcaholictheCalcaholic
committed
chore(core/share): Centralize logic for checking if a share is password protected in IShare->isPasswordProtected()
Signed-off-by: Tobias Knöppler <tobias@knoeppler.org>
1 parent 5d3637d commit 10dd884

15 files changed

Lines changed: 63 additions & 18 deletions

File tree

‎apps/dav/lib/Connector/LegacyPublicAuth.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ protected function validateUserPass($username, $password) {
6767
\OC_User::setIncognitoMode(true);
6868

6969
// check if the share is password protected
70-
if ($share->getPassword() !== null) {
70+
if ($share->isPasswordProtected()) {
7171
if ($share->getShareType() === IShare::TYPE_LINK
7272
|| $share->getShareType() === IShare::TYPE_EMAIL
7373
|| $share->getShareType() === IShare::TYPE_CIRCLE) {

‎apps/dav/lib/Connector/Sabre/PublicAuth.php‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ public function check(RequestInterface $request, ResponseInterface $response): a
6363
try {
6464
$this->throttler->sleepDelayOrThrowOnMax($this->request->getRemoteAddress(), self::BRUTEFORCE_ACTION);
6565

66-
if (count($_COOKIE) > 0 && !$this->request->passesStrictCookieCheck() && $this->getShare()->getPassword() !== null) {
66+
if (count($_COOKIE) > 0 && !$this->request->passesStrictCookieCheck() && $this->getShare()->isPasswordProtected()) {
6767
throw new PreconditionFailed('Strict cookie check failed');
6868
}
6969

@@ -142,7 +142,7 @@ private function checkToken(): array {
142142
}
143143

144144
// If the share is protected but user is not authenticated
145-
if ($share->getPassword() !== null) {
145+
if ($share->isPasswordProtected()) {
146146
$this->throttler->registerAttempt(self::BRUTEFORCE_ACTION, $this->request->getRemoteAddress());
147147
throw new NotAuthenticated();
148148
}
@@ -176,7 +176,7 @@ protected function validateUserPass($username, $password) {
176176
\OC_User::setIncognitoMode(true);
177177

178178
// check if the share is password protected
179-
if ($share->getPassword() !== null) {
179+
if ($share->isPasswordProtected()) {
180180
if ($share->getShareType() === IShare::TYPE_LINK
181181
|| $share->getShareType() === IShare::TYPE_EMAIL
182182
|| $share->getShareType() === IShare::TYPE_CIRCLE) {

‎apps/dav/tests/unit/Connector/LegacyPublicAuthTest.php‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ public function testNoShare(): void {
7373
public function testShareNoPassword(): void {
7474
$share = $this->createMock(IShare::class);
7575
$share->method('getPassword')->willReturn(null);
76+
$share->method('isPasswordProtected')->willReturn(false);
7677

7778
$this->shareManager->expects($this->once())
7879
->method('getShareByToken')
@@ -86,6 +87,7 @@ public function testShareNoPassword(): void {
8687
public function testSharePasswordFancyShareType(): void {
8788
$share = $this->createMock(IShare::class);
8889
$share->method('getPassword')->willReturn('password');
90+
$share->method('isPasswordProtected')->willReturn(true);
8991
$share->method('getShareType')->willReturn(42);
9092

9193
$this->shareManager->expects($this->once())
@@ -100,6 +102,7 @@ public function testSharePasswordFancyShareType(): void {
100102
public function testSharePasswordRemote(): void {
101103
$share = $this->createMock(IShare::class);
102104
$share->method('getPassword')->willReturn('password');
105+
$share->method('isPasswordProtected')->willReturn(true);
103106
$share->method('getShareType')->willReturn(IShare::TYPE_REMOTE);
104107

105108
$this->shareManager->expects($this->once())
@@ -114,6 +117,7 @@ public function testSharePasswordRemote(): void {
114117
public function testSharePasswordLinkValidPassword(): void {
115118
$share = $this->createMock(IShare::class);
116119
$share->method('getPassword')->willReturn('password');
120+
$share->method('isPasswordProtected')->willReturn(true);
117121
$share->method('getShareType')->willReturn(IShare::TYPE_LINK);
118122

119123
$this->shareManager->expects($this->once())
@@ -134,6 +138,7 @@ public function testSharePasswordLinkValidPassword(): void {
134138
public function testSharePasswordMailValidPassword(): void {
135139
$share = $this->createMock(IShare::class);
136140
$share->method('getPassword')->willReturn('password');
141+
$share->method('isPasswordProtected')->willReturn(true);
137142
$share->method('getShareType')->willReturn(IShare::TYPE_EMAIL);
138143

139144
$this->shareManager->expects($this->once())
@@ -154,6 +159,7 @@ public function testSharePasswordMailValidPassword(): void {
154159
public function testInvalidSharePasswordLinkValidSession(): void {
155160
$share = $this->createMock(IShare::class);
156161
$share->method('getPassword')->willReturn('password');
162+
$share->method('isPasswordProtected')->willReturn(true);
157163
$share->method('getShareType')->willReturn(IShare::TYPE_LINK);
158164
$share->method('getId')->willReturn('42');
159165

@@ -178,6 +184,7 @@ public function testInvalidSharePasswordLinkValidSession(): void {
178184
public function testSharePasswordLinkInvalidSession(): void {
179185
$share = $this->createMock(IShare::class);
180186
$share->method('getPassword')->willReturn('password');
187+
$share->method('isPasswordProtected')->willReturn(true);
181188
$share->method('getShareType')->willReturn(IShare::TYPE_LINK);
182189
$share->method('getId')->willReturn('42');
183190

@@ -202,6 +209,7 @@ public function testSharePasswordLinkInvalidSession(): void {
202209
public function testSharePasswordMailInvalidSession(): void {
203210
$share = $this->createMock(IShare::class);
204211
$share->method('getPassword')->willReturn('password');
212+
$share->method('isPasswordProtected')->willReturn(true);
205213
$share->method('getShareType')->willReturn(IShare::TYPE_EMAIL);
206214
$share->method('getId')->willReturn('42');
207215

‎apps/dav/tests/unit/Connector/Sabre/PublicAuthTest.php‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,7 @@ public function testCheckTokenValidShare(): void {
9797

9898
$share = $this->createMock(IShare::class);
9999
$share->method('getPassword')->willReturn(null);
100+
$share->method('isPasswordProtected')->willReturn(false);
100101

101102
$this->shareManager->expects($this->once())
102103
->method('getShareByToken')
@@ -146,6 +147,7 @@ public function testCheckTokenPasswordNotAuthenticated(): void {
146147

147148
$share = $this->createMock(IShare::class);
148149
$share->method('getPassword')->willReturn('password');
150+
$share->method('isPasswordProtected')->willReturn(true);
149151
$share->method('getShareType')->willReturn(42);
150152

151153
$this->shareManager->expects($this->once())
@@ -165,6 +167,7 @@ public function testCheckTokenPasswordAuthenticatedWrongShare(): void {
165167

166168
$share = $this->createMock(IShare::class);
167169
$share->method('getPassword')->willReturn('password');
170+
$share->method('isPasswordProtected')->willReturn(true);
168171
$share->method('getShareType')->willReturn(42);
169172

170173
$this->shareManager->expects($this->once())
@@ -199,6 +202,7 @@ public function testShareNoPassword(): void {
199202

200203
$share = $this->createMock(IShare::class);
201204
$share->method('getPassword')->willReturn(null);
205+
$share->method('isPasswordProtected')->willReturn(false);
202206

203207
$this->shareManager->expects($this->once())
204208
->method('getShareByToken')
@@ -216,6 +220,7 @@ public function testSharePasswordFancyShareType(): void {
216220

217221
$share = $this->createMock(IShare::class);
218222
$share->method('getPassword')->willReturn('password');
223+
$share->method('isPasswordProtected')->willReturn(true);
219224
$share->method('getShareType')->willReturn(42);
220225

221226
$this->shareManager->expects($this->once())
@@ -234,6 +239,7 @@ public function testSharePasswordRemote(): void {
234239

235240
$share = $this->createMock(IShare::class);
236241
$share->method('getPassword')->willReturn('password');
242+
$share->method('isPasswordProtected')->willReturn(true);
237243
$share->method('getShareType')->willReturn(IShare::TYPE_REMOTE);
238244

239245
$this->shareManager->expects($this->once())
@@ -252,6 +258,7 @@ public function testSharePasswordLinkValidPassword(): void {
252258

253259
$share = $this->createMock(IShare::class);
254260
$share->method('getPassword')->willReturn('password');
261+
$share->method('isPasswordProtected')->willReturn(true);
255262
$share->method('getShareType')->willReturn(IShare::TYPE_LINK);
256263

257264
$this->shareManager->expects($this->once())
@@ -276,6 +283,7 @@ public function testSharePasswordMailValidPassword(): void {
276283

277284
$share = $this->createMock(IShare::class);
278285
$share->method('getPassword')->willReturn('password');
286+
$share->method('isPasswordProtected')->willReturn(true);
279287
$share->method('getShareType')->willReturn(IShare::TYPE_EMAIL);
280288

281289
$this->shareManager->expects($this->once())
@@ -300,6 +308,7 @@ public function testInvalidSharePasswordLinkValidSession(): void {
300308

301309
$share = $this->createMock(IShare::class);
302310
$share->method('getPassword')->willReturn('password');
311+
$share->method('isPasswordProtected')->willReturn(true);
303312
$share->method('getShareType')->willReturn(IShare::TYPE_LINK);
304313
$share->method('getId')->willReturn('42');
305314

@@ -329,6 +338,7 @@ public function testSharePasswordLinkInvalidSession(): void {
329338

330339
$share = $this->createMock(IShare::class);
331340
$share->method('getPassword')->willReturn('password');
341+
$share->method('isPasswordProtected')->willReturn(true);
332342
$share->method('getShareType')->willReturn(IShare::TYPE_LINK);
333343
$share->method('getId')->willReturn('42');
334344

@@ -358,6 +368,7 @@ public function testSharePasswordMailInvalidSession(): void {
358368

359369
$share = $this->createMock(IShare::class);
360370
$share->method('getPassword')->willReturn('password');
371+
$share->method('isPasswordProtected')->willReturn(true);
361372
$share->method('getShareType')->willReturn(IShare::TYPE_EMAIL);
362373
$share->method('getId')->willReturn('42');
363374

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -98,8 +98,7 @@ public function createFederatedShare($shareWith, $token, $password = '') {
9898
$authenticated = in_array($share->getId(), $allowedShareIds)
9999
|| $this->shareManager->checkPassword($share, $password);
100100

101-
$storedPassword = $share->getPassword();
102-
if (!empty($storedPassword) && !$authenticated) {
101+
if ($share->isPasswordProtected() && !$authenticated) {
103102
$response = new JSONResponse(
104103
['message' => 'No permission to access the share'],
105104
Http::STATUS_BAD_REQUEST

‎apps/files_sharing/lib/Controller/PublicPreviewController.php‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
<?php
22

33
/**
4-
* SPDX-FileCopyrightText: 2016 Nextcloud GmbH and Nextcloud contributors
4+
* SPDX-FileCopyrightText: 2016-2026 Nextcloud GmbH and Nextcloud contributors
55
* SPDX-License-Identifier: AGPL-3.0-or-later
66
*/
77

@@ -60,7 +60,7 @@ public function isValidToken(): bool {
6060

6161
#[\Override]
6262
protected function isPasswordProtected(): bool {
63-
return $this->share->getPassword() !== null;
63+
return $this->share->isPasswordProtected();
6464
}
6565

6666
/**
@@ -181,7 +181,7 @@ public function directLink(string $token) {
181181
}
182182

183183
// Password protected shares have no direct link!
184-
if ($share->getPassword() !== null) {
184+
if ($share->isPasswordProtected()) {
185185
return new DataResponse([], Http::STATUS_FORBIDDEN);
186186
}
187187

‎apps/files_sharing/lib/Controller/ShareController.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
<?php
22

33
/**
4-
* SPDX-FileCopyrightText: 2016-2024 Nextcloud GmbH and Nextcloud contributors
4+
* SPDX-FileCopyrightText: 2016-2026 Nextcloud GmbH and Nextcloud contributors
55
* SPDX-FileCopyrightText: 2016 ownCloud, Inc.
66
* SPDX-License-Identifier: AGPL-3.0-only
77
*/
@@ -176,7 +176,7 @@ public function isValidToken(): bool {
176176

177177
#[\Override]
178178
protected function isPasswordProtected(): bool {
179-
return $this->share->getPassword() !== null;
179+
return $this->share->isPasswordProtected();
180180
}
181181

182182
#[\Override]

‎apps/files_sharing/lib/Controller/ShareInfoController.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
<?php
22

33
/**
4-
* SPDX-FileCopyrightText: 2016 Nextcloud GmbH and Nextcloud contributors
4+
* SPDX-FileCopyrightText: 2016-2026 Nextcloud GmbH and Nextcloud contributors
55
* SPDX-License-Identifier: AGPL-3.0-or-later
66
*/
77

@@ -70,7 +70,7 @@ public function info(string $t, ?string $password = null, ?string $dir = null, i
7070
return $response;
7171
}
7272

73-
if ($share->getPassword() && !$this->shareManager->checkPassword($share, $password)) {
73+
if ($share->isPasswordProtected() && !$this->shareManager->checkPassword($share, $password)) {
7474
$response = new JSONResponse([], Http::STATUS_FORBIDDEN);
7575
$response->throttle(['token' => $t]);
7676
return $response;

‎apps/files_sharing/tests/Controller/ShareAPIControllerTest.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -638,6 +638,7 @@ public function createShare(
638638
$share->method('getMailSend')->willReturn($mail_send);
639639
$share->method('getToken')->willReturn($token);
640640
$share->method('getPassword')->willReturn($password);
641+
$share->method('isPasswordProtected')->willReturn(!empty($password));
641642

642643
if ($shareType === IShare::TYPE_USER
643644
|| $shareType === IShare::TYPE_GROUP

‎apps/files_sharing/tests/Controller/ShareControllerTest.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -702,6 +702,7 @@ public function testShowShareInvalid(): void {
702702
public function testDownloadShareWithCreateOnlyShare(): void {
703703
$share = $this->getMockBuilder(IShare::class)->getMock();
704704
$share->method('getPassword')->willReturn('password');
705+
$share->method('isPasswordProtected')->willReturn(true);
705706
$share
706707
->expects($this->once())
707708
->method('getPermissions')
@@ -728,6 +729,7 @@ public function testDownloadShareWithoutDownloadPermission(): void {
728729

729730
$share = $this->createMock(IShare::class);
730731
$share->method('getPassword')->willReturn('password');
732+
$share->method('isPasswordProtected')->willReturn(true);
731733
$share->expects(self::once())
732734
->method('getPermissions')
733735
->willReturn(Constants::PERMISSION_READ);

0 commit comments

Comments
 (0)