Verify user emails by allisonking · Pull Request #1151 · knowledgefutures/pubplatform

allisonking · 2025-04-08T20:57:49Z

Issue(s) Resolved

Closes #1121

High-level Explanation of PR

Adds a verify email flow.

Test Plan

I don't think this will work in the preview environment since we need to use emails.

Happy path:

In a browser that is not signed in, visit a public form i.e. http://localhost:3000/c/croccroc/public/forms/review/fill
You should see the public sign up page. Fill it out
You should get to a page that says you need to verify your email
Open up inbucket at localhost:5432/monitor
You should have gotten an email with the subject 'Verify your email'. Click this link
You should be redirected to the public form, and a toast saying you've verified your email should appear at the bottom.

Other things to test:

Request another verification email. This should invalidate the first one, and you should get a second email with a new token
- You can try using the invalidated link and you will see a token invalid message, request another one
After step 3 above, instead of verifying your email, try to navigate to another part of the app. You should always be sent to the /verify page, but the "Forgot password" flow is still accessible

Screenshots (if applicable)

Screen.Recording.2025-04-10.at.2.49.30.PM.mov

Notes

…dle expired tokens

allisonking · 2025-04-10T18:40:11Z

-		new URL(`/not-found?from=${encodeURIComponent(redirectTo)}`, env.PUBPUB_URL),
-		opts
-	);
+	const url = createRedirectUrl(redirectTo, opts?.searchParams);


Moved this to a helper file since I found I needed it elsewhere (mostly to easily tack on search params). but I think @tefkah 's lil routes lib would be a lot nicer

allisonking · 2025-04-10T18:41:03Z


 	const { user: tokenUser, authTokenType } = tokenSettled.value;

+	if (!tokenUser.isVerified) {


by putting this here in the magic-link route, we automatically mark users who go through this route as verified since they would've come from their email. this lets us get the requirement "Forgot password also functions as verification, in case where user signs up, never completes verification, and deletes verification email" for free

allisonking · 2025-04-10T18:41:21Z

+	};
+
+	return (
+		<SubmitButton


very handy button!!

allisonking · 2025-04-10T18:43:37Z

-							<Toaster />
+							<Suspense>
+								<RootToaster />
+							</Suspense>


hopefully this is okay—I wasn't sure how else to get the toast after redirect to work. it needs Suspense since RootToaster needs to useSearchParams in order to figure out whether to render the 'verified' toast

I think this is correct! Not sure if their are any repercussions from this, but for now it seems fine

allisonking · 2025-04-10T18:46:51Z

 	}

-	redirect(`/c/${communitySlug}/stages`);
+	redirect(createRedirectUrl(`/c/${communitySlug}/stages`, params).toString());


the regular pages weren't passing on search params. I needed it to so that ?verified=true could get passed along to arbitrary pages

tefkah

i think it looks great! very elegant solution of making magic-links set the verified to true.

I made some optional suggestions which don't really need to be implemented if you don't feel like it, but there's one situation we do still need to cover.

if a user is invited through the email action, they are prompted to fill in their information+create a password. they can pick a different email during signup. i think in that case, we should force them to verify their email. this happens here:
https://github.com/pubpub/platform/blob/a54eb9b77fd202346153a294e3775c9dc9eb9bfb/core/lib/authentication/actions.ts#L493-L529

i think we'll get rid of this route as a special case soon, but we should still cover it for now!

greate work!

tefkah · 2025-04-14T14:51:53Z

-							<Toaster />
+							<Suspense>
+								<RootToaster />
+							</Suspense>


I think this is correct! Not sure if their are any repercussions from this, but for now it seems fine

tefkah · 2025-04-14T14:55:49Z

+	const router = useRouter();
+	const searchParams = useSearchParams();
+	const pathname = usePathname();
+
+	useEffect(() => {
+		if (searchParams.has(VERIFIED_PARAM)) {
+			toast({
+				title: "Verified",
+				description: (
+					<span className="flex items-center gap-1">
+						<CircleCheck size="16" /> Your email is now verified
+					</span>
+				),
+				variant: "success",
+			});
+
+			// Remove the param so we don't see the popover again
+			const params = new URLSearchParams(searchParams);
+			params.delete(VERIFIED_PARAM);
+			router.replace(`${pathname}?${params.toString()}`);
+		}
+	}, [searchParams]);


i think it may be nicer to use nuqs here. replacing the VERIFIED_PARAM causes another refresh of all the data on the server, which i don't think is what we want as the effect of the ?verified param is (i think) only relevant client side.

if you use nuqs, it by default uses shallow updating of the searchParams, which don't cause a server refresh. see here for how to do something like that

https://github.com/pubpub/platform/blob/415de673452e7952872c1d341ebfc01f3da8054c/core/app/components/FormBuilder/useIsChanged.tsx#L1-L13

might be nice to just wrap up all of this logic in a hook, eg usePersistedToasts or something, and define all the possible params in some configuration object, eg

const PERSISTED_TOAST = { 'verified': { title: "Verified", description: <span...>, variant: "success" } } as const satisfies ...

tefkah · 2025-04-14T15:02:19Z

 			slug?: string;
 			existing?: false;
+			/**
+			 * @default true


good default

tefkah · 2025-04-14T15:43:21Z

+	return buildSend(async () => {
+		const magicLink = await createMagicLink(
+			{
+				type: AuthTokenType.generic,


could you maybe add a small comment here why this needs to be AuthTokenType.generic vs AuthTokenType.verifyEmail? I think it's bc /magic-link will set their email to be verified, but i think that's easy to forget!

allisonking · 2025-04-14T19:36:29Z

Thanks @tefkah , did not realize the legacy signup needed this too! added, and also implemented using nuqs, great idea!

Screen.Recording.2025-04-14.at.3.35.18.PM.mov

allisonking force-pushed the aking/1121/verify-user-emails branch from e91bbd0 to 6983493 Compare April 10, 2025 01:05

allisonking added 15 commits April 10, 2025 11:49

Add isVerified col to users

edb53e5

Add initial email flow

27c93f2

Add resend verification email button

e4e1805

Kind of handle when an unverified user logs in

fc3e71a

Happy path test

e0e46fc

Use toast to confirm verification

12bc2c3

Make seedCommunity users verified by default

641ebae

Fix type

9d09133

Fix type and remove redirect page

65dd3b2

Wrap toaster in suspense

faa0f9e

Actually, still kind of go through the verify page so that we can han…

a54eb9b

…dle expired tokens

Handle expired tokens

2a01202

More redirect tests

1862549

Make sure seed users are verified

81e6e7b

Separate unverified users

63e97a0

allisonking force-pushed the aking/1121/verify-user-emails branch from 6983493 to 63e97a0 Compare April 10, 2025 16:01

allisonking added 3 commits April 10, 2025 12:52

Fix importing react in email

4afc8a1

Redirect unverified users

e04d409

One more test

9acc9f6

allisonking commented Apr 10, 2025

View reviewed changes

allisonking marked this pull request as ready for review April 10, 2025 18:50

allisonking requested a review from tefkah April 10, 2025 18:50

tefkah added preview Auto-deploys a preview application and removed preview Auto-deploys a preview application labels Apr 14, 2025

tefkah approved these changes Apr 14, 2025

View reviewed changes

allisonking added 5 commits April 14, 2025 12:21

Merge branch 'main' into aking/1121/verify-user-emails

8826baa

Add comment for token type

8f5ac9b

Use nuqs for root toaster

7cacc2b

Add verify email flow to legacy signup

20fa87c

Fix toast types

01f67d2

allisonking merged commit 0dff52f into main Apr 14, 2025
16 checks passed

allisonking deleted the aking/1121/verify-user-emails branch April 14, 2025 19:42


		const { user: tokenUser, authTokenType } = tokenSettled.value;

		if (!tokenUser.isVerified) {

Conversation

allisonking commented Apr 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Issue(s) Resolved

High-level Explanation of PR

Test Plan

Screenshots (if applicable)

Notes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tefkah left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

allisonking commented Apr 14, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

allisonking commented Apr 8, 2025 •

edited

Loading