Move 2FA challenge form assembly back to LoginForm

Replace `assembleVerificationForm()` on the `TwoFactor` interface with
a generic `assembleTwoFactorElements()` method on `LoginForm` itself.
The token input, verify button, and cancel button are now owned by the
login form rather than each hook implementation.

Remove the `TOKEN_INPUT`, `SUBMIT_VERIFY_2FA`, and `SUBMIT_CANCEL_2FA`
constants from the `TwoFactor` interface and define them directly on
`LoginForm`. Update `AuthenticationController` to reference the new
location.
This commit is contained in:
Johannes Rauh 2026-04-28 09:05:35 +02:00
parent 09fae0c6d1
commit 860bf995ab
3 changed files with 44 additions and 35 deletions

View file

@ -9,13 +9,11 @@ use GuzzleHttp\Psr7\ServerRequest;
use Icinga\Application\ClassLoader;
use Icinga\Application\Hook\AuthenticationHook;
use Icinga\Application\Hook\LoginButtonHook;
use Icinga\Application\Hook\TwoFactorHook;
use Icinga\Application\Icinga;
use Icinga\Application\Logger;
use Icinga\Authentication\LoginButton;
use Icinga\Authentication\LoginButtonForm;
use Icinga\Authentication\Auth;
use Icinga\Authentication\TwoFactor;
use Icinga\Authentication\User\ExternalBackend;
use Icinga\Common\Database;
use Icinga\Exception\AuthenticationException;
@ -65,7 +63,7 @@ class AuthenticationController extends Controller
})
->on(Form::ON_SENT, function (LoginForm $form) {
$isCsrfValid = $form->getElement('CSRFToken')->isValid();
$isCancelPressed = $form->getPressedSubmitElement()?->getName() === TwoFactor::SUBMIT_CANCEL_2FA;
$isCancelPressed = $form->getPressedSubmitElement()?->getName() === LoginForm::SUBMIT_CANCEL_2FA;
if ($isCsrfValid && $isCancelPressed) {
Session::getSession()->purge();

View file

@ -12,8 +12,6 @@ use Icinga\Application\Hook\TwoFactorHook;
use Icinga\Application\Icinga;
use Icinga\Application\Logger;
use Icinga\Authentication\Auth;
use Icinga\Authentication\TwoFactor;
use Icinga\Authentication\TwoFactorTotp;
use Icinga\Common\Database;
use Icinga\Exception\Http\HttpBadRequestException;
use Icinga\User;
@ -43,6 +41,15 @@ class LoginForm extends CompatForm
/** @var string */
const SUBMIT_LOGIN = 'btn_submit_login';
/** @var string Name of the token text input in the verification form */
const TOKEN_INPUT = '2fa_token';
/** @var string Name of the verify submit button in the verification form */
const SUBMIT_VERIFY_2FA = 'btn_submit_verify_2fa';
/** @var string Name of the cancel submit button in the verification form */
const SUBMIT_CANCEL_2FA = 'btn_submit_cancel_2fa';
public function __construct()
{
$this->setAttribute('name', 'form_login');
@ -63,7 +70,7 @@ class LoginForm extends CompatForm
*
* @return void
*/
public function assembleLoginElements(): void
protected function assembleLoginElements(): void
{
$this->addElement(
'text',
@ -131,6 +138,34 @@ class LoginForm extends CompatForm
);
}
protected function assembleTwoFactorElements(): void
{
$this->addElement('text', static::TOKEN_INPUT, [
'required' => true,
'class' => 'autofocus content-centered',
'placeholder' => $this->translate('Please enter your 2FA token'),
'autocomplete' => 'off',
'autocapitalize' => 'off',
'decorators' => [
'RenderElement' => new RenderElementDecorator(),
'Errors' => ['name' => 'Errors', 'options' => ['class' => 'errors']]
]
]);
$this->addElement('submit', static::SUBMIT_VERIFY_2FA, [
'data-progress-label' => $this->translate('Verifying'),
'label' => $this->translate('Verify')
]);
$this->addElement('submit', static::SUBMIT_CANCEL_2FA, [
'ignore' => true,
'formnovalidate' => true,
'class' => 'btn-cancel',
'label' => $this->translate('Cancel'),
'data-progress-label' => $this->translate('Canceling')
]);
}
protected function assemble(): void
{
$this->addCsrfCounterMeasure(Session::getSession()->getId());
@ -138,8 +173,7 @@ class LoginForm extends CompatForm
$session = Session::getSession();
if ($session->get('2fa_must_challenge', false)) {
TwoFactorHook::loadEnrolled($session->get('2fa_temporary_user'))
->assembleVerificationForm($this);
$this->assembleTwoFactorElements();
} else {
$this->assembleLoginElements();
}
@ -255,14 +289,14 @@ class LoginForm extends CompatForm
break;
case TwoFactor::SUBMIT_VERIFY_2FA:
case static::SUBMIT_VERIFY_2FA:
$session = Session::getSession();
/** @var User $user */
$user = $session->get('2fa_temporary_user');
$twoFactorMethod = TwoFactorHook::loadEnrolled($user);
if (
$this->getElement(TwoFactor::TOKEN_INPUT)
&& $twoFactorMethod->verify($this->getValue(TwoFactor::TOKEN_INPUT))
$this->getElement(static::TOKEN_INPUT)
&& $twoFactorMethod->verify($this->getValue(static::TOKEN_INPUT))
) {
$user->setTwoFactorSuccessful();
$session->delete('2fa_must_challenge');
@ -286,7 +320,7 @@ class LoginForm extends CompatForm
return;
}
$this->getElement(TwoFactor::TOKEN_INPUT)->addMessage($this->translate('Token is invalid!'));
$this->getElement(static::TOKEN_INPUT)->addMessage($this->translate('Token is invalid!'));
}
// Display the messages that were added to form or form elements

View file

@ -9,19 +9,9 @@ use Icinga\Exception\ConfigurationError;
use Icinga\Forms\Account\TwoFactorEnrollmentForm;
use Icinga\User;
use ipl\Html\FormElement\FieldsetElement;
use ipl\Web\Compat\CompatForm;
interface TwoFactor
{
/** @var string Name of the token text input in the verification form */
const TOKEN_INPUT = '2fa_token';
/** @var string Name of the verify submit button in the verification form */
const SUBMIT_VERIFY_2FA = 'btn_submit_verify_2fa';
/** @var string Name of the cancel submit button in the verification form */
const SUBMIT_CANCEL_2FA = 'btn_submit_cancel_2fa';
/**
* Get the unique machine-readable identifier for this 2FA method
*
@ -97,17 +87,4 @@ interface TwoFactor
* @param FieldsetElement $fieldset The method-specific fieldset to add elements to
*/
public function assembleEnrollmentFormElements(FieldsetElement $fieldset): void;
/**
* Add the method-specific form elements to the login form
*
* Called from the login form's assemble method when a 2FA challenge is required.
* Implementations add the token input using {@link TOKEN_INPUT} as its name and
* the verify submit button using {@link SUBMIT_VERIFY_2FA} as its name. To cancel
* the verification process a cancel button using {@link SUBMIT_CANCEL_2FA} as its
* name should be created.
*
* @param CompatForm $form The form to add elements to
*/
public function assembleVerificationForm(CompatForm $form): void;
}