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

CORS issue when sending email on enterprise extension #2934

Closed
5 tasks
tomholub opened this issue Sep 8, 2020 · 4 comments · Fixed by #2935
Closed
5 tasks

CORS issue when sending email on enterprise extension #2934

tomholub opened this issue Sep 8, 2020 · 4 comments · Fixed by #2935
Assignees
Labels

Comments

@tomholub
Copy link
Collaborator

tomholub commented Sep 8, 2020

image

  • there are two differences between enterprise extension release and regular: 1) the enterprise release has a more conservative release schedule 2) enterprise has a more restricted set of permissions in the extension manifest
  • some time ago, browser behavior has changed on Chrome, to enforce CORS for requests initiated from extension frames (compose window) inside other frames (Gmail). That time we only updated the consumer version, by mistake.
  • the CORS does not prevent the request from reaching Google, and therefore the actual messages are in fact sent. However the browser blocks the response of the request, causing the rendered error.
  • our automated tests do equal testing of both standard and enterprise release (all tests run on both versions). However the compose tests are run in a separate tab, not directly in the Gmail tab.
  • the CORS issue only affects frames inside the Gmail tab, so automated tests didn't uncover this
  • so far enterprise customers preferred using the standard version over the enterprise version (against my strong recommendation to use the enterprise one) to get features faster. This meant this issue in the enterprise version went undiscovered for longer than it should.

The fix was tested today locally by changing the manifest file of the extension to include "https://www.googleapis.com/*" as one of the permissions.

Today the following steps will be taken to fully reconcile this:

  • merge the fix
  • automated tests will be updated to include an email-sending test that runs directly on Gmail (we call these "live tests", and already have similar ones due to previous experience), on both regular and enterprise versions
  • code will be audited for possible opportunities for similar errors on other APIs when accessed on the enterprise version from inside the Gmail page
  • we'll switch to run the enterprise version during the regular feature development process (currently developers tend to spin up the regular one)
  • once automated tests and the fix are merged, and the code audited, we'll release a new version 7.9.1 later today
@tomholub tomholub added this to the 7.9.1: Bugfixes milestone Sep 8, 2020
@tomholub tomholub self-assigned this Sep 8, 2020
@tomholub tomholub added the bug label Sep 8, 2020
@tomholub tomholub changed the title CORS issue when sending email CORS issue when sending email on enterprise extension Sep 8, 2020
@tomholub
Copy link
Collaborator Author

tomholub commented Sep 8, 2020

This issue may be nastier to test than I thought.

Because puppeteer does not actually allow us to access out-of-process iframes, such as iframes inside Gmail page. The way we were testing such functionality until now was, to find the iframe, get the URL, then open that in a separate tab.

Which does not actually test this particular issue, the frame has to be inside Gmail.

The issue is here puppeteer/puppeteer#2548

My best attempt may be to have the mouse click blindly in the space where the send button is supposed to be. Then observe if the window goes away.. which would not work either because I don't have a way to fill out that compose window before clicking.

So for the typing, maybe we could convince the content script to relay the keypresses (and only enable this for testing..) but that becomes a real mess.

Other possibility would be to just try selenium, and see if this test case can run in selenium instead.

@tomholub
Copy link
Collaborator Author

tomholub commented Sep 8, 2020

There could also be some test crafted specifically for this, that doesn't really use the secure compose itself, but some sort of an automated interface to just test the cors issue. Which I find may be a too pointed approach - potentially not addressing other similar issues that may happen.

@tomholub
Copy link
Collaborator Author

tomholub commented Sep 8, 2020

Other possibility would be to just try selenium, and see if this test case can run in selenium instead.

did not work

@tomholub
Copy link
Collaborator Author

tomholub commented Sep 8, 2020

All of these approaches are near impossible (short of forking chromium), or do not actually test the intended issue.

The mechanism to create manifests.json file is dynamic, which caused this issue to be not obvious initially.

Therefore we will instead test that the appropriate necessary permissions are present in the resulting manifests.

tomholub pushed a commit that referenced this issue Sep 8, 2020
* issue 2934 fix enterprise CORS issue

* fix 0auth login button

* refactored build process

* add manifest tests

* fix 0auth login

Co-authored-by: Tom J <tom@holub.me>
@limonte limonte mentioned this issue Nov 7, 2021
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant