Figma oauth #1339
Figma oauth #1339
Conversation
Tagging OptionsShould a new tag be published when this PR is merged?
|
- Add OAuth callback action with Figma-specific Basic Auth - Add OAuth start loader for authorization redirect - Add whoami loader for user information retrieval - Add OAuth constants for Figma API endpoints and scopes
- Replace FigmaClient class with TypeScript interfaces - Add comprehensive API endpoint definitions with proper typing - Update mod.ts to use createOAuthHttpClient from MCP utils - Remove direct token access in favor of OAuth flow - Update README with OAuth configuration instructions
- Remove old FigmaClient class implementation - Replaced by new interface-based client in utils/client.ts
- Replace ctx.figma methods with direct ctx.client API calls - Update all loaders to handle Response objects and JSON parsing - Add proper error handling and status checking - Align with Google Gmail pattern for consistent API usage
- Add loader to retrieve current user information - Returns user ID, email, handle, and profile image URL - Follows OAuth client pattern for consistent API usage
- Update manifest.gen.ts with new OAuth loaders and actions - Maintain simplifier.ts functionality for API response processing
guitavano
force-pushed
the
figma-oauth
branch
from
5a095ae to
e6294a6
Compare
WalkthroughThe changes introduce OAuth 2.0 for Figma, remove the old Figma client, add OAuth start/callback/whoami flows, switch Figma loaders to a generic HTTP client, add user info loader, update the manifest and README, add shared Figma API types/constants, and apply minor formatting in HubSpot and Apify files. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant App
participant OAuthStart as Loader: oauth/start
participant FigmaAuth as Figma OAuth Auth
participant Callback as Action: oauth/callback
participant TokenAPI as Figma OAuth Token
participant State as App Context
participant Whoami as Loader: oauth/whoami
participant API as Figma API
User->>App: Initiate connect
App->>OAuthStart: Build authorize URL
OAuthStart-->>User: Redirect to FigmaAuth (response_type=code)
User->>FigmaAuth: Grant consent
FigmaAuth-->>Callback: Redirect with code, state
Callback->>TokenAPI: POST /token (code, redirect_uri, Basic auth)
TokenAPI-->>Callback: access_token, refresh_token, expires_in
Callback->>State: Persist tokens (+obtainedAt, client creds)
alt optional user lookup
Callback->>Whoami: Resolve account
Whoami->>API: GET /v1/me
API-->>Whoami: user info
Whoami-->>Callback: handle/email
end
Callback-->>User: Final redirect (installId, account)
sequenceDiagram
autonumber
participant Loader as Loader (e.g., getComponents)
participant HTTP as ctx.client
participant Figma as Figma API
Loader->>HTTP: GET /v1/files/:fileKey (searchParams)
HTTP->>Figma: Request
Figma-->>HTTP: Response (status, body)
alt response.ok
HTTP-->>Loader: status + JSON data
else error
HTTP-->>Loader: err + status + statusText
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
figma/loaders/getSimplifiedNodes.ts (1)
40-46: Type mismatch: document is a SimplifiedNode, not PartialsimplifyNode returns SimplifiedNode | null. The current type will cause TS errors or unsafe casts.
Apply:
-import { simplifyNode } from "../utils/simplifier.ts"; +import { simplifyNode } from "../utils/simplifier.ts"; +import type { SimplifiedNode } from "../utils/simplifier.ts";interface SimplifiedNodeData { - document: Partial<FigmaNode> | null; + document: SimplifiedNode | null; components: Record<string, FigmaComponent>; componentSets: Record<string, FigmaComponentSet>; styles: Record<string, FigmaStyle>; schemaVersion: number; }
🧹 Nitpick comments (17)
figma/README.MD (1)
215-230: Clarify OAuth setup; validate envs in sample; remove stale TODO.
- Add redirect URI and scopes guidance; validate env vars in the sample to avoid undefined values at runtime; drop the “TODO: Implement oAuth” header to prevent confusion.
Apply this diff to harden the sample:
```typescript import { App } from "figma/mod.ts"; -const figmaApp = App({ - clientId: Deno.env.get("FIGMA_CLIENT_ID"), - clientSecret: Deno.env.get("FIGMA_CLIENT_SECRET"), -}); +const clientId = Deno.env.get("FIGMA_CLIENT_ID"); +const clientSecret = Deno.env.get("FIGMA_CLIENT_SECRET"); +if (!clientId || !clientSecret) { + throw new Error("Missing FIGMA_CLIENT_ID/FIGMA_CLIENT_SECRET"); +} +const figmaApp = App({ clientId, clientSecret });Also add a note under the env list to register the app’s redirect URI in Figma and to confirm required scopes (e.g., “file_read”). Please confirm the exact scopes needed by the new loaders and OAuth whoami flow. </blockquote></details> <details> <summary>figma/loaders/getNodes.ts (1)</summary><blockquote> `60-80`: **Add input validation and try/catch for consistent error envelope.** - Guard empty nodeIds and dedupe for stability. - Wrap the HTTP call/JSON parse to avoid throwing and keep FigmaResponse semantics. Apply this diff: ```diff const { fileKey, nodeIds, version, depth, geometry } = props; - const response = await ctx.client["GET /v1/files/:fileKey/nodes"]({ - fileKey, - ids: nodeIds.join(","), - version, - depth, - geometry, - }); - - if (!response.ok) { - return { - err: `HTTP ${response.status}: ${response.statusText}`, - status: response.status, - }; - } - - const data = await response.json(); - - return { - status: response.status, - data: data, - }; + if (!nodeIds?.length) { + return { err: "nodeIds cannot be empty", status: 400 }; + } + const ids = [...new Set(nodeIds)].join(","); + try { + const response = await ctx.client["GET /v1/files/:fileKey/nodes"]({ + fileKey, + ids, + version, + depth, + geometry, + }); + if (!response.ok) { + return { + err: `HTTP ${response.status}: ${response.statusText}`, + status: response.status, + }; + } + const data = await response.json(); + return { status: response.status, data }; + } catch (e) { + return { err: e instanceof Error ? e.message : String(e), status: 500 }; + }figma/loaders/getSimplified.ts (1)
63-80: Wrap HTTP call/JSON parse in try/catch to avoid throwing outside the envelope.Apply this diff:
- const response = await ctx.client["GET /v1/files/:fileKey"]({ - fileKey, - version, - depth, - branch_data, - }); - - if (!response.ok) { - return { - err: `HTTP ${response.status}: ${response.statusText}`, - status: response.status, - }; - } - - const data = await response.json(); + try { + const response = await ctx.client["GET /v1/files/:fileKey"]({ + fileKey, + version, + depth, + branch_data, + }); + if (!response.ok) { + return { + err: `HTTP ${response.status}: ${response.statusText}`, + status: response.status, + }; + } + const data = await response.json(); + // Simplify the data + // (existing simplification code continues below) +``` Follow through by closing the try/catch at the end of the function and returning a { err, status: 500 } on catch. </blockquote></details> <details> <summary>figma/loaders/getImagesSpecificNode.ts (1)</summary><blockquote> `30-47`: **Validate nodeIds and add try/catch.** Ensure non-empty nodeIds and keep the error envelope on fetch/parse errors. Apply this diff: ```diff const { fileKey, nodeIds } = props; - const response = await ctx.client["GET /v1/images/:fileKey"]({ - fileKey, - ids: nodeIds.join(","), - }); - - if (!response.ok) { - return { - err: `HTTP ${response.status}: ${response.statusText}`, - status: response.status, - }; - } - - const data = await response.json(); - - return { - status: response.status, - data: data, - }; + if (!nodeIds?.length) { + return { err: "nodeIds cannot be empty", status: 400 }; + } + const ids = [...new Set(nodeIds)].join(","); + try { + const response = await ctx.client["GET /v1/images/:fileKey"]({ + fileKey, + ids, + }); + if (!response.ok) { + return { + err: `HTTP ${response.status}: ${response.statusText}`, + status: response.status, + }; + } + const data = await response.json(); + return { status: response.status, data }; + } catch (e) { + return { err: e instanceof Error ? e.message : String(e), status: 500 }; + }figma/loaders/getFileImages.ts (1)
93-119: Validate inputs, enforce scale range, and add try/catch.
- Guard empty nodeIds and enforce 0.01–4 scale per Figma docs.
- Keep error envelope on network/parse errors.
Apply this diff:
} = props; - const response = await ctx.client["GET /v1/images/:fileKey"]({ - fileKey, - ids: nodeIds.join(","), - scale, - format, - svg_outline_text, - svg_include_id, - svg_include_node_id, - svg_simplify_stroke, - contents_only, - use_absolute_bounds, - version, - }); - - if (!response.ok) { - return { - err: `HTTP ${response.status}: ${response.statusText}`, - status: response.status, - }; - } - - const data = await response.json(); - - return { - status: response.status, - data: data, - }; + if (!nodeIds?.length) { + return { err: "nodeIds cannot be empty", status: 400 }; + } + if (scale !== undefined && (scale < 0.01 || scale > 4)) { + return { err: "scale must be between 0.01 and 4", status: 400 }; + } + const ids = [...new Set(nodeIds)].join(","); + try { + const response = await ctx.client["GET /v1/images/:fileKey"]({ + fileKey, + ids, + scale, + format, + svg_outline_text, + svg_include_id, + svg_include_node_id, + svg_simplify_stroke, + contents_only, + use_absolute_bounds, + version, + }); + if (!response.ok) { + return { + err: `HTTP ${response.status}: ${response.statusText}`, + status: response.status, + }; + } + const data = await response.json(); + return { status: response.status, data }; + } catch (e) { + return { err: e instanceof Error ? e.message : String(e), status: 500 }; + }figma/loaders/oauth/start.ts (1)
9-21: Add an explicit return type for clarityDeclare the function’s return type as Response.
-export default function start(props: Props) { +export default function start(props: Props): Response {figma/loaders/getUserInfo.ts (2)
30-35: Type the parsed JSON to the expected shapeAnnotate the parsed JSON to catch mismatches at compile-time.
- const data = await response.json(); + const data: { + id: string; + email: string; + handle: string; + img_url: string; + } = await response.json();
37-42: Unify error message languageFor consistency across API responses, prefer English or adopt a single language repo-wide.
- err: `Erro interno: ${ + err: `Internal error: ${ error instanceof Error ? error.message : String(error) }`,figma/loaders/getComponents.ts (1)
56-62: Return shape vs. intent (“only components”)You spread the entire file payload with ...data and then restate specific fields. If the intention is to reduce payload, consider returning a narrower type (e.g., Pick<FigmaFile, "document" | "components" | "componentSets">) and updating the Promise type accordingly. Otherwise, update the comment to reflect that you return the full file.
figma/loaders/oauth/whoami.ts (1)
57-63: Unify error message languageMatch language across loaders for consistent UX/observability.
- throw new Error( - `Erro ao obter informações do usuário: ${ + throw new Error( + `Error fetching user info: ${ error instanceof Error ? error.message : String(error) }`, );figma/loaders/getImagesToFills.ts (1)
42-47: Honor imageRef filter (if provided)You accept imageRef but don’t filter the payload. Filter client-side for smaller responses.
- const data = await response.json(); - - return { - status: response.status, - data: data, - }; + const data = await response.json() as { images: Record<string, string> }; + const refs = props.imageRef ?? []; + const images = refs.length + ? Object.fromEntries(Object.entries(data.images).filter(([ref]) => refs.includes(ref))) + : data.images; + + return { + status: response.status, + data: { images }, + };figma/utils/client.ts (1)
63-83: Minor: snake_case vs camelCase in branchesFigma’s branches fields often come snake_cased; ensure downstream code expects these names. If you plan to camelCase elsewhere, document or transform consistently.
figma/actions/oauth/callback.ts (1)
11-17: Make redirectUri optional (code already provides a fallback)Props marks redirectUri as required but you branch as if it’s optional.
export interface Props { code: string; installId: string; clientId: string; clientSecret: string; - redirectUri: string; + redirectUri?: string; }figma/mod.ts (4)
54-58: Validate presence of client credentials early.Empty defaults can lead to confusing runtime failures at token exchange. Fail fast or source from a secure store.
const figmaProvider: OAuthProvider = { ...FigmaProvider, clientId: clientId ?? "", clientSecret: clientSecret ?? "", }; + + if (!figmaProvider.clientId || !figmaProvider.clientSecret) { + throw new Error("Figma OAuth requires clientId and clientSecret"); + }If
ctxexposes a secret manager (e.g.,ctx.secrets.get/set), prefer loadingclientSecretfrom there.
60-68: Avoid redundant header config; rely on DEFAULT_OAUTH_HEADERS.You already set
headers: DEFAULT_OAUTH_HEADERS. The customauthClientConfig.headersduplicates it and can drift.const options: OAuthClientOptions = { headers: DEFAULT_OAUTH_HEADERS, - authClientConfig: { - headers: new Headers({ - "Accept": "application/json", - "Content-Type": "application/x-www-form-urlencoded", - }), - }, };
39-39: AppContext alias appears unused and potentially misleading.Either use it or remove it to avoid confusion about the expected context shape.
20-27: Exporting a provider with blank secrets can be misused.Consider exporting a factory that injects credentials, or document clearly that
FigmaProvidermust be spread and overridden.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
apify/loaders/getActorRun.ts(1 hunks)figma/README.MD(1 hunks)figma/actions/oauth/callback.ts(1 hunks)figma/client.ts(0 hunks)figma/loaders/getComponents.ts(2 hunks)figma/loaders/getFileImages.ts(3 hunks)figma/loaders/getImagesSpecificNode.ts(2 hunks)figma/loaders/getImagesToFills.ts(2 hunks)figma/loaders/getNodes.ts(2 hunks)figma/loaders/getSimplified.ts(4 hunks)figma/loaders/getSimplifiedNodes.ts(2 hunks)figma/loaders/getUserInfo.ts(1 hunks)figma/loaders/oauth/start.ts(1 hunks)figma/loaders/oauth/whoami.ts(1 hunks)figma/manifest.gen.ts(2 hunks)figma/mod.ts(1 hunks)figma/utils/client.ts(1 hunks)figma/utils/constant.ts(1 hunks)figma/utils/simplifier.ts(1 hunks)hubspot/actions/conversations/sendThreadComment.ts(1 hunks)hubspot/actions/conversations/sendThreadMessage.ts(1 hunks)hubspot/loaders/conversations/getChannelAccounts.ts(1 hunks)
💤 Files with no reviewable changes (1)
- figma/client.ts
🧰 Additional context used
🧬 Code graph analysis (11)
hubspot/loaders/conversations/getChannelAccounts.ts (2)
hubspot/actions/conversations/sendThreadMessage.ts (1)
DeliveryIdentifier(36-48)hubspot/utils/client.ts (1)
HubSpotClient(23-411)
hubspot/actions/conversations/sendThreadComment.ts (2)
hubspot/actions/conversations/sendThreadMessage.ts (7)
Props(82-136)Attachment(4-34)Client(138-141)Sender(143-148)MessageRecipient(150-155)FailureDetails(157-160)MessageStatus(162-165)hubspot/utils/client.ts (1)
HubSpotClient(23-411)
figma/loaders/oauth/start.ts (1)
figma/utils/constant.ts (2)
SCOPES(1-3)OAUTH_URL_AUTH(7-7)
hubspot/actions/conversations/sendThreadMessage.ts (3)
hubspot/loaders/conversations/getChannelAccounts.ts (1)
DeliveryIdentifier(23-26)hubspot/actions/conversations/sendThreadComment.ts (6)
Attachment(4-34)Client(62-65)Sender(67-75)MessageRecipient(77-85)FailureDetails(87-90)MessageStatus(92-95)hubspot/utils/client.ts (1)
HubSpotClient(23-411)
figma/loaders/getImagesToFills.ts (1)
figma/utils/client.ts (1)
FigmaResponse(8-12)
figma/loaders/getSimplifiedNodes.ts (2)
figma/utils/client.ts (4)
FigmaNode(17-30)FigmaComponent(35-40)FigmaComponentSet(45-49)FigmaStyle(54-58)figma/utils/simplifier.ts (1)
simplifyNode(17-124)
figma/loaders/getSimplified.ts (2)
figma/utils/client.ts (2)
FigmaComponent(35-40)FigmaStyle(54-58)figma/utils/simplifier.ts (2)
simplifyStyle(179-195)simplifyDocument(202-205)
figma/actions/oauth/callback.ts (2)
figma/loaders/oauth/start.ts (1)
Props(3-7)figma/loaders/oauth/whoami.ts (1)
Props(3-9)
figma/loaders/getImagesSpecificNode.ts (1)
figma/utils/client.ts (2)
FigmaResponse(8-12)FigmaImagesResponse(101-103)
figma/mod.ts (4)
mcp/utils/types.ts (3)
OAuthProvider(12-21)OAuthTokens(3-10)OAuthClients(39-47)figma/utils/client.ts (2)
FigmaClient(116-171)AuthClient(174-192)figma/manifest.gen.ts (1)
Manifest(37-37)mcp/utils/config.ts (2)
OAuthClientOptions(28-34)DEFAULT_OAUTH_HEADERS(14-23)
figma/loaders/getUserInfo.ts (1)
figma/utils/client.ts (1)
FigmaResponse(8-12)
🔇 Additional comments (20)
hubspot/loaders/conversations/getChannelAccounts.ts (1)
57-98: Looks good to ship.Props typing, query construction, and the defensive response shaping all remain tight. No issues spotted.
hubspot/actions/conversations/sendThreadMessage.ts (1)
197-234: Docs-only adjustments look goodInterface annotations and the request payload wiring stay intact, with no behavioral change.
figma/loaders/getNodes.ts (1)
8-8: Import path update looks correct.figma/loaders/getSimplified.ts (1)
8-8: Import path update looks correct.figma/loaders/getImagesSpecificNode.ts (1)
2-2: Type import and return type update look good.Also applies to: 27-28
figma/loaders/getFileImages.ts (2)
2-2: Import path update looks correct.
79-105: Retain svg_include_node_id parametersvg_include_node_id is supported by Figma’s GET /v1/images endpoint; no rename or removal needed.
figma/loaders/oauth/start.ts (1)
10-16: Verify redirectUri parity with callbackEnsure the exact redirectUri used here matches the one sent to the token endpoint in the callback, otherwise the token exchange will fail.
figma/loaders/getComponents.ts (1)
38-44: OK to switch to HTTP client surfaceThe move to ctx.client with path params looks consistent with other loaders.
figma/loaders/oauth/whoami.ts (1)
31-46: Direct token path looks goodUsing a direct fetch when accessToken is provided is reasonable; headers are correct and you validate response.ok.
figma/loaders/getSimplifiedNodes.ts (2)
64-71: Good: HTTP client usage and paramsids join, optional version/depth/geometry are correctly passed.
72-77: Error envelope standardization looks goodConsistent FigmaResponse error shape with status and message.
figma/loaders/getImagesToFills.ts (1)
25-29: Good: standardized response envelopeSwitch to FigmaResponse wrapper is consistent with other loaders.
figma/utils/client.ts (1)
98-111: Types look comprehensive and align with Figma API responsesThe response shapes cover images and file nodes well.
figma/actions/oauth/callback.ts (1)
72-79: whoami usage: good fallback handlingGraceful degradation to undefined account when whoami fails is acceptable.
Ensure manifest wiring for figma.loaders.oauth.whoami matches this path.
figma/mod.ts (1)
98-98: Re-export of client types — LGTM.Keeps consumer imports tidy.
apify/loaders/getActorRun.ts (1)
49-53: No functional change detectedThanks for tightening up the call formatting; the dataset-items retrieval still hits the same endpoint with the same parameters, so behavior stays intact.
hubspot/actions/conversations/sendThreadComment.ts (1)
128-146: Comment payload construction still solidDouble-checked the payload shape—defaults and guards remain the same, so the request contract with HubSpot is unchanged. All good here.
figma/utils/simplifier.ts (1)
1-6: Import path realigned correctlyThe switch to
./client.tslines up with the new client location inutils/; no type surface changes otherwise. Looks good.figma/manifest.gen.ts (1)
5-32: Manifest entries updated appropriatelyThe new OAuth loader/action bindings are wired into the generated manifest with the expected module paths—this keeps the manifest in sync with the new handlers.
| const finalRedirectUri = redirectUri || | ||
| new URL("/oauth/callback", req.url).href; | ||
|
|
There was a problem hiding this comment.
Security: validate OAuth state
There’s no state verification, which weakens CSRF protections. Validate the state returned on the callback against the one issued in start.
Example patch:
- const finalRedirectUri = redirectUri ||
- new URL("/oauth/callback", req.url).href;
+ const finalRedirectUri = redirectUri ||
+ new URL("/oauth/callback", req.url).href;
+ const stateFromReq = new URL(req.url).searchParams.get("state");
+ // TODO: compare stateFromReq with a previously stored state value
+ // e.g., from props or server-side storage. Abort if mismatch.
+ // if (!stateFromReq || stateFromReq !== expectedState) {
+ // throw new Error("Invalid OAuth state");
+ // }Also consider adding state to Props or retrieving expected state from ctx.
📝 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.
| const finalRedirectUri = redirectUri || | |
| new URL("/oauth/callback", req.url).href; | |
| const finalRedirectUri = redirectUri || | |
| new URL("/oauth/callback", req.url).href; | |
| const stateFromReq = new URL(req.url).searchParams.get("state"); | |
| // TODO: compare stateFromReq with a previously stored state value | |
| // e.g., from props or server-side storage. Abort if mismatch. | |
| // if (!stateFromReq || stateFromReq !== expectedState) { | |
| // throw new Error("Invalid OAuth state"); | |
| // } |
| const response = await client["POST /token"]({}, { | ||
| body: { | ||
| code, | ||
| redirect_uri: finalRedirectUri, | ||
| grant_type: "authorization_code", | ||
| }, | ||
| headers: { | ||
| "Authorization": `Basic ${credentials}`, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
Increase token endpoint compatibility
Include client_id and client_secret in the body and set Accept; some providers require form-like params or both Basic and body credentials.
- const response = await client["POST /token"]({}, {
- body: {
- code,
- redirect_uri: finalRedirectUri,
- grant_type: "authorization_code",
- },
- headers: {
- "Authorization": `Basic ${credentials}`,
- },
- });
+ const response = await client["POST /token"]({}, {
+ body: {
+ code,
+ redirect_uri: finalRedirectUri,
+ grant_type: "authorization_code",
+ client_id: clientId,
+ client_secret: clientSecret,
+ },
+ headers: {
+ "Authorization": `Basic ${credentials}`,
+ "Accept": "application/json",
+ },
+ });📝 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.
| const response = await client["POST /token"]({}, { | |
| body: { | |
| code, | |
| redirect_uri: finalRedirectUri, | |
| grant_type: "authorization_code", | |
| }, | |
| headers: { | |
| "Authorization": `Basic ${credentials}`, | |
| }, | |
| }); | |
| const response = await client["POST /token"]({}, { | |
| body: { | |
| code, | |
| redirect_uri: finalRedirectUri, | |
| grant_type: "authorization_code", | |
| client_id: clientId, | |
| client_secret: clientSecret, | |
| }, | |
| headers: { | |
| "Authorization": `Basic ${credentials}`, | |
| "Accept": "application/json", | |
| }, | |
| }); |
🤖 Prompt for AI Agents
In figma/actions/oauth/callback.ts around lines 37 to 46, the token request only
sends code, redirect_uri and uses Basic auth; to maximize compatibility include
client_id and client_secret in the request body and set appropriate headers. Add
client_id and client_secret to the body payload alongside
code/redirect_uri/grant_type, and set headers to include "Accept":
"application/json" and "Content-Type": "application/x-www-form-urlencoded" (or
serialise the body as form-encoded) while retaining the existing Authorization
header so providers that require both Basic and body credentials are supported.
| const simplifiedDoc = simplifyDocument(data.document); | ||
| if (!simplifiedDoc) { | ||
| throw new Error("Failed to simplify document"); | ||
| } |
There was a problem hiding this comment.
Don’t throw; return a typed error in the FigmaResponse envelope.
Throwing breaks the contract. Return an error instead.
Apply this diff:
- const simplifiedDoc = simplifyDocument(data.document);
- if (!simplifiedDoc) {
- throw new Error("Failed to simplify document");
- }
+ const simplifiedDoc = simplifyDocument(data.document);
+ if (!simplifiedDoc) {
+ return { err: "Failed to simplify document", 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.
| const simplifiedDoc = simplifyDocument(data.document); | |
| if (!simplifiedDoc) { | |
| throw new Error("Failed to simplify document"); | |
| } | |
| const simplifiedDoc = simplifyDocument(data.document); | |
| if (!simplifiedDoc) { | |
| return { err: "Failed to simplify document", status: 500 }; | |
| } |
🤖 Prompt for AI Agents
In figma/loaders/getSimplified.ts around lines 110 to 113, currently the code
throws when simplifyDocument returns falsy; instead of throwing, return a
FigmaResponse-shaped error object: create and return a value that matches the
FigmaResponse<T> envelope (e.g., { success: false, error: { message: "Failed to
simplify document", code: "SIMPLIFY_FAILED" } } or whatever the project's error
shape is), preserving types so the function signature stays valid; remove the
throw and ensure callers handle the returned error envelope.
| status: response.status, | ||
| data: { | ||
| name: response.data.name, | ||
| role: response.data.role, | ||
| lastModified: response.data.lastModified, | ||
| editorType: response.data.editorType, | ||
| thumbnailUrl: response.data.thumbnailUrl, | ||
| version: response.data.version, | ||
| name: data.name, | ||
| role: data.role, | ||
| lastModified: data.lastModified, | ||
| editorType: data.editorType, | ||
| thumbnailUrl: data.thumbnailUrl, | ||
| version: data.version, | ||
| document: simplifiedDoc as FigmaNode, |
There was a problem hiding this comment.
Type mismatch: simplified document is not a FigmaNode.
simplifyDocument returns SimplifiedNode. Casting to FigmaNode is incorrect and leaks wrong types to consumers. Change the return type to use SimplifiedNode and drop the cast.
Apply these changes outside the changed ranges:
// imports
import type { SimplifiedNode } from "../utils/simplifier.ts";
// interface
interface SimplifiedResponse {
name: string;
role: string;
lastModified: string;
editorType: string;
thumbnailUrl: string;
version: string;
document: SimplifiedNode; // was FigmaNode
components: Record<string, FigmaComponent>;
componentSets: Record<string, FigmaComponentSet>;
styles: Record<string, FigmaStyle>;
}
// return shape
return {
status: response.status,
data: {
name: data.name,
role: data.role,
lastModified: data.lastModified,
editorType: data.editorType,
thumbnailUrl: data.thumbnailUrl,
version: data.version,
document: simplifiedDoc, // no cast
components: simplifiedComponents,
componentSets: simplifiedComponentSets,
styles: simplifiedStyles,
},
};🤖 Prompt for AI Agents
In figma/loaders/getSimplified.ts around lines 117 to 125, the code casts
simplifiedDoc to FigmaNode even though simplifyDocument returns SimplifiedNode;
remove the incorrect cast and update types: import type { SimplifiedNode } from
"../utils/simplifier.ts", change the SimplifiedResponse interface to use
document: SimplifiedNode instead of FigmaNode, and return document:
simplifiedDoc (no cast). Ensure the components/componentSets/styles types remain
unchanged and adjust any other references to the response type accordingly.
| const { tokens, clientId, clientSecret } = props; | ||
|
|
There was a problem hiding this comment.
Don’t include tokens/clientSecret in returned state (secrets leak).
state currently spreads props and adds tokens, exposing OAuth tokens and clientSecret. Avoid persisting/seeding secrets into public state; the client.oauth.tokens already carries them for API use.
- const { tokens, clientId, clientSecret } = props;
+ const { tokens, clientId, clientSecret, ...restProps } = props;
@@
- const state: State = {
- ...props,
- tokens,
- client,
- };
+ const state: State = {
+ ...restProps, // exclude tokens and clientSecret from persisted state
+ client,
+ };If you need durable storage, prefer a secure store (e.g., ctx.secrets) over state.
Also applies to: 85-89
🤖 Prompt for AI Agents
In figma/mod.ts around lines 52-53 (and similarly lines 85-89), the function is
spreading props into returned state which leaks secrets (tokens and
clientSecret); remove tokens and clientSecret from any persisted/returned state,
avoid spreading the entire props object that contains secrets, and instead
return only non-secret fields (or explicitly pick allowed fields). Keep using
client.oauth.tokens for runtime API calls but do not persist them; if durable
storage is required move secrets to a secure store (e.g., ctx.secrets or a
secrets manager) and reference them there.
| onTokenRefresh: async (newTokens: OAuthTokens) => { | ||
| if (ctx) { | ||
| await ctx.configure({ | ||
| ...ctx, | ||
| tokens: newTokens, | ||
| }); | ||
| } | ||
| }, | ||
| }); |
There was a problem hiding this comment.
Fix token persistence and avoid spreading ctx (risk of persisting non-serializable fields).
Spreading ctx into configure is incorrect and may persist methods/opaque fields. Also, if configure throws, it can break the in-flight request. Persist only the token patch and wrap in try/catch.
- onTokenRefresh: async (newTokens: OAuthTokens) => {
- if (ctx) {
- await ctx.configure({
- ...ctx,
- tokens: newTokens,
- });
- }
- },
+ onTokenRefresh: async (newTokens: OAuthTokens) => {
+ if (!ctx) return;
+ try {
+ await ctx.configure({ tokens: newTokens });
+ } catch (err) {
+ console.warn("Failed to persist refreshed Figma tokens", err);
+ }
+ },📝 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.
| onTokenRefresh: async (newTokens: OAuthTokens) => { | |
| if (ctx) { | |
| await ctx.configure({ | |
| ...ctx, | |
| tokens: newTokens, | |
| }); | |
| } | |
| }, | |
| }); | |
| onTokenRefresh: async (newTokens: OAuthTokens) => { | |
| if (!ctx) return; | |
| try { | |
| await ctx.configure({ tokens: newTokens }); | |
| } catch (err) { | |
| console.warn("Failed to persist refreshed Figma tokens", err); | |
| } | |
| }, |
🤖 Prompt for AI Agents
In figma/mod.ts around lines 75-83, stop spreading the entire ctx into
ctx.configure and instead persist only the new token patch; call ctx.configure({
tokens: newTokens }) and wrap the call in a try/catch so any configure error is
caught and logged/handled (but not rethrown) to avoid breaking the in-flight
request.
| export const SCOPES = [ | ||
| "file_read", | ||
| ]; |
There was a problem hiding this comment.
Add offline_access scope so OAuth tokens remain usable.
Figma’s OAuth endpoint only issues refresh tokens when the offline_access scope is requested. With the scope array limited to "file_read", the new OAuth flow will receive a short‑lived access token (≈1 hour) and no refresh token, breaking every call once the token expires. Please include offline_access so the integration can renew credentials as designed.
export const SCOPES = [
- "file_read",
+ "file_read",
+ "offline_access",
];📝 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.
| export const SCOPES = [ | |
| "file_read", | |
| ]; | |
| export const SCOPES = [ | |
| "file_read", | |
| "offline_access", | |
| ]; |
🤖 Prompt for AI Agents
In figma/utils/constant.ts around lines 1 to 3, the SCOPES array only contains
"file_read" so Figma will not issue refresh tokens; add "offline_access" to the
SCOPES array (e.g., include both "file_read" and "offline_access") so OAuth
responses include refresh tokens and the integration can renew credentials.
What is this Contribution About?
Please provide a brief description of the changes or enhancements you are proposing in this pull request.
Issue Link
Please link to the relevant issue that this pull request addresses:
Loom Video
Demonstration Link
Summary by CodeRabbit