Release 2.57 (#5929)

This commit is contained in:
Kevin Papst
2026-05-21 22:14:10 +02:00
committed by GitHub
parent f0ce1c7bd6
commit 976d38e8a4
101 changed files with 4226 additions and 1119 deletions

View File

@@ -11,10 +11,15 @@ namespace App\Tests\Controller;
use App\Entity\Activity;
use App\Entity\ActivityMeta;
use App\Entity\ActivityRate;
use App\Entity\Project;
use App\Entity\Role;
use App\Entity\RolePermission;
use App\Entity\Timesheet;
use App\Entity\User;
use App\Tests\DataFixtures\ActivityFixtures;
use App\Tests\DataFixtures\CustomerFixtures;
use App\Tests\DataFixtures\ProjectFixtures;
use App\Tests\DataFixtures\TeamFixtures;
use App\Tests\DataFixtures\TimesheetFixtures;
use App\Tests\Mocks\ActivityTestMetaFieldSubscriberMock;
@@ -191,6 +196,54 @@ class ActivityControllerTest extends AbstractControllerBaseTestCase
self::assertStringContainsString('123.45', $node->text(null, true));
}
public function testEditRateActionDeniesForeignRate(): void
{
$client = $this->getClientForAuthenticatedUser(User::ROLE_ADMIN);
$project = $this->getEntityManager()->getRepository(Project::class)->find(1);
self::assertInstanceOf(Project::class, $project);
$activity = $this->importFixture((new ActivityFixtures(1))->setProjects([$project]))[0];
$rate = new ActivityRate();
$rate->setActivity($activity);
$rate->setRate(123.45);
$em = $this->getEntityManager();
$em->persist($rate);
$em->flush();
$this->request($client, '/admin/activity/1/rate/' . $rate->getId());
$this->assertAccessDenied($client);
}
public function testCreateWithProjectActionDeniesUserWithoutEditProjectPermission(): void
{
$client = $this->getClientForAuthenticatedUser(User::ROLE_USER);
$user = $this->getUserByRole(User::ROLE_USER);
$customer = $this->importFixture(new CustomerFixtures(1))[0];
$project = $this->importFixture((new ProjectFixtures(1))->setCustomers([$customer]))[0];
$em = $this->getEntityManager();
$role = (new Role())->setName('TEST_CREATE_ACTIVITY_ONLY');
$permission = (new RolePermission())->setRole($role)->setPermission('create_activity')->setAllowed(true);
$roleName = $role->getName();
self::assertNotNull($roleName);
$user->addRole($roleName);
$em->persist($role);
$em->persist($permission);
$em->persist($user);
$em->flush();
$this->request($client, '/admin/activity/create/' . $project->getId());
$this->assertAccessDenied($client);
}
public function testCreateAction(): void
{
$client = $this->getClientForAuthenticatedUser(User::ROLE_ADMIN);

View File

@@ -10,8 +10,8 @@
namespace App\Tests\Controller;
use App\Entity\Customer;
use App\Entity\CustomerComment;
use App\Entity\CustomerMeta;
use App\Entity\CustomerRate;
use App\Entity\Timesheet;
use App\Entity\User;
use App\Tests\DataFixtures\CustomerFixtures;
@@ -175,6 +175,24 @@ class CustomerControllerTest extends AbstractControllerBaseTestCase
self::assertStringContainsString('123.45', $node->text(null, true));
}
public function testEditRateActionDeniesForeignRate(): void
{
$client = $this->getClientForAuthenticatedUser(User::ROLE_ADMIN);
$customer = $this->importFixture(new CustomerFixtures(1))[0];
$rate = new CustomerRate();
$rate->setCustomer($customer);
$rate->setRate(123.45);
$em = $this->getEntityManager();
$em->persist($rate);
$em->flush();
$this->request($client, '/admin/customer/1/rate/' . $rate->getId());
$this->assertAccessDenied($client);
}
public function testAddCommentAction(): void
{
$client = $this->getClientForAuthenticatedUser(User::ROLE_ADMIN);
@@ -198,78 +216,6 @@ class CustomerControllerTest extends AbstractControllerBaseTestCase
self::assertStringContainsString('<p>A beautiful and short comment <strong>with some</strong> markdown formatting</p>', $node->html());
}
public function testDeleteCommentAction(): void
{
$client = $this->getClientForAuthenticatedUser(User::ROLE_ADMIN);
$this->assertAccessIsGranted($client, '/admin/customer/1/details');
$form = $client->getCrawler()->filter('form[name=customer_comment_form]')->form();
$client->submit($form, [
'customer_comment_form' => [
'message' => 'Blah foo bar',
]
]);
$this->assertIsRedirect($client, $this->createUrl('/admin/customer/1/details'));
$client->followRedirect();
$node = $client->getCrawler()->filter('div.card#comments_box .card-body');
self::assertStringContainsString('Blah foo bar', $node->html());
$node = $client->getCrawler()->filter('div.card#comments_box .card-body a.delete-comment-link');
$this->request($client, $node->attr('href'));
$this->assertIsRedirect($client, $this->createUrl('/admin/customer/1/details'));
$client->followRedirect();
$node = $client->getCrawler()->filter('div.card#comments_box .card-body');
self::assertStringContainsString('There were no comments posted yet', $node->html());
}
public function testDeleteCommentActionWithoutToken(): void
{
$client = $this->getClientForAuthenticatedUser(User::ROLE_ADMIN);
$this->assertAccessIsGranted($client, '/admin/customer/1/details');
$form = $client->getCrawler()->filter('form[name=customer_comment_form]')->form();
$client->submit($form, [
'customer_comment_form' => [
'message' => 'Blah foo bar',
]
]);
$this->assertIsRedirect($client, $this->createUrl('/admin/customer/1/details'));
$client->followRedirect();
$comments = $this->getEntityManager()->getRepository(CustomerComment::class)->findAll();
$id = $comments[0]->getId();
$this->request($client, '/admin/customer/' . $id . '/comment_delete');
$this->assertRouteNotFound($client);
}
public function testPinCommentAction(): void
{
$client = $this->getClientForAuthenticatedUser(User::ROLE_ADMIN);
$this->assertAccessIsGranted($client, '/admin/customer/1/details');
$form = $client->getCrawler()->filter('form[name=customer_comment_form]')->form();
$client->submit($form, [
'customer_comment_form' => [
'message' => 'Blah foo bar',
]
]);
$this->assertIsRedirect($client, $this->createUrl('/admin/customer/1/details'));
$client->followRedirect();
$node = $client->getCrawler()->filter('div.card#comments_box .card-body');
self::assertStringContainsString('Blah foo bar', $node->html());
$node = $client->getCrawler()->filter('div.card#comments_box .card-body a.pin-comment-link.active');
self::assertEquals(0, $node->count());
$node = $client->getCrawler()->filter('div.card#comments_box .card-body a.pin-comment-link');
self::assertEquals(1, $node->count());
$this->request($client, $node->attr('href'));
$this->assertIsRedirect($client, $this->createUrl('/admin/customer/1/details'));
$client->followRedirect();
$node = $client->getCrawler()->filter('div.card#comments_box .card-body a.pin-comment-link.active');
self::assertEquals(1, $node->count());
self::assertStringContainsString('/admin/customer/', $node->attr('href'));
self::assertStringContainsString('/comment_pin/', $node->attr('href'));
}
public function testCreateDefaultTeamAction(): void
{
$client = $this->getClientForAuthenticatedUser(User::ROLE_ADMIN);

View File

@@ -10,7 +10,9 @@
namespace App\Tests\Controller;
use App\Entity\User;
use App\Repository\BookmarkRepository;
use App\Tests\DataFixtures\TimesheetFixtures;
use App\Timesheet\FavoriteRecordService;
use PHPUnit\Framework\Attributes\Group;
#[Group('integration')]
@@ -41,4 +43,119 @@ class FavoriteControllerTest extends AbstractControllerBaseTestCase
self::assertStringContainsString('<a class="api-link text-decoration-none text-body d-block" href="/api/timesheets/', $content);
self::assertStringContainsString('data-event="kimai.timesheetStart kimai.timesheetUpdate" data-method="PATCH" data-msg-error="timesheet', $content);
}
/**
* Regression test for the security issue in FavoriteController::add():
* an unprivileged user must NOT be able to add a favorite for a timesheet
* owned by another user, even if they know a valid timesheet ID.
*/
public function testAddFavoriteForOtherUsersTimesheetIsDenied(): void
{
// attacker is a plain user (ROLE_USER), not the timesheet owner
$client = $this->getClientForAuthenticatedUser(User::ROLE_USER);
$victim = $this->getUserByRole(User::ROLE_TEAMLEAD);
$fixture = new TimesheetFixtures();
$fixture->setAmount(1);
$fixture->setUser($victim);
$timesheets = $this->importFixture($fixture);
$timesheetId = $timesheets[0]->getId();
self::assertNotNull($timesheetId);
$this->request($client, '/favorite/timesheet/add/' . $timesheetId);
$this->assertAccessDenied($client);
// the victim's bookmark must not have been touched
/** @var BookmarkRepository $bookmarkRepository */
$bookmarkRepository = $this->getPrivateService(BookmarkRepository::class);
$this->getEntityManager()->clear();
$bookmark = $bookmarkRepository->findBookmark($this->getUserByRole(User::ROLE_TEAMLEAD), 'favorite', 'recent');
if ($bookmark !== null) {
self::assertNotContains($timesheetId, $bookmark->getContent(), 'attacker must not write to the victim\'s bookmark');
}
}
/**
* Regression test for the security issue in FavoriteController::remove():
* an unprivileged user must NOT be able to remove a favorite from another
* user's bookmark, even if they know a valid timesheet ID.
*/
public function testRemoveFavoriteForOtherUsersTimesheetIsDenied(): void
{
// attacker is a plain user (ROLE_USER), not the timesheet owner
$client = $this->getClientForAuthenticatedUser(User::ROLE_USER);
$victim = $this->getUserByRole(User::ROLE_TEAMLEAD);
$fixture = new TimesheetFixtures();
$fixture->setAmount(1);
$fixture->setUser($victim);
$timesheets = $this->importFixture($fixture);
$timesheet = $timesheets[0];
$timesheetId = $timesheet->getId();
self::assertNotNull($timesheetId);
// legitimately seed the victim's own favorites
/** @var FavoriteRecordService $favoriteRecordService */
$favoriteRecordService = $this->getPrivateService(FavoriteRecordService::class);
$favoriteRecordService->addFavorite($timesheet);
/** @var BookmarkRepository $bookmarkRepository */
$bookmarkRepository = $this->getPrivateService(BookmarkRepository::class);
$this->getEntityManager()->clear();
$bookmark = $bookmarkRepository->findBookmark($this->getUserByRole(User::ROLE_TEAMLEAD), 'favorite', 'recent');
self::assertNotNull($bookmark);
self::assertContains($timesheetId, $bookmark->getContent(), 'precondition: favorite exists for the victim');
// attacker (ROLE_USER) attempts to remove the favorite from the victim's bookmark
$this->request($client, '/favorite/timesheet/remove/' . $timesheetId);
$this->assertAccessDenied($client);
// the victim's favorite must still be there
/** @var BookmarkRepository $bookmarkRepository */
$bookmarkRepository = $this->getPrivateService(BookmarkRepository::class);
$this->getEntityManager()->clear();
$bookmark = $bookmarkRepository->findBookmark($this->getUserByRole(User::ROLE_TEAMLEAD), 'favorite', 'recent');
self::assertNotNull($bookmark);
self::assertContains($timesheetId, $bookmark->getContent(), 'attacker must not remove the victim\'s favorite');
}
/**
* Ensures the added `#[IsGranted('view', 'timesheet')]` voter does not
* break the legitimate use case: a user managing favorites for their own
* timesheet.
*/
public function testAddAndRemoveFavoriteForOwnTimesheetIsAllowed(): void
{
$client = $this->getClientForAuthenticatedUser(User::ROLE_USER);
$owner = $this->getUserByRole(User::ROLE_USER);
$fixture = new TimesheetFixtures();
$fixture->setAmount(1);
$fixture->setUser($owner);
$timesheets = $this->importFixture($fixture);
$timesheetId = $timesheets[0]->getId();
self::assertNotNull($timesheetId);
$this->request($client, '/favorite/timesheet/add/' . $timesheetId);
self::assertTrue($client->getResponse()->isRedirect());
/** @var BookmarkRepository $bookmarkRepository */
$bookmarkRepository = $this->getPrivateService(BookmarkRepository::class);
$this->getEntityManager()->clear();
$bookmark = $bookmarkRepository->findBookmark($this->getUserByRole(User::ROLE_USER), 'favorite', 'recent');
self::assertNotNull($bookmark);
self::assertContains($timesheetId, $bookmark->getContent());
$this->request($client, '/favorite/timesheet/remove/' . $timesheetId);
self::assertTrue($client->getResponse()->isRedirect());
/** @var BookmarkRepository $bookmarkRepository */
$bookmarkRepository = $this->getPrivateService(BookmarkRepository::class);
$this->getEntityManager()->clear();
$bookmark = $bookmarkRepository->findBookmark($this->getUserByRole(User::ROLE_USER), 'favorite', 'recent');
self::assertNotNull($bookmark);
self::assertNotContains($timesheetId, $bookmark->getContent());
}
}

View File

@@ -15,10 +15,13 @@ use App\Entity\ActivityRate;
use App\Entity\Project;
use App\Entity\ProjectMeta;
use App\Entity\ProjectRate;
use App\Entity\Role;
use App\Entity\RolePermission;
use App\Entity\Team;
use App\Entity\Timesheet;
use App\Entity\User;
use App\Tests\DataFixtures\ActivityFixtures;
use App\Tests\DataFixtures\CustomerFixtures;
use App\Tests\DataFixtures\ProjectFixtures;
use App\Tests\DataFixtures\TeamFixtures;
use App\Tests\DataFixtures\TimesheetFixtures;
@@ -212,6 +215,24 @@ class ProjectControllerTest extends AbstractControllerBaseTestCase
$this->assertAddRate($client, 123.45, 1);
}
public function testEditRateActionDeniesForeignRate(): void
{
$client = $this->getClientForAuthenticatedUser(User::ROLE_ADMIN);
$project = $this->importFixture(new ProjectFixtures(1))[0];
$rate = new ProjectRate();
$rate->setProject($project);
$rate->setRate(123.45);
$em = $this->getEntityManager();
$em->persist($rate);
$em->flush();
$this->request($client, '/admin/project/1/rate/' . $rate->getId());
$this->assertAccessDenied($client);
}
public function assertAddRate(HttpKernelBrowser $client, $rate, $projectId): void
{
$this->assertAccessIsGranted($client, '/admin/project/' . $projectId . '/rate');
@@ -310,57 +331,6 @@ class ProjectControllerTest extends AbstractControllerBaseTestCase
self::assertStringContainsString('<p>A beautiful and long comment <strong>with some</strong> markdown formatting</p>', $node->html());
}
public function testDeleteCommentAction(): void
{
$client = $this->getClientForAuthenticatedUser(User::ROLE_ADMIN);
$this->assertAccessIsGranted($client, '/admin/project/1/details');
$form = $client->getCrawler()->filter('form[name=project_comment_form]')->form();
$client->submit($form, [
'project_comment_form' => [
'message' => 'Foo bar blub',
]
]);
$this->assertIsRedirect($client, $this->createUrl('/admin/project/1/details'));
$client->followRedirect();
$node = $client->getCrawler()->filter('div.card#comments_box .card-body');
self::assertStringContainsString('Foo bar blub', $node->html());
$node = $client->getCrawler()->filter('div.card#comments_box .card-body a.delete-comment-link');
$this->request($client, $node->attr('href'));
$this->assertIsRedirect($client, $this->createUrl('/admin/project/1/details'));
$client->followRedirect();
$node = $client->getCrawler()->filter('div.card#comments_box .card-body');
self::assertStringContainsString('There were no comments posted yet', $node->html());
}
public function testPinCommentAction(): void
{
$client = $this->getClientForAuthenticatedUser(User::ROLE_ADMIN);
$this->assertAccessIsGranted($client, '/admin/project/1/details');
$form = $client->getCrawler()->filter('form[name=project_comment_form]')->form();
$client->submit($form, [
'project_comment_form' => [
'message' => 'Foo bar blub',
]
]);
$this->assertIsRedirect($client, $this->createUrl('/admin/project/1/details'));
$client->followRedirect();
$node = $client->getCrawler()->filter('div.card#comments_box .card-body');
self::assertStringContainsString('Foo bar blub', $node->html());
$node = $client->getCrawler()->filter('div.card#comments_box .card-body a.pin-comment-link.active');
self::assertEquals(0, $node->count());
$node = $client->getCrawler()->filter('div.card#comments_box .card-body a.pin-comment-link');
self::assertEquals(1, $node->count());
$this->request($client, $node->attr('href'));
$this->assertIsRedirect($client, $this->createUrl('/admin/project/1/details'));
$client->followRedirect();
$node = $client->getCrawler()->filter('div.card#comments_box .card-body a.pin-comment-link.active');
self::assertEquals(1, $node->count());
self::assertStringContainsString('/admin/project/', $node->attr('href'));
self::assertStringContainsString('/comment_pin/', $node->attr('href'));
}
public function testCreateDefaultTeamAction(): void
{
$client = $this->getClientForAuthenticatedUser(User::ROLE_ADMIN);
@@ -403,6 +373,32 @@ class ProjectControllerTest extends AbstractControllerBaseTestCase
self::assertEquals(5, $node->count());
}
public function testCreateWithCustomerActionDeniesUserWithoutEditCustomerPermission(): void
{
$client = $this->getClientForAuthenticatedUser(User::ROLE_USER);
$user = $this->getUserByRole(User::ROLE_USER);
$customer = $this->importFixture(new CustomerFixtures(1))[0];
$em = $this->getEntityManager();
$role = (new Role())->setName('TEST_CREATE_PROJECT_ONLY');
$permission = (new RolePermission())->setRole($role)->setPermission('create_project')->setAllowed(true);
$roleName = $role->getName();
self::assertNotNull($roleName);
$user->addRole($roleName);
$em->persist($role);
$em->persist($permission);
$em->persist($user);
$em->flush();
$this->request($client, '/admin/project/create/' . $customer->getId());
$this->assertAccessDenied($client);
}
public function testCreateAction(): void
{
$client = $this->getClientForAuthenticatedUser(User::ROLE_ADMIN);