WIP Modernize Sabre Connector

Signed-off-by: Thomas Citharel <tcit@tcit.fr>

apps/dav/lib/Connector/LegacyDAVACL.php

@@ -42,7 +42,7 @@ public function getCurrentUserPrincipals() {
 			return [];
 		}
 
-		$principalV1 = $this->convertPrincipal($principalV2, false);
+		$principalV1 = $this->convertPrincipal($principalV2);
 		return array_merge(
 			[
 				$principalV2,
@@ -52,11 +52,8 @@ public function getCurrentUserPrincipals() {
 		);
 	}
 
-	private function convertPrincipal($principal, $toV2) {
+	private function convertPrincipal(string $principal): string {
 		[, $name] = \Sabre\Uri\split($principal);
-		if ($toV2) {
-			return "principals/users/$name";
-		}
 		return "principals/$name";
 	}
 

apps/dav/lib/Connector/PublicAuth.php

@@ -30,12 +30,14 @@
 namespace OCA\DAV\Connector;
 
 use OC\Security\Bruteforce\Throttler;
+use OCP\Defaults;
 use OCP\IRequest;
 use OCP\ISession;
 use OCP\Share\Exceptions\ShareNotFound;
 use OCP\Share\IManager;
 use OCP\Share\IShare;
 use Sabre\DAV\Auth\Backend\AbstractBasic;
+use Sabre\DAV\Exception\NotAuthenticated;
 
 /**
  * Class PublicAuth
@@ -45,7 +47,7 @@
 class PublicAuth extends AbstractBasic {
 	private const BRUTEFORCE_ACTION = 'public_webdav_auth';
 
-	/** @var \OCP\Share\IShare */
+	/** @var IShare */
 	private $share;
 
 	/** @var IManager */
@@ -76,7 +78,7 @@ public function __construct(IRequest $request,
 		$this->throttler = $throttler;
 
 		// setup realm
-		$defaults = new \OCP\Defaults();
+		$defaults = new Defaults();
 		$this->realm = $defaults->getName();
 	}
 
@@ -90,7 +92,7 @@ public function __construct(IRequest $request,
 	 * @param string $password
 	 *
 	 * @return bool
-	 * @throws \Sabre\DAV\Exception\NotAuthenticated
+	 * @throws NotAuthenticated
 	 */
 	protected function validateUserPass($username, $password) {
 		$this->throttler->sleepDelayOrThrowOnMax($this->request->getRemoteAddress(), self::BRUTEFORCE_ACTION);
@@ -121,7 +123,7 @@ protected function validateUserPass($username, $password) {
 						// do not re-authenticate over ajax, use dummy auth name to prevent browser popup
 						http_response_code(401);
 						header('WWW-Authenticate: DummyBasic realm="' . $this->realm . '"');
-						throw new \Sabre\DAV\Exception\NotAuthenticated('Cannot authenticate over ajax calls');
+						throw new NotAuthenticated('Cannot authenticate over ajax calls');
 					}
 
 					$this->throttler->registerAttempt(self::BRUTEFORCE_ACTION, $this->request->getRemoteAddress());
@@ -138,10 +140,7 @@ protected function validateUserPass($username, $password) {
 		}
 	}
 
-	/**
-	 * @return \OCP\Share\IShare
-	 */
-	public function getShare() {
+	public function getShare(): IShare {
 		return $this->share;
 	}
 }

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

@@ -51,15 +51,17 @@ public function initialize(\Sabre\DAV\Server $server) {
 	}
 
 	/**
+	 * @param $path
 	 * @return bool
 	 */
-	public function isRequestInRoot($path) {
+	public function isRequestInRoot($path): bool {
 		return $path === '' || (is_string($path) && strpos($path, '/') === false);
 	}
 
 	/**
-	 * @throws \Sabre\DAV\Exception\Forbidden
-	 * @return bool
+	 * @param RequestInterface $request
+	 * @param ResponseInterface $response
+	 * @return false|void
 	 */
 	public function handleAnonymousOptions(RequestInterface $request, ResponseInterface $response) {
 		$isOffice = preg_match('/Microsoft Office/i', $request->getHeader('User-Agent') ?? '');

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

@@ -45,13 +45,13 @@ class AppEnabledPlugin extends ServerPlugin {
 	private $app;
 
 	/**
-	 * @var \OCP\App\IAppManager
+	 * @var IAppManager
 	 */
 	private $appManager;
 
 	/**
 	 * @param string $app
-	 * @param \OCP\App\IAppManager $appManager
+	 * @param IAppManager $appManager
 	 */
 	public function __construct($app, IAppManager $appManager) {
 		$this->app = $app;
@@ -77,10 +77,10 @@ public function initialize(\Sabre\DAV\Server $server) {
 	/**
 	 * This method is called before any HTTP after auth and checks if the user has access to the app
 	 *
-	 * @throws \Sabre\DAV\Exception\Forbidden
-	 * @return bool
+	 * @return void
+	 * @throws Forbidden
 	 */
-	public function checkAppEnabled() {
+	public function checkAppEnabled(): void {
 		if (!$this->appManager->isEnabledForUser($this->app)) {
 			throw new Forbidden();
 		}

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

@@ -37,8 +37,10 @@
 use OC\Authentication\Exceptions\PasswordLoginForbiddenException;
 use OC\Authentication\TwoFactorAuth\Manager;
 use OC\Security\Bruteforce\Throttler;
+use OC\User\LoginException;
 use OC\User\Session;
 use OCA\DAV\Connector\Sabre\Exception\PasswordLoginForbidden;
+use OCP\Defaults;
 use OCP\IRequest;
 use OCP\ISession;
 use OCP\IUserSession;
@@ -87,7 +89,7 @@ public function __construct(ISession $session,
 		$this->principalPrefix = $principalPrefix;
 
 		// setup realm
-		$defaults = new \OCP\Defaults();
+		$defaults = new Defaults();
 		$this->realm = $defaults->getName();
 	}
 
@@ -102,7 +104,7 @@ public function __construct(ISession $session,
 	 * @param string $username
 	 * @return bool
 	 */
-	public function isDavAuthenticated($username) {
+	public function isDavAuthenticated(string $username): bool {
 		return !is_null($this->session->get(self::DAV_AUTHENTICATED)) &&
 		$this->session->get(self::DAV_AUTHENTICATED) === $username;
 	}
@@ -116,7 +118,7 @@ public function isDavAuthenticated($username) {
 	 * @param string $username
 	 * @param string $password
 	 * @return bool
-	 * @throws PasswordLoginForbidden
+	 * @throws PasswordLoginForbidden|LoginException
 	 */
 	protected function validateUserPass($username, $password) {
 		if ($this->userSession->isLoggedIn() &&
@@ -169,7 +171,7 @@ public function check(RequestInterface $request, ResponseInterface $response) {
 	 *
 	 * @return bool
 	 */
-	private function requiresCSRFCheck() {
+	private function requiresCSRFCheck(): bool {
 		// GET requires no check at all
 		if ($this->request->getMethod() === 'GET') {
 			return false;
@@ -209,7 +211,7 @@ private function requiresCSRFCheck() {
 	 * @return array
 	 * @throws NotAuthenticated
 	 */
-	private function auth(RequestInterface $request, ResponseInterface $response) {
+	private function auth(RequestInterface $request, ResponseInterface $response): array {
 		$forcedLogout = false;
 
 		if (!$this->request->passesCSRFCheck() &&
@@ -219,15 +221,15 @@ private function auth(RequestInterface $request, ResponseInterface $response) {
 				$forcedLogout = true;
 			} else {
 				$response->setStatus(401);
-				throw new \Sabre\DAV\Exception\NotAuthenticated('CSRF check not passed.');
+				throw new NotAuthenticated('CSRF check not passed.');
 			}
 		}
 
 		if ($forcedLogout) {
 			$this->userSession->logout();
 		} else {
 			if ($this->twoFactorManager->needsSecondFactor($this->userSession->getUser())) {
-				throw new \Sabre\DAV\Exception\NotAuthenticated('2FA challenge not passed.');
+				throw new NotAuthenticated('2FA challenge not passed.');
 			}
 			if (
 				//Fix for broken webdav clients
@@ -248,7 +250,7 @@ private function auth(RequestInterface $request, ResponseInterface $response) {
 			// do not re-authenticate over ajax, use dummy auth name to prevent browser popup
 			$response->addHeader('WWW-Authenticate','DummyBasic realm="' . $this->realm . '"');
 			$response->setStatus(401);
-			throw new \Sabre\DAV\Exception\NotAuthenticated('Cannot authenticate over ajax calls');
+			throw new NotAuthenticated('Cannot authenticate over ajax calls');
 		}
 
 		$data = parent::check($request, $response);

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

@@ -23,6 +23,7 @@
  */
 namespace OCA\DAV\Connector\Sabre;
 
+use OCP\Defaults;
 use OCP\IRequest;
 use OCP\ISession;
 use OCP\IUserSession;
@@ -49,18 +50,18 @@ class BearerAuth extends AbstractBearer {
 	public function __construct(IUserSession $userSession,
 								ISession $session,
 								IRequest $request,
-								$principalPrefix = 'principals/users/') {
+								string $principalPrefix = 'principals/users/') {
 		$this->userSession = $userSession;
 		$this->session = $session;
 		$this->request = $request;
 		$this->principalPrefix = $principalPrefix;
 
 		// setup realm
-		$defaults = new \OCP\Defaults();
+		$defaults = new Defaults();
 		$this->realm = $defaults->getName();
 	}
 
-	private function setupUserFs($userId) {
+	private function setupUserFs(string $userId): string {
 		\OC_Util::setupFS($userId);
 		$this->session->close();
 		return $this->principalPrefix . $userId;

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

@@ -26,6 +26,7 @@
 namespace OCA\DAV\Connector\Sabre;
 
 use OCP\IConfig;
+use Sabre\DAV\Exception\Forbidden;
 use Sabre\DAV\ServerPlugin;
 use Sabre\HTTP\RequestInterface;
 
@@ -61,7 +62,7 @@ public function initialize(\Sabre\DAV\Server $server) {
 	 * Detects all unsupported clients and throws a \Sabre\DAV\Exception\Forbidden
 	 * exception which will result in a 403 to them.
 	 * @param RequestInterface $request
-	 * @throws \Sabre\DAV\Exception\Forbidden If the client version is not supported
+	 * @throws Forbidden If the client version is not supported
 	 */
 	public function beforeHandler(RequestInterface $request) {
 		$userAgent = $request->getHeader('User-Agent');
@@ -76,7 +77,7 @@ public function beforeHandler(RequestInterface $request) {
 		preg_match("/(?:mirall\\/)([\d.]+)/i", $userAgent, $versionMatches);
 		if (isset($versionMatches[1]) &&
 			version_compare($versionMatches[1], $minimumSupportedDesktopVersion) === -1) {
-			throw new \Sabre\DAV\Exception\Forbidden('Unsupported client version.');
+			throw new Forbidden('Unsupported client version.');
 		}
 	}
 }

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

@@ -30,9 +30,9 @@ class CachingTree extends Tree {
 	 * Store a node in the cache
 	 *
 	 * @param Node $node
-	 * @param null|string $path
+	 * @param string|null $path
 	 */
-	public function cacheNode(Node $node, $path = null) {
+	public function cacheNode(Node $node, string $path = null) {
 		if (is_null($path)) {
 			$path = $node->getPath();
 		}

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

@@ -40,7 +40,7 @@ class ChecksumList implements XmlSerializable {
 	/**
 	 * @param string $checksum
 	 */
-	public function __construct($checksum) {
+	public function __construct(string $checksum) {
 		$this->checksums = explode(',', $checksum);
 	}
 

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

@@ -27,6 +27,7 @@
 
 use OCP\Comments\ICommentsManager;
 use OCP\IUserSession;
+use Sabre\DAV\INode;
 use Sabre\DAV\PropFind;
 use Sabre\DAV\ServerPlugin;
 
@@ -97,19 +98,19 @@ private function cacheDirectory(Directory $directory) {
 	 * if requested.
 	 *
 	 * @param PropFind $propFind
-	 * @param \Sabre\DAV\INode $node
+	 * @param INode $node
 	 * @return void
 	 */
 	public function handleGetProperties(
 		PropFind $propFind,
-		\Sabre\DAV\INode $node
+		INode $node
 	) {
 		if (!($node instanceof File) && !($node instanceof Directory)) {
 			return;
 		}
 
 		// need prefetch ?
-		if ($node instanceof \OCA\DAV\Connector\Sabre\Directory
+		if ($node instanceof Directory
 			&& $propFind->getDepth() !== 0
 			&& !is_null($propFind->getStatus(self::PROPERTY_NAME_UNREAD))
 		) {
@@ -136,7 +137,7 @@ public function handleGetProperties(
 	 * returns a reference to the comments node
 	 *
 	 * @param Node $node
-	 * @return mixed|string
+	 * @return array|string|string[]|null
 	 */
 	public function getCommentsLink(Node $node) {
 		$href = $this->server->getBaseUri();
@@ -146,8 +147,7 @@ public function getCommentsLink(Node $node) {
 			return null;
 		}
 		$commentsPart = 'dav/comments/files/' . rawurldecode($node->getId());
-		$href = substr_replace($href, $commentsPart, $entryPoint + strlen('/remote.php/'));
-		return $href;
+		return substr_replace($href, $commentsPart, $entryPoint + strlen('/remote.php/'));
 	}
 
 	/**
@@ -157,7 +157,7 @@ public function getCommentsLink(Node $node) {
 	 * @param Node $node
 	 * @return Int|null
 	 */
-	public function getUnreadCount(Node $node) {
+	public function getUnreadCount(Node $node): ?int {
 		$user = $this->userSession->getUser();
 		if (is_null($user)) {
 			return null;

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

@@ -25,6 +25,7 @@
 namespace OCA\DAV\Connector\Sabre;
 
 use Sabre\DAV\Exception\NotFound;
+use Sabre\DAV\ServerPlugin;
 use Sabre\HTTP\RequestInterface;
 use Sabre\HTTP\ResponseInterface;
 
@@ -33,7 +34,7 @@
  * This is a workaround for setups that automatically strip
  * or mangle Etag headers.
  */
-class CopyEtagHeaderPlugin extends \Sabre\DAV\ServerPlugin {
+class CopyEtagHeaderPlugin extends ServerPlugin {
 
 	/** @var \Sabre\DAV\Server */
 	private $server;
@@ -57,7 +58,7 @@ public function initialize(\Sabre\DAV\Server $server) {
 	 * @param RequestInterface $request request
 	 * @param ResponseInterface $response response
 	 */
-	public function afterMethod(RequestInterface $request, ResponseInterface $response) {
+	public function afterMethod(RequestInterface $request, ResponseInterface $response): void {
 		$eTag = $response->getHeader('Etag');
 		if (!empty($eTag)) {
 			$response->setHeader('OC-ETag', $eTag);
@@ -73,7 +74,7 @@ public function afterMethod(RequestInterface $request, ResponseInterface $respon
 	 * @param string $destination
 	 * @return void
 	 */
-	public function afterMove($source, $destination) {
+	public function afterMove(string $source, string $destination): void {
 		try {
 			$node = $this->server->tree->getNodeForPath($destination);
 		} catch (NotFound $e) {