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

Allow other extensions to connect #3997

Merged
merged 1 commit into from
May 21, 2018
Merged

Allow other extensions to connect #3997

merged 1 commit into from
May 21, 2018

Conversation

jakubsta
Copy link
Contributor

As discussed in #940 this PR allows to connect to MetaMask from other extensions.

Resolves #940

@danfinlay
Copy link
Contributor

This looks good at a cursory glance, but please also understand we need to treat this carefully, and QA/security review it thoroughly before merging. I will try to prioritize completing this in the next sprint or the one after. Sorry for the wait, but I'm sure you don't want us neglecting security either ;)

@june07
Copy link

june07 commented Apr 24, 2018

@kumavis kumavis added the DO-NOT-MERGE Pull requests that should not be merged label Apr 25, 2018
@kumavis
Copy link
Member

kumavis commented Apr 25, 2018

adding label to make sure this doesn't get in without extra review

@kumavis
Copy link
Member

kumavis commented Apr 25, 2018

This does look good! Need to read up on the APIs and make sure they match our assumptions.

@danfinlay
Copy link
Contributor

I've reviewed the API, it does seem to do what we expect here:
https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/runtime/onConnectExternal

@danfinlay danfinlay removed the DO-NOT-MERGE Pull requests that should not be merged label May 10, 2018
@kumavis
Copy link
Member

kumavis commented May 12, 2018

Can you connect externally to *? I think I remember this being a problem when I looked at it a while back
maybe the "ids": ["*"] is the trick

Edit:
https://developer.chrome.com/extensions/manifest/externally_connectable#reference

ids checks apps/extensions, matches checks web pages

ids (array of string) - optional

The IDs of extensions or apps that are allowed to connect. If left empty or unspecified, no extensions or apps can connect.

The wildcard "*" will allow all extensions and apps to connect.

matches (array of string) - optional

The URL patterns for web pages that are allowed to connect. This does not affect content scripts. If left empty or unspecified, no web pages can connect.

Patterns cannot include wildcard domains nor subdomains of (effective) top level domains; ://google.com/ and http://.chromium.org/ are valid, while <all_urls>, http:///, ://.com/, and even http://.appspot.com/* are not.

@jakubsta
Copy link
Contributor Author

Yes I can connect from external extension like here: https://github.com/jakubsta/example-extension/blob/master/background.js#L7 (it connects to extension built from this PR)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants