Skip to content

fix: improve OAuth2 security implementation#27

Merged
coji merged 2 commits intomainfrom
fix/security-improvements
Jan 11, 2026
Merged

fix: improve OAuth2 security implementation#27
coji merged 2 commits intomainfrom
fix/security-improvements

Conversation

@coji
Copy link
Owner

@coji coji commented Jan 11, 2026

Summary

  • 乱数生成を Math.random() から crypto.randomUUID() に変更(暗号学的に安全)
  • nonce 検証を Firebase 署名検証後に実行するよう順序を修正
  • Google からのエラーレスポンス処理を追加
  • localStorage のキー名を明確化('v''google_oauth_validation'
  • エラーメッセージを一般化し、内部情報の漏洩を防止
  • Firestore セキュリティルールを追加

Test plan

  • Google ログインが正常に動作すること
  • ログインキャンセル時にエラーメッセージが表示されること
  • アカウント作成エラー時に一般的なメッセージが表示されること
  • Firestore ルールが正しくデプロイされていること

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added Firestore security rules enforcing read/write based on authentication and ownership.
  • Bug Fixes & Improvements

    • Improved Google sign-in flow with deterministic validation, clearer error messages, and stronger nonce/identity checks.
    • Replaced detailed account-creation error with a concise Japanese user-facing message: "アカウントの作成に失敗しました。しばらくしてから再度お試しください。"

✏️ Tip: You can customize this high-level summary in your review settings.

- Use crypto.randomUUID() instead of Math.random() for state/nonce
- Verify nonce after Firebase signature verification
- Add Google error response handling
- Rename localStorage key to 'google_oauth_validation'
- Sanitize error messages to prevent information leakage
- Add Firestore security rules for proper authorization

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@coderabbitai
Copy link

coderabbitai bot commented Jan 11, 2026

Caution

Review failed

The pull request is closed.

📝 Walkthrough

Walkthrough

OAuth validation moved from per-call async storage to a synchronous, centralized STORAGE_KEY; JWT payload decoding added for nonce validation; authentication callback now surfaces Google errors, verifies state, performs signature verification before nonce check, and deletes validation data. Firestore rules added enforcing ownership and auth restrictions.

Changes

Cohort / File(s) Summary
Google OAuth Flow Refactoring
app/libs/google-auth.ts
Centralizes OAuth validation under STORAGE_KEY; converts storeValidationValue/restoreValidationValue to synchronous functions; adds decodeJwtPayload; changes authenticate(request) to synchronous flow using crypto.randomUUID for state/nonce; surfaces Google error responses; verifies state, calls verifyUser (signature verification) then decodes id_token to validate nonce; improves and standardizes error messages.
Account Creation Error Handling
app/routes/welcome+/create_account/route.tsx
Replaces technical exception display with a generic Japanese user-facing error message and logs the error to console in Japanese.
Firestore Security Rules
firestore.rules
Adds rules: public reads for accounts/posts; create/update/delete allowed only by authenticated owner (UID checks) including nested /accounts/{handle}/posts/{postId}; deny all other access by default.

Sequence Diagram

sequenceDiagram
    actor User
    participant Browser as Browser Storage
    participant AuthLib as google-auth.ts
    participant GoogleOAuth as Google OAuth Service
    participant Backend as verifyUser (server)
    participant JWTDecoder as decodeJwtPayload

    User->>AuthLib: Initiate OAuth login
    AuthLib->>Browser: Store validation (STORAGE_KEY) with state & nonce
    AuthLib->>GoogleOAuth: Redirect to Google consent

    GoogleOAuth->>User: Redirect back with code & state (or error)
    User->>AuthLib: Callback request with code & state
    AuthLib->>Browser: Retrieve and delete stored validation data
    AuthLib->>AuthLib: Validate state matches

    AuthLib->>Backend: verifyUser (exchange code, verify signature)
    Backend-->>AuthLib: Returns id_token (signed)

    AuthLib->>JWTDecoder: Decode id_token payload (no signature verify)
    JWTDecoder-->>AuthLib: JWT payload with nonce
    AuthLib->>AuthLib: Validate nonce matches stored value
    AuthLib-->>User: Complete authentication / error if any step failed
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Poem

🐰 I hopped and hid a state and nonce, so neat,
I verified signatures, then decoded the treat,
Guards on Firestore, owner-only gates,
Logs in Japanese for unexpected fates,
A tiny rabbit celebrates secure feats! 🎉

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix: improve OAuth2 security implementation' accurately reflects the main changes: OAuth2 security improvements including secure randomization, nonce verification, error handling, and Firestore security rules.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f99e935 and dc739a7.

📒 Files selected for processing (2)
  • app/libs/google-auth.ts
  • app/routes/welcome+/create_account/route.tsx

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link

github-actions bot commented Jan 11, 2026

Visit the preview URL for this PR (updated for commit dc739a7):

https://remix-spa-example--pr27-fix-security-improve-a30g5i1w.web.app

(expires Sun, 18 Jan 2026 02:17:44 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: 48e666b485811f0b1bcd4fa4838b32a205a3ce05

@coji coji merged commit 5223049 into main Jan 11, 2026
2 checks passed
@coji coji deleted the fix/security-improvements branch January 11, 2026 02:18
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