improve export permission checks (#3027)

src/API/TimesheetController.php

@@ -36,6 +36,7 @@ use Sensio\Bundle\FrameworkExtraBundle\Configuration\Security;
 use Swagger\Annotations as SWG;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\Response;
+use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException;
 use Symfony\Component\HttpKernel\Exception\BadRequestHttpException;
 use Symfony\Component\Validator\Constraints;
 use Symfony\Contracts\EventDispatcher\EventDispatcherInterface;
@@ -704,6 +705,11 @@ class TimesheetController extends BaseApiController
     public function exportAction(Timesheet $id): Response
     {
         $timesheet = $id;
+
+        if ($timesheet->isExported() && !$this->isGranted('edit_exported_timesheet')) {
+            throw new AccessDeniedHttpException('User cannot edit an exported timesheet');
+        }
+
         $timesheet->setExported(!$timesheet->isExported());
 
         $this->service->updateTimesheet($timesheet);

src/Validator/Constraints/TimesheetExportedValidator.php

@@ -42,7 +42,11 @@ final class TimesheetExportedValidator extends ConstraintValidator
             return;
         }
 
-        if (null !== $this->security->getUser() && $this->security->isGranted('edit_exported_timesheet')) {
+        // this was "edit_exported_timesheet" before, but that was wrong, because the first time this
+        // can trigger is the moment when the "export" flag ist set from the "edit form".
+        // most teamleads should not have "edit_exported_timesheet" but only "edit_export_other_timesheet"
+
+        if (null !== $this->security->getUser() && $this->security->isGranted('edit_export', $timesheet)) {
             return;
         }
 

templates/export/index.html.twig

@@ -17,6 +17,7 @@
 } %}
 
 {% set tableName = 'export' %}
+{% set editExported = is_granted('edit_exported_timesheet') %}
 
 {% block page_title %}{{ 'export.title'|trans }}{% endblock %}
 {% block page_actions %}
@@ -185,10 +186,14 @@
                     <td class="{{ tables.data_table_column_class(tableName, columns, 'exported') }}">
                     {% if is_granted('edit_export', entry) %}
                         {% if entry.exported %}
+                            {% if editExported %}
                             <button type="button" class="btn btn-default exportBtn active" data-toggle="button" aria-pressed="true" autocomplete="off"
                                     data-exported-text="{{ 'entryState.exported'|trans }}" data-clean-text="{{ 'entryState.not_exported'|trans }}" data-timesheet="{{ entry.id }}">
                             {{ 'entryState.exported'|trans }}
                             </button>
+                            {% else %}
+                            {{ 'entryState.exported'|trans }}
+                            {% endif %}
                         {% else %}
                             <button type="button" class="btn btn-default exportBtn" data-toggle="button" aria-pressed="false" autocomplete="off"
                                     data-exported-text="{{ 'entryState.exported'|trans }}" data-clean-text="{{ 'entryState.not_exported'|trans }}" data-timesheet="{{ entry.id }}">
@@ -274,12 +279,19 @@
                 },
                 method: 'PATCH',
                 success: function(data) {
+                    let isShowAll = jQuery('select#exported').val() === '{{ constant('App\\Repository\\Query\\TimesheetQuery::STATE_ALL') }}';
                     if (exported) {
+                        {% if editExported %}
                         button.button('exported');
+                        {% else %}
+                        if (isShowAll) {
+                            button.replaceWith('{{ 'entryState.exported'|trans }}');
+                        }
+                        {% endif %}
                     } else {
                         button.button('clean');
                     }
-                    if (jQuery('select#exported').val() !== '{{ constant('App\\Repository\\Query\\TimesheetQuery::STATE_ALL') }}') {
+                    if (!isShowAll) {
                         button.closest('tr').hide('ease', function() {
                             jQuery(this).remove();
                             if(jQuery(this).closest('table').find('tbody tr:visible').length === 0) {

tests/Validator/Constraints/TimesheetExportedValidatorTest.php

@@ -36,7 +36,7 @@ class TimesheetExportedValidatorTest extends ConstraintValidatorTestCase
         $auth->method('isGranted')->willReturnCallback(
             function ($attributes, $subject = null) use ($allowEdit) {
                 switch ($attributes) {
-                    case 'edit_exported_timesheet':
+                    case 'edit_export':
                         return $allowEdit;
                 }