Skip to content

Commit c44012d

Browse files
johanibjohanib
committed
Fix deprecation: $request->get
$request->get is deprecated. $request->query->get for URL parameters or $request->request->get for POST should be used instead. See https://symfony.com/blog/new-in-symfony-7-4-request-class-improvements Phpstan: Fix get parameter types 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 01acfd1 commit c44012d

14 files changed

+149
-436
lines changed

‎ci/qa/phpstan-baseline.neon‎

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

‎config/packages/framework.yaml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ when@smoketest: &testOverride
2828
test: true
2929
profiler:
3030
collect: false
31+
collect_serializer_data: true
3132
php_errors:
3233
log: false # prevents user deprecated warnings
3334
session:

‎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: "%env(default::SYMFONY_IDE)%"
131+
* ide?: scalar|null, // Default: null
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: 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
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: true
408+
* throw?: bool, // Throw PHP errors as \ErrorException instances. // Default: false
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: true
471+
* disallow_search_engine_index?: bool, // Enabled by default when debug is enabled. // Default: false
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: true
1116-
* profiling?: bool, // Default: true
1115+
* logging?: bool, // Default: false
1116+
* profiling?: bool, // Default: false
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: true
1255+
* log_enabled?: bool, // Default: false
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: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020

2121
use Surfnet\Stepup\Identity\Value\IdentityId;
2222
use Surfnet\Stepup\Identity\Value\Institution;
23-
use Surfnet\StepupMiddleware\ApiBundle\Controller\AbstractController;
2423
use Surfnet\StepupMiddleware\ApiBundle\Exception\BadApiRequestException;
2524
use Surfnet\StepupMiddleware\ApiBundle\Identity\Query\SecondFactorAuditLogQuery;
2625
use Surfnet\StepupMiddleware\ApiBundle\Identity\Service\AuditLogService;
@@ -38,17 +37,17 @@ public function secondFactorAuditLog(Request $request, Institution $institution)
3837
{
3938
$this->denyAccessUnlessGrantedOneOff(['ROLE_RA', 'ROLE_READ']);
4039

41-
$identityId = $request->get('identityId');
40+
$identityId = $request->query->get('identityId');
4241
if (empty($identityId)) {
4342
throw new BadApiRequestException(['This API-call MUST include the identityId as get parameter']);
4443
}
4544

4645
$query = new SecondFactorAuditLogQuery();
4746
$query->identityInstitution = $institution;
4847
$query->identityId = new IdentityId($identityId);
49-
$query->orderBy = $request->get('orderBy', $query->orderBy);
50-
$query->orderDirection = $request->get('orderDirection', $query->orderDirection);
51-
$query->pageNumber = $request->get('p', 1);
48+
$query->orderBy = $request->query->get('orderBy', $query->orderBy);
49+
$query->orderDirection = $request->query->get('orderDirection', $query->orderDirection);
50+
$query->pageNumber = $request->query->getInt('p', 1);
5251

5352
$paginator = $this->auditLogService->searchSecondFactorAuditLog($query);
5453

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -55,10 +55,10 @@ public function collection(Request $request, Institution $institution): JsonColl
5555

5656
$query = new IdentityQuery();
5757
$query->institution = $institution;
58-
$query->nameId = $request->get('NameID');
59-
$query->commonName = $request->get('commonName');
60-
$query->email = $request->get('email');
61-
$query->pageNumber = (int)$request->get('p', 1);
58+
$query->nameId = $request->query->get('NameID');
59+
$query->commonName = $request->query->get('commonName');
60+
$query->email = $request->query->get('email');
61+
$query->pageNumber = $request->query->getInt('p', 1);
6262

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

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ public function get(Request $request, string $identityId): JsonResponse
3838
$this->denyAccessUnlessGrantedOneOff(['ROLE_RA', 'ROLE_READ']);
3939

4040
// Is the actor allowed to view the profile page?
41-
$actorId = $request->get('actorId');
41+
$actorId = $request->query->get('actorId');
4242
if ($identityId !== $actorId) {
4343
throw new AccessDeniedHttpException(
4444
"Identity and actor id should match. It is not yet allowed to view the profile of somebody else.",

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

Lines changed: 27 additions & 10 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->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();
51-
$query->institution = $request->get('institution');
52-
$query->commonName = $request->get('commonName');
53-
$query->email = $request->get('email');
54-
$query->secondFactorTypes = $request->get('secondFactorTypes');
55-
$query->raInstitution = $request->get('raInstitution');
56-
$query->pageNumber = (int)$request->get('p', 1);
64+
$query->institution = $request->query->get('institution');
65+
$query->commonName = $request->query->get('commonName');
66+
$query->email = $request->query->get('email');
67+
$query->secondFactorTypes = $secondFactorTypes;
68+
$query->raInstitution = $request->query->get('raInstitution');
69+
$query->pageNumber = $request->query->getInt('p', 1);
5770

5871
$query->authorizationContext = $this->authorizationService->buildSelectRaaInstitutionAuthorizationContext(
5972
$actorId,
@@ -69,13 +82,17 @@ public function search(Request $request): JsonCollectionResponse
6982
/**
7083
* @return JsonResponse
7184
*/
72-
public function get(Request $request): JsonResponse
85+
public function get(Request $request, string $identityId): JsonResponse
7386
{
7487
$this->denyAccessUnlessGrantedOneOff(['ROLE_RA', 'ROLE_READ']);
7588

76-
$actorId = new IdentityId($request->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

78-
$identityId = $request->get('identityId');
7996

8097
$authorizationContext = $this->authorizationService->buildInstitutionAuthorizationContext(
8198
$actorId,

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

Lines changed: 30 additions & 21 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
@@ -39,12 +39,17 @@ public function __construct(
3939
) {
4040
}
4141

42-
public function get(Request $request, string $identityId): JsonResponse
42+
public function get(Request $request, string $identityId, string $institution): JsonResponse
4343
{
4444
$this->denyAccessUnlessGrantedOneOff(['ROLE_RA', 'ROLE_READ']);
4545

46-
$actorId = new IdentityId($request->get('actorId'));
47-
$institution = new Institution($request->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+
$institutionObject = new Institution($institution);
4853

4954
$authorizationContext = $this->authorizationService->buildInstitutionAuthorizationContext(
5055
$actorId,
@@ -53,7 +58,7 @@ public function get(Request $request, string $identityId): JsonResponse
5358

5459
$raListing = $this->raListingService->findByIdentityIdAndRaInstitutionWithContext(
5560
new IdentityId($identityId),
56-
$institution,
61+
$institutionObject,
5762
$authorizationContext,
5863
);
5964

@@ -71,37 +76,41 @@ public function search(Request $request): JsonCollectionResponse
7176
{
7277
$this->denyAccessUnlessGrantedOneOff(['ROLE_RA', 'ROLE_READ']);
7378

74-
$actorId = new IdentityId($request->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

78-
if ($request->get('identityId')) {
79-
$query->identityId = new IdentityId($request->get('identityId'));
87+
if ($request->query->get('identityId')) {
88+
$query->identityId = new IdentityId($request->query->get('identityId'));
8089
}
8190

82-
if ($request->get('institution')) {
83-
$query->institution = $request->get('institution');
91+
if ($request->query->get('institution')) {
92+
$query->institution = $request->query->get('institution');
8493
}
8594

86-
if ($request->get('name')) {
87-
$query->name = $request->get('name');
95+
if ($request->query->get('name')) {
96+
$query->name = $request->query->get('name');
8897
}
8998

90-
if ($request->get('email')) {
91-
$query->email = $request->get('email');
99+
if ($request->query->get('email')) {
100+
$query->email = $request->query->get('email');
92101
}
93102

94-
if ($request->get('role')) {
95-
$query->role = $request->get('role');
103+
if ($request->query->get('role')) {
104+
$query->role = $request->query->get('role');
96105
}
97106

98-
if ($request->get('raInstitution')) {
99-
$query->raInstitution = $request->get('raInstitution');
107+
if ($request->query->get('raInstitution')) {
108+
$query->raInstitution = $request->query->get('raInstitution');
100109
}
101110

102-
$query->pageNumber = (int)$request->get('p', 1);
103-
$query->orderBy = $request->get('orderBy');
104-
$query->orderDirection = $request->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: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -40,21 +40,20 @@ public function search(Request $request, Institution $institution): JsonCollecti
4040

4141
$query = new RaLocationQuery();
4242
$query->institution = $institution;
43-
$query->orderBy = $request->get('orderBy', $query->orderBy);
44-
$query->orderDirection = $request->get('orderDirection', $query->orderDirection);
43+
$query->orderBy = $request->query->get('orderBy', $query->orderBy);
44+
$query->orderDirection = $request->query->get('orderDirection', $query->orderDirection);
4545

4646
$raLocations = $this->raLocationService->search($query);
4747
$count = count($raLocations);
4848

4949
return new JsonCollectionResponse($count, 1, $count, $raLocations);
5050
}
5151

52-
public function get(Request $request): JsonResponse
52+
public function get(string $raLocationId): JsonResponse
5353
{
5454
$this->denyAccessUnlessGrantedOneOff(['ROLE_RA', 'ROLE_SS', 'ROLE_READ']);
5555

56-
$raLocationId = new RaLocationId($request->get('raLocationId'));
57-
$raLocation = $this->raLocationService->findByRaLocationId($raLocationId);
56+
$raLocation = $this->raLocationService->findByRaLocationId(new RaLocationId($raLocationId));
5857

5958
return new JsonResponse($raLocation);
6059
}

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

Lines changed: 16 additions & 10 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,18 +66,23 @@ public function export(Request $request): JsonResponse
6566
*/
6667
private function buildRaSecondFactorQuery(Request $request): RaSecondFactorQuery
6768
{
68-
$actorId = new IdentityId($request->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->get('p', 1);
72-
$query->name = $request->get('name');
73-
$query->type = $request->get('type');
74-
$query->secondFactorId = $request->get('secondFactorId');
75-
$query->email = $request->get('email');
76-
$query->institution = $request->get('institution');
77-
$query->status = $request->get('status');
78-
$query->orderBy = $request->get('orderBy');
79-
$query->orderDirection = $request->get('orderDirection');
77+
$query->pageNumber = $request->query->getInt('p', 1);
78+
$query->name = $request->query->get('name');
79+
$query->type = $request->query->get('type');
80+
$query->secondFactorId = $request->query->get('secondFactorId');
81+
$query->email = $request->query->get('email');
82+
$query->institution = $request->query->get('institution');
83+
$query->status = $request->query->get('status');
84+
$query->orderBy = $request->query->get('orderBy');
85+
$query->orderDirection = $request->query->get('orderDirection');
8086
$query->authorizationContext = $this->authorizationService->buildInstitutionAuthorizationContext(
8187
$actorId,
8288
RegistrationAuthorityRole::ra(),

0 commit comments

Comments
 (0)