Code quality: - Create AppConstants class with shared constants: - FAR_FUTURE_DATE (replaces duplicated '2999-12-31' strings) - MAX_INPUT_LENGTH (replaces duplicated 128 in Payload and StringTrait) - Extract duplicated 'hi $id' response body into StringTrait::authSuccessResponse() method, used by AcceptListener, AllowListener, and LoginManager - Add missing @throws InvalidArgumentException annotations to MonitorCacheKeys (getItem, hasItem, deleteItem, deleteItems, commit) Configuration: - Add proper env var type casting in services.yaml: - COOKIE_TTL → env(int:) - SUBDOMAIN_REDIRECT → env(bool:) - IP_TTL → env(int:) - TEAPOT → env(bool:) Documentation: - Create CONTRIBUTING.md with development setup, code style, testing guidelines, and PR process
This commit is contained in:
@@ -0,0 +1,74 @@
|
||||
# Contributing to Preauth
|
||||
|
||||
Thank you for your interest in contributing to Preauth! This document
|
||||
outlines the process for contributing to the project.
|
||||
|
||||
## Development Setup
|
||||
|
||||
1. Clone the repository
|
||||
2. Install dependencies: `composer install`
|
||||
3. Copy `.env.example` to `.env` and configure as needed
|
||||
4. Run tests: `vendor/bin/phpunit`
|
||||
|
||||
## Code Style
|
||||
|
||||
This project follows [PSR-12](https://www.php-fig.org/psr/psr-12/) and
|
||||
includes `php-cs-fixer` as a dev dependency.
|
||||
|
||||
```bash
|
||||
# Check for style violations
|
||||
vendor/bin/php-cs-fixer fix --dry-run --diff
|
||||
|
||||
# Auto-fix
|
||||
vendor/bin/php-cs-fixer fix
|
||||
```
|
||||
|
||||
All code must pass the style check before it can be merged.
|
||||
|
||||
## Testing
|
||||
|
||||
All code changes must include tests. The project maintains 100% code
|
||||
coverage — new code must be fully tested.
|
||||
|
||||
```bash
|
||||
# Run tests
|
||||
vendor/bin/phpunit
|
||||
|
||||
# Run with coverage (requires Xdebug)
|
||||
XDEBUG_MODE=coverage vendor/bin/phpunit --coverage-text
|
||||
```
|
||||
|
||||
### Test Structure
|
||||
|
||||
- **Unit tests** go in `tests/Unit/` and mirror the `src/` directory structure
|
||||
- **Functional tests** go in `tests/Functional/` and test the full HTTP kernel
|
||||
- Use the support traits (`TotpTestHelper`, `ListenerTestHelper`) for
|
||||
reusable test fixtures
|
||||
|
||||
## Pull Request Process
|
||||
|
||||
1. Create a feature branch from `main`
|
||||
2. Make your changes, ensuring tests pass and code style is clean
|
||||
3. Update documentation if needed (README, CHANGELOG, docs/)
|
||||
4. Submit a pull request to `main`
|
||||
|
||||
### Commit Messages
|
||||
|
||||
Use conventional commit format:
|
||||
|
||||
- `feat:` new feature
|
||||
- `fix:` bug fix
|
||||
- `docs:` documentation only
|
||||
- `refactor:` code change that neither fixes a bug nor adds a feature
|
||||
- `test:` adding or correcting tests
|
||||
- `chore:` build process, tooling, etc.
|
||||
|
||||
## Architecture
|
||||
|
||||
Preauth is an event-listener-driven Symfony application (no controllers).
|
||||
See `ROADMAP.md` for the full architecture overview and design decisions.
|
||||
|
||||
## License
|
||||
|
||||
By contributing, you agree that your contributions will be licensed under
|
||||
the MIT License.
|
||||
@@ -56,12 +56,12 @@ parameters:
|
||||
|
||||
# --- application variables ---
|
||||
app.totp_uri: '%env(TOTP_URI)%'
|
||||
app.cookie_ttl: '%env(COOKIE_TTL)%'
|
||||
app.subdomain_redirect: '%env(SUBDOMAIN_REDIRECT)%'
|
||||
app.cookie_ttl: '%env(int:COOKIE_TTL)%'
|
||||
app.subdomain_redirect: '%env(bool:SUBDOMAIN_REDIRECT)%'
|
||||
app.auth_subdomain: '%env(AUTH_SUBDOMAIN)%'
|
||||
|
||||
app.ip_ttl: '%env(IP_TTL)%'
|
||||
app.teapot: '%env(TEAPOT)%'
|
||||
app.ip_ttl: '%env(int:IP_TTL)%'
|
||||
app.teapot: '%env(bool:TEAPOT)%'
|
||||
|
||||
app.error_message: '%env(ERROR_MESSAGE)%'
|
||||
app.teapot_title: '%env(TEAPOT_TITLE)%'
|
||||
|
||||
@@ -0,0 +1,25 @@
|
||||
<?php
|
||||
|
||||
declare(strict_types=1);
|
||||
|
||||
namespace App;
|
||||
|
||||
/**
|
||||
* Shared application constants.
|
||||
*/
|
||||
final class AppConstants
|
||||
{
|
||||
/**
|
||||
* Far-future expiration date used for persistent cache items
|
||||
* (TOTP secrets, backup codes) that should effectively never expire.
|
||||
* Per PSR-6, if no expiration is set, the implementation may set a
|
||||
* default — we use this to be explicit.
|
||||
*/
|
||||
public const string FAR_FUTURE_DATE = '2999-12-31';
|
||||
|
||||
/**
|
||||
* Maximum length for user-supplied input fields (id, nonce, token).
|
||||
* Also used for cache key truncation.
|
||||
*/
|
||||
public const int MAX_INPUT_LENGTH = 128;
|
||||
}
|
||||
@@ -4,6 +4,7 @@ declare(strict_types=1);
|
||||
|
||||
namespace App\Data;
|
||||
|
||||
use App\AppConstants;
|
||||
use App\Enum\Scope;
|
||||
use Symfony\Component\HttpFoundation\InputBag;
|
||||
|
||||
@@ -59,11 +60,11 @@ final class Payload
|
||||
|
||||
/* all input is limited */
|
||||
$payload = new Payload();
|
||||
$payload->id = mb_substr(trim($data->id), 0, 128);
|
||||
$payload->nonce = mb_substr(trim($data->nonce), 0, 128);
|
||||
$payload->id = mb_substr(trim($data->id), 0, AppConstants::MAX_INPUT_LENGTH);
|
||||
$payload->nonce = mb_substr(trim($data->nonce), 0, AppConstants::MAX_INPUT_LENGTH);
|
||||
$payload->json = ($data->json ?? true);
|
||||
$payload->scope = Scope::tryFrom($data->scope ?? '') ?? Scope::Cookie;
|
||||
$payload->token = mb_substr(trim($data->token), 0, 128);
|
||||
$payload->token = mb_substr(trim($data->token), 0, AppConstants::MAX_INPUT_LENGTH);
|
||||
|
||||
return Payload::constrict($payload);
|
||||
}
|
||||
|
||||
@@ -11,7 +11,6 @@ use App\Trait\StringTrait;
|
||||
use Psr\Cache\CacheItemPoolInterface;
|
||||
use Psr\Cache\InvalidArgumentException;
|
||||
use Symfony\Component\EventDispatcher\Attribute\AsEventListener;
|
||||
use Symfony\Component\HttpFoundation\Response;
|
||||
use Symfony\Component\HttpKernel\Event\RequestEvent;
|
||||
|
||||
final readonly class AcceptListener
|
||||
@@ -52,10 +51,7 @@ final readonly class AcceptListener
|
||||
|
||||
$id = $item->get();
|
||||
$this->logger->debug("has valid cookie-session: $id");
|
||||
$event->setResponse(new Response("hi $id", headers: [
|
||||
'Content-Type' => 'text/plain',
|
||||
'Remote-User' => $id,
|
||||
]));
|
||||
$event->setResponse($this->authSuccessResponse($id));
|
||||
} catch (InvalidArgumentException $e) {
|
||||
/* cache failure — fail closed (don't authenticate) */
|
||||
$this->logger->error("cache error in AcceptListener: {$e->getMessage()}");
|
||||
|
||||
@@ -10,7 +10,6 @@ use App\Trait\StringTrait;
|
||||
use Psr\Cache\CacheItemPoolInterface;
|
||||
use Psr\Cache\InvalidArgumentException;
|
||||
use Symfony\Component\EventDispatcher\Attribute\AsEventListener;
|
||||
use Symfony\Component\HttpFoundation\Response;
|
||||
use Symfony\Component\HttpKernel\Event\RequestEvent;
|
||||
|
||||
final readonly class AllowListener
|
||||
@@ -47,10 +46,7 @@ final readonly class AllowListener
|
||||
|
||||
$id = $item->get();
|
||||
$this->logger->debug("has valid ip-session: $id");
|
||||
$event->setResponse(new Response("hi $id", headers: [
|
||||
'Content-Type' => 'text/plain',
|
||||
'Remote-User' => $id,
|
||||
]));
|
||||
$event->setResponse($this->authSuccessResponse($id));
|
||||
} catch (InvalidArgumentException $e) {
|
||||
/* cache failure — fail closed (don't authenticate) */
|
||||
$this->logger->error("cache error in AllowListener: {$e->getMessage()}");
|
||||
|
||||
@@ -67,6 +67,7 @@ final readonly class MonitorCacheKeys implements CacheItemPoolInterface
|
||||
$this->cache->save($changeList);
|
||||
}
|
||||
|
||||
/** @throws InvalidArgumentException */
|
||||
public function getItem(string $key): CacheItemInterface
|
||||
{
|
||||
return $this->cache->getItem($key);
|
||||
@@ -79,6 +80,7 @@ final readonly class MonitorCacheKeys implements CacheItemPoolInterface
|
||||
return $this->cache->getItems($keys);
|
||||
}
|
||||
|
||||
/** @throws InvalidArgumentException */
|
||||
public function hasItem(string $key): bool
|
||||
{
|
||||
return $this->cache->hasItem($key);
|
||||
@@ -97,6 +99,7 @@ final readonly class MonitorCacheKeys implements CacheItemPoolInterface
|
||||
return true;
|
||||
}
|
||||
|
||||
/** @throws InvalidArgumentException */
|
||||
public function deleteItem(string $key): bool
|
||||
{
|
||||
$this->isValid($key);
|
||||
@@ -113,6 +116,7 @@ final readonly class MonitorCacheKeys implements CacheItemPoolInterface
|
||||
return $this->cache->deleteItem($key);
|
||||
}
|
||||
|
||||
/** @throws InvalidArgumentException */
|
||||
public function deleteItems(array $keys): bool
|
||||
{
|
||||
$this->allValid($keys);
|
||||
@@ -145,6 +149,7 @@ final readonly class MonitorCacheKeys implements CacheItemPoolInterface
|
||||
return $this->cache->saveDeferred($item);
|
||||
}
|
||||
|
||||
/** @throws InvalidArgumentException */
|
||||
public function commit(): bool
|
||||
{
|
||||
return $this->cache->commit();
|
||||
|
||||
@@ -4,6 +4,7 @@ declare(strict_types=1);
|
||||
|
||||
namespace App\Service;
|
||||
|
||||
use App\AppConstants;
|
||||
use App\MonitorCacheKeys;
|
||||
use App\Trait\HasLoggerTrait;
|
||||
use App\Trait\StringTrait;
|
||||
@@ -84,7 +85,7 @@ final readonly class BackupCodeManager implements BackupCodeInterface
|
||||
* we want this to keep forever, so a few hundred years should do it */
|
||||
$backupItem->expiresAt(DateTimeImmutable::createFromFormat(
|
||||
'Y-m-d',
|
||||
'2999-12-31'
|
||||
AppConstants::FAR_FUTURE_DATE
|
||||
));
|
||||
$this->sessionCache->save($backupItem);
|
||||
|
||||
@@ -104,7 +105,7 @@ final readonly class BackupCodeManager implements BackupCodeInterface
|
||||
* we want this to keep forever, so a few hundred years should do it */
|
||||
$backupItem->expiresAt(DateTimeImmutable::createFromFormat(
|
||||
'Y-m-d',
|
||||
'2999-12-31'
|
||||
AppConstants::FAR_FUTURE_DATE
|
||||
));
|
||||
$this->sessionCache->saveDeferred($backupItem);
|
||||
}
|
||||
|
||||
@@ -63,10 +63,7 @@ final readonly class LoginManager implements LoginInterface
|
||||
$cleanId = $this->makeCacheKey($payload->id);
|
||||
|
||||
/* if they just want this one page, return ok, to grant them access */
|
||||
$response = new Response("hi $cleanId", headers: [
|
||||
'Content-Type' => 'text/plain',
|
||||
'Remote-User' => $cleanId,
|
||||
]);
|
||||
$response = $this->authSuccessResponse($cleanId);
|
||||
|
||||
if ($payload->scope !== Scope::None) {
|
||||
/* grant access based on the requested scope */
|
||||
|
||||
@@ -4,6 +4,9 @@ declare(strict_types=1);
|
||||
|
||||
namespace App\Trait;
|
||||
|
||||
use App\AppConstants;
|
||||
use Symfony\Component\HttpFoundation\Response;
|
||||
|
||||
trait StringTrait
|
||||
{
|
||||
/* cache keys can safely use alphanumeric, "_", and ".", remove the rest */
|
||||
@@ -11,6 +14,18 @@ trait StringTrait
|
||||
|
||||
public function makeCacheKey(string $name): string
|
||||
{
|
||||
return mb_substr(preg_replace(static::KEY_REGEX, '_', $name), 0, 128);
|
||||
return mb_substr(preg_replace(static::KEY_REGEX, '_', $name), 0, AppConstants::MAX_INPUT_LENGTH);
|
||||
}
|
||||
|
||||
/**
|
||||
* Build the plain-text success response body and headers for an authenticated request.
|
||||
* The body is a simple greeting that includes the session id.
|
||||
*/
|
||||
public function authSuccessResponse(string $id): Response
|
||||
{
|
||||
return new Response("hi $id", headers: [
|
||||
'Content-Type' => 'text/plain',
|
||||
'Remote-User' => $id,
|
||||
]);
|
||||
}
|
||||
}
|
||||
|
||||
+1
-1
@@ -47,7 +47,7 @@ final readonly class Utilities
|
||||
* we want this to keep forever, so a few hundred years should do it */
|
||||
$totpItem->expiresAt(DateTimeImmutable::createFromFormat(
|
||||
'Y-m-d',
|
||||
'2999-12-31'
|
||||
AppConstants::FAR_FUTURE_DATE
|
||||
));
|
||||
$this->appPool->save($totpItem);
|
||||
return $totp;
|
||||
|
||||
Reference in New Issue
Block a user