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

Support for Google Chrome Extension #512

Merged
merged 8 commits into from Jan 23, 2020

Conversation

AndrewBastin
Copy link
Member

This PR intends to introduce compatibility with Postwoman Chrome Extension

This adds a new NetworkStrategy to hook with the Chrome Content Script and Background Script to run the query.

NOTE

Do NOT use the extension from the store to test the strategy, because, the extension is configured to only hook into the postwoman.io and postwoman.netlify.com domains. Other domains won't get hooked and hence won't get access to the extension hooks.

So, to test this PR, you have to clone the postwoman-chrome repo.

Then head into the manifest.json file and edit it to match this snippet below

  "content_scripts": [
    {
      "matches": [
        "https://postwoman.io/",
        "https://postwoman.io/*",
        "https://postwoman.netlify.com/*",
        "https://postwoman.netlify.com/"
      ],
      "js": [ "contentScript.js" ]
    }
  ],

Then run npm install and then npm run build.

This will create a folder called dist with the generated code.

Then, open Chrome and navigate to chrome://extensions, click on Load Unpacked and then select the generated dist folder.

Next, go to your postwoman cloned repo and open the file functions/strategies/ChromeStrategy.js and edit EXTENSION_ID constant to match with the extension ID you got while loading our Unpacked Extension in the prior step (to find it, just click on the extension details of our extension).
The defined EXTENSION_ID is for the extension in the Chrome Web Store.

After this you can navigate to the Postwoman app in localhost and access it, you can check if the hook was successful or not by opening the console on the Postwoman app and checking for the console log "Connected to the Postwoman Chrome Extension!" after load.

Once that is done, just fire a request anywhere and you will see that CORS restrictions won't be applied.

NOTE: The Chrome extension isn't marked public yet in the Chrome Web Store, it will be marked public as soon as this PR is approved

@ghost
Copy link

ghost commented Jan 22, 2020

DeepCode's analysis on #8b7fd2 found:

  • 0 critical issues. ⚠️ 0 warnings and 0 minor issues. ✔️ 0 issues were fixed.

💬 This comment has been generated by the DeepCode bot, installed by the owner of the repository. The DeepCode bot protects your repository by detecting and commenting on security vulnerabilities or other critical issues.


☺️ If you want to provide feedback on our bot, here is how to contact us.

@AndrewBastin AndrewBastin added core Changes regarding core concepts feature New feature or request labels Jan 22, 2020
@AndrewBastin AndrewBastin added this to the v2.0 milestone Jan 22, 2020
@TravisBuddy
Copy link

Hey @AndrewBastin,
Your changes look good to me!

View build log

TravisBuddy Request Identifier: b1523290-3cb6-11ea-b9f2-e91196c7ff19

@TravisBuddy
Copy link

Hey @AndrewBastin,
Your changes look good to me!

View build log

TravisBuddy Request Identifier: a6a7c600-3cbd-11ea-b9f2-e91196c7ff19

@liyasthomas
Copy link
Member

@AndrewBastin How to detect Chrome extension bg activity as in Firefox?

@AndrewBastin
Copy link
Member Author

@liyasthomas

The way Chrome has implemented extensions, code is segregated into 3 separate worlds (extension background script, extension content script and the page script), none can exactly access or query the other directly.

We can't use the same trick by detecting injected properties as in Firefox due to the above mentioned segregation rule.

So, inorder for detection in Chrome, the extension adds an empty span to the Postwoman page body with ID chromePWExtensionDetect (see src/contentScript.ts in postwoman-chrome).
The page code can make sure that the Chrome Extension is installed by first checking if window.chrome is defined (to check if we are dealing with Chrome) and then checking if an element with this ID is present, I have added a helper function hasChromeExtensionInstalled in functions/strategies/ChromeStrategy.js to do it.

Copy link
Member

@jamesgeorge007 jamesgeorge007 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make use of relative imports for the sake of consistency.

functions/network.js Outdated Show resolved Hide resolved
AndrewBastin and others added 2 commits January 22, 2020 12:38
Co-Authored-By: James George <jamesgeorge998001@gmail.com>
@TravisBuddy
Copy link

Hey @AndrewBastin,
Your changes look good to me!

View build log

TravisBuddy Request Identifier: 709701c0-3d3f-11ea-a8ac-8912d21a789c

@TravisBuddy
Copy link

Hey @AndrewBastin,
Your changes look good to me!

View build log

TravisBuddy Request Identifier: 6d4d06e0-3d3f-11ea-a8ac-8912d21a789c

liyasthomas
liyasthomas previously approved these changes Jan 22, 2020
@liyasthomas
Copy link
Member

@AndrewBastin npm run build gives error:

> postwoman-extension@0.2.0 build C:\Liyas\Projects\test\postwoman-chrome
> npx parcel build src/* --out-dir dist/ && npx copy manifest.json dist/ && npx copy icons/* dist/     

√  Built in 1.92s.

dist\index.js.map            66.95 KB     57ms
dist\index.js                21.12 KB    1.54s
dist\contentScript.js         1.26 KB     23ms
dist\contentScript.js.map       470 B     10ms
The syntax of the command is incorrect.
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! postwoman-extension@0.2.0 build: `npx parcel build src/* --out-dir dist/ && npx copy manifest.json dist/ && npx copy icons/* dist/`
npm ERR! Exit status 1
npm ERR!
npm ERR! Failed at the postwoman-extension@0.2.0 build script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.     

npm ERR! A complete log of this run can be found in:
npm ERR!     C:\Users\Brew\AppData\Roaming\npm-cache\_logs\2020-01-23T03_35_09_854Z-debug.

@liyasthomas liyasthomas self-requested a review January 23, 2020 03:37
@AndrewBastin
Copy link
Member Author

@liyasthomas seems like a buildscript issue in Windows, sorry for not catching that, I don't use Windows that often. I will fix that soon.

Can you report if running the following command separately works:

npx parcel build src/* --out-dir dist/
npx copy manifest.json dist/
npx copy icons/* dist/

The first command runs the parcel bundler to generate the code in the dist folder.
The second command copies the manifest.json file to the dist folder
The third command copies all the icons to the dist/icons folder

So, if you want to test, you could also copy the content manually for now.

@TravisBuddy
Copy link

Hey @AndrewBastin,
Your changes look good to me!

View build log

TravisBuddy Request Identifier: 59945950-3d94-11ea-8d60-2f6c30a51d00

@liyasthomas
Copy link
Member

@AndrewBastin

These does'nt work 👎 :
npx copy manifest.json dist/
npx copy icons/* dist/

These woked 👍 :
npx copy manifest.json dist
npx copy icons dist

@AndrewBastin
Copy link
Member Author

Ohkay, I will update it on the buildscript.

Denks.

Opening up Windows sucks 🤣

@AndrewBastin
Copy link
Member Author

Okay, so I have updated buildscript in the postwoman-chrome repo.

Should fix all the issues in Windows now.

@liyasthomas
Copy link
Member

Works like charm 🎉
GET request in ~3-5s 🔥

@liyasthomas
Copy link
Member

@AndrewBastin ready to be merged?

@AndrewBastin
Copy link
Member Author

@liyasthomas yup.

@TravisBuddy
Copy link

Hey @AndrewBastin,
Your changes look good to me!

View build log

TravisBuddy Request Identifier: 5dcb2760-3da3-11ea-8d60-2f6c30a51d00

@liyasthomas liyasthomas merged commit 5119505 into hoppscotch:master Jan 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Changes regarding core concepts feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants