Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Move all fetch() requests to background scripts #426

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Move all fetch() requests to background scripts #426

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
cb9e313
Move all fetch() requests to background scripts
maxxcrawford Oct 31, 2022
75e8c3e
Correct typo
maxxcrawford Oct 31, 2022
2ffa9f4
Update function name to be more readable
maxxcrawford Oct 31, 2022
83d7727
From the review, refactored the main argument for the function to be …
maxxcrawford Dec 5, 2022
74769b4
Fix rebase error - move relayNumbers request to new fetchRequestFromB…
maxxcrawford Dec 5, 2022
25255d1
Remove CSRF header setter from fetchRequestFromBackground()
maxxcrawford Dec 5, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
44 changes: 44 additions & 0 deletions src/js/background/background.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,45 @@ browser.runtime.onInstalled.addListener(async (details) => {
}
});

/**
* Function that makes any API/fetch request on behalf of a content script
*
* @async
* @function fetchRequestFromBackground
* @param {string} url=null - URL of API route
* @param {string} fetchMethod="GET" - fetch method type
* @param {object} body=null - Overwrite if the request type (PATCH, POST) is method that needs to send data to the server
* @param {object} opts=null} Option to add users apiToken for protected API calls
* @return {string} JSON formatted response from the API
*/
async function fetchRequestFromBackground({url = null, fetchMethod = "GET", body = null, opts=null} = {}) {
// URL is the only "required" argument to make a fetch.
// Default
if (url == null) {
return false;
}

const headers = new Headers();

headers.set("Content-Type", "application/json");
headers.set("Accept", "application/json");

if (opts && opts.auth) {
const apiToken = await browser.storage.local.get("apiToken");
headers.set("Authorization", `Token ${apiToken.apiToken}`);
}

const response = await fetch(url, {
mode: "same-origin",
method: fetchMethod,
headers: headers,
body,
});

const answer = await response.json();
return answer;
}

// This function is defined as global in the ESLint config _because_ it is created here:
// eslint-disable-next-line no-redeclare, no-unused-vars
async function getAliasesFromServer(method = "GET", opts=null) {
Expand Down Expand Up @@ -396,6 +435,11 @@ browser.runtime.onMessage.addListener(async (m, sender, _sendResponse) => {
case "iframeCloseRelayInPageMenu":
browser.tabs.sendMessage(sender.tab.id, {message: "iframeCloseRelayInPageMenu"});
break;
case "fetchRequestFromBackground":
response = await fetchRequestFromBackground(
m.fetchRequest
);
break;
case "fillInputWithAlias":
browser.tabs.sendMessage(sender.tab.id, m.message);
break;
Expand Down
116 changes: 67 additions & 49 deletions src/js/relay.firefox.com/get_profile_data.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,24 @@
const apiToken = profileMainElement.dataset.apiToken;
await browser.storage.local.set({ apiToken });

const cookieString =
typeof document.cookie === "string" ? document.cookie : "";

const cookieStringArray = cookieString
.split(";")
.map((individualCookieString) => individualCookieString.split("="))
.map(([cookieKey, cookieValue]) => [
cookieKey.trim(),
cookieValue.trim(),
]);

const [_csrfCookieKey, csrfCookieValue] = cookieStringArray.find(
([cookieKey, _cookieValue]) => cookieKey === "csrftoken"
);

await browser.storage.local.set({ csrfCookieValue: csrfCookieValue });


Comment on lines +49 to +66
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think you need a CSRF cookie if you're not making the request from the Relay website?

Suggested change
const cookieString =
typeof document.cookie === "string" ? document.cookie : "";
const cookieStringArray = cookieString
.split(";")
.map((individualCookieString) => individualCookieString.split("="))
.map(([cookieKey, cookieValue]) => [
cookieKey.trim(),
cookieValue.trim(),
]);
const [_csrfCookieKey, csrfCookieValue] = cookieStringArray.find(
([cookieKey, _cookieValue]) => cookieKey === "csrftoken"
);
await browser.storage.local.set({ csrfCookieValue: csrfCookieValue });

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dug into the git-blame on the cookie string origins. It was me. 🪞👨

200e2fb

I think the initial reasoning was to verify if a user had the server-storage pref enabled? I'll remove this and confirm this doesn't break anything.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correction: I wasn't reading the full use-case of it. The CSRF token is passed in the Headers object for the follow API calls:

  • patchMaskInfo
  • getServerStoragePref
  • postReportWebcompatIssue
  • getAliasesFromServer
  • makeRelayAddress

I did remove the CSRF header setter for the fetchRequestFromBackground() function.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See 25255d1

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah OK, I wasn't aware that there was already code in background.js that read csrfCookieValue from the extension storage. AFAIK that's not needed, but it's outside of the scope of this PR then anyway.

// API URL is ${RELAY_SITE_ORIGIN}/api/v1/
const { relayApiSource } = await browser.storage.local.get("relayApiSource");

Expand All @@ -54,50 +72,13 @@
const relayApiUrlDomainAddresses = `${relayApiSource}/domainaddresses/`;
const relayApiUrlRelayNumbers = `${relayApiSource}/relaynumber/`;

async function apiRequest(url, method = "GET", body = null, opts=null) {

const cookieString =
typeof document.cookie === "string" ? document.cookie : "";
const cookieStringArray = cookieString
.split(";")
.map((individualCookieString) => individualCookieString.split("="))
.map(([cookieKey, cookieValue]) => [
cookieKey.trim(),
cookieValue.trim(),
]);

const [_csrfCookieKey, csrfCookieValue] = cookieStringArray.find(
([cookieKey, _cookieValue]) => cookieKey === "csrftoken"
);

browser.storage.local.set({ csrfCookieValue: csrfCookieValue });


const headers = new Headers();


headers.set("X-CSRFToken", csrfCookieValue);
headers.set("Content-Type", "application/json");
headers.set("Accept", "application/json");

if (opts && opts.auth) {
const apiToken = await browser.storage.local.get("apiToken");
headers.set("Authorization", `Token ${apiToken.apiToken}`);

const serverProfileData = await browser.runtime.sendMessage({
method: "fetchRequestFromBackground",
fetchRequest: {
url: apiProfileURL
}


const response = await fetch(url, {
mode: "same-origin",
method,
headers: headers,
body,
});

const answer = await response.json();
return answer;
}

const serverProfileData = await apiRequest(apiProfileURL);
});

browser.storage.local.set({
profileID: parseInt(serverProfileData[0].id, 10),
Expand All @@ -108,7 +89,13 @@

const siteStorageEnabled = serverProfileData[0].server_storage;

const relayNumbers = await apiRequest(relayApiUrlRelayNumbers);
const relayNumbers = await await browser.runtime.sendMessage({
method: "fetchRequestFromBackground",
fetchRequest: {
url: relayApiUrlRelayNumbers
}
});

if (Array.isArray(relayNumbers) && relayNumbers.length > 0) {
browser.storage.local.set({
relayNumbers: relayNumbers,
Expand All @@ -121,10 +108,21 @@
* @param {{ fetchCustomMasks?: boolean }} options - Set `fetchCustomMasks` to `true` if the user is a Premium user.
*/
async function refreshLocalLabelCache(options = {}) {

/** @type {RandomMask[]} */
const relayAddresses = await apiRequest(relayApiUrlRelayAddresses);
const relayAddresses = await browser.runtime.sendMessage({
method: "fetchRequestFromBackground",
fetchRequest: {
url: relayApiUrlRelayAddresses
}
});
const domainAddresses = options.fetchCustomMasks
? await apiRequest(relayApiUrlDomainAddresses)
? await browser.runtime.sendMessage({
method: "fetchRequestFromBackground",
fetchRequest: {
url: relayApiUrlDomainAddresses
}
})
: [];
await browser.storage.local.set({
relayAddresses:
Expand Down Expand Up @@ -235,7 +233,15 @@

if (body.description.length > 0 || body.generated_for.length > 0 || body.used_on.length > 0) {
const endpoint = alias.mask_type === "custom" ? relayApiUrlDomainAddresses : relayApiUrlRelayAddresses;
await apiRequest(`${endpoint}${alias.id}/`, "PATCH", JSON.stringify(body), {auth: true});
await browser.runtime.sendMessage({
method: "fetchRequestFromBackground",
fetchRequest: {
url: `${endpoint}${alias.id}/`,
fetchMethod: "PATCH",
body: JSON.stringify(body),
opts: {auth: true}
}
});
}
}
}
Expand All @@ -257,9 +263,21 @@
if (siteStorageEnabled) {
// Sync alias data from server page.
// If local storage items exist AND have label metadata stored, sync it to the server.
const serverRelayAddresses = await apiRequest(relayApiUrlRelayAddresses);

const serverRelayAddresses = await browser.runtime.sendMessage({
method: "fetchRequestFromBackground",
fetchRequest: {
url: relayApiUrlRelayAddresses
}
});

const serverDomainAddresses = isPremiumUser
? await apiRequest(relayApiUrlDomainAddresses)
? await browser.runtime.sendMessage({
method: "fetchRequestFromBackground",
fetchRequest: {
url: relayApiUrlDomainAddresses
}
})
: [];

// let usage: This data may be overwritten when merging the local storage dataset with the server set.
Expand Down