From 76b0b493efe9b3704ea4218eeef0eb9df0108371 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Francisco?= <4301103+tomasfrancisco@users.noreply.github.com> Date: Thu, 26 Sep 2024 11:50:53 +0200 Subject: [PATCH] fix figma authentication when browser is not authenticated; add next query string to auth (#46) --- .gitignore | 3 + .../src/app/auth/_actions/auth.action.ts | 2 - .../src/app/auth/_components/auth-card.tsx | 5 +- .../src/app/auth/_components/auth-form.tsx | 5 +- apps/engine/src/app/auth/callback/route.ts | 26 ----- apps/engine/src/lib/middleware/utils.ts | 36 +++--- apps/engine/src/middleware.ts | 9 +- packages/figma-utilities/src/events.ts | 108 +++++++++++++----- packages/figma-widget/src/ui/app.tsx | 2 +- packages/figma-widget/src/ui/config.ts | 2 +- packages/figma-widget/src/ui/modules/auth.tsx | 4 +- .../src/ui/modules/project/project.ui.tsx | 2 +- .../ui/modules/providers/auth-provider.tsx | 9 +- .../modules/providers/projects-provider.tsx | 2 +- .../src/ui/modules/variables.ui.tsx | 4 +- .../src/widget/modules/auth/auth.actions.tsx | 15 ++- .../modules/project/project.actions.tsx | 19 ++- .../modules/variables/variables.actions.ts | 5 +- 18 files changed, 137 insertions(+), 121 deletions(-) delete mode 100644 apps/engine/src/app/auth/callback/route.ts diff --git a/.gitignore b/.gitignore index ec274517..b932f667 100644 --- a/.gitignore +++ b/.gitignore @@ -43,6 +43,9 @@ certificates # monorepo .turbo +# vite timestamp - remove after it's fixed: https://github.com/vitejs/vite/issues/13267 +**/vite.config.ts.timestamp-* + # package specific packages/components/src/components diff --git a/apps/engine/src/app/auth/_actions/auth.action.ts b/apps/engine/src/app/auth/_actions/auth.action.ts index 33450baf..9aa7e2f0 100644 --- a/apps/engine/src/app/auth/_actions/auth.action.ts +++ b/apps/engine/src/app/auth/_actions/auth.action.ts @@ -1,7 +1,6 @@ 'use server'; import { z } from 'zod'; -import { config } from '@/config'; import { unprotectedAction } from '@/lib/safe-action'; export const authAction = unprotectedAction @@ -16,7 +15,6 @@ export const authAction = unprotectedAction email: email, options: { shouldCreateUser: true, - emailRedirectTo: `${config.pageUrl}/auth/callback`, }, }); diff --git a/apps/engine/src/app/auth/_components/auth-card.tsx b/apps/engine/src/app/auth/_components/auth-card.tsx index e0e0b2e2..ecfcfef3 100644 --- a/apps/engine/src/app/auth/_components/auth-card.tsx +++ b/apps/engine/src/app/auth/_components/auth-card.tsx @@ -9,6 +9,7 @@ import { import Image from 'next/image'; import authBackground from '../_assets/auth-bg.svg'; import { AuthForm } from './auth-form'; +import { Suspense } from 'react'; export function AuthCard() { return ( @@ -29,7 +30,9 @@ export function AuthCard() { - + + + ); diff --git a/apps/engine/src/app/auth/_components/auth-form.tsx b/apps/engine/src/app/auth/_components/auth-form.tsx index 9d2c8760..559d8c9a 100644 --- a/apps/engine/src/app/auth/_components/auth-form.tsx +++ b/apps/engine/src/app/auth/_components/auth-form.tsx @@ -23,7 +23,7 @@ import { z } from 'zod'; import { zodResolver } from '@hookform/resolvers/zod'; import { useForm } from 'react-hook-form'; import { useState } from 'react'; -import { useRouter } from 'next/navigation'; +import { useRouter, useSearchParams } from 'next/navigation'; const FormSchema = z.object({ email: z.string().email({ @@ -39,6 +39,7 @@ const FormSchema = z.object({ export const AuthForm = () => { const router = useRouter(); + const searchParams = useSearchParams(); const [loading, setLoading] = useState(false); const [stage, setStage] = useState<'signin' | 'verify'>('signin'); const [error, setError] = useState(); @@ -59,7 +60,7 @@ export const AuthForm = () => { }); if (result?.data?.ok) { - router.replace('/app'); + router.replace(searchParams.get('next') ?? '/app'); } if (result?.data?.error) { diff --git a/apps/engine/src/app/auth/callback/route.ts b/apps/engine/src/app/auth/callback/route.ts deleted file mode 100644 index fa6adae5..00000000 --- a/apps/engine/src/app/auth/callback/route.ts +++ /dev/null @@ -1,26 +0,0 @@ -import { createServerClient } from '@ds-project/auth/server'; -import type { Database } from '@ds-project/database'; -import { NextResponse } from 'next/server'; - -export async function GET(request: Request) { - const { searchParams, origin } = new URL(request.url); - const code = searchParams.get('code'); - // if "next" is in param, use it as the redirect URL - const next = searchParams.get('next') ?? '/app'; - - if (code) { - const supabase = createServerClient(); - const { error, data } = await supabase.auth.exchangeCodeForSession(code); - const accessToken = data.session?.access_token; - if (!accessToken) { - return NextResponse.redirect(`${origin}/auth/error`); - } - - if (!error) { - return NextResponse.redirect(`${origin}${next}`); - } - } - - // return the user to an error page with instructions - return NextResponse.redirect(`${origin}/auth/error`); -} diff --git a/apps/engine/src/lib/middleware/utils.ts b/apps/engine/src/lib/middleware/utils.ts index 1ebee7cf..4ff6cf9e 100644 --- a/apps/engine/src/lib/middleware/utils.ts +++ b/apps/engine/src/lib/middleware/utils.ts @@ -17,12 +17,7 @@ export const getUrlFromResponse = ( }; export const isAuthPath = (url: URL): boolean => { - return ( - url.pathname === '/' || - url.pathname.startsWith('/auth/callback') || - url.pathname.startsWith('/auth/sign-in') || - url.pathname.startsWith('/auth/auth') - ); + return url.pathname === '/' || url.pathname.startsWith('/auth/sign-in'); }; export const isAuthenticatedPath = (url: URL): boolean => { @@ -36,17 +31,15 @@ export const isFigmaAuthPath = (url: URL): boolean => { ); }; -export const hasOnGoingFigmaAuth = (request: NextRequest): boolean => { - return request.cookies.has(config.FIGMA_COOKIE_KEY); +export const hasOnGoingFigmaAuth = (url: URL): boolean => { + return url.searchParams.has(config.FIGMA_QUERY_KEY); }; export const handleFigmaAuth = async ({ - request, response, supabase, url, }: { - request: NextRequest; response: NextResponse; url: URL; supabase: SupabaseClient; @@ -65,36 +58,33 @@ export const handleFigmaAuth = async ({ url, user, supabase, - request, }); } else { - response.cookies.set(config.FIGMA_COOKIE_KEY, figmaKey, { - maxAge: 5 * 60, - expires: 5 * 60 * 1000, - }); url.pathname = '/auth/sign-in'; - url.search = ''; + // Encode the next url which includes the figma key sign up page to return to after authentication + url.search = `?next=${encodeURI(`${url.pathname}${url.search}`)}`; return NextResponse.redirect(url, { ...response, status: 307 }); } }; export const exchangeApiKey = async ({ - request, response, supabase, url, user, }: { - request: NextRequest; response: NextResponse; url: URL; user: User; supabase: SupabaseClient; }) => { - const figmaKey = - request.cookies.get(config.FIGMA_COOKIE_KEY)?.value ?? - url.searchParams.get(config.FIGMA_QUERY_KEY); - const keyValue = await kv.getdel(figmaKey ?? ''); + const figmaKey = url.searchParams.get(config.FIGMA_QUERY_KEY); + + if (!figmaKey) { + return response; + } + + const keyValue = await kv.getdel(figmaKey); if (!keyValue) { return response; @@ -123,8 +113,8 @@ export const exchangeApiKey = async ({ return response; } - response.cookies.delete(config.FIGMA_COOKIE_KEY); url.pathname = '/auth/success'; + // Remove the figma key from the url and any other query strings url.search = ''; return NextResponse.redirect(url, { ...response, status: 307 }); }; diff --git a/apps/engine/src/middleware.ts b/apps/engine/src/middleware.ts index 73007e4a..be1485a4 100644 --- a/apps/engine/src/middleware.ts +++ b/apps/engine/src/middleware.ts @@ -34,10 +34,10 @@ export async function middleware(request: NextRequest) { // Figma middleware logic if (isFigmaAuthPath(url)) { - return handleFigmaAuth({ request, response, url, supabase }); + return handleFigmaAuth({ response, url, supabase }); } - if (hasOnGoingFigmaAuth(request)) { + if (hasOnGoingFigmaAuth(url)) { const { data: { user }, } = await supabase.auth.getUser(); @@ -47,7 +47,6 @@ export async function middleware(request: NextRequest) { url, user, supabase, - request, }); } } @@ -58,12 +57,14 @@ export async function middleware(request: NextRequest) { } = await supabase.auth.getUser(); if (!user && !isAuthPath(url) && isAuthenticatedPath(url)) { + // Encode the next url and redirect the user to the sign-in page + url.search = `?next=${encodeURI(`${url.pathname}${url.search}`)}`; url.pathname = '/auth/sign-in'; return NextResponse.redirect(url, { ...response, status: 307 }); } if (user && url.pathname.startsWith('/auth/sign-in')) { - url.pathname = '/app'; + url.pathname = url.searchParams.get('next') ?? '/app'; return NextResponse.redirect(url, { ...response, status: 307 }); } diff --git a/packages/figma-utilities/src/events.ts b/packages/figma-utilities/src/events.ts index fbef0bb0..1fc11e8e 100644 --- a/packages/figma-utilities/src/events.ts +++ b/packages/figma-utilities/src/events.ts @@ -6,6 +6,8 @@ import { import type { DesignTokens } from 'style-dictionary/types'; import type { Credentials } from './credentials'; +const DEFAULT_EVENT_TIMEOUT = 10 * 1000; // 10 seconds + type RequestResponse = Record< string, { @@ -63,60 +65,108 @@ export function on( name: Name, handler: ResponseHandler ) { - console.log(`🔁 on ${name}`); - return defaultOn(name, handler); + console.log(`[on] waiting for event '${name}'`); + const cancelOn = defaultOn(name, (data: Event[Name]['response']) => { + handler(data); + }); + + return () => { + console.log(`[on] cancelling event '${name}'`); + cancelOn(); + }; } export function once( name: Name, handler: ResponseHandler ) { - console.log(`🔁 once ${name}`); - return defaultOnce(name, handler); + console.log(`[once] waiting for event '${name}'`); + const cancelOnce = defaultOnce(name, (data: Event[Name]['response']) => { + handler(data); + }); + + return () => { + console.log(`[once] cancelling event '${name}'`); + cancelOnce(); + }; } export function emit( name: Name, data: Event[Name]['response'] ) { - console.log(`🚀 emit ${name}`); + console.log(`[emit] sending event '${name}'`); return defaultEmit(name, data); } -export async function request( +export function request( name: Name, - data: Event[Name]['request'] + data: Event[Name]['request'], + handler: ResponseHandler, + options: { timeout: number; onCanceled?: () => void } = { + timeout: DEFAULT_EVENT_TIMEOUT, + } ) { - console.log(`✈️ request ${name}`); - const response = new Promise((resolve, _reject) => { - console.log(`✈️ request 🚀 emit ${name}`); - defaultEmit(name, data); - - console.log(`✈️ request ⏰ wait for ${name}`); - defaultOn(name, resolve); + console.log(`[request] requesting event '${name}'`); + defaultEmit(name, data); + + console.log(`[request] waiting for event '${name}'`); + const cancelOnce = defaultOnce(name, handler); + + const timeout = setTimeout(() => { + console.log(`[request] event '${name}' timeout`); + cancelOnce(); + options.onCanceled?.(); + }, options.timeout); + + return () => { + console.log(`[request] cancelling event '${name}'`); + clearTimeout(timeout); + cancelOnce(); + options.onCanceled?.(); + }; +} - // TODO: handle timeout to reject the promise +export function requestAsync( + name: Name, + data: Event[Name]['request'], + options: { timeout: number } = { timeout: DEFAULT_EVENT_TIMEOUT } +): Promise { + return new Promise((resolve, reject) => { + request(name, data, resolve, { + ...options, + onCanceled: reject, + }); }); - return response; } -export async function handle( +export function handle( name: Name, handler: ( data: Event[Name]['request'] - ) => Promise | Event[Name]['response'] -): Promise { - const response = await new Promise( - (resolve, _reject) => { - console.log(`✈️ handle 🔁 on ${name}`); - defaultOn(name, (data: Event[Name]['request']) => { - console.log(`✈️ handle 🚀 emit ${name}`); - resolve(handler(data)); + ) => Promise | Event[Name]['response'], + options: { timeout: number } = { timeout: DEFAULT_EVENT_TIMEOUT } +): () => void { + console.log(`[handle] waiting for event '${name}'`); + const cancelOnce = defaultOnce(name, (data: Event[Name]['request']) => { + Promise.resolve(handler(data)) + .then((response) => { + console.log(`[handle] responding to event '${name}'`); + defaultEmit(name, response); + }) + .catch(() => { + console.error(`[handle] error handling event '${name}'`); }); + }); - // TODO: handle timeout to reject the promise - } - ); + const timeout = setTimeout(() => { + console.log(`[handle] event '${name}' timeout`); + cancelOnce(); + }, options.timeout); - return defaultEmit(name, response); + return () => { + console.log(`[handle] canceling event '${name}'`); + clearTimeout(timeout); + cancelOnce(); + }; } diff --git a/packages/figma-widget/src/ui/app.tsx b/packages/figma-widget/src/ui/app.tsx index 249a931d..c697cc6e 100644 --- a/packages/figma-widget/src/ui/app.tsx +++ b/packages/figma-widget/src/ui/app.tsx @@ -8,7 +8,7 @@ import { Container } from './components/container'; export function App() { useEffect(() => { // Announce to the plugin that the UI is ready to receive messages - emit('ui-is-ready', undefined); + return emit('ui-is-ready', undefined); }, []); return ( diff --git a/packages/figma-widget/src/ui/config.ts b/packages/figma-widget/src/ui/config.ts index 46e03f68..d72b6966 100644 --- a/packages/figma-widget/src/ui/config.ts +++ b/packages/figma-widget/src/ui/config.ts @@ -4,7 +4,7 @@ const featureFlags = { export const config = { API_HOST: import.meta.env.VITE_API_HOST, - READ_INTERVAL: 1 * 1000, // 1 seconds + API_KEY_READ_INTERVAL: 2 * 1000, // 2 seconds CREDENTIALS_KEY: 'ds-pro__credentials', PROJECT_ID_KEY: 'ds-pro__id', features: featureFlags, diff --git a/packages/figma-widget/src/ui/modules/auth.tsx b/packages/figma-widget/src/ui/modules/auth.tsx index f535a603..606cc074 100644 --- a/packages/figma-widget/src/ui/modules/auth.tsx +++ b/packages/figma-widget/src/ui/modules/auth.tsx @@ -8,15 +8,13 @@ export function Auth() { const { login } = useAuth(); useEffect(() => { - handle('connect', async () => { + return handle('connect', async () => { console.log('💅 Auth: Performing login'); const credentials = await login(); return { credentials, }; - }).catch((error) => { - console.error('💅 Auth: Failed to perform login', error); }); }, [login]); diff --git a/packages/figma-widget/src/ui/modules/project/project.ui.tsx b/packages/figma-widget/src/ui/modules/project/project.ui.tsx index 5ddb3245..eab70efc 100644 --- a/packages/figma-widget/src/ui/modules/project/project.ui.tsx +++ b/packages/figma-widget/src/ui/modules/project/project.ui.tsx @@ -13,7 +13,7 @@ export function ProjectUI() { } = useProjects(); useEffect(() => { - once('open-projects-ui', () => { + return once('set-project', () => { setIsVisible(true); }); }, []); diff --git a/packages/figma-widget/src/ui/modules/providers/auth-provider.tsx b/packages/figma-widget/src/ui/modules/providers/auth-provider.tsx index 4218590a..3a802448 100644 --- a/packages/figma-widget/src/ui/modules/providers/auth-provider.tsx +++ b/packages/figma-widget/src/ui/modules/providers/auth-provider.tsx @@ -66,13 +66,12 @@ export function AuthProvider({ children }: { children: React.ReactNode }) { return; } + setShouldUpdatePlugin(false); if (state === 'authorized' && credentials) { - void emit('set-credentials', { credentials }); + return emit('set-credentials', { credentials }); } else if (state === 'unauthorized' && !credentials) { - void emit('set-credentials', { credentials: null }); + return emit('set-credentials', { credentials: null }); } - - setShouldUpdatePlugin(false); }, [credentials, shouldUpdatePlugin, state]); const logout = useCallback(() => { @@ -129,7 +128,7 @@ export function AuthProvider({ children }: { children: React.ReactNode }) { } return; // continue polling - }, config.READ_INTERVAL); + }, config.API_KEY_READ_INTERVAL); }); }, []); diff --git a/packages/figma-widget/src/ui/modules/providers/projects-provider.tsx b/packages/figma-widget/src/ui/modules/providers/projects-provider.tsx index 30d9c968..aa81707e 100644 --- a/packages/figma-widget/src/ui/modules/providers/projects-provider.tsx +++ b/packages/figma-widget/src/ui/modules/providers/projects-provider.tsx @@ -52,7 +52,7 @@ export function ProjectsProvider({ children }: { children: React.ReactNode }) { await linkResource({ projectId, name: fileName }); setSelectedProjectId(projectId); - emit('set-project', { id: projectId, name: linkedProjectName }); + return emit('set-project', { id: projectId, name: linkedProjectName }); } catch (error) { console.error('Error linking project', { error }); } diff --git a/packages/figma-widget/src/ui/modules/variables.ui.tsx b/packages/figma-widget/src/ui/modules/variables.ui.tsx index c53deaee..f001b73d 100644 --- a/packages/figma-widget/src/ui/modules/variables.ui.tsx +++ b/packages/figma-widget/src/ui/modules/variables.ui.tsx @@ -13,7 +13,7 @@ export function VariablesUI() { const { selectedProjectId } = useProjects(); useEffect(() => { - handle('sync-variables', async ({ variables }) => { + return handle('sync-variables', async ({ variables }) => { // Update the design tokens when the variables are synced if (!fileName || !selectedProjectId) { return { @@ -30,8 +30,6 @@ export function VariablesUI() { return { lastSyncedAt: new Date().getTime(), }; - }).catch(() => { - console.error('Error handling sync-variables'); }); // eslint-disable-next-line react-hooks/exhaustive-deps -- TODO: perhaps refactor handle so no more than one listener to the same message type is added }, []); diff --git a/packages/figma-widget/src/widget/modules/auth/auth.actions.tsx b/packages/figma-widget/src/widget/modules/auth/auth.actions.tsx index 0dbe3a97..750c71f2 100644 --- a/packages/figma-widget/src/widget/modules/auth/auth.actions.tsx +++ b/packages/figma-widget/src/widget/modules/auth/auth.actions.tsx @@ -1,4 +1,4 @@ -import { request } from '@ds-project/figma-utilities'; +import { requestAsync } from '@ds-project/figma-utilities'; import { useUI } from '../../hooks/ui'; import { useCleanupSyncedState, useSyncedCredentials } from '../state'; @@ -14,9 +14,16 @@ export function useAuthActions() { const connect = async () => { await open(); - const { credentials } = await request('connect', undefined); - setSyncedCredentials(credentials); - close(); + + try { + const { credentials } = await requestAsync('connect', undefined, { + timeout: 5 * 60 * 1000, // 5 minutes + }); + + setSyncedCredentials(credentials); + } finally { + close(); + } }; return { diff --git a/packages/figma-widget/src/widget/modules/project/project.actions.tsx b/packages/figma-widget/src/widget/modules/project/project.actions.tsx index 263a5522..ba5c6850 100644 --- a/packages/figma-widget/src/widget/modules/project/project.actions.tsx +++ b/packages/figma-widget/src/widget/modules/project/project.actions.tsx @@ -1,4 +1,4 @@ -import { emit, once } from '@ds-project/figma-utilities'; +import { requestAsync } from '@ds-project/figma-utilities'; import { useUI } from '../../hooks/ui'; import { useSyncedLinkedProject } from '../state'; import { useAuthActions } from '../auth/auth.actions'; @@ -12,17 +12,12 @@ export function useProjectActions() { const selectProject = async () => { await open({ visible: true }); - // TODO: because it's a visible ui, we need to wait for the user to select a project. Maybe we could do this differently - await new Promise((resolve) => { - emit('open-projects-ui', undefined); - - once('set-project', (project) => { - console.log('project', project); - setSyncedLinkedProject(project); - close(); - resolve(void 0); - }); - }); + try { + const project = await requestAsync('set-project', undefined); + setSyncedLinkedProject(project); + } finally { + close(); + } }; return { diff --git a/packages/figma-widget/src/widget/modules/variables/variables.actions.ts b/packages/figma-widget/src/widget/modules/variables/variables.actions.ts index 118f2378..2a01f621 100644 --- a/packages/figma-widget/src/widget/modules/variables/variables.actions.ts +++ b/packages/figma-widget/src/widget/modules/variables/variables.actions.ts @@ -1,4 +1,4 @@ -import { request } from '@ds-project/figma-utilities'; +import { requestAsync } from '@ds-project/figma-utilities'; import { useUI } from '../../hooks/ui'; import { useSyncedLastSyncedAt } from '../state'; import { extractDesignTokens } from '../design-tokens/extract-design-tokens'; @@ -17,10 +17,9 @@ export function useVariablesActions() { await open(); - const { lastSyncedAt } = await request('sync-variables', { + const { lastSyncedAt } = await requestAsync('sync-variables', { variables: designTokens, }); - setLastSyncedAt(lastSyncedAt); };