Skip to content

Commit

Permalink
Add allowlist for isolated containers
Browse files Browse the repository at this point in the history
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: mozilla#1892 and
mozilla#1887 for additional
motivation.
  • Loading branch information
ncallaway committed Apr 25, 2021
1 parent 4b56a2f commit 5eab4b7
Show file tree
Hide file tree
Showing 8 changed files with 409 additions and 79 deletions.
16 changes: 16 additions & 0 deletions src/css/popup.css
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
70 changes: 44 additions & 26 deletions src/js/background/assignManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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.
*/
Expand All @@ -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
);
Expand All @@ -146,7 +146,7 @@ window.assignManager = {
continue;
}
const updatedSiteAssignment = macConfigs[configKey];
updatedSiteAssignment.identityMacAddonUUID =
updatedSiteAssignment.identityMacAddonUUID =
await identityState.lookupMACaddonUUID(match.cookieStoreId);
await this.set(
configKey,
Expand All @@ -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) {
Expand Down Expand Up @@ -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:
//
Expand All @@ -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
Expand All @@ -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: {
Expand Down Expand Up @@ -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
Expand All @@ -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;
Expand All @@ -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
Expand All @@ -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);
});

Expand Down Expand Up @@ -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);
Expand All @@ -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) {
Expand Down Expand Up @@ -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}`
});
Expand Down Expand Up @@ -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,
Expand Down
41 changes: 39 additions & 2 deletions src/js/background/backgroundLogic.js
Original file line number Diff line number Diff line change
Expand Up @@ -143,14 +143,50 @@ 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) {
console.error(`No container: ${cookieStoreId}`);
}
},

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");
Expand Down Expand Up @@ -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;
});
Expand Down
1 change: 1 addition & 0 deletions src/js/background/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -19,5 +19,6 @@
<script type="text/javascript" src="identityState.js"></script>
<script type="text/javascript" src="messageHandler.js"></script>
<script type="text/javascript" src="sync.js"></script>
<script type="text/javascript" src="../utils.js"></script>
</body>
</html>
10 changes: 10 additions & 0 deletions src/js/background/messageHandler.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Loading

0 comments on commit 5eab4b7

Please sign in to comment.