From 75a6bf3ef68a3bac051bcfa3d80704249b724d86 Mon Sep 17 00:00:00 2001 From: Kevin Papst Date: Tue, 19 Jun 2018 22:08:44 +0200 Subject: [PATCH] Updated security expressions #148 (#166) * replace has_role() with is_granted() * added basic integration test classes for main controller * added basic integration test classes for admin controller --- src/Controller/ActivityController.php | 3 +- src/Controller/Admin/ActivityController.php | 2 +- src/Controller/Admin/CustomerController.php | 2 +- src/Controller/Admin/ProjectController.php | 2 +- src/Controller/Admin/TimesheetController.php | 2 +- src/Controller/Admin/UserController.php | 2 +- src/Controller/DashboardController.php | 2 +- src/Controller/HelpController.php | 2 +- src/Controller/InvoiceController.php | 2 +- src/Controller/ProfileController.php | 2 +- src/Controller/SidebarController.php | 2 +- src/Controller/TimesheetController.php | 2 +- tests/Controller/ActivityControllerTest.php | 23 ++++ .../Admin/ActivityControllerTest.php | 34 ++++++ .../Admin/CustomerControllerTest.php | 34 ++++++ .../Admin/ProjectControllerTest.php | 34 ++++++ .../Admin/TimesheetControllerTest.php | 34 ++++++ tests/Controller/Admin/UserControllerTest.php | 34 ++++++ tests/Controller/ControllerBaseTest.php | 107 ++++++++++++++++-- tests/Controller/DashboardControllerTest.php | 31 +++++ tests/Controller/InvoiceControllerTest.php | 35 ++++++ tests/Controller/ProfileControllerTest.php | 40 +++++++ tests/Controller/SidebarControllerTest.php | 23 ++++ tests/Controller/TimesheetControllerTest.php | 33 ++++++ 24 files changed, 465 insertions(+), 22 deletions(-) create mode 100644 tests/Controller/ActivityControllerTest.php create mode 100644 tests/Controller/Admin/ActivityControllerTest.php create mode 100644 tests/Controller/Admin/CustomerControllerTest.php create mode 100644 tests/Controller/Admin/ProjectControllerTest.php create mode 100644 tests/Controller/Admin/TimesheetControllerTest.php create mode 100644 tests/Controller/Admin/UserControllerTest.php create mode 100644 tests/Controller/DashboardControllerTest.php create mode 100644 tests/Controller/InvoiceControllerTest.php create mode 100644 tests/Controller/ProfileControllerTest.php create mode 100644 tests/Controller/SidebarControllerTest.php create mode 100644 tests/Controller/TimesheetControllerTest.php diff --git a/src/Controller/ActivityController.php b/src/Controller/ActivityController.php index 96d66f1f..c779d221 100644 --- a/src/Controller/ActivityController.php +++ b/src/Controller/ActivityController.php @@ -18,8 +18,7 @@ use App\Repository\ActivityRepository; /** * Controller used to manage activity contents in the public part of the site. * - * @Route("/activity") - * @Security("has_role('ROLE_USER')") + * @Security("is_granted('ROLE_USER')") */ class ActivityController extends Controller { diff --git a/src/Controller/Admin/ActivityController.php b/src/Controller/Admin/ActivityController.php index 14afe316..6e86a747 100644 --- a/src/Controller/Admin/ActivityController.php +++ b/src/Controller/Admin/ActivityController.php @@ -25,7 +25,7 @@ use App\Repository\Query\ActivityQuery; * Controller used to manage activities in the admin part of the site. * * @Route("/admin/activity") - * @Security("has_role('ROLE_ADMIN')") + * @Security("is_granted('ROLE_ADMIN')") * @Security("is_granted('IS_AUTHENTICATED_FULLY')") */ class ActivityController extends AbstractController diff --git a/src/Controller/Admin/CustomerController.php b/src/Controller/Admin/CustomerController.php index bf4d5b1b..9e8d15e2 100644 --- a/src/Controller/Admin/CustomerController.php +++ b/src/Controller/Admin/CustomerController.php @@ -24,7 +24,7 @@ use App\Repository\Query\CustomerQuery; * Controller used to manage activities in the admin part of the site. * * @Route("/admin/customer") - * @Security("has_role('ROLE_ADMIN')") + * @Security("is_granted('ROLE_ADMIN')") * @Security("is_granted('IS_AUTHENTICATED_FULLY')") */ class CustomerController extends AbstractController diff --git a/src/Controller/Admin/ProjectController.php b/src/Controller/Admin/ProjectController.php index 7628717d..b467c500 100644 --- a/src/Controller/Admin/ProjectController.php +++ b/src/Controller/Admin/ProjectController.php @@ -26,7 +26,7 @@ use App\Repository\Query\ProjectQuery; * Controller used to manage projects in the admin part of the site. * * @Route("/admin/project") - * @Security("has_role('ROLE_ADMIN')") + * @Security("is_granted('ROLE_ADMIN')") * @Security("is_granted('IS_AUTHENTICATED_FULLY')") */ class ProjectController extends AbstractController diff --git a/src/Controller/Admin/TimesheetController.php b/src/Controller/Admin/TimesheetController.php index f55babd6..dcdb4748 100644 --- a/src/Controller/Admin/TimesheetController.php +++ b/src/Controller/Admin/TimesheetController.php @@ -25,7 +25,7 @@ use Sensio\Bundle\FrameworkExtraBundle\Configuration\Security; * Controller used for manage timesheet entries in the admin part of the site. * * @Route("/team/timesheet") - * @Security("has_role('ROLE_TEAMLEAD')") + * @Security("is_granted('ROLE_TEAMLEAD')") * @Security("is_granted('IS_AUTHENTICATED_FULLY')") */ class TimesheetController extends AbstractController diff --git a/src/Controller/Admin/UserController.php b/src/Controller/Admin/UserController.php index 1e3c8ade..7b4f2aff 100644 --- a/src/Controller/Admin/UserController.php +++ b/src/Controller/Admin/UserController.php @@ -24,7 +24,7 @@ use Symfony\Component\HttpFoundation\Request; * Controller used to manage users in the admin part of the site. * * @Route("/admin/user") - * @Security("has_role('ROLE_SUPER_ADMIN')") + * @Security("is_granted('ROLE_SUPER_ADMIN')") * @Security("is_granted('IS_AUTHENTICATED_FULLY')") */ class UserController extends AbstractController diff --git a/src/Controller/DashboardController.php b/src/Controller/DashboardController.php index 8512dc0e..a679dfdd 100644 --- a/src/Controller/DashboardController.php +++ b/src/Controller/DashboardController.php @@ -24,7 +24,7 @@ use Sensio\Bundle\FrameworkExtraBundle\Configuration\Security; * Dashboard controller for the admin area. * * @Route("/dashboard") - * @Security("has_role('ROLE_USER')") + * @Security("is_granted('ROLE_USER')") */ class DashboardController extends Controller { diff --git a/src/Controller/HelpController.php b/src/Controller/HelpController.php index 691b2e45..a469d940 100644 --- a/src/Controller/HelpController.php +++ b/src/Controller/HelpController.php @@ -19,7 +19,7 @@ use Sensio\Bundle\FrameworkExtraBundle\Configuration\Security; * This controller can render the markdown documentation from /var/docs/ * * @Route("/help") - * @Security("has_role('ROLE_USER')") + * @Security("is_granted('ROLE_USER')") */ class HelpController extends Controller { diff --git a/src/Controller/InvoiceController.php b/src/Controller/InvoiceController.php index ef68f98f..93749f38 100644 --- a/src/Controller/InvoiceController.php +++ b/src/Controller/InvoiceController.php @@ -27,7 +27,7 @@ use Symfony\Component\HttpFoundation\Request; * Controller used to manage invoices. * * @Route("/invoice") - * @Security("has_role('ROLE_TEAMLEAD')") + * @Security("is_granted('ROLE_TEAMLEAD')") */ class InvoiceController extends AbstractController { diff --git a/src/Controller/ProfileController.php b/src/Controller/ProfileController.php index 4b6832a1..79a93ac7 100644 --- a/src/Controller/ProfileController.php +++ b/src/Controller/ProfileController.php @@ -27,7 +27,7 @@ use Symfony\Component\HttpFoundation\Request; * User profile controller * * @Route("/profile") - * @Security("has_role('ROLE_USER')") + * @Security("is_granted('ROLE_USER')") */ class ProfileController extends AbstractController { diff --git a/src/Controller/SidebarController.php b/src/Controller/SidebarController.php index 05f0e0fc..e0dc0b64 100644 --- a/src/Controller/SidebarController.php +++ b/src/Controller/SidebarController.php @@ -15,7 +15,7 @@ use Sensio\Bundle\FrameworkExtraBundle\Configuration\Security; /** * Sidebar controller * - * @Security("has_role('ROLE_USER')") + * @Security("is_granted('ROLE_USER')") */ class SidebarController extends AbstractController { diff --git a/src/Controller/TimesheetController.php b/src/Controller/TimesheetController.php index 85f859ab..e048fbb5 100644 --- a/src/Controller/TimesheetController.php +++ b/src/Controller/TimesheetController.php @@ -25,7 +25,7 @@ use Symfony\Component\HttpFoundation\Request; * Controller used to manage timesheet contents in the public part of the site. * * @Route("/timesheet") - * @Security("has_role('ROLE_USER')") + * @Security("is_granted('ROLE_USER')") */ class TimesheetController extends AbstractController { diff --git a/tests/Controller/ActivityControllerTest.php b/tests/Controller/ActivityControllerTest.php new file mode 100644 index 00000000..2c26ab50 --- /dev/null +++ b/tests/Controller/ActivityControllerTest.php @@ -0,0 +1,23 @@ +markTestSkipped('no public route available'); + } +} diff --git a/tests/Controller/Admin/ActivityControllerTest.php b/tests/Controller/Admin/ActivityControllerTest.php new file mode 100644 index 00000000..0965a69a --- /dev/null +++ b/tests/Controller/Admin/ActivityControllerTest.php @@ -0,0 +1,34 @@ +assertUrlIsSecured('/admin/activity/'); + $this->assertUrlIsSecuredForRole(User::ROLE_TEAMLEAD, '/admin/activity/'); + } + + public function testIndexAction() + { + $client = $this->getClientForAuthenticatedUser(User::ROLE_ADMIN); + $this->assertAccessIsGranted($client, '/admin/activity/'); + $this->assertHasDataTable($client); + } +} diff --git a/tests/Controller/Admin/CustomerControllerTest.php b/tests/Controller/Admin/CustomerControllerTest.php new file mode 100644 index 00000000..88429d8a --- /dev/null +++ b/tests/Controller/Admin/CustomerControllerTest.php @@ -0,0 +1,34 @@ +assertUrlIsSecured('/admin/customer/'); + $this->assertUrlIsSecuredForRole(User::ROLE_TEAMLEAD, '/admin/customer/'); + } + + public function testIndexAction() + { + $client = $this->getClientForAuthenticatedUser(User::ROLE_ADMIN); + $this->assertAccessIsGranted($client, '/admin/customer/'); + $this->assertHasDataTable($client); + } +} diff --git a/tests/Controller/Admin/ProjectControllerTest.php b/tests/Controller/Admin/ProjectControllerTest.php new file mode 100644 index 00000000..b1df398d --- /dev/null +++ b/tests/Controller/Admin/ProjectControllerTest.php @@ -0,0 +1,34 @@ +assertUrlIsSecured('/admin/project/'); + $this->assertUrlIsSecuredForRole(User::ROLE_TEAMLEAD, '/admin/project/'); + } + + public function testIndexAction() + { + $client = $this->getClientForAuthenticatedUser(User::ROLE_ADMIN); + $this->assertAccessIsGranted($client, '/admin/project/'); + $this->assertHasDataTable($client); + } +} diff --git a/tests/Controller/Admin/TimesheetControllerTest.php b/tests/Controller/Admin/TimesheetControllerTest.php new file mode 100644 index 00000000..8de8b953 --- /dev/null +++ b/tests/Controller/Admin/TimesheetControllerTest.php @@ -0,0 +1,34 @@ +assertUrlIsSecured('/team/timesheet/'); + $this->assertUrlIsSecuredForRole(User::ROLE_USER, '/team/timesheet/'); + } + + public function testIndexAction() + { + $client = $this->getClientForAuthenticatedUser(User::ROLE_TEAMLEAD); + $this->assertAccessIsGranted($client, '/team/timesheet/'); + $this->assertHasDataTable($client); + } +} diff --git a/tests/Controller/Admin/UserControllerTest.php b/tests/Controller/Admin/UserControllerTest.php new file mode 100644 index 00000000..31eed5c9 --- /dev/null +++ b/tests/Controller/Admin/UserControllerTest.php @@ -0,0 +1,34 @@ +assertUrlIsSecured('/admin/user/'); + $this->assertUrlIsSecuredForRole(User::ROLE_ADMIN, '/admin/user/'); + } + + public function testIndexAction() + { + $client = $this->getClientForAuthenticatedUser(User::ROLE_SUPER_ADMIN); + $this->assertAccessIsGranted($client, '/admin/user/'); + $this->assertHasDataTable($client); + } +} diff --git a/tests/Controller/ControllerBaseTest.php b/tests/Controller/ControllerBaseTest.php index 13ae29bd..9d1f34aa 100644 --- a/tests/Controller/ControllerBaseTest.php +++ b/tests/Controller/ControllerBaseTest.php @@ -10,6 +10,7 @@ namespace App\Tests\Controller; use App\DataFixtures\AppFixtures; +use App\Entity\User; use Symfony\Bundle\FrameworkBundle\Client; use Symfony\Bundle\FrameworkBundle\Test\WebTestCase; @@ -22,14 +23,44 @@ abstract class ControllerBaseTest extends WebTestCase const DEFAULT_LANGUAGE = 'en'; /** + * @param string $role * @return Client */ - protected function getClientForAuthenticatedUser() + protected function getClientForAuthenticatedUser(string $role = User::ROLE_USER) { - $client = self::createClient([], [ - 'PHP_AUTH_USER' => AppFixtures::USERNAME_USER, - 'PHP_AUTH_PW' => AppFixtures::DEFAULT_PASSWORD, - ]); + switch($role) { + case User::ROLE_SUPER_ADMIN: + $client = self::createClient([], [ + 'PHP_AUTH_USER' => AppFixtures::USERNAME_SUPER_ADMIN, + 'PHP_AUTH_PW' => AppFixtures::DEFAULT_PASSWORD, + ]); + break; + + case User::ROLE_ADMIN: + $client = self::createClient([], [ + 'PHP_AUTH_USER' => AppFixtures::USERNAME_ADMIN, + 'PHP_AUTH_PW' => AppFixtures::DEFAULT_PASSWORD, + ]); + break; + + case User::ROLE_TEAMLEAD: + $client = self::createClient([], [ + 'PHP_AUTH_USER' => AppFixtures::USERNAME_TEAMLEAD, + 'PHP_AUTH_PW' => AppFixtures::DEFAULT_PASSWORD, + ]); + break; + + case User::ROLE_USER: + $client = self::createClient([], [ + 'PHP_AUTH_USER' => AppFixtures::USERNAME_USER, + 'PHP_AUTH_PW' => AppFixtures::DEFAULT_PASSWORD, + ]); + break; + + default: + $client = null; + break; + } return $client; } @@ -46,23 +77,64 @@ abstract class ControllerBaseTest extends WebTestCase } /** + * @param Client $client * @param string $url * @param string $method */ - protected function assertUrlIsSecured(string $url, $method = 'GET') + protected function assertRequestIsSecured(Client $client, string $url, $method = 'GET') { - $client = self::createClient(); $client->request($method, '/' . self::DEFAULT_LANGUAGE . $url); - $this->assertTrue($client->getResponse()->isRedirect()); + $this->assertTrue( + $client->getResponse()->isRedirect(), + sprintf('The secure URL %s is not protected.', $url . $client->getResponse()->getContent()) + ); $this->assertEquals( 'http://localhost/' . self::DEFAULT_LANGUAGE . '/login', $client->getResponse()->getTargetUrl(), - sprintf('The %s secure URL redirects to the login form.', $url) + sprintf('The secure URL %s does not redirect to the login form.', $url) ); } + /** + * @param string $url + * @param string $method + * @param Client|null $client + */ + protected function assertUrlIsSecured(string $url, $method = 'GET') + { + $client = self::createClient(); + $this->assertRequestIsSecured($client, $url, $method); + } + + /** + * @param string $role + * @param string $url + * @param string $method + */ + protected function assertUrlIsSecuredForRole(string $role, string $url, string $method = 'GET') + { + $client = $this->getClientForAuthenticatedUser($role); + $client->request($method, '/' . self::DEFAULT_LANGUAGE . $url); + $this->assertFalse( + $client->getResponse()->isSuccessful(), + sprintf('The secure URL %s is not protected for role %s', $url, $role) + ); + $this->assertContains('Symfony\Component\Security\Core\Exception\AccessDeniedException', $client->getResponse()->getContent()); + } + + /** + * @param Client $client + * @param string $url + */ + protected function assertAccessIsGranted(Client $client, $url) + { + $this->request($client, $url); + $this->assertTrue($client->getResponse()->isSuccessful()); + // TODO improve this test? + } + /** * @param Client $client */ @@ -71,4 +143,21 @@ abstract class ControllerBaseTest extends WebTestCase $this->assertFalse($client->getResponse()->isSuccessful()); $this->assertEquals(404, $client->getResponse()->getStatusCode()); } + + /** + * @param Client $client + * @param string $classname + */ + protected function assertMainContentClass(Client $client, $classname) + { + $this->assertContains('
', $client->getResponse()->getContent()); + } + + /** + * @param Client $client + */ + protected function assertHasDataTable(Client $client) + { + $this->assertContains('', $client->getResponse()->getContent()); + } } diff --git a/tests/Controller/DashboardControllerTest.php b/tests/Controller/DashboardControllerTest.php new file mode 100644 index 00000000..f64e235f --- /dev/null +++ b/tests/Controller/DashboardControllerTest.php @@ -0,0 +1,31 @@ +assertUrlIsSecured('/dashboard/'); + } + + public function testIndexAction() + { + $client = $this->getClientForAuthenticatedUser(); + $this->request($client, '/dashboard/'); + $this->assertTrue($client->getResponse()->isSuccessful()); + $this->assertMainContentClass($client, 'dashboard'); + } +} diff --git a/tests/Controller/InvoiceControllerTest.php b/tests/Controller/InvoiceControllerTest.php new file mode 100644 index 00000000..f4ff1ec3 --- /dev/null +++ b/tests/Controller/InvoiceControllerTest.php @@ -0,0 +1,35 @@ +assertUrlIsSecured('/invoice/'); + $this->assertUrlIsSecuredForRole(User::ROLE_USER, '/invoice/'); + } + + public function testIndexAction() + { + $this->markTestSkipped('create invoice template before this test case'); + $client = $this->getClientForAuthenticatedUser(User::ROLE_TEAMLEAD); + $this->request($client, '/invoice/'); + $this->assertTrue($client->getResponse()->isSuccessful()); + $this->assertMainContentClass($client, 'dashboard'); + } +} diff --git a/tests/Controller/ProfileControllerTest.php b/tests/Controller/ProfileControllerTest.php new file mode 100644 index 00000000..37ec0e91 --- /dev/null +++ b/tests/Controller/ProfileControllerTest.php @@ -0,0 +1,40 @@ +assertUrlIsSecured('/profile/' . AppFixtures::USERNAME_USER); + } + + public function testIndexAction() + { + $client = $this->getClientForAuthenticatedUser(); + $this->request($client, '/profile/' . AppFixtures::USERNAME_USER); + $this->assertTrue($client->getResponse()->isSuccessful()); + } + + public function testIndexActionWithDifferentUsername() + { + $client = $this->getClientForAuthenticatedUser(); + $this->request($client, '/profile/' . AppFixtures::USERNAME_TEAMLEAD); + $this->assertFalse($client->getResponse()->isSuccessful()); + } +} diff --git a/tests/Controller/SidebarControllerTest.php b/tests/Controller/SidebarControllerTest.php new file mode 100644 index 00000000..9617f6e3 --- /dev/null +++ b/tests/Controller/SidebarControllerTest.php @@ -0,0 +1,23 @@ +markTestSkipped('no public route available'); + } +} diff --git a/tests/Controller/TimesheetControllerTest.php b/tests/Controller/TimesheetControllerTest.php new file mode 100644 index 00000000..89fad7c5 --- /dev/null +++ b/tests/Controller/TimesheetControllerTest.php @@ -0,0 +1,33 @@ +assertUrlIsSecured('/timesheet/'); + } + + public function testIndexAction() + { + $client = $this->getClientForAuthenticatedUser(); + $this->request($client, '/timesheet/'); + $this->assertTrue($client->getResponse()->isSuccessful()); + } +}