diff --git a/.env.test b/.env.test index 1f045a3b14..eb012561a1 100644 --- a/.env.test +++ b/.env.test @@ -17,6 +17,8 @@ SLACK_VERIFICATION_TOKEN=test-token-123 GITHUB_CLIENT_ID=123; GITHUB_CLIENT_SECRET=123; GITHUB_APP_NAME=outline-test; +GITHUB_APP_ID=123 +GITHUB_APP_PRIVATE_KEY="-----BEGIN RSA PRIVATE KEY-----\nMIIEpAIBAAKCAQEA\n-----END RSA PRIVATE KEY-----" GITLAB_CLIENT_ID=123 GITLAB_CLIENT_SECRET=123 @@ -29,6 +31,15 @@ OIDC_USERINFO_URI=http://localhost/userinfo IFRAMELY_API_KEY=123 +NOTION_CLIENT_ID=123 +NOTION_CLIENT_SECRET=123 + +LINEAR_CLIENT_ID=123 +LINEAR_CLIENT_SECRET=123 + +FIGMA_CLIENT_ID=123 +FIGMA_CLIENT_SECRET=123 + RATE_LIMITER_ENABLED=false FILE_STORAGE=local diff --git a/app/utils/oauth.ts b/app/utils/oauth.ts new file mode 100644 index 0000000000..fc14dcbeb7 --- /dev/null +++ b/app/utils/oauth.ts @@ -0,0 +1,23 @@ +import { setCookie } from "tiny-cookie"; +import { randomString } from "@shared/random"; + +/** + * Generate a random nonce, persist it in a same-origin cookie, and return it + * for embedding in the `state` parameter of an outbound OAuth flow. + * + * The callback handler must read the same cookie and timing-safe-compare it + * against the nonce on the returned state. + * + * @param cookieName The cookie used to persist the nonce, unique per provider. + * @returns The generated nonce. + */ +export function generateOAuthStateNonce(cookieName: string): string { + const nonce = randomString(32); + setCookie(cookieName, nonce, { + path: "/", + "max-age": 600, + samesite: "Lax", + secure: window.location.protocol === "https:", + }); + return nonce; +} diff --git a/plugins/figma/client/components/FigmaButton.tsx b/plugins/figma/client/components/FigmaButton.tsx index 254c6138b0..7ba41f8501 100644 --- a/plugins/figma/client/components/FigmaButton.tsx +++ b/plugins/figma/client/components/FigmaButton.tsx @@ -2,21 +2,21 @@ import * as React from "react"; import { useTranslation } from "react-i18next"; import Button, { type Props } from "~/components/Button"; import useCurrentTeam from "~/hooks/useCurrentTeam"; +import { generateOAuthStateNonce } from "~/utils/oauth"; import { redirectTo } from "~/utils/urls"; -import { FigmaUtils } from "../../shared/FigmaUtils"; +import { FigmaOAuthNonceCookie, FigmaUtils } from "../../shared/FigmaUtils"; export function FigmaConnectButton(props: Props) { const { t } = useTranslation(); const team = useCurrentTeam(); + const handleConnect = React.useCallback(() => { + const nonce = generateOAuthStateNonce(FigmaOAuthNonceCookie); + redirectTo(FigmaUtils.authUrl({ state: { teamId: team.id, nonce } })); + }, [team.id]); + return ( - ); diff --git a/plugins/figma/server/api/figma.test.ts b/plugins/figma/server/api/figma.test.ts new file mode 100644 index 0000000000..d0f19386c4 --- /dev/null +++ b/plugins/figma/server/api/figma.test.ts @@ -0,0 +1,44 @@ +import { buildUser } from "@server/test/factories"; +import { getTestServer } from "@server/test/support"; + +const server = getTestServer(); + +describe("#figma.callback", () => { + it("should reject callback when state nonce does not match cookie", async () => { + const user = await buildUser(); + const state = JSON.stringify({ + teamId: user.teamId, + nonce: "attacker-nonce", + }); + const res = await server.get( + `/api/figma.callback?state=${encodeURIComponent( + state + )}&code=123&token=${user.getJwtToken()}`, + { redirect: "manual" } + ); + const body = await res.json(); + expect(res.status).toEqual(400); + expect(body.error).toEqual("state_mismatch"); + }); + + it("should reject callback when nonce is missing from state", async () => { + const user = await buildUser(); + const state = JSON.stringify({ teamId: user.teamId }); + const res = await server.get( + `/api/figma.callback?state=${encodeURIComponent( + state + )}&code=123&token=${user.getJwtToken()}`, + { redirect: "manual" } + ); + expect(res.status).toEqual(400); + }); + + it("should fail when state is not valid JSON", async () => { + const user = await buildUser(); + const res = await server.get( + `/api/figma.callback?state=bad&code=123&token=${user.getJwtToken()}`, + { redirect: "manual" } + ); + expect(res.status).toEqual(400); + }); +}); diff --git a/plugins/figma/server/api/figma.ts b/plugins/figma/server/api/figma.ts index 6e09b66cbf..9786c693b3 100644 --- a/plugins/figma/server/api/figma.ts +++ b/plugins/figma/server/api/figma.ts @@ -4,11 +4,16 @@ import * as T from "./schema"; import apexAuthRedirect from "@server/middlewares/apexAuthRedirect"; import type { APIContext } from "@server/types"; import validate from "@server/middlewares/validate"; -import { FigmaUtils } from "plugins/figma/shared/FigmaUtils"; +import { + FigmaOAuthNonceCookie, + FigmaUtils, +} from "plugins/figma/shared/FigmaUtils"; import { transaction } from "@server/middlewares/transaction"; import Logger from "@server/logging/Logger"; import { IntegrationService, IntegrationType } from "@shared/types"; +import { ValidationError } from "@server/errors"; import { Integration, IntegrationAuthentication } from "@server/models"; +import { verifyOAuthStateNonce } from "@server/utils/oauth"; import { addSeconds } from "date-fns"; import { Figma } from "../figma"; import UploadIntegrationLogoTask from "@server/queues/tasks/UploadIntegrationLogoTask"; @@ -30,7 +35,7 @@ router.get( }), transaction(), async (ctx: APIContext) => { - const { code, error } = ctx.input.query; + const { code, error, state } = ctx.input.query; // Check error after any sub-domain redirection. Otherwise, the user will be redirected to the root domain. if (error) { @@ -38,6 +43,13 @@ router.get( return; } + const parsedState = FigmaUtils.parseState(state); + if (!parsedState) { + throw ValidationError("Invalid state"); + } + + verifyOAuthStateNonce(ctx, FigmaOAuthNonceCookie, parsedState.nonce); + const { user } = ctx.state.auth; const { transaction } = ctx.state; diff --git a/plugins/figma/shared/FigmaUtils.ts b/plugins/figma/shared/FigmaUtils.ts index 1c6da86508..679e18169a 100644 --- a/plugins/figma/shared/FigmaUtils.ts +++ b/plugins/figma/shared/FigmaUtils.ts @@ -2,8 +2,11 @@ import queryString from "query-string"; import env from "@shared/env"; import { integrationSettingsPath } from "@shared/utils/routeHelpers"; +export const FigmaOAuthNonceCookie = "figmaOAuthNonce"; + export type OAuthState = { teamId: string; + nonce: string; }; export class FigmaUtils { @@ -16,8 +19,12 @@ export class FigmaUtils { private static settingsUrl = integrationSettingsPath("figma"); - static parseState(state: string): OAuthState { - return JSON.parse(state); + static parseState(state: string): OAuthState | undefined { + try { + return JSON.parse(state); + } catch { + return undefined; + } } static successUrl() { diff --git a/plugins/github/client/components/GitHubButton.tsx b/plugins/github/client/components/GitHubButton.tsx index cf4da512df..1f48be1bdd 100644 --- a/plugins/github/client/components/GitHubButton.tsx +++ b/plugins/github/client/components/GitHubButton.tsx @@ -2,19 +2,21 @@ import * as React from "react"; import { useTranslation } from "react-i18next"; import Button, { type Props } from "~/components/Button"; import useCurrentTeam from "~/hooks/useCurrentTeam"; +import { generateOAuthStateNonce } from "~/utils/oauth"; import { redirectTo } from "~/utils/urls"; -import { GitHubUtils } from "../../shared/GitHubUtils"; +import { GitHubOAuthNonceCookie, GitHubUtils } from "../../shared/GitHubUtils"; export function GitHubConnectButton(props: Props) { const { t } = useTranslation(); const team = useCurrentTeam(); + const handleConnect = React.useCallback(() => { + const nonce = generateOAuthStateNonce(GitHubOAuthNonceCookie); + redirectTo(GitHubUtils.authUrl({ teamId: team.id, nonce })); + }, [team.id]); + return ( - ); diff --git a/plugins/github/server/api/github.test.ts b/plugins/github/server/api/github.test.ts new file mode 100644 index 0000000000..b3bb4a387b --- /dev/null +++ b/plugins/github/server/api/github.test.ts @@ -0,0 +1,45 @@ +import { buildUser } from "@server/test/factories"; +import { getTestServer } from "@server/test/support"; +import { SetupAction } from "./schema"; + +const server = getTestServer(); + +describe("#github.callback", () => { + it("should reject callback when state nonce does not match cookie", async () => { + const user = await buildUser(); + const state = JSON.stringify({ + teamId: user.teamId, + nonce: "attacker-nonce", + }); + const res = await server.get( + `/api/github.callback?state=${encodeURIComponent( + state + )}&code=123&setup_action=${SetupAction.install}&installation_id=1&token=${user.getJwtToken()}`, + { redirect: "manual" } + ); + const body = await res.json(); + expect(res.status).toEqual(400); + expect(body.error).toEqual("state_mismatch"); + }); + + it("should reject callback when nonce is missing from state", async () => { + const user = await buildUser(); + const state = JSON.stringify({ teamId: user.teamId }); + const res = await server.get( + `/api/github.callback?state=${encodeURIComponent( + state + )}&code=123&setup_action=${SetupAction.install}&installation_id=1&token=${user.getJwtToken()}`, + { redirect: "manual" } + ); + expect(res.status).toEqual(400); + }); + + it("should fail when state is not valid JSON", async () => { + const user = await buildUser(); + const res = await server.get( + `/api/github.callback?state=bad&code=123&setup_action=${SetupAction.install}&installation_id=1&token=${user.getJwtToken()}`, + { redirect: "manual" } + ); + expect(res.status).toEqual(400); + }); +}); diff --git a/plugins/github/server/api/github.ts b/plugins/github/server/api/github.ts index 928a892e35..400b9e9a00 100644 --- a/plugins/github/server/api/github.ts +++ b/plugins/github/server/api/github.ts @@ -2,6 +2,7 @@ import Router from "koa-router"; import find from "lodash/find"; import { IntegrationService, IntegrationType } from "@shared/types"; import { createContext } from "@server/context"; +import { ValidationError } from "@server/errors"; import apexAuthRedirect from "@server/middlewares/apexAuthRedirect"; import auth from "@server/middlewares/authentication"; import { transaction } from "@server/middlewares/transaction"; @@ -9,7 +10,8 @@ import validate from "@server/middlewares/validate"; import validateWebhook from "@server/middlewares/validateWebhook"; import { IntegrationAuthentication, Integration } from "@server/models"; import type { APIContext } from "@server/types"; -import { GitHubUtils } from "../../shared/GitHubUtils"; +import { verifyOAuthStateNonce } from "@server/utils/oauth"; +import { GitHubOAuthNonceCookie, GitHubUtils } from "../../shared/GitHubUtils"; import env from "../env"; import { GitHub } from "../github"; import GitHubWebhookTask from "../tasks/GitHubWebhookTask"; @@ -22,7 +24,7 @@ router.get( auth({ optional: true }), validate(T.GitHubCallbackSchema), apexAuthRedirect({ - getTeamId: (ctx) => ctx.input.query.state, + getTeamId: (ctx) => GitHubUtils.parseState(ctx.input.query.state)?.teamId, getRedirectPath: (ctx, team) => GitHubUtils.callbackUrl({ baseUrl: team.url, @@ -34,7 +36,7 @@ router.get( async (ctx: APIContext) => { const { code, - state: teamId, + state, error, installation_id: installationId, setup_action: setupAction, @@ -52,7 +54,14 @@ router.get( return; } - const client = await GitHub.authenticateAsUser(code!, teamId); + const parsedState = GitHubUtils.parseState(state); + if (!parsedState) { + throw ValidationError("Invalid state"); + } + + verifyOAuthStateNonce(ctx, GitHubOAuthNonceCookie, parsedState.nonce); + + const client = await GitHub.authenticateAsUser(code!, state); const installationsByUser = await client.requestAppInstallations(); const installation = find( installationsByUser, diff --git a/plugins/github/server/api/schema.ts b/plugins/github/server/api/schema.ts index f7430b986b..48f6914814 100644 --- a/plugins/github/server/api/schema.ts +++ b/plugins/github/server/api/schema.ts @@ -12,7 +12,7 @@ export const GitHubCallbackSchema = BaseSchema.extend({ query: z .object({ code: z.string().nullish(), - state: z.uuid().nullish(), + state: z.string(), error: z.string().nullish(), installation_id: z.coerce.number().optional(), setup_action: z.enum(SetupAction), diff --git a/plugins/github/shared/GitHubUtils.ts b/plugins/github/shared/GitHubUtils.ts index e38fde944d..0b9a70aa7a 100644 --- a/plugins/github/shared/GitHubUtils.ts +++ b/plugins/github/shared/GitHubUtils.ts @@ -2,6 +2,13 @@ import queryString from "query-string"; import env from "@shared/env"; import { integrationSettingsPath } from "@shared/utils/routeHelpers"; +export const GitHubOAuthNonceCookie = "githubOAuthNonce"; + +export type OAuthState = { + teamId: string; + nonce: string; +}; + export class GitHubUtils { public static clientId = env.GITHUB_CLIENT_ID; @@ -31,16 +38,24 @@ export class GitHubUtils { : `${baseUrl}/api/github.callback`; } - static authUrl(state: string): string { + static authUrl(state: OAuthState): string { const baseUrl = `https://github.com/apps/${env.GITHUB_APP_NAME}/installations/new`; const params = { client_id: this.clientId, redirect_uri: this.callbackUrl(), - state, + state: JSON.stringify(state), }; return `${baseUrl}?${queryString.stringify(params)}`; } + static parseState(state: string): OAuthState | undefined { + try { + return JSON.parse(state); + } catch { + return undefined; + } + } + static installRequestUrl(): string { return `${this.url}?install_request=true`; } diff --git a/plugins/gitlab/server/api/gitlab.test.ts b/plugins/gitlab/server/api/gitlab.test.ts new file mode 100644 index 0000000000..3617f9cd99 --- /dev/null +++ b/plugins/gitlab/server/api/gitlab.test.ts @@ -0,0 +1,44 @@ +import { buildUser } from "@server/test/factories"; +import { getTestServer } from "@server/test/support"; + +const server = getTestServer(); + +describe("#gitlab.callback", () => { + it("should reject callback when state nonce does not match cookie", async () => { + const user = await buildUser(); + const state = JSON.stringify({ + teamId: user.teamId, + nonce: "attacker-nonce", + }); + const res = await server.get( + `/api/gitlab.callback?state=${encodeURIComponent( + state + )}&code=123&token=${user.getJwtToken()}`, + { redirect: "manual" } + ); + const body = await res.json(); + expect(res.status).toEqual(400); + expect(body.error).toEqual("state_mismatch"); + }); + + it("should reject callback when nonce is missing from state", async () => { + const user = await buildUser(); + const state = JSON.stringify({ teamId: user.teamId }); + const res = await server.get( + `/api/gitlab.callback?state=${encodeURIComponent( + state + )}&code=123&token=${user.getJwtToken()}`, + { redirect: "manual" } + ); + expect(res.status).toEqual(400); + }); + + it("should fail when state is not valid JSON", async () => { + const user = await buildUser(); + const res = await server.get( + `/api/gitlab.callback?state=bad&code=123&token=${user.getJwtToken()}`, + { redirect: "manual" } + ); + expect(res.status).toEqual(400); + }); +}); diff --git a/plugins/gitlab/server/api/gitlab.ts b/plugins/gitlab/server/api/gitlab.ts index 9270db0c90..c1a74dd7f8 100644 --- a/plugins/gitlab/server/api/gitlab.ts +++ b/plugins/gitlab/server/api/gitlab.ts @@ -2,6 +2,7 @@ import Router from "koa-router"; import { Op } from "sequelize"; import { IntegrationService, IntegrationType } from "@shared/types"; import { createContext } from "@server/context"; +import { ValidationError } from "@server/errors"; import apexAuthRedirect from "@server/middlewares/apexAuthRedirect"; import auth from "@server/middlewares/authentication"; import { transaction } from "@server/middlewares/transaction"; @@ -10,14 +11,18 @@ import validateWebhook from "@server/middlewares/validateWebhook"; import { IntegrationAuthentication, Integration } from "@server/models"; import { authorize } from "@server/policies"; import type { APIContext } from "@server/types"; +import { + generateOAuthStateNonce, + verifyOAuthStateNonce, +} from "@server/utils/oauth"; import { validateUrlNotPrivate } from "@server/utils/url"; import { addSeconds } from "date-fns"; import Logger from "@server/logging/Logger"; -import { GitLabUtils } from "../../shared/GitLabUtils"; +import { GitLabOAuthNonceCookie, GitLabUtils } from "../../shared/GitLabUtils"; import { GitLab } from "../gitlab"; import env from "../env"; import GitLabWebhookTask from "../tasks/GitLabWebhookTask"; -import * as T from "../schema"; +import * as T from "./schema"; const router = new Router(); @@ -111,7 +116,12 @@ router.post( } } - const redirectUrl = GitLabUtils.authUrl(user.teamId, url, clientId); + const nonce = generateOAuthStateNonce(ctx, GitLabOAuthNonceCookie); + const redirectUrl = GitLabUtils.authUrl( + { teamId: user.teamId, nonce }, + url, + clientId + ); ctx.body = { data: { redirectUrl }, }; @@ -123,7 +133,7 @@ router.get( auth({ optional: true }), validate(T.GitLabCallbackSchema), apexAuthRedirect({ - getTeamId: (ctx) => ctx.input.query.state, + getTeamId: (ctx) => GitLabUtils.parseState(ctx.input.query.state)?.teamId, getRedirectPath: (ctx, team) => GitLabUtils.callbackUrl({ baseUrl: team.url, @@ -133,7 +143,7 @@ router.get( }), transaction(), async (ctx: APIContext) => { - const { code, error } = ctx.input.query; + const { code, error, state } = ctx.input.query; const { user } = ctx.state.auth; const { transaction } = ctx.state; @@ -142,6 +152,13 @@ router.get( return; } + const parsedState = GitLabUtils.parseState(state); + if (!parsedState) { + throw ValidationError("Invalid state"); + } + + verifyOAuthStateNonce(ctx, GitLabOAuthNonceCookie, parsedState.nonce); + try { // Check for a pending IntegrationAuthentication with custom credentials const pendingAuth = await IntegrationAuthentication.findOne({ diff --git a/plugins/gitlab/server/schema.ts b/plugins/gitlab/server/api/schema.ts similarity index 96% rename from plugins/gitlab/server/schema.ts rename to plugins/gitlab/server/api/schema.ts index 8766d056f2..dc8720e055 100644 --- a/plugins/gitlab/server/schema.ts +++ b/plugins/gitlab/server/api/schema.ts @@ -6,7 +6,7 @@ export const GitLabCallbackSchema = BaseSchema.extend({ query: z .object({ code: z.string().nullish(), - state: z.string().uuid().nullish(), + state: z.string(), error: z.string().nullish(), }) .refine((req) => !(isEmpty(req.code) && isEmpty(req.error)), { diff --git a/plugins/gitlab/shared/GitLabUtils.ts b/plugins/gitlab/shared/GitLabUtils.ts index b0dbbef748..dde4bb6c99 100644 --- a/plugins/gitlab/shared/GitLabUtils.ts +++ b/plugins/gitlab/shared/GitLabUtils.ts @@ -2,6 +2,13 @@ import env from "@shared/env"; import { integrationSettingsPath } from "@shared/utils/routeHelpers"; import { UnfurlResourceType } from "@shared/types"; +export const GitLabOAuthNonceCookie = "gitlabOAuthNonce"; + +export type OAuthState = { + teamId: string; + nonce: string; +}; + export class GitLabUtils { public static defaultGitlabUrl = "https://gitlab.com"; @@ -67,13 +74,13 @@ export class GitLabUtils { /** * Generates the authorization URL for GitLab OAuth. * - * @param state - A unique state string to prevent CSRF attacks. + * @param state - The OAuth state with teamId for routing and nonce for CSRF. * @param customUrl - Optional custom GitLab URL from integration settings. * @param customClientId - Optional custom OAuth client ID from integration settings. * @returns The full URL to redirect the user to GitLab's OAuth authorization page. */ public static authUrl( - state: string, + state: OAuthState, customUrl?: string, customClientId?: string ): string { @@ -81,13 +88,27 @@ export class GitLabUtils { client_id: customClientId || env.GITLAB_CLIENT_ID, redirect_uri: this.callbackUrl(), response_type: "code", - state, + state: JSON.stringify(state), scope: "read_api read_user", }); return `${this.getOauthUrl(customUrl)}/authorize?${params.toString()}`; } + /** + * Parses an OAuth state string from a GitLab callback. + * + * @param state - The state string carried in the callback query. + * @returns The parsed OAuth state. + */ + public static parseState(state: string): OAuthState | undefined { + try { + return JSON.parse(state); + } catch { + return undefined; + } + } + /** * Generates the installation request URL. * diff --git a/plugins/linear/client/components/LinearButton.tsx b/plugins/linear/client/components/LinearButton.tsx index d607b2b430..5a05deba72 100644 --- a/plugins/linear/client/components/LinearButton.tsx +++ b/plugins/linear/client/components/LinearButton.tsx @@ -2,21 +2,21 @@ import * as React from "react"; import { useTranslation } from "react-i18next"; import Button, { type Props } from "~/components/Button"; import useCurrentTeam from "~/hooks/useCurrentTeam"; +import { generateOAuthStateNonce } from "~/utils/oauth"; import { redirectTo } from "~/utils/urls"; -import { LinearUtils } from "../../shared/LinearUtils"; +import { LinearOAuthNonceCookie, LinearUtils } from "../../shared/LinearUtils"; export function LinearConnectButton(props: Props) { const { t } = useTranslation(); const team = useCurrentTeam(); + const handleConnect = React.useCallback(() => { + const nonce = generateOAuthStateNonce(LinearOAuthNonceCookie); + redirectTo(LinearUtils.authUrl({ state: { teamId: team.id, nonce } })); + }, [team.id]); + return ( - ); diff --git a/plugins/linear/server/api/linear.test.ts b/plugins/linear/server/api/linear.test.ts new file mode 100644 index 0000000000..278bc27ede --- /dev/null +++ b/plugins/linear/server/api/linear.test.ts @@ -0,0 +1,44 @@ +import { buildUser } from "@server/test/factories"; +import { getTestServer } from "@server/test/support"; + +const server = getTestServer(); + +describe("#linear.callback", () => { + it("should reject callback when state nonce does not match cookie", async () => { + const user = await buildUser(); + const state = JSON.stringify({ + teamId: user.teamId, + nonce: "attacker-nonce", + }); + const res = await server.get( + `/api/linear.callback?state=${encodeURIComponent( + state + )}&code=123&token=${user.getJwtToken()}`, + { redirect: "manual" } + ); + const body = await res.json(); + expect(res.status).toEqual(400); + expect(body.error).toEqual("state_mismatch"); + }); + + it("should reject callback when nonce is missing from state", async () => { + const user = await buildUser(); + const state = JSON.stringify({ teamId: user.teamId }); + const res = await server.get( + `/api/linear.callback?state=${encodeURIComponent( + state + )}&code=123&token=${user.getJwtToken()}`, + { redirect: "manual" } + ); + expect(res.status).toEqual(400); + }); + + it("should fail when state is not valid JSON", async () => { + const user = await buildUser(); + const res = await server.get( + `/api/linear.callback?state=bad&code=123&token=${user.getJwtToken()}`, + { redirect: "manual" } + ); + expect(res.status).toEqual(400); + }); +}); diff --git a/plugins/linear/server/api/linear.ts b/plugins/linear/server/api/linear.ts index 2e2f16c685..ab10230011 100644 --- a/plugins/linear/server/api/linear.ts +++ b/plugins/linear/server/api/linear.ts @@ -1,5 +1,6 @@ import Router from "koa-router"; import { IntegrationService, IntegrationType } from "@shared/types"; +import { ValidationError } from "@server/errors"; import Logger from "@server/logging/Logger"; import apexAuthRedirect from "@server/middlewares/apexAuthRedirect"; import auth from "@server/middlewares/authentication"; @@ -7,10 +8,14 @@ import { transaction } from "@server/middlewares/transaction"; import validate from "@server/middlewares/validate"; import { IntegrationAuthentication, Integration } from "@server/models"; import type { APIContext } from "@server/types"; +import { verifyOAuthStateNonce } from "@server/utils/oauth"; import { Linear } from "../linear"; import UploadIntegrationLogoTask from "@server/queues/tasks/UploadIntegrationLogoTask"; import * as T from "./schema"; -import { LinearUtils } from "plugins/linear/shared/LinearUtils"; +import { + LinearOAuthNonceCookie, + LinearUtils, +} from "plugins/linear/shared/LinearUtils"; import { addSeconds } from "date-fns"; const router = new Router(); @@ -32,7 +37,7 @@ router.get( }), transaction(), async (ctx: APIContext) => { - const { code, error } = ctx.input.query; + const { code, error, state } = ctx.input.query; const { user } = ctx.state.auth; const { transaction } = ctx.state; @@ -42,6 +47,13 @@ router.get( return; } + const parsedState = LinearUtils.parseState(state); + if (!parsedState) { + throw ValidationError("Invalid state"); + } + + verifyOAuthStateNonce(ctx, LinearOAuthNonceCookie, parsedState.nonce); + try { // validation middleware ensures that code is non-null at this point. const oauth = await Linear.oauthAccess(code!); diff --git a/plugins/linear/shared/LinearUtils.ts b/plugins/linear/shared/LinearUtils.ts index dda21dee49..6c2e3f1b13 100644 --- a/plugins/linear/shared/LinearUtils.ts +++ b/plugins/linear/shared/LinearUtils.ts @@ -2,8 +2,11 @@ import queryString from "query-string"; import env from "@shared/env"; import { integrationSettingsPath } from "@shared/utils/routeHelpers"; +export const LinearOAuthNonceCookie = "linearOAuthNonce"; + export type OAuthState = { teamId: string; + nonce: string; }; export class LinearUtils { @@ -15,8 +18,12 @@ export class LinearUtils { private static settingsUrl = integrationSettingsPath("linear"); - static parseState(state: string): OAuthState { - return JSON.parse(state); + static parseState(state: string): OAuthState | undefined { + try { + return JSON.parse(state); + } catch { + return undefined; + } } static successUrl() { diff --git a/plugins/notion/client/Imports.tsx b/plugins/notion/client/Imports.tsx index af7f3b8853..36ce26a31f 100644 --- a/plugins/notion/client/Imports.tsx +++ b/plugins/notion/client/Imports.tsx @@ -9,8 +9,9 @@ import Button from "~/components/Button"; import useCurrentTeam from "~/hooks/useCurrentTeam"; import useQuery from "~/hooks/useQuery"; import useStores from "~/hooks/useStores"; +import { generateOAuthStateNonce } from "~/utils/oauth"; import { redirectTo } from "~/utils/urls"; -import { NotionUtils } from "../shared/NotionUtils"; +import { NotionOAuthNonceCookie, NotionUtils } from "../shared/NotionUtils"; import { ImportDialog } from "./components/ImportDialog"; export const Notion = observer(() => { @@ -22,7 +23,6 @@ export const Notion = observer(() => { const queryParams = useQuery(); const appName = env.APP_NAME; - const authUrl = NotionUtils.authUrl({ state: { teamId: team.id } }); const service = queryParams.get("service"); const oauthSuccess = queryParams.get("success") === ""; @@ -88,10 +88,15 @@ export const Notion = observer(() => { } }, [t, appName, oauthError]); + const handleConnect = React.useCallback(() => { + const nonce = generateOAuthStateNonce(NotionOAuthNonceCookie); + redirectTo(NotionUtils.authUrl({ state: { teamId: team.id, nonce } })); + }, [team.id]); + return (