1899389 - Source maps requiring credentials no longer load

Status: VERIFIED FIXED

(DevTools :: Framework, defect, P3)

Description

[Emanuel Hoogeveen [:ehoogeveen]]

Bisected to bug 1881800 using mozregression: https://phabricator.services.mozilla.com/D203334

I'm a developer for MedSpace, a scheduling web application for medical professionals. Most of our files are only served to authenticated users, and our source maps are similarly protected. Recently I noticed that our source maps stopped working in Firefox, where they still work fine in Chrome and other browsers. Source maps are a key component of our development process, as the files we serve are minified.

I don't know if there's something particular about the way source maps are loaded in our environment, but looking at the regressing bug I'm hoping that the issue is general enough to be easy to identify.

I should be able to set up a testing account to help demonstrate the problem if needed (I know this ticket is missing links and/or testcases right now).

Steps to reproduce:

1. Visit a website that requires authentication, where sources do not load (e.g. redirect to a login page) if credentials are not passed.
2. Open the devtools and view a file that should have a source map.

Expected result:
Source maps are loaded using the same credentials as the sources.

Actual result:
The console prints a warning about the source map being invalid (due to receiving a login page instead) and the source map does not load.

Comment 1

[Liz Henry (:lizzard) (relman/hg->git project)]

Gijs, can you take a look since you may have written the regressing patch? Thanks!

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1881800

Comment 3

[:Gijs (out for now; he/him)]

I'm very sorry but I don't have cycles to poke at this right now due to an unusually high number of other things I'm firefighting.

Julien, can you take a look? Where does source map fetching code live? Both fetch and XMLHttpRequest have ways in which to override the new default (see regressing bug and patch discussions). The code changes in bug 1881800 didn't touch devtools code and I haven't yet touched NetUtil.newChannel, so I'm not sure what consumer I missed that was expecting the previous default (I did try to audit all existing consumers...).

Emanuel: as a temporary workaround, you can flip network.fetch.systemDefaultsToOmittingCredentials to false in about:config and that should resolve the issue. Do let us know if that doesn't work...

Liz, do we think this is high-severity enough that we need to push a pref change via Normandy/Nimbus in the interim for 126? For 127 if we don't have something better in the next 24h we could evaluate just making the code change to that effect (bool pref from true to false).

I'm kind of surprised this passed tests and didn't get picked up at devedition/nightly cycle time - it's just rode the trains. When we fix we should ensure we have automated tests for credential'd source maps.

Comment 4

[Emanuel Hoogeveen [:ehoogeveen]]

I can confirm that setting network.fetch.systemDefaultsToOmittingCredentials to false works around the issue!

Do let me know if a reproduction link with a temporary account would help; the only reason I haven't so far is that our production environment doesn't have anything ready to go and our development server isn't externally accessible.

Comment 5

[Julian Descottes [:jdescottes]]

Usually the sourcemaps in devtools are retrieved via a fetch call performed by a worker in the parent process at https://searchfox.org/mozilla-central/rev/f60bb10a5fe6936f9e9f9e8a90d52c18a0ffd818/devtools/client/shared/source-map-loader/utils/network-request.js#23-26

I imagine that we should add { credentials: "same-origin" }, but I'd like to check what kind of sourcemap is problematic here first.

(In reply to Emanuel Hoogeveen [:ehoogeveen] from comment #4)

I can confirm that setting network.fetch.systemDefaultsToOmittingCredentials to false works around the issue!

Do let me know if a reproduction link with a temporary account would help; the only reason I haven't so far is that our production environment doesn't have anything ready to go and our development server isn't externally accessible.

Yes a test page would very helpful here. Also are you talking about JS or CSS sourcemaps here?

Comment 6

[Emanuel Hoogeveen [:ehoogeveen]]

(In reply to Julian Descottes [:jdescottes] from comment #5)

Yes a test page would very helpful here. Also are you talking about JS or CSS sourcemaps here?

I've noticed it with JS source maps, but I think CSS source maps are affected too. They don't look minified but I think it might be doing some pretty printing automatically.

I've set up a temporary test account:

1. Go to https://secure.medspace.nl/medspace/demonl.nsf
2. Log in with username testuser123 and password Bug1899389!
3. In the debugger, open resources/0/production/202310/js/ext/core-js.js
4. Note that it says "Source Map Error: JSON.parse: unexpected character at line 1 column 1 of the JSON data" and the source map is not available.
5. Set network.fetch.systemDefaultsToOmittingCredentials to false
6. Note that the source map is now available, displaying the source resources/0/production/202310/js/ext/full/core-js.js

This is a demo environment and the account has limited privileges (note that it can't access all source maps - just the ones for 3rd party libraries). Let me know if you can't access the link - we've had issues with some foreign IP addresses in the past so we may need to allow access.

Comment 7

[Julian Descottes [:jdescottes]]

Thanks for the test page! I could reproduce the issue, and verify that adding { credentials: "same-origin" } to the DevTools fetch call fixes the issue.
Gijs: do you have any pointer to tests using a similar setup so that I could add one for devtools?

Comment 8

[:Gijs (out for now; he/him)]

(In reply to Julian Descottes [:jdescottes] from comment #7)

Thanks for the test page! I could reproduce the issue, and verify that adding { credentials: "same-origin" } to the DevTools fetch call fixes the issue.
Gijs: do you have any pointer to tests using a similar setup so that I could add one for devtools?

Something like https://searchfox.org/mozilla-central/rev/f60bb10a5fe6936f9e9f9e8a90d52c18a0ffd818/browser/components/privatebrowsing/test/browser/title.sjs#14 for an sjs file where the response depends on the cookie being present or not? Would want to do that for the source map, the actual script file could be a "normal" support-file. Then load in a tab and do whatever devtools tests normally do to check the source map loads.

A unit test (rather than integration with a "full" page load, opening the devtools, etc.) would be more straightforward in the sense that you can do something similar to https://searchfox.org/mozilla-central/rev/f60bb10a5fe6936f9e9f9e8a90d52c18a0ffd818/dom/fetch/tests/browser_default_credentialless_fetch.js and set a cookie using the cookie manager, then fetch the source map, and verify that the request to get the source map sends the cookie.

I had a quick look earlier today and I'm nervous that there might be other fetch() callers in devtools that would also want to change. But I'm not sure how to check for sure - https://searchfox.org/mozilla-central/rev/f60bb10a5fe6936f9e9f9e8a90d52c18a0ffd818/devtools/client/performance-new/shared/profiler_get_symbols.js#535 and https://searchfox.org/mozilla-central/rev/f60bb10a5fe6936f9e9f9e8a90d52c18a0ffd818/devtools/client/performance-new/shared/background.sys.mjs#826,840 may both not want credentials and/or not run with system principal? I'm also unsure what happens with https://searchfox.org/mozilla-central/rev/f60bb10a5fe6936f9e9f9e8a90d52c18a0ffd818/devtools/server/actors/utils/sources-manager.js#467-471 and/or how to test it.

Comment 9

[:Gijs (out for now; he/him)]

https://searchfox.org/mozilla-central/rev/f60bb10a5fe6936f9e9f9e8a90d52c18a0ffd818/devtools/server/actors/utils/stylesheet-utils.js#108,115,126 and that sources-manager.js bit both pass principal, but if I'm reading things right, that still won't override the principal we use to determine whether we send credentials. We probably should. That probably wants to be factored out into a separate bug at this point, as we could/should probably fix it in fetch code itself.

Edit: to be clear, haven't had a chance to check. So perhaps I'm wrong?

Comment 10

[:Gijs (out for now; he/him)]

... though anyway, https://searchfox.org/mozilla-central/source/dom/webidl/Request.webidl#58 doesn't have a principal member? So perhaps that fetch call isn't actually going through "normal" fetch directly?

Edit: right, it's using DevToolsUtils.js which goes through NetUtil. Perhaps this tripped me up when I originally tried to audit these? Is the network-request.js case the only one that doesn't do that? Looks like the performance-new code also doesn't, but again, not sure what the state of that code is.

Comment 11

[Julian Descottes [:jdescottes]]

I think you're right, most of the time DevTools' fetch is actually using NetUtil apart from the source maps worker.
Regarding performance-new, let's ask Julien, I'm not sure in which contexts those fetch calls are issued.

Julien, do you know if the following fetch calls could be impacted by the issue discussed here (basically can they target URLs potentially using credentials and are they using the system principal):

• https://searchfox.org/mozilla-central/rev/f60bb10a5fe6936f9e9f9e8a90d52c18a0ffd818/devtools/client/performance-new/shared/profiler_get_symbols.js#535
• https://searchfox.org/mozilla-central/rev/f60bb10a5fe6936f9e9f9e8a90d52c18a0ffd818/devtools/client/performance-new/shared/background.sys.mjs#826,840

I suspect the answer is no?

Comment 12

[Liz Henry (:lizzard) (relman/hg->git project)]

Donal this is more a question for you and your call! If we haven't had more complaints about this maybe it is OK to leave this alone for 126, but you will likely have a better idea as 126 owner.

Comment 13

[Emanuel Hoogeveen [:ehoogeveen]]

I can only speak for myself but I noticed this pretty quickly in our environment. Unfortunately the change landed just before I went on holiday, and I've been busy with backend development since so I only ran into it again recently (and finally had enough time to report it).

I can think of a few reasons why this might not come up that much in practice:

1. Since source code is often static, perhaps it isn't subject to authentication in most environments. You need to authenticate to reach a private area, but the source code for that area need not be private.
2. Due to the legacy nature of our backend, we do a lot of our debugging in the browser without a lot of ability to hotswap the code to something unminified. Perhaps most web developers can just spin up a local server and debug in their IDE of choice, with hot reloads and so on.

Comment 14

[Donal Meehan [:dmeehan]]

I haven't seen any reports for this other than here. As it stands now, I would say Fx126 is ok to leave as is.
However couple of things:

• This has not been triaged for severity (adding an NI on triage owner)
• Fx127 builds next week, so we have little time to address this either way for Fx127.

Comment 15

[Julian Descottes [:jdescottes]]

Attachment: Bug 1899389 - [devtools] Use credentials for devtools sourcemap fetch call

Comment 16

[Julien Wajsberg [:julienw]]

(In reply to Julian Descottes [:jdescottes] from comment #11)

I think you're right, most of the time DevTools' fetch is actually using NetUtil apart from the source maps worker.
Regarding performance-new, let's ask Julien, I'm not sure in which contexts those fetch calls are issued.

Julien, do you know if the following fetch calls could be impacted by the issue discussed here (basically can they target URLs potentially using credentials and are they using the system principal):

• https://searchfox.org/mozilla-central/rev/f60bb10a5fe6936f9e9f9e8a90d52c18a0ffd818/devtools/client/performance-new/shared/profiler_get_symbols.js#535

I believe we never go this path currently. The input is already a WebAssembly Module (checked in https://searchfox.org/mozilla-central/source/devtools/client/performance-new/shared/symbolication.worker.js#224-229).
In case this ever changes, this would always use a hardcoded URL anyway (https://searchfox.org/mozilla-central/rev/23efe2c8c5b3a3182d449211ff9036fb34fe0219/devtools/client/performance-new/shared/symbolication.sys.mjs#49-50)

The fetch for this URL is currently in https://searchfox.org/mozilla-central/rev/23efe2c8c5b3a3182d449211ff9036fb34fe0219/devtools/client/performance-new/shared/symbolication.sys.mjs#75.

• https://searchfox.org/mozilla-central/rev/f60bb10a5fe6936f9e9f9e8a90d52c18a0ffd818/devtools/client/performance-new/shared/background.sys.mjs#826,840

This one comes from a preference, so theorically it could include credentials I guess? But encoded in the URL with username:password. Not sure if this is relevant to this discussion.

Comment 17

[Alexandre Poirot [:ochameau]]

jdescottes is on it, still a chance to bundle that in 127?

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/754e6dd6610e
[devtools] Use credentials for devtools sourcemap fetch call r=devtools-reviewers,ochameau

Comment 19

[Donal Meehan [:dmeehan]]

(In reply to Alexandre Poirot [:ochameau] from comment #17)

jdescottes is on it, still a chance to bundle that in 127?

Thanks for setting the severity.
The timing looks good to get this into 127 since a fix is already on autoland.

:jdescottes please add a beta uplift request when ready. Only mentioning this now to save some turnaround time, since the final 127 beta builds tomorrow.
However, we wouldn't uplift until after it lands on central, depending on the risk, etc.

Comment 20

[Julian Descottes [:jdescottes]]

Comment on attachment 9404663 [details]
Bug 1899389 - [devtools] Use credentials for devtools sourcemap fetch call

Beta/Release Uplift Approval Request

• User impact if declined: DevTools users relying on sourcemaps with authentication are no longer able to see the mapping to original sources and cannot debug their applications.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This change is restoring the credentials argument of a DevTools fetch call to the value it had before Bug 1881800, and the change is covered by an automated test.
• String changes made/needed:
• Is Android affected?: No

Comment 21

[Julian Descottes [:jdescottes]]

(In reply to Donal Meehan [:dmeehan] from comment #19)

(In reply to Alexandre Poirot [:ochameau] from comment #17)

jdescottes is on it, still a chance to bundle that in 127?

Thanks for setting the severity.
The timing looks good to get this into 127 since a fix is already on autoland.

:jdescottes please add a beta uplift request when ready. Only mentioning this now to save some turnaround time, since the final 127 beta builds tomorrow.
However, we wouldn't uplift until after it lands on central, depending on the risk, etc.

I think the patch itself is really low-risk, it should be fine to uplift as soon as it sticks on central.

Comment 22

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/754e6dd6610e

Comment 23

[Pascal Chevrel:pascalc]

P3/S3 and it just landed in central, this is too late for our last beta tomorrow, let's reevaluate for the planned dot release.

Comment 24

[Emanuel Hoogeveen [:ehoogeveen]]

Verified in the latest Nightly - source maps work again!