-
Notifications
You must be signed in to change notification settings - Fork 282
Two Factor Authentication V2 #5430
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
e8d3b69 to
ecd73a6
Compare
|
The failed phpcs tests are not due to my changes. |
I'm afraid I have to convert this PR to draft in this case.
Well, the current master is all-✅. |
| CONSTRAINT idx_icingaweb_schema_version UNIQUE (version) | ||
| ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin ROW_FORMAT=DYNAMIC; | ||
|
|
||
| CREATE TABLE `icingaweb_totp`( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to @nilmerg, IW2 PRs with schema changes shall include schema upgrades.
| `username` varchar(254) COLLATE utf8mb4_unicode_ci NOT NULL, | ||
| `secret` varchar(255) NOT NULL, | ||
| `ctime` timestamp NULL DEFAULT NULL, | ||
| PRIMARY KEY (`username`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mtime is missing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mtime is not needed, because the entry is never modified.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't it? What about replacement #5430 (comment)? Shouldn't it be NULLed on removal, so that I can always see when I changed 2FA the last time?
If no, why is it in the Model then?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On removal the secret should be completely removed. The column in the model will be removed.
| @@ -0,0 +1,38 @@ | |||
| <?php | |||
|
|
|||
| namespace Icinga\Model; | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
License header missing. Also check other newly added files.
|
|
||
| class TotpModel extends Model | ||
| { | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| CONSTRAINT idx_icingaweb_schema_version UNIQUE (version) | ||
| ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin ROW_FORMAT=DYNAMIC; | ||
|
|
||
| CREATE TABLE `icingaweb_totp`( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Even if I imagine for a moment I had a schema upgrade, when I log in as admin to apply it via web UI, IW2 greets me with:
SQLSTATE[42S02]: Base table or view not found: 1146 Table 'iw2.icingaweb_totp' doesn't exist
| * | ||
| * This form is used to manage the TOTP settings of a user account. | ||
| */ | ||
| class TotpConfigForm extends Form |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As a user I may get a new phone sooner or later. To transfer 2FA from one phone to another, I could imagine disabling it temporarily. But this may be not wanted for security or policy reasons. Then I have to add a second TOTP temporarily, but neither the code nor the schema permit this.
At least I'd allow replacing a TOTP atomically.
| * | ||
| * This form is used to manage the TOTP settings of a user account. | ||
| */ | ||
| class TotpConfigForm extends Form |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As a user I'd like to see the secret exactly once, after enabling/replacing 2FA, together with a hint that I should use the secret as recovery code.
Or at least a hint that I can save the QR code somewhere for recovery purposes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The text input that shows what secret is stored in the database, was only a temporary solution. I thought about removing it completely and show the secret only in the token verification form in the totp_manual_token_url textarea.
|
|
||
| public function createElements(array $formData): void | ||
| { | ||
| if (! IcingaTotp::hasDbSecret($this->getDb(), $this->user->getUsername())) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't this DB query superfluous as you load the TOTP below anyway?
The column is never used and won't in the future.
Previously, a new 2FA secret was generated and stored temporarily in the session. It was only persisted to the database when the user clicked the "Save Changes" button. This behavior has been removed. Now, the secret is written directly to the database once it has been initially verified with a token. To generate a new secret if one already exists, just remove the old one. There is no longer a separate setting to toggle whether 2FA is enabled for a user — 2FA is considered enabled if a secret exists for that user in the database.
Add cancel submit button explicitly instead of setting it implicitly
by calling `setSubmitLabel('...')` to add `.btn-cancel` class so the
cancel button does not look like a primary button.
Rename from 'must_challenge_2fa_token' to '2fa_must_challenge_token' for a uniform naming schema in the future always starting with '2fa_'.
Remove the cookie for a successful 2fa, because there is no need for it. Instead set the 2fa directly to successful on the user.
If 2fa is enable the remember me cookie only gets set if the 2fa authentication was successful. To log the user back in from the cookie the 2fa will be skipped.
This PR is based on #5397 and will therefore close it. Introduces basic two-factor authentication (2FA) using Time-based One-Time Passwords (TOTP) support for Icinga Web 2.
TOTP (Time-based One-Time Password) is a type of two-factor authentication (2FA) method. It generates short-lived numeric codes (usually 6-8 digits) based on:
Since the code changes frequently and is only valid for a short window, it makes accounts harder to hack, even if someone steals your password.
After enabling 2FA, users authenticate with their usual Icinga Web credentials and are then prompted to provide a valid TOTP code from their authenticator app (e.g., Google Authenticator, Authy).
We will use the defaults of 6 digit tokens, 30 seconds interval and sha-1 algorithm for secret generation. The generated token is valid for 10 seconds before and after the current time to allow some clock drift.
Current Authentication Process
Currently, Icinga Web authentication follows this workflow:
redirect.2FA Implementation
This implementation is split in two main parts.
Configuration
The configuration of TOTP-based 2FA is fully integrated into the user’s account settings. A new form (
TotpConfigForm) allows users to enable, verify, or remove a TOTP secret directly from their account settings.When a user with the required permission (
user/two-factor-authentication) accesses their account, the AccountController loads the current TOTP secret from the database (IcingaTotp::loadFromDb()). If none exists, a new secret is generated (IcingaTotp::generate()) and passed into the form.The form (
TotpConfigForm) then guides the user through the following workflow:IcingaTotp::createQRCode()) and a manual provisioning URI. The user can scan this code with an authenticator app.IcingaTotpclass. Only if the verification succeeds will the secret be persisted to the database (saveToDb()).removeFromDb()).Throughout this process, user feedback is provided through notifications on success or failure (e.g., “TOTP 2FA has been configured successfully” or “The verification code is invalid”).
This design ensures that TOTP setup is entirely self-service for end users: they can enable, verify, and disable 2FA directly from their own account settings.
Authentication
Once a user has a TOTP secret configured, the login flow gains a second step. After the username and password are verified, the user is marked as authenticated but flagged as pending a second factor. The session sets a
2fa_must_challenge_tokenmarker, and if “Stay logged in” was checked, a temporary remember-me cookie is stored in the session but not yet issued. This prevents bypassing 2FA by simply reusing the cookie.Auth::isAuthenticated()also enforces that a user is only considered fully logged in once the TOTP challenge has succeeded, unless explicitly skipped for trusted cookies.The controller then decides whether to render the regular login form or
Challenge2FAForm. If the 2FA flag is set, the latter prompts for the one-time code. A valid remember-me cookie can bypass the challenge: if present and verified, the cookie is renewed, persisted, andisAuthenticated(true)passes without asking for a token.When the challenge form is submitted, the secret is loaded from the database and the entered code is verified. A successful check sets
TwoFactorSuccessful, clears the session marker, persists any deferred remember-me cookie, and callsAuthenticationHook::triggerLogin(). On failure, an inline error is shown. A companionCancel2FAFormlets users abort, which purges the session and resets the login.Until the token is verified, the user in session isn’t fully authorized and authentication checks will fail. Only after a valid TOTP code (or a trusted cookie) is confirmed does the session become a complete login, ensuring enforcement server-side, preventing redirect abuse, and avoiding issuance of persistent cookies before verification.
Future
Possible extensions for the future could be:
Requires Icinga/icinga-php-thirdparty#53
Closes #5397