Skip to content
This repository was archived by the owner on Jan 29, 2020. It is now read-only.

Commit d585429

Browse files
thexpandthexpand
committed
Change the ClientRepository::getClientEntity() implementation and re-write tests
1 parent 5807c4e commit d585429

File tree

3 files changed

+82
-57
lines changed

3 files changed

+82
-57
lines changed

‎src/Repository/Pdo/ClientRepository.php‎

Lines changed: 48 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -21,39 +21,69 @@ class ClientRepository extends AbstractRepository implements ClientRepositoryInt
2121
/**
2222
* {@inheritDoc}
2323
*/
24-
public function getClientEntity(
25-
$clientIdentifier,
26-
$grantType = null,
27-
$clientSecret = null,
28-
$mustValidateSecret = true
29-
) {
30-
$sth = $this->pdo->prepare(
31-
'SELECT * FROM oauth_clients WHERE name = :clientIdentifier'
32-
);
33-
$sth->bindParam(':clientIdentifier', $clientIdentifier);
24+
public function getClientEntity($clientIdentifier) : ?ClientEntityInterface
25+
{
26+
$clientData = $this->getClientData($clientIdentifier);
3427

35-
if (false === $sth->execute()) {
28+
if (empty($clientData)) {
3629
return null;
3730
}
38-
$row = $sth->fetch();
39-
if (empty($row) || ! $this->isGranted($row, $grantType)) {
31+
32+
return new ClientEntity(
33+
$clientIdentifier,
34+
$clientData['name'] ?? '',
35+
$clientData['redirect'] ?? '',
36+
);
37+
}
38+
39+
/**
40+
* {@inheritDoc}
41+
*/
42+
public function validateClient($clientIdentifier, $clientSecret, $grantType) : bool
43+
{
44+
$clientData = $this->getClientData($clientIdentifier);
45+
46+
if (empty($clientData)) {
47+
return false;
48+
}
49+
50+
if (! $this->isGranted($clientData, $grantType)) {
51+
return false;
52+
}
53+
54+
if (empty($clientData['secret']) || ! password_verify((string) $clientSecret, $clientData['secret'])) {
55+
return false;
56+
}
57+
58+
return true;
59+
}
60+
61+
protected function getClientData(string $clientIdentifier) : ?array
62+
{
63+
$statement = $this->pdo->prepare(
64+
'SELECT * FROM oauth_clients WHERE name = :clientIdentifier'
65+
);
66+
$statement->bindParam(':clientIdentifier', $clientIdentifier);
67+
68+
if ($statement->execute() === false) {
4069
return null;
4170
}
4271

43-
if ($mustValidateSecret
44-
&& (empty($row['secret']) || ! password_verify((string) $clientSecret, $row['secret']))
45-
) {
72+
$row = $statement->fetch();
73+
74+
if (empty($row)) {
4675
return null;
4776
}
4877

49-
return new ClientEntity($clientIdentifier, $row['name'], $row['redirect']);
78+
return $row;
5079
}
5180

5281
/**
5382
* Check the grantType for the client value, stored in $row
5483
*
55-
* @param array $row
84+
* @param array $row
5685
* @param string $grantType
86+
*
5787
* @return bool
5888
*/
5989
protected function isGranted(array $row, string $grantType = null) : bool
@@ -69,18 +99,4 @@ protected function isGranted(array $row, string $grantType = null) : bool
6999
return true;
70100
}
71101
}
72-
73-
/**
74-
* {@inheritDoc}
75-
*/
76-
public function validateClient($clientIdentifier, $clientSecret, $grantType) : bool
77-
{
78-
$client = $this->getClientEntity(
79-
$clientIdentifier,
80-
$grantType,
81-
$clientSecret
82-
);
83-
84-
return $client instanceof ClientEntityInterface;
85-
}
86102
}

‎test/Pdo/OAuth2PdoMiddlewareTest.php‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212

1313
use DateInterval;
1414
use League\OAuth2\Server\AuthorizationServer;
15+
use League\OAuth2\Server\CodeChallengeVerifiers\S256Verifier;
1516
use League\OAuth2\Server\Grant\AuthCodeGrant;
1617
use League\OAuth2\Server\Grant\ClientCredentialsGrant;
1718
use League\OAuth2\Server\Grant\ImplicitGrant;
@@ -64,6 +65,8 @@ class OAuth2PdoMiddlewareTest extends TestCase
6465
const PRIVATE_KEY = __DIR__ .'/../TestAsset/private.key';
6566
const ENCRYPTION_KEY = 'T2x2+1OGrEzfS+01OUmwhOcJiGmE58UD1fllNn6CGcQ=';
6667

68+
const CODE_VERIFIER = 'dBjftJeZ4CVP-mB92K27uhbUJU1p1r_wW1gFWFOEjXk';
69+
6770
/** @var AccessTokenRepository */
6871
private $accessTokenRepository;
6972

@@ -269,6 +272,17 @@ public function testProcessGetAuthorizationCode()
269272
'scope' => 'test',
270273
'state' => $state
271274
];
275+
276+
$codeVerifier = new S256Verifier();
277+
278+
$params['code_challenge_method'] = $codeVerifier->getMethod();
279+
$params['code_verifier'] = self::CODE_VERIFIER;
280+
$params['code_challenge'] = strtr(
281+
rtrim(base64_encode(hash('sha256', self::CODE_VERIFIER, true)), '='),
282+
'+/',
283+
'-_'
284+
);
285+
272286
$request = $this->buildServerRequest(
273287
'GET',
274288
'/auth_code?' . http_build_query($params),
@@ -324,8 +338,10 @@ public function testProcessFromAuthorizationCode(string $code)
324338
'client_id' => 'client_test2',
325339
'client_secret' => 'test',
326340
'redirect_uri' => '/redirect',
327-
'code' => $code
341+
'code' => $code,
342+
'code_verifier' => self::CODE_VERIFIER,
328343
];
344+
329345
$request = $this->buildServerRequest(
330346
'POST',
331347
'/access_token',

‎test/Repository/Pdo/ClientRepositoryTest.php‎

Lines changed: 17 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -36,10 +36,7 @@ public function testGetClientEntityReturnsNullIfStatementExecutionReturnsFalse()
3636
->will([$statement, 'reveal']);
3737

3838
$this->assertNull(
39-
$this->repo ->getClientEntity(
40-
'client_id',
41-
'grant_type'
42-
)
39+
$this->repo ->getClientEntity('client_id')
4340
);
4441
}
4542

@@ -59,10 +56,7 @@ public function testGetClientEntityReturnsNullIfNoRowReturned()
5956
$client = $this->prophesize(ClientEntityInterface::class);
6057

6158
$this->assertNull(
62-
$this->repo ->getClientEntity(
63-
'client_id',
64-
'grant_type'
65-
)
59+
$this->repo ->getClientEntity('client_id')
6660
);
6761
}
6862

@@ -85,7 +79,7 @@ public function invalidGrants()
8579
/**
8680
* @dataProvider invalidGrants
8781
*/
88-
public function testGetClientEntityReturnsNullIfRowIndicatesNotGranted(string $grantType, array $rowReturned)
82+
public function testValidateClientReturnsFalseIfRowIndicatesNotGranted(string $grantType, array $rowReturned)
8983
{
9084
$statement = $this->prophesize(PDOStatement::class);
9185
$statement->bindParam(':clientIdentifier', 'client_id')->shouldBeCalled();
@@ -100,22 +94,23 @@ public function testGetClientEntityReturnsNullIfRowIndicatesNotGranted(string $g
10094

10195
$client = $this->prophesize(ClientEntityInterface::class);
10296

103-
$this->assertNull(
104-
$this->repo ->getClientEntity(
97+
$this->assertFalse(
98+
$this->repo ->validateClient(
10599
'client_id',
100+
'',
106101
$grantType
107102
)
108103
);
109104
}
110105

111-
public function testGetClientReturnsNullForNonMatchingClientSecret()
106+
public function testValidateClientReturnsFalseForNonMatchingClientSecret()
112107
{
113108
$statement = $this->prophesize(PDOStatement::class);
114109
$statement->bindParam(':clientIdentifier', 'client_id')->shouldBeCalled();
115110
$statement->execute()->will(function () use ($statement) {
116111
$statement->fetch()->willReturn([
117112
'password_client' => true,
118-
'secret' => 'unknown password',
113+
'secret' => 'bar',
119114
]);
120115
return null;
121116
});
@@ -126,17 +121,16 @@ public function testGetClientReturnsNullForNonMatchingClientSecret()
126121

127122
$client = $this->prophesize(ClientEntityInterface::class);
128123

129-
$this->assertNull(
130-
$this->repo ->getClientEntity(
124+
$this->assertFalse(
125+
$this->repo ->validateClient(
131126
'client_id',
132-
'password_client',
133-
'password',
134-
true
127+
'foo',
128+
'password'
135129
)
136130
);
137131
}
138132

139-
public function testGetClientReturnsNullForEmptyClientSecret()
133+
public function testValidateClientReturnsFalseForEmptyClientSecret()
140134
{
141135
$statement = $this->prophesize(PDOStatement::class);
142136
$statement->bindParam(':clientIdentifier', 'client_id')->shouldBeCalled();
@@ -154,12 +148,11 @@ public function testGetClientReturnsNullForEmptyClientSecret()
154148

155149
$client = $this->prophesize(ClientEntityInterface::class);
156150

157-
$this->assertNull(
158-
$this->repo ->getClientEntity(
151+
$this->assertFalse(
152+
$this->repo ->validateClient(
159153
'client_id',
160-
'password_client',
161-
'password',
162-
true
154+
'foo',
155+
'password'
163156
)
164157
);
165158
}

0 commit comments

Comments
 (0)