[GH-ISSUE #8627] twoFactor plugin skips verification if account is setup to use OTA #28468

New Issue

GiteaMirror · 2026-04-17T19:54:48-05:00

GiteaMirror commented

2026-04-17 19:54:48 -05:00

Originally created by @kvanbere on GitHub (Mar 16, 2026).
Original GitHub issue: https://github.com/better-auth/better-auth/issues/8627

Is this suited for github?

Yes, this is suited for github

To Reproduce

The twoFactor (two factor authentication) plugin has two kinds of tokens that it can generate: OTA and TOTP.

OTP tokens can be sent to the user via SMS or Email, whereas TOTP tokens are generated using an authenticator app such as Google Authenticator, Microsoft Authenticator, 1Password, etc.

To enable the twoFactor plugin, you must set twoFactorEnabled on a user. If you want to use OTP, then you actually cannot use auth.api.enableTwoFactor because this begins the flow for TOTP. In order to enforce OTP, you must call updateUser directly and set twoFactorEnabled: true without generating a TOTP for the user (so i.e. twoFactorEnabled is set on the user, but no associated row is created in the two_factor table with their TOTP secret).

Once two factor authentication is enabled for a user:

Calling auth.api.signInEmail will now return twoFactorRedirect instead of an actual session. When this twoFactorRedirect is present in the response, the application should redirect to a page that handles MFA. For example:

		let signInEmailResult;
		try {
			// KEEP: Authenticate with database
			signInEmailResult = await locals.auth.api.signInEmail({
				body: {
					email: email.data,
					password: passwordParam,
					rememberMe: true,
					callbackURL: '/settings'
				},
				headers: request.headers
			});
		} catch (ex) {
			if (ex instanceof APIError) {
				// KEEP: Handle authentication errors thrown as exceptions
				return fail(422, {
					error: ex.name,
					description: ex.message
				});
			} else {
				// KEEP: Fall through
				console.error(ex);
			}
		}

		if (!signInEmailResult) {
			return fail(422, {
				error: 'internal_error',
				description: 'Please try again later'
			});
		}

		// KEEP: Two factor authentication may not be required if device is already trusted
		if (
			!('twoFactorRedirect' in signInEmailResult) ||
			signInEmailResult.twoFactorRedirect !== true
		) {
			throw redirect(
				303,
				signInEmailResult?.redirect && signInEmailResult?.url ? signInEmailResult.url : '/settings'
			);
		}

		// KEEP: Two factor authentication is required:
		// KEEP: Check whether to use OTP or TOTP for authenticating this user
		let twoFactorRecord;
		try {
			const ctx = await locals.auth.$context;

			// KEEP: Lookup user identifier for two factor cookie
			const userId = await getTwoFactorUserId(cookies, ctx);

			if (!userId) {
				return fail(422, {
					error: 'internal_error',
					description: 'Please try again later (2)'
				});
			}

			// KEEP: Look up whether the user has created any TOTP codes
			twoFactorRecord = await ctx.adapter.findOne({
				model: 'twoFactor',
				select: ['id'],
				where: [{ field: 'userId', value: userId }]
			});
		} catch (ex) {
			// KEEP: Send to observability
			console.error(ex);
		}

		// KEEP: Redirect to two factor verification
		const twoFactorMethod = twoFactorRecord ? 'totp' : 'otp';
		throw redirect(303, `/login/mfa/${twoFactorMethod}`);

If the user is using OTP for two factor authentication, then you call auth.api.sendTwoFactorOTP and redirect to another page that uses auth.api.verifyTwoFactorOTP. When calling auth.api.verifyTwoFactorOTP with the correct OTP, then the session becomes valid.

But if this user is using TOTP (that is, if they have a row in the two_factor table and they've generated a valid TOTP secret in the past and verified their TOTP generator by supplying a valid code) then using auth.api.verifyTOTP directly with a valid code from the authenticator app will cause the session to become valid.

If a user is using OTP, then once they are logged-in they can call auth.api.enableTwoFactor which will generate a TOTP secret and link it to their account.

But here lies the issue:

The twoFactor plugin uses the twoFactorEnabled flag on the account as a heuristic to track whether the TOTP has been verified during the TOTP enrolment flow.

So if you enforce OTP verification for an account by setting twoFactorEnabled: true on that account, and then allow the user to later add a TOTP token to their account, then the very moment that they add a TOTP code to their account then it automatically becomes valid without verification. That is, the entire Better Auth verification flow is ignored and the TOTP becomes instantly active.

This more or less means that the user would get locked out of their account if they do not add the TOTP secret to their authenticator app at that very moment it is shown to them, for example if they close the browser window or change their mind.

Current vs. Expected behavior

If the application is enforcing OTP on every login by setting twoFactorEnabled: true on the user, then this should continue to work.
If a user who is already using OTP wants to enroll in TOTP instead, then they should be able to go through the TOTP enrolment flow.
A user who goes through the TOTP enrolment flow should be able to abandon the enrolment flow midway through (before final verification), and on their next login they should be prompted to enter an OTP rather than a TOTP, the latter causing them to be locked out of their account.

Suggested fix:

Currently the TOTP secret becomes verified, active and in-use if twoFactorEnabled is set on the user account. This assumption is fundamentally flawed, because the user could be using some other 2FA method (for example OTP).

Therefore it seems that whether or not the 2FA method is verified should be a property of the row created in the two_factor table.

I propose that the verified: boolean becomes a property of the specific TOTP that is generated. This is so that twoFactorEnabled in the user table only tracks whether 2FA is required to generate an active session, and whether or not a specific TOTP secret is verified or not is actually tracked under the two_factor table in the row for that specific TOTP.

This also enables the correct behaviour in the following two scenarios:

A user deletes their TOTP and correctly falls back to using OTP again, or
A user deletes their TOTP because it has been accidentally leaked. They create a new TOTP with a new secret, and by deleting the original TOTP then they are correctly sent back through the TOTP verification onboarding flow rather than having their second TOTP immediately marked as active (and potentially locking themselves out).

What version of Better Auth are you using?

1.5.4

System info

N/A

Which area(s) are affected? (Select all that apply)

Backend

Auth config (if applicable)

Additional context

(As an aside, you can make OTA authentication mandatory for all users using this database hook:)

		databaseHooks: {
			user: {
				create: {
					before: async (user: any) => {
						return {
							data: {
								...user,
								// KEEP: New users created with two factor authentication enabled by default
								twoFactorEnabled: true
							}
						};
					}
				}
			}
		}

See also https://github.com/better-auth/better-auth/issues/5738 and comments on https://github.com/better-auth/better-auth/pull/5739 .

Originally created by @kvanbere on GitHub (Mar 16, 2026). Original GitHub issue: https://github.com/better-auth/better-auth/issues/8627 ### Is this suited for github? - [x] Yes, this is suited for github ### To Reproduce The `twoFactor` ([two factor authentication](https://better-auth.com/docs/plugins/2fa#enabling-2fa)) plugin has two kinds of tokens that it can generate: `OTA` and `TOTP`. `OTP` tokens can be sent to the user via SMS or Email, whereas `TOTP` tokens are generated using an authenticator app such as Google Authenticator, Microsoft Authenticator, 1Password, etc. To enable the `twoFactor` plugin, you must set `twoFactorEnabled` on a user. If you want to use `OTP`, then you actually cannot use `auth.api.enableTwoFactor` because this begins the flow for `TOTP`. In order to enforce `OTP`, you must call `updateUser` directly and set `twoFactorEnabled: true` without generating a `TOTP` for the user (so i.e. `twoFactorEnabled` is set on the user, but no associated row is created in the `two_factor` table with their `TOTP` secret). Once two factor authentication is enabled for a user: 1. Calling `auth.api.signInEmail` will now return `twoFactorRedirect` instead of an actual session. When this `twoFactorRedirect` is present in the response, the application should redirect to a page that handles MFA. For example: ```ts let signInEmailResult; try { // KEEP: Authenticate with database signInEmailResult = await locals.auth.api.signInEmail({ body: { email: email.data, password: passwordParam, rememberMe: true, callbackURL: '/settings' }, headers: request.headers }); } catch (ex) { if (ex instanceof APIError) { // KEEP: Handle authentication errors thrown as exceptions return fail(422, { error: ex.name, description: ex.message }); } else { // KEEP: Fall through console.error(ex); } } if (!signInEmailResult) { return fail(422, { error: 'internal_error', description: 'Please try again later' }); } // KEEP: Two factor authentication may not be required if device is already trusted if ( !('twoFactorRedirect' in signInEmailResult) || signInEmailResult.twoFactorRedirect !== true ) { throw redirect( 303, signInEmailResult?.redirect && signInEmailResult?.url ? signInEmailResult.url : '/settings' ); } // KEEP: Two factor authentication is required: // KEEP: Check whether to use OTP or TOTP for authenticating this user let twoFactorRecord; try { const ctx = await locals.auth.$context; // KEEP: Lookup user identifier for two factor cookie const userId = await getTwoFactorUserId(cookies, ctx); if (!userId) { return fail(422, { error: 'internal_error', description: 'Please try again later (2)' }); } // KEEP: Look up whether the user has created any TOTP codes twoFactorRecord = await ctx.adapter.findOne({ model: 'twoFactor', select: ['id'], where: [{ field: 'userId', value: userId }] }); } catch (ex) { // KEEP: Send to observability console.error(ex); } // KEEP: Redirect to two factor verification const twoFactorMethod = twoFactorRecord ? 'totp' : 'otp'; throw redirect(303, `/login/mfa/${twoFactorMethod}`); ``` 2. If the user is using `OTP` for two factor authentication, then you call `auth.api.sendTwoFactorOTP` and redirect to another page that uses `auth.api.verifyTwoFactorOTP`. When calling `auth.api.verifyTwoFactorOTP` with the correct OTP, then the session becomes valid. But if this user is using `TOTP` (that is, if they have a row in the `two_factor` table and they've generated a valid `TOTP` secret in the past and verified their `TOTP` generator by supplying a valid code) then using `auth.api.verifyTOTP` directly with a valid code from the authenticator app will cause the session to become valid. 3. If a user is using `OTP`, then once they are logged-in they can call `auth.api.enableTwoFactor` which will generate a `TOTP` secret and link it to their account. **But here lies the issue:** The `twoFactor` plugin uses the `twoFactorEnabled` flag on the account as a heuristic to track whether the `TOTP` has been verified during the `TOTP` enrolment flow. So if you enforce `OTP` verification for an account by setting `twoFactorEnabled: true` on that account, and then allow the user to later add a `TOTP` token to their account, then the very moment that they add a `TOTP` code to their account then it automatically becomes valid without verification. That is, the entire Better Auth verification flow is ignored and the `TOTP` becomes instantly active. This more or less means that the user would get locked out of their account if they do not add the `TOTP` secret to their authenticator app at that very moment it is shown to them, for example if they close the browser window or change their mind. ### Current vs. Expected behavior 1. If the application is enforcing `OTP` on every login by setting `twoFactorEnabled: true` on the user, then this should continue to work. 2. If a user who is already using `OTP` wants to enroll in `TOTP` instead, then they should be able to go through the `TOTP` enrolment flow. 3. A user who goes through the `TOTP` enrolment flow should be able to abandon the enrolment flow midway through (before final verification), and on their next login they should be prompted to enter an `OTP` rather than a `TOTP`, the latter causing them to be locked out of their account. **Suggested fix:** Currently the `TOTP` secret becomes verified, active and in-use if `twoFactorEnabled` is set on the user account. This assumption is fundamentally flawed, because the user could be using some other 2FA method (for example `OTP`). Therefore it seems that whether or not the 2FA method is verified should be a property of the row created in the `two_factor` table. I propose that the `verified: boolean` becomes a property of the specific `TOTP` that is generated. This is so that `twoFactorEnabled` in the user table only tracks whether 2FA is required to generate an active session, and whether or not a specific `TOTP` secret is verified or not is actually tracked under the `two_factor` table in the row for that specific `TOTP`. This also enables the correct behaviour in the following two scenarios: 1. A user deletes their `TOTP` and correctly falls back to using `OTP` again, or 2. A user deletes their `TOTP` because it has been accidentally leaked. They create a new TOTP with a new secret, and by deleting the original `TOTP` then they are correctly sent back through the `TOTP` verification onboarding flow rather than having their second `TOTP` immediately marked as active (and potentially locking themselves out). ### What version of Better Auth are you using? 1.5.4 ### System info ```bash N/A ``` ### Which area(s) are affected? (Select all that apply) Backend ### Auth config (if applicable) ```typescript ``` ### Additional context (As an aside, you can make `OTA` authentication mandatory for all users using this database hook:) ```ts databaseHooks: { user: { create: { before: async (user: any) => { return { data: { ...user, // KEEP: New users created with two factor authentication enabled by default twoFactorEnabled: true } }; } } } } ``` See also https://github.com/better-auth/better-auth/issues/5738 and comments on https://github.com/better-auth/better-auth/pull/5739 .

GiteaMirror added the security bug labels 2026-04-17 19:54:48 -05:00

GiteaMirror closed this issue

2026-04-17 19:54:48 -05:00

GiteaMirror commented

2026-04-17 19:54:49 -05:00

@kvanbere commented on GitHub (Mar 16, 2026):

It is actually quite difficult to determine whether an account is using OTP or TOTP, so I am including my helper function in this bug report that returns the User ID from the twoFactor pre-session cookie, which is required to call ctx.adapter.findOne.

I think this might be helpful for testing:

// See https://github.com/better-auth/better-auth/discussions/8411
import { makeSignature, constantTimeEqual } from "better-auth/crypto";

import type { Cookies } from "@sveltejs/kit";
import type { AuthContext } from 'better-auth';

export const getTwoFactorUserId = async (
  cookies: Cookies,
  ctx: AuthContext,
): Promise<string | null> => {
  // Use ctx to get the dynamic name (handles __Secure- prefixes automatically)
  const cookieName = ctx.createAuthCookie('two_factor').name;
  const signedValue = cookies.get(cookieName);

  if (!signedValue) return null;

  // Decode URL-encoded cookie value
  const decodedValue = decodeURIComponent(signedValue);

  // Format is "value.signature" - split at last dot
  const lastDotIndex = decodedValue.lastIndexOf(".");
  if (lastDotIndex === -1) return null;

  const value = decodedValue.substring(0, lastDotIndex);
  const signature = decodedValue.substring(lastDotIndex + 1);

  // Verify signature
  const expectedSignature = await makeSignature(value, ctx.secret);
  if (!constantTimeEqual(signature, expectedSignature)) return null;

  // Look up verification record and check expiration
  const verification = await ctx.internalAdapter.findVerificationValue(value);

  if (!verification) return null;
  if (new Date(verification.expiresAt) < new Date()) return null;

  return verification.value; // This is the userId
}

(But it would be even better if twoFactorRedirect could specify, not a boolean, but an array of the available twoFactor methods that could be used such as ['otp', 'totp'] or similar.

Although this is outside the scope of this bug which is of more immediate concern.)

@kvanbere commented on GitHub (Mar 16, 2026): It is actually quite difficult to determine whether an account is using `OTP` or `TOTP`, so I am including my helper function in this bug report that returns the User ID from the `twoFactor` pre-session cookie, which is required to call `ctx.adapter.findOne`. I think this might be helpful for testing: ```ts // See https://github.com/better-auth/better-auth/discussions/8411 import { makeSignature, constantTimeEqual } from "better-auth/crypto"; import type { Cookies } from "@sveltejs/kit"; import type { AuthContext } from 'better-auth'; export const getTwoFactorUserId = async ( cookies: Cookies, ctx: AuthContext, ): Promise<string | null> => { // Use ctx to get the dynamic name (handles __Secure- prefixes automatically) const cookieName = ctx.createAuthCookie('two_factor').name; const signedValue = cookies.get(cookieName); if (!signedValue) return null; // Decode URL-encoded cookie value const decodedValue = decodeURIComponent(signedValue); // Format is "value.signature" - split at last dot const lastDotIndex = decodedValue.lastIndexOf("."); if (lastDotIndex === -1) return null; const value = decodedValue.substring(0, lastDotIndex); const signature = decodedValue.substring(lastDotIndex + 1); // Verify signature const expectedSignature = await makeSignature(value, ctx.secret); if (!constantTimeEqual(signature, expectedSignature)) return null; // Look up verification record and check expiration const verification = await ctx.internalAdapter.findVerificationValue(value); if (!verification) return null; if (new Date(verification.expiresAt) < new Date()) return null; return verification.value; // This is the userId } ``` (But it would be even better if `twoFactorRedirect` could specify, not a boolean, but an array of the available `twoFactor` methods that could be used such as `['otp', 'totp']` or similar. Although this is outside the scope of this bug which is of more immediate concern.)

GiteaMirror commented

2026-04-17 19:54:50 -05:00

@kvanbere commented on GitHub (Mar 16, 2026):