From a8f15189829998dc2c5b7ee78a42a4eb1ec1d840 Mon Sep 17 00:00:00 2001 From: Federico Brigante Date: Mon, 21 Nov 2022 05:17:28 +0800 Subject: [PATCH 1/3] #4358: Use standard memoize instead of custom storage logic (#4660) --- src/background/auth.test.ts | 57 +++++++++++++++ src/background/auth.ts | 59 +++++---------- src/background/browserAction.ts | 10 +-- src/background/contextMenus.ts | 107 +++++----------------------- src/background/starterBlueprints.ts | 6 +- src/utils.test.ts | 15 +++- src/utils.ts | 35 +++++---- 7 files changed, 134 insertions(+), 155 deletions(-) create mode 100644 src/background/auth.test.ts diff --git a/src/background/auth.test.ts b/src/background/auth.test.ts new file mode 100644 index 0000000000..17ff80cbc1 --- /dev/null +++ b/src/background/auth.test.ts @@ -0,0 +1,57 @@ +/* + * Copyright (C) 2022 PixieBrix, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +import MockAdapter from "axios-mock-adapter"; +import axios from "axios"; +import { getToken } from "./auth"; +import { UUID } from "@/core"; +import { uuidv4 } from "@/types/helpers"; + +const axiosMock = new MockAdapter(axios); + +const getOneToken = async (id: UUID) => + getToken( + { + // @ts-expect-error The result isn't necessary at this time + getTokenContext: () => ({}), + isToken: true, + }, + { id } + ); + +describe("getToken", () => { + test("multiple requests are temporarily memoized", async () => { + let userId = 0; + axiosMock.onPost().reply(() => [200, userId++]); // Increase ID at every request + + const id1 = uuidv4(); + // Consecutive calls should make new requests + expect(await getOneToken(id1)).toBe(0); + expect(await getOneToken(id1)).toBe(1); + + // Parallel calls should make one request + expect( + await Promise.all([getOneToken(id1), getOneToken(id1)]) + ).toStrictEqual([2, 2]); + + // Parallel calls but with different auth.id’s should make multiple requests + const id2 = uuidv4(); + expect( + await Promise.all([getOneToken(id1), getOneToken(id2)]) + ).toStrictEqual([3, 4]); + }); +}); diff --git a/src/background/auth.ts b/src/background/auth.ts index 798ee2ecec..b88acf9bce 100644 --- a/src/background/auth.ts +++ b/src/background/auth.ts @@ -33,6 +33,7 @@ import { getErrorMessage } from "@/errors/errorHelpers"; import { expectContext } from "@/utils/expectContext"; import { UnknownObject } from "@/types"; import { BusinessError } from "@/errors/businessErrors"; +import { memoizeUntilSettled } from "@/utils"; const OAUTH2_STORAGE_KEY = "OAUTH2" as ManualStorageKey; @@ -98,18 +99,26 @@ export async function deleteCachedAuthData(serviceAuthId: UUID): Promise { ); } } - -// In-progress token requests to coalesce multiple requests for the same token. NOTE: this is not a cache, it only -// contains in-progress requests. -const tokenRequests = new Map>(); - /** - * @see getToken + * Exchange credentials for a token, and cache the token response. + * + * If a request for the token is already in progress, return the existing promise. */ -export async function _getToken( + +export const getToken = memoizeUntilSettled(_getToken, { + cacheKey: ([, auth]) => auth.id, +}); + +async function _getToken( service: IService, auth: RawServiceConfiguration ): Promise { + expectContext("background"); + + if (!service.isToken) { + throw new Error(`Service ${service.id} does not use token authentication`); + } + const { url, data: tokenData } = service.getTokenContext(auth.config); const { @@ -127,36 +136,6 @@ export async function _getToken( return responseData; } -/** - * Exchange credentials for a token, and cache the token response. - * - * If a request for the token is already in progress, return the existing promise. - * - * @param service - * @param auth - */ -export async function getToken( - service: IService, - auth: RawServiceConfiguration -): Promise { - expectContext("background"); - - if (!service.isToken) { - throw new Error(`Service ${service.id} does not use token authentication`); - } - - const existing = tokenRequests.get(auth.id); - if (existing != null) { - return existing; - } - - const tokenPromise = _getToken(service, auth); - tokenRequests.set(auth.id, tokenPromise); - // eslint-disable-next-line promise/prefer-await-to-then -- returning the promise outside this method - tokenPromise.finally(() => tokenRequests.delete(auth.id)); - return tokenPromise; -} - function parseResponseParams(url: URL): UnknownObject { const hasSearchParams = [...url.searchParams.keys()].length > 0; @@ -204,7 +183,7 @@ async function implicitGrantFlow( }); const responseUrl = await browser.identity.launchWebAuthFlow({ - url: authorizeURL.toString(), + url: authorizeURL.href, interactive: true, }); @@ -279,7 +258,7 @@ async function codeGrantFlow( } const responseUrl = await browser.identity.launchWebAuthFlow({ - url: authorizeURL.toString(), + url: authorizeURL.href, interactive: true, }); @@ -327,7 +306,7 @@ async function codeGrantFlow( let tokenResponse: AxiosResponse; try { - tokenResponse = await axios.post(tokenURL.toString(), tokenParams, { + tokenResponse = await axios.post(tokenURL.href, tokenParams, { headers: { "Content-Type": "application/x-www-form-urlencoded", }, diff --git a/src/background/browserAction.ts b/src/background/browserAction.ts index e32de3a8e8..302ca2cec5 100644 --- a/src/background/browserAction.ts +++ b/src/background/browserAction.ts @@ -18,9 +18,8 @@ import { ensureContentScript } from "@/background/contentScript"; import { rehydrateSidebar } from "@/contentScript/messenger/api"; import { executeScript } from "webext-content-scripts"; -import pMemoize from "p-memoize"; import webextAlert from "./webextAlert"; -import { isMac } from "@/utils"; +import { memoizeUntilSettled, isMac } from "@/utils"; import { notify } from "@/options/messenger/api"; import { browserAction, Tab } from "@/mv3/api"; import { isScriptableUrl } from "@/utils/permissions"; @@ -34,12 +33,7 @@ const MSG_NO_SIDEBAR_ON_OPTIONS_PAGE = `PixieBrix Tip 💜\n If you want to crea // The sidebar is always injected to into the top level frame const TOP_LEVEL_FRAME_ID = 0; -// Avoid triggering multiple requests at once and causing multiple error alerts. -// This patterns "debounces" calls while the promise is pending: -// https://github.com/sindresorhus/promise-fun/issues/15 -const toggleSidebar = pMemoize(_toggleSidebar, { - cache: false, -}); +const toggleSidebar = memoizeUntilSettled(_toggleSidebar); // Don't accept objects here as they're not easily memoizable async function _toggleSidebar(tabId: number, tabUrl: string): Promise { diff --git a/src/background/contextMenus.ts b/src/background/contextMenus.ts index 9e4017754b..de6e02ec88 100644 --- a/src/background/contextMenus.ts +++ b/src/background/contextMenus.ts @@ -17,9 +17,9 @@ import pTimeout from "p-timeout"; import { Menus, Tabs } from "webextension-polyfill"; -import { getErrorMessage, hasSpecificErrorCause } from "@/errors/errorHelpers"; +import chromeP from "webext-polyfill-kinda"; +import { hasSpecificErrorCause } from "@/errors/errorHelpers"; import reportError from "@/telemetry/reportError"; -import { noop } from "lodash"; import { handleMenuAction, notify } from "@/contentScript/messenger/api"; import { ensureContentScript } from "@/background/contentScript"; import { reportEvent } from "@/telemetry/events"; @@ -32,17 +32,9 @@ import { } from "@/extensionPoints/contextMenu"; import { loadOptions } from "@/store/extensionsStorage"; import { resolveDefinitions } from "@/registry/internal"; -import { allSettledValues } from "@/utils"; +import { allSettledValues, memoizeUntilSettled } from "@/utils"; import { CancelError } from "@/errors/businessErrors"; -type ExtensionId = UUID; -// This is the type the browser API has for menu ids. In practice they should be strings because that's what we're -// creating via `makeMenuId` -type MenuItemId = number | string; - -const extensionMenuItems = new Map(); -const pendingRegistration = new Set(); - const MENU_PREFIX = "pixiebrix-"; // This constant must be high enough to give Chrome time to inject the content script. ensureContentScript can take @@ -112,10 +104,7 @@ async function dispatchMenu( } function menuListener(info: Menus.OnClickData, tab: Tabs.Tab) { - if ( - typeof info.menuItemId === "string" && - info.menuItemId.startsWith(MENU_PREFIX) - ) { + if (String(info.menuItemId).startsWith(MENU_PREFIX)) { void dispatchMenu(info, tab); } else { console.debug(`Ignoring menu item: ${info.menuItemId}`); @@ -132,16 +121,7 @@ export async function uninstallContextMenu({ extensionId: UUID; }): Promise { try { - const menuItemId = extensionMenuItems.get(extensionId); - - if (menuItemId) { - extensionMenuItems.delete(extensionId); - await browser.contextMenus.remove(menuItemId); - } else { - // Our bookkeeping in `extensionMenuItems` might be off. Try removing the expected menuId just in case. - await browser.contextMenus.remove(makeMenuId(extensionId)); - } - + await browser.contextMenus.remove(makeMenuId(extensionId)); console.debug(`Uninstalled context menu ${extensionId}`); return true; } catch (error) { @@ -154,7 +134,10 @@ export async function uninstallContextMenu({ } } -export async function ensureContextMenu({ +export const ensureContextMenu = memoizeUntilSettled(_ensureContextMenu, { + cacheKey: ([{ extensionId }]) => extensionId, +}); +async function _ensureContextMenu({ extensionId, contexts, title, @@ -166,16 +149,6 @@ export async function ensureContextMenu({ throw new Error("extensionId is required"); } - // Handle the thundering herd of re-registrations when a contentScript.reactivate is broadcast - if (pendingRegistration.has(extensionId)) { - console.debug("contextMenu registration pending for %s", extensionId); - - // Is it OK to return immediately? Or do we need to track the common promise that all callers should see? - return; - } - - pendingRegistration.add(extensionId); - const updateProperties: Menus.UpdateUpdatePropertiesType = { type: "normal", title, @@ -184,60 +157,18 @@ export async function ensureContextMenu({ }; const expectedMenuId = makeMenuId(extensionId); - try { - let menuId = extensionMenuItems.get(extensionId); - - if (menuId) { - try { - await browser.contextMenus.update(menuId, updateProperties); - return; - } catch (error) { - console.debug("Cannot update context menu", { error }); - } - } else { - // Just to be safe if our `extensionMenuItems` bookkeeping is off, remove any stale menu item - await browser.contextMenus.remove(expectedMenuId).catch(noop); - } - - // The update failed, or this is a new context menu - extensionMenuItems.delete(extensionId); - - // The types of browser.contextMenus.create are wacky. I verified on Chrome that the method does take a callback - // even when using the browser polyfill - let createdMenuId: string | number; - menuId = await new Promise((resolve, reject) => { - // `browser.contextMenus.create` returns immediately with the assigned menu id - createdMenuId = browser.contextMenus.create( - { - ...updateProperties, - id: makeMenuId(extensionId), - }, - () => { - if (browser.runtime.lastError) { - reject(new Error(browser.runtime.lastError.message)); - } - - resolve(createdMenuId); - } - ); + // Try updating it first. It will fail if missing, so we attempt to create it instead + await browser.contextMenus.update(expectedMenuId, updateProperties); + } catch { + // WARNING: Do not remove `chromeP` + // The standard `contextMenus.create` does not return a Promise in any browser + // https://github.com/w3c/webextensions/issues/188#issuecomment-1112436359 + // eslint-disable-next-line @typescript-eslint/await-thenable -- The types don't really match `chromeP` + await chromeP.contextMenus.create({ + ...updateProperties, + id: expectedMenuId, }); - - extensionMenuItems.set(extensionId, menuId); - } catch (error) { - if ( - getErrorMessage(error).includes("Cannot create item with duplicate id") - ) { - // Likely caused by a concurrent update. In practice, our `pendingRegistration` set and `extensionMenuItems` - // should prevent this from happening - console.debug("Error registering context menu item", { error }); - return; - } - - console.error("Error registering context menu item", { error }); - throw error; - } finally { - pendingRegistration.delete(extensionId); } } diff --git a/src/background/starterBlueprints.ts b/src/background/starterBlueprints.ts index c36ff6226e..326cab306b 100644 --- a/src/background/starterBlueprints.ts +++ b/src/background/starterBlueprints.ts @@ -24,8 +24,8 @@ import { queueReactivateTab } from "@/contentScript/messenger/api"; import { ExtensionOptionsState } from "@/store/extensionsTypes"; import reportError from "@/telemetry/reportError"; import { debounce } from "lodash"; -import pMemoize from "p-memoize"; import { refreshRegistries } from "./refreshRegistries"; +import { memoizeUntilSettled } from "@/utils"; const { reducer, actions } = extensionsSlice; @@ -143,9 +143,7 @@ const _installStarterBlueprints = async (): Promise => { }; const debouncedInstallStarterBlueprints = debounce( - pMemoize(_installStarterBlueprints, { - cache: false, - }), + memoizeUntilSettled(_installStarterBlueprints), BLUEPRINT_INSTALLATION_DEBOUNCE_MS, { leading: true, diff --git a/src/utils.test.ts b/src/utils.test.ts index 5d5adf69fb..6bb54e2e02 100644 --- a/src/utils.test.ts +++ b/src/utils.test.ts @@ -24,6 +24,7 @@ import { makeURL, getScopeAndId, smartAppendPeriod, + memoizeUntilSettled, } from "@/utils"; import type { RegistryId, SafeString } from "@/core"; import { BusinessError } from "@/errors/businessErrors"; @@ -190,7 +191,7 @@ describe("makeURL", () => { ); }); - test("override existing search parameters if requested", () => { + test("override existing search parameters if called", () => { const origin = "https://pixiebrix.com?a=ORIGINAL&b=ORIGINAL&c=ORIGINAL"; expect(makeURL(origin, { a: null, c: null })).toBe( "https://pixiebrix.com/?b=ORIGINAL" @@ -271,3 +272,15 @@ describe("smartAppendPeriod", () => { } }); }); + +// From https://github.com/sindresorhus/p-memoize/blob/52fe6052ff2287f528c954c4c67fc5a61ff21360/test.ts#LL198 +test("memoizeUntilSettled", async () => { + let index = 0; + + const memoized = memoizeUntilSettled(async () => index++); + + expect(await memoized()).toBe(0); + expect(await memoized()).toBe(1); + expect(await memoized()).toBe(2); + expect(await Promise.all([memoized(), memoized()])).toStrictEqual([3, 3]); +}); diff --git a/src/utils.ts b/src/utils.ts index d02dbbb4b8..33a9f47d96 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -44,6 +44,7 @@ import { ApiVersion, RegistryId, SafeString } from "@/core"; import { UnknownObject } from "@/types"; import { RecipeDefinition } from "@/types/definitions"; import safeJsonStringify from "json-stringify-safe"; +import pMemoize from "p-memoize"; const specialCharsRegex = /[.[\]]/; @@ -249,10 +250,6 @@ export function boolean(value: unknown): boolean { return false; } -export function clone>(object: T): T { - return Object.assign(Object.create(null), object); -} - export function isObject(value: unknown): value is Record { return value && typeof value === "object"; } @@ -265,16 +262,6 @@ export function ensureJsonObject(value: Record): JsonObject { return JSON.parse(safeJsonStringify(value)) as JsonObject; } -export function clearObject(obj: Record): void { - for (const member in obj) { - if (Object.prototype.hasOwnProperty.call(obj, member)) { - // Checking to ensure own property - // eslint-disable-next-line security/detect-object-injection - delete obj[member]; - } - } -} - /** * Set values to undefined that can't be sent across the boundary between the host site context and the * content script context @@ -652,3 +639,23 @@ export function isValidUrl( return false; } } + +/** + * Ignores calls made with the same arguments while the first call is pending + * @example + * const memFetch = ignoreRepeatedCalls(fetch) + * await Promise([memFetch('/time'), memFetch('/time')]) + * // => both will return the exact same Promise + * await memFetch('/time') + * // => no concurrent calls at this time, so another request made + * + * @see https://github.com/sindresorhus/promise-fun/issues/15 + */ +export const memoizeUntilSettled: typeof pMemoize = ( + functionToMemoize, + options +) => + pMemoize(functionToMemoize, { + ...options, + cache: false, + }); From 910bff3037a5b8977b607fa1e6b301184ccaab99 Mon Sep 17 00:00:00 2001 From: Todd Schiller Date: Sun, 20 Nov 2022 21:56:32 -0500 Subject: [PATCH 2/3] Pin node to 18.x (#4687) --- package-lock.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package-lock.json b/package-lock.json index edd5849691..83eb265fdf 100644 --- a/package-lock.json +++ b/package-lock.json @@ -239,7 +239,7 @@ "zip-dir": "^2.0.0" }, "engines": { - "node": "16.x", + "node": "18.x", "npm": "8.x" } }, From 8583011220db48a9b26252f519e7eee392b8f255 Mon Sep 17 00:00:00 2001 From: Todd Schiller Date: Mon, 21 Nov 2022 21:19:30 -0500 Subject: [PATCH 3/3] #4665: support AA Community Edition on partner onboarding screen (#4669) --- src/auth/authTypes.ts | 13 ++ src/auth/authUtils.ts | 2 + src/auth/token.ts | 6 +- src/auth/useRequiredPartnerAuth.test.tsx | 68 ++++++ src/auth/useRequiredPartnerAuth.ts | 40 ++-- src/background/installer.test.ts | 220 ++++++++++++++++++ src/background/installer.ts | 175 ++++++++++++-- .../messenger/external/_implementation.ts | 10 +- .../automationanywhere/aaUtils.test.ts | 24 ++ src/contrib/automationanywhere/aaUtils.ts | 28 ++- src/options/options.html | 2 +- .../pages/onboarding/SetupPage.test.tsx | 78 +++++++ .../partner/ControlRoomTokenForm.tsx | 6 +- .../onboarding/partner/PartnerSetupCard.tsx | 43 ++-- src/services/locator.ts | 17 +- src/types/swagger.ts | 113 ++++----- 16 files changed, 714 insertions(+), 131 deletions(-) create mode 100644 src/background/installer.test.ts diff --git a/src/auth/authTypes.ts b/src/auth/authTypes.ts index d20935a22c..1914b0ed61 100644 --- a/src/auth/authTypes.ts +++ b/src/auth/authTypes.ts @@ -72,11 +72,23 @@ export type UserData = Partial<{ * @since 1.7.1 */ enforceUpdateMillis: number | null; + /** + * The partner, controlling theme, documentation links, etc. + * + * Introduced to support partner Community Edition users because they are not part of an organization. + * + * @since 1.7.14 + */ + readonly partner?: Me["partner"]; }>; // Exclude tenant information in updates (these are only updated on linking) export type UserDataUpdate = Required>; +/** + * User data keys (in addition to the token) to store in chrome.storage.local when linking the extension. + * @see updateUserData + */ export const USER_DATA_UPDATE_KEYS: Array = [ "email", "organizationId", @@ -85,6 +97,7 @@ export const USER_DATA_UPDATE_KEYS: Array = [ "groups", "flags", "enforceUpdateMillis", + "partner", ]; export interface TokenAuthData extends UserData { diff --git a/src/auth/authUtils.ts b/src/auth/authUtils.ts index 678ddbc139..5da9c79332 100644 --- a/src/auth/authUtils.ts +++ b/src/auth/authUtils.ts @@ -54,6 +54,7 @@ export function selectUserDataUpdate({ organization_memberships: organizationMemberships = [], group_memberships = [], flags = [], + partner, enforce_update_millis: enforceUpdateMillis, }: Me): UserDataUpdate { const organizations = selectOrganizations(organizationMemberships); @@ -66,6 +67,7 @@ export function selectUserDataUpdate({ flags, organizations, groups, + partner, enforceUpdateMillis, }; } diff --git a/src/auth/token.ts b/src/auth/token.ts index 3819372cac..8b4d19bfc2 100644 --- a/src/auth/token.ts +++ b/src/auth/token.ts @@ -182,16 +182,16 @@ export async function clearCachedAuthSecrets(): Promise { * @see linkExtension */ export async function updateUserData(update: UserDataUpdate): Promise { - const updated = await readAuthData(); + const result = await readAuthData(); for (const key of USER_DATA_UPDATE_KEYS) { // Intentionally overwrite values with null/undefined from the update. For some reason TypeScript was complaining // about assigning any to never. It's not clear why update[key] was being typed as never // eslint-disable-next-line security/detect-object-injection,@typescript-eslint/no-explicit-any -- keys from compile-time constant - (updated[key] as any) = update[key] as any; + (result[key] as any) = update[key] as any; } - await setStorage(STORAGE_EXTENSION_KEY, updated); + await setStorage(STORAGE_EXTENSION_KEY, result); } /** diff --git a/src/auth/useRequiredPartnerAuth.test.tsx b/src/auth/useRequiredPartnerAuth.test.tsx index 3e5411848f..cf10255edb 100644 --- a/src/auth/useRequiredPartnerAuth.test.tsx +++ b/src/auth/useRequiredPartnerAuth.test.tsx @@ -59,6 +59,7 @@ describe("useRequiredPartnerAuth", () => { data: { id: uuidv4(), partner: null, + milestones: [], }, })); @@ -95,6 +96,7 @@ describe("useRequiredPartnerAuth", () => { data: { id: uuidv4(), partner: null, + milestones: [], }, })); @@ -126,6 +128,7 @@ describe("useRequiredPartnerAuth", () => { id: uuidv4(), theme: "automation-anywhere", }, + milestones: [], organization: { control_room: { id: uuidv4(), @@ -151,6 +154,70 @@ describe("useRequiredPartnerAuth", () => { }); }); + test("requires integration for CE user", async () => { + const store = testStore(); + + (useGetMeQuery as jest.Mock).mockImplementation(() => ({ + isSuccess: true, + isLoading: false, + data: { + id: uuidv4(), + partner: { + id: uuidv4(), + theme: "automation-anywhere", + }, + milestones: [{ key: "aa_community_edition_register" }], + organization: null, + }, + })); + + const { result } = renderHook(() => useRequiredPartnerAuth(), { + wrapper: ({ children }) => {children}, + }); + + await waitForEffect(); + + expect(result.current).toStrictEqual({ + hasPartner: true, + partnerKey: "automation-anywhere", + requiresIntegration: true, + hasConfiguredIntegration: false, + isLoading: false, + error: undefined, + }); + }); + + test("does not require integration for CE user once partner is removed", async () => { + const store = testStore(); + + (useGetMeQuery as jest.Mock).mockImplementation(() => ({ + isSuccess: true, + isLoading: false, + data: { + id: uuidv4(), + // Partner becomes null once full PixieBrix is toggled on in the Admin Console + partner: null, + milestones: [{ key: "aa_community_edition_register" }], + organization: null, + }, + })); + + const { result } = renderHook(() => useRequiredPartnerAuth(), { + wrapper: ({ children }) => {children}, + }); + + await waitForEffect(); + + expect(result.current).toStrictEqual({ + hasPartner: false, + partnerKey: undefined, + requiresIntegration: false, + hasConfiguredIntegration: false, + isLoading: false, + error: undefined, + }); + }); + test("has required integration", async () => { const store = testStore({ auth: authSlice.getInitialState(), @@ -170,6 +237,7 @@ describe("useRequiredPartnerAuth", () => { isLoading: false, data: { id: uuidv4(), + milestones: [], partner: { id: uuidv4(), theme: "automation-anywhere", diff --git a/src/auth/useRequiredPartnerAuth.ts b/src/auth/useRequiredPartnerAuth.ts index 8f7bc32d38..f527cf5631 100644 --- a/src/auth/useRequiredPartnerAuth.ts +++ b/src/auth/useRequiredPartnerAuth.ts @@ -86,22 +86,22 @@ export type RequiredPartnerState = { function decidePartnerServiceIds({ authServiceIdOverride, - authMethod, + authMethodOverride, partnerId, }: { authServiceIdOverride: RegistryId | null; - authMethod: SettingsState["authMethod"]; + authMethodOverride: SettingsState["authMethod"]; partnerId: AuthState["partner"]["theme"] | null; }): Set { if (authServiceIdOverride) { return new Set([authServiceIdOverride]); } - if (authMethod === "partner-oauth2") { + if (authMethodOverride === "partner-oauth2") { return new Set([CONTROL_ROOM_OAUTH_SERVICE_ID]); } - if (authMethod === "partner-token") { + if (authMethodOverride === "partner-token") { return new Set([CONTROL_ROOM_SERVICE_ID]); } @@ -124,16 +124,18 @@ function useRequiredPartnerAuth(): RequiredPartnerState { const localAuth = useSelector(selectAuth); const { authServiceId: authServiceIdOverride, - authMethod, + authMethod: authMethodOverride, partnerId: partnerIdOverride, } = useSelector(selectSettings); const configuredServices = useSelector(selectConfiguredServices); + // Control Room URL specified by IT department during force-install const [managedControlRoomUrl] = useAsyncState( async () => readStorage(CONTROL_ROOM_URL_MANAGED_KEY, undefined, "managed"), [] ); + // Partner Id/Key specified by IT department during force-install const [managedPartnerId] = useAsyncState( async () => readStorage(PARTNER_MANAGED_KEY, undefined, "managed"), [] @@ -141,21 +143,28 @@ function useRequiredPartnerAuth(): RequiredPartnerState { // Prefer the latest remote data, but use local data to avoid blocking page load const { partner, organization } = me ?? localAuth; + // `organization?.control_room?.id` can only be set when authenticated or the auth is cached. For unauthorized users, // the organization will be null on result of useGetMeQuery const hasControlRoom = Boolean(organization?.control_room?.id) || Boolean(managedControlRoomUrl); + const isCommunityEditionUser = (me?.milestones ?? []).some( + ({ key }) => key === "aa_community_edition_register" + ); const hasPartner = - Boolean(partner) || Boolean(managedPartnerId) || hasControlRoom; + Boolean(partner) || + Boolean(managedPartnerId) || + hasControlRoom || + (Boolean(me?.partner) && isCommunityEditionUser); const partnerId = partnerIdOverride ?? managedPartnerId ?? partner?.theme ?? - (hasControlRoom ? "automation-anywhere" : null); + (hasControlRoom || isCommunityEditionUser ? "automation-anywhere" : null); const partnerServiceIds = decidePartnerServiceIds({ authServiceIdOverride, - authMethod, + authMethodOverride, partnerId, }); @@ -163,6 +172,7 @@ function useRequiredPartnerAuth(): RequiredPartnerState { partnerServiceIds.has(service.serviceId) ); + // WARNING: the logic in this method must match the logic in usePartnerLoginMode // `_` prefix so lint doesn't yell for unused variables in the destructuring const [ isMissingPartnerJwt, @@ -170,12 +180,12 @@ function useRequiredPartnerAuth(): RequiredPartnerState { _partnerJwtError, refreshPartnerJwtState, ] = useAsyncState(async () => { - if (authMethod === "pixiebrix-token") { + if (authMethodOverride === "pixiebrix-token") { // User forced pixiebrix-token authentication via Advanced Settings > Authentication Method return false; } - if (hasControlRoom || authMethod === "partner-oauth2") { + if (hasControlRoom || authMethodOverride === "partner-oauth2") { // Future improvement: check that the Control Room URL from readPartnerAuthData matches the expected // Control Room URL const { token: partnerToken } = await readPartnerAuthData(); @@ -183,7 +193,7 @@ function useRequiredPartnerAuth(): RequiredPartnerState { } return false; - }, [authMethod, localAuth, hasControlRoom]); + }, [authMethodOverride, localAuth, hasControlRoom]); useEffect(() => { // Listen for token invalidation @@ -202,11 +212,13 @@ function useRequiredPartnerAuth(): RequiredPartnerState { const requiresIntegration = // Primary organization has a partner and linked control room (hasPartner && Boolean(organization?.control_room)) || + // Community edition users are required to be linked until they join an organization + (me?.partner && isCommunityEditionUser) || // User has overridden local settings - authMethod === "partner-oauth2" || - authMethod === "partner-token"; + authMethodOverride === "partner-oauth2" || + authMethodOverride === "partner-token"; - if (authMethod === "pixiebrix-token") { + if (authMethodOverride === "pixiebrix-token") { // User forced pixiebrix-token authentication via Advanced Settings > Authentication Method. Keep the theme, // if any, but don't require a partner integration configuration. return { diff --git a/src/background/installer.test.ts b/src/background/installer.test.ts new file mode 100644 index 0000000000..841e677b73 --- /dev/null +++ b/src/background/installer.test.ts @@ -0,0 +1,220 @@ +/* + * Copyright (C) 2022 PixieBrix, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +import { requirePartnerAuth, openInstallPage } from "@/background/installer"; +import * as auth from "@/auth/token"; +import { locator } from "@/background/locator"; +import { uuidv4 } from "@/types/helpers"; + +const APP_BASE_URL = "https://app.pixiebrix.com"; + +jest.mock("@/services/baseService", () => ({ + // Can't use APP_BASE_URL because it's not defined yet when Jest defines the mock + getBaseURL: jest.fn().mockResolvedValue("https://app.pixiebrix.com"), +})); + +jest.mock("@/auth/token", () => ({ + isLinked: jest.fn().mockResolvedValue(false), + getExtensionToken: jest.fn().mockResolvedValue(null), + getUserData: jest.fn().mockResolvedValue(null), +})); + +jest.mock("@/background/locator", () => ({ + locator: { + locateAllForService: jest.fn().mockResolvedValue([]), + }, +})); + +const createTabMock = browser.tabs.create as jest.Mock; +const updateTabMock = browser.tabs.update as jest.Mock; +const queryTabsMock = browser.tabs.query as jest.Mock; +const getExtensionUrlMock = browser.runtime.getURL as jest.Mock; +const isLinkedMock = auth.isLinked as jest.Mock; +const getExtensionTokenMock = auth.getExtensionToken as jest.Mock; +const getUserData = auth.getUserData as jest.Mock; +const locateAllForServiceMock = locator.locateAllForService as jest.Mock; + +beforeEach(() => { + getExtensionUrlMock.mockImplementation( + (resource: string) => `chrome-extension://abc123/${resource}` + ); + jest.clearAllMocks(); +}); + +describe("openInstallPage", () => { + it("Redirects Admin Console tab for native PixieBrix setup flow", async () => { + queryTabsMock.mockResolvedValue([ + { + id: 1, + url: `${APP_BASE_URL}/setup`, + }, + ]); + await openInstallPage(); + expect(updateTabMock).toHaveBeenCalledWith(1, { + url: APP_BASE_URL, + active: true, + }); + expect(createTabMock.mock.calls.length).toBe(0); + }); + + it("Opens Extension Console in same tab for enterprise partner", async () => { + queryTabsMock.mockResolvedValue([ + { + id: 1, + url: `${APP_BASE_URL}/start?hostname=enterprise.com`, + }, + ]); + await openInstallPage(); + expect(updateTabMock).toHaveBeenCalledWith(1, { + url: "chrome-extension://abc123/options.html#/start?hostname=enterprise.com", + active: true, + }); + expect(createTabMock.mock.calls.length).toBe(0); + }); + + it("Opens Admin Console in same tab for community partner", async () => { + queryTabsMock.mockResolvedValue([ + { + id: 1, + url: `${APP_BASE_URL}/start?hostname=community2.cloud-2.automationanywhere.digital`, + }, + ]); + await openInstallPage(); + expect(updateTabMock).toHaveBeenCalledWith(1, { + url: APP_BASE_URL, + active: true, + }); + expect(createTabMock.mock.calls.length).toBe(0); + }); + + it("Opens new Extension Console tab if no Admin Console onboarding tab found", async () => { + queryTabsMock.mockResolvedValue([]); + await openInstallPage(); + expect(createTabMock).toHaveBeenCalledWith({ url: APP_BASE_URL }); + expect(updateTabMock.mock.calls.length).toBe(0); + }); +}); + +describe("checkPartnerAuth", () => { + it("skips if not linked", async () => { + isLinkedMock.mockResolvedValue(false); + + await requirePartnerAuth(); + + expect(createTabMock.mock.calls.length).toBe(0); + expect(updateTabMock.mock.calls.length).toBe(0); + }); + + it("skip if no partner", async () => { + isLinkedMock.mockResolvedValue(true); + getExtensionTokenMock.mockResolvedValue("abc123"); + getUserData.mockResolvedValue({ + partner: null, + }); + + await requirePartnerAuth(); + + expect(createTabMock.mock.calls.length).toBe(0); + expect(updateTabMock.mock.calls.length).toBe(0); + }); + + it("skip if partner JWT install", async () => { + isLinkedMock.mockResolvedValue(true); + getExtensionTokenMock.mockResolvedValue(null); + getUserData.mockResolvedValue({ + partner: { + id: uuidv4(), + theme: "automation-anywhere", + }, + }); + + await requirePartnerAuth(); + + expect(createTabMock.mock.calls.length).toBe(0); + expect(updateTabMock.mock.calls.length).toBe(0); + }); + + it("opens extension console if linked with partner and no services", async () => { + queryTabsMock.mockResolvedValue([]); + isLinkedMock.mockResolvedValue(true); + getExtensionTokenMock.mockResolvedValue("abc123"); + locateAllForServiceMock.mockResolvedValue([ + // Include a cloud configuration to clarify that local integration is still required + { id: uuidv4(), serviceId: "automation-anywhere", proxy: true }, + ]); + getUserData.mockResolvedValue({ + partner: { + id: uuidv4(), + theme: "automation-anywhere", + }, + }); + + await requirePartnerAuth(); + + expect(createTabMock.mock.calls.length).toBe(1); + expect(createTabMock).toHaveBeenCalledWith({ + url: "chrome-extension://abc123/options.html", + }); + expect(updateTabMock.mock.calls.length).toBe(0); + }); + + it("opens extension console in same tab if linked with partner and no services and extension console open", async () => { + queryTabsMock.mockResolvedValue([ + { + id: 1, + url: APP_BASE_URL, + }, + ]); + isLinkedMock.mockResolvedValue(true); + getExtensionTokenMock.mockResolvedValue("abc123"); + getUserData.mockResolvedValue({ + partner: { + id: uuidv4(), + theme: "automation-anywhere", + }, + }); + + await requirePartnerAuth(); + + expect(createTabMock.mock.calls.length).toBe(0); + expect(updateTabMock.mock.calls.length).toBe(1); + expect(updateTabMock).toHaveBeenCalledWith(1, { + url: "chrome-extension://abc123/options.html", + active: true, + }); + }); + + it("does not open extension console if integration is configured", async () => { + queryTabsMock.mockResolvedValue([]); + isLinkedMock.mockResolvedValue(true); + getExtensionTokenMock.mockResolvedValue("abc123"); + locateAllForServiceMock.mockResolvedValue([ + { id: uuidv4(), serviceId: "automation-anywhere" }, + ]); + getUserData.mockResolvedValue({ + partner: { + id: uuidv4(), + theme: "automation-anywhere", + }, + }); + + await requirePartnerAuth(); + + expect(createTabMock.mock.calls.length).toBe(0); + expect(updateTabMock.mock.calls.length).toBe(0); + }); +}); diff --git a/src/background/installer.ts b/src/background/installer.ts index 79a3377c83..f6120ca6c5 100644 --- a/src/background/installer.ts +++ b/src/background/installer.ts @@ -15,6 +15,7 @@ * along with this program. If not, see . */ +import { locator as serviceLocator } from "@/background/locator"; import { Runtime } from "webextension-polyfill"; import { reportEvent } from "@/telemetry/events"; import { initTelemetry } from "@/background/telemetry"; @@ -22,7 +23,11 @@ import { getUID } from "@/background/messenger/api"; import { DNT_STORAGE_KEY, allowsTrack } from "@/telemetry/dnt"; import { gt } from "semver"; import { getBaseURL } from "@/services/baseService"; -import { isLinked } from "@/auth/token"; +import { getExtensionToken, getUserData, isLinked } from "@/auth/token"; +import { isCommunityControlRoom } from "@/contrib/automationanywhere/aaUtils"; +import { isEmpty } from "lodash"; +import { expectContext } from "@/utils/expectContext"; +import { AUTOMATION_ANYWHERE_SERVICE_ID } from "@/contrib/automationanywhere/contract"; const UNINSTALL_URL = "https://www.pixiebrix.com/uninstall/"; @@ -34,10 +39,20 @@ const SERVICE_URL = process.env.SERVICE_URL; */ let _availableVersion: string | null = null; -async function openInstallPage() { - const [url, [accountTab]] = await Promise.all([ +/** + * Install handler to complete authentication configuration for the extension. + */ +export async function openInstallPage() { + expectContext("background"); + + // Look for an Admin Console tab that's showing an onboarding page + // /setup: normal onboarding screen + // /start: partner onboarding + const [appBaseUrl, [appOnboardingTab]] = await Promise.all([ getBaseURL(), browser.tabs.query({ + // Can't use SERVICE_URL directly because it contains a port number during development, resulting in an + // invalid URL match pattern url: [ new URL("setup", SERVICE_URL).href, `${new URL("start", SERVICE_URL).href}?*`, @@ -45,40 +60,160 @@ async function openInstallPage() { }), ]); - if (accountTab) { - const accountTabUrl = new URL(accountTab.url); + // There are 4 cases: + // Case 1a: there's an Admin Console partner onboarding tab showing an enterprise onboarding flow (/start) + // Case 1b: there's an Admin Console partner onboarding tab showing a community onboarding flow (/start) + // Case 2: there's an Admin Console native onboarding tab (/setup) + // Case 3: there's no Admin Console onboarding tab open + + if (appOnboardingTab) { + const appOnboardingTabUrl = new URL(appOnboardingTab.url); + + if (appOnboardingTabUrl.pathname === "/start") { + // Case 1a/1b: Admin Console is showing a partner onboarding flow + + const controlRoomHostname = + appOnboardingTabUrl.searchParams.get("hostname"); + + if ( + !isEmpty(controlRoomHostname) && + !isCommunityControlRoom(controlRoomHostname) + ) { + // Case 1a: Admin Console is showing enterprise onboarding flow + // + // Show the Extension Console /start page, where the user will be prompted to use OAuth2 to connect their + // AARI account. Include the Control Room hostname in the URL so that the ControlRoomOAuthForm can pre-fill + // the URL + const extensionStartUrl = new URL( + browser.runtime.getURL("options.html") + ); + extensionStartUrl.hash = `/start${appOnboardingTabUrl.search}`; + + await browser.tabs.update(appOnboardingTab.id, { + url: extensionStartUrl.href, + active: true, + }); - if (accountTabUrl.pathname === "/start") { - const extensionStartUrl = new URL(browser.runtime.getURL("options.html")); - extensionStartUrl.hash = `/start${new URL(accountTab.url).search}`; + return; + } - await browser.tabs.update(accountTab.id, { - url: extensionStartUrl.href, + // Case 1b: Admin Console is showing community onboarding flow + // + // Redirect to the main Admin Console page, which automatically "links" the extension (by passing the PixieBrix + // token to the extension). + // + // When the extension is linked, the extension reloads itself. On restart, it will automatically show the + // screen to configure the required AA integration. See installer:checkPartnerAuth below + // + // Reuse the tab that is part of the Admin Console onboarding flow to avoid multiple PixieBrix tabs. + // See discussion here: https://github.com/pixiebrix/pixiebrix-extension/pull/3506 + await browser.tabs.update(appOnboardingTab.id, { + url: appBaseUrl, active: true, }); + return; } - // Automatically reuse the tab that is part of the onboarding flow - // https://github.com/pixiebrix/pixiebrix-extension/pull/3506 - await browser.tabs.update(accountTab.id, { url }); + // Case 2: the Admin Console is showing the native PixieBrix onboarding tab. + // + // Redirect to the main Admin Console page, which automatically "links" the extension (by passing the PixieBrix + // token to the extension). + // + // Reuse the tab that is part of the Admin Console onboarding flow to avoid multiple PixieBrix tabs. + // See discussion here: https://github.com/pixiebrix/pixiebrix-extension/pull/3506 + await browser.tabs.update(appOnboardingTab.id, { + url: appBaseUrl, + active: true, + }); } else { - await browser.tabs.create({ url }); + // Case 3: there's no Admin Console onboarding tab open + // + // Open a new Admin Console tab which will automatically "links" the extension (by passing the native PixieBrix + // token to the extension). + await browser.tabs.create({ url: appBaseUrl }); } } -async function install({ reason }: Runtime.OnInstalledDetailsType) { +/** + * For partner installs, if a partner integration is not configured, automatically open the Extension Console + * to continue the partner authentication onboarding flow. + * + * @see useRequiredPartnerAuth + */ +export async function requirePartnerAuth(): Promise { + expectContext("background"); + + // Check for partner community edition install, where the extension is linked with the native PixieBrix token, but + // the partner integration is not configured yet. + // + // Use getExtensionToken instead of isLinked, because isLinked returns true for partner JWT also + if (!isEmpty(await getExtensionToken())) { + const userData = await getUserData(); + + console.debug("requirePartnerAuth", userData); + + if (userData.partner?.theme === "automation-anywhere") { + const configs = await serviceLocator.locateAllForService( + AUTOMATION_ANYWHERE_SERVICE_ID + ); + + if (!configs.some((x) => !x.proxy)) { + const extensionConsoleUrl = browser.runtime.getURL("options.html"); + + // Replace the Admin Console tab, if available. The Admin Console tab will be available during openInstallPage + const [adminConsoleTab] = await browser.tabs.query({ + // Can't use SERVICE_URL directly because it contains a port number during development, resulting in an + // invalid URL match pattern + url: [new URL(SERVICE_URL).href], + }); + + if (adminConsoleTab) { + await browser.tabs.update(adminConsoleTab.id, { + url: extensionConsoleUrl, + active: true, + }); + } else { + await browser.tabs.create({ url: extensionConsoleUrl }); + } + } + } + } +} + +async function install({ + reason, + previousVersion, +}: Runtime.OnInstalledDetailsType) { + // https://developer.chrome.com/docs/extensions/reference/runtime/#event-onInstalled + // https://developer.chrome.com/docs/extensions/reference/runtime/#type-OnInstalledReason + + console.debug("onInstalled", { reason, previousVersion }); + const { version } = browser.runtime.getManifest(); + if (reason === "install") { reportEvent("PixieBrixInstall", { - version: browser.runtime.getManifest().version, + version, }); + + // XXX: under what conditions could onInstalled fire, but the extension is already linked? if (!(await isLinked())) { void openInstallPage(); } } else if (reason === "update") { - reportEvent("PixieBrixUpdate", { - version: browser.runtime.getManifest().version, - }); + // `update` is also triggered on browser.runtime.reload() and manually reloading from the extensions page + void requirePartnerAuth(); + + if (version === previousVersion) { + reportEvent("PixieBrixReload", { + version, + }); + } else { + reportEvent("PixieBrixUpdate", { + version, + previousVersion, + }); + } } } @@ -107,7 +242,7 @@ async function setUninstallURL(): Promise { url.searchParams.set("uid", await getUID()); } - // We always want to show the uninstallation page so the user can optionally fill out the uninstallation form + // We always want to show the uninstallation page so the user can optionally fill out the uninstallation survey await browser.runtime.setUninstallURL(url.href); } diff --git a/src/background/messenger/external/_implementation.ts b/src/background/messenger/external/_implementation.ts index b3b9aacd9f..cdcd6b416b 100644 --- a/src/background/messenger/external/_implementation.ts +++ b/src/background/messenger/external/_implementation.ts @@ -23,16 +23,22 @@ import { linkExtension } from "@/auth/token"; import { TokenAuthData } from "@/auth/authTypes"; import { reportEvent } from "@/telemetry/events"; +const HACK_EXTENSION_LINK_RELOAD_DELAY_MS = 100; + export async function setExtensionAuth(auth: TokenAuthData) { const updated = await linkExtension(auth); if (updated) { reportEvent("LinkExtension"); + console.debug( + `Extension link updated, reloading extension in ${HACK_EXTENSION_LINK_RELOAD_DELAY_MS}ms` + ); - // A hack to ensure the SET_EXTENSION_AUTH response flows to the front-end before the backend + // A hack to ensure the SET_EXTENSION_AUTH messenger response flows to the front-end before the backend // page is reloaded. setTimeout(async () => { + console.debug("Reloading extension due to extension link update"); browser.runtime.reload(); - }, 100); + }, HACK_EXTENSION_LINK_RELOAD_DELAY_MS); } return updated; diff --git a/src/contrib/automationanywhere/aaUtils.test.ts b/src/contrib/automationanywhere/aaUtils.test.ts index b78dac078d..7c5b07613b 100644 --- a/src/contrib/automationanywhere/aaUtils.test.ts +++ b/src/contrib/automationanywhere/aaUtils.test.ts @@ -1,4 +1,5 @@ import { + hostnameToUrl, interfaceToInputSchema, isCommunityControlRoom, selectBotOutput, @@ -13,6 +14,13 @@ describe("isCommunityControlRoom", () => { expect(isCommunityControlRoom(url)).toBeTruthy(); }); + test.each([ + ["community2.cloud-2.automationanywhere.digital/"], + ["community.cloud.automationanywhere.digital"], + ])("detect community hostname: %s", (url: string) => { + expect(isCommunityControlRoom(url)).toBeTruthy(); + }); + test("detect enterprise cloud URL", () => { expect( isCommunityControlRoom("https://aa-dev-1.my.automationanywhere.digital/") @@ -20,6 +28,22 @@ describe("isCommunityControlRoom", () => { }); }); +describe("hostnameToUrl", () => { + test("nop for HTTPS URL", () => { + expect(hostnameToUrl("https://example.com")).toBe("https://example.com"); + }); + + test("nop for HTTP URL", () => { + expect(hostnameToUrl("http://foo.example.com")).toBe( + "http://foo.example.com" + ); + }); + + test("prefixes HTTPS:", () => { + expect(hostnameToUrl("foo.example.com")).toBe("https://foo.example.com"); + }); +}); + describe("interfaceToInputSchema", () => { test("convert to JSON Schema", () => { const schema = interfaceToInputSchema({ diff --git a/src/contrib/automationanywhere/aaUtils.ts b/src/contrib/automationanywhere/aaUtils.ts index 80434f9dab..c1f910fd7e 100644 --- a/src/contrib/automationanywhere/aaUtils.ts +++ b/src/contrib/automationanywhere/aaUtils.ts @@ -29,15 +29,31 @@ import { mapValues } from "lodash"; import { Primitive } from "type-fest"; import { BusinessError } from "@/errors/businessErrors"; -const COMMUNITY_HOSTNAME_REGEX = - /^community\d*\..*\.automationanywhere\.digital$/; +const COMMUNITY_CONTROL_ROOM_REGEX = + /^(https:\/\/)?community\d*\.\S+\.automationanywhere\.digital\/?$/; /** - * Return true if the control room URL corresponds to the AA community cloud. + * Returns true if the argument corresponds to an Automation Anywhere Community Edition Control Room. + * + * Returns false for malformed URLs, instead of throwing an error. + * + * @param hostnameOrUrl */ -export function isCommunityControlRoom(url: string): boolean { - const parsed = new URL(url); - return COMMUNITY_HOSTNAME_REGEX.test(parsed.hostname); +export function isCommunityControlRoom(hostnameOrUrl: string | null): boolean { + return COMMUNITY_CONTROL_ROOM_REGEX.test(hostnameOrUrl ?? ""); +} + +export function hostnameToUrl(hostname: string): string { + if (hostname == null) { + // Give hint to user to include https: scheme + return "https://"; + } + + if (/^[\da-z]+:\/\//.test(hostname)) { + return hostname; + } + + return `https://${hostname}`; } function selectDefaultValue(variable: Variable): JSONSchema7Type { diff --git a/src/options/options.html b/src/options/options.html index fdaa25d2ab..4727656b09 100644 --- a/src/options/options.html +++ b/src/options/options.html @@ -22,7 +22,7 @@ - Browser Extension | PixieBrix + Extension Console | PixieBrix { ).toStrictEqual("https://mycontrolroom.com"); }); + test("Start URL with Community Edition hostname if user is unauthenticated", async () => { + // User is authenticated + (useGetMeQuery as jest.Mock).mockImplementation(() => ({ + isLoading: false, + data: {}, + })); + + const history = createHashHistory(); + // Hostname comes as hostname, not URL + history.push( + "/start?hostname=community2.cloud-2.automationanywhere.digital" + ); + + // Needs to use HashRouter instead of MemoryRouter for the useLocation calls in the components to work correctly + // given the URL structure above + render( + + + + + + ); + + await waitFor(() => { + expect(screen.queryByTestId("loader")).toBeNull(); + }); + + expect(screen.getByTestId("link-account-btn")).not.toBeNull(); + expect(screen.queryByTestId("connect-aari-btn")).toBeNull(); + }); + + test("Start URL with Community Edition hostname if authenticated", async () => { + // User is authenticated + (useGetMeQuery as jest.Mock).mockImplementation(() => ({ + isLoading: false, + data: { + id: uuidv4(), + partner: { + id: uuidv4(), + theme: "automation-anywhere", + }, + }, + })); + + const history = createHashHistory(); + // Hostname comes as hostname, not URL + history.push( + "/start?hostname=community2.cloud-2.automationanywhere.digital" + ); + + // Needs to use HashRouter instead of MemoryRouter for the useLocation calls in the components to work correctly + // given the URL structure above + render( + + + + + + ); + + await waitFor(() => { + expect(screen.queryByTestId("loader")).toBeNull(); + }); + + expect(screen.queryByTestId("link-account-btn")).toBeNull(); + + expect(screen.getByText("Connect your AARI account")).not.toBeNull(); + expect( + screen.getByLabelText("Control Room URL").getAttribute("value") + // Schema get pre-pended automatically + ).toStrictEqual("https://community2.cloud-2.automationanywhere.digital"); + + expect( + screen.getByLabelText("Username") + // Schema get pre-pended automatically + ).not.toBeNull(); + }); + test("Managed Storage OAuth2 partner user", async () => { // User will be unauthenticated (useGetMeQuery as jest.Mock).mockImplementation(() => ({ diff --git a/src/options/pages/onboarding/partner/ControlRoomTokenForm.tsx b/src/options/pages/onboarding/partner/ControlRoomTokenForm.tsx index 9ee1010a36..4c290e8f19 100644 --- a/src/options/pages/onboarding/partner/ControlRoomTokenForm.tsx +++ b/src/options/pages/onboarding/partner/ControlRoomTokenForm.tsx @@ -93,7 +93,11 @@ const ControlRoomTokenForm: React.FunctionComponent<{ const renderSubmit: RenderSubmit = ({ isSubmitting, isValid }) => (
-
diff --git a/src/options/pages/onboarding/partner/PartnerSetupCard.tsx b/src/options/pages/onboarding/partner/PartnerSetupCard.tsx index e91ba03e6a..70ed03bb17 100644 --- a/src/options/pages/onboarding/partner/PartnerSetupCard.tsx +++ b/src/options/pages/onboarding/partner/PartnerSetupCard.tsx @@ -33,6 +33,10 @@ import { getBaseURL } from "@/services/baseService"; import settingsSlice from "@/store/settingsSlice"; import { ManualStorageKey, readStorage } from "@/chrome"; import { useLocation } from "react-router"; +import { + hostnameToUrl, + isCommunityControlRoom, +} from "@/contrib/automationanywhere/aaUtils"; function useInstallUrl() { const { data: me } = useGetMeQuery(); @@ -60,10 +64,21 @@ function useInstallUrl() { /** * Helper method to decide partner authentication method given local settings overrides, and current login state. + * + * @see useRequiredPartnerAuth */ function usePartnerLoginMode(): "token" | "oauth2" { + // WARNING: the logic in this method must match the logic in useRequiredPartnerAuth, otherwise logging in using + // the method determined here will not result in the user passing the partner auth gate. + + // Make sure to use useLocation because the location.search are on the hash route + const location = useLocation(); + const { authMethod } = useSelector(selectSettings); - const hasCachedLoggedIn = useSelector(selectIsLoggedIn); + const hasCachedNativeLogin = useSelector(selectIsLoggedIn); + + // Hostname passed from manual flow during manual setup initiated via Control Room link + const hostname = new URLSearchParams(location.search).get("hostname"); switch (authMethod) { case "partner-token": { @@ -81,28 +96,17 @@ function usePartnerLoginMode(): "token" | "oauth2" { } default: { - // If the user is logged in using PixieBrix, show the token configuration screen. They'll keep using their + // If the user is logged in using PixieBrix, show the AA token configuration screen. They'll keep using their // PixieBrix token login instead of switching to the AA JWT login - return hasCachedLoggedIn ? "token" : "oauth2"; + return hasCachedNativeLogin || isCommunityControlRoom(hostname) + ? "token" + : "oauth2"; } } } const CONTROL_ROOM_URL_MANAGED_KEY = "controlRoomUrl" as ManualStorageKey; -function hostnameToUrl(hostname: string): string { - if (hostname == null) { - // Give hint to user to include https: scheme - return "https://"; - } - - if (/^[\da-z]+:\/\//.test(hostname)) { - return hostname; - } - - return `https://${hostname}`; -} - /** * A card to set up a required partner integration. * @@ -122,6 +126,7 @@ const PartnerSetupCard: React.FunctionComponent = () => { // Prefer controlRoomUrl set by IT for force-installed extensions const fallbackControlRoomUrl = hostnameToUrl(hostname) ?? me?.organization?.control_room?.url ?? ""; + const [controlRoomUrl] = useAsyncState( async () => { try { @@ -187,7 +192,11 @@ const PartnerSetupCard: React.FunctionComponent = () => { title="Link the extension to a PixieBrix account" active > - diff --git a/src/services/locator.ts b/src/services/locator.ts index d5f8033868..7ba58feb20 100644 --- a/src/services/locator.ts +++ b/src/services/locator.ts @@ -38,6 +38,8 @@ import { MissingConfigurationError, NotConfiguredError, } from "@/errors/businessErrors"; +import { DoesNotExistError } from "@/baseRegistry"; +import { Service } from "@/types"; const REF_SECRETS = [ "https://app.pixiebrix.com/schemas/key#", @@ -215,7 +217,20 @@ class LazyLocatorFactory { return [await pixieServiceFactory()]; } - const service = await registry.lookup(serviceId); + let service: Service; + + // Handle case where locateAllForService is called before service definitions are loaded. (For example, because it's + // being called from the background page in installer.ts). + // In the future, we may want to expose an option on the method to control this behavior. + try { + service = await registry.lookup(serviceId); + } catch (error) { + if (error instanceof DoesNotExistError) { + return []; + } + + throw error; + } return this.options .filter((x) => x.serviceId === serviceId) diff --git a/src/types/swagger.ts b/src/types/swagger.ts index 82aba58a80..90f089e268 100644 --- a/src/types/swagger.ts +++ b/src/types/swagger.ts @@ -134,10 +134,6 @@ export interface paths { /** List config of current version of each package. */ get: operations["listExtentionPoints"]; }; - "/api/extension/token/": { - /** Return the token for the current user. */ - get: operations["retrieveExtensionToken"]; - }; "/api/extensions/": { get: operations["listUserExtensions"]; }; @@ -203,6 +199,10 @@ export interface paths { get: operations["retrieveMe"]; delete: operations["destroyMe"]; }; + "/api/me/token/": { + /** Return the token for the current user. */ + get: operations["retrieveMeToken"]; + }; "/api/memberships/{id}/": { /** Detail view for an organization's memberships. */ get: operations["retrieveOrganizationMembership"]; @@ -272,9 +272,6 @@ export interface paths { "/api/organizations/{organization_pk}/subscriptions/": { get: operations["listSubscriptions"]; }; - "/api/organizations/{organization_pk}/subscriptions/jobs/{id}/": { - get: operations["retrieveOrganizationSubscriptionsJob"]; - }; "/api/organizations/{organization_pk}/backup/": { get: operations["exportOrganizationBackup"]; }; @@ -312,7 +309,7 @@ export interface paths { /** List config of current version of each package. */ get: operations["listRecipes"]; }; - "/api/recipes/{name}": { + "/api/recipes/{name}/": { get: operations["retrievePackageConfig"]; }; "/api/services/shared/": { @@ -396,12 +393,12 @@ export interface paths { "/api/invitations/{id}/reject/": { post: operations["rejectPendingInvitation"]; }; + "/api/me/milestones/": { + post: operations["createMilestone"]; + }; "/api/onboarding/": { post: operations["createOnboarding"]; }; - "/api/organizations/{organization_pk}/subscriptions/jobs/": { - post: operations["createOrganizationSubscriptionsJob"]; - }; "/api/control-rooms/configurations/": { post: operations["createControlRoomConfiguration"]; }; @@ -822,9 +819,6 @@ export interface components { notify_error?: boolean; notify_uninstall?: boolean; }; - ExtensionToken: { - extension_token: string; - }; UserExtension: { /** Format: uuid */ id: string; @@ -1012,8 +1006,6 @@ export interface components { /** Format: date-time */ updated_at?: string; }[]; - depends_on?: string; - used_by?: string; image: { /** @description Alt text for the logo */ alt_text?: string | null; @@ -1048,6 +1040,7 @@ export interface components { updated_at?: string; }; Me: { + flags?: string[]; /** Format: uuid */ id?: string; scope?: string | null; @@ -1072,7 +1065,7 @@ export interface components { show_sidebar_logo?: boolean; /** * Format: uri - * @description The image url of a custom logo. Image format must be svg. + * @description The image url of a custom logo. Image format must be svg or png. */ logo?: string | null; }; @@ -1095,7 +1088,7 @@ export interface components { show_sidebar_logo?: boolean; /** * Format: uri - * @description The image url of a custom logo. Image format must be svg. + * @description The image url of a custom logo. Image format must be svg or png. */ logo?: string | null; }; @@ -1126,10 +1119,12 @@ export interface components { id: string; name: string; }[]; - is_onboarded?: string; + is_onboarded?: boolean; + milestones: { + key: string; + }[]; /** @description True if the account is an organization API service account */ service_account?: boolean; - flags?: string; partner?: { /** Format: uuid */ id?: string; @@ -1140,6 +1135,9 @@ export interface components { }; enforce_update_millis?: number; }; + MeToken: { + token?: string; + }; Membership: { id?: number; user?: { @@ -1222,7 +1220,7 @@ export interface components { show_sidebar_logo?: boolean; /** * Format: uri - * @description The image url of a custom logo. Image format must be svg. + * @description The image url of a custom logo. Image format must be svg or png. */ logo?: string | null; }; @@ -1478,6 +1476,9 @@ export interface components { uid?: string; data?: { [key: string]: unknown }; }; + Milestone: { + key: string; + }; Onboarding: { external?: boolean; }; @@ -2633,18 +2634,6 @@ export interface operations { }; }; }; - /** Return the token for the current user. */ - retrieveExtensionToken: { - parameters: {}; - responses: { - 200: { - content: { - "application/json; version=1.0": components["schemas"]["ExtensionToken"]; - "application/vnd.pixiebrix.api+json; version=1.0": components["schemas"]["ExtensionToken"]; - }; - }; - }; - }; listUserExtensions: { parameters: { query: { @@ -3231,6 +3220,18 @@ export interface operations { 204: never; }; }; + /** Return the token for the current user. */ + retrieveMeToken: { + parameters: {}; + responses: { + 200: { + content: { + "application/json; version=1.0": components["schemas"]["MeToken"]; + "application/vnd.pixiebrix.api+json; version=1.0": components["schemas"]["MeToken"]; + }; + }; + }; + }; /** Detail view for an organization's memberships. */ retrieveOrganizationMembership: { parameters: { @@ -3797,22 +3798,6 @@ export interface operations { }; }; }; - retrieveOrganizationSubscriptionsJob: { - parameters: { - path: { - organization_pk: string; - id: string; - }; - }; - responses: { - 200: { - content: { - "application/json; version=1.0": components["schemas"]["Job"]; - "application/vnd.pixiebrix.api+json; version=1.0": components["schemas"]["Job"]; - }; - }; - }; - }; exportOrganizationBackup: { parameters: { path: { @@ -4665,43 +4650,39 @@ export interface operations { }; }; }; - createOnboarding: { + createMilestone: { parameters: {}; responses: { 201: { content: { - "application/json; version=1.0": components["schemas"]["Onboarding"]; - "application/vnd.pixiebrix.api+json; version=1.0": components["schemas"]["Onboarding"]; + "application/json; version=1.0": components["schemas"]["Milestone"]; + "application/vnd.pixiebrix.api+json; version=1.0": components["schemas"]["Milestone"]; }; }; }; requestBody: { content: { - "application/json": components["schemas"]["Onboarding"]; - "application/x-www-form-urlencoded": components["schemas"]["Onboarding"]; - "multipart/form-data": components["schemas"]["Onboarding"]; + "application/json": components["schemas"]["Milestone"]; + "application/x-www-form-urlencoded": components["schemas"]["Milestone"]; + "multipart/form-data": components["schemas"]["Milestone"]; }; }; }; - createOrganizationSubscriptionsJob: { - parameters: { - path: { - organization_pk: string; - }; - }; + createOnboarding: { + parameters: {}; responses: { 201: { content: { - "application/json; version=1.0": components["schemas"]["Job"]; - "application/vnd.pixiebrix.api+json; version=1.0": components["schemas"]["Job"]; + "application/json; version=1.0": components["schemas"]["Onboarding"]; + "application/vnd.pixiebrix.api+json; version=1.0": components["schemas"]["Onboarding"]; }; }; }; requestBody: { content: { - "application/json": components["schemas"]["Job"]; - "application/x-www-form-urlencoded": components["schemas"]["Job"]; - "multipart/form-data": components["schemas"]["Job"]; + "application/json": components["schemas"]["Onboarding"]; + "application/x-www-form-urlencoded": components["schemas"]["Onboarding"]; + "multipart/form-data": components["schemas"]["Onboarding"]; }; }; };