public/preauth
2
0
Fork 0
Code Issues 3 Pull Requests Actions Packages Projects Releases Wiki Activity

Extract session issuing so both login paths share it

Browse Source 
SessionIssuer now owns 'grant access after authenticating', which LoginManager
previously did internally. The passkey ceremony needs the same behaviour, and
two implementations would inevitably drift — most likely in cookie attributes,
where a difference stays invisible until it breaks in a browser.

LoginManager keeps what is specific to code-based login: verifying the TOTP or
backup code and enforcing the single-use nonce. Its 18 existing tests pass
unchanged, which is the evidence that this is behaviour-preserving rather than a
rewrite.

Also folds the redundant early-return into a single guard in checkToken so the
success path reads straight through.
This commit is contained in:
lyra
2026-09-27 10:45:58 +00:00
parent 436450cdc2
commit fed7b1b48c
6 changed files with 241 additions and 114 deletions
Download Patch File Download Diff File Expand all files Collapse all files
config/services.yaml
+1
View File
@@ -138,6 +138,7 @@ services:
App\Service\PasskeyInterface: '@App\Service\PasskeyManager'
App\Service\PasskeyCeremonyStoreInterface: '@App\Service\PasskeyCeremonyStore'
App\Service\PasskeyCredentialStoreInterface: '@App\Service\PasskeyCredentialStore'
App\Service\SessionIssuerInterface: '@App\Service\SessionIssuer'
# the ceremony factory takes no constructor arguments and holds no state, so
# it is built once and shared rather than re-created per ceremony
phpstan-baseline.neon
+10 -4
View File
@@ -349,14 +349,20 @@ parameters:
path: src/Service/LoginManager.php
-
message: '#^Class App\\Service\\LoginManager has an uninitialized readonly property \$nonceCache\. Assign it in the constructor\.$#'
message: '#^Class App\\Service\\SessionIssuer has an uninitialized readonly property \$logger\. Assign it in the constructor\.$#'
identifier: property.uninitializedReadonly
count: 1
path: src/Service/LoginManager.php
path: src/Service/SessionIssuer.php
-
message: '#^Method App\\Service\\LoginManager\:\:checkToken\(\) overrides method App\\Service\\LoginInterface\:\:checkToken\(\) but is missing the \#\[\\Override\] attribute\.$#'
identifier: method.missingOverride
message: '#^Readonly property App\\Service\\SessionIssuer\:\:\$logger is assigned outside of the constructor\.$#'
identifier: property.readOnlyAssignNotInConstructor
count: 1
path: src/Service/SessionIssuer.php
-
message: '#^Class App\\Service\\LoginManager has an uninitialized readonly property \$nonceCache\. Assign it in the constructor\.$#'
identifier: property.uninitializedReadonly
count: 1
path: src/Service/LoginManager.php
src/Service/LoginManager.php
+32 -106
View File
@@ -6,39 +6,38 @@ namespace App\Service;
use App\Data\Payload;
use App\Enum\Scope;
use App\MonitorCacheKeys;
use App\Trait\CookieNameTrait;
use App\Trait\GetTotpTrait;
use App\Trait\MakeNonceTrait;
use App\Trait\StringTrait;
use Psr\Cache\CacheItemPoolInterface;
use Override;
use Psr\Cache\InvalidArgumentException;
use Symfony\Component\DependencyInjection\Attribute\Target;
use Symfony\Component\HttpFoundation\Cookie;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\HttpKernel\Exception\HttpException;
use Symfony\Component\Uid\Ulid;
/**
* Authenticates a TOTP code (or backup code) and, on success, grants access.
*
* The "grant access" half now lives in {@see SessionIssuer} so the passkey
* ceremony produces an identical response. This class keeps the part that is
* genuinely specific to code-based login: verifying the code and enforcing the
* single-use nonce.
*/
final readonly class LoginManager implements LoginInterface
{
use CookieNameTrait;
use GetTotpTrait;
use MakeNonceTrait;
use StringTrait;
private CacheItemPoolInterface $sessionCache;
/** @throws InvalidArgumentException */
public function __construct(
#[Target('sessionCache')] CacheItemPoolInterface $sessionCache,
private BackupCodeInterface $backupCodeManager,
private DomainInterface $domainManager,
private SessionIssuerInterface $sessionIssuer,
) {
$this->sessionCache = new MonitorCacheKeys($sessionCache);
}
/** @throws InvalidArgumentException */
/**
* @throws InvalidArgumentException
*/
#[Override]
public function checkToken(Payload $payload, Request $request): ?Response
{
/* when scope is IP but ip-access is disabled, scope is to be considered cookie */
@@ -47,103 +46,30 @@ final readonly class LoginManager implements LoginInterface
$payload->scope = Scope::Cookie;
}
if ($this->getTotp()->verify($payload->token, null, 1)
|| $this->backupCodeManager->verifyAndConsume($payload->token)
if (!$this->getTotp()->verify($payload->token, null, 1)
&& !$this->backupCodeManager->verifyAndConsume($payload->token)
) {
/* token is correct (TOTP or Backup) */
/* if server nonce is found and is valid */
$nonceItem = $this->nonceCache->getItem($this->makeCacheKey($payload->nonce));
if ($nonceItem->isHit() && $nonceItem->get()) {
/* mark nonce as spent */
$nonceItem->set(false); /* invalid */
$nonceItem->expiresAfter(self::NONCE_TTL); /* keep briefly */
$this->nonceCache->save($nonceItem);
/* token authentication successful, grant access and set response */
$cleanId = $this->makeCacheKey($payload->id);
/* if they just want this one page, return ok, to grant them access */
$response = $this->authSuccessResponse($cleanId, $this->config);
if (Scope::None !== $payload->scope) {
/* grant access based on the requested scope */
if (Scope::Cookie === $payload->scope) {
$response->headers->setCookie($this->setCookie($cleanId, $request->getHost()));
} elseif (Scope::Ip === $payload->scope) {
$this->setIp($cleanId, $request->getClientIp());
}
if ($payload->json) {
$contentType = 'application/json';
$content = json_encode([
'message' => 'Login successful',
'nonce' => null,
]);
} else {
$contentType = 'text/html';
$content = "hi $cleanId, please reload";
}
$location = $request->query->has('return')
&& $this->domainManager->validReturn($request->query->get('return')) ?
"{$request->query->get('return')}" :
"{$request->getPathInfo()}{$request->getQueryString()}";
/* force redirect to use GET method (important when using central auth) */
$response->setContent($content)
->setStatusCode(Response::HTTP_SEE_OTHER)
->headers->set('Location', $location);
$response->headers->set('Content-Type', $contentType);
}
$this->logger->debug("successful login for: $cleanId");
return $response;
}
return null;
}
return null;
}
/* token is correct (TOTP or Backup) */
/** @throws InvalidArgumentException */
private function setCookie(string $id, string $host): Cookie
{
/* successful auth with token, store session and set the cookie */
$ulid = new Ulid();
$sessionCookie = $this->sessionCache->getItem(
$this->makeCacheKey("cookie_$ulid"),
);
if ($sessionCookie->isHit()) {
/* it is supposed to be impossible to have collisions */
$this->logger->error('aborting: ULID collision');
throw new HttpException(Response::HTTP_INTERNAL_SERVER_ERROR, 'Internal Server Error');
/* if server nonce is found and is valid */
$nonceItem = $this->nonceCache->getItem($this->makeCacheKey($payload->nonce));
if (!$nonceItem->isHit() || !$nonceItem->get()) {
return null;
}
$sessionCookie->set($id);
$sessionCookie->expiresAfter($this->config->cookieTtl());
$this->sessionCache->save($sessionCookie);
return Cookie::create(
name: $this->sessionCookieName($this->domainManager),
value: $ulid->toString(),
expire: time() + $this->config->cookieTtl(),
path: '/',
domain: $this->sessionCookieDomain($this->domainManager, $host),
secure: true,
httpOnly: true,
sameSite: Cookie::SAMESITE_STRICT,
/* mark nonce as spent */
$nonceItem->set(false); /* invalid */
$nonceItem->expiresAfter(self::NONCE_TTL); /* keep briefly */
$this->nonceCache->save($nonceItem);
return $this->sessionIssuer->issue(
$payload->id,
$payload->scope,
$request,
$payload->json,
);
}
/** @throws InvalidArgumentException */
private function setIp(string $id, string $ip): void
{
/* successful auth with token, requested scope of ip (and ip access enabled) */
$ipKey = $this->makeCacheKey("ip_$ip");
$sessionIp = $this->sessionCache->getItem($ipKey);
$sessionIp->set($id);
$sessionIp->expiresAfter($this->config->ipTtl());
$this->sessionCache->save($sessionIp);
}
}
src/Service/SessionIssuer.php
+151
View File
@@ -0,0 +1,151 @@
<?php
declare(strict_types=1);
namespace App\Service;
use App\ConfigBag;
use App\Enum\Scope;
use App\MonitorCacheKeys;
use App\Trait\CookieNameTrait;
use App\Trait\HasLoggerTrait;
use App\Trait\StringTrait;
use Override;
use Psr\Cache\CacheItemPoolInterface;
use Psr\Cache\InvalidArgumentException;
use Symfony\Component\DependencyInjection\Attribute\Target;
use Symfony\Component\HttpFoundation\Cookie;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\HttpKernel\Exception\HttpException;
use Symfony\Component\Uid\Ulid;
/**
* Grants access once an identity has been authenticated, by whatever method.
*
* Extracted from `LoginManager` so the passkey ceremony and the TOTP form
* produce **byte-identical** outcomes. Two implementations would inevitably
* drift — most likely in cookie attributes, where a difference is invisible
* until it breaks in a browser.
*
* This class deliberately knows nothing about *how* authentication happened; it
* only records the result.
*
* @see SessionIssuerInterface
*/
final readonly class SessionIssuer implements SessionIssuerInterface
{
use CookieNameTrait;
use HasLoggerTrait;
use StringTrait;
private MonitorCacheKeys $sessionCache;
/**
* @throws InvalidArgumentException
*/
public function __construct(
#[Target('sessionCache')] CacheItemPoolInterface $sessionCache,
private DomainInterface $domainManager,
private ConfigBag $config,
) {
$this->sessionCache = new MonitorCacheKeys($sessionCache);
}
/**
* @throws InvalidArgumentException
*/
#[Override]
public function issue(string $identity, Scope $scope, Request $request, bool $json): Response
{
/* the same normalisation LoginManager has always applied, so cache keys
* and Remote-User values stay identical between the two login paths */
$cleanId = $this->makeCacheKey($identity);
$response = $this->authSuccessResponse($cleanId, $this->config);
/* when the caller only wanted this one page, there is nothing to store */
if (Scope::None === $scope) {
$this->logger->debug("successful login for: $cleanId");
return $response;
}
if (Scope::Cookie === $scope) {
$response->headers->setCookie($this->setCookie($cleanId, $request->getHost()));
} elseif (Scope::Ip === $scope) {
$this->setIp($cleanId, (string) $request->getClientIp());
}
if ($json) {
$contentType = 'application/json';
$content = (string) json_encode([
'message' => 'Login successful',
'nonce' => null,
]);
} else {
$contentType = 'text/html';
$content = "hi $cleanId, please reload";
}
$location = $request->query->has('return')
&& $this->domainManager->validReturn((string) $request->query->get('return')) ?
"{$request->query->get('return')}" :
"{$request->getPathInfo()}{$request->getQueryString()}";
/* force redirect to use GET method (important when using central auth) */
$response->setContent($content)
->setStatusCode(Response::HTTP_SEE_OTHER)
->headers->set('Location', $location);
$response->headers->set('Content-Type', $contentType);
$this->logger->debug("successful login for: $cleanId");
return $response;
}
/**
* @throws InvalidArgumentException
*/
private function setCookie(string $id, string $host): Cookie
{
/* successful auth with token, store session and set the cookie */
$ulid = new Ulid();
$sessionCookie = $this->sessionCache->getItem(
$this->makeCacheKey("cookie_$ulid"),
);
if ($sessionCookie->isHit()) {
/* it is supposed to be impossible to have collisions */
$this->logger->error('aborting: ULID collision');
throw new HttpException(Response::HTTP_INTERNAL_SERVER_ERROR, 'Internal Server Error');
}
$sessionCookie->set($id);
$sessionCookie->expiresAfter($this->config->cookieTtl());
$this->sessionCache->save($sessionCookie);
return Cookie::create(
name: $this->sessionCookieName($this->domainManager),
value: $ulid->toString(),
expire: time() + $this->config->cookieTtl(),
path: '/',
domain: $this->sessionCookieDomain($this->domainManager, $host),
secure: true,
httpOnly: true,
sameSite: Cookie::SAMESITE_STRICT,
);
}
/**
* @throws InvalidArgumentException
*/
private function setIp(string $id, string $ip): void
{
/* successful auth with token, requested scope of ip (and ip access enabled) */
$ipKey = $this->makeCacheKey("ip_$ip");
$sessionIp = $this->sessionCache->getItem($ipKey);
$sessionIp->set($id);
$sessionIp->expiresAfter($this->config->ipTtl());
$this->sessionCache->save($sessionIp);
}
}
src/Service/SessionIssuerInterface.php
+29
View File
@@ -0,0 +1,29 @@
<?php
declare(strict_types=1);
namespace App\Service;
use App\Enum\Scope;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Response;
/**
* Grants access after a successful authentication, regardless of method.
*
* Exists so the TOTP form and the passkey ceremony cannot drift apart: they must
* produce identical cookies, headers and redirects, and the only reliable way to
* guarantee that is for both to call the same code.
*/
interface SessionIssuerInterface
{
/**
* Record the authenticated identity according to the requested scope and
* build the response the caller should return.
*
* @param string $identity the session id, as typed by the user
* @param Scope $scope whether to set a cookie, an IP session, or neither
* @param bool $json JSON for an AJAX caller, HTML for a form post
*/
public function issue(string $identity, Scope $scope, Request $request, bool $json): Response;
}
tests/Unit/Service/LoginManagerTest.php
+18 -4
View File
@@ -9,6 +9,7 @@ use App\Enum\Scope;
use App\Service\BackupCodeInterface;
use App\Service\DomainManager;
use App\Service\LoginManager;
use App\Service\SessionIssuer;
use App\Tests\Support\TotpTestHelper;
use App\Trait\StringTrait;
use PHPUnit\Framework\TestCase;
@@ -28,6 +29,7 @@ final class LoginManagerTest extends TestCase
private ArrayAdapter $pool;
private BackupCodeInterface $backupCodeManager;
private DomainManager $domainManager;
private ?SessionIssuer $sessionIssuer = null;
private function makeLoginManager(
?int $ipTtl = 0,
@@ -38,7 +40,12 @@ final class LoginManagerTest extends TestCase
$this->backupCodeManager = $this->createStub(BackupCodeInterface::class);
$this->domainManager = new DomainManager($subdomainRedirect, $authSubdomain);
$manager = new LoginManager($this->pool, $this->backupCodeManager, $this->domainManager);
/* session issuing is shared with the passkey flow, so it is built as the
* same collaborator the container would inject */
$this->sessionIssuer = new SessionIssuer($this->pool, $this->domainManager, $this->makeConfig(ipTtl: $ipTtl));
$this->sessionIssuer->setLogger(new NullLogger());
$manager = new LoginManager($this->backupCodeManager, $this->sessionIssuer);
$manager->setConfig($this->makeConfig(ipTtl: $ipTtl));
$manager->setLogger(new NullLogger());
$manager->setNonceCache(new ArrayAdapter());
@@ -277,8 +284,10 @@ final class LoginManagerTest extends TestCase
self::assertFalse($response->headers->has('Set-Cookie'));
// verify the IP session exists in the cache
$reflection = new ReflectionProperty(LoginManager::class, 'sessionCache');
$sessionCache = $reflection->getValue($manager);
$issuer = $this->sessionIssuer;
self::assertNotNull($issuer, 'makeLoginManager() should have built a session issuer.');
$reflection = new ReflectionProperty(SessionIssuer::class, 'sessionCache');
$sessionCache = $reflection->getValue($issuer);
self::assertTrue($sessionCache->hasItem('ip_1.2.3.4'));
}
@@ -361,7 +370,12 @@ final class LoginManagerTest extends TestCase
$this->backupCodeManager = $this->createStub(BackupCodeInterface::class);
$this->backupCodeManager->method('verifyAndConsume')->willReturn(false);
$manager = new LoginManager($pool, $this->backupCodeManager, $this->domainManager);
/* the colliding pool must be the one the session issuer writes through,
* because that is where the cookie is stored */
$issuer = new SessionIssuer($pool, $this->domainManager, $this->makeConfig());
$issuer->setLogger(new NullLogger());
$manager = new LoginManager($this->backupCodeManager, $issuer);
$manager->setConfig($this->makeConfig());
$manager->setLogger(new NullLogger());
$manager->setNonceCache(new ArrayAdapter());