Skip to content

Two Factor Authentication V2#5430

Draft
jrauh01 wants to merge 52 commits intomainfrom
2fa-new
Draft

Two Factor Authentication V2#5430
jrauh01 wants to merge 52 commits intomainfrom
2fa-new

Conversation

@jrauh01
Copy link
Copy Markdown
Contributor

@jrauh01 jrauh01 commented Oct 2, 2025

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:

  1. A shared secret key (known to both the server and your authenticator app).
  2. The current time (in 30-second intervals by default).

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 token 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:

  • First it checks if there is set a remember me cookie from past logins.
  • If so, it attempts to log the user in using the encrypted credentials stored in the cookie.
  • If there is no remember me cookie set, it will show a login form.
  • If submitted it will compare the password with the hash in the database.
  • If correct it will setup the user and store it in the database.
  • Additionally it will create the remember me cookie if the slider for that in the login form is checked.
  • Then it will redirect to the dashboard or another target set by url param redirect.

2FA Implementation

This implementation is split in two main parts.

  1. The configuration of 2FA via the account settings
  2. The actual authentication process with token input

Configuration

The configuration of TOTP-based 2FA is fully integrated into the user’s account settings. A new form (TwoFactorConfigForm) allows users to enable 2FA, 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 (TwoFactorTotp::loadFromDb()). If none exists, a new secret is generated (TwoFactorTotp::generate()) and passed into the form.

The form (TwoFactorConfigForm) then guides the user through the following workflow:

  • Enable 2FA: If no TOTP secret is stored, the user can opt in via a checkbox. Once enabled, a new secret is created, and the form displays both a QR code (rendered by TwoFactorTotp::createQRCode()) and a manual provisioning URI. The user can scan this code with an authenticator app or copy the URI via a copy-to-clipboard element.
  • Verification Step: To prevent misconfiguration, the user must enter a valid TOTP code generated by their app. The code is checked using the verify method of the TwoFactorTotp class. Only if the verification succeeds will the secret be persisted to the database (saveToDb()).
  • Remove 2FA: If a secret already exists in the database, the form instead provides the option to remove it. On form submit, the stored secret is deleted (removeFromDb()). (Should be extended in the future to ask for confirmation!)

Authentication

Once a user has a TOTP secret stored, the login flow gains a second step. After the username and password are verified, the session sets a 2fa_must_challenge_token flag, stores a temporary user object, and if “Stay logged in” was checked, a temporary remember-me cookie in the session. The remember-me cookie is not persisted yet. 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 LoginForm is rewritten now extending ipl\Web\Compat\CompatForm and decides which elements it should render. If the 2fa_must_challenge_token flag is set in the session, the elements to verify the TOTP token are displayed, otherwise the normal login form elements. A valid remember-me cookie can bypass both, the password login and the token challenge: if present and verified, the cookie is renewed, persisted, and the user is logged in.

When the LoginForm is submitted via the btn_submit_verify_2fa submit button, the secret is loaded from the database and the entered code is verified. A successful check sets $twoFactorSuccessful to true on the user object that was temporarily stored in the session after password login, clears the 2fa_must_challenge_token session flag, persists any deferred remember-me cookie, and calls AuthenticationHook::triggerLogin(). Submitting the form via the btn_submit_cancel_2fa button lets users abort, which purges the session and resets the login.

Future

Possible extensions for the future could be:

  • Encrypt secrets in the database
  • Option for administrators to enforce 2fa for users
  • Ask for password verification to add or remove a TOTP secret in the account settings

Requires GD php extension for QR code generation
Requires Icinga/icinga-php-thirdparty#53
Closes #5397

@jrauh01 jrauh01 self-assigned this Oct 2, 2025
@cla-bot cla-bot bot added the cla/signed label Oct 2, 2025
@jrauh01 jrauh01 force-pushed the 2fa-new branch 2 times, most recently from e8d3b69 to ecd73a6 Compare October 6, 2025 05:53
@jrauh01 jrauh01 requested a review from lippserd October 6, 2025 05:56
@jrauh01
Copy link
Copy Markdown
Contributor Author

jrauh01 commented Oct 6, 2025

The failed phpcs tests are not due to my changes.

@lippserd lippserd requested a review from Al2Klimov October 24, 2025 07:32
@Al2Klimov Al2Klimov marked this pull request as draft October 24, 2025 09:08
@Al2Klimov
Copy link
Copy Markdown
Member

Requires Icinga/icinga-php-thirdparty#53

I'm afraid I have to convert this PR to draft in this case.

The failed phpcs tests are not due to my changes.

Well, the current master is all-✅.

Comment thread schema/mysql.schema.sql Outdated
Comment thread schema/mysql.schema.sql Outdated
CREATE TABLE `icingaweb_totp`(
`username` varchar(254) COLLATE utf8mb4_unicode_ci NOT NULL,
`secret` varchar(255) NOT NULL,
`ctime` timestamp NULL DEFAULT NULL,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why NULL here? From a high-level PoV, 2FA entries always have a creation time.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would simply stick to how all other ctime columns are created in the schema.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"all other ctime columns" are timestamp, yours a bigint.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like we discussed offline I left them bigint and set to NOT NULL.

Comment thread schema/mysql.schema.sql
Comment thread library/Icinga/Model/TotpModel.php Outdated
Comment thread library/Icinga/Model/TotpModel.php Outdated
Comment thread schema/mysql.schema.sql Outdated
Comment thread application/forms/Account/TotpConfigForm.php Outdated
Comment thread application/forms/Account/TotpConfigForm.php Outdated
Comment thread library/Icinga/Authentication/IcingaTotp.php Outdated
Comment thread application/forms/Account/TotpConfigForm.php Outdated
@jrauh01 jrauh01 force-pushed the 2fa-new branch 3 times, most recently from faa2b5f to 62f1087 Compare December 2, 2025 14:41
@jrauh01 jrauh01 requested a review from Al2Klimov December 2, 2025 14:46
Copy link
Copy Markdown
Member

@Al2Klimov Al2Klimov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tbh, I as a reviewer don't wanna even think about whose (branch) fault the red GHA is.

Just (someone) get it green, please. Thanks.

(Notes for the future myself:

Test system (I use NixOS btw)
{ pkgs, ... }: {
  imports = [
    ./hardware-configuration.nix


  ];

  boot.tmp.cleanOnBoot = true;
  networking.hostName = "aklimov-2fa";
  networking.domain = "";
  services.openssh.enable = true;
  users.users.root.openssh.authorizedKeys.keys = [''ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIBIroHYGSRaRNFxlK90SS0aHwWjEME30pK5J1N/V1w6a aklimov@ws-aklimov7777777.local'' ];
  system.stateVersion = "23.11";

networking.firewall.allowedTCPPorts = [ 80 ];

virtualisation.oci-containers = {
backend = "podman";
containers.postgresql = {
autoStart = true;
image = "postgres:9.1";
environment = {
POSTGRES_PASSWORD = "123456";
};
ports = [ "127.0.0.1:5432:5432" ];
volumes = [ "postgres:/var/lib/postgresql/data" ];
};
};

  services.icingaweb2 = {
    enable = true;
    #virtualHost = "iw2.aklimov.net-dump.de";
    generalConfig.global.config_resource = "iw2";
    modules.monitoring.enable = false;
    authentications = {
      pg = {
        backend = "db";
        resource = "iw2";
      };
    };
    resources.iw2 = {
        type = "db";
        db = "pgsql";
        host = "127.0.0.1";
        port = "5432";
        dbname = "postgres";
        username = "postgres";
        password = "123456";
        charset = "utf8";
      };
    roles = {
      adm = {
        users = "icingaadmin,Alexander A. Klimov";
        permissions = "*";
      };
    };
    modulePackages = {
    };
  };

nixpkgs.overlays = [
(_: prev: {
icingaweb2-ipl = prev.icingaweb2-ipl.overrideAttrs (old: {
src = prev.fetchFromGitHub {
owner = "Icinga";
repo = "icinga-php-library";
rev = "v0.18.1";
hash = "sha256-V2PkRtVzMnzBpNLUMsSl5NUSnrycgEHGXLGilbhcGfw=";
};
}); })
(_: prev: {
icingaweb2-thirdparty = prev.icingaweb2-thirdparty.overrideAttrs (old: {
src = prev.fetchFromGitHub {
owner = "Icinga";
repo = "icinga-php-thirdparty";
#rev = "274212c7e02223687eacdfbc78c48181878ac910"; # feature/2fa
#hash = "sha256-K+NRUDnQjGG0Lmi5AOtftfueZM+VE6qkOWXF9qLzi/Y=";
rev = "1983c72cd3bbc0f4f0df5f464db4985417b4e73d"; # aklimov/feature/2fa
hash = "sha256-3CIAfhSr9GGhytOPnSfnnt/H6xWJOlQzyrZpux0VZbI=";
};
}); })
    (_: prev: { icingaweb2 = prev.icingaweb2.overrideAttrs (old: {
      src = prev.fetchFromGitHub {
        owner = "Icinga";
        repo = "icingaweb2";
        #rev = "62f10871e9e270789def9a3776656ff15d094013"; # 2fa-new
        #hash = "sha256-iCoCCEGLO3vf9VdbDWmecBMaRuUAKJc7f9AeNKLQv0A=";
rev = "e0e2a22fbeb90ae721b5d301af235467aa22d3ef"; # aklimov/2fa-new
hash = "sha256-0SaOkROWkc20j7MxGWsx5q4aSQoRTDaQazd2SJjCPbU=";
#rev = "70f29827f921f24b2afc0556c4800dfa55a2d0c1"; # main
#hash = "sha256-fCd5kyJk7LXdKkZS7kppPO1wfeNTOD4esliqvSAtyV4=";
      };

      patches = [
        ./opcache_reset.patch

        # https://github.com/Icinga/icingaweb2/issues/5427
        ./migrations-db-no-pw.patch
      ];

      # https://github.com/NixOS/nixpkgs/pull/380065
      installPhase = old.installPhase + "\ncp -ra schema $out";
    }); })
  ];

}

use Icinga\Forms\PreferenceForm;
use Icinga\User\Preferences\PreferencesStore;
use Icinga\Web\Controller;
use ipl\Html\Contract\Form;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Heads up! ipl\Html\Contract\Form was added recently to ipl, so our icingaweb2 2.13.0(?) packages MUST raise their dependencies. I didn't check all classes, though.

Copy link
Copy Markdown
Contributor Author

@jrauh01 jrauh01 Dec 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@@ -0,0 +1,6 @@
CREATE TABLE "icingaweb_2fa" (
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While I can login in order to apply this migration, /account says SQLSTATE[42P01]: Undefined table: 7 ERROR: relation "icingaweb_2fa" does not exist until applied.

Can't speak for others, but I personally could imagine just doing nothing here – i.e let the users fix it by applying the migration.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I also would let the user fix it by applying the migration.

*
* This form is used to manage the 2FA settings of a user account.
*/
class TwoFactorConfigForm extends CompatForm
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was barely able to integrate Icinga/icinga-php-thirdparty#53 for testing this.

  • composer update on the PR said The process "patch '-p1' --no-backup-if-mismatch -d '/Users/aklimov/NET/WS/icingaweb2/icinga-php-thirdparty/vendor/shardj/zf1-future' < '/Users/aklimov/NET/WS/icingaweb2/icinga-php-thirdparty/patches/shardj-zf1-future.patch'" exceeded the timeout of 300 seconds.
  • composer require spomky-labs/otphp chillerlan/php-qrcode on v0.14.0 missed the class for QR codes, so I had to comment those out

Please:

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Try to check out to 2686bdc3a0761519a4ffe432086a10cb19be921d, or just pull Icinga/icinga-php-thirdparty#53.

*
* This form is used to manage the 2FA settings of a user account.
*/
class TwoFactorConfigForm extends CompatForm
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I had no QR code, I made one myself from the shown URL:

  1. qrencode -o qr.png
  2. Paste otpauth:// URL
  3. Enter
  4. Ctrl-D
  5. open qr.png

But it didn't work:

IMG_5175

So I had to copy and paste just the secret to my phone.

* Form for user authentication
*/
class LoginForm extends Form
class LoginForm extends CompatForm
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Heads up, security hole! On Postgres, I can bypass 2FA for e.g icingaadmin by logging in as ICINGAADMIN.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nilmerg FYI same applies to user preferences. (Not a security hole, though.) I.e ICINGAadmin and icingaADMIN have distinct user preferences.😅

Copy link
Copy Markdown
Member

@Al2Klimov Al2Klimov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Almost forgot! The commit history got out of hand. Consider making one or a few large commits with shared authorship where applicable:

https://docs.github.com/de/pull-requests/committing-changes-to-your-project/creating-and-editing-commits/creating-a-commit-with-multiple-authors

Copy link
Copy Markdown
Member

@Al2Klimov Al2Klimov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lippserd Please write your requirements on 2FA + API requests.

  • They just shall not work? (What a pity.)
  • They just shall work with user:pass? (Security hole, our forms already allow API requests implicitly!)
  • OTP header required?
  • OTP is to be appended to the password? (username:password;otp)
  • ...

@Al2Klimov
Copy link
Copy Markdown
Member

... or personal access token? (idea (c) @TheSyscall)

lippserd and others added 5 commits December 11, 2025 14:20
The column is never used and won't in the future.
- 'TotpForm' to 'TotpConfigForm'
- 'Authentication\Totp' to 'IcingaTotp'
- 'Model\Totp' to 'TotpModel'
Change the label to disable the 2FA from 'Remove TOTP Secret' to
'Disable 2FA'.
Removed description for `rememberme` input, because it wasn't
displayed anywhere.
It validates two things:
1. The token must only contain numbers
2. The token must have a specific length (this is configurable via
   the constructor, the default is 6)
The form displays either the login inputs or the inputs to verify
the totp token depending on whether `'2fa_must_challenge_token'`
is set `true` in the session.
A new class `FakeFormElement` is introduced which integrates
`ValidHtml` into IPL Web `CompatForm`s. The following parts of the
`TwoFactorConfigForm` are now added to the Form via the new class:

- The QR code image
- The manual auth URL for TOTP

The manual auth URL is now wrapped by a copy-to-clipboard element,
so the user doesn’t have to select it manually. Additionally the
QR code image and the manual auth url have now descriptions.
Requires `endroid/qr-code` library in
Icinga/icinga-php-thirdparty#53.

Because we use **GPL-2.0** license for Icinga Web:

- `chillerlan/php-qrcode: **Apache-2.0** (incompatible)
- `endroid/qr-code`: **MIT** (compatible)
So the user can download the QR code to backup if they lose access
to their device.

The download currently does only work in chrome and firefox, but
not in safari. The reason is, that the action link uses
`ipl\Web\Url` which adds '///' between the schema and the base
path. The safari browser can't handle this.

'data:image/png;base64,...' is converted to
'data:///image/png;base64...'.
When using postgres db as authentication backend it was possible
to bypass 2fa by using e.g. `ADMIN` instead of `admin` as username.

This is now prevented by:
1. Ensuring there is only one secret per user no matter the case
2. Lowering all usernames for filtering in the database
This is more user friendly, because most authenticator apps just
want the secret for manual creation.
The following commands are added:
- `icingacli twofactor list` lists all users that have 2FA enabled.
- `icingacli twofactor disable [<user>]` disables 2FA for a user.
The user should not need to ask for more security.
If the device running the authenticator app is lost, the secret can
be used to set up an authenticator app on another device. But for
that the secret has to be stored on another device when setting up
2FA.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants