Skip to content

Commit 1f75b46

Browse files
eugene-borovoveugene-borovov
committed
Merge branch 'master' into psr-event-dispatcher
# Conflicts: # composer.json # src/Grant/AuthCodeGrant.php # src/Grant/ClientCredentialsGrant.php # src/Grant/PasswordGrant.php # src/Grant/RefreshTokenGrant.php
2 parents 924ed04 + a603133 commit 1f75b46

File tree

9 files changed

+167
-9
lines changed

9 files changed

+167
-9
lines changed

‎CHANGELOG.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,12 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
55
and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).
66

77
## [Unreleased]
8+
### Added
9+
- Events emitted now include the refresh token and access token payloads (PR #1211)
10+
811
### Fixed
912
- The server will now only recognise and handle an authorization header if the value of the header is non-empty. This is to circumvent issues where some common frameworks set this header even if no value is present (PR #1170)
13+
- Added type validation for redirect uri, client ID, client secret, scopes, auth code, state, username, and password inputs (PR #1210)
1014

1115
## [8.2.4] - released 2020-12-10
1216
### Fixed

‎composer.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
"require": {
77
"php": "^7.2 || ^8.0",
88
"ext-openssl": "*",
9-
"lcobucci/jwt": "^3.4 || ^4.0",
9+
"lcobucci/jwt": "^3.4 || ~4.0.0",
1010
"psr/event-dispatcher": "^1.0",
1111
"psr/http-message": "^1.0.1",
1212
"defuse/php-encryption": "^2.2.1",

‎src/Grant/AbstractGrant.php‎

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,10 @@ protected function validateClient(ServerRequestInterface $request)
191191
$redirectUri = $this->getRequestParameter('redirect_uri', $request, null);
192192

193193
if ($redirectUri !== null) {
194+
if (!\is_string($redirectUri)) {
195+
throw OAuthServerException::invalidRequest('redirect_uri');
196+
}
197+
194198
$this->validateRedirectUri($redirectUri, $client, $request);
195199
}
196200

@@ -238,12 +242,16 @@ protected function getClientCredentials(ServerRequestInterface $request)
238242

239243
$clientId = $this->getRequestParameter('client_id', $request, $basicAuthUser);
240244

241-
if (\is_null($clientId)) {
245+
if (!\is_string($clientId)) {
242246
throw OAuthServerException::invalidRequest('client_id');
243247
}
244248

245249
$clientSecret = $this->getRequestParameter('client_secret', $request, $basicAuthPassword);
246250

251+
if ($clientSecret !== null && !\is_string($clientSecret)) {
252+
throw OAuthServerException::invalidRequest('client_secret');
253+
}
254+
247255
return [$clientId, $clientSecret];
248256
}
249257

@@ -287,10 +295,16 @@ protected function validateRedirectUri(
287295
*/
288296
public function validateScopes($scopes, $redirectUri = null)
289297
{
290-
if (!\is_array($scopes)) {
298+
if ($scopes === null) {
299+
$scopes = [];
300+
} elseif (\is_string($scopes)) {
291301
$scopes = $this->convertScopesQueryStringToArray($scopes);
292302
}
293303

304+
if (!\is_array($scopes)) {
305+
throw OAuthServerException::invalidRequest('scope');
306+
}
307+
294308
$validScopes = [];
295309

296310
foreach ($scopes as $scopeItem) {
@@ -313,7 +327,7 @@ public function validateScopes($scopes, $redirectUri = null)
313327
*
314328
* @return array
315329
*/
316-
private function convertScopesQueryStringToArray($scopes)
330+
private function convertScopesQueryStringToArray(string $scopes)
317331
{
318332
return \array_filter(\explode(self::SCOPE_DELIMITER_STRING, \trim($scopes)), function ($scope) {
319333
return !empty($scope);

‎src/Grant/AuthCodeGrant.php‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ public function respondToAccessTokenRequest(
108108

109109
$encryptedAuthCode = $this->getRequestParameter('code', $request, null);
110110

111-
if ($encryptedAuthCode === null) {
111+
if (!\is_string($encryptedAuthCode)) {
112112
throw OAuthServerException::invalidRequest('code');
113113
}
114114

@@ -262,6 +262,10 @@ public function validateAuthorizationRequest(ServerRequestInterface $request)
262262
$redirectUri = $this->getQueryStringParameter('redirect_uri', $request);
263263

264264
if ($redirectUri !== null) {
265+
if (!\is_string($redirectUri)) {
266+
throw OAuthServerException::invalidRequest('redirect_uri');
267+
}
268+
265269
$this->validateRedirectUri($redirectUri, $client, $request);
266270
} elseif (empty($client->getRedirectUri()) ||
267271
(\is_array($client->getRedirectUri()) && \count($client->getRedirectUri()) !== 1)) {

‎src/Grant/ImplicitGrant.php‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ public function validateAuthorizationRequest(ServerRequestInterface $request)
120120
$this->getServerParameter('PHP_AUTH_USER', $request)
121121
);
122122

123-
if (\is_null($clientId)) {
123+
if (!\is_string($clientId)) {
124124
throw OAuthServerException::invalidRequest('client_id');
125125
}
126126

@@ -129,6 +129,10 @@ public function validateAuthorizationRequest(ServerRequestInterface $request)
129129
$redirectUri = $this->getQueryStringParameter('redirect_uri', $request);
130130

131131
if ($redirectUri !== null) {
132+
if (!\is_string($redirectUri)) {
133+
throw OAuthServerException::invalidRequest('redirect_uri');
134+
}
135+
132136
$this->validateRedirectUri($redirectUri, $client, $request);
133137
} elseif (\is_array($client->getRedirectUri()) && \count($client->getRedirectUri()) !== 1
134138
|| empty($client->getRedirectUri())) {
@@ -147,6 +151,10 @@ public function validateAuthorizationRequest(ServerRequestInterface $request)
147151

148152
$stateParameter = $this->getQueryStringParameter('state', $request);
149153

154+
if ($stateParameter !== null && !\is_string($stateParameter)) {
155+
throw OAuthServerException::invalidRequest('state');
156+
}
157+
150158
$authorizationRequest = new AuthorizationRequest();
151159
$authorizationRequest->setGrantTypeId($this->getIdentifier());
152160
$authorizationRequest->setClient($client);

‎src/Grant/PasswordGrant.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,13 +86,13 @@ protected function validateUser(ServerRequestInterface $request, ClientEntityInt
8686
{
8787
$username = $this->getRequestParameter('username', $request);
8888

89-
if (\is_null($username)) {
89+
if (!\is_string($username)) {
9090
throw OAuthServerException::invalidRequest('username');
9191
}
9292

9393
$password = $this->getRequestParameter('password', $request);
9494

95-
if (\is_null($password)) {
95+
if (!\is_string($password)) {
9696
throw OAuthServerException::invalidRequest('password');
9797
}
9898

‎src/Grant/RefreshTokenGrant.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ public function respondToAccessTokenRequest(
9494
protected function validateOldRefreshToken(ServerRequestInterface $request, $clientId)
9595
{
9696
$encryptedRefreshToken = $this->getRequestParameter('refresh_token', $request);
97-
if (\is_null($encryptedRefreshToken)) {
97+
if (!\is_string($encryptedRefreshToken)) {
9898
throw OAuthServerException::invalidRequest('refresh_token');
9999
}
100100

‎tests/Grant/AbstractGrantTest.php‎

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,70 @@ public function testHttpBasicNoColon()
8989
$this->assertSame([null, null], $basicAuthMethod->invoke($grantMock, $serverRequest));
9090
}
9191

92+
public function testGetClientCredentialsClientIdNotAString()
93+
{
94+
$clientRepositoryMock = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock();
95+
96+
/** @var AbstractGrant $grantMock */
97+
$grantMock = $this->getMockForAbstractClass(AbstractGrant::class);
98+
$grantMock->setClientRepository($clientRepositoryMock);
99+
100+
$abstractGrantReflection = new \ReflectionClass($grantMock);
101+
102+
$serverRequest = new ServerRequest(
103+
[],
104+
[],
105+
null,
106+
'POST',
107+
'php://input',
108+
[],
109+
[],
110+
[],
111+
[
112+
'client_id' => ['not', 'a', 'string'],
113+
'client_secret' => 'client_secret',
114+
]
115+
);
116+
$getClientCredentialsMethod = $abstractGrantReflection->getMethod('getClientCredentials');
117+
$getClientCredentialsMethod->setAccessible(true);
118+
119+
$this->expectException(\League\OAuth2\Server\Exception\OAuthServerException::class);
120+
121+
$getClientCredentialsMethod->invoke($grantMock, $serverRequest, true, true);
122+
}
123+
124+
public function testGetClientCredentialsClientSecretNotAString()
125+
{
126+
$clientRepositoryMock = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock();
127+
128+
/** @var AbstractGrant $grantMock */
129+
$grantMock = $this->getMockForAbstractClass(AbstractGrant::class);
130+
$grantMock->setClientRepository($clientRepositoryMock);
131+
132+
$abstractGrantReflection = new \ReflectionClass($grantMock);
133+
134+
$serverRequest = new ServerRequest(
135+
[],
136+
[],
137+
null,
138+
'POST',
139+
'php://input',
140+
[],
141+
[],
142+
[],
143+
[
144+
'client_id' => 'client_id',
145+
'client_secret' => ['not', 'a', 'string'],
146+
]
147+
);
148+
$getClientCredentialsMethod = $abstractGrantReflection->getMethod('getClientCredentials');
149+
$getClientCredentialsMethod->setAccessible(true);
150+
151+
$this->expectException(\League\OAuth2\Server\Exception\OAuthServerException::class);
152+
153+
$getClientCredentialsMethod->invoke($grantMock, $serverRequest, true, true);
154+
}
155+
92156
public function testValidateClientPublic()
93157
{
94158
$client = new ClientEntity();
@@ -261,6 +325,32 @@ public function testValidateClientInvalidRedirectUriArray()
261325
$validateClientMethod->invoke($grantMock, $serverRequest, true, true);
262326
}
263327

328+
public function testValidateClientMalformedRedirectUri()
329+
{
330+
$client = new ClientEntity();
331+
$client->setRedirectUri('http://foo/bar');
332+
$clientRepositoryMock = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock();
333+
$clientRepositoryMock->method('getClientEntity')->willReturn($client);
334+
335+
/** @var AbstractGrant $grantMock */
336+
$grantMock = $this->getMockForAbstractClass(AbstractGrant::class);
337+
$grantMock->setClientRepository($clientRepositoryMock);
338+
339+
$abstractGrantReflection = new \ReflectionClass($grantMock);
340+
341+
$serverRequest = (new ServerRequest())->withParsedBody([
342+
'client_id' => 'foo',
343+
'redirect_uri' => ['not', 'a', 'string'],
344+
]);
345+
346+
$validateClientMethod = $abstractGrantReflection->getMethod('validateClient');
347+
$validateClientMethod->setAccessible(true);
348+
349+
$this->expectException(\League\OAuth2\Server\Exception\OAuthServerException::class);
350+
351+
$validateClientMethod->invoke($grantMock, $serverRequest, true, true);
352+
}
353+
264354
public function testValidateClientBadClient()
265355
{
266356
$clientRepositoryMock = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock();

‎tests/Grant/AuthCodeGrantTest.php‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1112,6 +1112,44 @@ public function testRespondToAccessTokenRequestWithRefreshTokenInsteadOfAuthCode
11121112
}
11131113
}
11141114

1115+
public function testRespondToAccessTokenRequestWithAuthCodeNotAString()
1116+
{
1117+
$client = new ClientEntity();
1118+
$client->setRedirectUri('http://foo/bar');
1119+
1120+
$clientRepositoryMock = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock();
1121+
$clientRepositoryMock->method('getClientEntity')->willReturn($client);
1122+
1123+
$grant = new AuthCodeGrant(
1124+
$this->getMockBuilder(AuthCodeRepositoryInterface::class)->getMock(),
1125+
$this->getMockBuilder(RefreshTokenRepositoryInterface::class)->getMock(),
1126+
new DateInterval('PT10M')
1127+
);
1128+
1129+
$grant->setClientRepository($clientRepositoryMock);
1130+
$grant->setEncryptionKey($this->cryptStub->getKey());
1131+
1132+
$request = new ServerRequest(
1133+
[],
1134+
[],
1135+
null,
1136+
'POST',
1137+
'php://input',
1138+
[],
1139+
[],
1140+
[],
1141+
[
1142+
'grant_type' => 'authorization_code',
1143+
'client_id' => 'foo',
1144+
'redirect_uri' => 'http://foo/bar',
1145+
'code' => ['not', 'a', 'string'],
1146+
]
1147+
);
1148+
1149+
$this->expectException(\League\OAuth2\Server\Exception\OAuthServerException::class);
1150+
$grant->respondToAccessTokenRequest($request, new StubResponseType(), new DateInterval('PT10M'));
1151+
}
1152+
11151153
public function testRespondToAccessTokenRequestExpiredCode()
11161154
{
11171155
$client = new ClientEntity();

0 commit comments

Comments
 (0)