Add optional TOTP two-factor authentication for the admin page by tom27052006 · Pull Request #7444 · dani-garcia/vaultwarden

tom27052006 · 2026-07-19T14:08:34Z

Adds an optional TOTP second factor for the /admin page.

Setting ADMIN_TOTP_SECRET to a Base32 secret requires a 6-digit one-time code in addition to the admin token at login. It allows one 30-second step of time drift, rejects replayed codes, and keeps the existing admin rate limiting. TOTP verification is shared with the existing user 2FA implementation.

Admin TOTP can be managed from the normal config page. A new 20-byte secret and QR code are generated entirely in the browser using Web Crypto and a locally bundled QR generator. No external service is used. The secret is only saved through the normal config flow, and existing saved secrets are never sent back to the browser or shown again. Setting, replacing, and removing the secret all use the existing config flow, without a separate enrollment page or an additional TOTP check when disabling it.

Environment-provided secrets still work and can be overridden by the saved config. Removing the saved override falls back to the environment value.

Has no effect when DISABLE_ADMIN_TOKEN is enabled.

RaphaelRoumezin

Overall, the idea and implementation is good, and I'd love to see that become a feature.
However I feel pretty major design flaws in the overall execution, mainly from the configuration and enrollment flow in the admin panel.

Do consider I'm not a VW maintainer, and this is my personal opinion.

RaphaelRoumezin · 2026-07-20T08:02:26Z

+#[post("/totp/generate", format = "application/json")]
+fn totp_generate(_token: AdminToken) -> JsonResult {
+    use qrcode::{QrCode, render::svg};
+
+    let secret = crate::crypto::encode_random_bytes::<20>(&data_encoding::BASE32);
+    let uri = format!("otpauth://totp/Vaultwarden%20Admin?secret={secret}&issuer=Vaultwarden%20Admin");
+    let Ok(qr) = QrCode::new(uri.as_bytes()) else {
+        err!("Failed to generate QR code")
+    };
+    let qr_svg = qr.render::<svg::Color<'_>>().min_dimensions(240, 240).build();
+
+    Ok(Json(json!({
+        "secret": secret,
+        "uri": uri,
+        "qr_svg": qr_svg,
+    })))
+}


This could be done client side

I kept this on the server on purpose. The TOTP secret is the root secret, so I'd rather generate it in one place with the server's RNG than in the browser, where it lives in the page and depends on the page not being tampered with. It also has to reach the server to be saved anyway, so doing it in the browser doesn't really save anything — but it would mean adding a JS crypto/QR library to the admin panel, which is intentionally small, plain JS with no external/CDN code. The qrcode crate is build-time only, so there's no runtime cost. That said, this is a preference — if you or the maintainers would rather keep it fully client-side, I'm happy to change it.

If the browser has been tampered with and the sysadmin logs in the admin panel anyway, I think other problems might appear before RNG manipulation.
Plus, client-side secret generation is already performed for the cryptographic keys in Vaultwarden and for the password generator in clients, so staying on this track seems more appropriate.

Also, the qrcode crate is not build-time only, since you use it in runtime.

RaphaelRoumezin · 2026-07-20T08:09:57Z

+    if !check_totp_code(&secret, data.code.trim()) {
+        err!("Invalid TOTP code, please try again")
+    }


The admin is already authenticated, he shouldn't need to re-input the TOTP.

I would keep this too. Most 2FA systems ask for the second factor to turn the second factor off, and for a good reason: 2FA is meant to protect the case where the admin token leaked but the attacker doesn't have the device. If disabling needed no code, then just stealing a session cookie would be enough to remove 2FA, which defeats the whole point. It also doesn't cause lockouts, because the real recovery path for a lost authenticator is removing the secret from the config/env. But of course this is a judgment call — if the maintainers feel an active admin session should be trusted fully, I can drop the check.

If someone steals a session cookie they don't have to remove 2FA, they can just attack the configuration.
This panel is mainly a convenience for the config/env, and shouldn't constrain the admin in what they can edit or not (other than technical limitations ofc). If that was the case, the admin token should be protected in the same manner.

RaphaelRoumezin · 2026-07-20T08:14:11Z

In my opinion, the whole TOTP configuration flow could be done entirely client-side, on the config page. The ADMIN_TOTP_SECRET should be as editable as the others, and a modal could provide a helper for configuring it (with QR code and everything).
It would then just use the configuration edit flow to set/clear the TOTP secret.

This would also avoid the unnecessary complex code in src/config.rs to make the value non-editable from the admin page and have special states whether it was set from the environment.

I would keep this server-side. As a normal editable field, any admin session could read or delete the secret in one click, and there'd be no server check that a code works before enabling it (lockout risk). A browser-side check can also be skipped by posting the value directly, so it has to run on the server anyway. Part of the config.rs code also fixes an existing bug (_duo_akey being dropped on save), so I'd rather keep it. But it's the biggest design call here — happy to switch to the simpler approach if the maintainers prefer it.

Again, it's also a risk having the admin token be unprotected, but it's still done anyway. This panel doesn't need "restrict you for your own good" features.

Extract a single `verify_totp()` helper in `two_factor::authenticator` that performs the time-window check, and use it from both `validate_totp_code` (user 2FA) and the admin-page TOTP check instead of duplicating the loop. Each caller keeps its own last-used storage (DB vs. in-memory) and drift policy. The user path now compares in constant time, and a unit test for the helper is added.

`admin_totp_status()` now checks the environment first and reports "config" instead of "enrolled", since a secret written to `config.json` by hand is indistinguishable from one enrolled via the admin page. This also fixes the case where the secret is set in both env and config: it is now correctly reported as not removable from the admin page, because the environment value stays effective.

tom27052006 · 2026-07-21T16:37:45Z

Overall, the idea and implementation is good, and I'd love to see that become a feature. However I feel pretty major design flaws in the overall execution, mainly from the configuration and enrollment flow in the admin panel.

Do consider I'm not a VW maintainer, and this is my personal opinion.

Thanks for the review! I've made a few changes, removed the recovery code, shared the TOTP check with the existing user 2FA, and made the env-vs-config status more honest. I kept the enable/disable flow on the server on purpose and explained why in the threads, but I'm happy to change more if you or the maintainers see it differently.

RaphaelRoumezin · 2026-07-21T17:55:25Z

I still believe most of the flows in this code should be done client-side, without even having server-side running code. This code is way too complex for the admin panel, which is supposed to be a helper web page for sysadmins.
At the very core, this feature should only be enabled through yet another configuration entry, and a client side helper interface should come second to complete the feature.

As I take time to read your code and write those reviews, I'd also appreciate if you actually read them instead of pasting them into your AI agent.

tom27052006 · 2026-07-21T19:00:06Z

I still believe most of the flows in this code should be done client-side, without even having server-side running code. This code is way too complex for the admin panel, which is supposed to be a helper web page for sysadmins. At the very core, this feature should only be enabled through yet another configuration entry, and a client side helper interface should come second to complete the feature.

As I take time to read your code and write those reviews, I'd also appreciate if you actually read them instead of pasting them into your AI agent.

Thanks, you are right about two things. I was wrong about qrcode. It is used at runtime. I also got the config order wrong. config.json overrides the environment, so my check is wrong.
I looked at the code again. The admin page can show the TOTP secret. The normal config reset can also bypass the extra TOTP check. So my earlier security argument was not right
I will make the code simpler and follow the existing config system. I will keep the TOTP secret hidden. I will move the other config fix into a separate change.
I used AI to help with some replies, but I am responsible for what I write and for the code. I should have checked these points better. Thanks for taking the time to review it!

tom27052006 · 2026-07-21T21:00:16Z

I’ve simplified the setup and moved it into the normal config page. The secret and QR code are now generated entirely in the browser. Saving, replacing, and removing the secret uses the existing config flow, and saved secrets are not shown again. I also removed the separate TOTP page, the server-side QR generation, and the extra code check when disabling TOTP.
@RaphaelRoumezin does this match what you had in mind?

tom27052006 added 3 commits July 19, 2026 16:06

Add optional TOTP second factor for the admin page

77debec

Add QR code enrollment and management page for admin TOTP

e049caa

Fix admin TOTP endpoint URLs

478c01d

RaphaelRoumezin suggested changes Jul 20, 2026

View reviewed changes

tom27052006 added 2 commits July 21, 2026 18:30

tom27052006 force-pushed the admin-totp branch from c2ce023 to cfc685e Compare July 21, 2026 16:31

Simplify admin TOTP configuration flow

7798f40

tom27052006 added 3 commits July 21, 2026 23:04

Merge branch 'main' into admin-totp

4e3d999

Merge branch 'main' into admin-totp

e1f7a46

Merge branch 'main' into admin-totp

7a34371

Uh oh!

Conversation

tom27052006 commented Jul 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RaphaelRoumezin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tom27052006 commented Jul 21, 2026

Uh oh!

RaphaelRoumezin commented Jul 21, 2026

Uh oh!

tom27052006 commented Jul 21, 2026

Uh oh!

tom27052006 commented Jul 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

tom27052006 commented Jul 19, 2026 •

edited

Loading