Skip to content

fix: redirect to 404 page for invalid email verification token#9499

Merged
sean-brydon merged 3 commits into
calcom:mainfrom
iamr-kumar:ritik/add-invalid-token-page
Jun 19, 2023
Merged

fix: redirect to 404 page for invalid email verification token#9499
sean-brydon merged 3 commits into
calcom:mainfrom
iamr-kumar:ritik/add-invalid-token-page

Conversation

@iamr-kumar
Copy link
Copy Markdown
Contributor

@iamr-kumar iamr-kumar commented Jun 14, 2023

What does this PR do?

Fixes #9469

Type of change

  • Bug fix (non-breaking change which fixes an issue)
Screenshot 2023-06-19 at 12 01 55 AM

How should this be tested?

  • Send email verification link. Copy paste the link in browser and modify the token.
  • Go to /api/auth/verify-email?token={some-invalid-token}

Mandatory Tasks

  • Make sure you have self-reviewed the code. A decent size PR without self-review might be rejected.

@vercel
Copy link
Copy Markdown

vercel Bot commented Jun 14, 2023

@iamr-kumar is attempting to deploy a commit to the cal Team on Vercel.

A member of the Team first needs to authorize it.

@vercel
Copy link
Copy Markdown

vercel Bot commented Jun 14, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
ui ❌ Failed (Inspect) Jun 19, 2023 9:00am

@github-actions github-actions Bot added 🐛 bug Something isn't working 🧹 Improvements Improvements to existing features. Mostly UX/UI labels Jun 14, 2023
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 14, 2023

📦 Next.js Bundle Analysis for @calcom/web

This analysis was generated by the Next.js Bundle Analysis action. 🤖

This PR introduced no changes to the JavaScript bundle! 🙌

@Udit-takkar Udit-takkar added the Low priority Created by Linear-GitHub Sync label Jun 16, 2023
Copy link
Copy Markdown
Contributor

@Udit-takkar Udit-takkar left a comment

Choose a reason for hiding this comment

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

well this is what is see when i enter incorrect token in param. The text says username /404 does not exist

Screenshot 2023-06-16 at 2 35 37 PM

we should display correct text here like "Your token has expired".
you can check /pages/404 page to see how you can show custom message based on condition

@iamr-kumar
Copy link
Copy Markdown
Contributor Author

Thanks for the input. Also could you help me with how to get the strings translated? Or is it done by the maintainers?

@github-actions github-actions Bot added ❗️ .env changes contains changes to env variables ❗️ migrations contains migration files labels Jun 18, 2023
@iamr-kumar iamr-kumar force-pushed the ritik/add-invalid-token-page branch from 0498cf8 to 2dd949d Compare June 18, 2023 18:30
@iamr-kumar
Copy link
Copy Markdown
Contributor Author

iamr-kumar commented Jun 18, 2023

Updated the 404 page

@Udit-takkar
Copy link
Copy Markdown
Contributor

Thanks for the input. Also could you help me with how to get the strings translated? Or is it done by the maintainers?

You just have to create string in english common/en.json file and use it like t("page_does_not_exist") in the code.

@iamr-kumar
Copy link
Copy Markdown
Contributor Author

Thanks for the input. Also could you help me with how to get the strings translated? Or is it done by the maintainers?

You just have to create string in english common/en.json file and use it like t("page_does_not_exist") in the code.

My point was if the translation to other languages is automated or not. But thanks anyway. I didn't need to create any new string as found some existing ones that fit the use case :)

@iamr-kumar iamr-kumar requested a review from Udit-takkar June 18, 2023 18:59
Comment thread apps/web/pages/404.tsx Outdated
Signed-off-by: Udit Takkar <udit.07814802719@cse.mait.ac.in>
Copy link
Copy Markdown
Contributor

@Udit-takkar Udit-takkar left a comment

Choose a reason for hiding this comment

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

LGTM. works fine

Copy link
Copy Markdown
Member

@sean-brydon sean-brydon left a comment

Choose a reason for hiding this comment

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

thank you! Nice work

@sean-brydon sean-brydon merged commit f469a9c into calcom:main Jun 19, 2023
Comment thread apps/web/pages/404.tsx
const isCalcom = process.env.NEXT_PUBLIC_WEBAPP_URL === "https://app.cal.com";

// In case of invalid email verification token, we intentionally redirect to 404 from the API
const isInvalidToken = router.asPath === "/404";
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@sean-brydon @Udit-takkar this is causing other 404 pages to show as invalid token when it's not the case. Will revert this PR.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@zomars could you specify which pages (or pattern) are getting affected, as isInvalidToken should only be true when intentionally redirecting to 404, which I don't see being done from anywhere else.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🐛 bug Something isn't working ❗️ .env changes contains changes to env variables 🧹 Improvements Improvements to existing features. Mostly UX/UI Low priority Created by Linear-GitHub Sync ❗️ migrations contains migration files

Projects

No open projects
Status: No status

Development

Successfully merging this pull request may close these issues.

Add a page for invalid token while verifying email.

4 participants