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

signInWithRedirect #6

Merged
merged 1 commit into from
Sep 14, 2024
Merged

signInWithRedirect #6

merged 1 commit into from
Sep 14, 2024

Conversation

ciscorn
Copy link
Member

@ciscorn ciscorn commented Sep 14, 2024

Summary by CodeRabbit

  • 新機能

    • 認証フローを改善し、リダイレクトによるサインインをサポート。
    • 認証処理を簡素化し、再利用可能なフックを導入。
    • ログイン機能に非同期処理を追加し、ユーザー体験を向上。
  • バグ修正

    • APIレスポンスのエラーチェックを追加し、失敗時の安定性を向上。
  • ドキュメンテーション

    • 認証プロセスに関するコメントを明確化。
  • リファクタリング

    • 認証ロジックを整理し、メンテナンス性を向上。
    • プロパティの型を明示的に定義し、可読性を向上。
    • 認証機能の実装を簡素化し、柔軟性を向上。

Copy link

cloudflare-workers-and-pages bot commented Sep 14, 2024

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

Latest commit: a33e002
Status: ✅  Deploy successful!
Preview URL: https://8ed48a74.sveltekit-firebaseauth-ssr-stripe-demo.pages.dev
Branch Preview URL: https://signin-with-redirect.sveltekit-firebaseauth-ssr-stripe-demo.pages.dev

View logs

@ciscorn ciscorn force-pushed the signin-with-redirect branch from 7969021 to dbbcf48 Compare September 14, 2024 02:05
Copy link

coderabbitai bot commented Sep 14, 2024

Walkthrough

このプルリクエストでは、認証ロジックに関する複数の変更が行われました。主な変更点は、getAuthWithKV 関数が getAuth に名前変更され、認証処理が簡素化されたことです。また、Firebase 認証のフローが改善され、リダイレクトを用いたサインインが追加されました。さらに、Svelte コンポーネント内での型安全性が向上し、API 呼び出しのエラーハンドリングも強化されました。

Changes

ファイルパス 変更概要
src/api/auth.ts getAuthWithKVgetAuth に変更し、認証ロジックを更新。
src/hooks.server.ts verifySessionTokenauthGuard を統合し、createAuthHook によるシンプルなハンドラーを実装。
src/lib/firebase/client.ts リダイレクトによるサインインを追加し、waitForRedirectResult 関数を導入。
src/lib/firebase/server.ts getAuthWithKVgetAuth に変更し、メモリストレージ機能を追加。
src/routes/+layout.svelte $props のプロパティを明示的に型定義。
src/routes/+page.svelte ボタンとチェックボックスの状態管理を削除し、静的表示に変更。
src/routes/login/+page.svelte 非同期処理を導入し、リダイレクト結果を待つロジックを追加。
src/routes/private/+layout.svelte $props のプロパティを明示的に型定義。
src/routes/private/+page.ts API 呼び出しのレスポンスを確認する条件チェックを追加。
src/routes/session/+server.ts getAuthWithKVgetAuth に変更し、CSRF 保護についてのコメントを更新。

Possibly related PRs

  • MIERUNE/asia-turidoco-poc#116: 認証ロジックの変更に関連し、getAuthWithKV の名前変更が行われているため。
  • MIERUNE/asia-turidoco-poc#115: 認証変更とは直接関係ないが、ユーザーインタラクションに関連する可能性がある。
  • MIERUNE/asia-turidoco-poc#136: コードの整理に関する変更で、認証ロジックには直接関わっていない。

うさぎが跳ねて、喜びの舞
認証が新しく、スムーズに進む
リダイレクトでサインイン、楽しい日々
型が明示され、エラーも減る
みんなで笑顔、コードは輝く ✨🐇


Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between ff247a0 and a33e002.

Files ignored due to path filters (2)
  • package-lock.json is excluded by !**/package-lock.json, !**/*.json
  • package.json is excluded by !**/*.json
Files selected for processing (10)
  • src/api/auth.ts (2 hunks)
  • src/hooks.server.ts (1 hunks)
  • src/lib/firebase/client.ts (2 hunks)
  • src/lib/firebase/server.ts (1 hunks)
  • src/routes/+layout.svelte (1 hunks)
  • src/routes/+page.svelte (0 hunks)
  • src/routes/login/+page.svelte (1 hunks)
  • src/routes/private/+layout.svelte (1 hunks)
  • src/routes/private/+page.ts (1 hunks)
  • src/routes/session/+server.ts (1 hunks)
Files not reviewed due to no reviewable changes (1)
  • src/routes/+page.svelte
Files skipped from review as they are similar to previous changes (7)
  • src/api/auth.ts
  • src/lib/firebase/client.ts
  • src/routes/+layout.svelte
  • src/routes/login/+page.svelte
  • src/routes/private/+layout.svelte
  • src/routes/private/+page.ts
  • src/routes/session/+server.ts
Additional comments not posted (7)
src/hooks.server.ts (1)

1-8: 認証ロジックのリファクタリングにより、コードの保守性と可読性が向上しました。

以前の実装では、セッションの検証と認証ガードの機能が別々のハンドラーに分かれていましたが、createAuthHook関数によって単一の再利用可能なフックに統合されました。この変更により、認証ロジックがよりモジュール化され、コードの重複が減り、制御の流れがシンプルになりました。

guardPathの正規表現/^\/(private|shop)(\/.*)?/を使用して、認証が必要なパスを指定しているのは適切です。これにより、特定のルートを認証されていないアクセスから保護しつつ、意図した機能を維持しています。

全体的に、リファクタリングによってコードの品質と保守性が向上しています。

src/lib/firebase/server.ts (6)

9-9: LGTM!

Handleredirect のインポートは、新しい createAuthHook 関数に必要です。


13-14: LGTM!

projectIdServiceAccountCredential の初期化は、Firebase Auth インスタンスを初期化するために必要です。


16-19: LGTM!

getAuth 関数の変更により、Firebase Auth インスタンスの初期化がより柔軟になりました。kv の有無に基づいて WorkersKVStoreSingle または makeMemoryStorer を使用するアプローチは、異なる環境をサポートするための良い方法です。


21-23: LGTM!

getAuthWithStorer 関数の名前変更と可視性の変更は、実装の詳細を非公開に保つための良いリファクタリングの決定です。


25-41: LGTM!

makeMemoryStorer 関数は、Cloudflare 以外の環境に適した優れたフォールバックメカニズムを提供します。キャッシュメカニズムの実装は正確で効率的に見えます。


43-91: LGTM!

createAuthHook 関数は、包括的な認証ミドルウェアソリューションを提供します。Firebase 認証プロキシリクエストの処理、セッション Cookie の検証と event.locals.currentUser の設定、正規表現に基づく認証ガードのロジック、ログインページにアクセスした認証済みユーザーのリダイレクトが適切に実装されています。


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>.
    • 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 generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @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

@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 (2)
src/routes/login/+page.svelte (1)

5-6: LGTM!

redirectResult の割り当ては正しく行われています。この変数は、ログインプロセスの非同期処理の結果を保持するために使用されます。

6行目の空行は不要なので、削除することをお勧めします。

const redirectResult = waitForRedirectResult();
-
let { data } = $props();
src/api/auth.ts (1)

19-19: getAuth 関数の呼び出しは適切ですが、c.env のnullチェックを追加することを検討してください。

getAuth 関数の呼び出しは、認証メカニズムがKVストアと連携することを示唆しており、PRの目的に沿ったものです。ただし、c.env が未定義の場合があることを考慮し、nullチェックを追加することをお勧めします。

nullチェックを追加するには、以下のように変更してください。

-const auth = getAuth(c.env?.KV);
+const auth = getAuth(c.env && c.env.KV);
Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 19c9e8f and dbbcf48.

Files ignored due to path filters (2)
  • package-lock.json is excluded by !**/package-lock.json, !**/*.json
  • package.json is excluded by !**/*.json
Files selected for processing (11)
  • src/api/auth.ts (2 hunks)
  • src/hooks.server.ts (1 hunks)
  • src/lib/firebase/client.ts (2 hunks)
  • src/lib/firebase/server.ts (1 hunks)
  • src/routes/+layout.svelte (1 hunks)
  • src/routes/+page.svelte (0 hunks)
  • src/routes/login/+page.svelte (1 hunks)
  • src/routes/private/+layout.svelte (1 hunks)
  • src/routes/private/+page.ts (1 hunks)
  • src/routes/session/+server.ts (1 hunks)
  • svelte.config.js (1 hunks)
Files not reviewed due to no reviewable changes (1)
  • src/routes/+page.svelte
Files skipped from review due to trivial changes (1)
  • src/routes/session/+server.ts
Additional comments not posted (16)
src/hooks.server.ts (1)

1-8: 良くできました!

認証ロジックのリファクタリングは、コードの可読性と保守性を向上させています。createAuthHook関数を使用することで、認証ロジックがモジュール化され、再利用可能になっています。また、guardPath正規表現を使用して、認証が必要なパスを正しく指定しています。

全体的に、このリファクタリングは認証ロジックを簡素化し、コードの品質を向上させています。

src/routes/private/+page.ts (1)

6-10: エラーハンドリングの改善

APIレスポンスが正常でない場合に、空のposts配列を返すことで、エラーハンドリングが改善されています。これにより、失敗したレスポンスからJSONをパースしようとすることによる潜在的なランタイムエラーを防ぐことができます。

この変更は、アプリケーションの安定性を向上させるための防御的プログラミングアプローチを反映しており、APIの失敗に直面してもアプリケーションが安定したままであることを保証します。

src/routes/private/+layout.svelte (2)

6-6: 型の安全性が向上しました!

./$types から PageData 型をインポートすることで、$propsdata プロパティの構造が明確になり、型の安全性が向上しました。これは良い習慣です。


8-14: 可読性と型の安全性が向上しました!

$props からプロパティを複数行で分割することで、コードの可読性が向上しました。また、data プロパティを明示的に PageData 型として指定することで、型の安全性と明確さが向上しました。これらの変更はコンポーネントの全体的なロジックや制御の流れを変更するものではありませんが、コードの保守性を高めています。

src/routes/login/+page.svelte (2)

2-2: LGTM!

waitForRedirectResult のインポートは正しく行われています。この関数は、ログインプロセスの非同期処理に使用されるようです。


12-21: LGTM!

{#await} ブロックを使用して、redirectResult の非同期処理を適切に処理しています。以下の点が優れています:

  • 結果を待っている間、ローディングメッセージを表示して、ユーザーにすぐにフィードバックを提供しています。
  • 結果が null かどうかをチェックし、null の場合は URL に next パラメータがあるかどうかを確認して、ログインメッセージを表示するかどうかを決定しています。
  • 現在のユーザーが検出された場合、Google でのサインインボタンが無効になり、不要なログイン試行を防止しています。

これらの変更により、ログインプロセスのユーザーエクスペリエンスが向上しています。

svelte.config.js (1)

1-1: アダプターの変更を承認しますが、アプリケーションの動作と展開プロセスを検証することをお勧めします。

@sveltejs/adapter-cloudflare から @sveltejs/adapter-auto へのアダプターの変更により、さまざまなプラットフォームでの展開が容易になり、柔軟性が向上します。ただし、この変更により、以前のアダプターとは異なる最適化や設定が適用される可能性があり、アプリケーションの動作に影響を与える可能性があります。

新しいアダプターでアプリケーションが期待どおりに機能し、展開プロセスがスムーズであることを確認してください。

src/routes/+layout.svelte (1)

6-12: 明示的な型アノテーションの追加は素晴らしいですね!

$props のデストラクチャリングに明示的な型アノテーションを追加したことで、コードの型安全性と明確性が向上しています。data プロパティを PageData 型として明示的に型付けし、children プロパティを Snippet 型として型付けすることで、コンポーネントに渡されるデータの期待される構造が明確になります。

この変更により、コードのメンテナンス性が向上し、型の不一致に関連する潜在的なランタイムエラーを防ぐのに役立ちます。また、TypeScriptの型チェック機能を活用することで、開発プロセスにおいてより良いドキュメントを提供し、サポートすることができます。

全体的なコンポーネントの機能は変更されていませんが、型アノテーションを追加することで、コードの意図がより明確になり、将来の保守性が向上します。

素晴らしい改善だと思います!

src/api/auth.ts (1)

5-5: インポート文の変更は適切です。

getAuthWithKV から getAuth へのインポート文の変更は、認証ロジックのリファクタリングを示唆しています。この変更は、PRの目的である signInWithRedirect の使用に沿ったものであり、適切だと思われます。

src/lib/firebase/client.ts (3)

39-50: LGTM!

リダイレクト方式のサインインフローを適切に処理しています。セッションの更新後に auth:session を無効化しているのは良い習慣ですね。


55-65: LGTM!

プロトコルに基づいて適切なサインインフローを使い分けています。HTTPSでは signInWithRedirect を使用しているのは、モバイル環境でのポップアップブロック対策として良い習慣ですね。


71-71: LGTM!

auth インスタンスの signOut メソッドを使用してユーザーをサインアウトしています。

src/lib/firebase/server.ts (4)

16-19: LGTM!

KVNamespace を受け取るようにしたことで、Cloudflare Workers 以外の環境でも使えるようになりました。makeMemoryStorer を使ってメモリにキャッシュするのは良いアイデアですね。


26-41: LGTM!

シンプルながら有効なメモリキャッシュの実装ですね。これにより、Cloudflare 以外の環境でもパフォーマンスを向上させることができます。


46-91: LGTM!

認証ミドルウェアの実装が素晴らしいです。Firebase Authentication との連携がスムーズで、認証が必要なページへの未認証ユーザーのアクセスを適切にガードしています。また、認証済みユーザーがログインページにアクセスした場合のリダイレクトも適切に処理されています。


21-23: LGTM!

getAuthWithStorer を内部ヘルパー関数にしたのは良い判断だと思います。外部に公開する必要はないので、内部関数にするのが適切ですね。

@ciscorn ciscorn force-pushed the signin-with-redirect branch 5 times, most recently from 155c416 to ff247a0 Compare September 14, 2024 02:37
@ciscorn ciscorn force-pushed the signin-with-redirect branch from ff247a0 to a33e002 Compare September 14, 2024 02:42
@ciscorn ciscorn merged commit b587a88 into main Sep 14, 2024
1 check passed
@ciscorn ciscorn deleted the signin-with-redirect branch September 14, 2024 02:47
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