Skip to content

Commit 7596935

Browse files
SephsterSephster
authored
Merge pull request #1407 from eugene-borovov/fix-passport-compatibility
Parse request parameters
2 parents d1dc453 + df39bf6 commit 7596935

16 files changed

+95
-84
lines changed

‎phpstan.neon.dist

+1-5
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,4 @@ parameters:
22
level: 8
33
paths:
44
- src
5-
- tests
6-
ignoreErrors:
7-
-
8-
message: '#Call to an undefined method League\\OAuth2\\Server\\ResponseTypes\\ResponseTypeInterface::getAccessToken\(\)\.#'
9-
path: tests/Grant/ClientCredentialsGrantTest.php
5+
- tests

‎src/CryptKey.php

+2-2
Original file line numberDiff line numberDiff line change
@@ -43,13 +43,13 @@ class CryptKey implements CryptKeyInterface
4343

4444
public function __construct(string $keyPath, protected ?string $passPhrase = null, bool $keyPermissionsCheck = true)
4545
{
46-
if (strpos($keyPath, self::FILE_PREFIX) !== 0 && $this->isValidKey($keyPath, $this->passPhrase ?? '')) {
46+
if (!str_starts_with($keyPath, self::FILE_PREFIX) && $this->isValidKey($keyPath, $this->passPhrase ?? '')) {
4747
$this->keyContents = $keyPath;
4848
$this->keyPath = '';
4949
// There's no file, so no need for permission check.
5050
$keyPermissionsCheck = false;
5151
} elseif (is_file($keyPath)) {
52-
if (strpos($keyPath, self::FILE_PREFIX) !== 0) {
52+
if (!str_starts_with($keyPath, self::FILE_PREFIX)) {
5353
$keyPath = self::FILE_PREFIX . $keyPath;
5454
}
5555

‎src/Exception/OAuthServerException.php

+3-3
Original file line numberDiff line numberDiff line change
@@ -269,9 +269,9 @@ public function generateHttpResponse(ResponseInterface $response, bool $useFragm
269269

270270
if ($this->redirectUri !== null) {
271271
if ($useFragment === true) {
272-
$this->redirectUri .= (strstr($this->redirectUri, '#') === false) ? '#' : '&';
272+
$this->redirectUri .= (!str_contains($this->redirectUri, '#')) ? '#' : '&';
273273
} else {
274-
$this->redirectUri .= (strstr($this->redirectUri, '?') === false) ? '?' : '&';
274+
$this->redirectUri .= (!str_contains($this->redirectUri, '?')) ? '?' : '&';
275275
}
276276

277277
return $response->withStatus(302)->withHeader('Location', $this->redirectUri . http_build_query($payload));
@@ -310,7 +310,7 @@ public function getHttpHeaders(): array
310310
// include the "WWW-Authenticate" response header field
311311
// matching the authentication scheme used by the client.
312312
if ($this->errorType === 'invalid_client' && $this->requestHasAuthorizationHeader()) {
313-
$authScheme = strpos($this->serverRequest->getHeader('Authorization')[0], 'Bearer') === 0 ? 'Bearer' : 'Basic';
313+
$authScheme = str_starts_with($this->serverRequest->getHeader('Authorization')[0], 'Bearer') ? 'Bearer' : 'Basic';
314314

315315
$headers['WWW-Authenticate'] = $authScheme . ' realm="OAuth"';
316316
}

‎src/Grant/AbstractAuthorizeGrant.php

+2-3
Original file line numberDiff line numberDiff line change
@@ -18,16 +18,15 @@
1818
use League\OAuth2\Server\RequestTypes\AuthorizationRequestInterface;
1919

2020
use function http_build_query;
21-
use function strstr;
2221

2322
abstract class AbstractAuthorizeGrant extends AbstractGrant
2423
{
2524
/**
26-
* @param mixed[] $params
25+
* @param array<array-key,mixed> $params
2726
*/
2827
public function makeRedirectUri(string $uri, array $params = [], string $queryDelimiter = '?'): string
2928
{
30-
$uri .= (strstr($uri, $queryDelimiter) === false) ? $queryDelimiter : '&';
29+
$uri .= str_contains($uri, $queryDelimiter) ? '&' : $queryDelimiter;
3130

3231
return $uri . http_build_query($params);
3332
}

‎src/Grant/AbstractGrant.php

+63-35
Original file line numberDiff line numberDiff line change
@@ -49,10 +49,8 @@
4949
use function base64_decode;
5050
use function bin2hex;
5151
use function explode;
52-
use function is_null;
5352
use function is_string;
5453
use function random_bytes;
55-
use function strpos;
5654
use function substr;
5755
use function trim;
5856

@@ -129,19 +127,19 @@ public function setRefreshTokenTTL(DateInterval $refreshTokenTTL): void
129127
/**
130128
* Set the private key
131129
*/
132-
public function setPrivateKey(CryptKeyInterface $key): void
130+
public function setPrivateKey(CryptKeyInterface $privateKey): void
133131
{
134-
$this->privateKey = $key;
132+
$this->privateKey = $privateKey;
135133
}
136134

137135
public function setDefaultScope(string $scope): void
138136
{
139137
$this->defaultScope = $scope;
140138
}
141139

142-
public function revokeRefreshTokens(bool $revokeRefreshTokens): void
140+
public function revokeRefreshTokens(bool $willRevoke): void
143141
{
144-
$this->revokeRefreshTokens = $revokeRefreshTokens;
142+
$this->revokeRefreshTokens = $willRevoke;
145143
}
146144

147145
/**
@@ -161,13 +159,9 @@ protected function validateClient(ServerRequestInterface $request): ClientEntity
161159
$client = $this->getClientEntityOrFail($clientId, $request);
162160

163161
// If a redirect URI is provided ensure it matches what is pre-registered
164-
$redirectUri = $this->getRequestParameter('redirect_uri', $request, null);
162+
$redirectUri = $this->getRequestParameter('redirect_uri', $request);
165163

166164
if ($redirectUri !== null) {
167-
if (!is_string($redirectUri)) {
168-
throw OAuthServerException::invalidRequest('redirect_uri');
169-
}
170-
171165
$this->validateRedirectUri($redirectUri, $client, $request);
172166
}
173167

@@ -183,6 +177,7 @@ protected function validateClient(ServerRequestInterface $request): ClientEntity
183177
* doesn't actually enforce non-null returns/exception-on-no-client so
184178
* getClientEntity might return null. By contrast, this method will
185179
* always either return a ClientEntityInterface or throw.
180+
* @throws OAuthServerException
186181
*/
187182
protected function getClientEntityOrFail(string $clientId, ServerRequestInterface $request): ClientEntityInterface
188183
{
@@ -200,25 +195,22 @@ protected function getClientEntityOrFail(string $clientId, ServerRequestInterfac
200195
* Gets the client credentials from the request from the request body or
201196
* the Http Basic Authorization header
202197
*
203-
* @return mixed[]
198+
* @return array{0:non-empty-string,1:string}
199+
* @throws OAuthServerException
204200
*/
205201
protected function getClientCredentials(ServerRequestInterface $request): array
206202
{
207203
[$basicAuthUser, $basicAuthPassword] = $this->getBasicAuthCredentials($request);
208204

209205
$clientId = $this->getRequestParameter('client_id', $request, $basicAuthUser);
210206

211-
if (is_null($clientId)) {
207+
if ($clientId === null) {
212208
throw OAuthServerException::invalidRequest('client_id');
213209
}
214210

215211
$clientSecret = $this->getRequestParameter('client_secret', $request, $basicAuthPassword);
216212

217-
if ($clientSecret !== null && !is_string($clientSecret)) {
218-
throw OAuthServerException::invalidRequest('client_secret');
219-
}
220-
221-
return [$clientId, $clientSecret];
213+
return [$clientId, $clientSecret ?? ''];
222214
}
223215

224216
/**
@@ -279,19 +271,43 @@ public function validateScopes(string|array|null $scopes, string $redirectUri =
279271
*/
280272
private function convertScopesQueryStringToArray(string $scopes): array
281273
{
282-
return array_filter(explode(self::SCOPE_DELIMITER_STRING, trim($scopes)), function ($scope) {
283-
return $scope !== '';
284-
});
274+
return array_filter(explode(self::SCOPE_DELIMITER_STRING, trim($scopes)), static fn($scope) => $scope !== '');
285275
}
286276

287277
/**
288-
* Retrieve request parameter.
278+
* Parse request parameter.
279+
* @param array<array-key, mixed> $request
280+
* @return non-empty-string|null
281+
* @throws OAuthServerException
289282
*/
290-
protected function getRequestParameter(string $parameter, ServerRequestInterface $request, mixed $default = null): mixed
283+
private static function parseParam(string $parameter, array $request, ?string $default = null): ?string
291284
{
292-
$requestParameters = (array) $request->getParsedBody();
285+
$value = $request[$parameter] ?? '';
286+
if (is_scalar($value)) {
287+
$value = trim((string)$value);
288+
} else {
289+
throw OAuthServerException::invalidRequest($parameter);
290+
}
291+
292+
if ($value === '') {
293+
$value = $default === null ? null : trim($default);
294+
if ($value === '') {
295+
$value = null;
296+
}
297+
}
298+
299+
return $value;
300+
}
293301

294-
return $requestParameters[$parameter] ?? $default;
302+
/**
303+
* Retrieve request parameter.
304+
*
305+
* @return non-empty-string|null
306+
* @throws OAuthServerException
307+
*/
308+
protected function getRequestParameter(string $parameter, ServerRequestInterface $request, ?string $default = null): ?string
309+
{
310+
return self::parseParam($parameter, (array) $request->getParsedBody(), $default);
295311
}
296312

297313
/**
@@ -301,7 +317,7 @@ protected function getRequestParameter(string $parameter, ServerRequestInterface
301317
* not exist, or is otherwise an invalid HTTP Basic header, return
302318
* [null, null].
303319
*
304-
* @return string[]|null[]
320+
* @return array{0:non-empty-string,1:string}|array{0:null,1:null}
305321
*/
306322
protected function getBasicAuthCredentials(ServerRequestInterface $request): array
307323
{
@@ -310,7 +326,7 @@ protected function getBasicAuthCredentials(ServerRequestInterface $request): arr
310326
}
311327

312328
$header = $request->getHeader('Authorization')[0];
313-
if (strpos($header, 'Basic ') !== 0) {
329+
if (!str_starts_with($header, 'Basic ')) {
314330
return [null, null];
315331
}
316332

@@ -320,35 +336,47 @@ protected function getBasicAuthCredentials(ServerRequestInterface $request): arr
320336
return [null, null];
321337
}
322338

323-
if (strpos($decoded, ':') === false) {
339+
if (!str_contains($decoded, ':')) {
324340
return [null, null]; // HTTP Basic header without colon isn't valid
325341
}
326342

327-
return explode(':', $decoded, 2);
343+
[$username, $password] = explode(':', $decoded, 2);
344+
345+
if ($username === '') {
346+
return [null, null];
347+
}
348+
349+
return [$username, $password];
328350
}
329351

330352
/**
331353
* Retrieve query string parameter.
354+
* @return non-empty-string|null
355+
* @throws OAuthServerException
332356
*/
333-
protected function getQueryStringParameter(string $parameter, ServerRequestInterface $request, mixed $default = null): mixed
357+
protected function getQueryStringParameter(string $parameter, ServerRequestInterface $request, ?string $default = null): ?string
334358
{
335-
return isset($request->getQueryParams()[$parameter]) ? $request->getQueryParams()[$parameter] : $default;
359+
return self::parseParam($parameter, $request->getQueryParams(), $default);
336360
}
337361

338362
/**
339363
* Retrieve cookie parameter.
364+
* @return non-empty-string|null
365+
* @throws OAuthServerException
340366
*/
341-
protected function getCookieParameter(string $parameter, ServerRequestInterface $request, mixed $default = null): mixed
367+
protected function getCookieParameter(string $parameter, ServerRequestInterface $request, ?string $default = null): ?string
342368
{
343-
return isset($request->getCookieParams()[$parameter]) ? $request->getCookieParams()[$parameter] : $default;
369+
return self::parseParam($parameter, $request->getCookieParams(), $default);
344370
}
345371

346372
/**
347373
* Retrieve server parameter.
374+
* @return non-empty-string|null
375+
* @throws OAuthServerException
348376
*/
349-
protected function getServerParameter(string $parameter, ServerRequestInterface $request, mixed $default = null): mixed
377+
protected function getServerParameter(string $parameter, ServerRequestInterface $request, ?string $default = null): ?string
350378
{
351-
return isset($request->getServerParams()[$parameter]) ? $request->getServerParams()[$parameter] : $default;
379+
return self::parseParam($parameter, $request->getServerParams(), $default);
352380
}
353381

354382
/**

‎src/Grant/AuthCodeGrant.php

+4-5
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,6 @@
4141
use function implode;
4242
use function in_array;
4343
use function is_array;
44-
use function is_string;
4544
use function json_decode;
4645
use function json_encode;
4746
use function preg_match;
@@ -106,9 +105,9 @@ public function respondToAccessTokenRequest(
106105
$this->validateClient($request);
107106
}
108107

109-
$encryptedAuthCode = $this->getRequestParameter('code', $request, null);
108+
$encryptedAuthCode = $this->getRequestParameter('code', $request);
110109

111-
if (!is_string($encryptedAuthCode)) {
110+
if ($encryptedAuthCode === null) {
112111
throw OAuthServerException::invalidRequest('code');
113112
}
114113

@@ -128,7 +127,7 @@ public function respondToAccessTokenRequest(
128127
throw OAuthServerException::invalidRequest('code', 'Cannot decrypt the authorization code', $e);
129128
}
130129

131-
$codeVerifier = $this->getRequestParameter('code_verifier', $request, null);
130+
$codeVerifier = $this->getRequestParameter('code_verifier', $request);
132131

133132
// If a code challenge isn't present but a code verifier is, reject the request to block PKCE downgrade attack
134133
if (!isset($authCodePayload->code_challenge) && $codeVerifier !== null) {
@@ -219,7 +218,7 @@ private function validateAuthorizationCode(
219218
}
220219

221220
// The redirect URI is required in this request
222-
$redirectUri = $this->getRequestParameter('redirect_uri', $request, null);
221+
$redirectUri = $this->getRequestParameter('redirect_uri', $request);
223222
if ($authCodePayload->redirect_uri !== '' && $redirectUri === null) {
224223
throw OAuthServerException::invalidRequest('redirect_uri');
225224
}

‎src/Grant/DeviceCodeGrant.php

+1-3
Original file line numberDiff line numberDiff line change
@@ -304,9 +304,7 @@ protected function generateUserCode(int $length = 8): string
304304

305305
return $userCode;
306306
// @codeCoverageIgnoreStart
307-
} catch (TypeError $e) {
308-
throw OAuthServerException::serverError('An unexpected error has occurred', $e);
309-
} catch (Error $e) {
307+
} catch (TypeError | Error $e) {
310308
throw OAuthServerException::serverError('An unexpected error has occurred', $e);
311309
} catch (Exception $e) {
312310
// If you get this message, the CSPRNG failed hard.

‎src/Grant/PasswordGrant.php

+4-12
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,6 @@
2626
use League\OAuth2\Server\ResponseTypes\ResponseTypeInterface;
2727
use Psr\Http\Message\ServerRequestInterface;
2828

29-
use function is_string;
30-
3129
/**
3230
* Password grant class.
3331
*/
@@ -84,17 +82,11 @@ public function respondToAccessTokenRequest(
8482
*/
8583
protected function validateUser(ServerRequestInterface $request, ClientEntityInterface $client): UserEntityInterface
8684
{
87-
$username = $this->getRequestParameter('username', $request);
88-
89-
if (!is_string($username)) {
90-
throw OAuthServerException::invalidRequest('username');
91-
}
85+
$username = $this->getRequestParameter('username', $request)
86+
?? throw OAuthServerException::invalidRequest('username');
9287

93-
$password = $this->getRequestParameter('password', $request);
94-
95-
if (!is_string($password)) {
96-
throw OAuthServerException::invalidRequest('password');
97-
}
88+
$password = $this->getRequestParameter('password', $request)
89+
?? throw OAuthServerException::invalidRequest('password');
9890

9991
$user = $this->userRepository->getUserEntityByUserCredentials(
10092
$username,

‎src/Grant/RefreshTokenGrant.php

+2-6
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
2626

2727
use function implode;
2828
use function in_array;
29-
use function is_string;
3029
use function json_decode;
3130
use function time;
3231

@@ -103,11 +102,8 @@ public function respondToAccessTokenRequest(
103102
*/
104103
protected function validateOldRefreshToken(ServerRequestInterface $request, string $clientId): array
105104
{
106-
$encryptedRefreshToken = $this->getRequestParameter('refresh_token', $request);
107-
108-
if (!is_string($encryptedRefreshToken)) {
109-
throw OAuthServerException::invalidRequest('refresh_token');
110-
}
105+
$encryptedRefreshToken = $this->getRequestParameter('refresh_token', $request)
106+
?? throw OAuthServerException::invalidRequest('refresh_token');
111107

112108
// Validate refresh token
113109
try {

‎src/ResponseTypes/BearerTokenResponse.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ public function generateHttpResponse(ResponseInterface $response): ResponseInter
7373
* AuthorizationServer::getResponseType() to pull in your version of
7474
* this class rather than the default.
7575
*
76-
* @return mixed[]
76+
* @return array<array-key,mixed>
7777
*/
7878
protected function getExtraParams(AccessTokenEntityInterface $accessToken): array
7979
{

‎src/ResponseTypes/DeviceCodeResponse.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ public function includeVerificationUriComplete(): void
8484
* AuthorizationServer::getResponseType() to pull in your version of
8585
* this class rather than the default.
8686
*
87-
* @return mixed[]
87+
* @return array<array-key,mixed>
8888
*/
8989
protected function getExtraParams(DeviceCodeEntityInterface $deviceCode): array
9090
{

0 commit comments

Comments
 (0)