From 97d2cf9f85393c97fadde7d3ebc38c465338c909 Mon Sep 17 00:00:00 2001 From: Giancarlo Buomprisco Date: Thu, 12 Dec 2024 12:26:50 +0100 Subject: [PATCH] Expired links (#94) 1. Handle expired links on signup 2.Reject invitations when user is already a member 3. Make sure not to display errors due to Next.js redirection during team creation 4. Fix documentation sidebar --- apps/e2e/tests/invitations/invitations.po.ts | 14 ++- .../e2e/tests/invitations/invitations.spec.ts | 25 +++++ .../docs/_components/docs-navigation.tsx | 4 + apps/web/app/auth/callback/error/page.tsx | 70 ++++++++----- apps/web/public/locales/en/auth.json | 6 +- .../cms/keystatic/src/keystatic-client.ts | 6 ++ packages/features/auth/package.json | 3 +- .../src/components/resend-auth-link-form.tsx | 98 ++++++++++++++----- .../components/create-team-account-dialog.tsx | 14 ++- .../services/account-invitations.service.ts | 42 ++++++++ .../supabase/src/auth-callback.service.ts | 57 +++++++++-- 11 files changed, 268 insertions(+), 71 deletions(-) diff --git a/apps/e2e/tests/invitations/invitations.po.ts b/apps/e2e/tests/invitations/invitations.po.ts index 628296f53..9deac3c02 100644 --- a/apps/e2e/tests/invitations/invitations.po.ts +++ b/apps/e2e/tests/invitations/invitations.po.ts @@ -57,11 +57,15 @@ export class InvitationsPageObject { } navigateToMembers() { - return this.page - .locator('a', { - hasText: 'Members', - }) - .click(); + return expect(async () => { + await this.page + .locator('a', { + hasText: 'Members', + }) + .click(); + + return expect(this.page.url()).toContain('members'); + }).toPass() } async openInviteForm() { diff --git a/apps/e2e/tests/invitations/invitations.spec.ts b/apps/e2e/tests/invitations/invitations.spec.ts index 5c36fc4bd..29eb0e694 100644 --- a/apps/e2e/tests/invitations/invitations.spec.ts +++ b/apps/e2e/tests/invitations/invitations.spec.ts @@ -60,6 +60,31 @@ test.describe('Invitations', () => { 'Owner', ); }); + + test('user cannot invite a member of the team again', async ({ page }) => { + await invitations.navigateToMembers(); + + const email = invitations.auth.createRandomEmail(); + + const invites = [ + { + email, + role: 'member', + }, + ]; + + await invitations.openInviteForm(); + await invitations.inviteMembers(invites); + + await expect(invitations.getInvitations()).toHaveCount(1); + + // Try to invite the same member again + // This should fail + await invitations.openInviteForm(); + await invitations.inviteMembers(invites); + await page.waitForTimeout(500); + await expect(invitations.getInvitations()).toHaveCount(1); + }); }); test.describe('Full Invitation Flow', () => { diff --git a/apps/web/app/(marketing)/docs/_components/docs-navigation.tsx b/apps/web/app/(marketing)/docs/_components/docs-navigation.tsx index becaa8991..4fc154745 100644 --- a/apps/web/app/(marketing)/docs/_components/docs-navigation.tsx +++ b/apps/web/app/(marketing)/docs/_components/docs-navigation.tsx @@ -92,6 +92,10 @@ function Tree({ )); } + if (pages.length === 0) { + return null; + } + return ( {pages.map((treeNode, index) => ( diff --git a/apps/web/app/auth/callback/error/page.tsx b/apps/web/app/auth/callback/error/page.tsx index 3b7738dc3..ef7084620 100644 --- a/apps/web/app/auth/callback/error/page.tsx +++ b/apps/web/app/auth/callback/error/page.tsx @@ -1,6 +1,8 @@ import Link from 'next/link'; -import { redirect } from 'next/navigation'; +import type { AuthError } from '@supabase/supabase-js'; + +import { ResendAuthLinkForm } from '@kit/auth/resend-email-link'; import { Alert, AlertDescription, AlertTitle } from '@kit/ui/alert'; import { Button } from '@kit/ui/button'; import { Trans } from '@kit/ui/trans'; @@ -11,41 +13,59 @@ import { withI18n } from '~/lib/i18n/with-i18n'; interface AuthCallbackErrorPageProps { searchParams: Promise<{ error: string; - invite_token: string; + callback?: string; + email?: string; + code?: AuthError['code']; }>; } async function AuthCallbackErrorPage(props: AuthCallbackErrorPageProps) { - const { error, invite_token } = await props.searchParams; - const queryParam = invite_token ? `?invite_token=${invite_token}` : ''; - const signInPath = pathsConfig.auth.signIn + queryParam; - - // if there is no error, redirect the user to the sign-in page - if (!error) { - redirect(signInPath); - } + const { error, callback, code } = await props.searchParams; + const signInPath = pathsConfig.auth.signIn; + const redirectPath = callback ?? pathsConfig.auth.callback; return (
-
- - - - + + + + - - - - -
+ + + + - +
); } +function AuthCallbackForm(props: { + signInPath: string; + redirectPath?: string; + code?: AuthError['code']; +}) { + switch (props.code) { + case 'otp_expired': + return ; + default: + return ; + } +} + +function SignInButton(props: { signInPath: string }) { + return ( + + ); +} + export default withI18n(AuthCallbackErrorPage); diff --git a/apps/web/public/locales/en/auth.json b/apps/web/public/locales/en/auth.json index 3c1a15e6d..6327f66de 100644 --- a/apps/web/public/locales/en/auth.json +++ b/apps/web/public/locales/en/auth.json @@ -52,7 +52,8 @@ "emailConfirmationAlertHeading": "We sent you a confirmation email.", "emailConfirmationAlertBody": "Welcome! Please check your email and click the link to verify your account.", "resendLink": "Resend link", - "resendLinkSuccess": "We sent you a new link to your email! Follow the link to sign in.", + "resendLinkSuccessDescription": "We sent you a new link to your email! Follow the link to sign in.", + "resendLinkSuccess": "Check your email!", "authenticationErrorAlertHeading": "Authentication Error", "authenticationErrorAlertBody": "Sorry, we could not authenticate you. Please try again.", "sendEmailCode": "Get code to your Email", @@ -77,6 +78,7 @@ "minPasswordNumbers": "Password must contain at least one number", "minPasswordSpecialChars": "Password must contain at least one special character", "uppercasePassword": "Password must contain at least one uppercase letter", - "insufficient_aal": "Please sign-in with your current multi-factor authentication to perform this action" + "insufficient_aal": "Please sign-in with your current multi-factor authentication to perform this action", + "otp_expired": "The email link has expired. Please try again." } } diff --git a/packages/cms/keystatic/src/keystatic-client.ts b/packages/cms/keystatic/src/keystatic-client.ts index 77964c9a2..c3a3da8ea 100644 --- a/packages/cms/keystatic/src/keystatic-client.ts +++ b/packages/cms/keystatic/src/keystatic-client.ts @@ -146,6 +146,11 @@ class KeystaticClient implements CmsClient { continue; } + // If the parent is already set, we don't need to do anything + if (item.entry.parent !== null) { + continue; + } + if (isIndexFile(item.slug)) { item.entry.parent = null; results[i] = item; @@ -167,6 +172,7 @@ class KeystaticClient implements CmsClient { item.entry.parent = findClosestValidParent(pathParts); } } + results[i] = item; } diff --git a/packages/features/auth/package.json b/packages/features/auth/package.json index cdea5cba5..98d04a6c6 100644 --- a/packages/features/auth/package.json +++ b/packages/features/auth/package.json @@ -15,7 +15,8 @@ "./shared": "./src/shared.ts", "./mfa": "./src/mfa.ts", "./captcha/client": "./src/captcha/client/index.ts", - "./captcha/server": "./src/captcha/server/index.ts" + "./captcha/server": "./src/captcha/server/index.ts", + "./resend-email-link": "./src/components/resend-auth-link-form.tsx" }, "devDependencies": { "@hookform/resolvers": "^3.9.1", diff --git a/packages/features/auth/src/components/resend-auth-link-form.tsx b/packages/features/auth/src/components/resend-auth-link-form.tsx index df1db861b..96b6f92cc 100644 --- a/packages/features/auth/src/components/resend-auth-link-form.tsx +++ b/packages/features/auth/src/components/resend-auth-link-form.tsx @@ -1,60 +1,105 @@ 'use client'; +import { zodResolver } from '@hookform/resolvers/zod'; import { useMutation } from '@tanstack/react-query'; +import { useForm } from 'react-hook-form'; +import { z } from 'zod'; import { useSupabase } from '@kit/supabase/hooks/use-supabase'; -import { Alert, AlertDescription } from '@kit/ui/alert'; +import { Alert, AlertDescription, AlertTitle } from '@kit/ui/alert'; import { Button } from '@kit/ui/button'; +import { + Form, + FormControl, + FormField, + FormItem, + FormLabel, +} from '@kit/ui/form'; import { Input } from '@kit/ui/input'; -import { Label } from '@kit/ui/label'; import { Trans } from '@kit/ui/trans'; -function ResendAuthLinkForm() { +import { useCaptchaToken } from '../captcha/client'; + +export function ResendAuthLinkForm(props: { + redirectPath?: string; +}) { const resendLink = useResendLink(); + const form = useForm({ + resolver: zodResolver(z.object({ email: z.string().email() })), + defaultValues: { + email: '', + }, + }); + if (resendLink.data && !resendLink.isPending) { return ( + + + + - + ); } return ( -
{ - data.preventDefault(); + + { + return resendLink.mutate({ + email: data.email, + redirectPath: props.redirectPath, + }); + })} + > + { + return ( + + + + - const email = new FormData(data.currentTarget).get('email') as string; + + + + + ); + }} + name={'email'} + /> - return resendLink.mutateAsync(email); - }} - > - - - - + + + ); } -export default ResendAuthLinkForm; - function useResendLink() { const supabase = useSupabase(); + const { captchaToken } = useCaptchaToken(); - const mutationKey = ['resend-link']; - const mutationFn = async (email: string) => { + const mutationFn = async (props: { + email: string; + redirectPath?: string; + }) => { const response = await supabase.auth.resend({ - email, + email: props.email, type: 'signup', + options: { + emailRedirectTo: props.redirectPath, + captchaToken, + }, }); if (response.error) { @@ -65,7 +110,6 @@ function useResendLink() { }; return useMutation({ - mutationKey, mutationFn, }); } diff --git a/packages/features/team-accounts/src/components/create-team-account-dialog.tsx b/packages/features/team-accounts/src/components/create-team-account-dialog.tsx index 0df61e8b0..45a94d827 100644 --- a/packages/features/team-accounts/src/components/create-team-account-dialog.tsx +++ b/packages/features/team-accounts/src/components/create-team-account-dialog.tsx @@ -2,6 +2,8 @@ import { useState, useTransition } from 'react'; +import { isRedirectError } from 'next/dist/client/components/redirect-error'; + import { zodResolver } from '@hookform/resolvers/zod'; import { useForm } from 'react-hook-form'; import { z } from 'zod'; @@ -76,10 +78,16 @@ function CreateOrganizationAccountForm(props: { onClose: () => void }) { data-test={'create-team-form'} onSubmit={form.handleSubmit((data) => { startTransition(async () => { - const { error } = await createTeamAccountAction(data); + try { + const { error } = await createTeamAccountAction(data); - if (error) { - setError(true); + if (error) { + setError(true); + } + } catch (error) { + if (!isRedirectError(error)) { + setError(true); + } } }); })} diff --git a/packages/features/team-accounts/src/server/services/account-invitations.service.ts b/packages/features/team-accounts/src/server/services/account-invitations.service.ts index 87d75a71a..deb1fec67 100644 --- a/packages/features/team-accounts/src/server/services/account-invitations.service.ts +++ b/packages/features/team-accounts/src/server/services/account-invitations.service.ts @@ -101,6 +101,30 @@ class AccountInvitationsService { return data; } + async validateInvitation( + invitation: z.infer['invitations'][number], + accountSlug: string, + ) { + const { data: members, error } = await this.client.rpc( + 'get_account_members', + { + account_slug: accountSlug, + }, + ); + + if (error) { + throw error; + } + + const isUserAlreadyMember = members.find((member) => { + return member.email === invitation.email; + }); + + if (isUserAlreadyMember) { + throw new Error('User already member of the team'); + } + } + /** * @name sendInvitations * @description Sends invitations to join a team. @@ -123,6 +147,24 @@ class AccountInvitationsService { logger.info(ctx, 'Storing invitations...'); + try { + await Promise.all( + invitations.map((invitation) => + this.validateInvitation(invitation, accountSlug), + ), + ); + } catch (error) { + logger.error( + { + ...ctx, + error: (error as Error).message, + }, + 'Error validating invitations', + ); + + throw error; + } + const accountResponse = await this.client .from('accounts') .select('name') diff --git a/packages/supabase/src/auth-callback.service.ts b/packages/supabase/src/auth-callback.service.ts index d2c35ff5c..a1cdef7a0 100644 --- a/packages/supabase/src/auth-callback.service.ts +++ b/packages/supabase/src/auth-callback.service.ts @@ -1,6 +1,9 @@ import 'server-only'; -import { type EmailOtpType, SupabaseClient } from '@supabase/supabase-js'; + + +import {AuthError, type EmailOtpType, SupabaseClient} from '@supabase/supabase-js'; + /** * @name createAuthCallbackService @@ -105,6 +108,14 @@ class AuthCallbackService { if (!error) { return url; } + + if (error.code) { + url.searchParams.set('code', error.code); + } + + const errorMessage = getAuthErrorMessage({ error: error.message, code: error.code }); + + url.searchParams.set('error', errorMessage); } // return the user to an error page with some instructions @@ -163,6 +174,7 @@ class AuthCallbackService { // if we have an error, we redirect to the error page if (error) { return onError({ + code: error.code, error: error.message, path: errorPath, }); @@ -179,6 +191,7 @@ class AuthCallbackService { const message = error instanceof Error ? error.message : error; return onError({ + code: (error as AuthError)?.code, error: message as string, path: errorPath, }); @@ -198,8 +211,16 @@ class AuthCallbackService { } } -function onError({ error, path }: { error: string; path: string }) { - const errorMessage = getAuthErrorMessage(error); +function onError({ + error, + path, + code, +}: { + error: string; + path: string; + code?: string; +}) { + const errorMessage = getAuthErrorMessage({ error, code }); console.error( { @@ -209,7 +230,12 @@ function onError({ error, path }: { error: string; path: string }) { `An error occurred while signing user in`, ); - const nextPath = `${path}?error=${errorMessage}`; + const searchParams = new URLSearchParams({ + error: errorMessage, + code: code ?? '', + }); + + const nextPath = `${path}?${searchParams.toString()}`; return { nextPath, @@ -227,8 +253,23 @@ function isVerifierError(error: string) { return error.includes('both auth code and code verifier should be non-empty'); } -function getAuthErrorMessage(error: string) { - return isVerifierError(error) - ? `auth:errors.codeVerifierMismatch` - : `auth:authenticationErrorAlertBody`; +function getAuthErrorMessage(params: { + error: string; + code?: string; +}) { + // this error arises when the user tries to sign in with an expired email link + if (params.code) { + if (params.code === 'otp_expired') { + return 'auth:errors.otp_expired'; + } + } + + // this error arises when the user is trying to sign in with a different + // browser than the one they used to request the sign in link + if (isVerifierError(params.error)) { + return 'auth:errors.codeVerifierMismatch'; + } + + // fallback to the default error message + return `auth:authenticationErrorAlertBody`; }