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>

apps/dav/lib/Connector/LegacyPublicAuth.php

@@ -67,7 +67,7 @@ class LegacyPublicAuth extends AbstractBasic {
 		\OC_User::setIncognitoMode(true);
 
 		// check if the share is password protected
-		if ($share->getPassword() !== null) {
+		if ($share->isPasswordProtected()) {
 			if ($share->getShareType() === IShare::TYPE_LINK
 				|| $share->getShareType() === IShare::TYPE_EMAIL
 				|| $share->getShareType() === IShare::TYPE_CIRCLE) {

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

@@ -63,7 +63,7 @@ class PublicAuth extends AbstractBasic {
 		try {
 			$this->throttler->sleepDelayOrThrowOnMax($this->request->getRemoteAddress(), self::BRUTEFORCE_ACTION);
 
-			if (count($_COOKIE) > 0 && !$this->request->passesStrictCookieCheck() && $this->getShare()->getPassword() !== null) {
+			if (count($_COOKIE) > 0 && !$this->request->passesStrictCookieCheck() && $this->getShare()->isPasswordProtected()) {
 				throw new PreconditionFailed('Strict cookie check failed');
 			}
 
@@ -142,7 +142,7 @@ class PublicAuth extends AbstractBasic {
 		}
 
 		// If the share is protected but user is not authenticated
-		if ($share->getPassword() !== null) {
+		if ($share->isPasswordProtected()) {
 			$this->throttler->registerAttempt(self::BRUTEFORCE_ACTION, $this->request->getRemoteAddress());
 			throw new NotAuthenticated();
 		}
@@ -176,7 +176,7 @@ class PublicAuth extends AbstractBasic {
 		\OC_User::setIncognitoMode(true);
 
 		// check if the share is password protected
-		if ($share->getPassword() !== null) {
+		if ($share->isPasswordProtected()) {
 			if ($share->getShareType() === IShare::TYPE_LINK
 				|| $share->getShareType() === IShare::TYPE_EMAIL
 				|| $share->getShareType() === IShare::TYPE_CIRCLE) {

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

@@ -73,6 +73,7 @@ class LegacyPublicAuthTest extends TestCase {
 	public function testShareNoPassword(): void {
 		$share = $this->createMock(IShare::class);
 		$share->method('getPassword')->willReturn(null);
+		$share->method('isPasswordProtected')->willReturn(false);
 
 		$this->shareManager->expects($this->once())
 			->method('getShareByToken')
@@ -86,6 +87,7 @@ class LegacyPublicAuthTest extends TestCase {
 	public function testSharePasswordFancyShareType(): void {
 		$share = $this->createMock(IShare::class);
 		$share->method('getPassword')->willReturn('password');
+		$share->method('isPasswordProtected')->willReturn(true);
 		$share->method('getShareType')->willReturn(42);
 
 		$this->shareManager->expects($this->once())
@@ -100,6 +102,7 @@ class LegacyPublicAuthTest extends TestCase {
 	public function testSharePasswordRemote(): void {
 		$share = $this->createMock(IShare::class);
 		$share->method('getPassword')->willReturn('password');
+		$share->method('isPasswordProtected')->willReturn(true);
 		$share->method('getShareType')->willReturn(IShare::TYPE_REMOTE);
 
 		$this->shareManager->expects($this->once())
@@ -114,6 +117,7 @@ class LegacyPublicAuthTest extends TestCase {
 	public function testSharePasswordLinkValidPassword(): void {
 		$share = $this->createMock(IShare::class);
 		$share->method('getPassword')->willReturn('password');
+		$share->method('isPasswordProtected')->willReturn(true);
 		$share->method('getShareType')->willReturn(IShare::TYPE_LINK);
 
 		$this->shareManager->expects($this->once())
@@ -134,6 +138,7 @@ class LegacyPublicAuthTest extends TestCase {
 	public function testSharePasswordMailValidPassword(): void {
 		$share = $this->createMock(IShare::class);
 		$share->method('getPassword')->willReturn('password');
+		$share->method('isPasswordProtected')->willReturn(true);
 		$share->method('getShareType')->willReturn(IShare::TYPE_EMAIL);
 
 		$this->shareManager->expects($this->once())
@@ -154,6 +159,7 @@ class LegacyPublicAuthTest extends TestCase {
 	public function testInvalidSharePasswordLinkValidSession(): void {
 		$share = $this->createMock(IShare::class);
 		$share->method('getPassword')->willReturn('password');
+		$share->method('isPasswordProtected')->willReturn(true);
 		$share->method('getShareType')->willReturn(IShare::TYPE_LINK);
 		$share->method('getId')->willReturn('42');
 
@@ -178,6 +184,7 @@ class LegacyPublicAuthTest extends TestCase {
 	public function testSharePasswordLinkInvalidSession(): void {
 		$share = $this->createMock(IShare::class);
 		$share->method('getPassword')->willReturn('password');
+		$share->method('isPasswordProtected')->willReturn(true);
 		$share->method('getShareType')->willReturn(IShare::TYPE_LINK);
 		$share->method('getId')->willReturn('42');
 
@@ -202,6 +209,7 @@ class LegacyPublicAuthTest extends TestCase {
 	public function testSharePasswordMailInvalidSession(): void {
 		$share = $this->createMock(IShare::class);
 		$share->method('getPassword')->willReturn('password');
+		$share->method('isPasswordProtected')->willReturn(true);
 		$share->method('getShareType')->willReturn(IShare::TYPE_EMAIL);
 		$share->method('getId')->willReturn('42');
 

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

@@ -97,6 +97,7 @@ class PublicAuthTest extends \Test\TestCase {
 
 		$share = $this->createMock(IShare::class);
 		$share->method('getPassword')->willReturn(null);
+		$share->method('isPasswordProtected')->willReturn(false);
 
 		$this->shareManager->expects($this->once())
 			->method('getShareByToken')
@@ -146,6 +147,7 @@ class PublicAuthTest extends \Test\TestCase {
 
 		$share = $this->createMock(IShare::class);
 		$share->method('getPassword')->willReturn('password');
+		$share->method('isPasswordProtected')->willReturn(true);
 		$share->method('getShareType')->willReturn(42);
 
 		$this->shareManager->expects($this->once())
@@ -165,6 +167,7 @@ class PublicAuthTest extends \Test\TestCase {
 
 		$share = $this->createMock(IShare::class);
 		$share->method('getPassword')->willReturn('password');
+		$share->method('isPasswordProtected')->willReturn(true);
 		$share->method('getShareType')->willReturn(42);
 
 		$this->shareManager->expects($this->once())
@@ -199,6 +202,7 @@ class PublicAuthTest extends \Test\TestCase {
 
 		$share = $this->createMock(IShare::class);
 		$share->method('getPassword')->willReturn(null);
+		$share->method('isPasswordProtected')->willReturn(false);
 
 		$this->shareManager->expects($this->once())
 			->method('getShareByToken')
@@ -216,6 +220,7 @@ class PublicAuthTest extends \Test\TestCase {
 
 		$share = $this->createMock(IShare::class);
 		$share->method('getPassword')->willReturn('password');
+		$share->method('isPasswordProtected')->willReturn(true);
 		$share->method('getShareType')->willReturn(42);
 
 		$this->shareManager->expects($this->once())
@@ -234,6 +239,7 @@ class PublicAuthTest extends \Test\TestCase {
 
 		$share = $this->createMock(IShare::class);
 		$share->method('getPassword')->willReturn('password');
+		$share->method('isPasswordProtected')->willReturn(true);
 		$share->method('getShareType')->willReturn(IShare::TYPE_REMOTE);
 
 		$this->shareManager->expects($this->once())
@@ -252,6 +258,7 @@ class PublicAuthTest extends \Test\TestCase {
 
 		$share = $this->createMock(IShare::class);
 		$share->method('getPassword')->willReturn('password');
+		$share->method('isPasswordProtected')->willReturn(true);
 		$share->method('getShareType')->willReturn(IShare::TYPE_LINK);
 
 		$this->shareManager->expects($this->once())
@@ -276,6 +283,7 @@ class PublicAuthTest extends \Test\TestCase {
 
 		$share = $this->createMock(IShare::class);
 		$share->method('getPassword')->willReturn('password');
+		$share->method('isPasswordProtected')->willReturn(true);
 		$share->method('getShareType')->willReturn(IShare::TYPE_EMAIL);
 
 		$this->shareManager->expects($this->once())
@@ -300,6 +308,7 @@ class PublicAuthTest extends \Test\TestCase {
 
 		$share = $this->createMock(IShare::class);
 		$share->method('getPassword')->willReturn('password');
+		$share->method('isPasswordProtected')->willReturn(true);
 		$share->method('getShareType')->willReturn(IShare::TYPE_LINK);
 		$share->method('getId')->willReturn('42');
 
@@ -329,6 +338,7 @@ class PublicAuthTest extends \Test\TestCase {
 
 		$share = $this->createMock(IShare::class);
 		$share->method('getPassword')->willReturn('password');
+		$share->method('isPasswordProtected')->willReturn(true);
 		$share->method('getShareType')->willReturn(IShare::TYPE_LINK);
 		$share->method('getId')->willReturn('42');
 
@@ -358,6 +368,7 @@ class PublicAuthTest extends \Test\TestCase {
 
 		$share = $this->createMock(IShare::class);
 		$share->method('getPassword')->willReturn('password');
+		$share->method('isPasswordProtected')->willReturn(true);
 		$share->method('getShareType')->willReturn(IShare::TYPE_EMAIL);
 		$share->method('getId')->willReturn('42');
 

apps/federatedfilesharing/lib/Controller/MountPublicLinkController.php

@@ -98,8 +98,7 @@ class MountPublicLinkController extends Controller {
 		$authenticated = in_array($share->getId(), $allowedShareIds)
 			|| $this->shareManager->checkPassword($share, $password);
 
-		$storedPassword = $share->getPassword();
-		if (!empty($storedPassword) && !$authenticated) {
+		if ($share->isPasswordProtected() && !$authenticated) {
 			$response = new JSONResponse(
 				['message' => 'No permission to access the share'],
 				Http::STATUS_BAD_REQUEST

apps/files_sharing/lib/Controller/PublicPreviewController.php

@@ -1,7 +1,7 @@
 <?php
 
 /**
- * SPDX-FileCopyrightText: 2016 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-FileCopyrightText: 2016-2026 Nextcloud GmbH and Nextcloud contributors
  * SPDX-License-Identifier: AGPL-3.0-or-later
  */
 
@@ -60,7 +60,7 @@ class PublicPreviewController extends PublicShareController {
 
 	#[\Override]
 	protected function isPasswordProtected(): bool {
-		return $this->share->getPassword() !== null;
+		return $this->share->isPasswordProtected();
 	}
 
 	/**
@@ -181,7 +181,7 @@ class PublicPreviewController extends PublicShareController {
 		}
 
 		// Password protected shares have no direct link!
-		if ($share->getPassword() !== null) {
+		if ($share->isPasswordProtected()) {
 			return new DataResponse([], Http::STATUS_FORBIDDEN);
 		}
 

apps/files_sharing/lib/Controller/ShareController.php

@@ -1,7 +1,7 @@
 <?php
 
 /**
- * SPDX-FileCopyrightText: 2016-2024 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-FileCopyrightText: 2016-2026 Nextcloud GmbH and Nextcloud contributors
  * SPDX-FileCopyrightText: 2016 ownCloud, Inc.
  * SPDX-License-Identifier: AGPL-3.0-only
  */
@@ -176,7 +176,7 @@ class ShareController extends AuthPublicShareController {
 
 	#[\Override]
 	protected function isPasswordProtected(): bool {
-		return $this->share->getPassword() !== null;
+		return $this->share->isPasswordProtected();
 	}
 
 	#[\Override]

apps/files_sharing/lib/Controller/ShareInfoController.php

@@ -1,7 +1,7 @@
 <?php
 
 /**
- * SPDX-FileCopyrightText: 2016 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-FileCopyrightText: 2016-2026 Nextcloud GmbH and Nextcloud contributors
  * SPDX-License-Identifier: AGPL-3.0-or-later
  */
 
@@ -70,7 +70,7 @@ class ShareInfoController extends ApiController {
 			return $response;
 		}
 
-		if ($share->getPassword() && !$this->shareManager->checkPassword($share, $password)) {
+		if ($share->isPasswordProtected() && !$this->shareManager->checkPassword($share, $password)) {
 			$response = new JSONResponse([], Http::STATUS_FORBIDDEN);
 			$response->throttle(['token' => $t]);
 			return $response;

apps/files_sharing/tests/Controller/ShareAPIControllerTest.php

@@ -638,6 +638,7 @@ class ShareAPIControllerTest extends TestCase {
 		$share->method('getMailSend')->willReturn($mail_send);
 		$share->method('getToken')->willReturn($token);
 		$share->method('getPassword')->willReturn($password);
+		$share->method('isPasswordProtected')->willReturn(!empty($password));
 
 		if ($shareType === IShare::TYPE_USER
 			|| $shareType === IShare::TYPE_GROUP

apps/files_sharing/tests/Controller/ShareControllerTest.php

@@ -702,6 +702,7 @@ class ShareControllerTest extends \Test\TestCase {
 	public function testDownloadShareWithCreateOnlyShare(): void {
 		$share = $this->getMockBuilder(IShare::class)->getMock();
 		$share->method('getPassword')->willReturn('password');
+		$share->method('isPasswordProtected')->willReturn(true);
 		$share
 			->expects($this->once())
 			->method('getPermissions')
@@ -728,6 +729,7 @@ class ShareControllerTest extends \Test\TestCase {
 
 		$share = $this->createMock(IShare::class);
 		$share->method('getPassword')->willReturn('password');
+		$share->method('isPasswordProtected')->willReturn(true);
 		$share->expects(self::once())
 			->method('getPermissions')
 			->willReturn(Constants::PERMISSION_READ);

apps/files_sharing/tests/Controller/ShareInfoControllerTest.php

@@ -51,6 +51,7 @@ class ShareInfoControllerTest extends TestCase {
 		$share = $this->createMock(IShare::class);
 		$share->method('getPassword')
 			->willReturn('sharePass');
+		$share->method('isPasswordProtected')->willReturn(true);
 
 		$this->shareManager->method('getShareByToken')
 			->with('token')
@@ -68,6 +69,7 @@ class ShareInfoControllerTest extends TestCase {
 		$share = $this->createMock(IShare::class);
 		$share->method('getPassword')
 			->willReturn('sharePass');
+		$share->method('isPasswordProtected')->willReturn(true);
 		$share->method('getPermissions')
 			->willReturn(Constants::PERMISSION_CREATE);
 
@@ -109,6 +111,7 @@ class ShareInfoControllerTest extends TestCase {
 		$share = $this->createMock(IShare::class);
 		$share->method('getPassword')
 			->willReturn('sharePass');
+		$share->method('isPasswordProtected')->willReturn(true);
 		$share->method('getPermissions')
 			->willReturn(Constants::PERMISSION_READ | Constants::PERMISSION_UPDATE);
 		$share->method('getNode')
@@ -141,6 +144,7 @@ class ShareInfoControllerTest extends TestCase {
 		$share = $this->createMock(IShare::class);
 		$share->method('getPassword')
 			->willReturn('sharePass');
+		$share->method('isPasswordProtected')->willReturn(true);
 		$share->method('getPermissions')
 			->willReturn(Constants::PERMISSION_READ);
 		$share->method('getNode')
@@ -222,6 +226,7 @@ class ShareInfoControllerTest extends TestCase {
 		$share = $this->createMock(IShare::class);
 		$share->method('getPassword')
 			->willReturn('sharePass');
+		$share->method('isPasswordProtected')->willReturn(true);
 		$share->method('getPermissions')
 			->willReturn(Constants::PERMISSION_READ | Constants::PERMISSION_UPDATE);
 		$share->method('getNode')

lib/private/Share20/Manager.php

@@ -1,7 +1,7 @@
 <?php
 
 /**
- * SPDX-FileCopyrightText: 2016-2024 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-FileCopyrightText: 2016-2026 Nextcloud GmbH and Nextcloud contributors
  * SPDX-FileCopyrightText: 2016 ownCloud, Inc.
  * SPDX-License-Identifier: AGPL-3.0-only
  */
@@ -1427,8 +1427,8 @@ class Manager implements IManager {
 	#[Override]
 	public function checkPassword(IShare $share, ?string $password): bool {
 
-		// if there is no password on the share object / passsword is null, there is nothing to check
-		if ($password === null || $share->getPassword() === null) {
+		// if the share is not password protected or the password to check is empty, there is nothing to check
+		if ($password === null || !$share->isPasswordProtected()) {
 			return false;
 		}
 

lib/private/Share20/Share.php

@@ -1,7 +1,7 @@
 <?php
 
 /**
- * SPDX-FileCopyrightText: 2016-2024 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-FileCopyrightText: 2016-2026 Nextcloud GmbH and Nextcloud contributors
  * SPDX-FileCopyrightText: 2016 ownCloud, Inc.
  * SPDX-License-Identifier: AGPL-3.0-only
  */
@@ -507,6 +507,14 @@ class Share implements IShare {
 		return $this->password;
 	}
 
+	/**
+	 * @inheritdoc
+	 */
+	#[\Override]
+	public function isPasswordProtected(): bool {
+		return $this->password !== '' && $this->password !== null;
+	}
+
 	/**
 	 * @inheritdoc
 	 */

lib/public/Share/IShare.php

@@ -1,7 +1,7 @@
 <?php
 
 /**
- * SPDX-FileCopyrightText: 2016-2024 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-FileCopyrightText: 2016-2026 Nextcloud GmbH and Nextcloud contributors
  * SPDX-FileCopyrightText: 2016 ownCloud, Inc.
  * SPDX-License-Identifier: AGPL-3.0-only
  */
@@ -468,6 +468,13 @@ interface IShare {
 	 */
 	public function getPassword();
 
+	/**
+	 * Returns whether the share is password protected by any means (e.g. password or OTP)
+	 * @return bool
+	 * @since 35.0.0
+	 */
+	public function isPasswordProtected(): bool;
+
 	/**
 	 * Set the password's expiration time of this share.
 	 *

tests/lib/Share20/ManagerTest.php

@@ -955,6 +955,7 @@ class ManagerTest extends \Test\TestCase {
 		$share->method('getAttributes')->willReturn($attributes);
 		$share->method('getExpirationDate')->willReturn($expireDate);
 		$share->method('getPassword')->willReturn($password);
+		$share->method('isPasswordProtected')->willReturn(!empty($password));
 
 		return $share;
 	}
@@ -4126,6 +4127,7 @@ class ManagerTest extends \Test\TestCase {
 		$this->assertFalse($this->manager->checkPassword($share, 'password'));
 
 		$share->method('getPassword')->willReturn('password');
+		$share->method('isPasswordProtected')->willReturn(true);
 		$this->assertFalse($this->manager->checkPassword($share, null));
 	}
 
@@ -4133,6 +4135,7 @@ class ManagerTest extends \Test\TestCase {
 		$share = $this->createMock(IShare::class);
 		$share->method('getShareType')->willReturn(IShare::TYPE_LINK);
 		$share->method('getPassword')->willReturn('password');
+		$share->method('isPasswordProtected')->willReturn(true);
 
 		$this->hasher->method('verify')->with('invalidpassword', 'password', '')->willReturn(false);
 
@@ -4143,6 +4146,7 @@ class ManagerTest extends \Test\TestCase {
 		$share = $this->createMock(IShare::class);
 		$share->method('getShareType')->willReturn(IShare::TYPE_LINK);
 		$share->method('getPassword')->willReturn('passwordHash');
+		$share->method('isPasswordProtected')->willReturn(true);
 
 		$this->hasher->method('verify')->with('password', 'passwordHash', '')->willReturn(true);