Skip to content

Commit b8959cc

Browse files
sukhwinder33445sukhwinder33445
committed
ApiV1(*)Controller: Validate request body of correct data type
1 parent 8ede2ff commit b8959cc

File tree

2 files changed

+80
-73
lines changed

2 files changed

+80
-73
lines changed

‎application/controllers/ApiV1ContactgroupsController.php‎

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -370,19 +370,32 @@ private function removeContactgroup(int $id): void
370370
*/
371371
private function assertValidData(array $data): void
372372
{
373-
if (! isset($data['id'], $data['name'])) {
374-
$this->httpBadRequest('The request body must contain the fields id and name');
373+
$msgPrefix = 'Invalid request body: ';
374+
375+
if (! isset($data['id'], $data['name'])
376+
|| ! is_string($data['id'])
377+
|| ! is_string($data['full_name'])
378+
) {
379+
$this->httpBadRequest(
380+
$msgPrefix . 'the fields id and name must be present and of type string'
381+
);
375382
}
376383

377384
if (! Uuid::isValid($data['id'])) {
378-
$this->httpBadRequest('Given id in request body is not a valid UUID');
385+
$this->httpBadRequest($msgPrefix . 'given id is not a valid UUID');
379386
}
380387

381388
if (! empty($data['users'])) {
389+
if (! is_array($data['users'])) {
390+
$this->httpBadRequest($msgPrefix . 'expects users be an array');
391+
}
392+
382393
foreach ($data['users'] as $user) {
383-
if (! Uuid::isValid($user)) {
384-
$this->httpBadRequest('User identifiers in request body must be valid UUIDs');
394+
if (! is_string($user) || ! Uuid::isValid($user)) {
395+
$this->httpBadRequest($msgPrefix . 'user identifiers must be valid UUIDs');
385396
}
397+
398+
//TODO: check if users exist, here?
386399
}
387400
}
388401
}

‎application/controllers/ApiV1ContactsController.php‎

Lines changed: 62 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -221,10 +221,6 @@ function (Filter\Condition $condition) {
221221

222222
$contactId = $this->getContactId($identifier);
223223
if ($contactId !== null) {
224-
if (isset($data['username'])) {
225-
$this->assertUniqueUsername($data['username']);
226-
}
227-
228224
$db->update('contact', [
229225
'full_name' => $data['full_name'],
230226
'username' => $data['username'] ?? null,
@@ -397,10 +393,6 @@ protected function getContactId(string $identifier): ?int
397393
*/
398394
private function addContact(array $data): void
399395
{
400-
if (isset($data['username'])) {
401-
$this->assertUniqueUsername($data['username']);
402-
}
403-
404396
Database::get()->insert('contact', [
405397
'full_name' => $data['full_name'],
406398
'username' => $data['username'] ?? null,
@@ -419,55 +411,6 @@ private function addContact(array $data): void
419411
}
420412
}
421413

422-
/**
423-
* Assert that the username is unique
424-
*
425-
* @param string $username
426-
*
427-
* @return void
428-
*
429-
* @throws HttpBadRequestException if the username already exists
430-
*/
431-
private function assertUniqueUsername(string $username): void
432-
{
433-
$user = Database::get()->fetchOne(
434-
(new Select())
435-
->from('contact')
436-
->columns(1)
437-
->where(['username = ?' => $username])
438-
);
439-
440-
if ($user !== false) {
441-
$this->httpBadRequest('Username already exists');
442-
}
443-
}
444-
445-
/**
446-
* Assert that the address type exists
447-
*
448-
* @param string[] $addressTypes
449-
*
450-
* @return void
451-
*
452-
* @throws HttpBadRequestException if the username already exists
453-
*/
454-
private function assertAddressTypesExist(array $addressTypes): void
455-
{
456-
$types = Database::get()->fetchCol(
457-
(new Select())
458-
->from('available_channel_type')
459-
->columns('type')
460-
->where(['type IN (?)' => $addressTypes])
461-
);
462-
463-
if (count($types) !== count($addressTypes)) {
464-
$this->httpBadRequest(sprintf(
465-
'Undefined address type %s given',
466-
implode(', ', array_diff($addressTypes, $types))
467-
));
468-
}
469-
}
470-
471414
/**
472415
* Add the groups to the given contact
473416
*
@@ -498,8 +441,6 @@ private function addGroups(int $contactId, array $groups): void
498441
*/
499442
private function addAddresses(int $contactId, array $addresses): void
500443
{
501-
$this->assertAddressTypesExist(array_keys($addresses));
502-
503444
foreach ($addresses as $type => $address) {
504445
Database::get()->insert('contact_address', [
505446
'contact_id' => $contactId,
@@ -534,26 +475,79 @@ private function removeContact(int $id): void
534475
*/
535476
private function assertValidData(array $data): void
536477
{
537-
if (! isset($data['id'], $data['full_name'], $data['default_channel'])) {
538-
$this->httpBadRequest('The request body must contain the fields id, full_name and default_channel');
478+
$msgPrefix = 'Invalid request body: ';
479+
480+
if (! isset($data['id'], $data['full_name'], $data['default_channel'])
481+
|| ! is_string($data['id'])
482+
|| ! is_string($data['full_name'])
483+
|| ! is_string($data['default_channel'])
484+
) {
485+
$this->httpBadRequest(
486+
$msgPrefix . 'the fields id, full_name and default_channel must be present and of type string'
487+
);
539488
}
540489

541490
if (! Uuid::isValid($data['id'])) {
542-
$this->httpBadRequest('Given id in the request body is not a valid UUID');
491+
$this->httpBadRequest($msgPrefix . 'given id is not a valid UUID');
492+
}
493+
494+
if (! empty($data['username'])) {
495+
if (! is_string($data['username'])) {
496+
$this->httpBadRequest($msgPrefix . 'expects username to be a string');
497+
}
498+
499+
$user = Database::get()->fetchOne(
500+
(new Select())
501+
->from('contact')
502+
->columns(1)
503+
->where(['username = ?' => $data['username']])
504+
);
505+
506+
if ($user !== false) {
507+
$this->httpBadRequest($msgPrefix . 'username already exists');
508+
}
543509
}
544510

545511
if (! empty($data['groups'])) {
512+
if (! is_array($data['groups'])) {
513+
$this->httpBadRequest($msgPrefix . 'expects groups to be an array');
514+
}
515+
546516
foreach ($data['groups'] as $group) {
547-
if (! Uuid::isValid($group)) {
548-
$this->httpBadRequest('Group identifiers in the request body must be valid UUIDs');
517+
if (! is_string($group) || ! Uuid::isValid($group)) {
518+
$this->httpBadRequest($msgPrefix . 'group identifiers must be valid UUIDs');
549519
}
550520
}
551521
}
552522

553-
if (! empty($data['addresses']['email'])
554-
&& ! (new EmailAddressValidator())->isValid($data['addresses']['email'])
555-
) {
556-
$this->httpBadRequest('Request body contains an invalid email address');
523+
if (! empty($data['addresses'])) {
524+
if (! is_array($data['addresses'])) {
525+
$this->httpBadRequest($msgPrefix . 'expects addresses to be an array');
526+
}
527+
528+
$addressTypes = array_keys($data['addresses']);
529+
530+
$types = Database::get()->fetchCol(
531+
(new Select())
532+
->from('available_channel_type')
533+
->columns('type')
534+
->where(['type IN (?)' => $addressTypes])
535+
);
536+
537+
if (count($types) !== count($addressTypes)) {
538+
$this->httpBadRequest(sprintf(
539+
$msgPrefix . 'undefined address type %s given',
540+
implode(', ', array_diff($addressTypes, $types))
541+
));
542+
}
543+
//TODO: cant decide if this is a a good idea to check valid types and username here, if yes,
544+
//default_contact and group identifiers must be checked here as well..404 OR 400?
545+
546+
if (! empty($data['addresses']['email'])
547+
&& ! (new EmailAddressValidator())->isValid($data['addresses']['email'])
548+
) {
549+
$this->httpBadRequest($msgPrefix . 'an invalid email address given');
550+
}
557551
}
558552
}
559553
}

0 commit comments

Comments
 (0)