diff --git a/toolkit/components/extensions/parent/ext-cookies.js b/toolkit/components/extensions/parent/ext-cookies.js index 4cf890f0fdc1..e6f63c244b3c 100644 --- a/toolkit/components/extensions/parent/ext-cookies.js +++ b/toolkit/components/extensions/parent/ext-cookies.js @@ -221,7 +221,6 @@ const checkSetCookiePermissions = (extension, uri, cookie) => { * * @param {Object} details * The details received from the extension. - * @param {BaseContext} context * @param {boolean} allowPattern * Whether to potentially return an OriginAttributesPattern instead of * OriginAttributes. The get/set/remove cookie methods operate on exact @@ -230,46 +229,16 @@ const checkSetCookiePermissions = (extension, uri, cookie) => { * @returns {Object} An object with the following properties: * - originAttributes {OriginAttributes|OriginAttributesPattern} * - isPattern {boolean} Whether originAttributes is a pattern. - * - isPrivate {boolean} Whether the cookie belongs to private browsing mode. - * - storeId {string} The storeId of the cookie. **/ -const oaFromDetails = (details, context, allowPattern) => { +const oaFromDetails = (details, allowPattern) => { // Default values, may be filled in based on details. let originAttributes = { - userContextId: 0, - privateBrowsingId: 0, - // The following two keys may be deleted if allowPattern=true + userContextId: 0, // TODO bug 1669716: merge from query*/cookies.set. + privateBrowsingId: 0, // TODO bug 1669716: merge from query*/cookies.set. + // The following two keys may be deleted if useDefaultOA=false firstPartyDomain: details.firstPartyDomain ?? "", partitionKey: fromExtPartitionKey(details.partitionKey), }; - - let isPrivate = context.incognito; - let storeId = isPrivate ? PRIVATE_STORE : DEFAULT_STORE; - if (details.storeId) { - storeId = details.storeId; - if (isDefaultCookieStoreId(storeId)) { - isPrivate = false; - } else if (isPrivateCookieStoreId(storeId)) { - isPrivate = true; - } else { - isPrivate = false; - let userContextId = getContainerForCookieStoreId(storeId); - if (!userContextId) { - throw new ExtensionError(`Invalid cookie store id: "${storeId}"`); - } - originAttributes.userContextId = userContextId; - } - } - - if (isPrivate) { - originAttributes.privateBrowsingId = 1; - if (!context.privateBrowsingAllowed) { - throw new ExtensionError( - "Extension disallowed access to the private cookies storeId." - ); - } - } - // If any of the originAttributes's keys are deleted, this becomes true. let isPattern = false; if (allowPattern) { @@ -294,15 +263,15 @@ const oaFromDetails = (details, context, allowPattern) => { isPattern = true; } } - return { originAttributes, isPattern, isPrivate, storeId }; + return { originAttributes, isPattern }; }; /** * Query the cookie store for matching cookies. * @param {Object} detailsIn * @param {Array} props Properties the extension is interested in matching against. - * The firstPartyDomain / partitionKey / storeId - * props are always accounted for. + * The firstPartyDomain / partitionKey props are + * always accounted for. * @param {BaseContext} context The context making the query. * @param {boolean} allowPattern Whether to allow the query to match distinct * origin attributes instead of falling back to @@ -316,24 +285,46 @@ const query = function*(detailsIn, props, context, allowPattern) { } }); - let parsedOA; - try { - parsedOA = oaFromDetails(detailsIn, context, allowPattern); - } catch (e) { - if (e.message.startsWith("Invalid cookie store id")) { - // For backwards-compatibility with previous versions of Firefox, fail - // silently (by not returning any results) instead of throwing an error. - return; - } - throw e; - } - let { originAttributes, isPattern, isPrivate, storeId } = parsedOA; + let { originAttributes, isPattern } = oaFromDetails(detailsIn, allowPattern); if ("domain" in details) { details.domain = details.domain.toLowerCase().replace(/^\./, ""); details.domain = dropBracketIfIPv6(details.domain); } + let isPrivate = context.incognito; + if (details.storeId) { + if (!isValidCookieStoreId(details.storeId)) { + return; + } + + if (isDefaultCookieStoreId(details.storeId)) { + isPrivate = false; + } else if (isPrivateCookieStoreId(details.storeId)) { + isPrivate = true; + } else if (isContainerCookieStoreId(details.storeId)) { + isPrivate = false; + let userContextId = getContainerForCookieStoreId(details.storeId); + if (!userContextId) { + return; + } + originAttributes.userContextId = userContextId; + } + } + + let storeId = DEFAULT_STORE; + if (isPrivate) { + storeId = PRIVATE_STORE; + originAttributes.privateBrowsingId = 1; + } else if ("storeId" in details) { + storeId = details.storeId; + } + if (storeId == PRIVATE_STORE && !context.privateBrowsingAllowed) { + throw new ExtensionError( + "Extension disallowed access to the private cookies storeId." + ); + } + // We can use getCookiesFromHost for faster searching. let cookies; let host; @@ -459,7 +450,7 @@ this.cookies = class extends ExtensionAPI { validateFirstPartyDomain(details); // FIXME: We don't sort by length of path and creation time. - let allowed = ["url", "name"]; + let allowed = ["url", "name", "storeId"]; for (let cookie of query(details, allowed, context)) { return Promise.resolve(convertCookie(cookie)); } @@ -474,7 +465,16 @@ this.cookies = class extends ExtensionAPI { validateFirstPartyDomain(details); } - let allowed = ["url", "name", "domain", "path", "secure", "session"]; + let allowed = [ + "url", + "name", + "domain", + "path", + "secure", + "session", + "storeId", + ]; + let result = Array.from( query(details, allowed, context, /* allowPattern = */ true), convertCookie @@ -514,8 +514,30 @@ this.cookies = class extends ExtensionAPI { let expiry = isSession ? Number.MAX_SAFE_INTEGER : details.expirationDate; - - let { originAttributes } = oaFromDetails(details, context); + let isPrivate = context.incognito; + let userContextId = 0; + if (isDefaultCookieStoreId(details.storeId)) { + isPrivate = false; + } else if (isPrivateCookieStoreId(details.storeId)) { + if (!context.privateBrowsingAllowed) { + return Promise.reject({ + message: + "Extension disallowed access to the private cookies storeId.", + }); + } + isPrivate = true; + } else if (isContainerCookieStoreId(details.storeId)) { + let containerId = getContainerForCookieStoreId(details.storeId); + if (containerId === null) { + return Promise.reject({ + message: `Illegal storeId: ${details.storeId}`, + }); + } + isPrivate = false; + userContextId = containerId; + } else if (details.storeId !== null) { + return Promise.reject({ message: "Unknown storeId" }); + } let cookieAttrs = { host: details.domain, @@ -530,6 +552,13 @@ this.cookies = class extends ExtensionAPI { }); } + let originAttributes = { + userContextId, + privateBrowsingId: isPrivate ? 1 : 0, + firstPartyDomain: details.firstPartyDomain ?? "", + partitionKey: fromExtPartitionKey(details.partitionKey), + }; + let sameSite = SAME_SITE_STATUSES.indexOf(details.sameSite); let schemeType = Ci.nsICookie.SCHEME_UNSET; @@ -563,8 +592,14 @@ this.cookies = class extends ExtensionAPI { remove: function(details) { validateFirstPartyDomain(details); - let allowed = ["url", "name"]; + let allowed = ["url", "name", "storeId"]; for (let { cookie, storeId } of query(details, allowed, context)) { + if ( + isPrivateCookieStoreId(details.storeId) && + !context.privateBrowsingAllowed + ) { + return Promise.reject({ message: "Unknown storeId" }); + } Services.cookies.remove( cookie.host, cookie.name, diff --git a/toolkit/components/extensions/test/xpcshell/test_ext_cookies_errors.js b/toolkit/components/extensions/test/xpcshell/test_ext_cookies_errors.js deleted file mode 100644 index 0a7fac53f86b..000000000000 --- a/toolkit/components/extensions/test/xpcshell/test_ext_cookies_errors.js +++ /dev/null @@ -1,159 +0,0 @@ -"use strict"; - -add_task(async function setup_cookies() { - let extension = ExtensionTestUtils.loadExtension({ - incognitoOverride: "spanning", - async background() { - const url = "http://example.com/"; - const name = "dummyname"; - await browser.cookies.set({ url, name, value: "from_setup:normal" }); - await browser.cookies.set({ - url, - name, - value: "from_setup:private", - storeId: "firefox-private", - }); - await browser.cookies.set({ - url, - name, - value: "from_setup:container", - storeId: "firefox-container-1", - }); - browser.test.sendMessage("setup_done"); - }, - manifest: { - permissions: ["cookies", "http://example.com/"], - }, - }); - await extension.startup(); - await extension.awaitMessage("setup_done"); - await extension.unload(); -}); - -add_task(async function test_error_messages() { - async function background() { - const url = "http://example.com/"; - const name = "dummyname"; - // Shorthands to minimize boilerplate. - const set = d => browser.cookies.set({ url, name, value: "x", ...d }); - const remove = d => browser.cookies.remove({ url, name, ...d }); - const get = d => browser.cookies.get({ url, name, ...d }); - const getAll = d => browser.cookies.getAll(d); - - // Host permission permission missing. - await browser.test.assertRejects( - set({}), - /^Permission denied to set cookie \{.*\}$/, - "cookies.set without host permissions rejects with error" - ); - browser.test.assertEq( - null, - await remove({}), - "cookies.remove without host permissions does not remove any cookies" - ); - browser.test.assertEq( - null, - await get({}), - "cookies.get without host permissions does not match any cookies" - ); - browser.test.assertEq( - "[]", - JSON.stringify(await getAll({})), - "cookies.getAll without host permissions does not match any cookies" - ); - - // Private browsing cookies without access to private browsing mode. - await browser.test.assertRejects( - set({ storeId: "firefox-private" }), - "Extension disallowed access to the private cookies storeId.", - "cookies.set cannot modify private cookies without permission" - ); - await browser.test.assertRejects( - remove({ storeId: "firefox-private" }), - "Extension disallowed access to the private cookies storeId.", - "cookies.remove cannot modify private cookies without permission" - ); - await browser.test.assertRejects( - get({ storeId: "firefox-private" }), - "Extension disallowed access to the private cookies storeId.", - "cookies.get cannot read private cookies without permission" - ); - await browser.test.assertRejects( - getAll({ storeId: "firefox-private" }), - "Extension disallowed access to the private cookies storeId.", - "cookies.getAll cannot read private cookies without permission" - ); - - // Invalid storeId. - await browser.test.assertRejects( - set({ storeId: "firefox-container-99" }), - `Invalid cookie store id: "firefox-container-99"`, - "cookies.set with invalid storeId (non-existent container)" - ); - - await browser.test.assertRejects( - set({ storeId: "0" }), - `Invalid cookie store id: "0"`, - "cookies.set with invalid storeId (format not recognized)" - ); - - for (let method of [remove, get, getAll]) { - let resultWithInvalidStoreId = method == getAll ? [] : null; - browser.test.assertEq( - JSON.stringify(await method({ storeId: "firefox-container-99" })), - JSON.stringify(resultWithInvalidStoreId), - `cookies.${method.name} with invalid storeId (non-existent container)` - ); - - browser.test.assertEq( - JSON.stringify(await method({ storeId: "0" })), - JSON.stringify(resultWithInvalidStoreId), - `cookies.${method.name} with invalid storeId (format not recognized)` - ); - } - - browser.test.sendMessage("test_done"); - } - - let extension = ExtensionTestUtils.loadExtension({ - background, - manifest: { - permissions: ["cookies"], - }, - }); - await extension.startup(); - await extension.awaitMessage("test_done"); - await extension.unload(); -}); - -add_task(async function expected_cookies_at_end_of_test() { - let extension = ExtensionTestUtils.loadExtension({ - incognitoOverride: "spanning", - async background() { - async function checkCookie(storeId, value) { - let cookies = await browser.cookies.getAll({ storeId }); - let index = cookies.findIndex(c => c.value === value); - browser.test.assertTrue(index !== -1, `Found cookie: ${value}`); - if (index >= 0) { - cookies.splice(index, 1); - } - browser.test.assertEq( - "[]", - JSON.stringify(cookies), - `No more cookies left in cookieStoreId=${storeId}` - ); - } - // Added in setup. - await checkCookie("firefox-default", "from_setup:normal"); - await checkCookie("firefox-private", "from_setup:private"); - await checkCookie("firefox-container-1", "from_setup:container"); - browser.test.sendMessage("final_check_done"); - }, - manifest: { - permissions: ["cookies", ""], - }, - }); - await extension.startup(); - await extension.awaitMessage("final_check_done"); - await extension.unload(); -}); diff --git a/toolkit/components/extensions/test/xpcshell/xpcshell-common.ini b/toolkit/components/extensions/test/xpcshell/xpcshell-common.ini index e9c63228cd7a..4ea984f7fb6a 100644 --- a/toolkit/components/extensions/test/xpcshell/xpcshell-common.ini +++ b/toolkit/components/extensions/test/xpcshell/xpcshell-common.ini @@ -47,7 +47,6 @@ run-sequentially = node server exceptions dont replay well skip-if = appname == "thunderbird" || tsan || os == "android" # Bug 1683730, Android: Bug 1700482 apple_silicon # Disabled due to bleedover with other tests when run in regular suites; passes in "failures" jobs -[test_ext_cookies_errors.js] [test_ext_cookies_firstParty.js] skip-if = appname == "thunderbird" || os == "android" || tsan # Android: Bug 1680132. tsan: Bug 1683730 [test_ext_cookies_partitionKey.js]