Route passkey registration through the TOTP check, not the listener
Corrects a design error in the previous commit. I had exposed register-begin as a listener operation and gated it on a session cookie, but the approved flow has no session at that point: the whole point is that a valid TOTP code is what authorises registration, and the session is only issued once the new credential has been verified. Two consequences, both bad: - There is no session cookie to check, so the gate could never have worked. It would have been dead code that looked like a security control. - More seriously, a listener-side register-begin would hand out a challenge without proving anything. Anyone could obtain ceremony options and attempt registration. The cookie check was not a weak control; the operation itself was the hole. Registration is now started by LoginManager, after it has verified both the code and the nonce, and its options are returned with the login response. That is the flow in the plan, and it keeps nonce validation in the one place that already enforces it. The capability for register-finish is the single-use ceremonyId, which is server-issued and bound to the identity that passed the check. The listener now serves three operations, and a test pins that register-begin is not one of them. Also adds Payload::$register so the checkbox intent survives from the form to LoginManager, and moves the ceremony marker constant to AppConstants since both LoginManager and the listener now produce marked responses.
This commit is contained in:
@@ -4,6 +4,7 @@ declare(strict_types=1);
|
||||
|
||||
namespace App\Service;
|
||||
|
||||
use App\AppConstants;
|
||||
use App\Data\Payload;
|
||||
use App\Enum\Scope;
|
||||
use App\Trait\GetTotpTrait;
|
||||
@@ -13,14 +14,21 @@ use Override;
|
||||
use Psr\Cache\InvalidArgumentException;
|
||||
use Symfony\Component\HttpFoundation\Request;
|
||||
use Symfony\Component\HttpFoundation\Response;
|
||||
use Throwable;
|
||||
|
||||
/**
|
||||
* Authenticates a TOTP code (or backup code) and, on success, grants access.
|
||||
* Authenticates a TOTP code (or backup code) and, on success, either grants
|
||||
* access or starts a passkey registration.
|
||||
*
|
||||
* 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.
|
||||
* The "grant access" half lives in {@see SessionIssuer} so the passkey ceremony
|
||||
* produces an identical response. This class keeps the part genuinely specific
|
||||
* to code-based login: verifying the code and enforcing the single-use nonce.
|
||||
*
|
||||
* **Why the registration hand-off lives here.** Ticking "register this device"
|
||||
* turns the form submission into a registration ceremony, and the TOTP check is
|
||||
* what authorises it. That check — and the nonce check — already happen here, so
|
||||
* a ceremony started anywhere earlier would mean validating the nonce somewhere
|
||||
* new and risking spending it twice.
|
||||
*/
|
||||
final readonly class LoginManager implements LoginInterface
|
||||
{
|
||||
@@ -31,6 +39,7 @@ final readonly class LoginManager implements LoginInterface
|
||||
public function __construct(
|
||||
private BackupCodeInterface $backupCodeManager,
|
||||
private SessionIssuerInterface $sessionIssuer,
|
||||
private PasskeyInterface $passkeys,
|
||||
) {
|
||||
}
|
||||
|
||||
@@ -65,6 +74,12 @@ final readonly class LoginManager implements LoginInterface
|
||||
$nonceItem->expiresAfter(self::NONCE_TTL); /* keep briefly */
|
||||
$this->nonceCache->save($nonceItem);
|
||||
|
||||
/* the code and the nonce are both good from here on */
|
||||
|
||||
if ($payload->register) {
|
||||
return $this->startRegistration($payload);
|
||||
}
|
||||
|
||||
return $this->sessionIssuer->issue(
|
||||
$payload->id,
|
||||
$payload->scope,
|
||||
@@ -72,4 +87,32 @@ final readonly class LoginManager implements LoginInterface
|
||||
$payload->json,
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* The registration hand-off: authorisation is already proven, so this issues
|
||||
* the ceremony options back to the page instead of a session.
|
||||
*
|
||||
* `SessionIssuer` is deliberately not involved — the session is granted only
|
||||
* once the new credential has actually been verified, at `register-finish`.
|
||||
*/
|
||||
private function startRegistration(Payload $payload): ?Response
|
||||
{
|
||||
try {
|
||||
$payloadOut = $this->passkeys->beginRegistration($payload->id);
|
||||
} catch (Throwable) {
|
||||
/* a ceremony that cannot start must not become a 500 on the login
|
||||
* page; falling through to the caller's failure path is the same
|
||||
* treatment a wrong code gets */
|
||||
return null;
|
||||
}
|
||||
|
||||
$response = new Response(
|
||||
(string) json_encode(['register' => $payloadOut]),
|
||||
Response::HTTP_OK,
|
||||
['Content-Type' => 'application/json'],
|
||||
);
|
||||
$response->headers->set(AppConstants::PASSKEY_CEREMONY_MARKER, '1');
|
||||
|
||||
return $response;
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user