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

fix: dynamic origin handling and updated redirect URLs for environmen… #388

Open
wants to merge 4 commits into
base: dev
Choose a base branch
from

Conversation

OlenaReukova
Copy link
Collaborator

@OlenaReukova OlenaReukova commented Dec 6, 2024

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review on my code
  • I have made corresponding changes to the documentation (if any were needed)
  • My changes generate no new warnings or errors
  • I have added tests that prove my fix is effective or that my feature works
  • New and previously existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

Description

**Relates #385 OR Closes #385 **

Instead of defaulting to http://localhost:3000/, the reset password email link should dynamically generate the correct URL based on the environment.

In the deployed version, the reset password email link should point to the actual domain of the website (e.g., https://your-deployed-domain.com/login/reset-password) instead of the localhost URL.

Code Update:

  • Refactored origin detection to handle cases where the value is missing or undefined smoothly.
  • Default to http://localhost:3000 for development mode.
  • Dynamically set the origin for production using NEXT_PUBLIC_SITE_URL environment variable.
const headersList = await headers();
  let origin: string | null = headersList.get('origin') || null;

  if (!origin) {
    origin =
      process.env.NODE_ENV === 'development'
        ? 'http://localhost:3000'
        : process.env.NEXT_PUBLIC_SITE_URL || null;
  }

Configuration Update:

Updated config.toml to include additional production redirect URLs "https://**cool-creponne-3e1272.netlify.app/**", "https://cool-creponne-3e1272.netlify.app/login/reset-password" to ensure Supabase accepts them, preventing failed password reset flows due to unlisted URLs.

(Supabase Docs)

Files changed

  • app/(auth)/login/forgot-password/page.tsx
  • config.toml

UI changes

no UI changes

Changes to Documentation

No changes needed.

Tests

All previous tests passed during local development.

Copy link

netlify bot commented Dec 6, 2024

👷 Deploy request for cool-creponne-3e1272 pending review.

Visit the deploys page to approve it

Name Link
🔨 Latest commit 88ae7ff

Copy link
Contributor

@camelPhonso camelPhonso left a comment

Choose a reason for hiding this comment

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

I have a theory that this needs to be sorted with some tweaks in supabase but I no longer have access because I'm not logged in with my old email account. Can someone reach out on discord and give me access on my personal email?

app/(auth)/login/forgot-password/page.tsx Outdated Show resolved Hide resolved
supabase/config.toml Outdated Show resolved Hide resolved
removed redundant logic for setting the origin value based on NODE_ENV and NEXT_PUBLIC_SITE_URL

Replaced with a simplified implementation: const headerData = await headers(); const origin = headerData.get('origin')

The previous fallback logic was unnecessary since the origin header is reliably retrieved using the headers() function in most environments.
@OlenaReukova
Copy link
Collaborator Author

OlenaReukova commented Jan 17, 2025

The console.log("Origin", origin) confirms that the origin value is fetched correctly during my tests, and no errors (e.g., null or unexpected results) occur. This makes me believe that the production deployment should work as expected, provided the server environment properly sets the origin header.

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.

Bug incorrect Reset Password URLs in a deployed environment
2 participants