feat: TOTP/HOTP ジェネレータを追加 (totp-hotp) - #422
Conversation
RFC 6238 (TOTP) と RFC 4226 (HOTP) のワンタイムコードをブラウザ上で 生成・検証するツールを追加。 - `src/utils/totp-hotp.ts`: Base32 デコード/エンコード、HOTP、TOTP、 verifyTotp、buildOtpauthUri を Web Crypto API のみで実装(依存追加ゼロ) - RFC 公式テストベクタ(HOTP Appendix D: 10 ケース、TOTP Appendix B: SHA-1/256/512 × 6 timestamp = 18 ケース)による unit test - `TotpHotpGenerator.tsx`: TOTP(250ms ポーリング・プログレスバー)/ HOTP(カウンタ生成)/ 検証モード、otpauth URI 表示 - E2E 10 ケース(CSP 適用下)、陽性対照(コロン issuer エラー検知)同梱 - シークレット鍵はサーバーへ送信せず、QR 直接描画も省略(外部露出防止) Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
🖼️ Visual Regression Test 結果
|
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
多角レビュー(セキュリティ / フロントエンド / ロジック / アーキテクチャ)別 Claude セッションで独立レビューを実施。RFC ベクタ+陽性対照を含む高水準のテスト規律と、Web Crypto API のみで ✅ 良い点
🔴 修正推奨(high)[H1] E2E のプレースホルダー比較が文字数ミスマッチで実質 no-op - ).not.toHaveText('─────');
+ ).not.toHaveText('──────');
🟡 修正推奨(medium)[M1] function timingSafeEqual(a: string, b: string): boolean {
if (a.length !== b.length) return false;
let diff = 0;
for (let i = 0; i < a.length; i++) diff |= a.charCodeAt(i) ^ b.charCodeAt(i);
return diff === 0;
}[M2] Base32 デコードで invalid length が silent truncation const validRemainders = new Set([0, 2, 4, 5, 7]);
if (!validRemainders.has(s.length % 8)) {
throw new Error(`Invalid Base32 length: ${s.length}`);
}[M3] const secretBytes = useMemo(() => {
try { return base32Decode(secretBase32.trim()); }
catch { return null; }
}, [secretBase32]);🟢 nit / 設計コメント
総合評価APPROVE 推奨([H1] のみ本 PR 内で修正してほしい)。RFC 準拠の正確性とテスト規律は高水準で、設計上の判断(QR 直接描画回避・依存ゼロ)が |
PR #422 多角レビューでの指摘を本 PR で巻き取り。 - [H1] E2E placeholder 比較を regex 化(旧 `'─────'` 5文字 vs 実装 6文字で 即 pass する no-op 状態を解消) - [M1] `verifyTotp` を constant-time 比較化(`timingSafeEqual` 追加、window 全件走査して早期 return も廃止) - [M2] `base32Decode` で RFC 4648 §6 の有効長 (mod 8 ∈ {0,2,4,5,7}) を検証。 陽性対照テスト 3 ケース同梱(1/3/6 文字 → throw) - [M3] `secretBytes` を useMemo でキャッシュし 250ms tick での再デコードを廃止 - [N1] HOTP カウンタの非数値・負値を silent に 0 へ丸めず明示エラー表示 - [N2] 検証モード E2E の `or` fallback を廃止、`toContainText(/有効|無効/)` で 結果テキストを直接 assert(aria-live 要素の存在だけで pass を解消) - [N3] `buildOtpauthUri` で secret を `encodeURIComponent` で defensive に encode Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
レビューありがとうございます。[N4](コンポーネント分割)以外を本 PR で巻き取りました(commit d5e3d96)。 対応内容
動作確認
[N4] のコンポーネント分割は本 PR スコープから外し、別 issue で追従検討します。 |
再レビュー(fix commit
|
| ID | 指摘内容 | 対応 | 評価 |
|---|---|---|---|
| H1 | E2E プレースホルダー比較が 5 文字 vs 実装 6 文字で no-op | regex ^[─\s]+$ 化(tests/e2e/totp-hotp.spec.ts:29,42,98) |
✓ digits 7/8 拡張にも追従できる堅牢な書き方 |
| M1 | verifyTotp の非定数時間比較 |
timingSafeEqual 追加、window 全件走査・早期 return 廃止(src/utils/totp-hotp.ts:111-135) |
✓ コメントで現実的脅威は低いことを明示している点も好印象 |
| M2 | Base32 invalid length silent truncation | VALID_BASE32_REMAINDERS = {0,2,4,5,7} で validate、陽性対照 3 ケース同梱 |
✓ コメントが RFC 4648 §6 を明示参照していて maintainability ◎ |
| M3 | 250ms tick での Base32 再 decode | useMemo<Uint8Array<ArrayBuffer> | null> で secret bytes キャッシュ、effect deps を secretBytes に切替 |
✓ secretError を useState から派生 state に再構成した副次クリーンアップも良い |
| N1 | HOTP counter の silent 0 丸め | counterError で非数値・負値を明示エラー表示、<ErrorMessage /> 連動 |
✓ |
| N2 | 検証 E2E の or fallback で silent regression |
toContainText(/有効|無効/) で結果テキストを直接 assert |
✓ |
| N3 | buildOtpauthUri の secret 未 encode |
encodeURIComponent(opts.secretBase32) で defensive 化 |
✓ |
微小な気づき(blocker ではない、本 PR で対応不要)
- constant-time の厳密性:
verifyTotp内のif (timingSafeEqual(expected, code) && !result.valid)はresult.validの短絡で代入の有無に微小な時間差が出得ます。理論的には全 window 走査の最大時間に揃えるならresult = timingSafeEqual(...) && !result.valid ? {valid:true,offset} : resultで常に三項演算する方が defensive。ただし JS の microtask ノイズの方が支配的なので実害なし crypto.subtle.importKeyの再呼び出し:hotpが呼ばれるたびに毎回 import している。verify では window=1 で 3 回。秘密鍵のサイズが小さく実害は無視できるレベルですが、将来 window 拡張時の perf を考えるとverifyTotp内で 1 度だけ import → 各 counter で sign する形に refactor の余地あり- secret padding の URL encode 影響:
encodeURIComponent('ME======')='ME%3D%3D%3D%3D%3D%3D'。一部の旧 authenticator アプリは%3Dを decode しない実装もあるとの報告あり(実害は限定的)。今回は input 側で padding を strip しないので、利用者が=込みで貼り付けた場合のみ発生。優先度低 - エラー文言: 「長さ 2/4/5/7/8n 文字」の
8nが一瞬「8 と n」に読めるので、「2/4/5/7 文字 または 8 の倍数」表記の方が user にとって明瞭
総合評価
APPROVE ✅ — H1 を含む全指摘が本 PR 内で適切に解決。中盤の useMemo 化で secretError を派生 state に変えたリファクタは元の指摘範囲を超えた好改善でした。merge 後の追従 issue 化も不要と判断します。
|
再レビューありがとうございます。APPROVE いただきました。気づき項目について判断しました。
E2E は文言の部分文字列マッチなのでこの修正は影響しません(10 passed 確認済み)。 merge 準備完了です。 |
概要
TOTP/HOTP ジェネレータを新ツールとして追加します。
主な変更
src/utils/totp-hotp.ts: Base32 デコード・HMAC-OTP 計算・otpauth URI 生成src/components/tools/TotpHotpGenerator.tsx: TOTP(リアルタイム更新)・HOTP・検証の3モード UIsrc/pages/tools/totp-hotp.astro: ルーティングページPAGES追加済み設計上の判断
/tools/qr-codeへ手動コピー誘導Uint8Array<ArrayBuffer>明示: TS 5.9.3 で Web Crypto API が要求する型と合致させるためテスト計画
npm run test— 1228 passed (58 files)node_modules/.bin/astro check— 0 errorsnpm run test:e2e --grep TOTP— 10 passedUpdate Visual Regression Baselineworkflow をworkflow_dispatchで実行(CI Linux runner 必須)🤖 Generated with Claude Code