conferencetools /
opentickets
This project does not seem to handle request data directly as such no vulnerable execution paths were found.
include, or for example
via PHP's auto-loading mechanism.
These results are based on our legacy PHP analysis, consider migrating to our new PHP analysis engine instead. Learn more
| 1 | <?php |
||
| 2 | |||
| 3 | namespace ConferenceTools\Tickets\Controller; |
||
| 4 | |||
| 5 | use Carnage\Cqrs\MessageBus\MessageBusInterface; |
||
| 6 | use Doctrine\ORM\EntityManager; |
||
| 7 | use ConferenceTools\Tickets\Domain\Command\Ticket\AssignToDelegate; |
||
| 8 | use ConferenceTools\Tickets\Domain\Command\Ticket\CompletePurchase; |
||
| 9 | use ConferenceTools\Tickets\Domain\Command\Ticket\ReserveTickets; |
||
| 10 | use ConferenceTools\Tickets\Domain\Event\Ticket\TicketPurchaseCreated; |
||
| 11 | use ConferenceTools\Tickets\Domain\ReadModel\TicketCounts\TicketCounter; |
||
| 12 | use ConferenceTools\Tickets\Domain\ReadModel\TicketRecord\PurchaseRecord; |
||
| 13 | use ConferenceTools\Tickets\Domain\Service\Configuration; |
||
| 14 | use ConferenceTools\Tickets\Domain\Service\TicketAvailability\TicketAvailability; |
||
| 15 | use ConferenceTools\Tickets\Domain\ValueObject\Delegate; |
||
| 16 | use ConferenceTools\Tickets\Domain\ValueObject\TicketReservationRequest; |
||
| 17 | use ConferenceTools\Tickets\Form\ManageTicket; |
||
| 18 | use ConferenceTools\Tickets\Form\PurchaseForm; |
||
| 19 | use Zend\Form\FormElementManager\FormElementManagerV2Polyfill; |
||
| 20 | use Zend\Stdlib\ArrayObject; |
||
| 21 | use Zend\View\Model\ViewModel; |
||
| 22 | use ZfrStripe\Client\StripeClient; |
||
| 23 | use ZfrStripe\Exception\CardErrorException; |
||
| 24 | |||
| 25 | class TicketController extends AbstractController |
||
| 26 | { |
||
| 27 | private static $cardErrorMessages = [ |
||
| 28 | 'invalid_number' => 'The card number is not a valid credit card number.', |
||
| 29 | 'invalid_expiry_month' => 'The card\'s expiration month is invalid.', |
||
| 30 | 'invalid_expiry_year' => 'The card\'s expiration year is invalid.', |
||
| 31 | 'invalid_cvc' => 'The card\'s security code/CVC is invalid.', |
||
| 32 | 'invalid_swipe_data' => 'The card\'s swipe data is invalid.', |
||
| 33 | 'incorrect_number' => 'The card number is incorrect.', |
||
| 34 | 'expired_card' => 'The card has expired.', |
||
| 35 | 'incorrect_cvc' => 'The card\'s security code/CVC is incorrect.', |
||
| 36 | 'incorrect_zip' => 'The address for your card did not match the card\'s billing address.', |
||
| 37 | 'card_declined' => 'The card was declined.', |
||
| 38 | 'missing' => 'There is no card on a customer that is being charged.', |
||
| 39 | 'processing_error' => 'An error occurred while processing the card.', |
||
| 40 | ]; |
||
| 41 | /** |
||
| 42 | * @var TicketAvailability |
||
| 43 | */ |
||
| 44 | private $ticketAvailability; |
||
| 45 | /** |
||
| 46 | * @var FormElementManagerV2Polyfill |
||
| 47 | */ |
||
| 48 | private $formElementManager; |
||
| 49 | |||
| 50 | public function __construct( |
||
| 51 | MessageBusInterface $commandBus, |
||
| 52 | EntityManager $entityManager, |
||
| 53 | StripeClient $stripeClient, |
||
| 54 | Configuration $configuration, |
||
| 55 | TicketAvailability $ticketAvailability, |
||
| 56 | FormElementManagerV2Polyfill $formElementManager |
||
| 57 | ) { |
||
| 58 | parent::__construct($commandBus, $entityManager, $stripeClient, $configuration); |
||
| 59 | $this->ticketAvailability = $ticketAvailability; |
||
| 60 | $this->formElementManager = $formElementManager; |
||
| 61 | } |
||
| 62 | |||
| 63 | public function indexAction() |
||
| 64 | { |
||
| 65 | return $this->redirect()->toRoute('tickets/select-tickets'); |
||
|
0 ignored issues
–
show
|
|||
| 66 | } |
||
| 67 | |||
| 68 | public function selectTicketsAction() |
||
| 69 | { |
||
| 70 | $tickets = $this->ticketAvailability->fetchAllAvailableTickets(); |
||
| 71 | |||
| 72 | if ($this->getRequest()->isPost()) { |
||
|
0 ignored issues
–
show
It seems like you code against a concrete implementation and not the interface
Zend\Stdlib\RequestInterface as the method isPost() does only exist in the following implementations of said interface: Zend\Http\PhpEnvironment\Request, Zend\Http\Request.
Let’s take a look at an example: interface User
{
/** @return string */
public function getPassword();
}
class MyUser implements 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 implementation of User which does not have a getDisplayName() method, the code will break. Available Fixes
Note: PHP Analyzer uses reverse abstract interpretation to narrow down the types
inside the if block in such a case.
Loading history...
|
|||
| 73 | $data = $this->getRequest()->getPost(); |
||
|
0 ignored issues
–
show
It seems like you code against a concrete implementation and not the interface
Zend\Stdlib\RequestInterface as the method getPost() does only exist in the following implementations of said interface: Zend\Http\PhpEnvironment\Request, Zend\Http\Request.
Let’s take a look at an example: interface User
{
/** @return string */
public function getPassword();
}
class MyUser implements 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 implementation of User which does not have a getDisplayName() method, the code will break. Available Fixes
Note: PHP Analyzer uses reverse abstract interpretation to narrow down the types
inside the if block in such a case.
Loading history...
|
|||
| 74 | $failed = false; |
||
| 75 | try { |
||
| 76 | $purchases = $this->validateSelectedTickets($data, $tickets); |
||
|
0 ignored issues
–
show
It seems like
$tickets defined by $this->ticketAvailabilit...chAllAvailableTickets() on line 70 can also be of type object<Doctrine\Common\Collections\Collection>; however, ConferenceTools\Tickets\...lidateSelectedTickets() does only seem to accept array<integer,object<Con...tCounts\TicketCounter>>, maybe add an additional type check?
If a method or function can return multiple different values and unless you are sure that you only can receive a single value in this context, we recommend to add an additional type check: /**
* @return array|string
*/
function returnsDifferentValues($x) {
if ($x) {
return 'foo';
}
return array();
}
$x = returnsDifferentValues($y);
if (is_array($x)) {
// $x is an array.
}
If this a common case that PHP Analyzer should handle natively, please let us know by opening an issue. Loading history...
|
|||
| 77 | } catch (\InvalidArgumentException $e) { |
||
| 78 | $failed = true; |
||
| 79 | } |
||
| 80 | |||
| 81 | try { |
||
| 82 | $discountCode = $this->validateDiscountCode($data); |
||
|
0 ignored issues
–
show
Are you sure the assignment to
$discountCode is correct as $this->validateDiscountCode($data) (which targets ConferenceTools\Tickets\...:validateDiscountCode()) 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 The reason is most likely that a function or method is imcomplete or has been reduced for debug purposes. Loading history...
|
|||
| 83 | $discountCodeStr = $data['discount_code']; |
||
| 84 | } catch (\InvalidArgumentException $e) { |
||
| 85 | $failed = true; |
||
| 86 | $discountCodeStr = ''; |
||
| 87 | } |
||
| 88 | |||
| 89 | if (!$failed) { |
||
| 90 | if ($discountCode !== null) { |
||
| 91 | $command = ReserveTickets::withDiscountCode($discountCode, ...$purchases); |
||
| 92 | } else { |
||
| 93 | $command = new ReserveTickets(...$purchases); |
||
| 94 | } |
||
| 95 | |||
| 96 | $this->getCommandBus()->dispatch($command); |
||
| 97 | /** @var TicketPurchaseCreated $event */ |
||
| 98 | $event = $this->events()->getEventsByType(TicketPurchaseCreated::class)[0]; |
||
| 99 | return $this->redirect()->toRoute('tickets/purchase', ['purchaseId' => $event->getId()]); |
||
| 100 | } |
||
| 101 | } else { |
||
| 102 | try { |
||
| 103 | $discountCodeStr = $this->params()->fromRoute('discount-code'); |
||
| 104 | $this->validateDiscountCode(['discount_code' => $discountCodeStr]); |
||
| 105 | } catch (\InvalidArgumentException $e) { |
||
| 106 | $discountCodeStr = ''; |
||
| 107 | } |
||
| 108 | } |
||
| 109 | |||
| 110 | return new ViewModel(['tickets' => $tickets, 'discountCode' => $discountCodeStr]); |
||
| 111 | } |
||
| 112 | |||
| 113 | /** |
||
| 114 | * @param $data |
||
| 115 | * @param TicketCounter[] $tickets |
||
| 116 | * @return array |
||
| 117 | */ |
||
| 118 | private function validateSelectedTickets($data, $tickets): array |
||
| 119 | { |
||
| 120 | $total = 0; |
||
| 121 | $purchases = []; |
||
| 122 | $errors = false; |
||
| 123 | foreach ($data['quantity'] as $id => $quantity) { |
||
| 124 | if (!is_numeric($quantity) || $quantity < 0) { |
||
| 125 | $this->flashMessenger()->addErrorMessage('Quantity needs to be a number :)'); |
||
| 126 | $errors = true; |
||
| 127 | } elseif (!$this->ticketAvailability->isAvailable($tickets[$id]->getTicketType(), $quantity)) { |
||
| 128 | $this->flashMessenger()->addErrorMessage( |
||
| 129 | sprintf('Not enough %s remaining', $tickets[$id]->getTicketType()->getDisplayName()) |
||
| 130 | ); |
||
| 131 | $total++; |
||
| 132 | $errors = true; |
||
| 133 | } elseif ($quantity > 0) { |
||
| 134 | $total += $quantity; |
||
| 135 | $purchases[] = new TicketReservationRequest($tickets[$id]->getTicketType(), (int) $quantity); |
||
| 136 | } |
||
| 137 | } |
||
| 138 | |||
| 139 | if ($total < 1) { |
||
| 140 | $this->flashMessenger()->addErrorMessage('You must specify at least 1 ticket to purchase'); |
||
| 141 | $errors = true; |
||
| 142 | } |
||
| 143 | |||
| 144 | if ($errors) { |
||
| 145 | throw new \InvalidArgumentException('input contained errors'); |
||
| 146 | } |
||
| 147 | |||
| 148 | return $purchases; |
||
| 149 | } |
||
| 150 | |||
| 151 | /** |
||
| 152 | * @param $data |
||
| 153 | * @return ?DiscountCode |
||
|
0 ignored issues
–
show
The doc-type
?DiscountCode could not be parsed: Unknown type name "?DiscountCode" at position 0. (view supported doc-types)
This check marks PHPDoc comments that could not be parsed by our parser. To see which comment annotations we can parse, please refer to our documentation on supported doc-types. Loading history...
|
|||
| 154 | */ |
||
| 155 | private function validateDiscountCode($data) |
||
| 156 | { |
||
| 157 | $errors = false; |
||
| 158 | |||
| 159 | $validCodes = $this->getConfiguration()->getDiscountCodes(); |
||
| 160 | $validCodes[''] = null; |
||
| 161 | |||
| 162 | $discountCode = strtolower($data['discount_code']); |
||
| 163 | |||
| 164 | if (!array_key_exists($discountCode, $validCodes)) { |
||
| 165 | $this->flashMessenger()->addErrorMessage('Invalid discount code'); |
||
| 166 | $errors = true; |
||
| 167 | } |
||
| 168 | |||
| 169 | if ($errors) { |
||
| 170 | throw new \InvalidArgumentException('input contained errors'); |
||
| 171 | } |
||
| 172 | |||
| 173 | return $validCodes[$discountCode]; |
||
| 174 | } |
||
| 175 | |||
| 176 | public function purchaseAction() |
||
| 177 | { |
||
| 178 | $purchaseId = $this->params()->fromRoute('purchaseId'); |
||
| 179 | $noPayment = false; |
||
| 180 | $purchase = $this->fetchPurchaseRecord($purchaseId); |
||
| 181 | |||
| 182 | if ($purchase === null || $purchase->hasTimedout()) { |
||
| 183 | $this->flashMessenger()->addErrorMessage('Purchase Id invalid or your purchase timed out'); |
||
| 184 | return $this->redirect()->toRoute('tickets/select-tickets'); |
||
| 185 | } |
||
| 186 | |||
| 187 | if ($purchase->isPaid()) { |
||
| 188 | $this->flashMessenger()->addInfoMessage('This purchase has already been paid for'); |
||
| 189 | return $this->redirect()->toRoute('tickets/complete', ['purchaseId' => $purchaseId]); |
||
| 190 | } |
||
| 191 | |||
| 192 | $form = new PurchaseForm($purchase->getTicketCount()); |
||
| 193 | |||
| 194 | if ($this->getRequest()->isPost()) { |
||
|
0 ignored issues
–
show
It seems like you code against a concrete implementation and not the interface
Zend\Stdlib\RequestInterface as the method isPost() does only exist in the following implementations of said interface: Zend\Http\PhpEnvironment\Request, Zend\Http\Request.
Let’s take a look at an example: interface User
{
/** @return string */
public function getPassword();
}
class MyUser implements 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 implementation of User which does not have a getDisplayName() method, the code will break. Available Fixes
Note: PHP Analyzer uses reverse abstract interpretation to narrow down the types
inside the if block in such a case.
Loading history...
|
|||
| 195 | $noPayment = true; |
||
| 196 | $data = $this->params()->fromPost(); |
||
| 197 | $form->setData($data); |
||
| 198 | if ($form->isValid()) { |
||
| 199 | try { |
||
| 200 | $this->getStripeClient()->createCharge([ |
||
| 201 | "amount" => $purchase->getTotalCost()->getGross()->getAmount(), |
||
| 202 | "currency" => $purchase->getTotalCost()->getGross()->getCurrency(), |
||
| 203 | 'source' => $data['stripe_token'], |
||
| 204 | 'metadata' => [ |
||
| 205 | 'email' => $data['purchase_email'], |
||
| 206 | 'purchaseId' => $purchaseId |
||
| 207 | ] |
||
| 208 | ]); |
||
| 209 | |||
| 210 | $delegateInfo = []; |
||
| 211 | |||
| 212 | for ($i = 0; $i < $purchase->getTicketCount(); $i++) { |
||
| 213 | $delegateInfo[] = Delegate::fromArray($data['delegates_' . $i]); |
||
| 214 | } |
||
| 215 | |||
| 216 | $command = new CompletePurchase($purchaseId, $data['purchase_email'], ...$delegateInfo); |
||
| 217 | $this->getCommandBus()->dispatch($command); |
||
| 218 | $this->flashMessenger() |
||
| 219 | ->addSuccessMessage( |
||
| 220 | 'Your ticket purchase is completed. ' . |
||
| 221 | 'You will receive an email shortly with your receipt. ' . |
||
| 222 | 'Tickets will be sent to the delegates shortly before the event' |
||
| 223 | ); |
||
| 224 | return $this->redirect()->toRoute('tickets/complete', ['purchaseId' => $purchaseId]); |
||
| 225 | } catch (CardErrorException $e) { |
||
| 226 | $this->flashMessenger()->addErrorMessage( |
||
| 227 | sprintf( |
||
| 228 | 'There was an issue with taking your payment: %s Please try again.', |
||
| 229 | $this->getDetailedErrorMessage($e) |
||
| 230 | ) |
||
| 231 | ); |
||
| 232 | $noPayment = false; |
||
| 233 | } |
||
| 234 | } |
||
| 235 | } |
||
| 236 | |||
| 237 | $this->flashMessenger()->addInfoMessage('Your tickets have been reserved for 30 mins, please complete payment before then'); |
||
| 238 | return new ViewModel(['purchase' => $purchase, 'form' => $form, 'noPayment' => $noPayment]); |
||
| 239 | } |
||
| 240 | |||
| 241 | /** |
||
| 242 | * @param $purchaseId |
||
| 243 | * @return PurchaseRecord|null |
||
| 244 | */ |
||
| 245 | private function fetchPurchaseRecord($purchaseId) |
||
| 246 | { |
||
| 247 | /** @var PurchaseRecord $purchase */ |
||
| 248 | $purchase = $this->getEntityManager()->getRepository(PurchaseRecord::class)->findOneBy([ |
||
| 249 | 'purchaseId' => $purchaseId |
||
| 250 | ]); |
||
| 251 | return $purchase; |
||
| 252 | } |
||
| 253 | |||
| 254 | private function getDetailedErrorMessage(CardErrorException $e) |
||
| 255 | { |
||
| 256 | $response = $e->getResponse(); |
||
| 257 | $errors = json_decode($response->getBody(true), true); |
||
| 258 | $code = isset($errors['error']['code']) ? $errors['error']['code'] : 'processing_error'; |
||
| 259 | $code = isset(static::$cardErrorMessages[$code]) ? $code : 'processing_error'; |
||
|
0 ignored issues
–
show
Since
$cardErrorMessages is declared private, accessing it with static will lead to errors in possible sub-classes; consider using self, or increasing the visibility of $cardErrorMessages to at least protected.
Let’s assume you have a class which uses late-static binding: class YourClass
{
private static $someVariable;
public static function getSomeVariable()
{
return static::$someVariable;
}
}
The code above will run fine in your PHP runtime. However, if you now create a
sub-class and call the class YourSubClass extends YourClass { }
YourSubClass::getSomeVariable(); // Will cause an access error.
In the case above, it makes sense to update class SomeClass
{
private static $someVariable;
public static function getSomeVariable()
{
return self::$someVariable; // self works fine with private.
}
}
Loading history...
|
|||
| 260 | |||
| 261 | return static::$cardErrorMessages[$code]; |
||
|
0 ignored issues
–
show
Since
$cardErrorMessages is declared private, accessing it with static will lead to errors in possible sub-classes; consider using self, or increasing the visibility of $cardErrorMessages to at least protected.
Let’s assume you have a class which uses late-static binding: class YourClass
{
private static $someVariable;
public static function getSomeVariable()
{
return static::$someVariable;
}
}
The code above will run fine in your PHP runtime. However, if you now create a
sub-class and call the class YourSubClass extends YourClass { }
YourSubClass::getSomeVariable(); // Will cause an access error.
In the case above, it makes sense to update class SomeClass
{
private static $someVariable;
public static function getSomeVariable()
{
return self::$someVariable; // self works fine with private.
}
}
Loading history...
|
|||
| 262 | } |
||
| 263 | |||
| 264 | public function completeAction() |
||
| 265 | { |
||
| 266 | $purchaseId = $this->params()->fromRoute('purchaseId'); |
||
| 267 | $purchase = $this->fetchPurchaseRecord($purchaseId); |
||
| 268 | |||
| 269 | if ($purchase === null) { |
||
| 270 | $this->flashMessenger()->addErrorMessage('Purchase Id invalid'); |
||
| 271 | return $this->redirect()->toRoute('tickets/select-tickets'); |
||
| 272 | } |
||
| 273 | |||
| 274 | return new ViewModel(['purchase' => $purchase]); |
||
| 275 | } |
||
| 276 | |||
| 277 | public function manageAction() |
||
| 278 | { |
||
| 279 | $purchaseId = $this->params()->fromRoute('purchaseId'); |
||
| 280 | $ticketId = $this->params()->fromRoute('ticketId'); |
||
| 281 | |||
| 282 | $purchase = $this->fetchPurchaseRecord($purchaseId); |
||
| 283 | $ticketRecord = $purchase->getTicketRecord($ticketId); |
||
| 284 | $delegate = $ticketRecord->getDelegate(); |
||
| 285 | |||
| 286 | $form = $this->formElementManager->get(ManageTicket::class); |
||
| 287 | $data = [ |
||
| 288 | 'delegate' => [ |
||
| 289 | 'firstname' => $delegate->getFirstname(), |
||
| 290 | 'lastname' => $delegate->getLastname(), |
||
| 291 | 'email' => $delegate->getEmail(), |
||
| 292 | 'company' => $delegate->getCompany(), |
||
| 293 | 'twitter' => $delegate->getTwitter(), |
||
| 294 | 'requirements' => $delegate->getRequirements() |
||
| 295 | ] |
||
| 296 | ]; |
||
| 297 | |||
| 298 | $form->bind(new ArrayObject($data)); |
||
| 299 | |||
| 300 | if ($this->getRequest()->isPost()) { |
||
|
0 ignored issues
–
show
It seems like you code against a concrete implementation and not the interface
Zend\Stdlib\RequestInterface as the method isPost() does only exist in the following implementations of said interface: Zend\Http\PhpEnvironment\Request, Zend\Http\Request.
Let’s take a look at an example: interface User
{
/** @return string */
public function getPassword();
}
class MyUser implements 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 implementation of User which does not have a getDisplayName() method, the code will break. Available Fixes
Note: PHP Analyzer uses reverse abstract interpretation to narrow down the types
inside the if block in such a case.
Loading history...
|
|||
| 301 | $form->setData($this->params()->fromPost()); |
||
| 302 | if ($form->isValid()) { |
||
| 303 | $data = $form->getData(); |
||
| 304 | $newDelegateInfo = Delegate::fromArray($data['delegate']); |
||
| 305 | |||
| 306 | $command = new AssignToDelegate($newDelegateInfo, $ticketId, $purchaseId); |
||
| 307 | $this->getCommandBus()->dispatch($command); |
||
| 308 | $this->flashMessenger() |
||
| 309 | ->addSuccessMessage( |
||
| 310 | 'Details updated successfully' |
||
| 311 | ); |
||
| 312 | return $this->redirect()->refresh(); |
||
| 313 | } |
||
| 314 | } |
||
| 315 | |||
| 316 | return new ViewModel(['purchase' => $purchase, 'form' => $form]); |
||
| 317 | } |
||
| 318 | } |
||
| 319 |
If you return a value from a function or method, it should be a sub-type of the type that is given by the parent type f.e. an interface, or abstract method. This is more formally defined by the Lizkov substitution principle, and guarantees that classes that depend on the parent type can use any instance of a child type interchangably. This principle also belongs to the SOLID principles for object oriented design.
Let’s take a look at an example:
Our function
my_functionexpects aPostobject, and outputs the author of the post. The base classPostreturns a simple string and outputting a simple string will work just fine. However, the child classBlogPostwhich is a sub-type ofPostinstead decided to return anobject, and is therefore violating the SOLID principles. If aBlogPostwere passed tomy_function, PHP would not complain, but ultimately fail when executing thestrtouppercall in its body.