From 619234ff3bc386fca1ba67831b53bf84eba93289 Mon Sep 17 00:00:00 2001 From: OJ Abba Date: Wed, 11 Jun 2025 10:58:34 +0100 Subject: [PATCH 1/8] [Refactor] Improve error message shown to the user when authentication fails in `useLogin` --- src/pages/login/useLogin.ts | 21 +++++++++++++++++---- src/types/quickbooks.ts | 2 +- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/src/pages/login/useLogin.ts b/src/pages/login/useLogin.ts index eaa0dd1..1d6f063 100644 --- a/src/pages/login/useLogin.ts +++ b/src/pages/login/useLogin.ts @@ -25,6 +25,8 @@ export default function useLogin(): UseLoginResult { const { context } = useDeskproLatestAppContext() const user = context?.data?.user + const mode = context?.settings.use_advanced_connect === false ? 'global' : 'local'; + const isUsingSandbox = context?.settings.use_sandbox === true // @todo: Update useInitialisedDeskproAppClient typing in the // App SDK to to properly handle both async and sync functions @@ -36,7 +38,6 @@ export default function useLogin(): UseLoginResult { }; const clientID = context.settings.client_id; - const mode = context.settings.use_advanced_connect ? 'local' : 'global'; if (mode === 'local' && typeof clientID !== 'string') { return; @@ -96,13 +97,25 @@ export default function useLogin(): UseLoginResult { if (error instanceof QuickBooksError && isQuickBooksFaultError(error.data)) { const fault = error.data.Fault ?? error.data.fault; - if (fault?.type === "AuthorizationFault") { - throw new Error("The user logging in isn't a part of the company specified in the app setup. Contact your admin for more information"); + switch (fault?.type) { + case "AUTHENTICATION": + throw new Error("An error occurred while authenticating the user.") + case "AuthorizationFault": + throw new Error("The user logging in isn't a part of the company specified in the app setup. Contact your admin for more information."); + case "AuthenticationFault": + if (fault.Error?.[0].Message === "Accessing Wrong Cluster") { + const errorMessage = mode === "global" ? + "Error authenticating user: Sandbox accounts cannot be used with Quick Connect." + : + `Error authenticating user: Ensure the QuickBooks app is setup to use ${isUsingSandbox ? "sandbox" : "production"} accounts.` + throw new Error(errorMessage) + } + break } }; // generic error for all other errors - throw new Error('error authenticating user. Are you using a sandbox company (double-check on QuickBooks, and the settings drawer of the QuickBooks app in Deskpro)?'); + throw new Error(`An unexpected error occurred: ${error instanceof Error ? error.message : "Unknown Error"}.`); }; } catch (error) { setError(error instanceof Error ? error.message : 'unknown error'); diff --git a/src/types/quickbooks.ts b/src/types/quickbooks.ts index d39e256..efa4b7a 100644 --- a/src/types/quickbooks.ts +++ b/src/types/quickbooks.ts @@ -20,7 +20,7 @@ export interface QuickBooksErrorObject { export interface QuickBooksFault { error?: Pick[] Error?: Pick[] - type: "AuthorizationFault" | "AUTHENTICATION" + type: "AuthorizationFault" | "AUTHENTICATION" | "SERVICE" | "AuthenticationFault" } export interface QuickBooksFaultError { From 6282bc43ae2f3d6e8ddefa6a4b5838f8c8827340 Mon Sep 17 00:00:00 2001 From: OJ Abba Date: Wed, 11 Jun 2025 11:01:05 +0100 Subject: [PATCH 2/8] [Bug] Update authorization proxy placeholder in `baseRequest` --- src/api/quickbooks/baseRequest/baseRequest.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/api/quickbooks/baseRequest/baseRequest.ts b/src/api/quickbooks/baseRequest/baseRequest.ts index 5d67a37..5da85f5 100644 --- a/src/api/quickbooks/baseRequest/baseRequest.ts +++ b/src/api/quickbooks/baseRequest/baseRequest.ts @@ -40,7 +40,7 @@ export default async function baseRequest(client: IDeskproClient, reqProps: R method, body: data, headers: { - "Authorization": `Bearer ${placeholders.OAUTH2_ACCESS_TOKEN_PATH}`, + "Authorization": `Bearer [user[${placeholders.OAUTH2_ACCESS_TOKEN_PATH}]]`, "Accept": "application/json", ...customHeaders, ...(data ? { "Content-Type": "application/json" } : {}), From d8c9e5e71877c6ac86f2032286e0140f101fb062 Mon Sep 17 00:00:00 2001 From: OJ Abba Date: Wed, 11 Jun 2025 12:02:33 +0100 Subject: [PATCH 3/8] [Refactor] Prevent refreshing access token when using global proxy --- src/api/quickbooks/baseRequest/baseRequest.ts | 57 +++++++++---------- src/api/quickbooks/refreshAccessToken.ts | 30 ++++++---- src/pages/login/useLogin.ts | 5 ++ 3 files changed, 52 insertions(+), 40 deletions(-) diff --git a/src/api/quickbooks/baseRequest/baseRequest.ts b/src/api/quickbooks/baseRequest/baseRequest.ts index 5da85f5..1db0013 100644 --- a/src/api/quickbooks/baseRequest/baseRequest.ts +++ b/src/api/quickbooks/baseRequest/baseRequest.ts @@ -15,6 +15,8 @@ import refreshAccessToken from '../refreshAccessToken'; export default async function baseRequest(client: IDeskproClient, reqProps: RequestParams): Promise { const { endpoint, realmId, data, method = "GET", queryParams = {}, headers: customHeaders } = reqProps + const isUsingGlobalProxy = (await client.getUserState("isUsingGlobalProxy"))[0]?.data ?? false + const dpFetch = await proxyFetch(client) // Check if the endpoint already contains query parameters @@ -36,43 +38,38 @@ export default async function baseRequest(client: IDeskproClient, reqProps: R requestUrl = `${baseUrl}?${params}` } - const options: RequestInit = { - method, - body: data, - headers: { - "Authorization": `Bearer [user[${placeholders.OAUTH2_ACCESS_TOKEN_PATH}]]`, - "Accept": "application/json", - ...customHeaders, - ...(data ? { "Content-Type": "application/json" } : {}), - }, - }; - - let res = await dpFetch(requestUrl, options); - - if (res.status === 401) { - try { - const tokens = await refreshAccessToken(client); + async function makeRequest(): Promise { + const options: RequestInit = { + method, + body: data, + headers: { + "Authorization": `Bearer [user[${placeholders.OAUTH2_ACCESS_TOKEN_PATH}]]`, + "Accept": "application/json", + ...customHeaders, + ...(data ? { "Content-Type": "application/json" } : {}), + }, + } - await client.setUserState(placeholders.OAUTH2_ACCESS_TOKEN_PATH, tokens.access_token, {backend: true}); + return await dpFetch(requestUrl, options) + } - if (tokens.refresh_token) { - await client.setUserState(placeholders.OAUTH2_REFRESH_TOKEN_PATH, tokens.refresh_token, {backend: true}); - }; + let response = await makeRequest() - options.headers = { - ...options.headers, - 'Authorization': `Bearer ${tokens.access_token}` - }; + // We cannot refresh the access token if we are using the global proxy + // because the client id and secret key will not be available. + if (response.status === 401 && !isUsingGlobalProxy) { + try { + await refreshAccessToken(client) + response = await makeRequest() - res = await dpFetch(requestUrl, options); } catch (refreshError) { - throw new QuickBooksError('failed to refresh access token', {statusCode: 401, data: refreshError}); + throw new QuickBooksError('failed to refresh access token', { statusCode: 401, data: refreshError }); }; }; - if (res.status < 200 || res.status > 399) { + if (response.status < 200 || response.status > 399) { let errorData: unknown; - const rawText = await res.text() + const rawText = await response.text() try { errorData = JSON.parse(rawText) @@ -80,11 +77,11 @@ export default async function baseRequest(client: IDeskproClient, reqProps: R errorData = { message: "Non-JSON error response received", raw: rawText } } - throw new QuickBooksError("Request failed", { statusCode: res.status, data: errorData }) + throw new QuickBooksError("Request failed", { statusCode: response.status, data: errorData }) } try { - return await res.json() as T + return await response.json() as T } catch (e) { throw new QuickBooksError("Failed to parse JSON response", { statusCode: 500, data: e }) } diff --git a/src/api/quickbooks/refreshAccessToken.ts b/src/api/quickbooks/refreshAccessToken.ts index be044b7..0623813 100644 --- a/src/api/quickbooks/refreshAccessToken.ts +++ b/src/api/quickbooks/refreshAccessToken.ts @@ -1,22 +1,32 @@ import { IDeskproClient, OAuth2Result, proxyFetch } from '@deskpro/app-sdk'; import { placeholders } from '@/constants'; -export default async function refreshAccessToken(client: IDeskproClient) { - const fetch = await proxyFetch(client); +export default async function refreshAccessToken(client: IDeskproClient): Promise { + const dpFetch = await proxyFetch(client); - const response = await fetch('https://oauth.platform.intuit.com/oauth2/v1/tokens/bearer', { - method: 'POST', + const body = `grant_type=refresh_token&refresh_token=[user[${placeholders.OAUTH2_REFRESH_TOKEN_PATH}]]` + + + const refreshRequestOptions: RequestInit = { + method: "POST", + body: body, headers: { - 'Authorization': "Basic __client_id+':'+client_secret.base64__", - 'Content-Type': 'application/x-www-form-urlencoded', - 'Accept': 'application/json' + "Content-Type": "application/x-www-form-urlencoded", + Authorization: `Basic __integration_key+':'+secret_key.base64__`, }, - body: `grant_type=refresh_token&refresh_token=[user[${placeholders.OAUTH2_REFRESH_TOKEN_PATH}]]` - }); + } + + const response = await dpFetch('https://oauth.platform.intuit.com/oauth2/v1/tokens/bearer', refreshRequestOptions); if (!response.ok) { throw new Error(`QuickBooks token refresh failed with status code: ${response.status}`); }; - return await response.json() as OAuth2Result['data']; + const data = await response.json() as OAuth2Result['data'] + + await client.setUserState(placeholders.OAUTH2_ACCESS_TOKEN_PATH, data.access_token, { backend: true }); + + if (data.refresh_token) { + await client.setUserState(placeholders.OAUTH2_REFRESH_TOKEN_PATH, data.refresh_token, { backend: true }); + } }; \ No newline at end of file diff --git a/src/pages/login/useLogin.ts b/src/pages/login/useLogin.ts index 1d6f063..e5ef1fd 100644 --- a/src/pages/login/useLogin.ts +++ b/src/pages/login/useLogin.ts @@ -111,6 +111,11 @@ export default function useLogin(): UseLoginResult { throw new Error(errorMessage) } break + case "SERVICE": + if(fault.error?.[0].code === "3100"){ + throw new Error(`Error authenticating user: Ensure the QuickBooks app is setup to use ${isUsingSandbox ? "sandbox" : "production"} accounts.`) + + } } }; From 5bcbf65d7c24507e46d04194478e7956125de3c7 Mon Sep 17 00:00:00 2001 From: OJ Abba Date: Wed, 11 Jun 2025 12:29:00 +0100 Subject: [PATCH 4/8] [Feat] Set `isUsingGlobalProxy` flag on app load --- src/pages/loading/LoadingPage.tsx | 12 ++++++++---- src/pages/login/useLogin.ts | 1 + 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/pages/loading/LoadingPage.tsx b/src/pages/loading/LoadingPage.tsx index 82d691b..0016773 100644 --- a/src/pages/loading/LoadingPage.tsx +++ b/src/pages/loading/LoadingPage.tsx @@ -21,21 +21,25 @@ export default function LoadingPage() { clearElements(); registerElement('refresh', { type: 'refresh_button' }); }, []); + const isUsingGlobalProxy = context?.settings.use_advanced_connect === false + const isUsingSandbox = context?.settings.use_sandbox === true; + const companyId = context?.settings.company_id // eslint-disable-next-line @typescript-eslint/no-misused-promises useInitialisedDeskproAppClient(async client => { client.setTitle('QuickBooks'); - if (!context || !user) { + if (!companyId || !user) { return; }; - const isUsingSandbox = context.settings.use_sandbox; await client.setUserState(placeholders.IS_USING_SANDBOX, isUsingSandbox); + await client.setUserState("isUsingGlobalProxy", isUsingGlobalProxy) + try { - const company = await getCompanyInfo(client, context.settings.company_id); + const company = await getCompanyInfo(client, companyId); if (company) { setIsAuthenticated(true); @@ -46,7 +50,7 @@ export default function LoadingPage() { } finally { setIsFetchingAuth(false); }; - }, [context, context?.settings]); + }, [isUsingSandbox, companyId, isUsingGlobalProxy]); if (!client || !user || isFetchingAuth) { diff --git a/src/pages/login/useLogin.ts b/src/pages/login/useLogin.ts index e5ef1fd..007fb2b 100644 --- a/src/pages/login/useLogin.ts +++ b/src/pages/login/useLogin.ts @@ -116,6 +116,7 @@ export default function useLogin(): UseLoginResult { throw new Error(`Error authenticating user: Ensure the QuickBooks app is setup to use ${isUsingSandbox ? "sandbox" : "production"} accounts.`) } + break } }; From 69d5b0207347dc1a0b5fc05b96c944f5ba9eded5 Mon Sep 17 00:00:00 2001 From: OJ Abba Date: Wed, 11 Jun 2025 12:48:49 +0100 Subject: [PATCH 5/8] [Chore] Attempt fixing workflow --- .github/workflows/subworkflow-build.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/subworkflow-build.yml b/.github/workflows/subworkflow-build.yml index e8311e7..d21d061 100644 --- a/.github/workflows/subworkflow-build.yml +++ b/.github/workflows/subworkflow-build.yml @@ -49,7 +49,8 @@ jobs: if [ -n "${{ github.event.pull_request.number }}" ]; then # Regular PR case echo "PR number found: ${{ github.event.pull_request.number }}" - labels=$(jq -r '[.[] | .name] | join(",")' <<< '${{ toJson(github.event.pull_request.labels) }}') + json_labels='${{ toJson(github.event.pull_request.labels) }}' + labels=$(echo "$json_labels" | jq -r '[.[] | .name] | join(",")') else # Merge queue case - find PR by head ref echo "No PR number found, checking for merge group" From f5e2e1694a094a970f789942b347883ebf11e97e Mon Sep 17 00:00:00 2001 From: OJ Abba Date: Thu, 12 Jun 2025 10:56:32 +0100 Subject: [PATCH 6/8] [Bug] Fix `manifest.json` conditional logic --- manifest.json | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/manifest.json b/manifest.json index e80e362..adc5aec 100644 --- a/manifest.json +++ b/manifest.json @@ -33,7 +33,7 @@ "isRequired": false, "isBackendOnly": false, "default": false, - "condition": "settings.use_advanced_connect == true", + "condition": "settings.use_advanced_connect != false", "order": 2 }, "company_id": { @@ -48,7 +48,7 @@ "type": "string", "isRequired": false, "isBackendOnly": false, - "condition": "settings.use_advanced_connect == true", + "condition": "settings.use_advanced_connect != false", "order": 4 }, "client_secret": { @@ -56,7 +56,7 @@ "type": "string", "isRequired": false, "isBackendOnly": true, - "condition": "settings.use_advanced_connect == true", + "condition": "settings.use_advanced_connect != false", "order": 5 }, "callback_url": { @@ -65,7 +65,7 @@ "options": { "entrypoint": "#/admin/callback", "height": "80px" }, "isRequired": false, "isBackendOnly": false, - "condition": "settings.use_advanced_connect == true", + "condition": "settings.use_advanced_connect != false", "order": 6 } }, From e8e71b9077653352d864955afc59a1eeaf1cee61 Mon Sep 17 00:00:00 2001 From: OJ Abba Date: Thu, 12 Jun 2025 10:57:02 +0100 Subject: [PATCH 7/8] [Bug] Add feedback message for missing client ids in `useLogin` --- src/pages/login/useLogin.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/pages/login/useLogin.ts b/src/pages/login/useLogin.ts index 007fb2b..c42da88 100644 --- a/src/pages/login/useLogin.ts +++ b/src/pages/login/useLogin.ts @@ -39,7 +39,9 @@ export default function useLogin(): UseLoginResult { const clientID = context.settings.client_id; - if (mode === 'local' && typeof clientID !== 'string') { + if (mode === 'local' && (typeof clientID !== 'string' || clientID.trim() === "")) { + // Local mode requires a clientId. + setError("No client id was provided while setting up the app, a client id is required when using advanced connect.") return; }; @@ -72,7 +74,7 @@ export default function useLogin(): UseLoginResult { setAuthUrl(oauth2Response.authorizationUrl) setOAuth2Context(oauth2Response) - }, [context, setAuthUrl]); + }, [context?.settings, setAuthUrl, mode, user]); useInitialisedDeskproAppClient((client) => { @@ -111,8 +113,8 @@ export default function useLogin(): UseLoginResult { throw new Error(errorMessage) } break - case "SERVICE": - if(fault.error?.[0].code === "3100"){ + case "SERVICE": + if (fault.error?.[0].code === "3100") { throw new Error(`Error authenticating user: Ensure the QuickBooks app is setup to use ${isUsingSandbox ? "sandbox" : "production"} accounts.`) } From d31dfdac8ed4ff0be6751e12b29df7bd4b15f3cd Mon Sep 17 00:00:00 2001 From: OJ Abba Date: Thu, 12 Jun 2025 11:17:21 +0100 Subject: [PATCH 8/8] [Chore] Rename settings --- src/pages/login/useLogin.ts | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/pages/login/useLogin.ts b/src/pages/login/useLogin.ts index c42da88..59089b8 100644 --- a/src/pages/login/useLogin.ts +++ b/src/pages/login/useLogin.ts @@ -27,19 +27,20 @@ export default function useLogin(): UseLoginResult { const user = context?.data?.user const mode = context?.settings.use_advanced_connect === false ? 'global' : 'local'; const isUsingSandbox = context?.settings.use_sandbox === true + const settings = context?.settings // @todo: Update useInitialisedDeskproAppClient typing in the // App SDK to to properly handle both async and sync functions // eslint-disable-next-line @typescript-eslint/no-misused-promises useInitialisedDeskproAppClient(async client => { - if (!context?.settings || !user) { + if (!settings || !user) { return; }; - const clientID = context.settings.client_id; + const clientId = settings.client_id; - if (mode === 'local' && (typeof clientID !== 'string' || clientID.trim() === "")) { + if (mode === 'local' && (typeof clientId !== 'string' || clientId.trim() === "")) { // Local mode requires a clientId. setError("No client id was provided while setting up the app, a client id is required when using advanced connect.") return; @@ -50,7 +51,7 @@ export default function useLogin(): UseLoginResult { ({ state, callbackUrl }) => { return `https://appcenter.intuit.com/connect/oauth2?${createSearchParams([ ["response_type", "code"], - ["client_id", clientID || ''], + ["client_id", clientId ?? ''], ["redirect_uri", callbackUrl], ['scope', SCOPE], ["state", state], @@ -74,11 +75,11 @@ export default function useLogin(): UseLoginResult { setAuthUrl(oauth2Response.authorizationUrl) setOAuth2Context(oauth2Response) - }, [context?.settings, setAuthUrl, mode, user]); + }, [settings, setAuthUrl, mode, user]); useInitialisedDeskproAppClient((client) => { - if (!user || !oauth2Context) { + if (!user || !oauth2Context || !settings?.company_id) { return } @@ -93,7 +94,7 @@ export default function useLogin(): UseLoginResult { } try { - await getCompanyInfo(client, context.settings.company_id); + await getCompanyInfo(client, settings.company_id); void navigate('/'); } catch (error) { if (error instanceof QuickBooksError && isQuickBooksFaultError(error.data)) { @@ -136,7 +137,7 @@ export default function useLogin(): UseLoginResult { if (isPolling) { void startPolling() } - }, [isPolling, user, oauth2Context, navigate, context?.settings.company_id]); + }, [isPolling, user, oauth2Context, navigate, settings?.company_id]); const onSignIn = useCallback(() => { setIsLoading(true);