mirrors/wallabag
1
0
Fork 0
mirror of https://github.com/wallabag/wallabag.git synced 2025-03-13 22:52:39 +00:00
Code Issues Projects Releases Wiki Activity

Merge 4651307712 into a4a6eb580b

Browse source
This commit is contained in:
Yassine Guedidi 2025-03-12 23:19:51 +00:00 committed by GitHub
commit 8c398070f6
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
15 changed files with 218 additions and 40 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
app/config/security.yml
View file

@ -74,6 +74,5 @@ security:
- { path: /(unread|starred|archive|annotated).xml$, roles: IS_AUTHENTICATED_ANONYMOUSLY } # For backwards compatibility
- { path: ^/share, roles: IS_AUTHENTICATED_ANONYMOUSLY }
- { path: ^/settings, roles: ROLE_SUPER_ADMIN }
- { path: ^/annotations, roles: ROLE_USER }
- { path: ^/2fa, role: IS_AUTHENTICATED_2FA_IN_PROGRESS }
- { path: ^/, roles: ROLE_USER }

2
phpstan-baseline.neon
View file

@ -6,7 +6,7 @@ parameters:
path: src/Controller/AnnotationController.php
-
message: "#^Method Wallabag\\\\Controller\\\\AnnotationController\\:\\:putAnnotationAction\\(\\) should return Symfony\\\\Component\\\\HttpFoundation\\\\JsonResponse but returns Symfony\\\\Component\\\\Form\\\\FormInterface\\<null\\>\\.$#"
message: "#^Method Wallabag\\\\Controller\\\\AnnotationController\\:\\:putAnnotationAction\\(\\) should return Symfony\\\\Component\\\\HttpFoundation\\\\JsonResponse but returns Symfony\\\\Component\\\\Form\\\\FormInterface\\<mixed\\>\\.$#"
count: 1
path: src/Controller/AnnotationController.php

24
src/Controller/AnnotationController.php
View file

@ -5,6 +5,7 @@ namespace Wallabag\Controller;
use Doctrine\ORM\EntityManagerInterface;
use FOS\RestBundle\Controller\AbstractFOSRestController;
use JMS\Serializer\SerializerInterface;
use Sensio\Bundle\FrameworkExtraBundle\Configuration\IsGranted;
use Symfony\Component\Form\FormFactoryInterface;
use Symfony\Component\HttpFoundation\JsonResponse;
use Symfony\Component\HttpFoundation\Request;
@ -36,6 +37,7 @@ class AnnotationController extends AbstractFOSRestController
* @see Api\WallabagRestController
*
* @Route("/annotations/{entry}.{_format}", name="annotations_get_annotations", methods={"GET"}, defaults={"_format": "json"})
* @IsGranted("LIST_ANNOTATIONS", subject="entry")
*
* @return JsonResponse
*/
@ -57,6 +59,7 @@ class AnnotationController extends AbstractFOSRestController
* @see Api\WallabagRestController
*
* @Route("/annotations/{entry}.{_format}", name="annotations_post_annotation", methods={"POST"}, defaults={"_format": "json"})
* @IsGranted("CREATE_ANNOTATIONS", subject="entry")
*
* @return JsonResponse
*/
@ -91,14 +94,13 @@ class AnnotationController extends AbstractFOSRestController
* @see Api\WallabagRestController
*
* @Route("/annotations/{annotation}.{_format}", name="annotations_put_annotation", methods={"PUT"}, defaults={"_format": "json"})
* @IsGranted("EDIT", subject="annotation")
*
* @return JsonResponse
*/
public function putAnnotationAction(Request $request, AnnotationRepository $annotationRepository, int $annotation)
public function putAnnotationAction(Request $request, Annotation $annotation)
{
try {
$annotation = $this->validateAnnotation($annotationRepository, $annotation, $this->getUser()->getId());
$data = json_decode($request->getContent(), true, 512, \JSON_THROW_ON_ERROR);
$form = $this->formFactory->createNamed('', EditAnnotationType::class, $annotation, [
@ -128,14 +130,13 @@ class AnnotationController extends AbstractFOSRestController
* @see Api\WallabagRestController
*
* @Route("/annotations/{annotation}.{_format}", name="annotations_delete_annotation", methods={"DELETE"}, defaults={"_format": "json"})
* @IsGranted("DELETE", subject="annotation")
*
* @return JsonResponse
*/
public function deleteAnnotationAction(AnnotationRepository $annotationRepository, int $annotation)
public function deleteAnnotationAction(Annotation $annotation)
{
try {
$annotation = $this->validateAnnotation($annotationRepository, $annotation, $this->getUser()->getId());
$this->entityManager->remove($annotation);
$this->entityManager->flush();
@ -157,15 +158,4 @@ class AnnotationController extends AbstractFOSRestController
return $user;
}
private function validateAnnotation(AnnotationRepository $annotationRepository, int $annotationId, int $userId)
{
$annotation = $annotationRepository->findOneByIdAndUserId($annotationId, $userId);
if (null === $annotation) {
throw new NotFoundHttpException();
}
return $annotation;
}
}

29
src/Event/Subscriber/AccessDeniedToNotFoundSubscriber.php Normal file
View file

@ -0,0 +1,29 @@
<?php
namespace Wallabag\Event\Subscriber;
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
use Symfony\Component\HttpKernel\Event\ExceptionEvent;
use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException;
use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
use Symfony\Component\HttpKernel\KernelEvents;
class AccessDeniedToNotFoundSubscriber implements EventSubscriberInterface
{
public static function getSubscribedEvents(): array
{
return [
KernelEvents::EXCEPTION => 'onKernelException',
];
}
public function onKernelException(ExceptionEvent $event): void
{
$exception = $event->getThrowable();
if ($exception instanceof AccessDeniedHttpException) {
$notFoundException = new NotFoundHttpException('', $exception);
$event->setThrowable($notFoundException);
}
}
}

46
src/Security/Voter/AnnotationVoter.php Normal file
View file

@ -0,0 +1,46 @@
<?php
namespace Wallabag\Security\Voter;
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
use Symfony\Component\Security\Core\Authorization\Voter\Voter;
use Wallabag\Entity\Annotation;
use Wallabag\Entity\User;
class AnnotationVoter extends Voter
{
public const EDIT = 'EDIT';
public const DELETE = 'DELETE';
protected function supports(string $attribute, $subject): bool
{
if (!$subject instanceof Annotation) {
return false;
}
if (!\in_array($attribute, [self::EDIT, self::DELETE], true)) {
return false;
}
return true;
}
protected function voteOnAttribute(string $attribute, $subject, TokenInterface $token): bool
{
\assert($subject instanceof Annotation);
$user = $token->getUser();
if (!$user instanceof User) {
return false;
}
switch ($attribute) {
case self::EDIT:
case self::DELETE:
return $subject->getUser() === $user;
}
return false;
}
}

6
src/Security/Voter/EntryVoter.php
View file

@ -17,6 +17,8 @@ class EntryVoter extends Voter
public const SHARE = 'SHARE';
public const UNSHARE = 'UNSHARE';
public const DELETE = 'DELETE';
public const LIST_ANNOTATIONS = 'LIST_ANNOTATIONS';
public const CREATE_ANNOTATIONS = 'CREATE_ANNOTATIONS';
protected function supports(string $attribute, $subject): bool
{
@ -24,7 +26,7 @@ class EntryVoter extends Voter
return false;
}
if (!\in_array($attribute, [self::VIEW, self::EDIT, self::RELOAD, self::STAR, self::ARCHIVE, self::SHARE, self::UNSHARE, self::DELETE], true)) {
if (!\in_array($attribute, [self::VIEW, self::EDIT, self::RELOAD, self::STAR, self::ARCHIVE, self::SHARE, self::UNSHARE, self::DELETE, self::LIST_ANNOTATIONS, self::CREATE_ANNOTATIONS], true)) {
return false;
}
@ -50,6 +52,8 @@ class EntryVoter extends Voter
case self::SHARE:
case self::UNSHARE:
case self::DELETE:
case self::LIST_ANNOTATIONS:
case self::CREATE_ANNOTATIONS:
return $user === $subject->getUser();
}

5
tests/Controller/AnnotationControllerTest.php
View file

@ -82,10 +82,7 @@ class AnnotationControllerTest extends WallabagTestCase
}
$this->client->request('GET', $prefixUrl . '/' . $entry->getId() . '.json');
$this->assertSame(200, $this->client->getResponse()->getStatusCode());
$content = json_decode($this->client->getResponse()->getContent(), true);
$this->assertGreaterThanOrEqual(0, $content['total']);
$this->assertSame(404, $this->client->getResponse()->getStatusCode());
}
/**

2
tests/Controller/Api/DeveloperControllerTest.php
View file

@ -105,7 +105,7 @@ class DeveloperControllerTest extends WallabagTestCase
$this->logInAs('bob');
$client->request('POST', '/developer/client/delete/' . $adminApiClient->getId());
$this->assertSame(403, $client->getResponse()->getStatusCode());
$this->assertSame(404, $client->getResponse()->getStatusCode());
// Try to remove the admin's client with the good user
$this->logInAs('admin');

6
tests/Controller/Api/EntryRestControllerTest.php
View file

@ -110,7 +110,7 @@ class EntryRestControllerTest extends WallabagApiTestCase
$this->client->request('GET', '/api/entries/' . $entry->getId() . '.json');
$this->assertSame(403, $this->client->getResponse()->getStatusCode());
$this->assertSame(404, $this->client->getResponse()->getStatusCode());
}
public function testGetEntries()
@ -1260,14 +1260,14 @@ class EntryRestControllerTest extends WallabagApiTestCase
{
$this->client->request('GET', '/api/entries/exists?url=');
$this->assertSame(403, $this->client->getResponse()->getStatusCode());
$this->assertSame(404, $this->client->getResponse()->getStatusCode());
}
public function testGetEntriesExistsWithNoHashedUrl()
{
$this->client->request('GET', '/api/entries/exists?hashed_url=');
$this->assertSame(403, $this->client->getResponse()->getStatusCode());
$this->assertSame(404, $this->client->getResponse()->getStatusCode());
}
public function testReloadEntryErrorWhileFetching()

18
tests/Controller/ConfigControllerTest.php
View file

@ -577,9 +577,9 @@ class ConfigControllerTest extends WallabagTestCase
$crawler = $client->request('GET', '/tagging-rule/delete/' . $rule->getId());
$this->assertSame(403, $client->getResponse()->getStatusCode());
$this->assertSame(404, $client->getResponse()->getStatusCode());
$this->assertGreaterThan(1, $body = $crawler->filter('body')->extract(['_text']));
$this->assertStringContainsString('You can not access this rule', $body[0]);
$this->assertStringContainsString('404: Not Found', $body[0]);
}
public function testEditingTaggingRuleFromAnOtherUser()
@ -593,9 +593,9 @@ class ConfigControllerTest extends WallabagTestCase
$crawler = $client->request('GET', '/tagging-rule/edit/' . $rule->getId());
$this->assertSame(403, $client->getResponse()->getStatusCode());
$this->assertSame(404, $client->getResponse()->getStatusCode());
$this->assertGreaterThan(1, $body = $crawler->filter('body')->extract(['_text']));
$this->assertStringContainsString('You can not access this rule', $body[0]);
$this->assertStringContainsString('404: Not Found', $body[0]);
}
public function testIgnoreOriginRuleCreation()
@ -714,9 +714,9 @@ class ConfigControllerTest extends WallabagTestCase
$crawler = $client->request('GET', '/ignore-origin-user-rule/edit/' . $rule->getId());
$this->assertSame(403, $client->getResponse()->getStatusCode());
$this->assertSame(404, $client->getResponse()->getStatusCode());
$this->assertGreaterThan(1, $body = $crawler->filter('body')->extract(['_text']));
$this->assertStringContainsString('You can not access this rule', $body[0]);
$this->assertStringContainsString('404: Not Found', $body[0]);
}
public function testEditingIgnoreOriginRuleFromAnOtherUser()
@ -730,9 +730,9 @@ class ConfigControllerTest extends WallabagTestCase
$crawler = $client->request('GET', '/ignore-origin-user-rule/edit/' . $rule->getId());
$this->assertSame(403, $client->getResponse()->getStatusCode());
$this->assertSame(404, $client->getResponse()->getStatusCode());
$this->assertGreaterThan(1, $body = $crawler->filter('body')->extract(['_text']));
$this->assertStringContainsString('You can not access this rule', $body[0]);
$this->assertStringContainsString('404: Not Found', $body[0]);
}
public function testDeleteUserButtonVisibility()
@ -767,7 +767,7 @@ class ConfigControllerTest extends WallabagTestCase
$this->assertStringNotContainsString('config.form_user.delete.button', $body[0]);
$client->request('POST', '/account/delete');
$this->assertSame(403, $client->getResponse()->getStatusCode());
$this->assertSame(404, $client->getResponse()->getStatusCode());
$user = $em
->getRepository(User::class)

2
tests/Controller/EntryControllerTest.php
View file

@ -781,7 +781,7 @@ class EntryControllerTest extends WallabagTestCase
$client->request('GET', '/view/' . $content->getId());
$this->assertSame(403, $client->getResponse()->getStatusCode());
$this->assertSame(404, $client->getResponse()->getStatusCode());
}
public function testFilterOnReadingTime()

2
tests/Controller/SettingsControllerTest.php
View file

@ -27,6 +27,6 @@ class SettingsControllerTest extends WallabagTestCase
$crawler = $client->request('GET', '/settings');
$this->assertSame(403, $client->getResponse()->getStatusCode());
$this->assertSame(404, $client->getResponse()->getStatusCode());
}
}

2
tests/Controller/SiteCredentialControllerTest.php
View file

@ -114,7 +114,7 @@ class SiteCredentialControllerTest extends WallabagTestCase
$client->request('GET', '/site-credentials/' . $credential->getId() . '/edit');
$this->assertSame(403, $client->getResponse()->getStatusCode());
$this->assertSame(404, $client->getResponse()->getStatusCode());
}
public function testDeleteSiteCredential()

85
tests/Security/Voter/AnnotationVoterTest.php Normal file
View file

@ -0,0 +1,85 @@
<?php
namespace Security\Voter;
use PHPUnit\Framework\TestCase;
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface;
use Wallabag\Entity\Annotation;
use Wallabag\Entity\User;
use Wallabag\Security\Voter\AnnotationVoter;
class AnnotationVoterTest extends TestCase
{
private $token;
private $annotationVoter;
protected function setUp(): void
{
$this->token = $this->createMock(TokenInterface::class);
$this->annotationVoter = new AnnotationVoter();
}
public function testVoteReturnsAbstainForInvalidSubject(): void
{
$this->assertSame(VoterInterface::ACCESS_ABSTAIN, $this->annotationVoter->vote($this->token, new \stdClass(), [AnnotationVoter::EDIT]));
}
public function testVoteReturnsAbstainForInvalidAttribute(): void
{
$this->assertSame(VoterInterface::ACCESS_ABSTAIN, $this->annotationVoter->vote($this->token, new Annotation(new User()), ['INVALID']));
}
public function testVoteReturnsDeniedForUnauthenticatedEdit(): void
{
$this->token->method('getUser')->willReturn(null);
$this->assertSame(VoterInterface::ACCESS_DENIED, $this->annotationVoter->vote($this->token, new Annotation(new User()), [AnnotationVoter::EDIT]));
}
public function testVoteReturnsDeniedForOtherUserEdit(): void
{
$currentUser = new User();
$annotationUser = new User();
$this->token->method('getUser')->willReturn($currentUser);
$this->assertSame(VoterInterface::ACCESS_DENIED, $this->annotationVoter->vote($this->token, new Annotation($annotationUser), [AnnotationVoter::EDIT]));
}
public function testVoteReturnsGrantedForAnnotationUserEdit(): void
{
$user = new User();
$this->token->method('getUser')->willReturn($user);
$this->assertSame(VoterInterface::ACCESS_GRANTED, $this->annotationVoter->vote($this->token, new Annotation($user), [AnnotationVoter::EDIT]));
}
public function testVoteReturnsDeniedForUnauthenticatedDelete(): void
{
$this->token->method('getUser')->willReturn(null);
$this->assertSame(VoterInterface::ACCESS_DENIED, $this->annotationVoter->vote($this->token, new Annotation(new User()), [AnnotationVoter::DELETE]));
}
public function testVoteReturnsDeniedForOtherUserDelete(): void
{
$currentUser = new User();
$annotationUser = new User();
$this->token->method('getUser')->willReturn($currentUser);
$this->assertSame(VoterInterface::ACCESS_DENIED, $this->annotationVoter->vote($this->token, new Annotation($annotationUser), [AnnotationVoter::DELETE]));
}
public function testVoteReturnsGrantedForAnnotationUserDelete(): void
{
$user = new User();
$this->token->method('getUser')->willReturn($user);
$this->assertSame(VoterInterface::ACCESS_GRANTED, $this->annotationVoter->vote($this->token, new Annotation($user), [AnnotationVoter::DELETE]));
}
}

28
tests/Security/Voter/EntryVoterTest.php
View file

@ -146,4 +146,32 @@ class EntryVoterTest extends TestCase
$this->assertSame(VoterInterface::ACCESS_GRANTED, $this->entryVoter->vote($this->token, $this->entry, [EntryVoter::DELETE]));
}
public function testVoteReturnsDeniedForNonEntryUserListAnnotations(): void
{
$this->token->method('getUser')->willReturn(new User());
$this->assertSame(VoterInterface::ACCESS_DENIED, $this->entryVoter->vote($this->token, $this->entry, [EntryVoter::LIST_ANNOTATIONS]));
}
public function testVoteReturnsGrantedForEntryUserListAnnotations(): void
{
$this->token->method('getUser')->willReturn($this->user);
$this->assertSame(VoterInterface::ACCESS_GRANTED, $this->entryVoter->vote($this->token, $this->entry, [EntryVoter::LIST_ANNOTATIONS]));
}
public function testVoteReturnsDeniedForNonEntryUserCreateAnnotations(): void
{
$this->token->method('getUser')->willReturn(new User());
$this->assertSame(VoterInterface::ACCESS_DENIED, $this->entryVoter->vote($this->token, $this->entry, [EntryVoter::CREATE_ANNOTATIONS]));
}
public function testVoteReturnsGrantedForEntryUserCreateAnnotations(): void
{
$this->token->method('getUser')->willReturn($this->user);
$this->assertSame(VoterInterface::ACCESS_GRANTED, $this->entryVoter->vote($this->token, $this->entry, [EntryVoter::CREATE_ANNOTATIONS]));
}
}