diff --git a/lib/private/Files/Storage/DAV.php b/lib/private/Files/Storage/DAV.php index efcceba7e6892..fe9eacc880f0e 100644 --- a/lib/private/Files/Storage/DAV.php +++ b/lib/private/Files/Storage/DAV.php @@ -241,7 +241,7 @@ protected function init(): void { // For Basic auth, the share token is kept as the user name $token = $this->user; // If using Bearer auth, use stored access token or exchange refresh token for access token - if ($this->authType !== null && ($this->authType & BearerAuthAwareSabreClient::AUTH_BEARER)) { + if ($this->isBearerAuth()) { // Check if we already have an access token stored (password field) if (!empty($this->password)) { $token = $this->password; @@ -410,11 +410,56 @@ protected function exchangeRefreshToken(): string { } /** - * Check if bearer authentication is being used + * Check if bearer authentication is being used. */ protected function isBearerAuth(): bool { return $this->authType !== null - && ($this->authType & BearerAuthAwareSabreClient::AUTH_BEARER); + && ($this->authType & BearerAuthAwareSabreClient::AUTH_BEARER) !== 0; + } + + /** + * Check if digest authentication is being used. + */ + protected function isDigestAuth(): bool { + return $this->authType !== null + && ($this->authType & Client::AUTH_DIGEST) !== 0; + } + + /** + * Return authentication options for the generic HTTP client that are + * equivalent to the authentication configured for the Sabre DAV client. + * + * @return array{ + * auth: list, + * headers?: array, + * } + */ + protected function getHttpAuthOptions(): array { + if ($this->isBearerAuth()) { + return [ + 'auth' => [], + 'headers' => [ + 'Authorization' => 'Bearer ' . $this->bearerToken, + ], + ]; + } + + if ($this->isDigestAuth()) { + return [ + 'auth' => [ + $this->user, + $this->password, + 'digest', + ], + ]; + } + + return [ + 'auth' => [ + $this->user, + $this->password, + ], + ]; } /** Guard against re-entry while a Guzzle-path 401 is being recovered. */ @@ -661,23 +706,15 @@ public function fopen(string $path, string $mode) { case 'rb': try { $response = $this->withAuthRetry(function () use ($path) { - if ($this->authType === BearerAuthAwareSabreClient::AUTH_BEARER) { - $auth = []; - $headers = ['Authorization' => 'Bearer ' . $this->bearerToken]; - } else { - $auth = [$this->user, $this->password]; - $headers = []; - } + $options = $this->getHttpAuthOptions(); + $options['stream'] = true; + // set download timeout for users with slow connections or large files + $options['timeout'] = $this->timeout; + $options['verify'] = $this->verify; + return $this->httpClientService ->newClient() - ->get($this->createBaseUri() . $this->encodePath($path), [ - 'headers' => $headers, - 'auth' => $auth, - 'stream' => true, - // set download timeout for users with slow connections or large files - 'timeout' => $this->timeout, - 'verify' => $this->verify, - ]); + ->get($this->createBaseUri() . $this->encodePath($path), $options); }); } catch (\GuzzleHttp\Exception\ClientException $e) { if ($e->getResponse() instanceof ResponseInterface @@ -824,22 +861,15 @@ protected function uploadFile(string $path, string $target): void { $this->withAuthRetry(function () use ($path, $target): void { $source = fopen($path, 'r'); - $auth = [$this->user, $this->password]; - $headers = []; - if ($this->authType === BearerAuthAwareSabreClient::AUTH_BEARER) { - $auth = []; - $headers = ['Authorization' => 'Bearer ' . $this->bearerToken]; - } + $options = $this->getHttpAuthOptions(); + $options['body'] = $source; + // set upload timeout for users with slow connections or large files + $options['timeout'] = $this->timeout; + $options['verify'] = $this->verify; + $this->httpClientService ->newClient() - ->put($this->createBaseUri() . $this->encodePath($target), [ - 'body' => $source, - 'headers' => $headers, - 'auth' => $auth, - // set upload timeout for users with slow connections or large files - 'timeout' => $this->timeout, - 'verify' => $this->verify, - ]); + ->put($this->createBaseUri() . $this->encodePath($target), $options); }); $this->removeCachedFile($target); diff --git a/tests/lib/Files/Storage/DAVTest.php b/tests/lib/Files/Storage/DAVTest.php new file mode 100644 index 0000000000000..d7774f768b9f6 --- /dev/null +++ b/tests/lib/Files/Storage/DAVTest.php @@ -0,0 +1,87 @@ +getHttpAuthOptions(); + } +} + +class DAVTest extends TestCase { + /** + * @return array, headers?: array}}> + */ + public static function httpAuthOptionsProvider(): array { + return [ + 'no auth type' => [ + null, + [ + 'auth' => ['user', 'password'], + ], + ], + 'basic auth' => [ + SabreClient::AUTH_BASIC, + [ + 'auth' => ['user', 'password'], + ], + ], + 'digest auth' => [ + SabreClient::AUTH_DIGEST, + [ + 'auth' => ['user', 'password', 'digest'], + ], + ], + 'bearer auth' => [ + BearerAuthAwareSabreClient::AUTH_BEARER, + [ + 'auth' => [], + 'headers' => [ + 'Authorization' => 'Bearer access-token', + ], + ], + ], + ]; + } + + /** + * @param int|null $authType + * @param array{auth: list, headers?: array} $expected + */ + #[DataProvider('httpAuthOptionsProvider')] + public function testGetHttpAuthOptions(?int $authType, array $expected): void { + $storage = $this->createStorageWithoutConstructor(); + + $this->setProperty($storage, 'authType', $authType); + $this->setProperty($storage, 'user', 'user'); + $this->setProperty($storage, 'password', 'password'); + $this->setProperty($storage, 'bearerToken', 'access-token'); + + $this->assertSame($expected, $storage->getHttpAuthOptionsForTest()); + } + + private function createStorageWithoutConstructor(): DAVTestStorage { + $reflection = new \ReflectionClass(DAVTestStorage::class); + + /** @var DAVTestStorage */ + return $reflection->newInstanceWithoutConstructor(); + } + + private function setProperty(object $object, string $property, mixed $value): void { + $reflection = new \ReflectionProperty(DAV::class, $property); + $reflection->setValue($object, $value); + } +} diff --git a/tests/lib/Http/Client/ClientTest.php b/tests/lib/Http/Client/ClientTest.php index b2cecfdb0fff7..67a828cfa04cb 100644 --- a/tests/lib/Http/Client/ClientTest.php +++ b/tests/lib/Http/Client/ClientTest.php @@ -336,6 +336,26 @@ public function testGetWithOptions(): void { $this->assertEquals(418, $this->client->get('http://localhost/', $options)->getStatusCode()); } + public function testGetPreservesDigestAuthenticationOptions(): void { + $this->setUpDefaultRequestOptions(); + + $options = array_merge($this->defaultRequestOptions, [ + 'auth' => ['user', 'password', 'digest'], + ]); + + $this->guzzleClient + ->expects($this->once()) + ->method('request') + ->with('get', 'http://localhost/', $options) + ->willReturn(new Response(200)); + + $response = $this->client->get('http://localhost/', [ + 'auth' => ['user', 'password', 'digest'], + ]); + + $this->assertSame(200, $response->getStatusCode()); + } + public function testPost(): void { $this->setUpDefaultRequestOptions(); @@ -388,6 +408,32 @@ public function testPutWithOptions(): void { $this->assertEquals(418, $this->client->put('http://localhost/', $options)->getStatusCode()); } + public function testPutPreservesDigestAuthenticationOptions(): void { + $this->setUpDefaultRequestOptions(); + + $body = fopen('php://memory', 'r'); + self::assertIsResource($body); + + $options = array_merge($this->defaultRequestOptions, [ + 'auth' => ['user', 'password', 'digest'], + 'body' => $body, + ]); + + $this->guzzleClient + ->expects($this->once()) + ->method('request') + ->with('put', 'http://localhost/', $options) + ->willReturn(new Response(201)); + + $response = $this->client->put('http://localhost/', [ + 'auth' => ['user', 'password', 'digest'], + 'body' => $body, + ]); + + $this->assertSame(201, $response->getStatusCode()); + fclose($body); + } + public function testDelete(): void { $this->setUpDefaultRequestOptions();