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

[FH-2] Figma Oauth Implementation (#201) #206

Merged
merged 8 commits into from
Oct 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .env.example
Original file line number Diff line number Diff line change
Expand Up @@ -35,3 +35,8 @@ ARBSCAN_API_KEY=
FANTOMSCAN_API_KEY=
POLYGONSCAN_API_KEY=
BSCSCAN_API_KEY=

# Figma App for OAuth
# Create these here: https://www.figma.com/developers/apps
FIGMA_CLIENT_ID=
FIGMA_CLIENT_SECRET=
88 changes: 88 additions & 0 deletions app/api/auth/figma/callback/route.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
import { type NextRequest, NextResponse } from 'next/server';

/**
* Figma OAuth callback
* See: https://www.figma.com/developers/api#oauth2
*/
export async function GET(request: NextRequest) {
const { searchParams } = new URL(request.url);
const { FIGMA_CLIENT_ID, FIGMA_CLIENT_SECRET, NEXT_PUBLIC_HOST } = process.env;

Comment on lines +9 to +10
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add validation for essential environment variables.

To prevent runtime errors and enhance error handling, ensure that FIGMA_CLIENT_ID, FIGMA_CLIENT_SECRET, and NEXT_PUBLIC_HOST are defined before proceeding.

Apply this diff to add validation:

+    if (!FIGMA_CLIENT_ID || !FIGMA_CLIENT_SECRET || !NEXT_PUBLIC_HOST) {
+        return NextResponse.json({ error: 'Missing environment variables' }, { status: 500 });
+    }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const { FIGMA_CLIENT_ID, FIGMA_CLIENT_SECRET, NEXT_PUBLIC_HOST } = process.env;
const { FIGMA_CLIENT_ID, FIGMA_CLIENT_SECRET, NEXT_PUBLIC_HOST } = process.env;
if (!FIGMA_CLIENT_ID || !FIGMA_CLIENT_SECRET || !NEXT_PUBLIC_HOST) {
return NextResponse.json({ error: 'Missing environment variables' }, { status: 500 });
}

// biome-ignore lint/complexity/useSimplifiedLogicExpression: <explanation>
if (!FIGMA_CLIENT_ID || !FIGMA_CLIENT_SECRET || !NEXT_PUBLIC_HOST) {
return NextResponse.json({ error: 'Missing environment variables' }, { status: 500 });
}

// Retrieve the stored state from the cookie and verify the CSRF token
const stateParam = searchParams.get('state');
const storedState = request.cookies.get('oauth_state')?.value;
// biome-ignore lint/complexity/useSimplifiedLogicExpression: <explanation>
if (!stateParam || !storedState) {
return NextResponse.json({ error: 'Missing state parameter' }, { status: 500 });
}
if (storedState !== stateParam) {
return NextResponse.json({ error: 'State mismatch. Possible CSRF attack.' }, { status: 500 });
}

// Get the original page URL from the state
let state;
try {
state = JSON.parse(decodeURIComponent(stateParam!));
} catch {
return NextResponse.json({ error: 'Invalid state parameter' }, { status: 400 });
}
Comment on lines +30 to +33
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Handle potential JSON parsing errors.

Wrap the JSON parsing in a try-catch block to handle any potential errors during decoding or parsing of the stateParam.

Apply this diff to enhance error handling:

     let state;
     try {
         state = JSON.parse(decodeURIComponent(stateParam!));
+    } catch (error) {
+        return NextResponse.json({ error: 'Invalid state parameter' }, { status: 400 });
     }

Committable suggestion was skipped due to low confidence.


// Exchange the authorization code for an access token
const code = searchParams.get('code');
const tokenUrl = 'https://api.figma.com/v1/oauth/token';
Comment on lines +36 to +37
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Validate the presence of the authorization code.

Ensure that the code parameter is present before attempting to exchange it for a token.

Apply this diff to add validation:

     const code = searchParams.get('code');
+    if (!code) {
+        return NextResponse.json({ error: 'Missing authorization code' }, { status: 400 });
+    }
     const tokenUrl = 'https://api.figma.com/v1/oauth/token';
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const code = searchParams.get('code');
const tokenUrl = 'https://api.figma.com/v1/oauth/token';
const code = searchParams.get('code');
if (!code) {
return NextResponse.json({ error: 'Missing authorization code' }, { status: 400 });
}
const tokenUrl = 'https://api.figma.com/v1/oauth/token';

const redirectUri = `${NEXT_PUBLIC_HOST}/api/auth/figma/callback`;

const body = new URLSearchParams({
client_id: FIGMA_CLIENT_ID as string,
client_secret: FIGMA_CLIENT_SECRET as string,
redirect_uri: redirectUri,
code: code as string,
grant_type: 'authorization_code',
});

const tokenResponse = await fetch(tokenUrl, {
method: 'POST',
headers: { 'Content-Type': 'application/x-www-form-urlencoded' },
body: body.toString(),
});

const tokenData = await tokenResponse.json();

if (!tokenResponse.ok) {
return NextResponse.json({ error: 'Failed to exchange authorization code for token' }, { status: 400 });
}

Comment on lines +54 to +59
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Validate token data before usage.

Ensure that the tokenData contains all required properties before proceeding to set cookies.

Apply this diff to add validation:

     const tokenData = await tokenResponse.json();

     if (!tokenResponse.ok) {
         return NextResponse.json({ error: 'Failed to exchange authorization code for token' }, { status: 400 });
     }
+    if (!tokenData.access_token || !tokenData.refresh_token || !tokenData.expires_in || !tokenData.user_id) {
+        return NextResponse.json({ error: 'Incomplete token data received from Figma' }, { status: 500 });
+    }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const tokenData = await tokenResponse.json();
if (!tokenResponse.ok) {
return NextResponse.json({ error: 'Failed to exchange authorization code for token' }, { status: 400 });
}
const tokenData = await tokenResponse.json();
if (!tokenResponse.ok) {
return NextResponse.json({ error: 'Failed to exchange authorization code for token' }, { status: 400 });
}
if (!tokenData.access_token || !tokenData.refresh_token || !tokenData.expires_in || !tokenData.user_id) {
return NextResponse.json({ error: 'Incomplete token data received from Figma' }, { status: 500 });
}

const response = NextResponse.redirect(state.originalUrl);
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Validate redirect URL to prevent open redirects.

Ensure that state.originalUrl is a trusted URL within your application's domain to prevent open redirect vulnerabilities.

Apply this diff to validate the redirect URL:

     const response = NextResponse.redirect(state.originalUrl);
+    const allowedHosts = [NEXT_PUBLIC_HOST];
+    const redirectUrl = new URL(state.originalUrl);
+    if (!allowedHosts.includes(redirectUrl.origin)) {
+        return NextResponse.json({ error: 'Invalid redirect URL' }, { status: 400 });
+    }

Committable suggestion was skipped due to low confidence.

response.cookies.set('figma_access_token', tokenData.access_token, {
httpOnly: true,
secure: true,
sameSite: 'lax',
path: '/',
});
response.cookies.set('figma_refresh_token', tokenData.refresh_token, {
httpOnly: true,
secure: true,
sameSite: 'lax',
path: '/',
});
response.cookies.set('figma_user_id', tokenData.user_id.toString(), {
httpOnly: true,
secure: true,
sameSite: 'lax',
path: '/',
});
const expiresAt = Date.now() + tokenData.expires_in * 1000;
response.cookies.set('figma_token_expires_at', expiresAt.toString(), {
httpOnly: true,
secure: true,
sameSite: 'lax',
path: '/',
});
Comment on lines +61 to +85
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Set cookie expiration using 'maxAge' for better control.

Instead of storing the token's expiration time in a separate cookie (figma_token_expires_at), consider using the maxAge option to set the expiration directly on the cookies.

Apply this diff to utilize maxAge:

     response.cookies.set('figma_access_token', tokenData.access_token, {
         httpOnly: true,
         secure: true,
         sameSite: 'lax',
         path: '/',
+        maxAge: tokenData.expires_in,
     });
     response.cookies.set('figma_refresh_token', tokenData.refresh_token, {
         httpOnly: true,
         secure: true,
         sameSite: 'lax',
         path: '/',
+        maxAge: /* appropriate max age for refresh token */,
     });
-    response.cookies.set('figma_token_expires_at', expiresAt.toString(), {
-        httpOnly: true,
-        secure: true,
-        sameSite: 'lax',
-        path: '/',
-    });

Ensure that maxAge is set appropriately for each token type, and remove the unnecessary figma_token_expires_at cookie.

Committable suggestion was skipped due to low confidence.


return response;
}
37 changes: 37 additions & 0 deletions app/api/auth/figma/signin/route.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import { type NextRequest, NextResponse } from 'next/server';

/**
* Figma OAuth signin redirect
* See: https://www.figma.com/developers/api#oauth2
*/
export async function GET(request: NextRequest) {
const { FIGMA_CLIENT_ID, NEXT_PUBLIC_HOST } = process.env;

// biome-ignore lint/complexity/useSimplifiedLogicExpression: <explanation>
if (!FIGMA_CLIENT_ID || !NEXT_PUBLIC_HOST) {
Comment on lines +10 to +11
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Provide an explanation for ignoring lint rule

The comment on line 10 is missing an explanation for why the linter rule is being ignored.

// biome-ignore lint/complexity/useSimplifiedLogicExpression: <explanation>

Please provide a meaningful explanation to help future maintainers understand the reasoning.

Apply this diff to include the explanation:

-// biome-ignore lint/complexity/useSimplifiedLogicExpression: <explanation>
+// biome-ignore lint/complexity/useSimplifiedLogicExpression: Both environment variables are required, and the current logic enhances readability.

return NextResponse.json({ error: 'Missing environment variables' }, { status: 500 });
}

// Get the original page URL from query params or referrer header
const originalUrl = request.nextUrl.searchParams.get('original_url');

// URL redirection attacks
if (!originalUrl?.startsWith(NEXT_PUBLIC_HOST)) {
return NextResponse.json({ error: 'Invalid redirect URL' }, { status: 400 });
}

const redirectUri = `${NEXT_PUBLIC_HOST}/api/auth/figma/callback`;
const state = JSON.stringify({
csrfToken: Math.random().toString(36).substring(2),
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Use a cryptographically secure method for generating CSRF tokens

Using Math.random() is not suitable for generating CSRF tokens because it is not cryptographically secure. This could lead to predictable tokens and potential security vulnerabilities.

Apply this diff:

+import { randomBytes } from 'crypto';

...

-        csrfToken: Math.random().toString(36).substring(2),
+        csrfToken: randomBytes(32).toString('hex'),

Committable suggestion was skipped due to low confidence.

originalUrl: originalUrl,
});

const response = NextResponse.redirect(
`https://www.figma.com/oauth?client_id=${FIGMA_CLIENT_ID}&redirect_uri=${encodeURIComponent(redirectUri)}&scope=files:read&response_type=code&state=${encodeURIComponent(state)}`
);

// Set CSRF token in a cookie (optional for added security)
response.cookies.set('oauth_state', state, { httpOnly: true, secure: true, sameSite: 'none', path: '/' });

return response;
}
16 changes: 16 additions & 0 deletions app/api/auth/figma/signout/route.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import { NextResponse } from 'next/server';

/**
* Figma signout
*/
export async function POST() {
const response = NextResponse.json({ message: 'Signout successful' });

// Clear Figma-related cookies
response.cookies.delete('figma_access_token');
response.cookies.delete('figma_refresh_token');
response.cookies.delete('figma_user_id');
response.cookies.delete('figma_token_expires_at');

return response;
}
16 changes: 16 additions & 0 deletions app/api/auth/figma/status/route.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import { type NextRequest, NextResponse } from 'next/server';

/**
* Returns the current Figma access token for the user.
*
* We use httpOnly cookies to prevent XSS, token leaks and request forgery.
*/
export async function GET(request: NextRequest) {
const figmaAccessToken = request.cookies.get('figma_access_token')?.value;

if (figmaAccessToken) {
return NextResponse.json({ accessToken: figmaAccessToken });
}

return NextResponse.json({ accessToken: null });
}
Comment on lines +8 to +16
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Enhance security with token validation.

The use of httpOnly cookies and avoiding token exposure in URLs are good security practices. However, consider adding token validation before returning it to the client.

Add a simple validation check for the token's format:

 export async function GET(request: NextRequest) {
     const figmaAccessToken = request.cookies.get('figma_access_token')?.value;
 
-    if (figmaAccessToken) {
+    if (figmaAccessToken && typeof figmaAccessToken === 'string' && figmaAccessToken.trim() !== '') {
         return NextResponse.json({ accessToken: figmaAccessToken });
     }
 
     return NextResponse.json({ accessToken: null });
 }

This change ensures that only non-empty string tokens are returned, providing an additional layer of security.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export async function GET(request: NextRequest) {
const figmaAccessToken = request.cookies.get('figma_access_token')?.value;
if (figmaAccessToken) {
return NextResponse.json({ accessToken: figmaAccessToken });
}
return NextResponse.json({ accessToken: null });
}
export async function GET(request: NextRequest) {
const figmaAccessToken = request.cookies.get('figma_access_token')?.value;
if (figmaAccessToken && typeof figmaAccessToken === 'string' && figmaAccessToken.trim() !== '') {
return NextResponse.json({ accessToken: figmaAccessToken });
}
return NextResponse.json({ accessToken: null });
}

1 change: 0 additions & 1 deletion templates/figma/Config.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import type { FigmaTextLayer } from './utils/FigmaApi'

export interface FramePressConfig {
figmaPAT: string
slides: SlideConfig[]
nextSlideId: number
}
Expand Down
63 changes: 21 additions & 42 deletions templates/figma/Inspector.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,23 +6,21 @@ import { Configuration } from '@/sdk/inspector'
import {
ArrowBigLeftDashIcon,
ArrowBigRightDashIcon,
KeySquareIcon,
Trash2Icon,
} from 'lucide-react'
import { useEffect, useMemo, useRef, useState } from 'react'
import { useLocalStorage } from 'usehooks-ts'
import type { FramePressConfig, SlideConfig, TextLayerConfigs } from './Config'
import FigmaTokenEditor from './components/FigmaTokenEditor'
import SlideEditor from './components/SlideEditor'
import { DEFAULT_SLIDES, INITIAL_BUTTONS } from './constants'
import FontConfig from './utils/FontConfig'
import { FigmaView } from './views/FigmaView'
import { FigmaTokenProvider } from './components/FigmaTokenContext'
import FigmaConnector from './components/FigmaConnector'

export default function Inspector() {
const [config, updateConfig] = useFrameConfig<FramePressConfig>()
const [_, setPreviewData] = useFramePreview()

const [editingFigmaPAT, setEditingFigmaPAT] = useState(config.figmaPAT === undefined)
const [selectedSlideIndex, setSelectedSlideIndex] = useState(0)

const [figmaUnderstood, setFigmaUnderstood] = useLocalStorage('figmaUnderstood', false)
Expand All @@ -36,18 +34,11 @@ export default function Inspector() {
buttonIndex: 0,
inputText: '',
params: `slideId=${id}`,
postUrl: undefined
})
}

// Configuration updates
function updateFigmaPAT(updatedPAT: string) {
console.debug('Inspector::updateFigmaPAT()')
setEditingFigmaPAT(false)
updateConfig({
figmaPAT: updatedPAT,
})
}

function updateSlide(updatedSlide: SlideConfig) {
console.debug(`Inspector::updateSlide(${updatedSlide.id})`)

Expand Down Expand Up @@ -126,6 +117,7 @@ export default function Inspector() {
}

// Must run after rendering as it modifies the document <head>
// biome-ignore lint/correctness/useExhaustiveDependencies: circular rulese
useEffect(() => {
if (!config.slides) return
for (const slide of config.slides) {
Expand All @@ -137,7 +129,7 @@ export default function Inspector() {
}
}
}
}, [config.slides, identifyFontsUsed])
}, [config.slides])

// Setup default slides if this is a new instance
useEffect(() => {
Expand Down Expand Up @@ -168,23 +160,12 @@ export default function Inspector() {
const canDelete = config.slides?.length != 1 // must always be one slide visible

return (
<Configuration.Root>
<Configuration.Section title="PAT">
{editingFigmaPAT ? (
<FigmaTokenEditor
figmaPAT={config.figmaPAT}
onChange={updateFigmaPAT}
onCancel={() => setEditingFigmaPAT(false)}
/>
) : (
<Button onClick={() => setEditingFigmaPAT(true)} variant={'secondary'}>
<KeySquareIcon className="mr-1" />
Figma PAT
</Button>
)}
</Configuration.Section>
<FigmaTokenProvider>
<Configuration.Root>
<Configuration.Section title="Figma Login">
<FigmaConnector />
</Configuration.Section>

{!editingFigmaPAT ? (
<Configuration.Section title="Figma Designs">
<div className="w-full flex items-center justify-between">
<div className="flex flex-row items-center justify-end gap-2">
Expand All @@ -211,11 +192,10 @@ export default function Inspector() {
setSelectedSlideIndex(index)
previewSlide(slideConfig.id)
}}
className={`w-40 h-40 flex items-center justify-center mr-1 border-[1px] rounded-md cursor-pointer select-none ${
selectedSlideIndex === index
? 'border-highlight'
: 'border-input'
}`}
className={`w-40 h-40 flex items-center justify-center mr-1 border-[1px] rounded-md cursor-pointer select-none ${selectedSlideIndex === index
? 'border-highlight'
: 'border-input'
}`}
>
<div
style={{
Expand All @@ -226,11 +206,11 @@ export default function Inspector() {
// Handle the case where no image has been configured but we need a min-width
...(!slideConfig.baseImagePaths
? {
'width':
slideConfig.aspectRatio == '1:1'
? dimensionsForRatio['1/1'].width
: dimensionsForRatio['1.91/1'].height,
}
'width':
slideConfig.aspectRatio == '1:1'
? dimensionsForRatio['1/1'].width
: dimensionsForRatio['1.91/1'].height,
}
: {}),
'overflow': 'clip',
}}
Expand Down Expand Up @@ -296,14 +276,13 @@ export default function Inspector() {
<SlideEditor
key={config.slides[selectedSlideIndex].id}
slideConfig={config.slides[selectedSlideIndex]}
figmaPAT={config.figmaPAT}
buttonTargets={buttonTargets}
onUpdate={(updatedSlideConfig) => updateSlide(updatedSlideConfig)}
Comment on lines 277 to 280
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Update SlideEditor props to use figmaAccessToken

The AI summary mentioned that figmaPAT references were updated to figmaAccessToken, but this change is not reflected in the SlideEditor component. Please update the SlideEditor component to use figmaAccessToken instead of figmaPAT to maintain consistency with the Figma OAuth implementation.

Apply this change to the SlideEditor component:

<SlideEditor
    key={config.slides[selectedSlideIndex].id}
    slideConfig={config.slides[selectedSlideIndex]}
+   figmaAccessToken={config.figmaAccessToken}
    buttonTargets={buttonTargets}
    onUpdate={(updatedSlideConfig) => updateSlide(updatedSlideConfig)}
/>

Also, ensure that any other references to figmaPAT in this file or related components are updated to figmaAccessToken.

Committable suggestion was skipped due to low confidence.

/>
)}
</Configuration.Section>
) : undefined}
</Configuration.Root>
</Configuration.Root>
</FigmaTokenProvider>
)
}

Expand Down
50 changes: 50 additions & 0 deletions templates/figma/components/FigmaConnector.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
'use client';
import { Button } from '@/sdk/components';
import { useFigmaToken } from './FigmaTokenContext';
import { useRouter } from 'next/navigation';
import { useState } from 'react';

export default function FigmaConnector() {
const router = useRouter();
const { figmaAccessToken, loading } = useFigmaToken();
const [isConnecting, setIsConnecting] = useState(false); // To track connection state

const handleConnectFigma = () => {
setIsConnecting(true); // Set connecting state

// Change cursor to 'wait'
document.body.style.cursor = 'wait';

const currentPage = window.location.href;
router.push(`/api/auth/figma/signin?original_url=${encodeURIComponent(currentPage)}`);
};

const handleSignout = async () => {
// Change cursor to 'wait'
document.body.style.cursor = 'wait';

await fetch('/api/auth/figma/signout', {
method: 'POST'
});

// Refresh the page
// biome-ignore lint/correctness/noSelfAssign: this is legit
window.location.href = window.location.href;
};

return (
<div>
{loading || isConnecting ? (
<p>Loading...</p>
) : !figmaAccessToken ? (
<Button onClick={handleConnectFigma} variant="primary">
Connect Figma Account
</Button>
) : (
<Button onClick={handleSignout} variant="secondary">
Sign Out
</Button>
)}
</div>
);
}
Loading