make sure that timezone is properly validated (#2663)

src/Command/ImportTimesheetCommand.php

@@ -206,6 +206,9 @@ class ImportTimesheetCommand extends Command
 
             default:
                 try {
+                    if (!\in_array($timezone, \DateTimeZone::listIdentifiers())) {
+                        throw new \InvalidArgumentException('Not a known PHP timezone');
+                    }
                     $timezone = new \DateTimeZone($timezone);
                 } catch (\Exception $ex) {
                     $io->error('Invalid timezone given, import canceled.');
@@ -245,7 +248,7 @@ class ImportTimesheetCommand extends Command
         $all = $csv->getRecords();
         $total = iterator_count($all);
 
-        $io->text(sprintf('Found %s timesheets to import, validating now', $total));
+        $io->text(sprintf('Found %s timesheets to import, pre-validating now', $total));
 
         $records = [];
         $doImport = true;
@@ -288,6 +291,7 @@ class ImportTimesheetCommand extends Command
             $progressBar->advance();
         }
         $progressBar->finish();
+        $io->writeln('');
 
         if (!$ignoreErrors && !$doImport) {
             $io->caution(sprintf('Not importing, previous %s errors need to be fixed first.', $errors));
@@ -295,8 +299,8 @@ class ImportTimesheetCommand extends Command
             return 5;
         }
 
-        $io->text(sprintf('Validated %s rows.', $countAll));
+        $io->writeln('');
-        $io->text(sprintf('Importing %s of %s rows, skipping %s with validation errors.', \count($records), iterator_count($all), $errors));
+        $io->text(sprintf('Processing %s of %s rows, skipping %s with pre-validation errors.', \count($records), iterator_count($all), $errors));
 
         // values for new users
         $password = $input->getOption('password');
@@ -306,6 +310,8 @@ class ImportTimesheetCommand extends Command
 
         $durationParser = new Duration();
         $row = 0;
+        $imported = 0;
+        $failed = 0;
 
         $isBatchUpdate = $input->getOption('batch');
         $batches = [];
@@ -401,10 +407,11 @@ class ImportTimesheetCommand extends Command
                 } else {
                     $this->timesheets->save($timesheet);
                 }
+
+                $imported++;
             } catch (\Exception $ex) {
                 $io->error(sprintf('Failed importing timesheet row %s with: %s', $row, $ex->getMessage()));
-
+                $failed++;
-                return 6;
             }
 
             $progressBar->advance();
@@ -416,6 +423,9 @@ class ImportTimesheetCommand extends Command
 
         $progressBar->finish();
 
+        $io->writeln('');
+        $io->writeln('');
+
         if ($this->createdUsers > 0) {
             $io->success(sprintf('Created %s users', $this->createdUsers));
         }
@@ -429,7 +439,13 @@ class ImportTimesheetCommand extends Command
             $io->success(sprintf('Created %s activities', $this->createdActivities));
         }
 
-        $io->success(sprintf('Imported %s rows', $row));
+        if ($failed > 0) {
+            $io->warning(sprintf('Failed validating %s rows', $failed));
+        }
+
+        if ($imported > 0) {
+            $io->success(sprintf('Imported %s rows', $imported));
+        }
 
         return 0;
     }

src/Entity/Timesheet.php

@@ -140,6 +140,7 @@ class Timesheet implements EntityWithMetaFields, ExportItemInterface
      * @internal for storing the timezone of "begin" and "end" date
      *
      * @ORM\Column(name="timezone", type="string", length=64, nullable=false)
+     * @Assert\Timezone
      */
     private $timezone;
     /**

src/Validator/Constraints/TimesheetBudgetUsed.php

@@ -9,18 +9,9 @@
 
 namespace App\Validator\Constraints;
 
-use App\Validator\TimesheetBudgetUsedValidator;
+final class TimesheetBudgetUsed extends TimesheetConstraint
-
-final class TimesheetBudgetUsedConstraint extends TimesheetConstraint
 {
-    public const BUDGET_SPENT = 'kimai-timesheet-budget-used-01';
-
     // same messages, so we can re-use the validation translation!
     public $messageRate = 'The budget is completely used.';
     public $messageTime = 'The budget is completely used.';
-
-    public function validatedBy()
-    {
-        return TimesheetBudgetUsedValidator::class;
-    }
 }

src/Validator/Constraints/TimesheetBudgetUsedValidator.php

@@ -7,7 +7,7 @@
  * file that was distributed with this source code.
  */
 
-namespace App\Validator;
+namespace App\Validator\Constraints;
 
 use App\Activity\ActivityStatisticService;
 use App\Configuration\SystemConfiguration;
@@ -18,7 +18,6 @@ use App\Repository\TimesheetRepository;
 use App\Timesheet\RateServiceInterface;
 use App\Utils\Duration;
 use App\Utils\LocaleHelper;
-use App\Validator\Constraints\TimesheetBudgetUsedConstraint;
 use Symfony\Component\Validator\Constraint;
 use Symfony\Component\Validator\ConstraintValidator;
 use Symfony\Component\Validator\Exception\UnexpectedTypeException;
@@ -48,8 +47,8 @@ final class TimesheetBudgetUsedValidator extends ConstraintValidator
      */
     public function validate($timesheet, Constraint $constraint)
     {
-        if (!($constraint instanceof TimesheetBudgetUsedConstraint)) {
+        if (!($constraint instanceof TimesheetBudgetUsed)) {
-            throw new UnexpectedTypeException($constraint, TimesheetBudgetUsedConstraint::class);
+            throw new UnexpectedTypeException($constraint, TimesheetBudgetUsed::class);
         }
 
         if (!\is_object($timesheet) || !($timesheet instanceof Timesheet)) {
@@ -127,7 +126,7 @@ final class TimesheetBudgetUsedValidator extends ConstraintValidator
         }
     }
 
-    private function checkActivity(TimesheetBudgetUsedConstraint $constraint, Timesheet $timesheet, int $duration, float $rate): bool
+    private function checkActivity(TimesheetBudgetUsed $constraint, Timesheet $timesheet, int $duration, float $rate): bool
     {
         $activity = $timesheet->getActivity();
 
@@ -156,7 +155,7 @@ final class TimesheetBudgetUsedValidator extends ConstraintValidator
         return false;
     }
 
-    private function checkProject(TimesheetBudgetUsedConstraint $constraint, Timesheet $timesheet, int $duration, float $rate): bool
+    private function checkProject(TimesheetBudgetUsed $constraint, Timesheet $timesheet, int $duration, float $rate): bool
     {
         $project = $timesheet->getProject();
 
@@ -185,7 +184,7 @@ final class TimesheetBudgetUsedValidator extends ConstraintValidator
         return false;
     }
 
-    private function checkCustomer(TimesheetBudgetUsedConstraint $constraint, Timesheet $timesheet, int $duration, float $rate): bool
+    private function checkCustomer(TimesheetBudgetUsed $constraint, Timesheet $timesheet, int $duration, float $rate): bool
     {
         $customer = $timesheet->getProject()->getCustomer();
 
@@ -214,7 +213,7 @@ final class TimesheetBudgetUsedValidator extends ConstraintValidator
         return false;
     }
 
-    private function addBudgetViolation(TimesheetBudgetUsedConstraint $constraint, Timesheet $timesheet, string $field, float $budget, float $rate)
+    private function addBudgetViolation(TimesheetBudgetUsed $constraint, Timesheet $timesheet, string $field, float $budget, float $rate)
     {
         // using the locale of the assigned user is not the best solution, but allows to be independent from the request stack
         $helper = new LocaleHelper($timesheet->getUser()->getLanguage());
@@ -235,7 +234,7 @@ final class TimesheetBudgetUsedValidator extends ConstraintValidator
         ;
     }
 
-    private function addTimeBudgetViolation(TimesheetBudgetUsedConstraint $constraint, string $field, int $budget, int $duration)
+    private function addTimeBudgetViolation(TimesheetBudgetUsed $constraint, string $field, int $budget, int $duration)
     {
         $durationFormat = new Duration();
 

src/Validator/Constraints/TimesheetLockdownValidator.php

@@ -11,19 +11,19 @@ namespace App\Validator\Constraints;
 
 use App\Entity\Timesheet as TimesheetEntity;
 use App\Timesheet\LockdownService;
-use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface;
+use Symfony\Component\Security\Core\Security;
 use Symfony\Component\Validator\Constraint;
 use Symfony\Component\Validator\ConstraintValidator;
 use Symfony\Component\Validator\Exception\UnexpectedTypeException;
 
 final class TimesheetLockdownValidator extends ConstraintValidator
 {
-    private $auth;
     private $lockdownService;
+    private $security;
 
-    public function __construct(AuthorizationCheckerInterface $auth, LockdownService $lockdownService)
+    public function __construct(Security $security, LockdownService $lockdownService)
     {
-        $this->auth = $auth;
+        $this->security = $security;
         $this->lockdownService = $lockdownService;
     }
 
@@ -50,7 +50,7 @@ final class TimesheetLockdownValidator extends ConstraintValidator
         }
 
         // lockdown never takes effect for users with special permission
-        if ($this->auth->isGranted('lockdown_override_timesheet')) {
+        if (null !== $this->security->getUser() && $this->security->isGranted('lockdown_override_timesheet')) {
             return;
         }
 
@@ -67,7 +67,10 @@ final class TimesheetLockdownValidator extends ConstraintValidator
             }
         }
 
-        $allowEditInGracePeriod = $this->auth->isGranted('lockdown_grace_timesheet');
+        $allowEditInGracePeriod = false;
+        if (null !== $this->security->getUser() && $this->security->isGranted('lockdown_grace_timesheet')) {
+            $allowEditInGracePeriod = true;
+        }
 
         if ($this->lockdownService->isEditable($timesheet, $now, $allowEditInGracePeriod)) {
             return;

src/Validator/Constraints/TimesheetRestartValidator.php

@@ -11,26 +11,20 @@ namespace App\Validator\Constraints;
 
 use App\Entity\Timesheet as TimesheetEntity;
 use App\Timesheet\TrackingModeService;
-use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface;
+use Symfony\Component\Security\Core\Security;
 use Symfony\Component\Validator\Constraint;
 use Symfony\Component\Validator\ConstraintValidator;
 use Symfony\Component\Validator\Exception\UnexpectedTypeException;
 
 final class TimesheetRestartValidator extends ConstraintValidator
 {
-    /**
-     * @var TrackingModeService
-     */
     private $trackingModeService;
-    /**
+    private $security;
-     * @var AuthorizationCheckerInterface
-     */
-    private $auth;
 
-    public function __construct(TrackingModeService $service, AuthorizationCheckerInterface $auth)
+    public function __construct(Security $security, TrackingModeService $service)
     {
+        $this->security = $security;
         $this->trackingModeService = $service;
-        $this->auth = $auth;
     }
 
     /**
@@ -58,7 +52,7 @@ final class TimesheetRestartValidator extends ConstraintValidator
             return;
         }
 
-        if ($this->auth->isGranted('start', $timesheet)) {
+        if (null !== $this->security->getUser() && $this->security->isGranted('start', $timesheet)) {
             return;
         }
 
@@ -76,7 +70,5 @@ final class TimesheetRestartValidator extends ConstraintValidator
             ->setTranslationDomain('validators')
             ->setCode(TimesheetRestart::START_DISALLOWED)
             ->addViolation();
-
-        return;
     }
 }

tests/Validator/Constraints/TimesheetBudgetUsedValidatorTest.php

@@ -7,7 +7,7 @@
  * file that was distributed with this source code.
  */
 
-namespace App\Tests\Validator;
+namespace App\Tests\Validator\Constraints;
 
 use App\Activity\ActivityStatisticService;
 use App\Configuration\SystemConfiguration;
@@ -25,16 +25,16 @@ use App\Repository\TimesheetRepository;
 use App\Timesheet\Rate;
 use App\Timesheet\RateService;
 use App\Timesheet\RateServiceInterface;
-use App\Validator\Constraints\TimesheetBudgetUsedConstraint;
+use App\Validator\Constraints\TimesheetBudgetUsed;
-use App\Validator\TimesheetBudgetUsedValidator;
+use App\Validator\Constraints\TimesheetBudgetUsedValidator;
 use DateTime;
 use Symfony\Component\Validator\Constraints\NotBlank;
 use Symfony\Component\Validator\Exception\UnexpectedTypeException;
 use Symfony\Component\Validator\Test\ConstraintValidatorTestCase;
 
 /**
- * @covers \App\Validator\Constraints\TimesheetBudgetUsedConstraint
+ * @covers \App\Validator\Constraints\TimesheetBudgetUsed
- * @covers \App\Validator\TimesheetBudgetUsedValidator
+ * @covers \App\Validator\Constraints\TimesheetBudgetUsedValidator
  */
 class TimesheetBudgetUsedValidatorTest extends ConstraintValidatorTestCase
 {
@@ -83,7 +83,7 @@ class TimesheetBudgetUsedValidatorTest extends ConstraintValidatorTestCase
         $this->validator->initialize($this->context);
         $this->context->addViolation('FOOOOOOOOO');
 
-        $this->validator->validate(new Timesheet(), new TimesheetBudgetUsedConstraint());
+        $this->validator->validate(new Timesheet(), new TimesheetBudgetUsed());
         $this->buildViolation('FOOOOOOOOO')->assertRaised();
     }
 
@@ -91,7 +91,7 @@ class TimesheetBudgetUsedValidatorTest extends ConstraintValidatorTestCase
     {
         $this->expectException(UnexpectedTypeException::class);
 
-        $this->validator->validate('foo', new TimesheetBudgetUsedConstraint());
+        $this->validator->validate('foo', new TimesheetBudgetUsed());
     }
 
     public function testWithMissingEnd()
@@ -99,7 +99,7 @@ class TimesheetBudgetUsedValidatorTest extends ConstraintValidatorTestCase
         $timesheet = new Timesheet();
         $timesheet->setBegin(new DateTime());
 
-        $this->validator->validate($timesheet, new TimesheetBudgetUsedConstraint());
+        $this->validator->validate($timesheet, new TimesheetBudgetUsed());
         $this->assertNoViolation();
     }
 
@@ -109,7 +109,7 @@ class TimesheetBudgetUsedValidatorTest extends ConstraintValidatorTestCase
         $timesheet->setBegin(new DateTime());
         $timesheet->setEnd(new DateTime());
 
-        $this->validator->validate($timesheet, new TimesheetBudgetUsedConstraint());
+        $this->validator->validate($timesheet, new TimesheetBudgetUsed());
         $this->assertNoViolation();
     }
 
@@ -120,7 +120,7 @@ class TimesheetBudgetUsedValidatorTest extends ConstraintValidatorTestCase
         $timesheet->setEnd(new DateTime());
         $timesheet->setUser(new User());
 
-        $this->validator->validate($timesheet, new TimesheetBudgetUsedConstraint());
+        $this->validator->validate($timesheet, new TimesheetBudgetUsed());
         $this->assertNoViolation();
     }
 
@@ -135,7 +135,7 @@ class TimesheetBudgetUsedValidatorTest extends ConstraintValidatorTestCase
         $timesheet->setUser(new User());
         $timesheet->setProject($project);
 
-        $this->validator->validate($timesheet, new TimesheetBudgetUsedConstraint());
+        $this->validator->validate($timesheet, new TimesheetBudgetUsed());
         $this->assertNoViolation();
     }
 
@@ -161,7 +161,7 @@ class TimesheetBudgetUsedValidatorTest extends ConstraintValidatorTestCase
         $timesheet->setProject($project);
         $timesheet->setActivity($activity);
 
-        $this->validator->validate($timesheet, new TimesheetBudgetUsedConstraint());
+        $this->validator->validate($timesheet, new TimesheetBudgetUsed());
         $this->assertNoViolation();
     }
 
@@ -372,7 +372,7 @@ class TimesheetBudgetUsedValidatorTest extends ConstraintValidatorTestCase
         $this->validator = $this->createValidator(false, $activityStatistic, $projectStatistic, $customerStatistic, $rawData, $rate);
         $this->validator->initialize($this->context);
 
-        $this->validator->validate($timesheet, new TimesheetBudgetUsedConstraint());
+        $this->validator->validate($timesheet, new TimesheetBudgetUsed());
 
         if (null === $used && null === $budget && null === $free && $path === null) {
             $this->assertNoViolation();

tests/Validator/Constraints/TimesheetLockdownValidatorTest.php

@@ -12,10 +12,11 @@ namespace App\Tests\Validator\Constraints;
 use App\Configuration\ConfigLoaderInterface;
 use App\Configuration\SystemConfiguration;
 use App\Entity\Timesheet;
+use App\Entity\User;
 use App\Timesheet\LockdownService;
 use App\Validator\Constraints\TimesheetLockdown;
 use App\Validator\Constraints\TimesheetLockdownValidator;
-use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface;
+use Symfony\Component\Security\Core\Security;
 use Symfony\Component\Validator\Constraints\NotBlank;
 use Symfony\Component\Validator\Exception\UnexpectedTypeException;
 use Symfony\Component\Validator\Test\ConstraintValidatorTestCase;
@@ -32,7 +33,8 @@ class TimesheetLockdownValidatorTest extends ConstraintValidatorTestCase
 
     protected function createMyValidator(bool $allowOverwriteFull, bool $allowOverwriteGrace, ?string $start, ?string $end, ?string $grace)
     {
-        $auth = $this->createMock(AuthorizationCheckerInterface::class);
+        $auth = $this->createMock(Security::class);
+        $auth->method('getUser')->willReturn(new User());
         $auth->method('isGranted')->willReturnCallback(
             function ($attributes, $subject = null) use ($allowOverwriteFull, $allowOverwriteGrace) {
                 switch ($attributes) {

tests/Validator/Constraints/TimesheetRestartValidatorTest.php

@@ -13,11 +13,12 @@ use App\Entity\Activity;
 use App\Entity\Customer;
 use App\Entity\Project;
 use App\Entity\Timesheet;
+use App\Entity\User;
 use App\Tests\Mocks\TrackingModeServiceFactory;
 use App\Validator\Constraints\TimesheetOverlapping;
 use App\Validator\Constraints\TimesheetRestart;
 use App\Validator\Constraints\TimesheetRestartValidator;
-use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface;
+use Symfony\Component\Security\Core\Security;
 use Symfony\Component\Validator\Constraints\NotBlank;
 use Symfony\Component\Validator\Exception\UnexpectedTypeException;
 use Symfony\Component\Validator\Test\ConstraintValidatorTestCase;
@@ -34,12 +35,13 @@ class TimesheetRestartValidatorTest extends ConstraintValidatorTestCase
 
     protected function createMyValidator(bool $allowed, string $trackingMode)
     {
-        $auth = $this->createMock(AuthorizationCheckerInterface::class);
+        $auth = $this->createMock(Security::class);
+        $auth->method('getUser')->willReturn(new User());
         $auth->method('isGranted')->willReturn($allowed);
 
         $service = (new TrackingModeServiceFactory($this))->create($trackingMode);
 
-        return new TimesheetRestartValidator($service, $auth);
+        return new TimesheetRestartValidator($auth, $service);
     }
 
     public function testConstraintIsInvalid()