From 7dd11016a4e0af34d52b32a5eeada7faba4db092 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Kamil=20Og=C3=B3rek?= <kamil.ogorek@gmail.com>
Date: Mon, 16 Sep 2024 16:46:33 +0200
Subject: [PATCH] ref: Use local state to improve GitHub integration security
 (#29321)

---
 .../github-authorization-create-mutation.ts   | 24 +++++++++++--------
 apps/studio/lib/constants/index.ts            |  1 +
 apps/studio/lib/github.tsx                    | 14 +++++++++--
 .../pages/integrations/github/authorize.tsx   | 16 ++++++++-----
 4 files changed, 37 insertions(+), 18 deletions(-)

diff --git a/apps/studio/data/integrations/github-authorization-create-mutation.ts b/apps/studio/data/integrations/github-authorization-create-mutation.ts
index ea3136245e7..9c344ddbfe7 100644
--- a/apps/studio/data/integrations/github-authorization-create-mutation.ts
+++ b/apps/studio/data/integrations/github-authorization-create-mutation.ts
@@ -3,12 +3,25 @@ import { toast } from 'sonner'
 
 import { handleError, post } from 'data/fetchers'
 import type { ResponseError } from 'types'
+import { LOCAL_STORAGE_KEYS } from 'lib/constants'
 
 export type GitHubAuthorizationCreateVariables = {
   code: string
+  state: string
 }
 
-export async function createGitHubAuthorization({ code }: GitHubAuthorizationCreateVariables) {
+export async function createGitHubAuthorization({
+  code,
+  state,
+}: GitHubAuthorizationCreateVariables) {
+  const localState = localStorage.getItem(LOCAL_STORAGE_KEYS.GITHUB_AUTHORIZATION_STATE)
+
+  if (state !== localState) {
+    throw new Error('GitHub authorization state mismatch')
+  } else {
+    localStorage.removeItem(LOCAL_STORAGE_KEYS.GITHUB_AUTHORIZATION_STATE)
+  }
+
   const { data, error } = await post('/platform/integrations/github/authorization', {
     body: { code },
   })
@@ -31,21 +44,12 @@ export const useGitHubAuthorizationCreateMutation = ({
   >,
   'mutationFn'
 > = {}) => {
-  const queryClient = useQueryClient()
-
   return useMutation<
     GitHubAuthorizationCreateData,
     ResponseError,
     GitHubAuthorizationCreateVariables
   >((vars) => createGitHubAuthorization(vars), {
     async onSuccess(data, variables, context) {
-      // const { projectRef, id } = variables
-
-      // await Promise.all([
-      //   queryClient.invalidateQueries(githubAuthorizationKeys.list(projectRef)),
-      //   queryClient.invalidateQueries(githubAuthorizationKeys.githubAuthorization(projectRef, id)),
-      // ])
-
       await onSuccess?.(data, variables, context)
     },
     async onError(data, variables, context) {
diff --git a/apps/studio/lib/constants/index.ts b/apps/studio/lib/constants/index.ts
index 8dc44a4ef5e..99f2b23f766 100644
--- a/apps/studio/lib/constants/index.ts
+++ b/apps/studio/lib/constants/index.ts
@@ -68,6 +68,7 @@ export const LOCAL_STORAGE_KEYS = {
   // Track position of nodes for schema visualizer
   SCHEMA_VISUALIZER_POSITIONS: (ref: string, schemaId: number) =>
     `schema-visualizer-positions-${ref}-${schemaId}`,
+  GITHUB_AUTHORIZATION_STATE: 'supabase-github-authorization-state',
 }
 
 export const OPT_IN_TAGS = {
diff --git a/apps/studio/lib/github.tsx b/apps/studio/lib/github.tsx
index 7416e9681e5..e3c8702cb3c 100644
--- a/apps/studio/lib/github.tsx
+++ b/apps/studio/lib/github.tsx
@@ -1,3 +1,6 @@
+import { LOCAL_STORAGE_KEYS } from './constants'
+import { makeRandomString } from './helpers'
+
 const GITHUB_INTEGRATION_APP_NAME =
   process.env.NEXT_PUBLIC_ENVIRONMENT === 'prod'
     ? `supabase`
@@ -34,8 +37,15 @@ export function openInstallGitHubIntegrationWindow(type: 'install' | 'authorize'
       ? document.documentElement.clientHeight
       : screen.height
 
-  const windowUrl =
-    type === 'install' ? GITHUB_INTEGRATION_INSTALLATION_URL : GITHUB_INTEGRATION_AUTHORIZATION_URL
+  let windowUrl
+  if (type === 'install') {
+    windowUrl = GITHUB_INTEGRATION_INSTALLATION_URL
+  } else if (type === 'authorize') {
+    const state = makeRandomString(32)
+    localStorage.setItem(LOCAL_STORAGE_KEYS.GITHUB_AUTHORIZATION_STATE, state)
+    windowUrl = `${GITHUB_INTEGRATION_AUTHORIZATION_URL}&state=${state}`
+  }
+
   const systemZoom = width / window.screen.availWidth
   const left = (width - w) / 2 / systemZoom + dualScreenLeft
   const top = (height - h) / 2 / systemZoom + dualScreenTop
diff --git a/apps/studio/pages/integrations/github/authorize.tsx b/apps/studio/pages/integrations/github/authorize.tsx
index 52a9c784eb1..b8622936a10 100644
--- a/apps/studio/pages/integrations/github/authorize.tsx
+++ b/apps/studio/pages/integrations/github/authorize.tsx
@@ -4,25 +4,29 @@ import { useParams } from 'common'
 import { useGitHubAuthorizationCreateMutation } from 'data/integrations/github-authorization-create-mutation'
 
 const GitHubIntegrationAuthorize = () => {
-  const { code } = useParams()
+  const { code, state } = useParams()
 
-  const { mutate, isSuccess } = useGitHubAuthorizationCreateMutation({
+  const { mutate, isSuccess, isError } = useGitHubAuthorizationCreateMutation({
     onSuccess() {
       window.close()
     },
   })
 
   useEffect(() => {
-    if (code) {
-      mutate({ code })
+    if (code && state) {
+      mutate({ code, state })
     }
-  }, [code, mutate])
+  }, [code, state, mutate])
 
   return (
     <div className="h-screen flex flex-col justify-center items-center gap-4">
       <h2 className="text-xl">Completing GitHub Authorization...</h2>
 
-      {isSuccess ? <p>You can now close this window.</p> : <p />}
+      {isSuccess ? (
+        <p>You can now close this window.</p>
+      ) : (
+        <p>Unable to authorize. Please try again.</p>
+      )}
     </div>
   )
 }