Completed
Push — master ( a7cf71...06c6ea )
by Craig
06:43
created

AbstractEditHandler::getDefaultMessage()   C

Complexity

Conditions 7
Paths 7

Size

Total Lines 29
Code Lines 22

Duplication

Lines 21
Ratio 72.41 %

Importance

Changes 0
Metric Value
cc 7
eloc 22
nc 7
nop 2
dl 21
loc 29
rs 6.7272
c 0
b 0
f 0
1
<?php
2
/**
3
 * Routes.
4
 *
5
 * @copyright Zikula contributors (Zikula)
6
 * @license http://www.gnu.org/licenses/lgpl.html GNU Lesser General Public License
7
 * @author Zikula contributors <[email protected]>.
8
 * @link http://www.zikula.org
9
 * @link http://zikula.org
10
 * @version Generated by ModuleStudio 0.7.0 (http://modulestudio.de).
11
 */
12
13
namespace Zikula\RoutesModule\Form\Handler\Common\Base;
14
15
use Symfony\Component\DependencyInjection\ContainerBuilder;
16
use Symfony\Component\Form\AbstractType;
17
use Symfony\Component\HttpFoundation\RedirectResponse;
18
use Symfony\Component\HttpFoundation\Request;
19
use Symfony\Component\HttpFoundation\RequestStack;
20
use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
21
use Symfony\Component\Routing\RouterInterface;
22
use Symfony\Component\Security\Core\Exception\AccessDeniedException;
23
use Zikula\Common\Translator\TranslatorInterface;
24
use Zikula\Common\Translator\TranslatorTrait;
25
use Zikula\Core\Doctrine\EntityAccess;
26
use Zikula\Core\RouteUrl;
27
use ModUtil;
28
use RuntimeException;
29
use UserUtil;
30
31
/**
32
 * This handler class handles the page events of editing forms.
33
 * It collects common functionality required by different object types.
34
 */
35
abstract class AbstractEditHandler
36
{
37
    use TranslatorTrait;
38
39
    /**
40
     * Name of treated object type.
41
     *
42
     * @var string
43
     */
44
    protected $objectType;
45
46
    /**
47
     * Name of treated object type starting with upper case.
48
     *
49
     * @var string
50
     */
51
    protected $objectTypeCapital;
52
53
    /**
54
     * Lower case version.
55
     *
56
     * @var string
57
     */
58
    protected $objectTypeLower;
59
60
    /**
61
     * Permission component based on object type.
62
     *
63
     * @var string
64
     */
65
    protected $permissionComponent;
66
67
    /**
68
     * Reference to treated entity instance.
69
     *
70
     * @var EntityAccess
71
     */
72
    protected $entityRef = null;
73
74
    /**
75
     * List of identifier names.
76
     *
77
     * @var array
78
     */
79
    protected $idFields = [];
80
81
    /**
82
     * List of identifiers of treated entity.
83
     *
84
     * @var array
85
     */
86
    protected $idValues = [];
87
88
    /**
89
     * Code defining the redirect goal after command handling.
90
     *
91
     * @var string
92
     */
93
    protected $returnTo = null;
94
95
    /**
96
     * Whether a create action is going to be repeated or not.
97
     *
98
     * @var boolean
99
     */
100
    protected $repeatCreateAction = false;
101
102
    /**
103
     * Url of current form with all parameters for multiple creations.
104
     *
105
     * @var string
106
     */
107
    protected $repeatReturnUrl = null;
108
109
    /**
110
     * Whether an existing item is used as template for a new one.
111
     *
112
     * @var boolean
113
     */
114
    protected $hasTemplateId = false;
115
116
    /**
117
     * Whether the PageLock extension is used for this entity type or not.
118
     *
119
     * @var boolean
120
     */
121
    protected $hasPageLockSupport = false;
122
123
    /**
124
     * @var ContainerBuilder
125
     */
126
    protected $container;
127
128
    /**
129
     * The current request.
130
     *
131
     * @var Request
132
     */
133
    protected $request = null;
134
135
    /**
136
     * The router.
137
     *
138
     * @var RouterInterface
139
     */
140
    protected $router = null;
141
142
    /**
143
     * The handled form type.
144
     *
145
     * @var AbstractType
146
     */
147
    protected $form = null;
148
149
    /**
150
     * Template parameters.
151
     *
152
     * @var array
153
     */
154
    protected $templateParameters = [];
155
156
    /**
157
     * Constructor.
158
     *
159
     * @param ContainerBuilder    $container    ContainerBuilder service instance
160
     * @param TranslatorInterface $translator   Translator service instance
161
     * @param RequestStack        $requestStack RequestStack service instance
162
     * @param RouterInterface     $router       Router service instance
163
     */
164
    public function __construct(ContainerBuilder $container, TranslatorInterface $translator, RequestStack $requestStack, RouterInterface $router)
165
    {
166
        $this->container = $container;
167
        $this->setTranslator($translator);
168
        $this->request = $requestStack->getCurrentRequest();
169
        $this->router = $router;
170
    }
171
172
    /**
173
     * Sets the translator.
174
     *
175
     * @param TranslatorInterface $translator Translator service instance
176
     */
177
    public function setTranslator(/*TranslatorInterface */$translator)
178
    {
179
        $this->translator = $translator;
0 ignored issues
show
Documentation Bug introduced by
$translator is of type object<Zikula\Common\Tra...or\TranslatorInterface>, but the property $translator was declared to be of type object<Zikula\Common\Translator\Translator>. Are you sure that you always receive this specific sub-class here, or does it make sense to add an instanceof check?

Our type inference engine has found a suspicous assignment of a value to a property. This check raises an issue when a value that can be of a given class or a super-class is assigned to a property that is type hinted more strictly.

Either this assignment is in error or an instanceof check should be added for that assignment.

class Alien {}

class Dalek extends Alien {}

class Plot
{
    /** @var  Dalek */
    public $villain;
}

$alien = new Alien();
$plot = new Plot();
if ($alien instanceof Dalek) {
    $plot->villain = $alien;
}
Loading history...
180
    }
181
182
    /**
183
     * Initialise form handler.
184
     *
185
     * This method takes care of all necessary initialisation of our data and form states.
186
     *
187
     * @param array $templateParameters List of preassigned template variables
188
     *
189
     * @return boolean False in case of initialisation errors, otherwise true
190
     *
191
     * @throws NotFoundHttpException Thrown if item to be edited isn't found
192
     * @throws RuntimeException      Thrown if the workflow actions can not be determined
193
     */
194
    public function processForm(array $templateParameters)
195
    {
196
        $this->templateParameters = $templateParameters;
197
        $this->templateParameters['inlineUsage'] = UserUtil::getTheme() == 'ZikulaPrinterTheme' ? true : false;
198
    
199
    
200
        // initialise redirect goal
201
        $this->returnTo = $this->request->query->get('returnTo', null);
202
        if (null === $this->returnTo) {
203
            // default to referer
204
            if ($this->request->getSession()->has('referer')) {
205
                $this->returnTo = $this->request->getSession()->get('referer');
206
            } elseif ($this->request->headers->has('referer')) {
207
                $this->returnTo = $this->request->headers->get('referer');
0 ignored issues
show
Documentation Bug introduced by
It seems like $this->request->headers->get('referer') can also be of type array. However, the property $returnTo is declared as type string. Maybe add an additional type check?

Our type inference engine has found a suspicous assignment of a value to a property. This check raises an issue when a value that can be of a mixed type is assigned to a property that is type hinted more strictly.

For example, imagine you have a variable $accountId that can either hold an Id object or false (if there is no account id yet). Your code now assigns that value to the id property of an instance of the Account class. This class holds a proper account, so the id value must no longer be false.

Either this assignment is in error or a type check should be added for that assignment.

class Id
{
    public $id;

    public function __construct($id)
    {
        $this->id = $id;
    }

}

class Account
{
    /** @var  Id $id */
    public $id;
}

$account_id = false;

if (starsAreRight()) {
    $account_id = new Id(42);
}

$account = new Account();
if ($account instanceof Id)
{
    $account->id = $account_id;
}
Loading history...
208
                $this->request->getSession()->set('referer', $this->returnTo);
209
            } elseif ($this->request->server->has('HTTP_REFERER')) {
210
                $this->returnTo = $this->request->server->get('HTTP_REFERER');
211
                $this->request->getSession()->set('referer', $this->returnTo);
212
            }
213
        }
214
        // store current uri for repeated creations
215
        $this->repeatReturnUrl = $this->request->getSchemeAndHttpHost() . $this->request->getBasePath() . $this->request->getPathInfo();
216
    
217
        $this->permissionComponent = 'ZikulaRoutesModule:' . $this->objectTypeCapital . ':';
218
    
219
        $selectionHelper = $this->container->get('zikula_routes_module.selection_helper');
220
        $this->idFields = $selectionHelper->getIdFields($this->objectType);
221
    
222
        // retrieve identifier of the object we wish to view
223
        $controllerHelper = $this->container->get('zikula_routes_module.controller_helper');
224
    
225
        $this->idValues = $controllerHelper->retrieveIdentifier($this->request, [], $this->objectType, $this->idFields);
226
        $hasIdentifier = $controllerHelper->isValidIdentifier($this->idValues);
227
    
228
        $entity = null;
0 ignored issues
show
Unused Code introduced by
$entity is not used, you could remove the assignment.

This check looks for variable assignements that are either overwritten by other assignments or where the variable is not used subsequently.

$myVar = 'Value';
$higher = false;

if (rand(1, 6) > 3) {
    $higher = true;
} else {
    $higher = false;
}

Both the $myVar assignment in line 1 and the $higher assignment in line 2 are dead. The first because $myVar is never used and the second because $higher is always overwritten for every possible time line.

Loading history...
229
        $this->templateParameters['mode'] = $hasIdentifier ? 'edit' : 'create';
230
    
231
        $permissionApi = $this->container->get('zikula_permissions_module.api.permission');
232
    
233
        if ($this->templateParameters['mode'] == 'edit') {
234
            if (!$permissionApi->hasPermission($this->permissionComponent, $this->createCompositeIdentifier() . '::', ACCESS_EDIT)) {
235
                throw new AccessDeniedException();
236
            }
237
    
238
            $entity = $this->initEntityForEditing();
239
            if (!is_object($entity)) {
240
                return false;
241
            }
242
    
243
            if (true === $this->hasPageLockSupport && \ModUtil::available('ZikulaPageLockModule')) {
244
                // try to guarantee that only one person at a time can be editing this entity
245
                $lockingApi = $this->container->get('zikula_pagelock_module.api.locking');
246
                $lockName = 'ZikulaRoutesModule' . $this->objectTypeCapital . $this->createCompositeIdentifier();
247
                $lockingApi->addLock($lockName, $this->getRedirectUrl(null));
0 ignored issues
show
Bug introduced by
It seems like you code against a specific sub-type and not the parent class Zikula\RoutesModule\Form...ase\AbstractEditHandler as the method getRedirectUrl() does only exist in the following sub-classes of Zikula\RoutesModule\Form...ase\AbstractEditHandler: Zikula\RoutesModule\Form...ase\AbstractEditHandler, Zikula\RoutesModule\Form\Handler\Route\EditHandler. Maybe you want to instanceof check for one of these explicitly?

Let’s take a look at an example:

abstract class User
{
    /** @return string */
    abstract public function getPassword();
}

class MyUser extends User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}

In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different sub-classes of User which does not have a getDisplayName() method, the code will break.

Available Fixes

  1. Change the type-hint for the parameter:

    class AuthSystem
    {
        public function authenticate(MyUser $user) { /* ... */ }
    }
    
  2. Add an additional type-check:

    class AuthSystem
    {
        public function authenticate(User $user)
        {
            if ($user instanceof MyUser) {
                $this->logger->info(/** ... */);
            }
    
            // or alternatively
            if ( ! $user instanceof MyUser) {
                throw new \LogicException(
                    '$user must be an instance of MyUser, '
                   .'other instances are not supported.'
                );
            }
    
        }
    }
    
Note: PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case.
  1. Add the method to the parent class:

    abstract class User
    {
        /** @return string */
        abstract public function getPassword();
    
        /** @return string */
        abstract public function getDisplayName();
    }
    
Loading history...
248
            }
249
        } else {
250
            if (!$permissionApi->hasPermission($this->permissionComponent, '::', ACCESS_EDIT)) {
251
                throw new AccessDeniedException();
252
            }
253
    
254
            $entity = $this->initEntityForCreation();
255
        }
256
    
257
        // save entity reference for later reuse
258
        $this->entityRef = $entity;
259
    
260
    
261
        $workflowHelper = $this->container->get('zikula_routes_module.workflow_helper');
262
        $actions = $workflowHelper->getActionsForObject($entity);
263
        if (false === $actions || !is_array($actions)) {
264
            $this->request->getSession()->getFlashBag()->add('error', $this->__('Error! Could not determine workflow actions.'));
265
            $logger = $this->container->get('logger');
266
            $logArgs = ['app' => 'ZikulaRoutesModule', 'user' => $this->container->get('zikula_users_module.current_user')->get('uname'), 'entity' => $this->objectType, 'id' => $entity->createCompositeIdentifier()];
267
            $logger->error('{app}: User {user} tried to edit the {entity} with id {id}, but failed to determine available workflow actions.', $logArgs);
268
            throw new \RuntimeException($this->__('Error! Could not determine workflow actions.'));
269
        }
270
    
271
        $this->templateParameters['actions'] = $actions;
272
    
273
        $this->form = $this->createForm();
0 ignored issues
show
Bug introduced by
Are you sure the assignment to $this->form is correct as $this->createForm() (which targets Zikula\RoutesModule\Form...itHandler::createForm()) seems to always return null.

This check looks for function or method calls that always return null and whose return value is assigned to a variable.

class A
{
    function getObject()
    {
        return null;
    }

}

$a = new A();
$object = $a->getObject();

The method getObject() can return nothing but null, so it makes no sense to assign that value to a variable.

The reason is most likely that a function or method is imcomplete or has been reduced for debug purposes.

Loading history...
274
        if (!is_object($this->form)) {
275
            return false;
276
        }
277
    
278
        // handle form request and check validity constraints of edited entity
279
        if ($this->form->handleRequest($this->request) && $this->form->isSubmitted()) {
280
            if ($this->form->isValid()) {
281
                $result = $this->handleCommand();
282
                if (false === $result) {
283
                    $this->templateParameters['form'] = $this->form->createView();
284
                }
285
    
286
                return $result;
287
            }
288
            if ($this->form->get('cancel')->isClicked()) {
289
                return new RedirectResponse($this->getRedirectUrl(['commandName' => 'cancel']), 302);
0 ignored issues
show
Bug introduced by
It seems like you code against a specific sub-type and not the parent class Zikula\RoutesModule\Form...ase\AbstractEditHandler as the method getRedirectUrl() does only exist in the following sub-classes of Zikula\RoutesModule\Form...ase\AbstractEditHandler: Zikula\RoutesModule\Form...ase\AbstractEditHandler, Zikula\RoutesModule\Form\Handler\Route\EditHandler. Maybe you want to instanceof check for one of these explicitly?

Let’s take a look at an example:

abstract class User
{
    /** @return string */
    abstract public function getPassword();
}

class MyUser extends User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}

In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different sub-classes of User which does not have a getDisplayName() method, the code will break.

Available Fixes

  1. Change the type-hint for the parameter:

    class AuthSystem
    {
        public function authenticate(MyUser $user) { /* ... */ }
    }
    
  2. Add an additional type-check:

    class AuthSystem
    {
        public function authenticate(User $user)
        {
            if ($user instanceof MyUser) {
                $this->logger->info(/** ... */);
            }
    
            // or alternatively
            if ( ! $user instanceof MyUser) {
                throw new \LogicException(
                    '$user must be an instance of MyUser, '
                   .'other instances are not supported.'
                );
            }
    
        }
    }
    
Note: PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case.
  1. Add the method to the parent class:

    abstract class User
    {
        /** @return string */
        abstract public function getPassword();
    
        /** @return string */
        abstract public function getDisplayName();
    }
    
Loading history...
Security Cross-Site Scripting introduced by
$this->getRedirectUrl(ar...mandName' => 'cancel')) can contain request data and is used in output context(s) leading to a potential security vulnerability.

3 paths for user data to reach this point

  1. Path: HeaderBag::get() returns tainted data, and AbstractEditHandler::$returnTo is assigned in src/system/RoutesModule/Form/Handler/Common/Base/AbstractEditHandler.php on line 207
  1. HeaderBag::get() returns tainted data, and AbstractEditHandler::$returnTo is assigned
    in src/system/RoutesModule/Form/Handler/Common/Base/AbstractEditHandler.php on line 207
  2. Tainted property AbstractEditHandler::$returnTo is read
    in src/system/RoutesModule/Form/Handler/Route/Base/AbstractEditHandler.php on line 118
  3. AbstractEditHandler::getDefaultReturnUrl() returns tainted data
    in src/system/RoutesModule/Form/Handler/Route/Base/AbstractEditHandler.php on line 252
  4. AbstractEditHandler::getRedirectUrl() returns tainted data
    in src/system/RoutesModule/Form/Handler/Common/Base/AbstractEditHandler.php on line 289
  2. Path: ParameterBag::get() returns tainted data, and AbstractEditHandler::$returnTo is assigned in src/system/RoutesModule/Form/Handler/Common/Base/AbstractEditHandler.php on line 201
  1. ParameterBag::get() returns tainted data, and AbstractEditHandler::$returnTo is assigned
    in src/system/RoutesModule/Form/Handler/Common/Base/AbstractEditHandler.php on line 201
  2. Tainted property AbstractEditHandler::$returnTo is read
    in src/system/RoutesModule/Form/Handler/Route/Base/AbstractEditHandler.php on line 118
  3. AbstractEditHandler::getDefaultReturnUrl() returns tainted data
    in src/system/RoutesModule/Form/Handler/Route/Base/AbstractEditHandler.php on line 252
  4. AbstractEditHandler::getRedirectUrl() returns tainted data
    in src/system/RoutesModule/Form/Handler/Common/Base/AbstractEditHandler.php on line 289
  3. Path: Session::get() returns tainted data, and AbstractEditHandler::$returnTo is assigned in src/system/RoutesModule/Form/Handler/Common/Base/AbstractEditHandler.php on line 205
  1. Session::get() returns tainted data, and AbstractEditHandler::$returnTo is assigned
    in src/system/RoutesModule/Form/Handler/Common/Base/AbstractEditHandler.php on line 205
  2. Tainted property AbstractEditHandler::$returnTo is read
    in src/system/RoutesModule/Form/Handler/Route/Base/AbstractEditHandler.php on line 118
  3. AbstractEditHandler::getDefaultReturnUrl() returns tainted data
    in src/system/RoutesModule/Form/Handler/Route/Base/AbstractEditHandler.php on line 252
  4. AbstractEditHandler::getRedirectUrl() returns tainted data
    in src/system/RoutesModule/Form/Handler/Common/Base/AbstractEditHandler.php on line 289

Used in output context

  1. RedirectResponse::__construct() uses RedirectResponse::setTargetUrl() ($url)
    in vendor/src/Symfony/Component/HttpFoundation/RedirectResponse.php on line 39
  2. RedirectResponse::setTargetUrl() uses Response::setContent() ($content)
    in vendor/src/Symfony/Component/HttpFoundation/RedirectResponse.php on line 86
  3. Response::setContent() uses property Response::$content for writing
    in vendor/src/Symfony/Component/HttpFoundation/Response.php on line 447
  4. Property Response::$content is used in echo
    in vendor/src/Symfony/Component/HttpFoundation/Response.php on line 406

Preventing Cross-Site-Scripting Attacks

Cross-Site-Scripting allows an attacker to inject malicious code into your website - in particular Javascript code, and have that code executed with the privileges of a visiting user. This can be used to obtain data, or perform actions on behalf of that visiting user.

In order to prevent this, make sure to escape all user-provided data:

// for HTML
$sanitized = htmlentities($tainted, ENT_QUOTES);

// for URLs
$sanitized = urlencode($tainted);

General Strategies to prevent injection

In general, it is advisable to prevent any user-data to reach this point. This can be done by white-listing certain values:

if ( ! in_array($value, array('this-is-allowed', 'and-this-too'), true)) {
    throw new \InvalidArgumentException('This input is not allowed.');
}

For numeric data, we recommend to explicitly cast the data:

$sanitized = (integer) $tainted;
Loading history...
290
            }
291
        }
292
    
293
        $this->templateParameters['form'] = $this->form->createView();
294
    
295
        // everything okay, no initialisation errors occured
296
        return true;
297
    }
298
    
299
    /**
300
     * Creates the form type.
301
     */
302
    protected function createForm()
303
    {
304
        // to be customised in sub classes
305
        return null;
306
    }
307
    
308
    /**
309
     * Returns the template parameters.
310
     *
311
     * @return array
312
     */
313
    public function getTemplateParameters()
314
    {
315
        return $this->templateParameters;
316
    }
317
    
318
    /**
319
     * Create concatenated identifier string (for composite keys).
320
     *
321
     * @return String concatenated identifiers
322
     */
323
    protected function createCompositeIdentifier()
324
    {
325
        $itemId = '';
326
        foreach ($this->idFields as $idField) {
327
            if (!empty($itemId)) {
328
                $itemId .= '_';
329
            }
330
            $itemId .= $this->idValues[$idField];
331
        }
332
    
333
        return $itemId;
334
    }
335
    
336
    /**
337
     * Initialise existing entity for editing.
338
     *
339
     * @return EntityAccess|null Desired entity instance or null
340
     *
341
     * @throws NotFoundHttpException Thrown if item to be edited isn't found
342
     */
343
    protected function initEntityForEditing()
344
    {
345
        $selectionHelper = $this->container->get('zikula_routes_module.selection_helper');
346
        $entity = $selectionHelper->getEntity($this->objectType, $this->idValues);
347
        if (null === $entity) {
348
            throw new NotFoundHttpException($this->__('No such item.'));
349
        }
350
    
351
        $entity->initWorkflow();
352
    
353
        return $entity;
354
    }
355
    
356
    /**
357
     * Initialise new entity for creation.
358
     *
359
     * @return EntityAccess|null Desired entity instance or null
360
     *
361
     * @throws NotFoundHttpException Thrown if item to be cloned isn't found
362
     */
363
    protected function initEntityForCreation()
364
    {
365
        $this->hasTemplateId = false;
366
        $templateId = $this->request->query->get('astemplate', '');
367
        $entity = null;
368
    
369
        if (!empty($templateId)) {
370
            $templateIdValueParts = explode('_', $templateId);
371
            $this->hasTemplateId = count($templateIdValueParts) == count($this->idFields);
372
    
373
            if (true === $this->hasTemplateId) {
374
                $templateIdValues = [];
375
                $i = 0;
376
                foreach ($this->idFields as $idField) {
377
                    $templateIdValues[$idField] = $templateIdValueParts[$i];
378
                    $i++;
379
                }
380
                // reuse existing entity
381
                $selectionHelper = $this->container->get('zikula_routes_module.selection_helper');
382
                $entityT = $selectionHelper->getEntity($this->objectType, $templateIdValues);
383
                if (null === $entityT) {
384
                    throw new NotFoundHttpException($this->__('No such item.'));
385
                }
386
                $entity = clone $entityT;
387
            }
388
        }
389
    
390
        if (is_null($entity)) {
391
            $factory = $this->container->get('zikula_routes_module.' . $this->objectType . '_factory');
392
            $createMethod = 'create' . ucfirst($this->objectType);
393
            $entity = $factory->$createMethod();
394
        }
395
    
396
        return $entity;
397
    }
398
399
    /**
400
     * Get list of allowed redirect codes.
401
     *
402
     * @return array list of possible redirect codes
403
     */
404
    protected function getRedirectCodes()
405
    {
406
        $codes = [];
407
    
408
        // index page of admin area
409
        $codes[] = 'admin';
410
        // admin list of entities
411
        $codes[] = 'adminView';
412
        // admin display page of treated entity
413
        $codes[] = 'adminDisplay';
414
        // index page of ajax area
415
        $codes[] = 'ajax';
416
        // index page of user area
417
        $codes[] = 'user';
418
        // user list of entities
419
        $codes[] = 'userView';
420
        // user display page of treated entity
421
        $codes[] = 'userDisplay';
422
    
423
        return $codes;
424
    }
425
426
    /**
427
     * Command event handler.
428
     *
429
     * @param array $args List of arguments
430
     *
431
     * @return mixed Redirect or false on errors
432
     */
433
    public function handleCommand($args = [])
434
    {
435
        // build $args for BC (e.g. used by redirect handling)
436 View Code Duplication
        foreach ($this->templateParameters['actions'] as $action) {
0 ignored issues
show
Duplication introduced by
This code seems to be duplicated across your project.

Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation.

You can also find more detailed suggestions in the “Code” section of your repository.

Loading history...
437
            if ($this->form->get($action['id'])->isClicked()) {
0 ignored issues
show
Bug introduced by
The method get() does not seem to exist on object<Symfony\Component\Form\AbstractType>.

This check looks for calls to methods that do not seem to exist on a given type. It looks for the method on the type itself as well as in inherited classes or implemented interfaces.

This is most likely a typographical error or the method has been renamed.

Loading history...
438
                $args['commandName'] = $action['id'];
439
            }
440
        }
441
    
442
        $action = $args['commandName'];
443
        $isRegularAction = !in_array($action, ['delete', 'cancel']);
444
    
445
        if ($isRegularAction || $action == 'delete') {
446
            $this->fetchInputData($args);
447
        }
448
    
449
        // get treated entity reference from persisted member var
450
        $entity = $this->entityRef;
0 ignored issues
show
Unused Code introduced by
$entity is not used, you could remove the assignment.

This check looks for variable assignements that are either overwritten by other assignments or where the variable is not used subsequently.

$myVar = 'Value';
$higher = false;

if (rand(1, 6) > 3) {
    $higher = true;
} else {
    $higher = false;
}

Both the $myVar assignment in line 1 and the $higher assignment in line 2 are dead. The first because $myVar is never used and the second because $higher is always overwritten for every possible time line.

Loading history...
451
    
452
        if ($isRegularAction || $action == 'delete') {
453
            $success = $this->applyAction($args);
454
            if (!$success) {
455
                // the workflow operation failed
456
                return false;
457
            }
458
        }
459
    
460
        if (true === $this->hasPageLockSupport && $this->templateParameters['mode'] == 'edit' && \ModUtil::available('ZikulaPageLockModule')) {
461
            $lockingApi = $this->container->get('zikula_pagelock_module.api.locking');
462
            $lockName = 'ZikulaRoutesModule' . $this->objectTypeCapital . $this->createCompositeIdentifier();
463
            $lockingApi->releaseLock($lockName);
464
        }
465
    
466
        return new RedirectResponse($this->getRedirectUrl($args), 302);
0 ignored issues
show
Bug introduced by
It seems like you code against a specific sub-type and not the parent class Zikula\RoutesModule\Form...ase\AbstractEditHandler as the method getRedirectUrl() does only exist in the following sub-classes of Zikula\RoutesModule\Form...ase\AbstractEditHandler: Zikula\RoutesModule\Form...ase\AbstractEditHandler, Zikula\RoutesModule\Form\Handler\Route\EditHandler. Maybe you want to instanceof check for one of these explicitly?

Let’s take a look at an example:

abstract class User
{
    /** @return string */
    abstract public function getPassword();
}

class MyUser extends User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}

In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different sub-classes of User which does not have a getDisplayName() method, the code will break.

Available Fixes

  1. Change the type-hint for the parameter:

    class AuthSystem
    {
        public function authenticate(MyUser $user) { /* ... */ }
    }
    
  2. Add an additional type-check:

    class AuthSystem
    {
        public function authenticate(User $user)
        {
            if ($user instanceof MyUser) {
                $this->logger->info(/** ... */);
            }
    
            // or alternatively
            if ( ! $user instanceof MyUser) {
                throw new \LogicException(
                    '$user must be an instance of MyUser, '
                   .'other instances are not supported.'
                );
            }
    
        }
    }
    
Note: PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case.
  1. Add the method to the parent class:

    abstract class User
    {
        /** @return string */
        abstract public function getPassword();
    
        /** @return string */
        abstract public function getDisplayName();
    }
    
Loading history...
Security Cross-Site Scripting introduced by
$this->getRedirectUrl($args) can contain request data and is used in output context(s) leading to a potential security vulnerability.

3 paths for user data to reach this point

  1. Path: HeaderBag::get() returns tainted data, and AbstractEditHandler::$returnTo is assigned in src/system/RoutesModule/Form/Handler/Common/Base/AbstractEditHandler.php on line 207
  1. HeaderBag::get() returns tainted data, and AbstractEditHandler::$returnTo is assigned
    in src/system/RoutesModule/Form/Handler/Common/Base/AbstractEditHandler.php on line 207
  2. Tainted property AbstractEditHandler::$returnTo is read
    in src/system/RoutesModule/Form/Handler/Route/Base/AbstractEditHandler.php on line 118
  3. AbstractEditHandler::getDefaultReturnUrl() returns tainted data
    in src/system/RoutesModule/Form/Handler/Route/Base/AbstractEditHandler.php on line 252
  4. AbstractEditHandler::getRedirectUrl() returns tainted data
    in src/system/RoutesModule/Form/Handler/Common/Base/AbstractEditHandler.php on line 466
  2. Path: ParameterBag::get() returns tainted data, and AbstractEditHandler::$returnTo is assigned in src/system/RoutesModule/Form/Handler/Common/Base/AbstractEditHandler.php on line 201
  1. ParameterBag::get() returns tainted data, and AbstractEditHandler::$returnTo is assigned
    in src/system/RoutesModule/Form/Handler/Common/Base/AbstractEditHandler.php on line 201
  2. Tainted property AbstractEditHandler::$returnTo is read
    in src/system/RoutesModule/Form/Handler/Route/Base/AbstractEditHandler.php on line 118
  3. AbstractEditHandler::getDefaultReturnUrl() returns tainted data
    in src/system/RoutesModule/Form/Handler/Route/Base/AbstractEditHandler.php on line 252
  4. AbstractEditHandler::getRedirectUrl() returns tainted data
    in src/system/RoutesModule/Form/Handler/Common/Base/AbstractEditHandler.php on line 466
  3. Path: Session::get() returns tainted data, and AbstractEditHandler::$returnTo is assigned in src/system/RoutesModule/Form/Handler/Common/Base/AbstractEditHandler.php on line 205
  1. Session::get() returns tainted data, and AbstractEditHandler::$returnTo is assigned
    in src/system/RoutesModule/Form/Handler/Common/Base/AbstractEditHandler.php on line 205
  2. Tainted property AbstractEditHandler::$returnTo is read
    in src/system/RoutesModule/Form/Handler/Route/Base/AbstractEditHandler.php on line 118
  3. AbstractEditHandler::getDefaultReturnUrl() returns tainted data
    in src/system/RoutesModule/Form/Handler/Route/Base/AbstractEditHandler.php on line 252
  4. AbstractEditHandler::getRedirectUrl() returns tainted data
    in src/system/RoutesModule/Form/Handler/Common/Base/AbstractEditHandler.php on line 466

Used in output context

  1. RedirectResponse::__construct() uses RedirectResponse::setTargetUrl() ($url)
    in vendor/src/Symfony/Component/HttpFoundation/RedirectResponse.php on line 39
  2. RedirectResponse::setTargetUrl() uses Response::setContent() ($content)
    in vendor/src/Symfony/Component/HttpFoundation/RedirectResponse.php on line 86
  3. Response::setContent() uses property Response::$content for writing
    in vendor/src/Symfony/Component/HttpFoundation/Response.php on line 447
  4. Property Response::$content is used in echo
    in vendor/src/Symfony/Component/HttpFoundation/Response.php on line 406

Preventing Cross-Site-Scripting Attacks

Cross-Site-Scripting allows an attacker to inject malicious code into your website - in particular Javascript code, and have that code executed with the privileges of a visiting user. This can be used to obtain data, or perform actions on behalf of that visiting user.

In order to prevent this, make sure to escape all user-provided data:

// for HTML
$sanitized = htmlentities($tainted, ENT_QUOTES);

// for URLs
$sanitized = urlencode($tainted);

General Strategies to prevent injection

In general, it is advisable to prevent any user-data to reach this point. This can be done by white-listing certain values:

if ( ! in_array($value, array('this-is-allowed', 'and-this-too'), true)) {
    throw new \InvalidArgumentException('This input is not allowed.');
}

For numeric data, we recommend to explicitly cast the data:

$sanitized = (integer) $tainted;
Loading history...
467
    }
468
    
469
    /**
470
     * Get success or error message for default operations.
471
     *
472
     * @param array   $args    arguments from handleCommand method
473
     * @param Boolean $success true if this is a success, false for default error
474
     *
475
     * @return String desired status or error message
476
     */
477
    protected function getDefaultMessage($args, $success = false)
478
    {
479
        $message = '';
480
        switch ($args['commandName']) {
481 View Code Duplication
            case 'create':
0 ignored issues
show
Duplication introduced by
This code seems to be duplicated across your project.

Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation.

You can also find more detailed suggestions in the “Code” section of your repository.

Loading history...
482
                if (true === $success) {
483
                    $message = $this->__('Done! Item created.');
484
                } else {
485
                    $message = $this->__('Error! Creation attempt failed.');
486
                }
487
                break;
488 View Code Duplication
            case 'update':
0 ignored issues
show
Duplication introduced by
This code seems to be duplicated across your project.

Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation.

You can also find more detailed suggestions in the “Code” section of your repository.

Loading history...
489
                if (true === $success) {
490
                    $message = $this->__('Done! Item updated.');
491
                } else {
492
                    $message = $this->__('Error! Update attempt failed.');
493
                }
494
                break;
495 View Code Duplication
            case 'delete':
0 ignored issues
show
Duplication introduced by
This code seems to be duplicated across your project.

Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation.

You can also find more detailed suggestions in the “Code” section of your repository.

Loading history...
496
                if (true === $success) {
497
                    $message = $this->__('Done! Item deleted.');
498
                } else {
499
                    $message = $this->__('Error! Deletion attempt failed.');
500
                }
501
                break;
502
        }
503
    
504
        return $message;
505
    }
506
    
507
    /**
508
     * Add success or error message to session.
509
     *
510
     * @param array   $args    arguments from handleCommand method
511
     * @param Boolean $success true if this is a success, false for default error
512
     *
513
     * @throws RuntimeException Thrown if executing the workflow action fails
514
     */
515
    protected function addDefaultMessage($args, $success = false)
516
    {
517
        $message = $this->getDefaultMessage($args, $success);
518
        if (empty($message)) {
519
            return;
520
        }
521
    
522
        $flashType = true === $success ? 'status' : 'error';
523
        $this->request->getSession()->getFlashBag()->add($flashType, $message);
524
        $logger = $this->container->get('logger');
525
        $logArgs = ['app' => 'ZikulaRoutesModule', 'user' => $this->container->get('zikula_users_module.current_user')->get('uname'), 'entity' => $this->objectType, 'id' => $this->entityRef->createCompositeIdentifier()];
0 ignored issues
show
Bug introduced by
It seems like you code against a specific sub-type and not the parent class Zikula\Core\Doctrine\EntityAccess as the method createCompositeIdentifier() does only exist in the following sub-classes of Zikula\Core\Doctrine\EntityAccess: Zikula\RoutesModule\Enti...ase\AbstractRouteEntity, Zikula\RoutesModule\Entity\RouteEntity. Maybe you want to instanceof check for one of these explicitly?

Let’s take a look at an example:

abstract class User
{
    /** @return string */
    abstract public function getPassword();
}

class MyUser extends User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}

In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different sub-classes of User which does not have a getDisplayName() method, the code will break.

Available Fixes

  1. Change the type-hint for the parameter:

    class AuthSystem
    {
        public function authenticate(MyUser $user) { /* ... */ }
    }
    
  2. Add an additional type-check:

    class AuthSystem
    {
        public function authenticate(User $user)
        {
            if ($user instanceof MyUser) {
                $this->logger->info(/** ... */);
            }
    
            // or alternatively
            if ( ! $user instanceof MyUser) {
                throw new \LogicException(
                    '$user must be an instance of MyUser, '
                   .'other instances are not supported.'
                );
            }
    
        }
    }
    
Note: PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case.
  1. Add the method to the parent class:

    abstract class User
    {
        /** @return string */
        abstract public function getPassword();
    
        /** @return string */
        abstract public function getDisplayName();
    }
    
Loading history...
526
        if (true === $success) {
527
            $logger->notice('{app}: User {user} updated the {entity} with id {id}.', $logArgs);
528
        } else {
529
            $logger->error('{app}: User {user} tried to update the {entity} with id {id}, but failed.', $logArgs);
530
        }
531
    }
532
533
    /**
534
     * Input data processing called by handleCommand method.
535
     *
536
     * @param array $args Additional arguments
537
     */
538
    public function fetchInputData($args)
0 ignored issues
show
Unused Code introduced by
The parameter $args is not used and could be removed.

This check looks from parameters that have been defined for a function or method, but which are not used in the method body.

Loading history...
539
    {
540
        // fetch posted data input values as an associative array
541
        $formData = $this->form->getData();
0 ignored issues
show
Bug introduced by
The method getData() does not seem to exist on object<Symfony\Component\Form\AbstractType>.

This check looks for calls to methods that do not seem to exist on a given type. It looks for the method on the type itself as well as in inherited classes or implemented interfaces.

This is most likely a typographical error or the method has been renamed.

Loading history...
542
    
543
        if ($this->templateParameters['mode'] == 'create' && isset($this->form['repeatCreation']) && $this->form['repeatCreation']->getData() == 1) {
544
            $this->repeatCreateAction = true;
545
        }
546
    
547
        if (isset($this->form['additionalNotificationRemarks']) && $this->form['additionalNotificationRemarks']->getData() != '') {
548
            $this->request->getSession()->set('ZikulaRoutesModuleAdditionalNotificationRemarks', $this->form['additionalNotificationRemarks']->getData());
549
        }
550
    
551
        // return remaining form data
552
        return $formData;
553
    }
554
555
    /**
556
     * This method executes a certain workflow action.
557
     *
558
     * @param array $args Arguments from handleCommand method
559
     *
560
     * @return bool Whether everything worked well or not
561
     */
562
    public function applyAction(array $args = [])
563
    {
564
        // stub for subclasses
565
        return false;
566
    }
567
}
568