search

cschiller / zhongwen

Official source code of the "Zhongwen" Chrome extension
https://chrome.google.com/webstore/detail/zhongwen-chinese-english/kkmlkkjojmombglmlpbpapmhcaljjkde
GNU General Public License v2.0
322 stars 54 forks source link

Reduce necessary extension permissions #54

Closed jrmaingo closed 4 years ago

jrmaingo commented 4 years ago

These changes make it so installing the extension doesn't require the user to explicitly give any permissions. The extension uses the activeTab permission instead of the tabs permission and content scripts that access all_urls in manifest.json. The changes depend on tabs.executeScript which is implemented differently in Chrome so I've pulled in browser-polyfill so the same behaviour can be achieved across browsers.

cschiller commented 4 years ago

Have you tested this? It doesn't seem to work with more than one tab.

Also, it adds a lot of complexity to the code. In particular, a minified JavaScript file without the original source code is a potential security risk.

What's wrong with asking for permissions?

jrmaingo commented 4 years ago

Thanks for the quick response!

Have you tested this? It doesn't seem to work with more than one tab.

Only in a single tab unfortunately. I intended to only load the scripts once per tab but instead it is being loaded only once ever. I'll fix that, shouldn't be too difficult.

Also, it adds a lot of complexity to the code. In particular, a minified JavaScript file without the original source code is a potential security risk.

What's wrong with asking for permissions?

I know it adds some complexity but I think it's worth the trade off to be more clear about what data the extension has access too. Right now, the extension has permission to "Access your data for all websites" which allows it to "read the content of any web page you visit as well as data you enter into those web pages, such as usernames and passwords"[1]. I decided to make this PR because I personally feel better knowing that I don't need to grant this permission to the extension if possible and I figured others may appreciate it as well.

I don't think there's anything wrong with asking for permissions but I figure that it would be good to not ask for unnecessary permissions.

As for the minified JS file, I'd be happy to use a package manager to pull in the dep automatically but I decided to match the way that jquery has been currently setup. Unfortunately the library[2] doesn't provide package signatures but you can download the lib yourself and compare the files/signatures.

[1] https://support.mozilla.org/en-US/kb/permission-request-messages-firefox-extensions?as=u&utm_source=inproduct#w_access-your-data-for-all-websites [2] https://github.com/mozilla/webextension-polyfill

jrmaingo commented 4 years ago

So there are still issues where the the extension scripts fail to load when a new tab is opened in the background or the user switches between tabs quickly. Unfortunately there isn't a good way that I can see to ensure effective coordination here without adding a lot of complexity to the script loading code. I'll close this out for now as it would need a lot more work to be stable enough to use.