Apply new CS rules on the webdav feature

This commit is contained in:
Julien Fastré 2024-01-15 20:38:03 +01:00
parent fca929f56f
commit 4cff706306
Signed by: julienfastre
GPG Key ID: BDE2190974723FCB
14 changed files with 104 additions and 108 deletions

View File

@ -19,6 +19,7 @@ use Symfony\Component\Security\Core\Authentication\Token\AbstractToken;
/**
* @internal
*
* @coversNothing
*/
class DavTokenAuthenticationEventSubscriberTest extends TestCase

View File

@ -16,7 +16,6 @@ use Chill\DocStoreBundle\Dav\Response\DavResponse;
use Chill\DocStoreBundle\Entity\StoredObject;
use Chill\DocStoreBundle\Security\Authorization\StoredObjectRoleEnum;
use Chill\DocStoreBundle\Service\StoredObjectManagerInterface;
use DateTimeInterface;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException;
@ -42,7 +41,7 @@ final readonly class WebdavController
public function __construct(
private \Twig\Environment $engine,
private StoredObjectManagerInterface $storedObjectManager,
private Security $security,
private Security $security,
) {
$this->requestAnalyzer = new PropfindRequestAnalyzer();
}
@ -73,11 +72,11 @@ final readonly class WebdavController
throw new AccessDeniedHttpException();
}
$response = (new DavResponse(""))
$response = (new DavResponse(''))
->setEtag($this->storedObjectManager->etag($storedObject))
;
//$response->headers->add(['Allow' => 'OPTIONS,GET,HEAD,DELETE,PROPFIND,PUT,PROPPATCH,COPY,MOVE,REPORT,PATCH,POST,TRACE']);
// $response->headers->add(['Allow' => 'OPTIONS,GET,HEAD,DELETE,PROPFIND,PUT,PROPPATCH,COPY,MOVE,REPORT,PATCH,POST,TRACE']);
$response->headers->add(['Allow' => 'OPTIONS,GET,HEAD,DELETE,PROPFIND,PUT']);
return $response;
@ -94,8 +93,8 @@ final readonly class WebdavController
$depth = $request->headers->get('depth');
if ("0" !== $depth && "1" !== $depth) {
throw new BadRequestHttpException("only 1 and 0 are accepted for Depth header");
if ('0' !== $depth && '1' !== $depth) {
throw new BadRequestHttpException('only 1 and 0 are accepted for Depth header');
}
[$properties, $lastModified, $etag, $length] = $this->parseDavRequest($request->getContent(), $storedObject);
@ -104,7 +103,7 @@ final readonly class WebdavController
$this->engine->render('@ChillDocStore/Webdav/directory_propfind.xml.twig', [
'stored_object' => $storedObject,
'properties' => $properties,
'last_modified' => $lastModified ,
'last_modified' => $lastModified,
'etag' => $etag,
'content_length' => $length,
'depth' => (int) $depth,
@ -114,7 +113,7 @@ final readonly class WebdavController
);
$response->headers->add([
'Content-Type' => 'text/xml'
'Content-Type' => 'text/xml',
]);
return $response;
@ -142,13 +141,13 @@ final readonly class WebdavController
throw new AccessDeniedHttpException();
}
$response = new DavResponse("");
$response = new DavResponse('');
$response->headers->add(
[
'Content-Length' => $this->storedObjectManager->getContentLength($storedObject),
'Content-Type' => $storedObject->getType(),
'Etag' => $this->storedObjectManager->etag($storedObject)
'Etag' => $this->storedObjectManager->etag($storedObject),
]
);
@ -164,7 +163,7 @@ final readonly class WebdavController
throw new AccessDeniedHttpException();
}
$response = (new DavResponse(""))
$response = (new DavResponse(''))
->setEtag($this->storedObjectManager->etag($storedObject))
;
@ -201,7 +200,7 @@ final readonly class WebdavController
$response
->headers->add([
'Content-Type' => 'text/xml'
'Content-Type' => 'text/xml',
]);
return $response;
@ -218,11 +217,11 @@ final readonly class WebdavController
$this->storedObjectManager->write($storedObject, $request->getContent());
return (new DavResponse("", Response::HTTP_NO_CONTENT));
return new DavResponse('', Response::HTTP_NO_CONTENT);
}
/**
* @return array{0: array, 1: DateTimeInterface, 2: string, 3: int} properties, lastModified, etag, length
* @return array{0: array, 1: \DateTimeInterface, 2: string, 3: int} properties, lastModified, etag, length
*/
private function parseDavRequest(string $content, StoredObject $storedObject): array
{
@ -247,7 +246,7 @@ final readonly class WebdavController
$properties,
$lastModified ?? null,
$etag ?? null,
$length ?? null
$length ?? null,
];
}
}

View File

@ -11,4 +11,6 @@ declare(strict_types=1);
namespace Chill\DocStoreBundle\Dav\Exception;
class ParseRequestException extends \UnexpectedValueException {}
class ParseRequestException extends \UnexpectedValueException
{
}

View File

@ -36,17 +36,17 @@ class PropfindRequestAnalyzer
$propfinds = $request->getElementsByTagNameNS('DAV:', 'propfind');
if (0 === $propfinds->count()) {
throw new ParseRequestException("any propfind element found");
throw new ParseRequestException('any propfind element found');
}
if (1 < $propfinds->count()) {
throw new ParseRequestException("too much propfind element found");
throw new ParseRequestException('too much propfind element found');
}
$propfind = $propfinds->item(0);
if (0 === $propfind->childNodes->count()) {
throw new ParseRequestException("no element under propfind");
throw new ParseRequestException('no element under propfind');
}
$unknows = [];
@ -79,7 +79,6 @@ class PropfindRequestAnalyzer
default => '',
};
}
}
$props = array_filter(array_values($props), fn (string $item) => '' !== $item);
@ -98,7 +97,7 @@ class PropfindRequestAnalyzer
self::KNOWN_PROPS,
array_fill(0, count(self::KNOWN_PROPS), $default)
),
'unknowns' => []
'unknowns' => [],
];
}
}

View File

@ -34,8 +34,7 @@ class StoredObjectVoter extends Voter
/** @var StoredObject $subject */
if (
!$token->hasAttribute(DavTokenAuthenticationEventSubscriber::STORED_OBJECT)
||
$subject->getUuid()->toString() !== $token->getAttribute(DavTokenAuthenticationEventSubscriber::STORED_OBJECT)
|| $subject->getUuid()->toString() !== $token->getAttribute(DavTokenAuthenticationEventSubscriber::STORED_OBJECT)
) {
return false;
}
@ -49,9 +48,8 @@ class StoredObjectVoter extends Voter
$token->getAttribute(DavTokenAuthenticationEventSubscriber::ACTIONS);
return match ($askedRole) {
StoredObjectRoleEnum::SEE =>
$tokenRoleAuthorization === StoredObjectRoleEnum::EDIT || $tokenRoleAuthorization === StoredObjectRoleEnum::SEE,
StoredObjectRoleEnum::EDIT => $tokenRoleAuthorization === StoredObjectRoleEnum::EDIT
StoredObjectRoleEnum::SEE => StoredObjectRoleEnum::EDIT === $tokenRoleAuthorization || StoredObjectRoleEnum::SEE === $tokenRoleAuthorization,
StoredObjectRoleEnum::EDIT => StoredObjectRoleEnum::EDIT === $tokenRoleAuthorization
};
}
}

View File

@ -27,9 +27,10 @@ final readonly class DavOnUrlTokenExtractor implements TokenExtractorInterface
{
public function __construct(
private LoggerInterface $logger,
) {}
) {
}
public function extract(Request $request): string|false
public function extract(Request $request): false|string
{
$uri = $request->getRequestUri();
@ -41,13 +42,13 @@ final readonly class DavOnUrlTokenExtractor implements TokenExtractorInterface
);
if (2 > count($segments)) {
$this->logger->info("not enough segment for parsing URL");
$this->logger->info('not enough segment for parsing URL');
return false;
}
if ('dav' !== $segments[0]) {
$this->logger->info("the first segment of the url must be DAV");
$this->logger->info('the first segment of the url must be DAV');
return false;
}

View File

@ -43,11 +43,9 @@ class DavTokenAuthenticationEventSubscriber implements EventSubscriberInterface
$token->setAttribute(self::ACTIONS, match ($payload['e']) {
0 => StoredObjectRoleEnum::SEE,
1 => StoredObjectRoleEnum::EDIT,
default => throw new \UnexpectedValueException("unsupported value for e parameter")
default => throw new \UnexpectedValueException('unsupported value for e parameter')
});
$token->setAttribute(self::STORED_OBJECT, $payload['so']);
}
}

View File

@ -17,14 +17,15 @@ use Lexik\Bundle\JWTAuthenticationBundle\Services\JWTTokenManagerInterface;
use Symfony\Component\Security\Core\Security;
/**
* Provide a JWT Token which will be valid for viewing or editing a document
* Provide a JWT Token which will be valid for viewing or editing a document.
*/
final readonly class JWTDavTokenProvider implements JWTDavTokenProviderInterface
{
public function __construct(
private JWTTokenManagerInterface $JWTTokenManager,
private Security $security,
) {}
) {
}
public function createToken(StoredObject $storedObject, StoredObjectRoleEnum $roleEnum): string
{
@ -44,5 +45,4 @@ final readonly class JWTDavTokenProvider implements JWTDavTokenProviderInterface
return \DateTimeImmutable::createFromFormat('U', (string) $jwt['exp']);
}
}

View File

@ -15,7 +15,7 @@ use Chill\DocStoreBundle\Entity\StoredObject;
use Chill\DocStoreBundle\Security\Authorization\StoredObjectRoleEnum;
/**
* Provide a JWT Token which will be valid for viewing or editing a document
* Provide a JWT Token which will be valid for viewing or editing a document.
*/
interface JWTDavTokenProviderInterface
{

View File

@ -15,7 +15,6 @@ use ChampsLibres\WopiLib\Contract\Service\Discovery\DiscoveryInterface;
use Chill\DocStoreBundle\Entity\StoredObject;
use Chill\DocStoreBundle\Security\Authorization\StoredObjectRoleEnum;
use Chill\DocStoreBundle\Security\Guard\JWTDavTokenProviderInterface;
use Namshi\JOSE\JWS;
use Symfony\Component\Routing\Generator\UrlGeneratorInterface;
use Symfony\Component\Serializer\Normalizer\AbstractNormalizer;
use Symfony\Component\Serializer\Normalizer\NormalizerInterface;
@ -129,7 +128,8 @@ final readonly class WopiEditTwigExtensionRuntime implements RuntimeExtensionInt
private NormalizerInterface $normalizer,
private JWTDavTokenProviderInterface $davTokenProvider,
private UrlGeneratorInterface $urlGenerator,
) {}
) {
}
/**
* return true if the document is editable.
@ -149,7 +149,7 @@ final readonly class WopiEditTwigExtensionRuntime implements RuntimeExtensionInt
* @throws \Twig\Error\RuntimeError
* @throws \Twig\Error\SyntaxError
*/
public function renderButtonGroup(Environment $environment, StoredObject $document, ?string $title = null, bool $canEdit = true, array $options = []): string
public function renderButtonGroup(Environment $environment, StoredObject $document, string $title = null, bool $canEdit = true, array $options = []): string
{
$accessToken = $this->davTokenProvider->createToken(
$document,
@ -174,7 +174,7 @@ final readonly class WopiEditTwigExtensionRuntime implements RuntimeExtensionInt
]);
}
public function renderEditButton(Environment $environment, StoredObject $document, ?array $options = null): string
public function renderEditButton(Environment $environment, StoredObject $document, array $options = null): string
{
return $environment->render(self::TEMPLATE, [
'document' => $document,

View File

@ -14,18 +14,16 @@ namespace Chill\DocStoreBundle\Tests\Controller;
use Chill\DocStoreBundle\Controller\WebdavController;
use Chill\DocStoreBundle\Entity\StoredObject;
use Chill\DocStoreBundle\Service\StoredObjectManagerInterface;
use DateTimeInterface;
use PHPUnit\Framework\TestCase;
use Prophecy\Argument;
use Prophecy\PhpUnit\ProphecyTrait;
use Ramsey\Uuid\Uuid;
use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\Security\Core\Security;
use Symfony\Component\Templating\EngineInterface;
/**
* @internal
*
* @coversNothing
*/
class WebdavControllerTest extends KernelTestCase
@ -124,7 +122,7 @@ class WebdavControllerTest extends KernelTestCase
self::assertEquals($expectedStatusCode, $response->getStatusCode());
self::assertContains('content-type', $response->headers->keys());
self::assertStringContainsString('text/xml', $response->headers->get('content-type'));
self::assertTrue((new \DOMDocument())->loadXML($response->getContent()), $message . " test that the xml response is a valid xml");
self::assertTrue((new \DOMDocument())->loadXML($response->getContent()), $message.' test that the xml response is a valid xml');
self::assertXmlStringEqualsXmlString($expectedXmlResponse, $response->getContent(), $message);
}
@ -137,13 +135,13 @@ class WebdavControllerTest extends KernelTestCase
$request = new Request([], [], [], [], [], [], $requestContent);
$request->setMethod('PROPFIND');
$request->headers->add(["Depth" => "0"]);
$request->headers->add(['Depth' => '0']);
$response = $controller->propfindDirectory($this->buildDocument(), '1234', $request);
self::assertEquals($expectedStatusCode, $response->getStatusCode());
self::assertContains('content-type', $response->headers->keys());
self::assertStringContainsString('text/xml', $response->headers->get('content-type'));
self::assertTrue((new \DOMDocument())->loadXML($response->getContent()), $message . " test that the xml response is a valid xml");
self::assertTrue((new \DOMDocument())->loadXML($response->getContent()), $message.' test that the xml response is a valid xml');
self::assertXmlStringEqualsXmlString($expectedXmlResponse, $response->getContent(), $message);
}
@ -192,7 +190,7 @@ class WebdavControllerTest extends KernelTestCase
</d:multistatus>
XML;
yield [$content, 207, $response, "get IsReadOnly and contenttype from server"];
yield [$content, 207, $response, 'get IsReadOnly and contenttype from server'];
$content =
<<<'XML'
@ -220,7 +218,7 @@ class WebdavControllerTest extends KernelTestCase
</d:multistatus>
XML;
yield [$content, 207, $response, "get property IsReadOnly"];
yield [$content, 207, $response, 'get property IsReadOnly'];
yield [
<<<'XML'
@ -246,7 +244,7 @@ class WebdavControllerTest extends KernelTestCase
</d:response>
</d:multistatus>
XML,
"Test requesting an unknow property"
'Test requesting an unknow property',
];
yield [
@ -274,7 +272,7 @@ class WebdavControllerTest extends KernelTestCase
</d:response>
</d:multistatus>
XML,
"test getting the last modified date"
'test getting the last modified date',
];
yield [
@ -311,7 +309,7 @@ class WebdavControllerTest extends KernelTestCase
</d:response>
</d:multistatus>
XML,
"test finding all properties"
'test finding all properties',
];
}
@ -356,7 +354,7 @@ class WebdavControllerTest extends KernelTestCase
</d:response>
</d:multistatus>
XML,
"test resourceType and IsReadOnly "
'test resourceType and IsReadOnly ',
];
yield [
@ -379,15 +377,14 @@ class WebdavControllerTest extends KernelTestCase
</d:response>
</d:multistatus>
XML,
"test creatableContentsInfo"
'test creatableContentsInfo',
];
}
}
class MockedStoredObjectManager implements StoredObjectManagerInterface
{
public function getLastModified(StoredObject $document): DateTimeInterface
public function getLastModified(StoredObject $document): \DateTimeInterface
{
return new \DateTimeImmutable('2023-09-13T14:15');
}
@ -402,11 +399,12 @@ class MockedStoredObjectManager implements StoredObjectManagerInterface
return 'abcde';
}
public function write(StoredObject $document, string $clearContent): void {}
public function write(StoredObject $document, string $clearContent): void
{
}
public function etag(StoredObject $document): string
{
return 'ab56b4d92b40713acc5af89985d4b786';
}
}

View File

@ -12,11 +12,11 @@ declare(strict_types=1);
namespace Chill\DocStoreBundle\Tests\Dav\Request;
use Chill\DocStoreBundle\Dav\Request\PropfindRequestAnalyzer;
use phpseclib3\Crypt\DSA\Formats\Keys\XML;
use PHPUnit\Framework\TestCase;
/**
* @internal
*
* @coversNothing
*/
class PropfindRequestAnalyzerTest extends TestCase
@ -33,7 +33,7 @@ class PropfindRequestAnalyzerTest extends TestCase
$actual = $analyzer->getRequestedProperties($request);
foreach ($expected as $key => $value) {
if ($key === 'unknowns') {
if ('unknowns' === $key) {
continue;
}
@ -59,17 +59,17 @@ class PropfindRequestAnalyzerTest extends TestCase
</propfind>
XML,
[
"resourceType" => false,
"contentType" => false,
"lastModified" => false,
"creationDate" => false,
"contentLength" => false,
"etag" => false,
"supportedLock" => false,
'resourceType' => false,
'contentType' => false,
'lastModified' => false,
'creationDate' => false,
'contentLength' => false,
'etag' => false,
'supportedLock' => false,
'unknowns' => [
['xmlns' => 'http://ucb.openoffice.org/dav/props/', 'prop' => 'BaseURI']
]
]
['xmlns' => 'http://ucb.openoffice.org/dav/props/', 'prop' => 'BaseURI'],
],
],
];
yield [
@ -80,15 +80,15 @@ class PropfindRequestAnalyzerTest extends TestCase
</propfind>
XML,
[
"resourceType" => true,
"contentType" => true,
"lastModified" => true,
"creationDate" => true,
"contentLength" => true,
"etag" => true,
"supportedLock" => true,
"unknowns" => [],
]
'resourceType' => true,
'contentType' => true,
'lastModified' => true,
'creationDate' => true,
'contentLength' => true,
'etag' => true,
'supportedLock' => true,
'unknowns' => [],
],
];
yield [
@ -101,15 +101,15 @@ class PropfindRequestAnalyzerTest extends TestCase
</propfind>
XML,
[
"resourceType" => false,
"contentType" => false,
"lastModified" => true,
"creationDate" => false,
"contentLength" => false,
"etag" => false,
"supportedLock" => false,
'unknowns' => []
]
'resourceType' => false,
'contentType' => false,
'lastModified' => true,
'creationDate' => false,
'contentLength' => false,
'etag' => false,
'supportedLock' => false,
'unknowns' => [],
],
];
yield [
@ -118,17 +118,17 @@ class PropfindRequestAnalyzerTest extends TestCase
<propfind xmlns="DAV:"><prop><resourcetype xmlns="DAV:"/><IsReadOnly xmlns="http://ucb.openoffice.org/dav/props/"/><getcontenttype xmlns="DAV:"/><supportedlock xmlns="DAV:"/></prop></propfind>
XML,
[
"resourceType" => true,
"contentType" => true,
"lastModified" => false,
"creationDate" => false,
"contentLength" => false,
"etag" => false,
"supportedLock" => false,
'resourceType' => true,
'contentType' => true,
'lastModified' => false,
'creationDate' => false,
'contentLength' => false,
'etag' => false,
'supportedLock' => false,
'unknowns' => [
['xmlns' => 'http://ucb.openoffice.org/dav/props/', 'prop' => 'IsReadOnly']
]
]
['xmlns' => 'http://ucb.openoffice.org/dav/props/', 'prop' => 'IsReadOnly'],
],
],
];
}
}

View File

@ -22,6 +22,7 @@ use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface;
/**
* @internal
*
* @coversNothing
*/
class StoredObjectVoterTest extends TestCase
@ -31,7 +32,7 @@ class StoredObjectVoterTest extends TestCase
/**
* @dataProvider provideDataVote
*/
public function testVote(TokenInterface $token, object|null $subject, string $attribute, mixed $expected): void
public function testVote(TokenInterface $token, null|object $subject, string $attribute, mixed $expected): void
{
$voter = new StoredObjectVoter();
@ -44,28 +45,28 @@ class StoredObjectVoterTest extends TestCase
$this->buildToken(StoredObjectRoleEnum::EDIT, new StoredObject()),
new \stdClass(),
'SOMETHING',
VoterInterface::ACCESS_ABSTAIN
VoterInterface::ACCESS_ABSTAIN,
];
yield [
$this->buildToken(StoredObjectRoleEnum::EDIT, $so = new StoredObject()),
$so,
'SOMETHING',
VoterInterface::ACCESS_ABSTAIN
VoterInterface::ACCESS_ABSTAIN,
];
yield [
$this->buildToken(StoredObjectRoleEnum::EDIT, $so = new StoredObject()),
$so,
StoredObjectRoleEnum::SEE->value,
VoterInterface::ACCESS_GRANTED
VoterInterface::ACCESS_GRANTED,
];
yield [
$this->buildToken(StoredObjectRoleEnum::EDIT, $so = new StoredObject()),
$so,
StoredObjectRoleEnum::EDIT->value,
VoterInterface::ACCESS_GRANTED
VoterInterface::ACCESS_GRANTED,
];
yield [
@ -79,7 +80,7 @@ class StoredObjectVoterTest extends TestCase
$this->buildToken(StoredObjectRoleEnum::SEE, $so = new StoredObject()),
$so,
StoredObjectRoleEnum::SEE->value,
VoterInterface::ACCESS_GRANTED
VoterInterface::ACCESS_GRANTED,
];
yield [
@ -97,8 +98,7 @@ class StoredObjectVoterTest extends TestCase
];
}
private function buildToken(?StoredObjectRoleEnum $storedObjectRoleEnum = null, ?StoredObject $storedObject = null): TokenInterface
private function buildToken(StoredObjectRoleEnum $storedObjectRoleEnum = null, StoredObject $storedObject = null): TokenInterface
{
$token = $this->prophesize(TokenInterface::class);
@ -110,7 +110,6 @@ class StoredObjectVoterTest extends TestCase
$token->getAttribute(DavTokenAuthenticationEventSubscriber::ACTIONS)->willThrow(new \InvalidArgumentException());
}
if (null !== $storedObject) {
$token->hasAttribute(DavTokenAuthenticationEventSubscriber::STORED_OBJECT)->willReturn(true);
$token->getAttribute(DavTokenAuthenticationEventSubscriber::STORED_OBJECT)->willReturn($storedObject->getUuid()->toString());

View File

@ -19,6 +19,7 @@ use Symfony\Component\HttpFoundation\Request;
/**
* @internal
*
* @coversNothing
*/
class DavOnUrlTokenExtractorTest extends TestCase
@ -28,7 +29,7 @@ class DavOnUrlTokenExtractorTest extends TestCase
/**
* @dataProvider provideDataUri
*/
public function testExtract(string $uri, string|false $expected): void
public function testExtract(string $uri, false|string $expected): void
{
$request = $this->prophesize(Request::class);
$request->getRequestUri()->willReturn($uri);