Skip to content

Commit 3c37d60

Browse files
chriskacerguischriskacerguis
committed
Refactor filters and models for improved type safety and code clarity
- Updated AuthFilter, CorsFilter, RateLimitFilter to enforce stricter type hints and nullability. - Enhanced error handling and response generation in filters. - Refactored Format class to use mixed types and improved string handling. - Added type annotations and docblocks in AccessModel, KeyModel, LimitModel, and LogModel for better code documentation. - Improved test stubs in ApiKeyFilterTest and FiltersTest for better type safety. - Ensured consistent use of ResponseInterface in response handling across filters and controllers.
1 parent 3e7693d commit 3c37d60

16 files changed

+429
-184
lines changed

‎.gitignore‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,4 +19,5 @@ application/logs/*
1919
!application/logs/.htaccess
2020
/vendor/
2121
*.phar
22-
.phpunit.result.cache
22+
.phpunit.result.cache
23+
phpstan.neon.dist

‎README.md‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -254,6 +254,15 @@ php spark migrate -n 'chriskacerguis\RestServer'
254254
curl -H 'X-API-KEY: YOUR_KEY' http://localhost:8080/api/users
255255
```
256256

257+
## Testing
258+
259+
```bash
260+
php phpcs.phar --standard=phpcs.xml
261+
php phpunit.phar -c phpunit.xml.dist --testdox
262+
php -d memory_limit=512M phpstan.phar analyse src --no-progress -c phpstan.neon.dist
263+
```
264+
265+
257266
## License
258267

259268
MIT

‎src/Config/Rest.php‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ class Rest extends BaseConfig
1313

1414
// Output formats
1515
public string $defaultFormat = 'json';
16+
/** @var array<string> */
1617
public array $supportedFormats = [
1718
'json', 'array', 'csv', 'html', 'jsonp', 'php', 'serialized', 'xml',
1819
];
@@ -30,6 +31,7 @@ class Rest extends BaseConfig
3031
public $auth = false;
3132
/** @var ''|'ldap'|'library' */
3233
public string $authSource = '';
34+
/** @var array<string,string> */
3335
public array $validLogins = ['admin' => '1234'];
3436

3537
// Allow Authentication and API Keys
@@ -63,7 +65,7 @@ class Rest extends BaseConfig
6365
/**
6466
* Override the model class used to validate API keys (for testing or customization)
6567
*
66-
* @var class-string
68+
* @var class-string<\chriskacerguis\RestServer\Models\KeyModel>
6769
*/
6870
public string $keyModelClass = 'chriskacerguis\\RestServer\\Models\\KeyModel';
6971

@@ -86,7 +88,7 @@ class Rest extends BaseConfig
8688
/**
8789
* Override the model class used to persist limits (for testing or customization)
8890
*
89-
* @var class-string
91+
* @var class-string<\chriskacerguis\RestServer\Models\LimitModel>
9092
*/
9193
public string $limitModelClass = 'chriskacerguis\\RestServer\\Models\\LimitModel';
9294

@@ -101,11 +103,15 @@ class Rest extends BaseConfig
101103

102104
// CORS
103105
public bool $checkCors = false;
106+
/** @var array<string> */
104107
public array $allowedCorsHeaders = [
105108
'Origin', 'X-Requested-With', 'Content-Type', 'Accept', 'Access-Control-Request-Method',
106109
];
110+
/** @var array<string> */
107111
public array $allowedCorsMethods = ['GET', 'POST', 'OPTIONS', 'PUT', 'PATCH', 'DELETE'];
108112
public bool $allowAnyCorsDomain = false;
113+
/** @var array<string> */
109114
public array $allowedCorsOrigins = [];
115+
/** @var array<string,string> */
110116
public array $forcedCorsHeaders = [];
111117
}

‎src/Filters/ApiKeyFilter.php‎

Lines changed: 30 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,30 +13,49 @@
1313

1414
class ApiKeyFilter implements FilterInterface
1515
{
16-
public function before(RequestInterface $request, $arguments = null)
16+
/**
17+
* @param array<string,mixed>|null $arguments
18+
*/
19+
public function before(RequestInterface $request, $arguments = null): ?ResponseInterface
1720
{
1821
$config = config(RestConfig::class);
22+
if (!$config instanceof RestConfig) {
23+
$config = new RestConfig();
24+
}
1925
if (!$config->enableKeys) {
2026
return null;
2127
}
2228

23-
$headerName = $config->keyHeaderName;
24-
$apiKey = $request->getHeaderLine($headerName)
25-
?: ($request->getGet('api_key') ?? ($request->getGet($headerName) ?? ''));
29+
$headerName = (string) $config->keyHeaderName;
30+
$apiKey = $request->getHeaderLine($headerName);
31+
if ($apiKey === '') {
32+
$queryKey = $request->getGet('api_key');
33+
if (!is_string($queryKey) || $queryKey === '') {
34+
$queryKey = $request->getGet($headerName);
35+
}
36+
$apiKey = is_string($queryKey) ? $queryKey : '';
37+
}
2638

2739
if ($apiKey === '') {
28-
$response = service('response') ?: new Response(config('App'));
40+
$response = service('response');
41+
if (!$response instanceof ResponseInterface) {
42+
$response = new Response(config('App'));
43+
}
2944
return $response->setStatusCode(401)->setJSON([
3045
$config->statusFieldName => false,
3146
$config->messageFieldName => 'API key missing',
3247
]);
3348
}
3449

3550
$modelClass = $config->keyModelClass ?? KeyModel::class;
51+
/** @var KeyModel $model */
3652
$model = new $modelClass();
37-
$row = $model->findValidKey($apiKey);
53+
$row = $model->findValidKey((string) $apiKey);
3854
if (!$row) {
39-
$response = service('response') ?: new Response(config('App'));
55+
$response = service('response');
56+
if (!$response instanceof ResponseInterface) {
57+
$response = new Response(config('App'));
58+
}
4059
return $response->setStatusCode(401)->setJSON([
4160
$config->statusFieldName => false,
4261
$config->messageFieldName => 'Invalid or expired API key',
@@ -46,7 +65,10 @@ public function before(RequestInterface $request, $arguments = null)
4665
return null;
4766
}
4867

49-
public function after(RequestInterface $request, ResponseInterface $response, $arguments = null)
68+
/**
69+
* @param array<string,mixed>|null $arguments
70+
*/
71+
public function after(RequestInterface $request, ResponseInterface $response, $arguments = null): void
5072
{
5173
// no-op
5274
}

‎src/Filters/AuthFilter.php‎

Lines changed: 68 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -12,75 +12,89 @@
1212

1313
class AuthFilter implements FilterInterface
1414
{
15-
public function before(RequestInterface $request, $arguments = null)
15+
/**
16+
* @param array<string,mixed>|null $arguments
17+
*/
18+
public function before(RequestInterface $request, $arguments = null): ?ResponseInterface
1619
{
1720
$config = config(RestConfig::class);
18-
19-
if ($config->auth === false || $config->auth === '') {
20-
// Auth disabled
21-
return null;
21+
if (!$config instanceof RestConfig) {
22+
$config = new RestConfig();
2223
}
2324

24-
if ($config->auth === 'session') {
25-
$session = service('session');
26-
if ($session && ($session->get('isLoggedIn') === true || $session->get('logged_in') === true)) {
27-
return null;
28-
}
29-
30-
return $this->unauthorizedResponse('basic', $config->realm, 'Session authentication required');
25+
if ($config->auth === false) {
26+
return null; // Auth disabled
3127
}
3228

33-
$authHeader = (string) ($request->getHeaderLine('Authorization') ?? '');
29+
$authHeader = $request->getHeaderLine('Authorization');
3430
$method = strtoupper($request->getMethod());
3531

36-
if ($config->auth === 'basic') {
37-
$creds = $this->parseBasic($authHeader);
38-
if (!$creds) {
39-
return $this->unauthorizedResponse('basic', $config->realm);
40-
}
41-
42-
$isValid = $this->validateCredentials($creds['username'], $creds['password'], $config);
43-
if ($isValid) {
44-
return null;
45-
}
46-
47-
return $this->unauthorizedResponse('basic', $config->realm);
48-
}
49-
50-
if ($config->auth === 'digest') {
51-
$digest = $this->parseDigest($authHeader);
52-
if (!$digest) {
53-
return $this->unauthorizedResponse('digest', $config->realm);
54-
}
55-
56-
$username = $digest['username'] ?? '';
57-
$password = $this->findPasswordForUser($username, $config);
58-
if ($password === null) {
59-
return $this->unauthorizedResponse('digest', $config->realm);
60-
}
61-
62-
$ha1 = md5($username . ':' . $config->realm . ':' . $password);
63-
$ha2 = md5($method . ':' . ($digest['uri'] ?? ''));
32+
switch ($config->auth) {
33+
case 'session':
34+
$session = service('session');
35+
if (
36+
is_object($session)
37+
&& method_exists($session, 'get')
38+
&& (
39+
$session->get('isLoggedIn') === true
40+
|| $session->get('logged_in') === true
41+
)
42+
) {
43+
return null;
44+
}
45+
return $this->unauthorizedResponse(
46+
'basic',
47+
(string) ($config->realm ?? ''),
48+
'Session authentication required'
49+
);
50+
51+
case 'basic':
52+
$creds = $this->parseBasic($authHeader);
53+
if (!$creds) {
54+
return $this->unauthorizedResponse('basic', (string) ($config->realm ?? ''));
55+
}
56+
if ($this->validateCredentials($creds['username'], $creds['password'], $config)) {
57+
return null;
58+
}
59+
return $this->unauthorizedResponse('basic', (string) ($config->realm ?? ''));
6460

65-
$data = $ha1 . ':' . ($digest['nonce'] ?? '') . ':' . ($digest['nc'] ?? '') . ':'
66-
. ($digest['cnonce'] ?? '') . ':' . ($digest['qop'] ?? '') . ':' . $ha2;
67-
$validResponse = md5($data);
61+
case 'digest':
62+
$digest = $this->parseDigest($authHeader);
63+
if (!$digest) {
64+
return $this->unauthorizedResponse('digest', (string) ($config->realm ?? ''));
65+
}
66+
$username = $digest['username'] ?? '';
67+
$password = $this->findPasswordForUser($username, $config);
68+
if ($password === null) {
69+
return $this->unauthorizedResponse('digest', (string) ($config->realm ?? ''));
70+
}
71+
$realm = (string) ($config->realm ?? '');
72+
$ha1 = md5($username . ':' . $realm . ':' . $password);
73+
$ha2 = md5($method . ':' . (string) ($digest['uri'] ?? ''));
74+
$data = $ha1 . ':' . ($digest['nonce'] ?? '') . ':' . ($digest['nc'] ?? '') . ':'
75+
. ($digest['cnonce'] ?? '') . ':' . ($digest['qop'] ?? '') . ':' . $ha2;
76+
$validResponse = md5($data);
77+
if (hash_equals($validResponse, (string) ($digest['response'] ?? ''))) {
78+
return null;
79+
}
80+
return $this->unauthorizedResponse('digest', (string) ($config->realm ?? ''));
6881

69-
if (hash_equals($validResponse, (string) ($digest['response'] ?? ''))) {
82+
default:
7083
return null;
71-
}
72-
73-
return $this->unauthorizedResponse('digest', $config->realm);
7484
}
75-
76-
return null;
7785
}
7886

79-
public function after(RequestInterface $request, ResponseInterface $response, $arguments = null)
87+
/**
88+
* @param array<string,mixed>|null $arguments
89+
*/
90+
public function after(RequestInterface $request, ResponseInterface $response, $arguments = null): void
8091
{
8192
// No-op
8293
}
8394

95+
/**
96+
* @return array{username:string,password:string}|null
97+
*/
8498
private function parseBasic(string $authHeader): ?array
8599
{
86100
if (!str_starts_with(strtolower($authHeader), 'basic ')) {
@@ -101,6 +115,9 @@ private function parseBasic(string $authHeader): ?array
101115
return ['username' => $parts[0], 'password' => $parts[1]];
102116
}
103117

118+
/**
119+
* @return array<string,string>|null
120+
*/
104121
private function parseDigest(string $authHeader): ?array
105122
{
106123
if (!str_starts_with(strtolower($authHeader), 'digest ')) {

‎src/Filters/CorsFilter.php‎

Lines changed: 24 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -11,37 +11,52 @@
1111

1212
class CorsFilter implements FilterInterface
1313
{
14-
public function before(RequestInterface $request, $arguments = null)
14+
/**
15+
* @param array<string,mixed>|null $arguments
16+
*/
17+
public function before(RequestInterface $request, $arguments = null): ?ResponseInterface
1518
{
1619
$config = config(RestConfig::class);
20+
if (!$config instanceof RestConfig) {
21+
$config = new RestConfig();
22+
}
1723

18-
$allowedHeaders = implode(', ', $config->allowedCorsHeaders);
19-
$allowedMethods = implode(', ', $config->allowedCorsMethods);
24+
$allowedHeaders = implode(', ', (array) $config->allowedCorsHeaders);
25+
$allowedMethods = implode(', ', (array) $config->allowedCorsMethods);
2026

2127
$origin = $request->getHeaderLine('Origin');
2228

23-
if ($config->allowAnyCorsDomain) {
29+
if ((bool) $config->allowAnyCorsDomain) {
2430
header('Access-Control-Allow-Origin: *');
2531
header('Access-Control-Allow-Headers: ' . $allowedHeaders);
2632
header('Access-Control-Allow-Methods: ' . $allowedMethods);
2733
header('Vary: Origin');
28-
} elseif ($origin && in_array($origin, $config->allowedCorsOrigins, true)) {
34+
} elseif ($origin && in_array($origin, (array) $config->allowedCorsOrigins, true)) {
2935
header('Access-Control-Allow-Origin: ' . $origin);
3036
header('Access-Control-Allow-Headers: ' . $allowedHeaders);
3137
header('Access-Control-Allow-Methods: ' . $allowedMethods);
3238
header('Vary: Origin');
3339
}
3440

35-
foreach ($config->forcedCorsHeaders as $h => $v) {
41+
foreach ((array) $config->forcedCorsHeaders as $h => $v) {
3642
header($h . ': ' . $v);
3743
}
3844

39-
if (strtoupper($request->getMethod()) === 'OPTIONS') {
40-
return service('response')->setStatusCode(204);
45+
if (strtoupper((string) $request->getMethod()) === 'OPTIONS') {
46+
$response = service('response');
47+
if ($response instanceof ResponseInterface) {
48+
return $response->setStatusCode(204);
49+
}
50+
return null;
4151
}
52+
53+
return null;
4254
}
4355

44-
public function after(RequestInterface $request, ResponseInterface $response, $arguments = null)
56+
/**
57+
* @param array<string,mixed>|null $arguments
58+
*/
59+
public function after(RequestInterface $request, ResponseInterface $response, $arguments = null): void
4560
{
4661
// no-op
4762
}

0 commit comments

Comments
 (0)