Bug 1656048 - Remove linter exception for newtab re: whitelist/blacklist, and fix errors r=andreio

Differential Revision: https://phabricator.services.mozilla.com/D85931
This commit is contained in:
emcminn 2020-08-10 16:47:39 +00:00
Родитель 2fc8fca5d8
Коммит 037a8e97b9
10 изменённых файлов: 49 добавлений и 56 удалений

Просмотреть файл

@ -4,7 +4,7 @@
Name | Used for | Type | Example value
--- | --- | --- | ---
`whitelistHosts` | Whitelist a host in order to fetch messages from its endpoint | `[String]` | `["gist.github.com", "gist.githubusercontent.com", "localhost:8000"]`
`allowHosts` | Allow a host in order to fetch messages from its endpoint | `[String]` | `["gist.github.com", "gist.githubusercontent.com", "localhost:8000"]`
`providers.snippets` | Message provider options for snippets | `Object` | [see below](#message-providers)
`providers.cfr` | Message provider options for cfr | `Object` | [see below](#message-providers)
`providers.onboarding` | Message provider options for onboarding | `Object` | [see below](#message-providers)

Просмотреть файл

@ -46,15 +46,15 @@ You can preview in the two different themes (light and dark) by adding `&theme=d
#### IMPORTANT NOTES
- Links to URLs starting with `about:newtab` cannot be clicked on directly. They must be copy and pasted into the address bar.
- Previews should only be tested in `Firefox 64` and later.
- The endpoint must be HTTPS, the host must be whitelisted (see testing instructions below)
- The endpoint must be HTTPS, the host must be allowed (see testing instructions below)
- Errors are surfaced in the `Console` tab of the `Browser Toolbox`
#### Testing instructions
- If your endpoint URL has a host name of `snippets-admin.mozilla.org`, you can paste the URL into the address bar view it without any further steps.
- If your endpoint URL starts with some other host name, it must be **whitelisted**. Open the Browser Toolbox devtools (Tools > Developer > Browser Toolbox) and paste the following code (where `gist.githubusercontent.com` is the hostname of your endpoint URL):
- If your endpoint URL starts with some other host name, it must be **allowed**. Open the Browser Toolbox devtools (Tools > Developer > Browser Toolbox) and paste the following code (where `gist.githubusercontent.com` is the hostname of your endpoint URL):
```js
Services.prefs.setStringPref(
"browser.newtab.activity-stream.asrouter.whitelistHosts",
"browser.newtab.activity-stream.asrouter.allowHosts",
"[\"gist.githubusercontent.com\"]"
);
```

Просмотреть файл

@ -143,7 +143,7 @@ Endpoint for when a user opts-out of sponsored content to delete the correspondi
- Default: `https://getpocket.cdn.mozilla.net/,https://spocs.getpocket.com/`
- Pref Type: AS
A whitelist of endpoints that are allowed to be used by Discovery Stream for remote content (eg: story metadata) and configuration (eg: remote layout definitions for experimentation).
A list of endpoints that are allowed to be used by Discovery Stream for remote content (eg: story metadata) and configuration (eg: remote layout definitions for experimentation).
#### `browser.newtabpage.activity-stream.discoverystream.engagementLabelEnabled`

Просмотреть файл

@ -75,12 +75,12 @@ const OUTGOING_MESSAGE_NAME = "ASRouter:parent-to-child";
const ONE_DAY_IN_MS = 24 * 60 * 60 * 1000;
// List of hosts for endpoints that serve router messages.
// Key is allowed host, value is a name for the endpoint host.
const DEFAULT_WHITELIST_HOSTS = {
const DEFAULT_ALLOWLIST_HOSTS = {
"activity-stream-icons.services.mozilla.com": "production",
"snippets-admin.mozilla.org": "preview",
};
const SNIPPETS_ENDPOINT_WHITELIST =
"browser.newtab.activity-stream.asrouter.whitelistHosts";
const SNIPPETS_ENDPOINT_ALLOWLIST =
"browser.newtab.activity-stream.asrouter.allowHosts";
// Max possible impressions cap for any message
const MAX_MESSAGE_LIFETIME_CAP = 100;
@ -931,7 +931,7 @@ class _ASRouter {
this.onMessage
);
this._storage = storage;
this.WHITELIST_HOSTS = this._loadSnippetsWhitelistHosts();
this.ALLOWLIST_HOSTS = this._loadSnippetsAllowHosts();
this.dispatchToAS = dispatchToAS;
ASRouterPreferences.init();
@ -1736,16 +1736,16 @@ class _ASRouter {
_validPreviewEndpoint(url) {
try {
const endpoint = new URL(url);
if (!this.WHITELIST_HOSTS[endpoint.host]) {
if (!this.ALLOWLIST_HOSTS[endpoint.host]) {
Cu.reportError(
`The preview URL host ${endpoint.host} is not in the whitelist.`
`The preview URL host ${endpoint.host} is not in the list of allowed hosts.`
);
}
if (endpoint.protocol !== "https:") {
Cu.reportError("The URL protocol is not https.");
}
return (
endpoint.protocol === "https:" && this.WHITELIST_HOSTS[endpoint.host]
endpoint.protocol === "https:" && this.ALLOWLIST_HOSTS[endpoint.host]
);
} catch (e) {
return false;
@ -1766,35 +1766,37 @@ class _ASRouter {
Services.obs.addObserver(addonInstallObs, "webextension-install-notify");
}
_loadSnippetsWhitelistHosts() {
_loadSnippetsAllowHosts() {
let additionalHosts = [];
const whitelistPrefValue = Services.prefs.getStringPref(
SNIPPETS_ENDPOINT_WHITELIST,
const allowPrefValue = Services.prefs.getStringPref(
SNIPPETS_ENDPOINT_ALLOWLIST,
""
);
try {
additionalHosts = JSON.parse(whitelistPrefValue);
additionalHosts = JSON.parse(allowPrefValue);
} catch (e) {
if (whitelistPrefValue) {
if (allowPrefValue) {
Cu.reportError(
`Pref ${SNIPPETS_ENDPOINT_WHITELIST} value is not valid JSON`
`Pref ${SNIPPETS_ENDPOINT_ALLOWLIST} value is not valid JSON`
);
}
}
if (!additionalHosts.length) {
return DEFAULT_WHITELIST_HOSTS;
return DEFAULT_ALLOWLIST_HOSTS;
}
// If there are additional hosts we want to whitelist, add them as
// If there are additional hosts we want to allow, add them as
// `preview` so that the updateCycle is 0
return additionalHosts.reduce(
(whitelist_hosts, host) => {
whitelist_hosts[host] = "preview";
Services.console.logStringMessage(`Adding ${host} to whitelist hosts.`);
return whitelist_hosts;
(allow_hosts, host) => {
allow_hosts[host] = "preview";
Services.console.logStringMessage(
`Adding ${host} to list of allowed hosts.`
);
return allow_hosts;
},
{ ...DEFAULT_WHITELIST_HOSTS }
{ ...DEFAULT_ALLOWLIST_HOSTS }
);
}

Просмотреть файл

@ -25,11 +25,11 @@ function isPrivateWindow(win) {
}
/**
* Check current location against the list of whitelisted hosts
* Check current location against the list of allowed hosts
* Additionally verify for redirects and check original request URL against
* the whitelist.
* the list.
*
* @returns {object} - {host, url} pair that matched the whitelist
* @returns {object} - {host, url} pair that matched the list of allowed hosts
*/
function checkURLMatch(aLocationURI, { hosts, matchPatternSet }, aRequest) {
// If checks pass we return a match
@ -41,7 +41,7 @@ function checkURLMatch(aLocationURI, { hosts, matchPatternSet }, aRequest) {
return false;
}
// Check current location against whitelisted hosts
// Check current location against allowed hosts
if (hosts.has(match.host)) {
return match;
}

Просмотреть файл

@ -27,8 +27,8 @@ var EXPORTED_SYMBOLS = ["RecipeExecutor"];
* {"function": "conditionally_nmf_tag", "fields": ["title", "description"]} ]
*
* Recipes are sandboxed by the fact that the step functions must be explicitly
* whitelisted. Functions whitelisted for builder recipes are specifed in the
* RecipeExecutor.ITEM_BUILDER_REGISTRY, while combiner functions are whitelisted
* allowed. Functions allowed for builder recipes are specifed in the
* RecipeExecutor.ITEM_BUILDER_REGISTRY, while combiner functions are allowed
* in RecipeExecutor.ITEM_COMBINER_REGISTRY .
*/
const RecipeExecutor = class RecipeExecutor {
@ -48,7 +48,7 @@ const RecipeExecutor = class RecipeExecutor {
scalar_add: this.scalarAdd,
vector_add: this.vectorAdd,
make_boolean: this.makeBoolean,
whitelist_fields: this.whitelistFields,
allow_fields: this.allowFields,
filter_by_value: this.filterByValue,
l2_normalize: this.l2Normalize,
prob_normalize: this.probNormalize,
@ -681,7 +681,7 @@ const RecipeExecutor = class RecipeExecutor {
*
* fields An array of strings indicating the fields to keep
*/
whitelistFields(item, config) {
allowFields(item, config) {
let newItem = {};
for (let ele of config.fields) {
if (ele in item) {

Просмотреть файл

@ -363,26 +363,26 @@ describe("ASRouter", () => {
FAKE_LOCAL_MESSAGES.length + FAKE_REMOTE_MESSAGES.length
);
});
it("should load additional whitelisted hosts", async () => {
getStringPrefStub.returns('["whitelist.com"]');
it("should load additional allowed hosts", async () => {
getStringPrefStub.returns('["allow.com"]');
await createRouterAndInit();
assert.propertyVal(Router.WHITELIST_HOSTS, "whitelist.com", "preview");
assert.propertyVal(Router.ALLOWLIST_HOSTS, "allow.com", "preview");
// Should still include the defaults
assert.lengthOf(Object.keys(Router.WHITELIST_HOSTS), 3);
assert.lengthOf(Object.keys(Router.ALLOWLIST_HOSTS), 3);
});
it("should fallback to defaults if pref parsing fails", async () => {
getStringPrefStub.returns("err");
await createRouterAndInit();
assert.lengthOf(Object.keys(Router.WHITELIST_HOSTS), 2);
assert.lengthOf(Object.keys(Router.ALLOWLIST_HOSTS), 2);
assert.propertyVal(
Router.WHITELIST_HOSTS,
Router.ALLOWLIST_HOSTS,
"snippets-admin.mozilla.org",
"preview"
);
assert.propertyVal(
Router.WHITELIST_HOSTS,
Router.ALLOWLIST_HOSTS,
"activity-stream-icons.services.mozilla.com",
"production"
);
@ -1744,7 +1744,7 @@ describe("ASRouter", () => {
)
);
});
it("should not add a url that is not from a whitelisted host", async () => {
it("should not add a url that is not from an allowed host", async () => {
const url = "https://mozilla.org";
const msg = fakeAsyncMessage({
type: "NEWTAB_MESSAGE_REQUEST",

Просмотреть файл

@ -71,18 +71,18 @@ describe("ASRTargeting docs", () => {
}
});
describe("No extra attributes in targeting-attributes.md", () => {
// whitelist includes targeting attributes that are not implemented by
// "allow" includes targeting attributes that are not implemented by
// ASRTargetingAttributes. For example trigger context passed to the evaluation
// context in when a trigger runs or ASRouter state used in the evaluation.
const whitelist = [
const allow = [
"personalizedCfrThreshold",
"personalizedCfrScores",
"messageImpressions",
];
for (const targetingParam of DOCS_TARGETING_HEADINGS.filter(
doc => !whitelist.includes(doc)
doc => !allow.includes(doc)
)) {
// If this test is failing, you might has spelled something wrong or removed a targeting param without
// If this test is failing, you might have spelled something wrong or removed a targeting param without
// removing its docs.
it(`should have an implementation for ${targetingParam} in ASRouterTargeting.Environment`, () => {
assert.include(

Просмотреть файл

@ -847,9 +847,9 @@ describe("RecipeExecutor", () => {
});
});
describe("#whitelistFields", () => {
describe("#allowFields", () => {
it("should filter the keys out of a map", () => {
item = instance.whitelistFields(item, {
item = instance.allowFields(item, {
fields: ["foo", "missing", "bar"],
});
assert.deepEqual(item, { foo: "FOO", bar: "BAR" });

Просмотреть файл

@ -49,16 +49,7 @@ avoid-blacklist-and-whitelist:
- browser/components/enterprisepolicies/helpers/WebsiteFilter.jsm
- browser/components/migration/ChromeMigrationUtils.jsm
- browser/components/migration/ChromeProfileMigrator.jsm
- browser/components/newtab/content-src/asrouter/docs/debugging-docs.md
- browser/components/newtab/content-src/asrouter/README.md
- browser/components/newtab/data/content/activity-stream.bundle.js
- browser/components/newtab/docs/v2-system-addon/preferences.md
- browser/components/newtab/lib/ASRouter.jsm
- browser/components/newtab/lib/ASRouterTriggerListeners.jsm
- browser/components/newtab/lib/PersonalityProvider/RecipeExecutor.jsm
- browser/components/newtab/test/unit/asrouter/ASRouter.test.js
- browser/components/newtab/test/unit/asrouter/TargetingDocs.test.js
- browser/components/newtab/test/unit/lib/PersonalityProvider/RecipeExecutor.test.js
- browser/components/pioneer/content/pioneer.js
- browser/components/preferences/privacy.inc.xhtml
- browser/components/preferences/privacy.js