fix: adjust extension permissions #1613
Conversation
| function addLocalhostPermissionsToManifest(originalManifest: string): string { | ||
| const manifest = JSON.parse(originalManifest); | ||
|
|
||
| manifest['permissions'].push('http://localhost:9050/*'); |
There was a problem hiding this comment.
Consider pulling this number out of testResourceServerConfig.port
|
|
||
| export interface ExtensionOptions { | ||
| suppressFirstTimeDialog: boolean; | ||
| addLocalhostToPermissions?: boolean; |
There was a problem hiding this comment.
I think it would probably help prevent bugs for this to be non-optional; if a test wants to not have the extra permissions, it has to choose that explicitly
There was a problem hiding this comment.
I see it the other way around: we don't give those permissions, unless the test explicitly set this flag to true
There was a problem hiding this comment.
I don't think it should default to true, I'm saying I think it should maybe not have any default value (force every test to choose explicitly)
There was a problem hiding this comment.
It defaults to false and having optional make tests that don't need this permissions less verbose about it.
smoralesd
force-pushed
the
permissions
branch
from
b33fa95
to
8b14da0
Compare
smoralesd
changed the title
smoralesd
force-pushed
the
permissions
branch
from
8b14da0
to
bf1ce68
Compare
|
This PR was more of an exploration and proof-of-concept. We have prove the permissions works and found at least one issue. I'll will abandon this PR and re-work it from latest master again. I'll be also taking care of the comments left on this PR. |
Description of changes
Using
"activeTab"permissions instead of wide-matching patterns ("https://*/*", "http://*/*", "file://*/*") in order to avoid getting flagged with in pending review status when updating the extension on google web store.We have filed issue #1591 to track cannary not being properly update since 10/24 but this affect all of our extensions. This is related to chrome extension permissions and an effort to prevent malisious extension on the web store. Chrome developer website has a transition guide.
While changing the permissions seems to be working fine on the extension itself(we have a pending general-pass validation session from the team), the change has broke end to end test where we need to inject js/css to the target page (e.g, there is a test for the ad hoc panel where we enable each and every toggle that breaks with a permissions denied-kind of error).
This is due to how the
"activeTab"permission works. Simply put, our extension gets permissions to inject js/css when the user activates our extension. There are basically 2 ways this happen:We're using
Puppeteerfor our end-to-end tests and there is currently no API to efectivle use 1. as mentioned here.I tried option 2. by using
Puppeteer's Keyboard api to no success. I also looked at a couple of npm packages that send keyboard strokes at the OS level (most notably: kbm-robot, node-key-sender) but they relay onJava, which seems a medium to mayor size/impact decission for our infrastructure. After that, I didn't want to invest more time with this approach as we need to unblock ourselves as fast as we can, finding a balance between time spend, robustness and correctness on the approach we choose.The choosen approach was based on @dbjorge suggestion to follow this chrome ligthouse PR where they decided to modified the
manifest.json, adding a permission to facilitated e2e testing.Pull request checklist
yarn fastpassyarn test)<rootDir>/test-results/unit/coveragefix:,chore:,feat(feature-name):,refactor:). Check workflow guide at:<rootDir>/docs/workflow.md