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.

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

Manifest v3 problems: ability to update the set-cookie header in response headers and also set dynamic header values #626

Open
vish30 opened this issue Jun 4, 2024 · 13 comments
Labels
needs-triage: safari Safari needs to assess this issue for the first time

Comments

@vish30
Copy link

vish30 commented Jun 4, 2024

There are two issues in manifest v3:

  1. Not being able to modify the "set-cookie" response header.
  2. Not being able to set dynamic (runtime) values for any response & request header synchronously (before they are consumed by the browser/application/server).

We have an extension wherein the users can have different login sessions in each tab.
Here is my manifest v2 code:

  (details) => {
    const { tabId, responseHeaders } = details;
    responseHeaders.forEach((responseHeader) => {
      if (
        responseHeader.name.toLowerCase() === 'set-cookie'
      ) {
        responseHeader.value = `${tabId}_${responseHeader.value}`;
      }
    });
    return { responseHeaders };
  },
  { urls: allowedURLs },
  [
    'blocking',
    'responseHeaders',
    chrome.webRequest.OnBeforeSendHeadersOptions.EXTRA_HEADERS
  ].filter(Boolean)
);

I am not able to replicate this in manifest v3 as the webRequestBlocking permission is not available and the rules (dynamic/static) doesn't support adding dynamic/runtime value for any header. Also, updating the "set-cookie" header is also not allowed.

@github-actions github-actions bot added needs-triage: chrome Chrome needs to assess this issue for the first time needs-triage: firefox Firefox needs to assess this issue for the first time needs-triage: safari Safari needs to assess this issue for the first time labels Jun 4, 2024
@vish30 vish30 changed the title Manifest v3 problems: ability to update the Set-cookie header in response headers and also set dynamic values Manifest v3 problems: ability to update the set-cookie header in response headers and also set dynamic header values Jun 5, 2024
@oliverdunk
Copy link
Member

In Chrome it is already possible to modify the set-cookie header:

{
  "id": 1,
  "priority": 1,
  "action": {
    "type": "modifyHeaders",
    "responseHeaders": [
      { "header": "set-cookie", "operation": "set", "value": "example=test" }
    ]
  }
}

I've put a full example here - note that due to this bug those changes aren't currently visible in the request tab of DevTools. However, visiting https://example.com and then checking the Cookies section in DevTools should show the cookie.

Setting dynamic values is a request we've heard in the past, but as you point out isn't currently possible.

Is the core feature of your extension providing the different sessions per tab or are you using that to solve another problem?

@oliverdunk oliverdunk removed the needs-triage: chrome Chrome needs to assess this issue for the first time label Jun 6, 2024
@vish30
Copy link
Author

vish30 commented Jun 6, 2024

@oliverdunk Thanks for the clarification on "set-cookie" header.

However, the second issue is critical for my extension to be migrated to V3. And, as you guessed correctly, the main intention of the extension is to provide multi-session support in the same window. For this, I am adding tabId as a prefix for the value of the "set-cookie" response header and request header, I am removing the prefix from the "Cookie" header.

Here is the v2 extension: Extension link

@Rob--W Rob--W removed the needs-triage: firefox Firefox needs to assess this issue for the first time label Jun 6, 2024
@vish30
Copy link
Author

vish30 commented Jun 12, 2024

@oliverdunk
Can you please let us know if setting dynamic header values will be possible or not? If yes, an ETA will be really helpful.
If no, we would be forced to shift to firefox.

@oliverdunk
Copy link
Member

Hi @vish30, we don't have any changes on the roadmap which would make adding the tab ID possible from a static rule.

However, you can add session rules with a tabId condition (docs). Combined with some upcoming planned work to support RegEx matching and substitution for headers, I believe your use case should be solvable.

It would look something like this:

chrome.declarativeNetRequest.updateSessionRules({
  addRules: [{
    "id": 1,
    "action": {
      "type": chrome.declarativeNetRequest.RuleActionType.MODIFY_HEADERS,
      "responseHeaders": [
        {
          "header": "Set-Cookie",
          "operation": chrome.declarativeNetRequest.HeaderOperation.SET,
          "regexFilter": ".*",
          "regexSubstitution": `${tabId}_\\1`
        }
      ]
    },
    "condition": { tabIds: [tabId] },
  }]
});

You would be able to call this each time a tab is created.

This is next up on our roadmap so will hopefully be available soon.

@vish30
Copy link
Author

vish30 commented Jun 24, 2024

Hi @oliverdunk, thanks for the update.

But, I doubt will this solution also work for removing the same prefix from the cookie header before sending a request from client to server?

Here is a sample implementation in v2:

chrome.webRequest.onBeforeSendHeaders.addListener(
  (details) => {
    const { tabId, requestHeaders, url } = details;
    requestHeaders.forEach((requestHeader) => {
      if (
        requestHeader.name.toLowerCase() === 'cookie'
      ) {
        requestHeader.value = fnToRemovePrefixFromCookies(
          requestHeader.value,
          tabId
        );
      }
    });
    return { requestHeaders };
  },
  { urls: allowedURLs },
  [
    'requestHeaders',
    chrome.webRequest.OnBeforeSendHeadersOptions.EXTRA_HEADERS
  ].filter(Boolean)
);

@oliverdunk
Copy link
Member

But, I doubt will this solution also work for removing the same prefix from the cookie header before sending a request from client to server?

Could you share your fnToRemovePrefixFromCookies implementation? Assuming this can be expressed in the declarative syntax, we do plan to support RegEx matching on both request and response headers, so that should be possible.

@kabir-afk
Copy link

kabir-afk commented Jul 1, 2024

I have been facing quite a similar problem in appending to the CSP header. I have been trying to append the rules statically , to CSP header media-src directive to accept videos from 2 other hosts i.e., https://commons.wikimedia.org and https://upload.wikimedia.org. But they dont append at all for some reason , this is how things looked in MV2.

browser.webRequest.onHeadersReceived.addListener(info => {
	const headers = info.responseHeaders; // original headers
	console.log(headers);
    for (let i=headers.length-1; i>=0; --i) {
        let header = headers[i].name.toLowerCase();
        if (header === "content-security-policy") { // csp header is found
            // modifying media-src; this implies that the directive is already present
            headers[i].value = headers[i].value.replace("media-src", "media-src https://commons.wikimedia.org https://upload.wikimedia.org");
        }
    }
    // return modified headers
    return {responseHeaders: headers};
}, {
    urls: [ "<all_urls>" ], // match all pages
    types: [ "main_frame" ] // to focus only the main document of a tab
}, ["blocking", "responseHeaders"]);

For MV3 , my manifest.json looks something like this

"declarative_net_request": {
		"rule_resources": [
			{
				"id": "ruleset",
				"enabled": true,
				"path": "rules.json"
			}
		]
	},
	"permissions": [
    "webRequest",
    "declarativeNetRequest",
  ],
  "host_permissions":["https://*/*"],

Rules.json looks something like this

[
    {
        "id": 1,
        "priority": 1,
        "action": {
            "type": "modifyHeaders",
            "responseHeaders": [
                {
                    "header": "Content-Security-Policy",
                    "operation": "append",
                    "value": "media-src https://commons.wikimedia.org https://upload.wikimedia.org;"
                }
            ]
        },
        "condition": {
            "urlFilter": "*",
            "resourceTypes": [
                "main_frame"
            ]
        }
    }
]

Maybe appending two hosts at the same time might not work , but it doesn't seem to work even if I try to append the other host in a different rule object , with lower prioritization. Also the way I am setting things neither makes videos of my extension work nor the videos of youtube when in youtube. Things dont change even when done dynamically. I am aware of the workarounds but I still want to believe that things can be achieved this way,any help would suffice.

@oliverdunk
Copy link
Member

                "operation": "append",
                "value": "media-src https://commons.wikimedia.org https://upload.wikimedia.org;"

In Chromium, there is an allowlist for the headers that we allow you to append to: https://source.chromium.org/chromium/chromium/src/+/main:extensions/browser/api/declarative_net_request/constants.h;l=290;drc=4c831d47d206e2998aa54c8200ff5b55d3f39ec3. This is a subset of the headers that support multiple values.

It might make sense to add Content-Security-Policy there, although regex substitution definitely feels useful for less-trivial cases where you need to overwrite an existing part of the CSP (e.g overwrite or update an existing media-src).

@kabir-afk
Copy link

kabir-afk commented Jul 2, 2024 via email

@oliverdunk
Copy link
Member

Using the append operation in general is not possible, even if you just want to append a single value.

@kabir-afk
Copy link

kabir-afk commented Jul 2, 2024 via email

@oliverdunk
Copy link
Member

I'll see if we can clarify that. Regular expressions aren't supported for header values yet (the snippet above is something we are looking to implement shortly) but they should work for this when available.

@kabir-afk
Copy link

kabir-afk commented Jul 2, 2024 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-triage: safari Safari needs to assess this issue for the first time
Projects
None yet
Development

No branches or pull requests

4 participants