Skip to content

Commit 539562d

Browse files
johanibjohanib
committed
Phpstan: Fix get parameter types
Prior to this change, type errors were thrown when invalid actorId/identityId parameters were passed. This change makes these errors explicit by throwing 400 bad request errors. Prior to this change, \Surfnet\StepupMiddleware\ApiBundle\Controller\RaCandidateController::search could receive `secondFactorTypes`, which could never be an array, yet the query expects an array. This change ensures the query parameters for this property are read sa an array read.
1 parent e248a0d commit 539562d

12 files changed

Lines changed: 99 additions & 381 deletions

‎ci/qa/phpstan-baseline.neon‎

Lines changed: 0 additions & 348 deletions
Large diffs are not rendered by default.

‎config/reference.php‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@
128128
* http_method_override?: bool, // Set true to enable support for the '_method' request parameter to determine the intended HTTP method on POST requests. // Default: false
129129
* allowed_http_method_override?: list<string>|null,
130130
* trust_x_sendfile_type_header?: scalar|null, // Set true to enable support for xsendfile in binary file responses. // Default: "%env(bool:default::SYMFONY_TRUST_X_SENDFILE_TYPE_HEADER)%"
131-
* ide?: scalar|null, // Default: null
131+
* ide?: scalar|null, // Default: "%env(default::SYMFONY_IDE)%"
132132
* test?: bool,
133133
* default_locale?: scalar|null, // Default: "en"
134134
* set_locale_from_accept_language?: bool, // Whether to use the Accept-Language HTTP header to set the Request locale (only when the "_locale" request attribute is not passed). // Default: false
@@ -285,7 +285,7 @@
285285
* paths?: array<string, scalar|null>,
286286
* excluded_patterns?: list<scalar|null>,
287287
* exclude_dotfiles?: bool, // If true, any files starting with "." will be excluded from the asset mapper. // Default: true
288-
* server?: bool, // If true, a "dev server" will return the assets from the public directory (true in "debug" mode only by default). // Default: false
288+
* server?: bool, // If true, a "dev server" will return the assets from the public directory (true in "debug" mode only by default). // Default: true
289289
* public_prefix?: scalar|null, // The public path where the assets will be written to (and served from when "server" is true). // Default: "/assets/"
290290
* missing_import_mode?: "strict"|"warn"|"ignore", // Behavior if an asset cannot be found when imported from JavaScript or CSS files - e.g. "import './non-existent.js'". "strict" means an exception is thrown, "warn" means a warning is logged, "ignore" means the import is left as-is. // Default: "warn"
291291
* extensions?: array<string, scalar|null>,
@@ -405,7 +405,7 @@
405405
* },
406406
* php_errors?: array{ // PHP errors handling configuration
407407
* log?: mixed, // Use the application logger instead of the PHP logger for logging PHP errors. // Default: true
408-
* throw?: bool, // Throw PHP errors as \ErrorException instances. // Default: false
408+
* throw?: bool, // Throw PHP errors as \ErrorException instances. // Default: true
409409
* },
410410
* exceptions?: array<string, array{ // Default: []
411411
* log_level?: scalar|null, // The level of log message. Null to let Symfony decide. // Default: null
@@ -468,7 +468,7 @@
468468
* scheduler?: bool|array{ // Scheduler configuration
469469
* enabled?: bool, // Default: false
470470
* },
471-
* disallow_search_engine_index?: bool, // Enabled by default when debug is enabled. // Default: false
471+
* disallow_search_engine_index?: bool, // Enabled by default when debug is enabled. // Default: true
472472
* http_client?: bool|array{ // HTTP Client configuration
473473
* enabled?: bool, // Default: false
474474
* max_host_connections?: int, // The maximum number of connections to a single host.
@@ -1112,8 +1112,8 @@
11121112
* platform_service?: scalar|null, // Deprecated: The "platform_service" configuration key is deprecated since doctrine-bundle 2.9. DBAL 4 will not support setting a custom platform via connection params anymore.
11131113
* auto_commit?: bool,
11141114
* schema_filter?: scalar|null,
1115-
* logging?: bool, // Default: false
1116-
* profiling?: bool, // Default: false
1115+
* logging?: bool, // Default: true
1116+
* profiling?: bool, // Default: true
11171117
* profiling_collect_backtrace?: bool, // Enables collecting backtraces when profiling is enabled // Default: false
11181118
* profiling_collect_schema_errors?: bool, // Enables collecting schema errors when profiling is enabled // Default: true
11191119
* disable_type_comments?: bool,
@@ -1252,7 +1252,7 @@
12521252
* pool?: scalar|null,
12531253
* },
12541254
* region_lock_lifetime?: scalar|null, // Default: 60
1255-
* log_enabled?: bool, // Default: false
1255+
* log_enabled?: bool, // Default: true
12561256
* region_lifetime?: scalar|null, // Default: 3600
12571257
* enabled?: bool, // Default: true
12581258
* factory?: scalar|null,

‎src/Surfnet/StepupMiddleware/ApiBundle/Controller/AuditLogController.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ public function secondFactorAuditLog(Request $request, Institution $institution)
4747
$query->identityId = new IdentityId($identityId);
4848
$query->orderBy = $request->query->get('orderBy', $query->orderBy);
4949
$query->orderDirection = $request->query->get('orderDirection', $query->orderDirection);
50-
$query->pageNumber = $request->query->get('p', 1);
50+
$query->pageNumber = $request->query->getInt('p', 1);
5151

5252
$paginator = $this->auditLogService->searchSecondFactorAuditLog($query);
5353

‎src/Surfnet/StepupMiddleware/ApiBundle/Controller/IdentityController.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ public function collection(Request $request, Institution $institution): JsonColl
5858
$query->nameId = $request->query->get('NameID');
5959
$query->commonName = $request->query->get('commonName');
6060
$query->email = $request->query->get('email');
61-
$query->pageNumber = (int)$request->query->get('p', 1);
61+
$query->pageNumber = $request->query->getInt('p', 1);
6262

6363
$paginator = $this->identityService->search($query);
6464

‎src/Surfnet/StepupMiddleware/ApiBundle/Controller/RaCandidateController.php‎

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
use Surfnet\StepupMiddleware\ApiBundle\Response\JsonCollectionResponse;
2828
use Symfony\Component\HttpFoundation\JsonResponse;
2929
use Symfony\Component\HttpFoundation\Request;
30+
use Symfony\Component\HttpKernel\Exception\BadRequestHttpException;
3031
use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
3132
use function sprintf;
3233

@@ -45,15 +46,27 @@ public function search(Request $request): JsonCollectionResponse
4546
{
4647
$this->denyAccessUnlessGrantedOneOff(['ROLE_RA', 'ROLE_READ']);
4748

48-
$actorId = new IdentityId($request->query->get('actorId'));
49+
$actorIdString = $request->query->get('actorId');
50+
if (!is_string($actorIdString)) {
51+
throw new BadRequestHttpException(sprintf('Invalid actorId "%s"', $actorIdString));
52+
}
53+
$actorId = new IdentityId($actorIdString);
54+
55+
$secondFactorTypes = $request->query->all('secondFactorTypes');
56+
foreach ($secondFactorTypes as $type) {
57+
if (!is_string($type)) {
58+
throw new BadRequestHttpException(sprintf('Invalid secondFactorType "%s", string expected.', $type));
59+
}
60+
}
61+
/** @var array<string> $secondFactorTypes */
4962

5063
$query = new RaCandidateQuery();
5164
$query->institution = $request->query->get('institution');
5265
$query->commonName = $request->query->get('commonName');
5366
$query->email = $request->query->get('email');
54-
$query->secondFactorTypes = $request->query->get('secondFactorTypes');
67+
$query->secondFactorTypes = $secondFactorTypes;
5568
$query->raInstitution = $request->query->get('raInstitution');
56-
$query->pageNumber = (int)$request->query->get('p', 1);
69+
$query->pageNumber = $request->query->getInt('p', 1);
5770

5871
$query->authorizationContext = $this->authorizationService->buildSelectRaaInstitutionAuthorizationContext(
5972
$actorId,
@@ -73,9 +86,17 @@ public function get(Request $request): JsonResponse
7386
{
7487
$this->denyAccessUnlessGrantedOneOff(['ROLE_RA', 'ROLE_READ']);
7588

76-
$actorId = new IdentityId($request->query->get('actorId'));
89+
$actorIdString = $request->query->get('actorId');
90+
if (!is_string($actorIdString)) {
91+
throw new BadRequestHttpException(sprintf('Invalid actorId "%s"', $actorIdString));
92+
}
93+
94+
$actorId = new IdentityId($actorIdString);
7795

7896
$identityId = $request->query->get('identityId');
97+
if (!is_string($identityId)) {
98+
throw new BadRequestHttpException(sprintf('Invalid identityId "%s"', $identityId));
99+
}
79100

80101
$authorizationContext = $this->authorizationService->buildInstitutionAuthorizationContext(
81102
$actorId,

‎src/Surfnet/StepupMiddleware/ApiBundle/Controller/RaListingController.php‎

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -22,13 +22,13 @@
2222
use Surfnet\Stepup\Identity\Value\Institution;
2323
use Surfnet\Stepup\Identity\Value\RegistrationAuthorityRole;
2424
use Surfnet\StepupMiddleware\ApiBundle\Authorization\Service\AuthorizationContextService;
25-
use Surfnet\StepupMiddleware\ApiBundle\Controller\AbstractController;
2625
use Surfnet\StepupMiddleware\ApiBundle\Identity\Entity\RaListing;
2726
use Surfnet\StepupMiddleware\ApiBundle\Identity\Query\RaListingQuery;
2827
use Surfnet\StepupMiddleware\ApiBundle\Identity\Service\RaListingService;
2928
use Surfnet\StepupMiddleware\ApiBundle\Response\JsonCollectionResponse;
3029
use Symfony\Component\HttpFoundation\JsonResponse;
3130
use Symfony\Component\HttpFoundation\Request;
31+
use Symfony\Component\HttpKernel\Exception\BadRequestHttpException;
3232
use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
3333

3434
class RaListingController extends AbstractController
@@ -43,8 +43,13 @@ public function get(Request $request, string $identityId): JsonResponse
4343
{
4444
$this->denyAccessUnlessGrantedOneOff(['ROLE_RA', 'ROLE_READ']);
4545

46-
$actorId = new IdentityId($request->query->get('actorId'));
47-
$institution = new Institution($request->query->get('institution'));
46+
$actorIdString = $request->query->get('actorId');
47+
if (!is_string($actorIdString)) {
48+
throw new BadRequestHttpException(sprintf('Invalid actorId "%s"', $actorIdString));
49+
}
50+
$actorId = new IdentityId($actorIdString);
51+
52+
$institution = new Institution($request->query->getString('institution'));
4853

4954
$authorizationContext = $this->authorizationService->buildInstitutionAuthorizationContext(
5055
$actorId,
@@ -71,7 +76,11 @@ public function search(Request $request): JsonCollectionResponse
7176
{
7277
$this->denyAccessUnlessGrantedOneOff(['ROLE_RA', 'ROLE_READ']);
7378

74-
$actorId = new IdentityId($request->query->get('actorId'));
79+
$actorIdString = $request->query->get('actorId');
80+
if (!is_string($actorIdString)) {
81+
throw new BadRequestHttpException(sprintf('Invalid actorId "%s"', $actorIdString));
82+
}
83+
$actorId = new IdentityId($actorIdString);
7584

7685
$query = new RaListingQuery();
7786

@@ -99,9 +108,9 @@ public function search(Request $request): JsonCollectionResponse
99108
$query->raInstitution = $request->query->get('raInstitution');
100109
}
101110

102-
$query->pageNumber = (int)$request->query->get('p', 1);
103-
$query->orderBy = $request->query->get('orderBy');
104-
$query->orderDirection = $request->query->get('orderDirection');
111+
$query->pageNumber = $request->query->getInt('p', 1);
112+
$query->orderBy = $request->query->getString('orderBy');
113+
$query->orderDirection = $request->query->getString('orderDirection');
105114
$query->authorizationContext = $this->authorizationService->buildInstitutionAuthorizationContext(
106115
$actorId,
107116
RegistrationAuthorityRole::raa(),

‎src/Surfnet/StepupMiddleware/ApiBundle/Controller/RaLocationController.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ public function get(Request $request): JsonResponse
5353
{
5454
$this->denyAccessUnlessGrantedOneOff(['ROLE_RA', 'ROLE_SS', 'ROLE_READ']);
5555

56-
$raLocationId = new RaLocationId($request->query->get('raLocationId'));
56+
$raLocationId = new RaLocationId($request->query->getString('raLocationId'));
5757
$raLocation = $this->raLocationService->findByRaLocationId($raLocationId);
5858

5959
return new JsonResponse($raLocation);

‎src/Surfnet/StepupMiddleware/ApiBundle/Controller/RaSecondFactorController.php‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
use Surfnet\StepupMiddleware\ApiBundle\Response\JsonCollectionResponse;
2828
use Symfony\Component\HttpFoundation\JsonResponse;
2929
use Symfony\Component\HttpFoundation\Request;
30+
use Symfony\Component\HttpKernel\Exception\BadRequestHttpException;
3031

3132
final class RaSecondFactorController extends AbstractController
3233
{
@@ -65,10 +66,15 @@ public function export(Request $request): JsonResponse
6566
*/
6667
private function buildRaSecondFactorQuery(Request $request): RaSecondFactorQuery
6768
{
68-
$actorId = new IdentityId($request->query->get('actorId'));
69+
$actorIdString = $request->query->get('actorId');
70+
if (!is_string($actorIdString)) {
71+
throw new BadRequestHttpException(sprintf('Invalid actorId "%s"', $actorIdString));
72+
}
73+
74+
$actorId = new IdentityId($actorIdString);
6975

7076
$query = new RaSecondFactorQuery();
71-
$query->pageNumber = (int)$request->query->get('p', 1);
77+
$query->pageNumber = $request->query->getInt('p', 1);
7278
$query->name = $request->query->get('name');
7379
$query->type = $request->query->get('type');
7480
$query->secondFactorId = $request->query->get('secondFactorId');

‎src/Surfnet/StepupMiddleware/ApiBundle/Controller/RecoveryTokenController.php‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,18 +23,19 @@
2323
use Surfnet\Stepup\Identity\Value\RecoveryTokenId;
2424
use Surfnet\Stepup\Identity\Value\RegistrationAuthorityRole;
2525
use Surfnet\StepupMiddleware\ApiBundle\Authorization\Service\AuthorizationContextService;
26-
use Surfnet\StepupMiddleware\ApiBundle\Controller\AbstractController;
2726
use Surfnet\StepupMiddleware\ApiBundle\Exception\NotFoundException;
2827
use Surfnet\StepupMiddleware\ApiBundle\Identity\Query\RecoveryTokenQuery;
2928
use Surfnet\StepupMiddleware\ApiBundle\Identity\Service\RecoveryTokenService;
3029
use Surfnet\StepupMiddleware\ApiBundle\Response\JsonCollectionResponse;
3130
use Symfony\Component\HttpFoundation\JsonResponse;
3231
use Symfony\Component\HttpFoundation\Request;
32+
use Symfony\Component\HttpKernel\Exception\BadRequestHttpException;
3333
use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
3434

3535
/**
3636
* Exposes the Recovery Tokens projection through the
3737
* Middleware Identity (read) API
38+
* @SuppressWarnings("PHPMD.CouplingBetweenObjects")
3839
*/
3940
class RecoveryTokenController extends AbstractController
4041
{
@@ -71,7 +72,7 @@ public function collection(Request $request): JsonCollectionResponse
7172
$query->institution = $request->query->get('institution');
7273
$query->email = $request->query->get('email');
7374
$query->name = $request->query->get('name');
74-
$query->pageNumber = (int)$request->query->get('p', 1);
75+
$query->pageNumber = $request->query->getInt('p', 1);
7576
$query->orderBy = $request->query->get('orderBy');
7677
$query->orderDirection = $request->query->get('orderDirection');
7778

@@ -80,6 +81,9 @@ public function collection(Request $request): JsonCollectionResponse
8081
if (!in_array('ROLE_SS', $roles)) {
8182
$actorId = $request->query->get('actorId', $request->query->get('identityId'));
8283
$this->logger->info(sprintf('Executing query on behalf of %s', $actorId));
84+
if (!is_string($actorId)) {
85+
throw new BadRequestHttpException('Invalid actorId or identityId, string expected.');
86+
}
8387
$actorId = new IdentityId($actorId);
8488
$query->authorizationContext = $this->authorizationService->buildInstitutionAuthorizationContext(
8589
$actorId,

‎src/Surfnet/StepupMiddleware/ApiBundle/Controller/UnverifiedSecondFactorController.php‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
use Surfnet\StepupMiddleware\ApiBundle\Identity\Query\UnverifiedSecondFactorQuery;
2626
use Surfnet\StepupMiddleware\ApiBundle\Identity\Service\SecondFactorService;
2727
use Surfnet\StepupMiddleware\ApiBundle\Response\JsonCollectionResponse;
28+
use Symfony\Component\HttpFoundation\Exception\BadRequestException;
2829
use Symfony\Component\HttpFoundation\JsonResponse;
2930
use Symfony\Component\HttpFoundation\Request;
3031
use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
@@ -54,9 +55,16 @@ public function collection(Request $request): JsonCollectionResponse
5455
$this->denyAccessUnlessGrantedOneOff(['ROLE_RA', 'ROLE_SS', 'ROLE_READ']);
5556

5657
$query = new UnverifiedSecondFactorQuery();
57-
$query->identityId = new IdentityId($request->query->get('identityId'));
58+
59+
$identityIdString = $request->query->get('identityId');
60+
if (!is_string($identityIdString)) {
61+
throw new BadRequestException(sprintf('Invalid identityId "%s"', $identityIdString));
62+
}
63+
64+
$query->identityId = new IdentityId($identityIdString);
65+
5866
$query->verificationNonce = $request->query->get('verificationNonce');
59-
$query->pageNumber = (int)$request->query->get('p', 1);
67+
$query->pageNumber = $request->query->getInt('p', 1);
6068

6169
$paginator = $this->secondFactorService->searchUnverifiedSecondFactors($query);
6270

0 commit comments

Comments
 (0)