From 5eab4b7a2c79e3b399dfdbfe8aab65ef95c2e3cb Mon Sep 17 00:00:00 2001 From: Noah Callaway Date: Sun, 25 Apr 2021 12:36:10 -0700 Subject: [PATCH 1/2] Add allowlist for isolated containers This implements a per-container allow-list that is applied to isolated containers. This can allow multiple containers to have an overlapping set of sites that they are isolated to. For example: Container A and Container B could both be isolated and have calendar.example.com in their allow list. Both Container A and Container B would then be able to visit calendar.example.com. See: https://github.com/mozilla/multi-account-containers/issues/1892 and https://github.com/mozilla/multi-account-containers/issues/1887 for additional motivation. --- src/css/popup.css | 16 +++ src/js/background/assignManager.js | 70 ++++++---- src/js/background/backgroundLogic.js | 41 +++++- src/js/background/index.html | 1 + src/js/background/messageHandler.js | 10 ++ src/js/popup.js | 193 +++++++++++++++++++++++---- src/js/utils.js | 102 ++++++++++++-- src/popup.html | 55 ++++++-- 8 files changed, 409 insertions(+), 79 deletions(-) diff --git a/src/css/popup.css b/src/css/popup.css index 4dfb8dc5..ece23636 100644 --- a/src/css/popup.css +++ b/src/css/popup.css @@ -585,6 +585,11 @@ manage things like container crud */ padding-inline-start: 0; } +.edit-allowed-sites-panel fieldset { + background: none; + border: none; +} + .edit-container-panel fieldset:last-of-type { margin-block-end: 0; } @@ -729,6 +734,7 @@ h3.title { } /* Maintain 1:1 square ratio for Favicons of websites added to a specific container */ +.edit-allowed-sites-panel .menu-icon, #edit-sites-assigned .menu-icon, #container-info-table .menu-icon { inline-size: 16px; @@ -952,6 +958,16 @@ tr:hover > td > .trash-button { height: 16px; } +#add-allowed-site-form { + align-items: end; + display: flex; + flex-direction: row; +} + +#add-allowed-site-form fieldset { + flex: 1; +} + @media (prefers-color-scheme: dark) { :root { --title-text-color: #fff; diff --git a/src/js/background/assignManager.js b/src/js/background/assignManager.js index 032adb1f..4e718c54 100644 --- a/src/js/background/assignManager.js +++ b/src/js/background/assignManager.js @@ -109,10 +109,10 @@ window.assignManager = { const siteConfigs = await this.area.get(); for(const urlKey of Object.keys(siteConfigs)) { if (urlKey.includes("siteContainerMap@@_")) { - // For some reason this is stored as string... lets check + // For some reason this is stored as string... lets check // them both as that - if (!!userContextId && - String(siteConfigs[urlKey].userContextId) + if (!!userContextId && + String(siteConfigs[urlKey].userContextId) !== String(userContextId)) { continue; } @@ -127,7 +127,7 @@ window.assignManager = { }, /* - * Looks for abandoned site assignments. If there is no identity with + * Looks for abandoned site assignments. If there is no identity with * the site assignment's userContextId (cookieStoreId), then the assignment * is removed. */ @@ -136,8 +136,8 @@ window.assignManager = { const macConfigs = await this.area.get(); for(const configKey of Object.keys(macConfigs)) { if (configKey.includes("siteContainerMap@@_")) { - const cookieStoreId = - "firefox-container-" + macConfigs[configKey].userContextId; + const cookieStoreId = + "firefox-container-" + macConfigs[configKey].userContextId; const match = identitiesList.find( localIdentity => localIdentity.cookieStoreId === cookieStoreId ); @@ -146,7 +146,7 @@ window.assignManager = { continue; } const updatedSiteAssignment = macConfigs[configKey]; - updatedSiteAssignment.identityMacAddonUUID = + updatedSiteAssignment.identityMacAddonUUID = await identityState.lookupMACaddonUUID(match.cookieStoreId); await this.set( configKey, @@ -164,7 +164,7 @@ window.assignManager = { _neverAsk(m) { const pageUrl = m.pageUrl; if (m.neverAsk === true) { - // If we have existing data and for some reason it hasn't been + // If we have existing data and for some reason it hasn't been // deleted etc lets update it this.storageArea.get(pageUrl).then((siteSettings) => { if (siteSettings) { @@ -210,9 +210,10 @@ window.assignManager = { return {}; } const userContextId = this.getUserContextIdFromCookieStore(tab); + const url = options.url; // https://github.com/mozilla/multi-account-containers/issues/847 - // + // // Handle the case where this request's URL is not assigned to any particular // container. We must do the following check: // @@ -228,8 +229,11 @@ window.assignManager = { // - the current tab's container is locked and only allows "www.google.com" // - the incoming request is for "www.amazon.com", which has no specific container assignment // - in this case, we must re-open "www.amazon.com" in a new tab in the default container - const siteIsolatedReloadInDefault = - await this._maybeSiteIsolatedReloadInDefault(siteSettings, tab); + const siteIsolatedReloadInDefault = await this._maybeSiteIsolatedReloadInDefault( + siteSettings, + tab, + url + ); if (!siteIsolatedReloadInDefault) { if (!siteSettings @@ -246,7 +250,7 @@ window.assignManager = { const openTabId = removeTab ? tab.openerTabId : tab.id; if (!this.canceledRequests[tab.id]) { - // we decided to cancel the request at this point, register + // we decided to cancel the request at this point, register // canceled request this.canceledRequests[tab.id] = { requestIds: { @@ -313,7 +317,7 @@ window.assignManager = { - As the history won't span from one container to another it seems most sane to not try and reopen a tab on history.back() - When users open a new tab themselves we want to make sure we - don't end up with three tabs as per: + don't end up with three tabs as per: https://github.com/mozilla/testpilot-containers/issues/421 If we are coming from an internal url that are used for the new tab page (NEW_TAB_PAGES), we can safely close as user is unlikely @@ -338,7 +342,7 @@ window.assignManager = { }; }, - async _maybeSiteIsolatedReloadInDefault(siteSettings, tab) { + async _maybeSiteIsolatedReloadInDefault(siteSettings, tab, url) { // Tab doesn't support cookies, so containers not supported either. if (!("cookieStoreId" in tab)) { return false; @@ -348,7 +352,7 @@ window.assignManager = { // I.e. it will be opened in that container anyway, so we don't need to check if the // current tab's container is locked or not. if (siteSettings) { - return false; + return false; } //tab is alredy reopening in the default container @@ -358,13 +362,27 @@ window.assignManager = { // Requested page is not assigned to a specific container. If the current tab's container // is locked, then the page must be reloaded in the default container. const currentContainerState = await identityState.storageArea.get(tab.cookieStoreId); - return currentContainerState && currentContainerState.isIsolated; + + // the container is not isolated, so any site can be opened + const isIsolated = currentContainerState && currentContainerState.isIsolated; + if (!isIsolated) { + return false; + } + + // the site is isolated, and it's *not* an assigned site, so check if it's in the allowed + // sites array. If it is we can open the site in the container, otherwise we should reload + // in the default container + const allowedSites = + (currentContainerState && currentContainerState.allowedSites) || []; + + const allowedKey = Utils.getAllowedSiteKeyFor(url); + return !allowedSites.includes(allowedKey); }, init() { browser.contextMenus.onClicked.addListener((info, tab) => { - info.bookmarkId ? - this._onClickedBookmark(info) : + info.bookmarkId ? + this._onClickedBookmark(info) : this._onClickedHandler(info, tab); }); @@ -479,7 +497,7 @@ window.assignManager = { async _onClickedBookmark(info) { async function _getBookmarksFromInfo(info) { - const [bookmarkTreeNode] = + const [bookmarkTreeNode] = await browser.bookmarks.get(info.bookmarkId); if (bookmarkTreeNode.type === "folder") { return browser.bookmarks.getChildren(bookmarkTreeNode.id); @@ -489,9 +507,9 @@ window.assignManager = { const bookmarks = await _getBookmarksFromInfo(info); for (const bookmark of bookmarks) { - // Some checks on the urls from + // Some checks on the urls from // https://github.com/Rob--W/bookmark-container-tab/ thanks! - if ( !/^(javascript|place):/i.test(bookmark.url) && + if ( !/^(javascript|place):/i.test(bookmark.url) && bookmark.type !== "folder") { const openInReaderMode = bookmark.url.startsWith("about:reader"); if(openInReaderMode) { @@ -569,12 +587,12 @@ window.assignManager = { actionName = "removed from assigned sites list"; // remove site isolation if now empty - await this._maybeRemoveSiteIsolation(userContextId); + await this._maybeRemoveSiteIsolation(userContextId); } if (tabId) { const tab = await browser.tabs.get(tabId); - setTimeout(function(){ + setTimeout(function(){ browser.tabs.sendMessage(tabId, { text: `Successfully ${actionName}` }); @@ -677,17 +695,17 @@ window.assignManager = { reloadPageInDefaultContainer(url, index, active, openerTabId) { // To create a new tab in the default container, it is easiest just to omit the // cookieStoreId entirely. - // + // // Unfortunately, if you create a new tab WITHOUT a cookieStoreId but WITH an openerTabId, // then the new tab automatically inherits the opener tab's cookieStoreId. // I.e. it opens in the wrong container! - // + // // So we have to explicitly pass in a cookieStoreId when creating the tab, since we // are specifying the openerTabId. There doesn't seem to be any way // to look up the default container's cookieStoreId programatically, so sadly // we have to hardcode it here as "firefox-default". This is potentially // not cross-browser compatible. - // + // // Note that we could have just omitted BOTH cookieStoreId and openerTabId. But the // drawback then is that if the user later closes the newly-created tab, the browser // does not automatically return to the original opener tab. To get this desired behaviour, diff --git a/src/js/background/backgroundLogic.js b/src/js/background/backgroundLogic.js index 76432216..8726274b 100644 --- a/src/js/background/backgroundLogic.js +++ b/src/js/background/backgroundLogic.js @@ -143,7 +143,7 @@ const backgroundLogic = { if ("isIsolated" in containerState || remove) { delete containerState.isIsolated; } else { - containerState.isIsolated = "locked"; + containerState.isIsolated = "locked"; } return await identityState.storageArea.set(cookieStoreId, containerState); } catch (error) { @@ -151,6 +151,42 @@ const backgroundLogic = { } }, + async addRemoveAllowedSite(cookieStoreId, allowedSiteUrl, remove = false) { + try { + const containerState = await identityState.storageArea.get(cookieStoreId); + const allowedSiteKey = Utils.getAllowedSiteKeyFor(allowedSiteUrl); + const allowedSites = containerState.allowedSites || []; + const allowedSiteIdx = allowedSites.indexOf(allowedSiteKey); + + if (!remove) { + if (allowedSiteIdx === -1) { + // only add the site if it's not already in the list. + allowedSites.push(allowedSiteKey); + containerState.allowedSites = allowedSites; + } + } else { + // remove + if (allowedSiteIdx >= 0) { + allowedSites.splice(allowedSiteIdx, 1); + } + } + containerState.allowedSites = allowedSites; + return await identityState.storageArea.set(cookieStoreId, containerState); + } catch (error) { + console.error(`No container: ${cookieStoreId}`); + } + }, + + async clearAllowedSites(cookieStoreId) { + try { + const containerState = await identityState.storageArea.get(cookieStoreId); + containerState.allowedSites = []; + return await identityState.storageArea.set(cookieStoreId, containerState); + } catch (error) { + console.error(`No container: ${cookieStoreId}`); + } + }, + async moveTabsToWindow(options) { const requiredArguments = ["cookieStoreId", "windowId"]; this.checkArgs(requiredArguments, options, "moveTabsToWindow"); @@ -257,7 +293,8 @@ const backgroundLogic = { hasOpenTabs: !!openTabs.length, numberOfHiddenTabs: containerState.hiddenTabs.length, numberOfOpenTabs: openTabs.length, - isIsolated: !!containerState.isIsolated + isIsolated: !!containerState.isIsolated, + allowedSites: containerState.allowedSites || [] }; return; }); diff --git a/src/js/background/index.html b/src/js/background/index.html index da380ba8..d9699371 100644 --- a/src/js/background/index.html +++ b/src/js/background/index.html @@ -19,5 +19,6 @@ + diff --git a/src/js/background/messageHandler.js b/src/js/background/messageHandler.js index b3270e52..f58e1b2d 100644 --- a/src/js/background/messageHandler.js +++ b/src/js/background/messageHandler.js @@ -35,6 +35,16 @@ const messageHandler = { case "addRemoveSiteIsolation": response = backgroundLogic.addRemoveSiteIsolation(m.cookieStoreId); break; + case "addRemoveAllowedSite": + response = backgroundLogic.addRemoveAllowedSite( + m.cookieStoreId, + m.allowedSiteUrl, + m.remove + ); + break; + case "clearAllowedSites": + response = backgroundLogic.clearAllowedSites(m.cookieStoreId); + break; case "getAssignment": response = browser.tabs.get(m.tabId).then((tab) => { return assignManager._getAssignment(tab); diff --git a/src/js/popup.js b/src/js/popup.js index 22a4b4cd..34a6e446 100644 --- a/src/js/popup.js +++ b/src/js/popup.js @@ -26,6 +26,7 @@ const OPEN_NEW_CONTAINER_PICKER = "new-tab"; const MANAGE_CONTAINERS_PICKER = "manage"; const REOPEN_IN_CONTAINER_PICKER = "reopen-in"; const ALWAYS_OPEN_IN_PICKER = "always-open-in"; +const ALLOW_OPEN_IN_PICKER = "allow-open-in"; const P_CONTAINER_INFO = "containerInfo"; const P_CONTAINER_EDIT = "containerEdit"; const P_CONTAINER_DELETE = "containerDelete"; @@ -40,6 +41,15 @@ function addRemoveSiteIsolation() { }); } +function addRemoveAllowedSite(cookieStoreId, allowedSiteUrl, remove = false) { + return browser.runtime.sendMessage({ + method: "addRemoveAllowedSite", + cookieStoreId: cookieStoreId, + allowedSiteUrl: allowedSiteUrl, + remove: remove + }); +} + async function getExtensionInfo() { const manifestPath = browser.extension.getURL("manifest.json"); const response = await fetch(manifestPath); @@ -225,6 +235,7 @@ const Logic = { identity.numberOfHiddenTabs = stateObject.numberOfHiddenTabs; identity.numberOfOpenTabs = stateObject.numberOfOpenTabs; identity.isIsolated = stateObject.isIsolated; + identity.allowedSites = stateObject.allowedSites; } if (containerOrder) { identity.order = containerOrder[identity.cookieStoreId]; @@ -302,6 +313,14 @@ const Logic = { return this._currentIdentity; }, + async refreshCurrentIdentity() { + const current = this.currentIdentity(); + await this.refreshIdentities(); + this._currentIdentity = this.identities().find( + identity => identity.cookieStoreId === current.cookieStoreId + ); + }, + currentUserContextId() { const identity = Logic.currentIdentity(); return Utils.userContextId(identity.cookieStoreId); @@ -645,6 +664,9 @@ Logic.registerPanel(P_CONTAINERS_LIST, { Utils.addEnterHandler(document.querySelector("#always-open-in"), () => { Logic.showPanel(ALWAYS_OPEN_IN_PICKER); }); + Utils.addEnterHandler(document.querySelector("#allow-open-in"), () => { + Logic.showPanel(ALLOW_OPEN_IN_PICKER); + }); Utils.addEnterHandler(document.querySelector("#info-icon"), () => { browser.runtime.openOptionsPage(); }); @@ -668,6 +690,13 @@ Logic.registerPanel(P_CONTAINERS_LIST, { async prepare() { const fragment = document.createDocumentFragment(); + const anyIsolatedContainers = Logic.identities().some( + identity => identity.isIsolated + ); + + const allowOpenIn = document.querySelector("#allow-open-in"); + allowOpenIn.hidden = !anyIsolatedContainers; + Logic.identities().forEach(identity => { const tr = document.createElement("tr"); tr.classList.add("menu-item", "hover-highlight", "keyboard-nav", "keyboard-right-arrow-override"); @@ -803,8 +832,8 @@ Logic.registerPanel(P_CONTAINER_INFO, { }); // Populating the panel: name and icon document.getElementById("container-info-title").textContent = identity.name; - - const alwaysOpen = document.querySelector("#always-open-in-info-panel"); + + const alwaysOpen = document.querySelector("#always-open-in-info-panel"); Utils.addEnterHandler(alwaysOpen, async () => { Utils.alwaysOpenInContainer(identity); window.close(); @@ -941,7 +970,7 @@ Logic.registerPanel(OPEN_NEW_CONTAINER_PICKER, { tr.setAttribute("tabindex", "0"); const td = document.createElement("td"); - td.innerHTML = Utils.escaped` + td.innerHTML = Utils.escaped`