Security audit notes: OAuth state validation, whitelist semantics, and encryption key fallback

项目地址：https://github.com/nap0o/2fauth-worker

Hi, thanks for open‑sourcing this project — I did a quick security review of the backend + frontend and wanted to share some findings/suggestions. This is written from the perspective of treating 2fauth-worker as a “2FA password manager”, i.e. very high sensitivity by design.

1. OAuth state only validated on the frontend (not on the server)

Files:

• frontend/src/features/auth/views/login.vue
• frontend/src/features/auth/views/oauthCallback.vue
• backend/src/features/auth/authRoutes.ts
• backend/src/features/auth/authService.ts

Current flow (simplified):

• GET /api/oauth/authorize/:provider generates a random state and returns it to the SPA:const state = crypto.randomUUID(); const result = await provider.getAuthorizeUrl(state); return { url: result.url, state, codeVerifier: result.codeVerifier };
• The frontend stores state in localStorage and redirects to the provider:localStorage.setItem('oauth_state', data.state) localStorage.setItem('oauth_provider', providerId) // ... window.location.href = data.authUrl
• On callback, the frontend does validate state:const savedState = localStorage.getItem('oauth_state') if (!savedState || savedState !== state) { errorMsg.value = '安全警告：State 校验失败，请求可能被篡改' return }
• But on the server, AuthService.handleOAuthCallback ignores state entirely:if (providerName === 'telegram') { const searchParams = new URLSearchParams(); Object.entries(body).forEach(([key, value]) => { ... }); params = searchParams; } else { if (!body.code) throw new AppError('Missing OAuth code', 400); params = body.code; // state is not used here } const userInfo = await provider.handleCallback(params, body.codeVerifier);

So OAuth state is only enforced in the browser, not at the backend boundary. Given:

• all callbacks are same-origin fetch calls with JSON bodies; and
• you’re using strict CORS and Hono’s secureHeaders / CSP;

this is not immediately exploitable as a classic CSRF attacker from another origin. However, from a defense‑in‑depth point of view:

• backend should also enforce that the callback is tied to a previously issued state, not just trust the SPA;
• otherwise, if the frontend or origin constraints are weakened in future changes, this becomes a more serious gap.

Suggested change:

• When issuing the authorize URL, store state on the server side as well. Options:
  • Put it in a httpOnly cookie bound to the provider (e.g. oauth_state_<provider>).
  • Or store in D1/KV keyed by some session identifier.
• In POST /api/oauth/callback/:provider:
  • Compare body.state (or a header) with the server‑side stored state;
  • Reject if not equal or missing;
  • Invalidate the stored state after one successful use (one‑time token).

This keeps the current SPA behavior, but closes the loop at the backend boundary.

2. Whitelist semantics: OAUTH_ALLOWED_USERS === JWT_SECRET disables whitelist

File:

• backend/src/features/auth/authService.ts

Current logic:

const allowedUsersStr = this.env.OAUTH_ALLOWED_USERS || '';
const allowedIdentities = allowedUsersStr.split(',')
    .map(e => e.trim().toLowerCase())
    .filter(Boolean);

const userEmail = (userInfo.email || '').toLowerCase();
const userName = (userInfo.username || '').toLowerCase();

// only enforce whitelist when configured AND OAUTH_ALLOWED_USERS != JWT_SECRET
if (allowedIdentities.length > 0 && this.env.OAUTH_ALLOWED_USERS !== this.env.JWT_SECRET) {
    let isAllowed = false;

    if (whitelistFields.includes('email') && userEmail && allowedIdentities.includes(userEmail)) {
        isAllowed = true;
    }
    if (whitelistFields.includes('username') && userName && allowedIdentities.includes(userName)) {
        isAllowed = true;
    }

    if (!isAllowed) {
        throw new AppError(`Unauthorized user. Email: ${userEmail}, Username: ${userName}`, 403);
    }
}

This means:

• if OAUTH_ALLOWED_USERS is non‑empty and not equal to JWT_SECRET, whitelist is enforced;
• if OAUTH_ALLOWED_USERS is accidentally set equal to JWT_SECRET, whitelist silently turns off.

This is clever for dev/“allow all” setups, but it’s also very easy to misconfigure in production:

• a copy‑paste or env‑templating mistake that sets OAUTH_ALLOWED_USERS to the same value as JWT_SECRET will disable all whitelist checks with no warning.

Suggested change:

• Treat whitelist configuration as explicit:
  • If OAUTH_ALLOWED_USERS is non‑empty, always enforce it.
  • If you need a dev “allow all” mode, introduce a clear flag (e.g. OAUTH_ALLOW_ALL=true) and document that it must never be used in production.
• Optionally log a warning (or even refuse to start) if OAUTH_ALLOWED_USERS === JWT_SECRET, since that almost certainly indicates a mistaken configuration.

3. Encryption key fallback to JWT_SECRET (DB secrets)

Files:

• backend/src/shared/utils/crypto.ts
• backend/src/shared/db/db.ts
• backend/src/features/vault/vaultService.ts

VaultService currently derives its encryption key as:

constructor(env: EnvBindings, repository: VaultRepository) {
    this.env = env;
    this.repository = repository;
    this.encryptionKey = env.ENCRYPTION_KEY || env.JWT_SECRET;
}

encryptField / decryptField then use this key with AES‑GCM:

export async function encryptField(data: any, key: string) {
    const encrypted = await encryptData(data, key);
    return JSON.stringify(encrypted);
}

Where “fast mode” encryptData does:

async function getFastKey(secret: string): Promise<CryptoKey> {
    const keyBuffer = await crypto.subtle.digest('SHA-256', encoder.encode(secret));
    return crypto.subtle.importKey('raw', keyBuffer, { name: 'AES-GCM' }, false, ['encrypt', 'decrypt']);
}

So:

• if ENCRYPTION_KEY is missing, all TOTP secrets in D1 are protected with a key derived from JWT_SECRET;
• a compromise of JWT_SECRET can then be used both to:
  • forge JWTs; and
  • decrypt all stored TOTP secrets.

For a TOTP vault, this is a fairly severe coupling between two security domains.

Suggested change:

• In production mode, require a separate, high‑entropy ENCRYPTION_KEY:
  • If ENCRYPTION_KEY is missing, refuse to start with a clear error.
  • Keep JWT_SECRET and ENCRYPTION_KEY logically independent.
• Optionally:
  • For new deployments, always use the “salt + PBKDF2 + AES‑GCM” style used for backup files, and reserve the “fast/legacy” path only for migration.

Even if the current fast mode is acceptable for D1 (since you still need Cloudflare secrets to read it), forcing a dedicated ENCRYPTION_KEY makes the threat model much cleaner.

4. Frontend security: notes (no obvious XSS, but high impact)

Files (sampling):

• frontend/src/app/router.js
• frontend/src/features/auth/views/login.vue
• frontend/src/features/auth/views/oauthCallback.vue
• frontend/src/features/vault/views/home.vue
• frontend/src/features/vault/components/vaultList.vue
• frontend/src/features/vault/components/addVaultManual.vue
• frontend/src/shared/utils/request.js
• frontend/src/shared/utils/crypto.js
• frontend/src/shared/utils/totp.js
• frontend/src/features/auth/store/userStore.js
• frontend/src/features/vault/store/vaultStore.js

Observations:

• Most user‑controlled data is rendered via normal {{ }} binding, not v-html.
• The only v-html I found is on the login page for provider icons:<span v-html="provider.icon" style="display: flex;"></span> But provider.icon comes from /api/oauth/providers, and those icons are hardcoded SVG strings in the backend provider classes. As long as these remain static and not env/user‑driven, this is safe.
• All API calls go through a request() helper that:
  • adds X-CSRF-Token from csrf_token cookie;
  • uses credentials: 'include';
  • handles 401 by clearing user info and redirecting to /login.
• The vault’s offline cache in localStorage is encrypted client‑side with PBKDF2+AES‑GCM keyed by the deviceKey persisted in IndexedDB. This is a nice extra layer.

Given the nature of this app (holding all TOTP seeds), any future introduction of XSS (e.g. via new v-html, unsafe HTML rendering, or third‑party script) would be catastrophic. I didn’t see a concrete XSS vector in the current code, but it’s worth documenting a strong “no dynamic HTML / no untrusted script” policy explicitly in the README.

Summary

From a security‑review perspective, the main things I’d recommend addressing are:

1. Add server‑side validation of OAuth state (in addition to the existing frontend check).
2. Remove or at least strongly discourage the “OAUTH_ALLOWED_USERS === JWT_SECRET disables whitelist” pattern; make whitelist semantics explicit.
3. Require a dedicated ENCRYPTION_KEY and avoid silently falling back to JWT_SECRET for vault encryption, at least in production.
4. Document the threat model more explicitly (Cloudflare account & Worker env are a single point of failure; any XSS = full vault compromise), and keep the frontend strict with regard to v-html and third‑party scripts.

Overall the codebase looks thoughtfully structured (Hono, strict CSP, CSRF middleware, offline vault encryption, Telegram HMAC verification etc.), which is great for this kind of application. The points above are mostly about tightening a few “sharp edges” and making misconfiguration harder.