Release 2.14 (#4710)

- show "link has expired message" in password reset screen
- added date objects as hydrator variables - for custom date formats in invoice templates
- show meta-fields with null values (e.g. booleans with `false` where hidden)
- fix permission check: allow to remove `view_own_timesheet` but still record times
- prevent error 500 if customer country is empty
- fix API 500 error if project does not exist when creating new timesheet
- fix tags are not created in remote-search mode
- do not check "export items" by default
- fix daterange query, if user an request locale are different
- added logging for invalid SAML responses (see various discussions)
This commit is contained in:
Kevin Papst
2024-04-05 12:38:21 +02:00
committed by GitHub
parent 19b2d47591
commit b6c98f871d
45 changed files with 318 additions and 317 deletions

View File

@@ -17,11 +17,11 @@ class Constants
/**
* The current release version
*/
public const VERSION = '2.13.0';
public const VERSION = '2.14.0';
/**
* The current release: major * 10000 + minor * 100 + patch
*/
public const VERSION_ID = 21300;
public const VERSION_ID = 21400;
/**
* The software name
*/

View File

@@ -255,6 +255,8 @@ abstract class AbstractController extends BaseAbstractController implements Serv
}
/**
* Use "performSearch=1" to skip loading session searches.
*
* @param array<string> $filterParams parameter names, which should not be saved (neither session, nor database)
* @throws \Exception
*/
@@ -298,7 +300,6 @@ abstract class AbstractController extends BaseAbstractController implements Serv
}
$searchName = $this->getSearchName($data);
/** @var BookmarkRepository $bookmarkRepo */
$bookmarkRepo = $this->getBookmark();
$bookmark = $bookmarkRepo->getSearchDefaultOptions($this->getUser(), $searchName);

View File

@@ -325,7 +325,7 @@ final class CustomerController extends AbstractController
$rates = $rateRepository->getRatesForCustomer($customer);
}
if (null !== $customer->getTimezone()) {
if ($customer->getTimezone() !== null && $customer->getTimezone() !== '') {
$timezone = new \DateTimeZone($customer->getTimezone());
}

View File

@@ -132,6 +132,8 @@ final class PasswordResetController extends AbstractController
}
if (!$user->isPasswordRequestNonExpired($this->configuration->getPasswordResetTokenLifetime())) {
$this->flashWarning('This link has already expired');
return $this->redirectToRoute('resetting_request');
}

View File

@@ -19,8 +19,15 @@ use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\Routing\Annotation\Route;
use Symfony\Component\Security\Http\Attribute\IsGranted;
/**
* No permission check on controller level, only for single routes.
*
* There was "view_own_timesheet" here once, but it is a bug, as some companies (rarely, but existing) want their
* employees to enter time, but not to see it afterward.
*
* It is legit to only own "create_own_timesheet" without "view_own_timesheet".
*/
#[Route(path: '/timesheet')]
#[IsGranted('view_own_timesheet')]
final class TimesheetController extends TimesheetAbstractController
{
#[Route(path: '/', defaults: ['page' => 1], name: 'timesheet', methods: ['GET'])]

View File

@@ -27,8 +27,15 @@ use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\Routing\Annotation\Route;
use Symfony\Component\Security\Http\Attribute\IsGranted;
/**
* No permission check on controller level, only for single routes.
*
* There was "view_other_timesheet" here once, but it is a bug.
* Some companies (rarely, but existing) want their employees to enter time, but not to see it afterward.
*
* It is legit to only own "create_other_timesheet" without "view_other_timesheet".
*/
#[Route(path: '/team/timesheet')]
#[IsGranted('view_other_timesheet')]
final class TimesheetTeamController extends TimesheetAbstractController
{
#[Route(path: '/', defaults: ['page' => 1], name: 'admin_timesheet', methods: ['GET'])]

View File

@@ -14,32 +14,30 @@ use App\Repository\TagRepository;
use Symfony\Component\Form\DataTransformerInterface;
use Symfony\Component\Form\Exception\TransformationFailedException;
/**
* @implements DataTransformerInterface<array<Tag>, string>
*/
final class TagArrayToStringTransformer implements DataTransformerInterface
{
private bool $create = true;
public function __construct(private TagRepository $tagRepository)
public function __construct(
private readonly TagRepository $tagRepository,
private readonly bool $create
)
{
}
public function setCreate(bool $create): void
{
$this->create = $create;
}
/**
* Transforms an array of tags to a string.
*
* @param Tag[]|null $tags
* @return string
* @param Tag[]|null $value
*/
public function transform(mixed $tags): mixed
public function transform(mixed $value): string
{
if (empty($tags)) {
if (empty($value)) {
return '';
}
return implode(', ', $tags);
return implode(', ', $value);
}
/**
@@ -47,18 +45,17 @@ final class TagArrayToStringTransformer implements DataTransformerInterface
*
* @see \Symfony\Bridge\Doctrine\Form\DataTransformer\CollectionToArrayTransformer::reverseTransform()
*
* @param string|null $stringOfTags
* @param string|null $value
* @return Tag[]
* @throws TransformationFailedException
*/
public function reverseTransform(mixed $stringOfTags): mixed
public function reverseTransform(mixed $value): mixed
{
// check for empty tag list
if ('' === $stringOfTags || null === $stringOfTags) {
if ('' === $value || null === $value) {
return [];
}
$names = array_filter(array_unique(array_map('trim', explode(',', $stringOfTags))));
$names = array_filter(array_unique(array_map('trim', explode(',', $value))));
// get the current tags and find the new ones that should be created
$tags = $this->tagRepository->findBy(['name' => $names]);
@@ -69,9 +66,9 @@ final class TagArrayToStringTransformer implements DataTransformerInterface
foreach ($newNames as $name) {
$tag = new Tag();
$tag->setName(mb_substr($name, 0, 100));
$tags[] = $tag;
$this->tagRepository->saveTag($tag);
// new tags persist automatically thanks to the cascade={"persist"}
$tags[] = $tag;
}
}

View File

@@ -73,14 +73,13 @@ trait FormTrait
if ($isNew && \is_int($project)) {
/** @var Project $project */
$project = $repo->find($project);
if ($project === null) {
throw new \Exception('Unknown project');
}
if (!$project->getCustomer()->isVisible()) {
$customer = null;
$project = null;
} elseif (!$project->isVisible()) {
$project = null;
if ($project !== null) {
if (!$project->getCustomer()->isVisible()) {
$customer = null;
$project = null;
} elseif (!$project->isVisible()) {
$project = null;
}
}
}

View File

@@ -42,7 +42,7 @@ final class CustomerToolbarForm extends AbstractType
$countries = $qb->getQuery()->getSingleColumnResult();
$choices = [];
foreach ($countries as $country) {
if (\is_string($country) && \is_string($options['locale'])) {
if (\is_string($country) && $country !== '' && \is_string($options['locale'])) {
$choices[$country] = Countries::getName($country, $options['locale']);
}
}

View File

@@ -65,7 +65,6 @@ final class DateRangeType extends AbstractType
return ['pattern' => $pattern . self::DATE_SPACER . $pattern];
});
}
public function buildView(FormView $view, FormInterface $form, array $options): void

View File

@@ -10,6 +10,7 @@
namespace App\Form\Type;
use App\Form\DataTransformer\TagArrayToStringTransformer;
use App\Repository\TagRepository;
use Symfony\Bridge\Doctrine\Form\DataTransformer\CollectionToArrayTransformer;
use Symfony\Component\Form\AbstractType;
use Symfony\Component\Form\Extension\Core\Type\TextType;
@@ -24,18 +25,17 @@ use Symfony\Component\Routing\Generator\UrlGeneratorInterface;
*/
final class TagsInputType extends AbstractType
{
public function __construct(private TagArrayToStringTransformer $transformer, private UrlGeneratorInterface $router)
public function __construct(
private readonly TagRepository $tagRepository,
private readonly UrlGeneratorInterface $router
)
{
}
public function buildForm(FormBuilderInterface $builder, array $options): void
{
if ($options['allow_create'] === false) {
$this->transformer->setCreate(false);
}
$builder->addModelTransformer(new CollectionToArrayTransformer(), true);
$builder->addModelTransformer($this->transformer, true);
$builder->addModelTransformer(new TagArrayToStringTransformer($this->tagRepository, (bool) $options['allow_create']), true);
}
public function configureOptions(OptionsResolver $resolver): void
@@ -48,6 +48,7 @@ final class TagsInputType extends AbstractType
'allow_create' => false,
'label' => 'tag',
]);
$resolver->setAllowedTypes('allow_create', 'bool');
}
public function buildView(FormView $view, FormInterface $form, array $options): void

View File

@@ -28,78 +28,47 @@ use Symfony\Component\OptionsResolver\OptionsResolver;
*/
final class TagsSelectType extends AbstractType
{
public function __construct(private TagRepository $tagRepository)
public function __construct(
private readonly TagRepository $tagRepository
)
{
}
public function buildForm(FormBuilderInterface $builder, array $options): void
{
$builder->addEventListener(FormEvents::PRE_SUBMIT, function (FormEvent $event) use ($options) {
if (!$options['allow_create']) {
return;
}
if (!$options['allow_create']) {
return;
}
$builder->addEventListener(FormEvents::PRE_SUBMIT, function (FormEvent $event) {
/** @var array<string> $tagIds */
$tagIds = $event->getData();
if (!\is_array($tagIds)) {
return;
}
$ids = array_filter($tagIds, function ($tagId) {
$tags = [];
foreach ($tagIds as $tagId) {
$tag = null;
if (is_numeric($tagId)) {
return true;
$tag = $this->tagRepository->find($tagId);
}
return false;
});
// get the current tags and find the new ones that should be created
$tags = $this->tagRepository->findBy(['id' => $ids]);
$foundIds = [];
foreach ($tags as $tag) {
$foundIds[] = (string) $tag->getId();
}
$newData = [];
/** @var array<string> $newNames */
$newNames = [];
foreach ($tagIds as $tag) {
if (!\in_array($tag, $foundIds, true)) {
$newNames[] = $tag;
} else {
$newData[] = $tag;
}
}
// 1. in case someone is using tags like "1234" this can interfere with the ID
// 2. if we would load only visible tags, we would try to create new ones below
// and that would trigger the unique constraint
$tags = $this->tagRepository->findTagsByName($newNames, null);
$foundTagNames = [];
foreach ($tags as $tag) {
$newData[] = (string) $tag->getId();
$foundTagNames[] = $tag->getName();
}
/** @var array<string> $newNamesCreate */
$newNamesCreate = array_udiff($newNames, $foundTagNames, function (mixed $userTag, mixed $existingTag) {
if (!\is_string($userTag) || !\is_string($existingTag)) {
return -1;
if ($tag === null) {
$tag = $this->tagRepository->findTagByName($tagId);
}
if (mb_strtolower($userTag) === mb_strtolower($existingTag)) {
return 0;
if ($tag === null) {
$tag = new Tag();
$tag->setName(mb_substr($tagId, 0, 100));
$this->tagRepository->saveTag($tag);
}
return strcmp($userTag, $existingTag);
});
foreach ($newNamesCreate as $name) {
$tag = new Tag();
$tag->setName(mb_substr($name, 0, 100));
$this->tagRepository->saveTag($tag);
$newData[] = $tag->getId();
$tags[] = $tag->getId();
}
$event->setData($newData);
$event->setData($tags);
}, 1000);
}
@@ -132,6 +101,8 @@ final class TagsSelectType extends AbstractType
return $repo->getQueryBuilderForFormType($query);
};
});
$resolver->setAllowedTypes('allow_create', 'bool');
}
public function buildView(FormView $view, FormInterface $form, array $options): void

View File

@@ -14,6 +14,8 @@ use App\Invoice\InvoiceModelHydrator;
final class InvoiceModelDefaultHydrator implements InvoiceModelHydrator
{
private const DATE_PROCESS_FORMAT = 'Y-m-d h:i:s';
public function hydrate(InvoiceModel $model): array
{
$currency = $model->getCurrency();
@@ -24,7 +26,9 @@ final class InvoiceModelDefaultHydrator implements InvoiceModelHydrator
$values = [
'invoice.due_date' => $formatter->getFormattedDateTime($model->getDueDate()),
'invoice.due_date_process' => $model->getDueDate()->format(self::DATE_PROCESS_FORMAT), // since 2.14
'invoice.date' => $formatter->getFormattedDateTime($model->getInvoiceDate()),
'invoice.date_process' => $model->getInvoiceDate()->format(self::DATE_PROCESS_FORMAT), // since 2.14
'invoice.number' => $model->getInvoiceNumber(),
'invoice.currency' => $currency,
'invoice.language' => $model->getTemplate()->getLanguage(), // since 1.9
@@ -55,11 +59,13 @@ final class InvoiceModelDefaultHydrator implements InvoiceModelHydrator
'query.begin' => '',
'query.begin_day' => '',
'query.begin_process' => null, // since 2.14
'query.begin_month' => '',
'query.begin_month_number' => '',
'query.begin_year' => '',
'query.end' => '', // since 1.9
'query.end_day' => '', // since 1.9
'query.end_process' => null, // since 2.14
'query.end_month' => '', // since 1.9
'query.end_month_number' => '', // since 1.9
'query.end_year' => '', // since 1.9
@@ -82,6 +88,7 @@ final class InvoiceModelDefaultHydrator implements InvoiceModelHydrator
'query.year' => $begin->format('Y'),
// @deprecated - but impossible to delete
'query.begin' => $formatter->getFormattedDateTime($begin),
'query.begin_process' => $begin->format(self::DATE_PROCESS_FORMAT), // since 2.14
'query.begin_day' => $begin->format('d'),
'query.begin_month' => $formatter->getFormattedMonthName($begin),
'query.begin_month_number' => $begin->format('m'),
@@ -93,13 +100,10 @@ final class InvoiceModelDefaultHydrator implements InvoiceModelHydrator
if ($end !== null) {
$values = array_merge($values, [
'query.end' => $formatter->getFormattedDateTime($end),
// since 1.9
'query.end_process' => $end->format(self::DATE_PROCESS_FORMAT), // since 2.14
'query.end_day' => $end->format('d'),
// since 1.9
'query.end_month' => $formatter->getFormattedMonthName($end),
// since 1.9
'query.end_month_number' => $end->format('m'),
// since 1.9
'query.end_year' => $end->format('Y'),
]);
}
@@ -145,7 +149,9 @@ final class InvoiceModelDefaultHydrator implements InvoiceModelHydrator
if ($min !== null && $max !== null) {
$values = array_merge($values, [
'invoice.first' => $formatter->getFormattedDateTime($min->getBegin()),
'invoice.first_process' => $min->getBegin()?->format(self::DATE_PROCESS_FORMAT), // since 2.14
'invoice.last' => $formatter->getFormattedDateTime($max->getEnd()),
'invoice.last_process' => $max->getEnd()?->format(self::DATE_PROCESS_FORMAT), // since 2.14
]);
}

View File

@@ -12,6 +12,7 @@ namespace App\Saml;
use App\Configuration\SamlConfigurationInterface;
use App\Entity\User;
use App\Repository\UserRepository;
use Psr\Log\LoggerInterface;
use Symfony\Component\Security\Core\Exception\AuthenticationException;
use Symfony\Component\Security\Core\Exception\UserNotFoundException;
use Symfony\Component\Security\Core\User\UserProviderInterface;
@@ -24,7 +25,8 @@ final class SamlProvider
public function __construct(
private readonly UserRepository $repository,
private readonly UserProviderInterface $userProvider,
private readonly SamlConfigurationInterface $configuration
private readonly SamlConfigurationInterface $configuration,
private readonly LoggerInterface $logger
) {
}
@@ -37,7 +39,8 @@ final class SamlProvider
/** @var User $user */
$user = $this->userProvider->loadUserByIdentifier($token->getUserIdentifier());
}
} catch (UserNotFoundException $e) {
} catch (UserNotFoundException $ex) {
$this->logger->error($ex->getMessage());
}
try {
@@ -49,6 +52,7 @@ final class SamlProvider
$this->repository->saveUser($user);
} catch (\Exception $ex) {
$this->logger->error($ex->getMessage());
throw new AuthenticationException(
sprintf('Failed creating or hydrating user "%s": %s', $token->getUserIdentifier(), $ex->getMessage())
);
@@ -110,7 +114,8 @@ final class SamlProvider
if (method_exists($user, $setter)) {
$user->$setter($value);
} else {
throw new \RuntimeException('Invalid mapping field given: ' . $field);
// this should never happen, because it is validated when the container is built
throw new \RuntimeException('Invalid SAML mapping field: ' . $field);
}
}
@@ -136,7 +141,7 @@ final class SamlProvider
if ($part[0] === '$') {
$key = substr($part, 1);
if (!\array_key_exists($key, $attributes)) {
throw new \RuntimeException('Missing user attribute: ' . $key);
throw new \RuntimeException('Missing SAML attribute in response: ' . $key);
}
if (\is_array($attributes[$key]) && isset($attributes[$key][0])) {

View File

@@ -69,7 +69,7 @@ final class ProjectVoter extends Voter
}
// those cannot be assigned to teams
if (\in_array($attribute, ['create', 'delete'])) {
if (\in_array($attribute, ['create', 'delete'], true)) {
return false;
}