Improved access control for part lists.

src/Controller/PartListsController.php

@@ -175,6 +175,8 @@ class PartListsController extends AbstractController
      */
     public function showCategory(Category $category, Request $request)
     {
+        $this->denyAccessUnlessGranted('@categories.read');
+
         return $this->showListWithFilter($request,
             'Parts/lists/category_list.html.twig',
             function (PartFilter $filter) use ($category) {
@@ -195,6 +197,8 @@ class PartListsController extends AbstractController
      */
     public function showFootprint(Footprint $footprint, Request $request)
     {
+        $this->denyAccessUnlessGranted('@footprints.read');
+
         return $this->showListWithFilter($request,
             'Parts/lists/footprint_list.html.twig',
             function (PartFilter $filter) use ($footprint) {
@@ -215,6 +219,8 @@ class PartListsController extends AbstractController
      */
     public function showManufacturer(Manufacturer $manufacturer, Request $request)
     {
+        $this->denyAccessUnlessGranted('@manufacturers.read');
+
         return $this->showListWithFilter($request,
             'Parts/lists/manufacturer_list.html.twig',
             function (PartFilter $filter) use ($manufacturer) {
@@ -235,6 +241,8 @@ class PartListsController extends AbstractController
      */
     public function showStorelocation(Storelocation $storelocation, Request $request)
     {
+        $this->denyAccessUnlessGranted('@storelocations.read');
+
         return $this->showListWithFilter($request,
             'Parts/lists/store_location_list.html.twig',
             function (PartFilter $filter) use ($storelocation) {
@@ -255,6 +263,8 @@ class PartListsController extends AbstractController
      */
     public function showSupplier(Supplier $supplier, Request $request)
     {
+        $this->denyAccessUnlessGranted('@suppliers.read');
+
         return $this->showListWithFilter($request,
             'Parts/lists/supplier_list.html.twig',
             function (PartFilter $filter) use ($supplier) {

src/DataTables/PartsDataTable.php

@@ -163,20 +163,29 @@ final class PartsDataTable implements DataTableTypeInterface
             ])
             ->add('description', MarkdownColumn::class, [
                 'label' => $this->translator->trans('part.table.description'),
-            ])
+            ]);
-            ->add('category', EntityColumn::class, [
+
+        if ($this->security->isGranted('@categories.read')) {
+            $dataTable->add('category', EntityColumn::class, [
                 'label' => $this->translator->trans('part.table.category'),
                 'property' => 'category',
-            ])
+            ]);
-            ->add('footprint', EntityColumn::class, [
+        }
+
+        if ($this->security->isGranted('@footprints.read')) {
+            $dataTable->add('footprint', EntityColumn::class, [
                 'property' => 'footprint',
                 'label' => $this->translator->trans('part.table.footprint'),
-            ])
+            ]);
-            ->add('manufacturer', EntityColumn::class, [
+        }
+        if ($this->security->isGranted('@manufacturers.read')) {
+            $dataTable->add('manufacturer', EntityColumn::class, [
                 'property' => 'manufacturer',
                 'label' => $this->translator->trans('part.table.manufacturer'),
-            ])
+            ]);
-            ->add('storelocation', TextColumn::class, [
+        }
+        if ($this->security->isGranted('@storelocations.read')) {
+            $dataTable->add('storelocation', TextColumn::class, [
                 'label' => $this->translator->trans('part.table.storeLocations'),
                 'render' => function ($value, Part $context) {
                     $tmp = [];
@@ -194,32 +203,38 @@ final class PartsDataTable implements DataTableTypeInterface
 
                     return implode('<br>', $tmp);
                 },
-            ])
+            ]);
-            ->add('amount', TextColumn::class, [
+        }
-                'label' => $this->translator->trans('part.table.amount'),
-                'render' => function ($value, Part $context) {
-                    $amount = $context->getAmountSum();
 
-                    return $this->amountFormatter->format($amount, $context->getPartUnit());
+        $dataTable->add('amount', TextColumn::class, [
-                },
+            'label' => $this->translator->trans('part.table.amount'),
-                'orderField' => 'amountSum'
+            'render' => function ($value, Part $context) {
-            ])
+                $amount = $context->getAmountSum();
+
+                return $this->amountFormatter->format($amount, $context->getPartUnit());
+            },
+            'orderField' => 'amountSum'
+        ])
             ->add('minamount', TextColumn::class, [
                 'label' => $this->translator->trans('part.table.minamount'),
                 'visible' => false,
                 'render' => function ($value, Part $context) {
                     return $this->amountFormatter->format($value, $context->getPartUnit());
                 },
-            ])
+            ]);
-            ->add('partUnit', TextColumn::class, [
+
+        if ($this->security->isGranted('@footprints.read')) {
+            $dataTable->add('partUnit', TextColumn::class, [
                 'field' => 'partUnit.name',
                 'label' => $this->translator->trans('part.table.partUnit'),
                 'visible' => false,
-            ])
+            ]);
-            ->add('addedDate', LocaleDateTimeColumn::class, [
+        }
-                'label' => $this->translator->trans('part.table.addedDate'),
+
-                'visible' => false,
+        $dataTable->add('addedDate', LocaleDateTimeColumn::class, [
-            ])
+            'label' => $this->translator->trans('part.table.addedDate'),
+            'visible' => false,
+        ])
             ->add('lastModified', LocaleDateTimeColumn::class, [
                 'label' => $this->translator->trans('part.table.lastModified'),
                 'visible' => false,

src/Security/Voter/PermissionVoter.php

@@ -80,7 +80,12 @@ class PermissionVoter extends ExtendedVoter
             $attribute = ltrim($attribute, '@');
             [$perm, $op] = explode('.', $attribute);
 
-            return $this->resolver->isValidOperation($perm, $op);
+            $valid = $this->resolver->isValidOperation($perm, $op);
+
+            //if an invalid operation is encountered, throw an exception so the developer knows it
+            //throw new \RuntimeException('Encountered invalid permission operation "'.$op.'" for permission "'.$perm.'"!');
+
+            return true;
         }
 
         return false;

src/Services/Parts/PartsTableActionHandler.php

@@ -76,9 +76,11 @@ final class PartsTableActionHandler
 
             switch ($action) {
                 case 'favorite':
+                    $this->denyAccessUnlessGranted('change_favorite', $part);
                     $part->setFavorite(true);
                     break;
                 case 'unfavorite':
+                    $this->denyAccessUnlessGranted('change_favorite', $part);
                     $part->setFavorite(false);
                     break;
                 case 'delete':
@@ -86,19 +88,19 @@ final class PartsTableActionHandler
                     $this->entityManager->remove($part);
                     break;
                 case 'change_category':
-                    $this->denyAccessUnlessGranted('category.edit', $part);
+                    $this->denyAccessUnlessGranted('@categories.read');
                     $part->setCategory($this->entityManager->find(Category::class, $target_id));
                     break;
                 case 'change_footprint':
-                    $this->denyAccessUnlessGranted('footprint.edit', $part);
+                    $this->denyAccessUnlessGranted('@footprints.read');
                     $part->setFootprint(null === $target_id ? null : $this->entityManager->find(Footprint::class, $target_id));
                     break;
                 case 'change_manufacturer':
-                    $this->denyAccessUnlessGranted('manufacturer.edit', $part);
+                    $this->denyAccessUnlessGranted('@manufacturers.read');
                     $part->setManufacturer(null === $target_id ? null : $this->entityManager->find(Manufacturer::class, $target_id));
                     break;
                 case 'change_unit':
-                    $this->denyAccessUnlessGranted('unit.edit', $part);
+                    $this->denyAccessUnlessGranted('@measurement_units.read');
                     $part->setPartUnit(null === $target_id ? null : $this->entityManager->find(MeasurementUnit::class, $target_id));
                     break;
 

templates/components/datatables.macro.html.twig

@@ -35,15 +35,15 @@
             <select class="selectpicker" name="action" data-controller="elements--selectpicker" {{ stimulus_action('elements/datatables/parts', 'updateTargetPicker', 'change') }}
                     title="{% trans %}part_list.action.action.title{% endtrans %}" required>
                 <optgroup label="{% trans %}part_list.action.action.group.favorite{% endtrans %}">
-                    <option {% if not is_granted('@parts.edit') %}disabled{% endif %} value="favorite">{% trans %}part_list.action.action.favorite{% endtrans %}</option>
+                    <option {% if not is_granted('@parts.change_favorite')  %}disabled{% endif %} value="favorite">{% trans %}part_list.action.action.favorite{% endtrans %}</option>
-                    <option {% if not is_granted('@parts.edit') %}disabled{% endif %} value="unfavorite">{% trans %}part_list.action.action.unfavorite{% endtrans %}</option>
+                    <option {% if not is_granted('@parts.change_favorite') %}disabled{% endif %} value="unfavorite">{% trans %}part_list.action.action.unfavorite{% endtrans %}</option>
                 </optgroup>
 
                 <optgroup label="{% trans %}part_list.action.action.group.change_field{% endtrans %}">
-                    <option {% if not is_granted('@parts_category.edit') %}disabled{% endif %} value="change_category" data-url="{{ path('select_category') }}">{% trans %}part_list.action.action.change_category{% endtrans %}</option>
+                    <option {% if not is_granted('@categories.read') %}disabled{% endif %} value="change_category" data-url="{{ path('select_category') }}">{% trans %}part_list.action.action.change_category{% endtrans %}</option>
-                    <option {% if not is_granted('@parts_footprint.edit') %}disabled{% endif %} value="change_footprint" data-url="{{ path('select_footprint') }}">{% trans %}part_list.action.action.change_footprint{% endtrans %}</option>
+                    <option {% if not is_granted('@footprints.read') %}disabled{% endif %} value="change_footprint" data-url="{{ path('select_footprint') }}">{% trans %}part_list.action.action.change_footprint{% endtrans %}</option>
-                    <option {% if not is_granted('@parts_manufacturer.edit') %}disabled{% endif %} value="change_manufacturer" data-url="{{ path('select_manufacturer') }}">{% trans %}part_list.action.action.change_manufacturer{% endtrans %}</option>
+                    <option {% if not is_granted('@manufacturers.read') %}disabled{% endif %} value="change_manufacturer" data-url="{{ path('select_manufacturer') }}">{% trans %}part_list.action.action.change_manufacturer{% endtrans %}</option>
-                    <option {% if not is_granted('@parts_unit.edit') %}disabled{% endif %} value="change_unit" data-url="{{ path('select_measurement_unit') }}">{% trans %}part_list.action.action.change_unit{% endtrans %}</option>
+                    <option {% if not is_granted('@measurement_units.read') %}disabled{% endif %} value="change_unit" data-url="{{ path('select_measurement_unit') }}">{% trans %}part_list.action.action.change_unit{% endtrans %}</option>
                 </optgroup>
 
                 <option {% if not is_granted('@parts.delete') %}disabled{% endif %} value="delete">{% trans %}part_list.action.action.delete{% endtrans %}</option>
@@ -53,7 +53,7 @@
                 {# This is left empty, as this will be filled by Javascript #}
             </select>
 
-            <button type="submit" class="btn btn-secondary">{% trans %}part_list.action.submit{% endtrans %}</button>
+            <button type="submit" class="btn btn-secondary" {% if not is_granted('@parts.edit') %}disabled{% endif %}>{% trans %}part_list.action.submit{% endtrans %}</button>
         </div>
 
         <div {{ stimulus_target('elements/datatables/parts', 'dt') }}>