Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

password reset and magic link #14

Merged
merged 3 commits into from
Dec 3, 2024
Merged

password reset and magic link #14

merged 3 commits into from
Dec 3, 2024

Conversation

ciscorn
Copy link
Member

@ciscorn ciscorn commented Dec 3, 2024

Summary by CodeRabbit

  • 新機能

    • 環境変数の管理を容易にするため、.envrcファイルにdotenvエントリを追加。
    • 開発環境のセットアップ手順をREADME.mdに追加。
    • ユーザー認証機能を強化し、メール確認機能を追加。
    • ログインページに複数の認証方法を提供。
  • バグ修正

    • 認証セッションの管理を改善し、ユーザーのメール確認を促すロジックを追加。
  • スタイル

    • 表示内容を日本語から英語に変更し、誤字を修正。
  • ドキュメント

    • コメントを英語に更新し、プロダクション環境での使用に関する警告を強調。

Copy link

coderabbitai bot commented Dec 3, 2024

Walkthrough

このプルリクエストでは、複数のファイルに対して変更が加えられています。.envrcファイルにdotenvエントリが追加され、環境変数の管理が改善されました。また、.gitignoreファイルにpackage-lock.jsonが追加され、Gitによる追跡を防ぎます。README.mdには開発環境のセットアップ手順が追加され、ユーザー認証機能が強化され、メール確認機能が実装されました。リダイレクトのHTTPステータスコードが303から307に変更され、ユーザーインターフェースも改善されています。

Changes

ファイルパス 変更概要
.envrc 新しいエントリdotenvが追加され、環境変数の管理が改善されました。
.gitignore package-lock.jsonを追加し、npm関連のファイルを無視するように設定されました。
README.md "Development"セクションが追加され、開発環境のセットアップ手順が記述されました。
src/api/index.ts /api/postsエンドポイントのauthorフィールドのロジックが更新され、currentUser.namenullまたはundefinedの場合に'Unknown'が設定されるようになりました。
src/lib/firebase-auth/client.ts refreshSession関数が削除され、認証機能とセッション管理のロジックが変更されました。
src/routes/+layout.svelte メール確認機能を追加し、ユーザー情報とログアウトボタンの表示ロジックが更新されました。
src/routes/+page.svelte SvelteKitのドキュメントへのリンクが削除され、"Home"という新しい段落が追加されました。
src/routes/login/+page.svelte ログイン機能が強化され、複数の認証方法が追加されました。
src/routes/private/+layout.server.ts load関数が更新され、dependsパラメータが追加され、リダイレクトのHTTPステータスコードが307に変更されました。
src/routes/shop/+layout.server.ts load関数が更新され、メール確認のための新しい条件分岐が追加されました。
src/routes/shop/+page.svelte 日本語から英語へのローカライズ調整が行われ、誤字が修正されました。
src/routes/shop/billing-portal/+server.ts GET関数のリダイレクトステータスコードが303から307に変更されました。
src/routes/shop/payment/+page.server.ts load関数のリダイレクトステータスコードが307に変更され、コメントが英語に更新されました。
src/routes/shop/subscription/[price_id]/+page.server.ts load関数のリダイレクトステータスコードが307に変更され、コメントが英語に更新されました。
src/routes/verify_email/+page.svelte メール確認コンポーネントのユーザーフィードバックが強化され、条件付きレンダリングが追加されました。

Possibly related PRs

  • demo: signinWithEmailAndPassword #12: src/api/auth.tsの変更は、エラーハンドリングと環境変数管理の強化を含み、.envrcファイルの新しいdotenvエントリと関連しています。

Poem

うさぎの耳で聞いたお話、
環境変数がすぐに使える、
ログインも簡単、メール確認、
みんなで楽しく開発しよう!
どら焼きも忘れずに、
さあ、跳ねて進もう! 🐰✨


🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

cloudflare-workers-and-pages bot commented Dec 3, 2024

Deploying sveltekit-firebaseauth-ssr-stripe-demo with  Cloudflare Pages  Cloudflare Pages

Latest commit: 0286358
Status: ✅  Deploy successful!
Preview URL: https://c01c63f8.sveltekit-firebaseauth-ssr-stripe-demo.pages.dev
Branch Preview URL: https://demo-resetmail-magiclink.sveltekit-firebaseauth-ssr-stripe-demo.pages.dev

View logs

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 9

🧹 Outside diff range and nitpick comments (13)
src/api/auth.ts (1)

44-44: エラーメッセージの環境変数名が不一致

エラーメッセージで 'FIREBASE_SERVICE_ACCOUNT_KEY is not set' と表示されていますが、実際のコードでは 'GOOGLE_SERVICE_ACCOUNT_KEY' を使用しています。変数名の不一致は開発者の混乱を招く可能性がありますので、エラーメッセージを修正してください。

以下の差分を適用してエラーメッセージを修正してください:

- console.error('FIREBASE_SERVICE_ACCOUNT_KEY is not set. Authentication will not work.');
+ console.error('GOOGLE_SERVICE_ACCOUNT_KEY is not set. Authentication will not work.');
src/routes/login/+page.svelte (1)

36-36: 未使用の変数 'result' の削除

signInWithEmailLink の成功時のコールバックで result を受け取っていますが、使用していません。未使用の変数はコードの可読性を低下させ、誤解を招く可能性がありますので、削除を検討してください。

以下の差分を適用して未使用の変数を削除してください:

- signInWithEmailLink(auth, email, window.location.href).then((result) => {
+ signInWithEmailLink(auth, email, window.location.href).then(() => {
🧰 Tools
🪛 eslint

[error] 36-36: 'result' is defined but never used.

(@typescript-eslint/no-unused-vars)

src/routes/+layout.server.ts (1)

8-9: 型の安全性を確保することを推奨します

currentIdTokenから取得するemailemail_verifiedプロパティの型安全性を確保するため、型アサーションまたは型ガードの追加を検討してください。

以下のような実装を提案します:

currentUser: locals.currentIdToken && {
    uid: locals.currentIdToken.uid,
-   email: locals.currentIdToken.email,
-   email_verified: locals.currentIdToken.email_verified
+   email: locals.currentIdToken.email ?? null,
+   email_verified: Boolean(locals.currentIdToken.email_verified)
}
src/api/index.ts (1)

15-15: 定数の抽出とより堅牢な型の使用を推奨

'Unknown'のようなフォールバック値は定数として抽出し、currentUser.nameの型安全性を向上させることを推奨します。

以下のような実装を提案します:

+ const DEFAULT_AUTHOR = 'Unknown' as const;
+ 
const app = new Hono<{ Bindings: Env; Variables: AuthVariables }>()
    .use(authMiddleware)
    .get('/api/posts', async (c) => {
        const currentUser = ensureUser(c);
        const posts = Array.from({ length: 20 }, () => ({
            title: 'Great Article',
-           author: currentUser.name ?? 'Unknown'
+           author: currentUser.name ?? DEFAULT_AUTHOR
        }));
        return c.json(posts);
    });
src/routes/shop/billing-portal/+server.ts (1)

Line range hint 11-18: 顧客IDの安全でない保存方法

cookieに顧客IDを直接保存することは、セキュリティリスクが高いです。以下の対策を推奨します:

  1. 暗号化されたセッションストレージの使用
  2. データベースでの顧客IDの管理
  3. cookieを使用する場合は、暗号化と適切なセキュリティフラグの設定

セキュアな実装について相談させていただけますか?

src/routes/shop/payment/+page.server.ts (1)

Line range hint 21-33: Stripeセッション作成のエラーハンドリング

Stripeセッション作成時のエラーハンドリングが必要です。また、セッションのバリデーションも推奨します。

+ try {
   const session = await stripe.checkout.sessions.create({
     ui_mode: 'embedded',
     customer: customerId,
     line_items: [
       {
         price: process.env.DORAYAKI_PRICE_ID,
         quantity: 1
       }
     ],
     mode: 'payment',
     return_url: url.origin + '/shop/thanks?session_id={CHECKOUT_SESSION_ID}'
   });
+   
+   if (!session?.client_secret) {
+     throw new Error('セッションの作成に失敗しました');
+   }
+ } catch (error) {
+   console.error('Stripe session error:', error);
+   throw error;
+ }
src/routes/shop/subscription/[price_id]/+page.server.ts (2)

Line range hint 24-36: サブスクリプション作成時の追加の検証

サブスクリプション作成時に、より詳細な検証とエラーハンドリングが必要です。

+ try {
   const session = await stripe.checkout.sessions.create({
     ui_mode: 'embedded',
     customer: customerId,
     line_items: [
       {
         price: priceId,
         quantity: 1
       }
     ],
     mode: 'subscription',
     return_url: url.origin + '/shop/thanks?session_id={CHECKOUT_SESSION_ID}'
   });

+   if (!session?.client_secret) {
+     throw new Error('サブスクリプションセッションの作成に失敗しました');
+   }
+ } catch (error) {
+   console.error('Stripe subscription error:', error);
+   throw error;
+ }

Line range hint 1-41: 全体的なセキュリティ強化の提案

現在の実装には複数のセキュリティ上の懸念があります。以下の改善を提案します:

  1. セッション管理の改善
  2. 適切な認証フロー
  3. 環境変数による設定管理
  4. エラーログの適切な処理

アーキテクチャの改善について詳細な提案をさせていただけますか?

README.md (1)

35-40: 開発セクションの説明を充実させることを推奨します

開発環境のセットアップ手順が追加されましたが、以下の点について説明を追加することを推奨します:

  • 各コマンドの目的と効果
  • 必要な前提条件(Node.js、pnpm、direnvのインストールなど)
  • 環境変数の設定方法
 ## Development
 
+### Prerequisites
+
+- Node.js v18以上
+- pnpm
+- direnv
+
+### Setup
+
+1. 環境変数を設定します:
+   ```
+   cp .env.example .env
+   # .envファイルを編集して必要な値を設定
+   ```
+
+2. 開発サーバーを起動します:
 ```bash
 direnv allow
 pnpm dev-in-emulator

</blockquote></details>
<details>
<summary>src/routes/+layout.svelte (2)</summary><blockquote>

`16-22`: **メール検証機能のセキュリティと UX の改善を推奨**

メール検証機能の実装について、以下の改善を推奨します:
- URLの構築にorigin以外のパスも含める可能性を考慮
- 検証メール送信後のユーザーフィードバックをより詳細に


```diff
 async function _sendEmailVerification() {
   const auth = getAuth();
   if (auth.currentUser) {
-    await sendEmailVerification(auth.currentUser, { url: $page.url.origin + '/verify_email' });
-    alert('Verification email sent!');
+    const verificationUrl = new URL('/verify_email', $page.url.origin);
+    await sendEmailVerification(auth.currentUser, { url: verificationUrl.toString() });
+    alert('確認メールを送信しました。メールボックスをご確認ください。\nメールが届かない場合は、迷惑メールフォルダもご確認ください。');
   }
 }

31-35: 未確認メール通知のUXを改善

警告メッセージのスタイリングとアクセシビリティを改善することを推奨します:

-<p style="color: white; background-color: brown; padding: 0.3em;">
-  Your email isn't verified yet. Check your inbox for the verification link. <button
-    onclick={() => _sendEmailVerification()}>Resend verification email.</button
-  >
-</p>
+<div role="alert" class="email-verification-alert">
+  <p>メールアドレスの確認が完了していません。確認メールに記載されたリンクをクリックしてください。</p>
+  <button
+    type="button"
+    class="resend-button"
+    onclick={() => _sendEmailVerification()}
+  >
+    確認メールを再送信
+  </button>
+</div>
+
+<style>
+  .email-verification-alert {
+    background-color: #fef2f2;
+    border: 1px solid #fee2e2;
+    border-radius: 0.375rem;
+    padding: 1rem;
+    margin: 1rem 0;
+  }
+  .resend-button {
+    margin-top: 0.5rem;
+    padding: 0.5rem 1rem;
+    background-color: #dc2626;
+    color: white;
+    border-radius: 0.25rem;
+    border: none;
+  }
+</style>
src/lib/firebase-auth/server.ts (2)

Line range hint 44-50: エラーハンドリングの改善が推奨されます

本番環境での適切なエラー追跡のため、以下の改善を検討してください:

  • console.errorの代わりに構造化されたログ出力の使用
  • エラーメトリクスの収集
  • より詳細なエラーコンテキストの提供
if (!serviceAccountCredential) {
  if (emulatorEnv.FIREBASE_AUTH_EMULATOR_HOST) {
    serviceAccountCredential ||= new NopCredential();
  } else {
-   console.error('service account credential is not set. Authentication will not work.');
+   const errorContext = {
+     error: 'MISSING_CREDENTIALS',
+     severity: 'CRITICAL',
+     component: 'Authentication',
+     timestamp: new Date().toISOString()
+   };
+   console.error('認証情報が設定されていません', errorContext);
+   // TODO: エラー監視サービスへの通知を追加
  }
}

セキュリティ強化のための実装が必要です

コードベースの分析結果から、以下の重要なセキュリティ機能が不足していることが確認されました:

  • レート制限機能が実装されていません
  • セキュリティヘッダー(例:X-Frame-OptionsContent-Security-Policy)が不足しています
  • アクセスログ機能が見当たりません

推奨される対策:

  • /api/auth エンドポイントにレート制限ミドルウェアを追加
  • セキュリティヘッダーを Response オブジェクトに追加
  • セッショントークンの再利用検知メカニズムの実装
  • アクセスログ機能の追加(特に認証失敗時)
🔗 Analysis chain

Line range hint 119-147: セッション管理のセキュリティ強化を推奨

セッションエンドポイントのセキュリティを強化するため、以下の対策を検討してください:

  1. レート制限の実装
  2. セッショントークンの再利用検知
  3. アクセスログの強化

Let me search for more security-related implementations before making the final response.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# レート制限の実装状況を確認
rg -A 5 "rate.*limit|throttle" 

# セキュリティヘッダーの実装状況を確認
ast-grep --pattern 'new Response($_, { headers: { $$ } })'

Length of output: 990


Script:

#!/bin/bash
# Check for security headers and middleware implementations
rg -A 5 "headers.*security|security.*headers"

# Check for session token validation
rg -A 5 "validateSessionCookie|verifySessionCookie"

# Check for any existing rate limiting middleware or implementations
fd middleware -t f -x rg -l "limit|throttle" {}

Length of output: 860

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between f3d8f04 and 80376d0.

⛔ Files ignored due to path filters (1)
  • package.json is excluded by !**/*.json
📒 Files selected for processing (19)
  • .envrc (1 hunks)
  • .gitignore (1 hunks)
  • README.md (1 hunks)
  • src/api/auth.ts (2 hunks)
  • src/api/index.ts (1 hunks)
  • src/lib/firebase-auth/client.ts (5 hunks)
  • src/lib/firebase-auth/server.ts (1 hunks)
  • src/lib/user.ts (1 hunks)
  • src/routes/+layout.server.ts (1 hunks)
  • src/routes/+layout.svelte (2 hunks)
  • src/routes/+page.svelte (1 hunks)
  • src/routes/login/+page.svelte (2 hunks)
  • src/routes/private/+layout.server.ts (1 hunks)
  • src/routes/shop/+layout.server.ts (1 hunks)
  • src/routes/shop/+page.svelte (1 hunks)
  • src/routes/shop/billing-portal/+server.ts (2 hunks)
  • src/routes/shop/payment/+page.server.ts (2 hunks)
  • src/routes/shop/subscription/[price_id]/+page.server.ts (1 hunks)
  • src/routes/verify_email/+page.svelte (1 hunks)
✅ Files skipped from review due to trivial changes (4)
  • .envrc
  • .gitignore
  • src/routes/+page.svelte
  • src/routes/shop/+page.svelte
🧰 Additional context used
🪛 eslint
src/routes/login/+page.svelte

[error] 36-36: 'result' is defined but never used.

(@typescript-eslint/no-unused-vars)

🔇 Additional comments (5)
src/lib/user.ts (1)

8-8: email_verified プロパティの追加は適切です

BasicPrivateUserInfo 型に email_verified?: boolean; を追加することで、ユーザーのメール確認ステータスを適切に管理できます。

src/routes/private/+layout.server.ts (1)

6-6: リダイレクトのステータスコード変更の確認

redirect(303, ...) から redirect(307, ...) に変更しています。307 ステータスコードはリダイレクト時に HTTP メソッドを維持します。これは意図した変更でしょうか?期待する動作を満たすことを確認してください。

src/routes/shop/+layout.server.ts (1)

5-7: メール確認のリダイレクト処理とステータスコード変更の確認

未確認のユーザーを /verify_email にリダイレクトする処理を追加しています。これはユーザーにメール確認を促すために適切な対応です。

また、リダイレクトのステータスコードを 303 から 307 に変更しています。307 ステータスコードはリダイレクト時に HTTP メソッドを維持します。これは意図した変更でしょうか?期待する動作を確認してください。

src/lib/firebase-auth/client.ts (1)

34-36: トークン更新戦略の確認

getIdToken(true)を使用してトークンを強制的に更新していますが、以下の点について確認することを推奨します:

  • トークン更新の頻度とパフォーマンスへの影響
  • バックグラウンドでの自動更新の必要性
✅ Verification successful

Let me analyze the code further to understand the token update strategy better.


トークン更新の実装は適切です

コードベースの分析から、以下の点が確認できました:

  • トークンの更新は必要な時のみ実行されています:
    • ユーザーのサインイン時
    • 認証状態の変更時
    • リダイレクト認証完了時
  • previousIdTokenによる重複更新の防止も実装されています
  • Firebase Auth SDKが提供する標準的なトークン管理メカニズムを利用しています

強制的なトークン更新(getIdToken(true))の使用は、認証状態の変更を確実に反映させるために適切な選択です。バックグラウンドでの自動更新は不要で、現在の実装で十分です。

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# トークン更新に関連するコードを検索
rg -A 5 "getIdToken|updateSession" --type ts

Length of output: 2254


Script:

#!/bin/bash
# Check for any token refresh interval or timer related code
rg -A 5 "forceRefresh|tokenRefresh|setInterval|setTimeout" --type ts

# Look for any Firebase Auth configuration
rg -A 5 "initializeAuth|firebase\.initializeApp|getAuth\(" --type ts

Length of output: 3834

src/lib/firebase-auth/server.ts (1)

16-17: エクスポート文の順序変更は適切です

型定義とクラスのエクスポート順序が整理され、コードの構造が改善されています。

src/api/auth.ts Outdated
// ignore
console.log('error', error);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

エラー情報のログ出力に関するセキュリティ考慮

console.log('error', error); でエラーオブジェクトをそのままログ出力していますが、これにより機密情報が漏洩する可能性があります。特に本番環境では、エラーメッセージにユーザー情報やシステムの詳細が含まれる場合があります。エラーメッセージを一般化するか、適切なログレベルを使用して詳細情報の露出を防いでください。

以下の差分を適用してログ出力を修正してください:

- console.log('error', error);
+ console.error('Authentication error occurred.');
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
console.log('error', error);
console.error('Authentication error occurred.');

Your email must be verified to access certain pages. Please check your inbox for the
verification link.
</p>
<pre>{JSON.stringify(data.currentIdToken, null, 2)}</pre>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

本番環境での機密情報の露出に注意

currentIdTokenの内容をそのまま画面に表示することは、セキュリティ上のリスクとなる可能性があります。本番環境では、デバッグ情報の表示を制限することを推奨します。

開発環境でのみデバッグ情報を表示するように修正することを提案します:

- <pre>{JSON.stringify(data.currentIdToken, null, 2)}</pre>
+ {#if import.meta.env.DEV}
+   <pre>{JSON.stringify(data.currentIdToken, null, 2)}</pre>
+ {/if}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<pre>{JSON.stringify(data.currentIdToken, null, 2)}</pre>
{#if import.meta.env.DEV}
<pre>{JSON.stringify(data.currentIdToken, null, 2)}</pre>
{/if}

Comment on lines 5 to 16
{#if !data.currentIdToken?.email_verified}
<h1>Email Verification Required</h1>
<p>
Your email must be verified to access certain pages. Please check your inbox for the
verification link.
</p>
<pre>{JSON.stringify(data.currentIdToken, null, 2)}</pre>
{:else}
<h1>Email Verification Succeeded!</h1>
<p>Thank you for verifying your email.</p>
<pre>{JSON.stringify(data.currentIdToken, null, 2)}</pre>
{/if}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

ユーザー体験の改善を提案

現在の実装では、以下の点で改善の余地があります:

  1. ローディング状態の処理がありません
  2. メール再送信機能がありません
  3. エラー状態の処理がありません

以下のような機能の追加を提案します:

<script lang="ts">
    let { data } = $props();
+   let isResending = false;
+   
+   async function resendVerificationEmail() {
+       isResending = true;
+       try {
+           // メール再送信のロジックを実装
+       } catch (error) {
+           // エラー処理
+       } finally {
+           isResending = false;
+       }
+   }
</script>

+ {#if !data.currentIdToken}
+   <p>読み込み中...</p>
+ {:else if !data.currentIdToken.email_verified}
    <h1>メール認証が必要です</h1>
    <p>
        特定のページにアクセスするには、メールアドレスの認証が必要です。
        受信トレイを確認して、認証リンクをクリックしてください。
    </p>
+   <button
+       on:click={resendVerificationEmail}
+       disabled={isResending}
+   >
+       {isResending ? '送信中...' : '認証メールを再送信'}
+   </button>
- {:else}
+   {#if import.meta.env.DEV}
        <pre>{JSON.stringify(data.currentIdToken, null, 2)}</pre>
+   {/if}
+ {:else}
    <h1>メール認証が完了しました!</h1>
    <p>メールアドレスの認証が完了しました。ご協力ありがとうございます。</p>
+ {/if}

Committable suggestion skipped: line range outside the PR's diff.

@@ -3,7 +3,7 @@ import { stripe } from '$lib/stripe/stripe';

export async function GET({ url, cookies, locals }) {
if (!locals.currentIdToken) {
redirect(303, '/');
redirect(307, '/');
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

リダイレクトステータスコードの変更に関する懸念

307へのステータスコード変更は、POSTリクエストのボディを保持したままリダイレクトを行うため、セキュリティ上のリスクが生じる可能性があります。303の使用を推奨します。

以下の修正を提案します:

- redirect(307, '/');
+ redirect(303, '/');

- throw redirect(307, '/');
+ throw redirect(303, '/');

- throw redirect(307, billingPortalSession.url);
+ throw redirect(303, billingPortalSession.url);

Also applies to: 22-22, 30-30

Comment on lines 24 to 29

const billingPortalSession = await stripe.billingPortal.sessions.create({
// configuration: bpc.id, // Billing Portal をカスタムする場合に使用
customer: customerId,
return_url: url.origin + '/'
});
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Stripeエラーハンドリングの追加が必要

Stripe APIコールのエラーハンドリングが実装されていません。以下の実装を推奨します:

+ try {
   const billingPortalSession = await stripe.billingPortal.sessions.create({
     customer: customerId,
     return_url: url.origin + '/'
   });
+ } catch (error) {
+   console.error('Stripe billing portal error:', error);
+   throw error;
+ }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const billingPortalSession = await stripe.billingPortal.sessions.create({
// configuration: bpc.id, // Billing Portal をカスタムする場合に使用
customer: customerId,
return_url: url.origin + '/'
});
try {
const billingPortalSession = await stripe.billingPortal.sessions.create({
// configuration: bpc.id, // Billing Portal をカスタムする場合に使用
customer: customerId,
return_url: url.origin + '/'
});
} catch (error) {
console.error('Stripe billing portal error:', error);
throw error;
}

@@ -23,7 +23,7 @@ export async function load({ url, cookies, locals }) {
customer: customerId,
line_items: [
{
price: 'price_1PkKYCLtNIgQdVMEIO2U71nd', // どら焼き
price: 'price_1PkKYCLtNIgQdVMEIO2U71nd', // Dorayaki
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

価格IDのハードコーディングを避ける

価格IDをコードに直接記述することは、メンテナンス性とセキュリティの観点から問題があります。

環境変数での管理を推奨します:

- price: 'price_1PkKYCLtNIgQdVMEIO2U71nd', // Dorayaki
+ price: process.env.DORAYAKI_PRICE_ID,

env.d.tsに型定義も追加してください:

declare namespace NodeJS {
  interface ProcessEnv {
    DORAYAKI_PRICE_ID: string;
  }
}

Comment on lines 13 to +15

// マネしちゃだめ!
// 永続化層がないので、 Cookieを使ってStripeの顧客IDを保持しておく
// DO NOT DO THIS IN PRODUCTION!!!
// DO NOT DO THIS IN PRODUCTION!!!
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

価格IDの検証が必要

URLパラメータから直接価格IDを使用する前に、適切な検証が必要です。

+ const allowedPriceIds = ['price_1', 'price_2']; // 有効な価格IDのリスト
  const priceId = params.price_id;
+ if (!allowedPriceIds.includes(priceId)) {
+   throw new Error('無効な価格IDです');
+ }

Committable suggestion skipped: line range outside the PR's diff.

Comment on lines 63 to 68
export async function signInWithEmailAndPassword(email: string, password: string) {
const auth = getAuth();
const cred = await _signInWithEmailAndPassword(auth, email, password);
await updateSession(await cred.user.getIdToken());
return cred;
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

エラーハンドリングの追加を推奨

メールパスワード認証にエラーハンドリングを追加することを推奨します:

 export async function signInWithEmailAndPassword(email: string, password: string) {
   const auth = getAuth();
-  const cred = await _signInWithEmailAndPassword(auth, email, password);
-  await updateSession(await cred.user.getIdToken());
-  return cred;
+  try {
+    const cred = await _signInWithEmailAndPassword(auth, email, password);
+    await updateSession(await cred.user.getIdToken());
+    return cred;
+  } catch (error: any) {
+    // Firebase Auth エラーコードに基づいてユーザーフレンドリーなメッセージを返す
+    switch (error.code) {
+      case 'auth/invalid-email':
+        throw new Error('メールアドレスの形式が正しくありません。');
+      case 'auth/user-disabled':
+        throw new Error('このアカウントは無効化されています。');
+      case 'auth/user-not-found':
+      case 'auth/wrong-password':
+        throw new Error('メールアドレスまたはパスワードが正しくありません。');
+      default:
+        throw new Error('ログインに失敗しました。しばらく経ってから再度お試しください。');
+    }
+  }
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export async function signInWithEmailAndPassword(email: string, password: string) {
const auth = getAuth();
const cred = await _signInWithEmailAndPassword(auth, email, password);
await updateSession(await cred.user.getIdToken());
return cred;
}
export async function signInWithEmailAndPassword(email: string, password: string) {
const auth = getAuth();
try {
const cred = await _signInWithEmailAndPassword(auth, email, password);
await updateSession(await cred.user.getIdToken());
return cred;
} catch (error: any) {
// Firebase Auth エラーコードに基づいてユーザーフレンドリーなメッセージを返す
switch (error.code) {
case 'auth/invalid-email':
throw new Error('メールアドレスの形式が正しくありません。');
case 'auth/user-disabled':
throw new Error('このアカウントは無効化されています。');
case 'auth/user-not-found':
case 'auth/wrong-password':
throw new Error('メールアドレスまたはパスワードが正しくありません。');
default:
throw new Error('ログインに失敗しました。しばらく経ってから再度お試しください。');
}
}
}

Comment on lines 70 to 75
export async function createUserWithEmailAndPassword(email: string, password: string) {
const auth = getAuth();
const cred = await _createUserWithEmailAndPassword(auth, email, password);
await updateSession(await cred.user.getIdToken());
return cred;
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

アカウント作成時のバリデーションとエラーハンドリングの追加を推奨

アカウント作成機能にバリデーションとエラーハンドリングを追加することを推奨します:

 export async function createUserWithEmailAndPassword(email: string, password: string) {
   const auth = getAuth();
-  const cred = await _createUserWithEmailAndPassword(auth, email, password);
-  await updateSession(await cred.user.getIdToken());
-  return cred;
+  // パスワードの強度チェック
+  if (password.length < 8) {
+    throw new Error('パスワードは8文字以上である必要があります。');
+  }
+  if (!/[A-Z]/.test(password)) {
+    throw new Error('パスワードは少なくとも1つの大文字を含む必要があります。');
+  }
+  if (!/[0-9]/.test(password)) {
+    throw new Error('パスワードは少なくとも1つの数字を含む必要があります。');
+  }
+
+  try {
+    const cred = await _createUserWithEmailAndPassword(auth, email, password);
+    await updateSession(await cred.user.getIdToken());
+    return cred;
+  } catch (error: any) {
+    switch (error.code) {
+      case 'auth/email-already-in-use':
+        throw new Error('このメールアドレスは既に使用されています。');
+      case 'auth/invalid-email':
+        throw new Error('メールアドレスの形式が正しくありません。');
+      case 'auth/operation-not-allowed':
+        throw new Error('メール/パスワードでの認証が有効になっていません。');
+      case 'auth/weak-password':
+        throw new Error('パスワードが脆弱です。より強力なパスワードを設定してください。');
+      default:
+        throw new Error('アカウントの作成に失敗しました。しばらく経ってから再度お試しください。');
+    }
+  }
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export async function createUserWithEmailAndPassword(email: string, password: string) {
const auth = getAuth();
const cred = await _createUserWithEmailAndPassword(auth, email, password);
await updateSession(await cred.user.getIdToken());
return cred;
}
export async function createUserWithEmailAndPassword(email: string, password: string) {
const auth = getAuth();
// パスワードの強度チェック
if (password.length < 8) {
throw new Error('パスワードは8文字以上である必要があります。');
}
if (!/[A-Z]/.test(password)) {
throw new Error('パスワードは少なくとも1つの大文字を含む必要があります。');
}
if (!/[0-9]/.test(password)) {
throw new Error('パスワードは少なくとも1つの数字を含む必要があります。');
}
try {
const cred = await _createUserWithEmailAndPassword(auth, email, password);
await updateSession(await cred.user.getIdToken());
return cred;
} catch (error: any) {
switch (error.code) {
case 'auth/email-already-in-use':
throw new Error('このメールアドレスは既に使用されています。');
case 'auth/invalid-email':
throw new Error('メールアドレスの形式が正しくありません。');
case 'auth/operation-not-allowed':
throw new Error('メール/パスワードでの認証が有効になっていません。');
case 'auth/weak-password':
throw new Error('パスワードが脆弱です。より強力なパスワードを設定してください。');
default:
throw new Error('アカウントの作成に失敗しました。しばらく経ってから再度お試しください。');
}
}
}

@ciscorn
Copy link
Member Author

ciscorn commented Dec 3, 2024

@codecov-ai-reviewer review

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Outside diff range and nitpick comments (3)
src/lib/firebase-auth/client.ts (3)

111-111: セッション無効化の最適化について

invalidate('auth:session')の呼び出しが常に行われるようになりましたが、これは不要な再描画を引き起こす可能性があります。以下のような条件付き無効化の実装を検討してください:

- invalidate('auth:session');
+ if (idToken !== previousIdToken) {
+   invalidate('auth:session');
+ }

Line range hint 74-83: プロバイダー認証のエラーハンドリング追加推奨

signInWithProvider関数にエラーハンドリングが実装されていません。以下のような実装を推奨します:

 export async function signInWithProvider(provider: AuthProvider, withRedirect = true) {
   const auth = getAuth();
   const app = getApp();
-  if (withRedirect && location.host === app.options.authDomain) {
-    signInWithRedirect(auth, provider);
-  } else {
-    const cred = await signInWithPopup(auth, provider);
-    await updateSession(await cred.user.getIdToken());
+  try {
+    if (withRedirect && location.host === app.options.authDomain) {
+      await signInWithRedirect(auth, provider);
+    } else {
+      const cred = await signInWithPopup(auth, provider);
+      await updateSession(await cred.user.getIdToken());
+    }
+  } catch (error: any) {
+    switch (error.code) {
+      case 'auth/popup-blocked':
+        throw new Error('ポップアップがブロックされました。ブラウザの設定を確認してください。');
+      case 'auth/popup-closed-by-user':
+        throw new Error('認証がキャンセルされました。');
+      case 'auth/cancelled-popup-request':
+        throw new Error('認証リクエストがキャンセルされました。');
+      case 'auth/account-exists-with-different-credential':
+        throw new Error('このメールアドレスは既に別の認証方法で登録されています。');
+      default:
+        throw new Error('認証に失敗しました。しばらく経ってから再度お試しください。');
+    }
   }
 }

Line range hint 117-124: セキュリティ強化の推奨事項

リダイレクト結果の処理において、以下のセキュリティ強化を推奨します:

  1. トークンの有効期限の検証
  2. エラー発生時のセッションクリア
  3. リダイレクト結果のタイムアウト処理
 function resetRedirectResultHandler() {
   redirectResultPromise = getRedirectResult(getAuth());
-  redirectResultPromise.then(async (result) => {
+  const timeoutPromise = new Promise((_, reject) => {
+    setTimeout(() => reject(new Error('認証タイムアウト')), 30000);
+  });
+
+  Promise.race([redirectResultPromise, timeoutPromise]).then(async (result) => {
     if (result) {
-      await updateSession(await result.user.getIdToken());
+      const decodedToken = await result.user.getIdTokenResult();
+      if (Date.now() >= decodedToken.expirationTime) {
+        await signOut();
+        throw new Error('認証の有効期限が切れています');
+      }
+      await updateSession(decodedToken.token);
       invalidate('auth:session');
     }
+  }).catch(async (error) => {
+    console.error('認証エラー:', error);
+    await signOut();
   });
 }
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 80376d0 and 0286358.

📒 Files selected for processing (3)
  • src/lib/firebase-auth/client.ts (1 hunks)
  • src/routes/shop/+page.svelte (1 hunks)
  • src/routes/verify_email/+page.svelte (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/routes/shop/+page.svelte
  • src/routes/verify_email/+page.svelte

@ciscorn ciscorn merged commit 04c4045 into main Dec 3, 2024
2 checks passed
@ciscorn ciscorn deleted the demo-resetmail-magiclink branch December 3, 2024 18:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant