`@pixiebrix/get` reports CORS errors and failed requests as "missing permissions"

fregante commented 2 years ago

This issue probably doesn't have a solution because it's a browser limitation.

Repro

Add "HTTP Get" brick
Set URL to https://ghosttext.fregante.com/
Run the extension

You will see that this request succeeds without permissions.

Append ?a=1 to the URL to skip the cache
Go offline
Run the extension

You will see ClientNetworkPermissionError: Insufficient browser permissions to make request. due to this logic:

https://github.com/pixiebrix/pixiebrix-extension/blob/4cf13fc048c6f3b5eac9248227b324b8b78c3cc3/src/services/requestErrorUtils.ts#L142-L151

But the logic is actually incorrect:

you don't need host permission to request the page
the request failed due to a network error rather than a permission error

Set URL to https://www.google.com
Run the extension

See CORS error in the background page:

Access to fetch at 'https://www.google.com/' from origin 'chrome-extension://mpjjildhmpddojocokjkgmlkkkfjnepo' has been blocked by CORS policy: No 'Access-Control-Allow-Origin' header is present on the requested resource. If an opaque response serves your needs, set the request's mode to 'no-cors' to fetch the resource with CORS disabled.

👆 this only happens if you're missing the www.google.com permissions, so the ClientNetworkPermissionError shown to the user is actually correct here.

Possible resolutions

[ ] Detect and differentiate network errors from CORS/permission errors

I don’t think this is actually feasible other than checking for navigator.onLine, but I don’t think it immediately turns false when a single request fails.

Almost related

twschiller commented 2 years ago

you don't need host permission to request the page

From a PixieBrix security model, the extension should declare the network calls it can make. Related: see https://github.com/pixiebrix/pixiebrix-extension/issues/3054.

As you point out, some GET requests don't technically require Chrome Extension permissions based on the CORS settings on the remote server. However, we don't want to rely on this nuance - messaging should be based on the permissions the user has granted

Long-term, we will want to have the check against the extensions declared permissions prior to making a network call and block unpermissioned requests (even if they would be allowed based on other permissions the user has granted). That will ensure that the extension will work when exported to other users

Go offline

Good point that we can't distinguish this. Could we potentially use https://developer.mozilla.org/en-US/docs/Web/API/NetworkInformation/type? Or, in the enrich method we could ping https://app.pixiebrix.com to detect network connectivity?

https://github.com/pixiebrix/pixiebrix-extension/issues/3054

fregante commented 2 years ago

There's navigator.onLine to detect when we're fully offline, but I don’t think that works when the connection is just bad (e.g. far from the wifi hotspot / someone is microwaving some popcorn)

in the enrich method

Yes if for this check we use fetch directly, or else we'll end up in a loop

fregante commented 2 years ago

A connectivity check was implemented. In the commit above I moved the check higher since being offline is the first thing the user should fix 🙂

It will be merged through https://github.com/pixiebrix/pixiebrix-extension/pull/3073

pixiebrix / pixiebrix-extension