Skip to content

Add JWT token extraction fallback for cross-browser compatibility#243

Merged
lsukaristone merged 4 commits intomainfrom
user-auth-fix
Aug 19, 2025
Merged

Add JWT token extraction fallback for cross-browser compatibility#243
lsukaristone merged 4 commits intomainfrom
user-auth-fix

Conversation

@lsukaristone
Copy link
Copy Markdown
Contributor

@lsukaristone lsukaristone commented Aug 19, 2025

  • Extract email directly from JWT token in URL hash
  • Fixes issue where Chrome/Firefox couldn't retrieve session
  • Works around stricter third-party cookie policies
  • Ensures email field is populated across all browsers

🤖 Generated with Claude Code


Important

Add JWT token extraction from URL hash in OnboardingPage.tsx for cross-browser compatibility and session handling improvements.

  • Behavior:
    • Extracts email from JWT token in URL hash in OnboardingPage.tsx for cross-browser compatibility.
    • Clears sensitive token fragments from URL after processing.
    • Locks email field once session is confirmed.
  • Bug Fixes:
    • Improves session-detection flow to reduce waiting and preserve timeout behavior.
    • Handles malformed or invalid token data gracefully, showing an error toast and stopping the auth-check spinner.
  • Misc:
    • Adds extractEmailFromUrl() and clearAuthFragment() functions to handle token extraction and URL cleaning.

This description was created by Ellipsis for 4b02e78. You can customize this summary. It will automatically update as commits are pushed.


Summary by CodeRabbit

  • New Features
    • Automatically pre-fills the email field on the onboarding page when arriving via a magic link and locks the field once the session is confirmed.
    • Clears sensitive token fragments from the URL after processing.
  • Bug Fixes
    • Improves session-detection flow to reduce waiting and preserve timeout behavior.
    • Handles malformed or invalid token data gracefully and surfaces an error toast while stopping the auth-check spinner.
  • Style/Documentation
    • Clarified comments around magic-link and timeout handling.

- Extract email directly from JWT token in URL hash
- Fixes issue where Chrome/Firefox couldn't retrieve session
- Works around stricter third-party cookie policies
- Ensures email field is populated across all browsers

🤖 Generated with [Claude Code](https://claude.ai/code)

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

coderabbitai Bot commented Aug 19, 2025

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Warning

Rate limit exceeded

@lsukaristone has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 13 minutes and 16 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between 294f9fd and 4b02e78.

📒 Files selected for processing (1)
  • src/components/OnboardingPage.tsx (4 hunks)

Walkthrough

Adds URL-hash token parsing and clearing to OnboardingPage: extracts an email from an access_token in location.hash, pre-fills the email input, clears sensitive fragments, and updates auth-check/session handling and email locking without changing public signatures. Error paths log and show a destructive toast.

Changes

Cohort / File(s) Summary of Changes
Onboarding auth flow & email prefill
src/components/OnboardingPage.tsx
Added extractEmailFromUrl (decodes access_token via jwt-decode), clearAuthFragment (removes sensitive keys from URL hash), emailLocked state, and updated auth/session-check flows: prefill email from URL hash before session retrieval, lock email when session exists, clear fragment after processing, add session retrieval error handling and destructive toast; replaced disabled={!!watch('email')} with disabled={emailLocked}.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant U as User
  participant P as OnboardingPage
  participant H as URL Hash
  participant X as extractEmailFromUrl
  participant A as Auth Client
  participant F as Form

  U->>P: Load OnboardingPage
  P->>H: read location.hash
  P->>X: parse access_token → jwtDecode(payload)
  alt email extracted
    X-->>P: email
    P->>F: setValue('email', email)  Note right of P: emailUnlocked until session
  else no email / decode failure
    X-->>P: null (console.error)
  end

  P->>A: checkSession()
  alt session with email
    A-->>P: session(user.email)
    P->>F: setValue('email', session.user.email)
    P->>P: set emailLocked = true
    P->>H: clearAuthFragment()
    P-->>U: stop auth-check spinner
  else no session
    A-->>P: null
    P-->>U: continue until AUTH_CHECK_TIMEOUT_MS then clear fragment and stop spinner
  end

  opt on session retrieval error
    A-->>P: error
    P->>P: console.error + show destructive toast
    P-->>U: stop auth-check spinner
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Poem

A rabbit peeked inside the hash,
Found a token, gave it a bash.
Pulled an email, set it neat,
Cleared the crumbs and hopped off fleet.
🐇✨

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch user-auth-fix

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
🪧 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.
    • 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.
  • 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 the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

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

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • 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
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Summary of Changes

Hello @lsukaristone, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request introduces a robust solution to improve cross-browser compatibility for user session retrieval, specifically addressing issues where Chrome and Firefox might fail to retrieve session information due to stricter third-party cookie policies. It implements a fallback mechanism to extract the user's email directly from the JWT token present in the URL hash, ensuring the email field is correctly populated and the onboarding process can proceed across all major browsers.

Highlights

  • JWT Extraction from URL Hash: A new utility function, extractEmailFromUrl, has been added to OnboardingPage.tsx. This function is responsible for parsing the URL hash, identifying an access_token (JWT), decoding it, and extracting the user's email address from the token's payload.
  • Enhanced Session Checking Logic: The checkSession function now first attempts to retrieve the email using the newly introduced extractEmailFromUrl function. If an email is successfully extracted from the URL hash, it is used to pre-populate the email input field, providing a workaround for scenarios where supabase.auth.getSession() might fail due to browser cookie restrictions.
  • Improved Cross-Browser Compatibility: This change directly addresses and fixes issues experienced in browsers like Chrome and Firefox, where stricter third-party cookie policies prevented proper session retrieval, by providing an alternative method to obtain essential user information (email) for the onboarding flow.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in issue comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Copy Markdown

@ellipsis-dev ellipsis-dev Bot left a comment

Choose a reason for hiding this comment

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

Caution

Changes requested ❌

Reviewed everything up to dfa5f69 in 1 minute and 23 seconds. Click for details.
  • Reviewed 60 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. src/components/OnboardingPage.tsx:86
  • Draft comment:
    After successful extraction of the email from the URL, consider clearing the URL hash to prevent repeated processing or misuse.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50% None

Workflow ID: wflow_EBu45eVA5MgqDOus

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

Comment thread src/components/OnboardingPage.tsx Outdated
if (accessToken) {
// Decode JWT to get email
try {
const base64Url = accessToken.split('.')[1]
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Consider validating that the JWT token has the expected three segments before decoding. A dedicated JWT library (e.g. jwt-decode) could improve reliability and maintainability.

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a fallback mechanism to extract the user's email from a JWT in the URL hash, which is a good strategy for improving the magic link flow on browsers with strict cookie policies. The overall logic is sound. My main feedback is focused on refactoring the new extractEmailFromUrl function in OnboardingPage.tsx to improve its structure, robustness, and maintainability. I've provided a detailed suggestion to move the function outside the component and to use a standard library for JWT decoding.

Comment thread src/components/OnboardingPage.tsx Outdated
Comment on lines +45 to +68
const extractEmailFromUrl = () => {
const hashParams = new URLSearchParams(window.location.hash.substring(1))
const accessToken = hashParams.get('access_token')

if (accessToken) {
// Decode JWT to get email
try {
const base64Url = accessToken.split('.')[1]
const base64 = base64Url.replace(/-/g, '+').replace(/_/g, '/')
const jsonPayload = decodeURIComponent(
atob(base64)
.split('')
.map(c => '%' + ('00' + c.charCodeAt(0).toString(16)).slice(-2))
.join('')
)
const payload = JSON.parse(jsonPayload)
return payload.email || null
} catch (error) {
console.error('Error extracting email from token:', error)
return null
}
}
return null
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

This new helper function is a great addition for cross-browser compatibility, but its implementation can be improved for better maintainability, robustness, and performance.

  1. Function Placement: The function is defined inside the useEffect hook, which means it gets redeclared on every component render. Since it's a pure utility function that doesn't depend on component state or props, it should be moved outside the OnboardingPage component definition entirely. This improves performance, cleans up the component's scope, and makes the function potentially reusable.

  2. JWT Decoding Logic: The manual JWT decoding logic is complex, hard to read, and can be fragile. It's much safer and cleaner to use a standard, well-tested library like jwt-decode to handle this. This would abstract away the complexities of Base64URL conversion and UTF-8 decoding, making the code more robust and maintainable.

Here is an example of how the function could be refactored to address these points:

// 1. Add dependency: `npm install jwt-decode` or `yarn add jwt-decode`
// 2. Import at the top of the file
import { jwtDecode } from 'jwt-decode';

// 3. Define the function outside the component
const extractEmailFromUrl = (): string | null => {
  const hashParams = new URLSearchParams(window.location.hash.substring(1));
  const accessToken = hashParams.get('access_token');

  if (!accessToken) {
    return null;
  }

  try {
    const payload: { email?: string } = jwtDecode(accessToken);
    return payload.email || null;
  } catch (error) {
    console.error('Error decoding token from URL:', error);
    return null;
  }
};

export default function OnboardingPage() {
  // ... component logic ...
  useEffect(() => {
    // ... you can now call the globally defined extractEmailFromUrl() here
  }, []);
  // ...
}

- Move extractEmailFromUrl function outside component for better performance
- Use jwt-decode library instead of manual JWT parsing for robustness
- Add proper TypeScript typing for JWT payload
- Improve code maintainability and reusability

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
@lsukaristone
Copy link
Copy Markdown
Contributor Author

@gemini-code-assist review

Copy link
Copy Markdown

@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: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/components/OnboardingPage.tsx (1)

101-108: Clear sensitive tokens from URL hash after processing.

Leaving access_token/refresh_token in the URL fragment exposes them to browser history and potential referrer leaks via same-page navigations. Clear the fragment after session is established, and also after the timeout path.

Apply this diff within the timeout:

           timeoutId = setTimeout(() => {
             if (isMounted) {
               setIsCheckingAuth(false) // Stop loading to show form
-              // Email may already be set from URL extraction
+              // Email may already be set from URL extraction; clear sensitive tokens from URL hash
+              clearAuthFragment()
             }
           }, AUTH_CHECK_TIMEOUT_MS)

Add this helper outside the effect:

function clearAuthFragment() {
  const url = new URL(window.location.href)
  if (!url.hash) return
  const params = new URLSearchParams(url.hash.substring(1))
  // Remove common auth fragments introduced by providers
  ;['access_token', 'refresh_token', 'expires_in', 'token_type', 'provider_token'].forEach(k => params.delete(k))
  const newHash = params.toString()
  const newUrl = `${url.origin}${url.pathname}${url.search}${newHash ? '#' + newHash : ''}`
  window.history.replaceState(null, '', newUrl)
}

Also call clearAuthFragment() right after a session is detected (Line 74 block) to scrub tokens in the success path as well.

🧹 Nitpick comments (2)
src/components/OnboardingPage.tsx (2)

86-92: Keep email editable when it’s only prefilled from URL (avoid locking on unverified data).

Today, the email field is disabled whenever it has a value (disabled={!!watch('email')}). If we prefill from the URL before a session is established, users can’t correct it if the token is stale or malformed. Prefer locking only when the email comes from a verified session.

Minimal change within this block:

         if (urlEmail) {
           setValue('email', urlEmail)
+          setEmailLocked(false) // Prefill only; allow edits until a session is established
           // Don't stop checking auth yet - wait for proper session
         }

Add the following outside this block:

  • Introduce a lock flag:
// near other state hooks
const [emailLocked, setEmailLocked] = useState(false)
  • When a session is present (Line 74), lock the email:
if (session?.user?.email) {
  clearTimeout(timeoutId)
  setValue('email', session.user.email)
  setEmailLocked(true)
  setIsCheckingAuth(false)
}
  • Update the input to use the lock flag (Lines 296-309):
<Input
  id='email'
  {...register('email', { /* ... */ })}
  placeholder='Enter your email'
  disabled={emailLocked}
/>

Please confirm desired UX: Should URL-prefilled email be editable until session confirmation? If so, the above keeps user control without weakening security.


44-68: Overall: solid, targeted fallback for cross-browser magic link flows.

The approach to prefill email from the JWT payload before session retrieval is pragmatic and improves UX on browsers with stricter third-party cookie policies. Once the decoding/padding tweak and URL-scrubbing are in, this should be robust.

Nit: extractEmailFromUrl can be moved outside useEffect for readability/testability; it has no closure deps.

Also applies to: 86-92, 101-108

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled
  • Linear integration is disabled

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between a523d89 and dfa5f69.

📒 Files selected for processing (1)
  • src/components/OnboardingPage.tsx (3 hunks)
🧰 Additional context used
🪛 ESLint
src/components/OnboardingPage.tsx

[error] 57-57: Unexpected string concatenation.

(prefer-template)


[error] 57-57: Unexpected string concatenation.

(prefer-template)

Comment thread src/components/OnboardingPage.tsx Outdated
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a clever fallback to extract the user's email from a JWT in the URL hash, effectively working around stricter third-party cookie policies in some browsers. The implementation is clean and correctly integrated into the onboarding flow. I have one minor suggestion to improve the type safety of the new token decoding logic.

}

try {
const payload: { email?: string } = jwtDecode(accessToken)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

To improve type safety, you can pass the expected payload type as a generic argument to jwtDecode. This is a more modern and type-safe approach than using a type assertion.

Suggested change
const payload: { email?: string } = jwtDecode(accessToken)
const payload = jwtDecode<{ email?: string }>(accessToken)

Copy link
Copy Markdown

@ellipsis-dev ellipsis-dev Bot left a comment

Choose a reason for hiding this comment

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

Important

Looks good to me! 👍

Reviewed cd90c57 in 1 minute and 13 seconds. Click for details.
  • Reviewed 70 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. src/components/OnboardingPage.tsx:13
  • Draft comment:
    Verify the jwt-decode import: it's commonly a default export. Consider using 'import jwtDecode from "jwt-decode"' if needed.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 0% vs. threshold = 50% While jwt-decode is commonly used as a default export, the current version (v4) actually supports both named and default exports. The code works either way. This is more of a style preference than a required change. The comment is asking to verify something that's already working. The code is currently working as-is. The comment is asking for verification rather than pointing out a clear issue. Even though the suggestion might be technically correct, it doesn't point out a clear problem that needs fixing - both import styles work. Delete the comment since it's asking for verification of something that already works, and doesn't point out a clear issue that needs fixing.
2. src/components/OnboardingPage.tsx:25
  • Draft comment:
    Good refactor using jwtDecode; consider adding explicit payload type checks to ensure the decoded token is in the expected format before accessing the email property.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50% None

Workflow ID: wflow_12KXgKDEWS91B68X

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

- Add clearAuthFragment() to remove sensitive tokens from URL after processing
- Implement emailLocked state to differentiate between URL-prefilled and session-verified emails
- Allow users to edit URL-prefilled email until session is established
- Clear auth tokens on both success and timeout paths
- Improve security by removing access_token, refresh_token, etc. from browser history

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
@lsukaristone
Copy link
Copy Markdown
Contributor Author

@gemini-code-assist review

Copy link
Copy Markdown

@ellipsis-dev ellipsis-dev Bot left a comment

Choose a reason for hiding this comment

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

Important

Looks good to me! 👍

Reviewed 294f9fd in 51 seconds. Click for details.
  • Reviewed 78 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 5 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. src/components/OnboardingPage.tsx:43
  • Draft comment:
    Good encapsulation in clearAuthFragment. The use of a prefixed semicolon before the array ensures correct parsing; make sure this pattern is consistently understood by the team.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50% None
2. src/components/OnboardingPage.tsx:60
  • Draft comment:
    The new emailLocked state provides clear control over the email input's editability. Ensure that its updates (locking/unlocking) align with all auth events.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50% None
3. src/components/OnboardingPage.tsx:85
  • Draft comment:
    Locking the email when a verified session is detected and calling clearAuthFragment helps secure the URL. Confirm that this behavior properly resets (unlocks) on sign out.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50% None
4. src/components/OnboardingPage.tsx:108
  • Draft comment:
    In the session-check fallback, using a timeout to stop loading and clear auth fragments is acceptable. Verify that users with only URL prefill (but no session) experience the intended behavior.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50% None
5. src/components/OnboardingPage.tsx:323
  • Draft comment:
    Switching the disabled attribute of the email input to rely on emailLocked offers finer control compared to checking for a value. Ensure this meets all UX expectations.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50% None

Workflow ID: wflow_Muhui4fQ4mxBbiv4

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a fallback mechanism to extract user email from a JWT in the URL hash, improving cross-browser compatibility for authentication, especially with strict cookie policies. The changes are well-implemented, adding helper functions to parse the token and clean the URL. My feedback focuses on improving maintainability by reducing code duplication and using constants for magic strings. Overall, this is a solid enhancement to the onboarding flow.

Comment thread src/components/OnboardingPage.tsx Outdated

const params = new URLSearchParams(url.hash.substring(1))
// Remove common auth fragments introduced by providers
;['access_token', 'refresh_token', 'expires_in', 'token_type', 'provider_token'].forEach(k => params.delete(k))
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The leading semicolon is unnecessary here and can be removed. For improved readability and maintainability, consider extracting this array of magic strings into a named constant at the module level.

Suggested change
;['access_token', 'refresh_token', 'expires_in', 'token_type', 'provider_token'].forEach(k => params.delete(k))
['access_token', 'refresh_token', 'expires_in', 'token_type', 'provider_token'].forEach(k => params.delete(k))

Comment on lines 109 to 114
if (session?.user?.email) {
setValue('email', session.user.email)
setEmailLocked(true) // Lock email when it comes from verified session
setIsCheckingAuth(false)
clearAuthFragment() // Clear sensitive tokens from URL
} else {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

This block of code for handling a successful session is nearly identical to the one in the onAuthStateChange handler (lines 82-87). To improve maintainability and adhere to the DRY (Don't Repeat Yourself) principle, it would be beneficial to extract this logic into a single helper function. This function could then be called from both places to ensure consistency and simplify future updates.

- Extract AUTH_TOKENS_TO_CLEAR constant to eliminate magic strings
- Remove unnecessary semicolon before array forEach
- Create handleSessionEstablished helper to eliminate code duplication
- Apply DRY principle for session handling logic

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
@lsukaristone lsukaristone merged commit 4423654 into main Aug 19, 2025
1 check passed
Copy link
Copy Markdown

@ellipsis-dev ellipsis-dev Bot left a comment

Choose a reason for hiding this comment

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

Important

Looks good to me! 👍

Reviewed 4b02e78 in 41 seconds. Click for details.
  • Reviewed 63 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 3 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. src/components/OnboardingPage.tsx:25
  • Draft comment:
    Good refactor: extracting auth token keys into a constant reduces duplication.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50% None
2. src/components/OnboardingPage.tsx:53
  • Draft comment:
    Nice update using AUTH_TOKENS_TO_CLEAR for token removal in URL hash.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50% None
3. src/components/OnboardingPage.tsx:81
  • Draft comment:
    DRYing up the session-established logic with a helper function improves maintainability.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50% None

Workflow ID: wflow_8bFVyUU1MLuB5KhI

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

Copy link
Copy Markdown

@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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/components/OnboardingPage.tsx (1)

124-134: Clear URL fragment on error path to avoid leaking tokens

If supabase.auth.getSession() throws, the code currently doesn’t clear the hash. This can leave sensitive tokens in the URL indefinitely (security/PII risk).

Apply this diff:

       toast({
         variant: 'destructive',
         title: 'Authentication Error',
         description: 'Failed to retrieve user session'
       })
       setIsCheckingAuth(false)
+      // Ensure sensitive auth fragments are not left in the URL on failure
+      clearAuthFragment()
♻️ Duplicate comments (1)
src/components/OnboardingPage.tsx (1)

25-41: Use jwtDecode generic type parameter for better type-safety

Passing the expected payload type as a generic to jwtDecode removes the need for a separate type annotation and improves readability.

Apply this diff:

-    const payload: { email?: string } = jwtDecode(accessToken)
+    const payload = jwtDecode<{ email?: string }>(accessToken)
🧹 Nitpick comments (2)
src/components/OnboardingPage.tsx (2)

43-56: Fix ESLint prefer-template violation when composing new URL

ESLint flagged Line 53 (prefer-template). Replace '#' + newHash with a template literal.

Apply this diff:

-  const newUrl = `${url.origin}${url.pathname}${url.search}${newHash ? '#' + newHash : ''}`
+  const newUrl = `${url.origin}${url.pathname}${url.search}${newHash ? `#${newHash}` : ''}`

25-56: Consider moving URL/JWT helpers to a shared util for reuse

extractEmailFromUrl and clearAuthFragment are pure and UI-agnostic. Moving them to a small helper module (e.g., src/lib/authUrl.ts) improves reusability and reduces component surface area.

If you want, I can draft a small authUrl.ts with tests.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled
  • Linear integration is disabled

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between cd90c57 and 294f9fd.

📒 Files selected for processing (1)
  • src/components/OnboardingPage.tsx (4 hunks)
🧰 Additional context used
🪛 ESLint
src/components/OnboardingPage.tsx

[error] 53-53: Unexpected string concatenation.

(prefer-template)

🔇 Additional comments (6)
src/components/OnboardingPage.tsx (6)

85-92: Auth state handling looks solid

Locking the email only when a verified session is present and unlocking on sign-out is the right UX. Clearing the URL fragment once a session is established avoids leaving sensitive data around.


97-104: Good fallback: prefill from URL token but keep editable until session

This gracefully handles stricter cookie policies without over-committing to an unverified email.


111-114: Nice: clear auth fragment on success and after timeout

Clearing the fragment both when a session is obtained and when timing out prevents tokens from lingering in the URL.

Also applies to: 119-121


60-60: Email lock state addition LGTM

Dedicated emailLocked state is clearer than deriving disabled-ness from form values.


323-323: Correctly gate input with emailLocked

This aligns the form control with the auth state. Good change.


13-13: jwt-decode import is correct for v4+

  • package.json declares "jwt-decode": "^4.0.0"
  • import { jwtDecode } from 'jwt-decode' matches the v4+ named-export API

No changes required.

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