Turning on MFA took one line. Making it true took three routes and a test kit

We are preparing Veridom, a property management app built on Kumiko, for a closed launch. One of the launch requirements was simple to state: every admin signs in with a second factor. In Kumiko that is a config value on the MFA feature:

return { issuer: BRAND_NAME, setupTokenSecret, challengeTokenSecret, requiredPolicy: "admins" };

The policy has three values: optional, admins and all. With admins, a password login by an admin who has no factor gets a setup step instead of a session, and an admin with a factor gets a challenge. The login handler enforces that.

The change was one line. Everything after it is what this post is about.

89 red tests, one message

The first effect was that the whole end-to-end suite went red. 89 Playwright specs, all failing with 401 on their first API call.

Every spec seeds a tenant and signs in its admin through a helper from our test kit. The helper posted the password, got back { mfaRequired: true, challengeToken } instead of a session cookie, and returned as if everything was fine. The 401 came one request later, from code that had nothing to do with login.

The fix for the suite was to teach the kit MFA. The more useful fix was to make the helper fail where the problem is:

if (reply.mfaRequired === true || reply.mfaSetupRequired === true) {
  throw new Error(
    `loginViaApi(${email}): the account needs MFA but no mfaTotpSecret was given; ` +
      `use seedTenant({ mfa: "totp" }) or addUser(roles, { mfa: "totp" })`,
  );
}

A test helper that swallows an unexpected state turns one clear failure into 89 confusing ones.

Teaching the kit TOTP

The kit now enrolls a real TOTP factor through the real endpoints. Under a policy that blocks the login, that is the pre-auth enrollment flow. Without one, it is the normal enrollment from a signed-in session. seedTenant() does this on its own when the admin's login answers with a setup step, so an app that switches on the policy does not have to touch every spec. Users created later can opt in with addUser(roles, { mfa: "totp" }), which matters when a spec creates a member and promotes them to admin afterwards.

One detail cost some thought. The server burns an accepted code for its whole validity window of one step either side, which is correct: a code seen once should not log anyone in twice. A suite that signs the same user in three times within 30 seconds then needs the next counter, and only codes up to one step ahead are valid. So the kit keeps the last counter per secret and, when it runs out of valid counters, waits until the clock catches up. That wait is tied to the TOTP window. It is not a test timeout, and we do not raise timeouts to make tests pass. In practice the full suite of 89 specs ran in 55 seconds with MFA on.

The policy had three doors around it

With the suite green, a security review of the release went through the diff. It found nothing blocking in the app. It found two problems in the framework and pointed at a third.

Switching tenants. A user can belong to several tenants. Picture someone who is a plain member in tenant A and an admin in tenant B, with no second factor. They sign in to A, where the policy does not apply to them, and get a session. Then they call POST /auth/switch-tenant for B. That route minted a fresh session for B without asking the MFA question. An admin session without a second factor, from a stolen password.

Accepting an invitation. Completing an invite signup issued the first session directly. That included admin invitations. The account holder could have enrolled a factor themselves, so this was hard to exploit, but it meant the first admin session of every invited admin had no second factor.

Signing up. Self-service signup creates a new tenant and makes the person who signed up its admin. Confirming the signup issued that admin session directly, again without the gate. Veridom launches without self-registration, so this one did not block us, but it is the same hole.

All three had the same root. The policy was enforced in the login handler, and these routes mint sessions without going through it. A policy flag is only as strong as the set of routes that create sessions, and nobody had made a list of that set.

The agent's first fix was worse than the bug

I gave the fix to a coding agent with a clear description: put the same MFA check in front of the switch-tenant session.

There is a layering wrinkle. The route lives in the framework core, the MFA gate lives in a bundled feature that the core cannot import. The agent solved that by creating a write handler for the gate and dispatching it from the route. Its first version took tenantId and roles from the request payload and was open to every signed-in user. Since all write handlers are reachable through the public write endpoint, anyone could have asked for an MFA challenge for any tenant, with any roles they liked. The second version restricted the handler to signed-in users and took the userId from the payload, which made it possible to trigger the gate for other people.

An automated security check flagged both versions while the agent was still working. I stopped it each time with the same rule, written out explicitly:

• only the system identity may call the handler, and the route dispatches it as the system user;
• the payload carries userId and tenantId, nothing else;
• the handler resolves the membership and the roles itself, on the server, and returns an error when there is no membership;
• a test calls the handler directly as a normal user and expects a 403.

That is the version we kept. The useful lesson for me: "put the same check here" is not a spec for auth code. The agent did what it was asked and filled the gaps with the most convenient assumption, which in auth code is the client's word. The rules that matter (who can call this, which inputs are trusted) have to be in the task, and a reviewer has to check them in the diff.

Tests that fail when the gate is gone

Each new test for the first two routes was checked by removing the thing it protects. Taking the gate out of the switch-tenant route turned two tests red. Taking it out of the invite signup turned one red. Opening the gate handler to normal users again turned four red, including the direct call.

The new integration tests run over real HTTP against a real stack and cover: switching to an admin tenant without a factor (setup step, no session cookie), with a factor (challenge, then a session after verifying), switching to a tenant where the user is only a member (direct session), switching to a tenant without membership (error, no token), the invite signup for an admin and for a member, and the signup confirmation under the policy, including a direct call through the public write endpoint.

Making the list

Each fix closes one route. To catch the next route that mints a session, the framework now keeps the list we were missing. Every production route mints its session through one function, and a guard test lists each of those routes together with how the MFA gate applies to it. A new route that mints a session without an entry fails the build, and so does a token signed anywhere else.

All of this shipped in Kumiko 0.345.0. Since 0.347.0, signing up under the policy also keeps the page the user wanted to land on after they enroll their factor. If you run Kumiko with an MFA policy, upgrade to at least 0.345.0.

If you run a multi-tenant app with per-tenant roles: does your MFA policy hold when a user switches tenants? We found out ours did not when a security review read the release that turned the policy on.