Add Access-Control-Allow-Origin: * to public XPI files

Rob--W commented 6 years ago

Describe the problem and steps to reproduce it:

Public XPI files are available at addons.mozilla.org and served (via a redirect) from addons.cdn.mozilla.net. Neither of these endpoints include CORS headers in the response.

For example, send a HEAD request as follows:

$ curl -H 'Origin: example.com' -I https://addons.mozilla.org/firefox/downloads/latest/crxviewer/crxviewer.xpi
$ curl -H 'Origin: example.com' -I 'https://addons.cdn.mozilla.net/user-media/addons/717783/extension_source_viewer-1.6.5-an+fx.xpi'

What happened?

Neither response includes the Access-Control-Allow-Origin: * header.

$ curl -H 'Origin: example.com' -I https://addons.mozilla.org/firefox/downloads/latest/crxviewer/crxviewer.xpi
HTTP/1.1 302 Found
Content-Length: 0
...
Location: https://addons.cdn.mozilla.net/user-media/addons/717783/extension_source_viewer-1.6.5-an+fx.xpi?filehash=sha256%3Abf016976118fcbb0fd299ebdfed530bbc18ecf02e8a9ee296dec6d1ad3fdeb03
...

$ curl -H 'Origin: example.com' -I 'https://addons.cdn.mozilla.net/user-media/addons/717783/extension_source_viewer-1.6.5-an+fx.xpi'
HTTP/1.1 200 OK
Content-Type: application/x-xpinstall
Content-Length: 210657
...

What did you expect to happen?

Both responses should include the Access-Control-Allow-Origin: * header, at least for publicly listed add-ons.

Anything else we should know?

I have an add-on to view the source code of extensions: https://github.com/Rob--W/crxviewer Firefox has started to prevent extensions from sending requests to AMO unless CORS is enabled: https://bugzilla.mozilla.org/show_bug.cgi?id=1450649

To avoid the need for an intermediate proxy, the XPI files have to be served with the Access-Control-Allow-Origin: * header.

(This is distinct from mozilla/addons#5649, which request CORS responses for authenticated API endpoints. I want CORS for XPI files without authentication).

(edit by @diox: add -H Origin: example.com to requests in STR)

diox commented 6 years ago

Note: like with mozilla/addons#5649 it's likely this would need to be done on the ops side

wagnerand commented 5 years ago

Paging @bqbn, @autrilla.

stale[bot] commented 5 years ago

This issue has been automatically marked as stale because it has not had recent activity. If you think this bug should stay open, please comment on the issue with further details. Thank you for your contributions.

Rob--W commented 5 years ago

The comments from the original report are still relevant.

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. If you think this bug should stay open, please comment on the issue with further details. Thank you for your contributions.

Rob--W commented 4 years ago

Still relevant, keep it open.

diox commented 4 years ago

Since it's an op issue, Rob, do you mind filing a bugzilla bug in Cloud Services / Operations: AMO ? Cc me on that and I'll get it moving.

Rob--W commented 4 years ago

Done: https://bugzilla.mozilla.org/show_bug.cgi?id=1620084

Rob--W commented 4 years ago

:wezhou asked whether we should add the header to both AMO and the CDN in the bug. I'd say yes to the CDN, but no for AMO. Adding the header in AMO itself seems to be quite straightforward:

Public addons are available on the CDN:

https://github.com/mozilla/addons-server/blob/b0f6f466cd63d3957152ecc6177e36fbe8af99f1/src/olympia/versions/views.py#L88-L91

When both of the above conditions are true (i.e. the xpi is fully public), then this issue would be fixed by adding the Access-Control-Allow-Origin: * header here:

https://github.com/mozilla/addons-server/blob/b0f6f466cd63d3957152ecc6177e36fbe8af99f1/src/olympia/versions/views.py#L107-L109

diox commented 4 years ago

Yeah I'll deal with the AMO side in code.

bqbn commented 4 years ago

The CDN should be using whatever header the app returns, which is to say if the app returns that header for addons that satisfy https://github.com/mozilla/addons-server/blob/b0f6f466cd63d3957152ecc6177e36fbe8af99f1/src/olympia/versions/views.py#L88-L91 then the CDN should do too.

We can verify if that's the case once the fix lands for the app.

diox commented 4 years ago

We redirect to the CDN (or sometimes directly link to it), and it's serving from nginx, bypassing the app completely.

Example with ublock origin:

https://addons.mozilla.org/firefox/downloads/file/3509800/ublock_origin-1.25.0-an+fx.xpi is served by the app (and we need to apply the right Access-Control-Allow-Origin here), it redirects to:
https://addons.cdn.mozilla.net/user-media/addons/607454/ublock_origin-1.25.0-an+fx.xpi?filehash=sha256%3A2c1becc2f25255e46cf4560c32ef196f9c95d4438c48e92282ce89251851d65f which is served by the CDN directly from nginx, the app never sees that request. We also need to add the Access-Control-Allow-Origin header here.

Rob--W commented 4 years ago

Thanks for fixing this @diox! When https://bugzilla.mozilla.org/show_bug.cgi?id=1620084 is fixed and AMO is updated, I can update my crxviewer so that the online version becomes equally fast as the extension version, at least for Firefox addons :)

diox commented 4 years ago

Sorry it took so long for something so trivial :) It dropped from my radar and I just caught it because of the stale notification...

AlexandraMoga commented 4 years ago

@diox I can see Access-Control-Allow-Origin: * in the response header for the following request:

curl -H 'Origin: example.com' -I https://addons-dev.allizom.org/firefox/downloads/latest/bookmarks-organizer/bookmarks_organizer-3.0.1-fx.xpi

but I'm not seeing it for this request:

curl -H 'Origin: example.com' -I https://addons-dev-cdn.allizom.org/user-media/addons/502867/bookmarks_organizer-3.0.1-fx.xpi

diox commented 4 years ago

@AlexandraMoga that second request will be covered by https://bugzilla.mozilla.org/show_bug.cgi?id=1620084 (it requires ops involvement)

mozilla / addons