Skip to content
Permalink
Browse files
improve permissison handling in invoice screen (#2965)
  • Loading branch information
kevinpapst committed Nov 21, 2021
1 parent 76e0944 commit ff9acab0fc81f0e9490462739ef15fe4ab028ea5
@@ -33,46 +33,22 @@ export default class KimaiForm extends KimaiPlugin {
this.getContainer().getPlugin('date-range-picker').destroyDateRangePicker(formSelector);
}

getFormData(form) {
/**
* @param {HTMLFormElement} form
* @param {Object} overwrites
* @returns {string}
*/
convertFormDataToQueryString(form, overwrites = {})
{
let serialized = [];
let data = new FormData(form);

// Loop through each field in the form
for (let i = 0; i < form.elements.length; i++) {

let field = form.elements[i];

// Don't serialize a couple of field types (button and submit are important to exclude, eg. invoice preview would fail otherwise)
if (!field.name || field.disabled || field.type === 'file' || field.type === 'reset' || field.type === 'submit' || field.type === 'button') {
continue;
}

// If a multi-select, get all selections
if (field.type === 'select-multiple') {
for (var n = 0; n < field.options.length; n++) {
if (!field.options[n].selected) {
continue;
}
serialized.push({
name: field.name,
value: field.options[n].value
});
}
} else if ((field.type !== 'checkbox' && field.type !== 'radio') || field.checked) {
serialized.push({
name: field.name,
value: field.value
});
}
for (const key in overwrites) {
data.set(key, overwrites[key]);
}

return serialized;
}

convertFormDataToQueryString(formData) {
let serialized = [];

for (let row of formData) {
serialized.push(encodeURIComponent(row.name) + "=" + encodeURIComponent(row.value));
for (let row of data) {
serialized.push(encodeURIComponent(row[0]) + "=" + encodeURIComponent(row[1]));
}

return serialized.join('&');

Large diffs are not rendered by default.

File renamed without changes.

This file was deleted.

@@ -3,7 +3,7 @@
"app": {
"js": [
"build/runtime.b8e7bb04.js",
"build/app.920ba43e.js"
"build/app.8609d161.js"
],
"css": [
"build/app.3bc2b4d9.css"
@@ -1,6 +1,6 @@
{
"build/app.css": "build/app.3bc2b4d9.css",
"build/app.js": "build/app.920ba43e.js",
"build/app.js": "build/app.8609d161.js",
"build/invoice.css": "build/invoice.ff32661a.css",
"build/invoice.js": "build/invoice.19f36eca.js",
"build/invoice-pdf.css": "build/invoice-pdf.9a7468ef.css",
@@ -48,21 +48,9 @@
*/
final class InvoiceController extends AbstractController
{
/**
* @var ServiceInvoice
*/
private $service;
/**
* @var InvoiceTemplateRepository
*/
private $templateRepository;
/**
* @var InvoiceRepository
*/
private $invoiceRepository;
/**
* @var EventDispatcherInterface
*/
private $dispatcher;

public function __construct(ServiceInvoice $service, InvoiceTemplateRepository $templateRepository, InvoiceRepository $invoiceRepository, EventDispatcherInterface $dispatcher)
@@ -130,10 +118,11 @@ public function indexAction(Request $request, SystemConfiguration $configuration
}

/**
* @Route(path="/preview/{customer}/{template}", name="invoice_preview", methods={"GET"})
* @Security("is_granted('view_invoice')")
* @Route(path="/preview/{customer}", name="invoice_preview", methods={"GET"})
* @Security("is_granted('access', customer)")
* @Security("is_granted('create_invoice')")
*/
public function previewAction(Customer $customer, InvoiceTemplate $template, Request $request, SystemConfiguration $configuration): Response
public function previewAction(Customer $customer, Request $request, SystemConfiguration $configuration): Response
{
if (!$this->templateRepository->hasTemplate()) {
return $this->redirectToRoute('invoice');
@@ -143,10 +132,8 @@ public function previewAction(Customer $customer, InvoiceTemplate $template, Req
$form = $this->getToolbarForm($query, $configuration->find('invoice.simple_form'));
$form->submit($request->query->all(), false);

if ($form->isValid() && $this->isGranted('create_invoice')) {
if ($form->isValid()) {
try {
$query->setTemplate($template);
$query->setCustomers([$customer]);
$model = $this->service->createModel($query);

return $this->service->renderInvoiceWithModel($model, $this->dispatcher);
@@ -156,12 +143,15 @@ public function previewAction(Customer $customer, InvoiceTemplate $template, Req
}
}

$this->flashFormError($form);

return $this->redirectToRoute('invoice');
}

/**
* @Route(path="/save-invoice/{customer}/{template}", name="invoice_create", methods={"GET"})
* @Security("is_granted('view_invoice')")
* @Security("is_granted('access', customer)")
* @Security("is_granted('create_invoice')")
*/
public function createInvoiceAction(Customer $customer, InvoiceTemplate $template, Request $request, SystemConfiguration $configuration): Response
{
@@ -173,62 +163,22 @@ public function createInvoiceAction(Customer $customer, InvoiceTemplate $templat
$form = $this->getToolbarForm($query, $configuration->find('invoice.simple_form'));
$form->submit($request->query->all(), false);

if ($form->isValid() && $this->isGranted('create_invoice')) {
if ($form->isValid()) {
$query->setTemplate($template);
$query->setCustomers([$customer]);

return $this->renderInvoice($query, $request);
}

return $this->redirectToRoute('invoice');
}

private function getDefaultQuery(): InvoiceQuery
{
$factory = $this->getDateTimeFactory();
$begin = $factory->getStartOfMonth();
$end = $factory->getEndOfMonth();

$query = new InvoiceQuery();
$query->setBegin($begin);
$query->setEnd($end);
// limit access to data from teams
$query->setCurrentUser($this->getUser());

if (!$this->isGranted('view_other_timesheet')) {
// limit access to own data
$query->setUser($this->getUser());
}

return $query;
}

private function renderInvoice(InvoiceQuery $query, Request $request)
{
// use the current request locale as fallback, if no translation was configured
if (null !== $query->getTemplate() && null === $query->getTemplate()->getLanguage()) {
$query->getTemplate()->setLanguage($request->getLocale());
}

try {
$invoices = $this->service->createInvoices($query, $this->dispatcher);

$this->flashSuccess('action.update.success');

if (\count($invoices) === 1) {
return $this->redirectToRoute('admin_invoice_list', ['id' => $invoices[0]->getId()]);
}

return $this->redirectToRoute('admin_invoice_list');
} catch (Exception $ex) {
$this->flashUpdateException($ex);
}
$this->flashFormError($form);

return $this->redirectToRoute('invoice');
}

/**
* @Route(path="/change-status/{id}/{status}", name="admin_invoice_status", methods={"GET", "POST"})
* @Security("is_granted('access', invoice.getCustomer())")
* @Security("is_granted('create_invoice')")
*/
public function changeStatusAction(Invoice $invoice, string $status, Request $request): Response
{
@@ -256,6 +206,8 @@ public function changeStatusAction(Invoice $invoice, string $status, Request $re

/**
* @Route(path="/delete/{id}/{token}", name="admin_invoice_delete", methods={"GET"})
* @Security("is_granted('access', invoice.getCustomer())")
* @Security("is_granted('delete_invoice')")
*/
public function deleteInvoiceAction(Invoice $invoice, string $token, CsrfTokenManagerInterface $csrfTokenManager): Response
{
@@ -279,6 +231,8 @@ public function deleteInvoiceAction(Invoice $invoice, string $token, CsrfTokenMa

/**
* @Route(path="/download/{id}", name="admin_invoice_download", methods={"GET"})
* @Security("is_granted('access', invoice.getCustomer())")
* @Security("is_granted('create_invoice')")
*/
public function downloadAction(Invoice $invoice): Response
{
@@ -295,6 +249,7 @@ public function downloadAction(Invoice $invoice): Response

/**
* @Route(path="/show/{page}", defaults={"page": 1}, requirements={"page": "[1-9]\d*"}, name="admin_invoice_list", methods={"GET"})
* @Security("is_granted('view_invoice')")
*/
public function showInvoicesAction(Request $request, int $page): Response
{
@@ -325,6 +280,7 @@ public function showInvoicesAction(Request $request, int $page): Response

/**
* @Route(path="/export", name="invoice_export", methods={"GET"})
* @Security("is_granted('view_invoice')")
*/
public function exportAction(Request $request, AnnotatedObjectExporter $exporter)
{
@@ -474,6 +430,60 @@ public function deleteTemplate(InvoiceTemplate $template, string $token, CsrfTok
return $this->redirectToRoute('admin_invoice_template');
}

private function getDefaultQuery(): InvoiceQuery
{
$factory = $this->getDateTimeFactory();
$begin = $factory->getStartOfMonth();
$end = $factory->getEndOfMonth();

$query = new InvoiceQuery();
$query->setBegin($begin);
$query->setEnd($end);
// limit access to data from teams
$query->setCurrentUser($this->getUser());

if (!$this->isGranted('view_other_timesheet')) {
// limit access to own data
$query->setUser($this->getUser());
}

return $query;
}

private function renderInvoice(InvoiceQuery $query, Request $request)
{
// use the current request locale as fallback, if no translation was configured
if (null !== $query->getTemplate() && null === $query->getTemplate()->getLanguage()) {
$query->getTemplate()->setLanguage($request->getLocale());
}

try {
$invoices = $this->service->createInvoices($query, $this->dispatcher);

$this->flashSuccess('action.update.success');

if (\count($invoices) === 1) {
return $this->redirectToRoute('admin_invoice_list', ['id' => $invoices[0]->getId()]);
}

return $this->redirectToRoute('admin_invoice_list');
} catch (Exception $ex) {
$this->flashUpdateException($ex);
}

return $this->redirectToRoute('invoice');
}

private function flashFormError(FormInterface $form): void
{
$err = '';
foreach ($form->getErrors(true, true) as $error) {
$err .= PHP_EOL . '[' . $error->getOrigin()->getName() . '] ' . $error->getMessage();
}

$this->flashError('action.update.error', ['%reason%' => $err]);
}

private function renderTemplateForm(InvoiceTemplate $template, Request $request): Response
{
$editForm = $this->createEditForm($template);
@@ -241,7 +241,8 @@ public function getQueryBuilderForFormType(CustomerFormTypeQuery $query): QueryB

$outerQuery = $qb->expr()->orX();

if ($query->hasCustomers()) {
// this is a risk, as a user can manipulate the query and inject IDs that would be hidden otherwise
if ($query->isAllowCustomerPreselect() && $query->hasCustomers()) {
$outerQuery->add($qb->expr()->in('c.id', ':customer'));
$qb->setParameter('customer', $query->getCustomers());
}
@@ -20,6 +20,7 @@ final class CustomerFormTypeQuery extends BaseFormTypeQuery
* @var Customer|null
*/
private $customerToIgnore;
private $allowCustomerPreselect = false;

/**
* @param Customer|int|null $customer
@@ -34,6 +35,16 @@ public function __construct($customer = null)
}
}

public function isAllowCustomerPreselect(): bool
{
return $this->allowCustomerPreselect;
}

public function setAllowCustomerPreselect(bool $allowCustomerPreselect): void
{
$this->allowCustomerPreselect = $allowCustomerPreselect;
}

/**
* @return Customer|null
*/
@@ -34,6 +34,7 @@ final class CustomerVoter extends Voter
'comments',
'comments_create',
'details',
'access',
];

private $permissionManager;
@@ -75,6 +76,22 @@ protected function voteOnAttribute($attribute, $subject, TokenInterface $token)
return false;
}

// this is a virtual permission, only meant to be used by developer
// it checks if access to the given customer is potentially possible
if ($attribute === 'access') {
if ($subject->getTeams()->count() === 0) {
return true;
}

foreach ($subject->getTeams() as $team) {
if ($user->isInTeam($team)) {
return true;
}
}

return false;
}

if ($this->permissionManager->hasRolePermission($user, $attribute . '_customer')) {
return true;
}
@@ -29,7 +29,6 @@ final class UserVoter extends Voter
'preferences',
'api-token',
'hourly-rate',
// teams_own_profile could be merged with view_team_member
'view_team_member',
];

0 comments on commit ff9acab

Please sign in to comment.