The `TwoFactor` interface methods that contain logic are implemented by module
developers and may contain arbitrary third-party code. Without guards at the
call sites, an uncaught exception from a faulty implementation could crash the
whole application.
Each guard logs the exception with a confidential trace. `isEnrolled()` in
`loadEnrolled()` rethrows so the caller aborts rather than silently continuing.
`verify()` and `assembleEnrollmentFormElements()` show a user-facing message
and call `onError()` to keep the user on the form.
The existing `enroll()` and `unenroll()` catches are also updated: two separate
`Logger::error()` calls become one, and the user-visible messages gain
translation context.
Cookies issued while 2FA was active retain their validity after unenrollment
without this change. Revoking them forces a fresh login and prevents sessions
that were established under the enrolled requirement from persisting in a state
where that requirement no longer applies.
Previously both methods were declared to throw only `ConfigurationError`
and callers did not catch it. This removes the now-overly-narrow
`@throws ConfigurationError` declarations and wraps both call sites in
`try/catch(Throwable)`, letting the enrollment form log the full
confidential trace and show the exception message inline rather than
leaving the user with an unhandled error page.
The enroll false path is also updated: `Notification::error()` is
replaced with `$this->onError()` to keep error feedback consistent with
the exception path.
The empty placeholder `' - Please choose - '` could be re-selected after a real
method had been picked, resulting in `TwoFactorHook::fromName()` searching for
an empty string. Setting `disabledOptions` to `['']` prevents that by making the
placeholder unselectable once the user has moved past it.
With `required` set, the form will not pass validation when no method is
selected, so `getValue(static::METHOD)` in `onSuccess` can never return null.
The `?? ''` fallback passed to `TwoFactorHook::fromName` is dead code once
form validation runs.
Setting the `icinga-module module-{name}` class on the fieldset before calling
`assembleEnrollmentFormElements` let hook implementations overwrite or clear it
by calling `setAttributes` on the fieldset themselves. Moving the assignment to
after assembly ensures the CSS scoping class is always present regardless of
what the hook does.
The guard on the fieldset name ensures a hook implementation cannot change the
fieldset name during assembly, which would break form submission handling.
Without unique ids, `getCSSPath()` in `loader.js` produces the same path for the
submit buttons of both forms. When the login form is submitted and the container
transitions to the two-factor challenge form, `renderContentToContainer()`
restores focus to that path, incorrectly focusing the challenge form's submit
button instead of the token input.
Unique ids make the paths differ so no match is found and the autofocus class on
the token input takes effect as intended.
Previously the `RememberMe` object (containing the AES-encrypted password and
the decryption key) was serialized directly into the PHP session while waiting
for the user to complete the 2FA challenge. Because PHP sessions are written
to disk in plaintext, this exposed the key and ciphertext in the same place,
sufficient to recover the user's password.
Fix by splitting the secret across the session and the database, mirroring the
design of the normal (non-2FA) remember-me flow:
- Call `persist()` at login time so the AES key goes to the database
immediately, never touching the session.
- Store only the cookie value string (ciphertext + IV) in the session.
The ciphertext is not exploitable without the key.
- After a successful 2FA challenge, reconstruct the `RememberMe` object via a
new `RememberMe::fromCookieData()` factory that does a DB lookup by IV and
restores the canonical expiry from the database row.
- Only then send the browser cookie, so the cookie never reaches the browser
unless the second factor was verified.
Canceled challenges remove the created DB row, while abandoned challenges
leave an orphaned DB row which is cleaned up by the existing
`RememberMe::removeExpired()` mechanism.
`RememberMe::fromCookieData()` sets `$expiresAt` from the database row so
the browser cookie issued after 2FA inherits the expiry stored at login time
rather than receiving a fresh 30-day window computed at challenge-completion
time. The renewal path in `AuthenticationController::loginAction()` is
unaffected, because `renew()` constructs a new object via `fromCredentials()`.
If the 2FA method a user was challenged for is no longer available when they
submit the token — e.g. the module providing it was disabled mid-session — show
an informative message instead of a fatal error. The message tells the user that
their method is gone, suggests contacting an administrator if unexpected, and
points them to 'Back to login' explaining that no second factor will be required
once the method is disabled.
Add `LoginRedirect` that extends `HiddenElement` with a single `getUrl()` method
that encapsulates the three-step redirect resolution used in both login and
two-factor challenge form: fall back to `LoginForm::REDIRECT_URL` when the value
is empty or points to the logout action, then reject external URLs with a 400.
`LoginForm` and `TwoFactorChallengeForm` both replace their plain `'hidden'`
element with `LoginRedirect` and drop their identical `createRedirectUrl()`
methods in favor of `$this->getElement('redirect')->getUrl()`.
`AuthenticationController::loginAction()` had a pre-assembly call to
`$form->createRedirectUrl()` for the already-authenticated path. At that point
`handleRequest()` has not yet been called, so the form is not assembled and the
`redirect` element does not exist — calling `getElement()` would throw. That
path is also only reached when no `redirect` query param is present (the param
is handled explicitly on the line above), so the call always returned the
fallback URL anyway. It is replaced with a direct
`Url::fromPath(LoginForm::REDIRECT_URL)`.
Add `RememberMe::removeAllByUsername()` which deletes all rows from
`icingaweb_rememberme` for a given user. Call it from
`TwoFactorEnrollmentForm::onSuccess()` after a successful enroll so that
pre-enrollment cookies on other devices cannot bypass the newly required
second factor.
`TwoFactorController::configAction()` renders `TwoFactorEnrollmentForm`, which
shows a method-selector dropdown (autosubmit), the method-specific fieldset
contributed by `assembleEnrollmentFormElements()`, and either an "Enroll" or
"Unenroll" submit button depending on current enrollment state.
`AccountController`, `MyDevicesController`, and `NavigationController` each gain
a "Two-Factor Auth" tab.
After successful password authentication, check whether the user is enrolled in
any 2FA method via `TwoFactorHook::loadEnrolled()`. If enrolled, stores the user
in `TwoFactorState`, stashes the `RememberMe` cookie for later (if the checkbox
was checked), and redirects to `authentication/twofactor`. Users without a
registered 2FA credential continue through the existing authentication flow
unchanged.
`TwoFactorChallengeForm` renders the second login step: a token input, a
"Verify" submit button, and a "Back to login" link button. `onSuccess()` calls
`TwoFactorHook::loadEnrolled()` and `TwoFactor::verify()` to verify the token,
calls `Auth::setAuthenticated()`, persists the `RememberMe` cookie if one was
stashed, completes the challenge, fires `AuthenticationHook::triggerLogin()`,
and redirects.
Add `.btn-back-to-login-link` to `login.less`. A link-styled submit button with
an arrow to the left with an underline on hover.
Replace the legacy Zend-based `LoginForm` with an ipl `CompatForm`-based form:
- Add `CsrfCounterMeasure` and `FormUid` traits
- Replace `init()`/`createElements()` with `__construct()´/`assemble()` using
inline ipl decorator objects instead of Zend decorator arrays
- Replace `getRedirectUrl()` with `createRedirectUrl()` because
`getRedirectUrl()` would override `ipl\Html\Form::getRedirectUrl()`
- use `hasBeenAssembled()` instead of `$this->created`
- use `str_contains()` instead of `strpos()`
- Rewrite `onSuccess()` for `CompatForm`:
- change visibility from `public` to `protected`
- void return type
- use `Icinga::app()->getResponse()` instead of `$this->getResponse()`
- use `addMessage()` instead of `addError()`
- call `setRedirectUrl()` explicitly
Besides that this is a destructive operation which shouldn't
get automatic focus… our native way to adhere to WCAG standards
is by focusing the newly rendered container which suffices.
Replace deprecated `PDO::MYSQL_*` constant usage with the driver-specific
`Pdo\Mysql::ATTR_*` constants introduced in PHP 8.4.
This prepares the code for PHP 8.5, where accessing MySQL driver constants
through the generic `PDO` class is deprecated.
This change requires a compatibility shim on older PHP versions to provide
`Pdo\Mysql` for runtimes that do not expose the driver-specific class yet. The
shim is provided in `ipl-sql`. As a consequence, the required version of the
Icinga PHP library has been raised.
Since PHP 8.4 implicitly nullable parameter types are deprecated.
Normalize scoped PHPDoc for nullable-parameter updates: use `?Type` instead of
`Type|null`, remove column alignment, and indent continuation lines with 2
spaces.
Co-authored-by: "Eric Lippmann <eric.lippmann@icinga.com>"
