-
Notifications
You must be signed in to change notification settings - Fork 25
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
#7618: increase managed storage delay and link extension immediately for CWS installs #7628
Changes from 11 commits
6a3f7bb
ce02163
929e2cc
809bd22
a0bf24f
9179f05
aa36b1d
7a20f9b
05283b0
f65e55e
27ee2a6
7d21b0b
972c762
8ce47de
8700c22
3d70082
61857c9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -27,7 +27,10 @@ import { getExtensionToken, getUserData, isLinked } from "@/auth/token"; | |
import { isCommunityControlRoom } from "@/contrib/automationanywhere/aaUtils"; | ||
import { isEmpty } from "lodash"; | ||
import { expectContext } from "@/utils/expectContext"; | ||
import { readManagedStorage } from "@/store/enterprise/managedStorage"; | ||
import { | ||
readManagedStorage, | ||
isInitialized as isManagedStorageInitialized, | ||
} from "@/store/enterprise/managedStorage"; | ||
import { Events } from "@/telemetry/events"; | ||
|
||
import { DEFAULT_SERVICE_URL, UNINSTALL_URL } from "@/urlConstants"; | ||
|
@@ -40,6 +43,35 @@ import { getExtensionConsoleUrl } from "@/utils/extensionUtils"; | |
*/ | ||
let _availableVersion: string | null = null; | ||
|
||
/** | ||
* Returns true if this appears to be a Chrome Web Store install and/or the user has an app URL where they're | ||
* authenticated so the extension can be linked. | ||
*/ | ||
async function isEndUserInstall(): Promise<boolean> { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There's no great way to test this method in Jest, b/c it would just involve mocking browser.tabs.query. So, we will rely on the Rainforest QA tests and manual testing There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: I might add "likely" to the name of this func, e.g. |
||
// Query existing app/CWS tabs: https://developer.chrome.com/docs/extensions/reference/api/tabs#method-query | ||
// `browser.tabs.query` supports https://developer.chrome.com/docs/extensions/develop/concepts/match-patterns | ||
const likelyOnboardingTabs = 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: [ | ||
// Setup page is page before sending user to the CWS | ||
new URL("setup", DEFAULT_SERVICE_URL).href, | ||
// Base page is extension linking page. Needs path to be a valid URL match pattern | ||
new URL("", DEFAULT_SERVICE_URL).href, | ||
// Known CWS URLs: https://docs.pixiebrix.com/enterprise-it-setup/network-email-firewall-configuration | ||
"https://chromewebstore.google.com/*", | ||
"https://chrome.google.com/webstore/*", | ||
], | ||
}); | ||
|
||
// The CWS install URL differs based on the extension listing slug. So instead, only match on the runtime id. | ||
return likelyOnboardingTabs.some( | ||
(tab) => | ||
tab.url.includes(DEFAULT_SERVICE_URL) || | ||
tab.url.includes(browser.runtime.id), | ||
); | ||
} | ||
|
||
/** | ||
* Install handler to complete authentication configuration for the extension. | ||
*/ | ||
|
@@ -127,7 +159,7 @@ export async function openInstallPage() { | |
active: true, | ||
}); | ||
} else { | ||
// Case 3: there's no Admin Console onboarding tab open | ||
// 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). | ||
|
@@ -199,18 +231,31 @@ export async function handleInstall({ | |
// XXX: under what conditions could onInstalled fire, but the extension is already linked? Is this the case during | ||
// development/loading an update of the extension from the file system? | ||
if (!(await isLinked())) { | ||
// PERFORMANCE: readManagedStorageByKey waits up to 2 seconds for managed storage to be available. Shouldn't be | ||
// notice-able for end-user relative to the extension download/install time | ||
const { ssoUrl, partnerId, controlRoomUrl, disableLoginTab } = | ||
await readManagedStorage(); | ||
// If an end-user appears to be installing, jump to linking directly vs. waiting for readManagedStorage because | ||
// readManagedStorage will wait until a timeout for managed storage to be available. | ||
if (!isManagedStorageInitialized() && (await isEndUserInstall())) { | ||
console.debug("Skipping readManagedStorage for end-user install"); | ||
|
||
await openInstallPage(); | ||
return; | ||
} | ||
|
||
// Reminder: readManagedStorageByKey waits up to 4.5 seconds for managed storage to be available | ||
twschiller marked this conversation as resolved.
Show resolved
Hide resolved
|
||
const { | ||
ssoUrl, | ||
partnerId, | ||
controlRoomUrl, | ||
disableLoginTab, | ||
managedOrganizationId, | ||
} = await readManagedStorage(); | ||
|
||
if (disableLoginTab) { | ||
// IT manager has disabled the login tab | ||
return; | ||
} | ||
|
||
if (ssoUrl) { | ||
// Don't launch the SSO page automatically. The SSO flow will be launched by deploymentUpdater.ts:updateDeployments | ||
if (ssoUrl || managedOrganizationId) { | ||
// Don't launch the page automatically. The SSO flow will be launched by deploymentUpdater.ts:updateDeployments | ||
return; | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No reason for it to be void because that's the standard behavior