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

Downloading update from private GitHub repository — mac support #1370

Closed
derek-duncan opened this issue Mar 14, 2017 · 48 comments · May be fixed by qcif/data-curator#563
Closed

Downloading update from private GitHub repository — mac support #1370

derek-duncan opened this issue Mar 14, 2017 · 48 comments · May be fixed by qcif/data-curator#563

Comments

@derek-duncan
Copy link

derek-duncan commented Mar 14, 2017

electron-updater

  • Version: 1.9.0
  • Target: dmg

I've been debugging Update download failed. The server sent an invalid response. Try again later. after electron-updater attempts to download from a private GitHub repo. I wanted to confirm the authentication token wasn't the problem, so I replicated the request in Postman. Here's the GET request I'm using:

Url: "https://api.github.com/repos/__OWNER__/__REPO__/releases/assets/__ASSET_ID__"
Headers: {
  Accept: "application/octet-stream"
  Authorization: __TOKEN__
}

This request fails, because Github requires query-token authentication over header authentication for octet stream requests. See the error returned:

<?xml version="1.0" encoding="UTF-8"?>
<Error>
    <Code>InvalidArgument</Code>
    <Message>Only one auth mechanism allowed; only the X-Amz-Algorithm query parameter, Signature query string parameter or the Authorization header should be specified</Message>
    <ArgumentName>Authorization</ArgumentName>
    <ArgumentValue>token __TOKEN__</ArgumentValue>
    <RequestId>88A9E737F583B6C9</RequestId>
    <HostId>TqzUjnu1qvKvN8WfyGNt7d0sNXkhF58kH3KsKZu1WCqZALaP4UADqQXl42v6+8NLbkDVueuYLhI=</HostId>
</Error>

Anyways, the solution to my postman error was changing the authentication to a query string:

Url: "https://api.github.com/repos/__OWNER__/__REPO__/releases/assets/__ASSET_ID__?access_token=__TOKEN__"
Headers: {
  Accept: "application/octet-stream"
}

The file from the private GitHub repo is then successfully downloaded.

Since this request in Postman worked, I manually made the change in the npm module to see if it fixed the Update download failed. The server sent an invalid response. Try again later. error. However, the same error message is still returned, and the download fails. I'm stuck for now and need some input on how to proceed. 🤔

Thank you 👍

(btw: I am using placeholder __VARS__ throughout my example code. Their actual values do appear in the error messages)

@derek-duncan derek-duncan changed the title Downloading update from private GitHub repository fails. Using wrong authentication. Downloading update from private GitHub repository fails. Using wrong authentication format. Mar 14, 2017
@derek-duncan
Copy link
Author

I'll also note that trying to use both authentication types will throw the same error. So anytime the header Authorization token __TOKEN__ is present, the request fails.

<?xml version="1.0" encoding="UTF-8"?>
<Error>
    <Code>InvalidArgument</Code>
    <Message>Only one auth mechanism allowed; only the X-Amz-Algorithm query parameter, Signature query string parameter or the Authorization header should be specified</Message>
    <ArgumentName>Authorization</ArgumentName>
    <ArgumentValue>token __TOKEN__</ArgumentValue>
    <RequestId>88A9E737F583B6C9</RequestId>
    <HostId>TqzUjnu1qvKvN8WfyGNt7d0sNXkhF58kH3KsKZu1WCqZALaP4UADqQXl42v6+8NLbkDVueuYLhI=</HostId>
</Error>

@develar
Copy link
Member

develar commented Mar 15, 2017

I cannot believe. Auth details in the URL it is bad, should be in the headers.

@develar
Copy link
Member

develar commented Mar 15, 2017

Fixed in electron-updater 1.1.0

@derek-duncan
Copy link
Author

@develar thanks for the quick response and update.

After updating to 1.1.0, I'm experiencing the Only one auth mechanism error when checking for an update. I tried to debug it from your pull request, but I'm in need of some help. Any ideas? I've attached the error stack trace as well.

[2017-03-15 09:19:07:0815] [info] Checking for update
[2017-03-15 09:19:10:0835] [error] Error: HttpError: 400 Bad Request
"<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n<Error><Code>InvalidArgument</Code><Message>Only one auth mechanism allowed; only the X-Amz-Algorithm query parameter, Signature query string parameter or the Authorization header should be specified</Message><ArgumentName>Authorization</ArgumentName><ArgumentValue>token __TOKEN__</ArgumentValue><RequestId>F901A556D1580C64</RequestId><HostId>2bf0oFVZQLUuezWkdyJBpbVgBFW0XiY91AHaj9Ne3iYhcCM3JrTV37GnfEcnmv/E</HostId></Error>"
Headers: {
  "connection": [
    "close"
  ],
  "content-type": [
    "application/xml"
  ],
  "date": [
    "Wed, 15 Mar 2017 14:19:10 GMT"
  ],
  "server": [
    "AmazonS3"
  ],
  "transfer-encoding": [
    "chunked"
  ],
  "x-amz-id-2": [
    "2bf0oFVZQLUuezWkdyJBpbVgBFW0XiY91AHaj9Ne3iYhcCM3JrTV37GnfEcnmv/E"
  ],
  "x-amz-request-id": [
    "F901A556D1580C64"
  ]
}
    at IncomingMessage.t.on (/Applications/__APP__.app/Contents/Resources/app.asar/main.js:4:498)
    at emitNone (events.js:86:13)
    at IncomingMessage.emit (events.js:185:7)
    at endReadableNT (_stream_readable.js:974:12)
    at _combinedTickCallback (internal/process/next_tick.js:74:11)
    at process._tickCallback (internal/process/next_tick.js:98:9)
From previous event:
    at i.p [as _captureStackTrace] (/Applications/__APP__.app/Contents/Resources/app.asar/main.js:4:165386)
    at new u (/Applications/__APP__.app/Contents/Resources/app.asar/main.js:4:181030)
    at /Applications/__APP__.app/Contents/Resources/app.asar/main.js:4:183979
    at u._checkForUpdates (/Applications/__APP__.app/Contents/Resources/app.asar/main.js:4:59746)
    at u.checkForUpdates (/Applications/__APP__.app/Contents/Resources/app.asar/main.js:4:59317)
    at WebContents.<anonymous> (/Applications/__APP__.app/Contents/Resources/app.asar/main.js:4:106876)
    at emitOne (events.js:96:13)
    at WebContents.emit (events.js:188:7)

Again, the manual request through Postman worked correctly, so an invalid token is not the issue. It also looks like the update-available event is never emitted, so the code never gets passed checking-for-update event.

@develar
Copy link
Member

develar commented Mar 15, 2017

@derek-duncan Please try 1.10.1 (sorry, first release was not good).

@derek-duncan
Copy link
Author

@develar Ah! Okay I've installed 1.10.1, however the same error above is occurring. Any ideas?

@develar develar reopened this Mar 15, 2017
@vinogradov-m
Copy link

@derek-duncan @develar I believe it happens because "net" module of electron (not HttpExecutor from 'electron-builder-http' package) is used to make api requests. So "Authorization" header isn't omitted for redirected to s3 request.

@develar
Copy link
Member

develar commented Mar 16, 2017

@vinogradov-m No, HttpExecutor is used, but for some reasons method removeAuthHeader https://github.com/electron-userland/electron-builder/blob/master/packages/electron-builder-http/src/httpExecutor.ts#L311 fails to remove it. If you can help (currently, I don't have time and will investigate in 2 days), please help.

BTW, test passed.

@vinogradov-m
Copy link

@develar I'm sorry, maybe I misunderstood you. I just meant that electron-updater uses ElectronHttpExecutor (which extends HttpExecutor) where method doApiRequest() is overridden, and net module of electron is used to download update packages from github via api requests: https://github.com/electron-userland/electron-builder/blob/master/packages/electron-updater/src/electronHttpExecutor.ts#L40

Anyway, I'm investigating this issue and will do PR if I find an acceptable solution.

@develar
Copy link
Member

develar commented Mar 16, 2017

@vinogradov-m Thanks a lot! Bingo — our tests uses node http executor, but ElectronHttpExecutor, that is used by production code, simply delegates to net.

@develar
Copy link
Member

develar commented Mar 16, 2017

Our position still remains — we MUST NOT pass token as query param to be sure that it is not logged.

@develar
Copy link
Member

develar commented Mar 16, 2017

@AlienHoboken As @vinogradov-m commented, ElectronHttpExecutor uses net from electron directly and removeAuthHeader is not used in the production :( and it leads to this error.

@AlienHoboken
Copy link
Contributor

AlienHoboken commented Mar 16, 2017

@develar good catch on the token as query param, forgot to move that to header so that's my fault! I am looking into this issue also.

I came to the same conclusion as @vinogradov-m. It looks like there is no way to stop electron from following redirects (see electron/electron#8868). So it will follow the redirect and apparently pass on the request headers, which is lame.

So our options currently look to be:

  1. Use a different request library to facilitate requests from ElectronHttpExecutor
  2. Resolve the issue in electron core
  3. Disable PrivateGitHubProvider until/if electron core resolves redirect issue.

Even if electron core does resolve this, then the newest version of electron will become a dependency to use private github autoupdater.

I'd suggest implementing https://github.com/request/request instead of electron.net. It is pretty battle hardened and has a similar usage.

How do you think we should proceed?

@develar
Copy link
Member

develar commented Mar 16, 2017

How do you think we should proceed?

I think for now we should use NodeHttpExecutor in the PrivateGitHubProvider. Yes, ElectronHttpExecutor (that uses electron net) has better proxy support. But it is a tradeoff.

e.g. instead of request, we should use httpExecutor.request in the PrivateGitHubProvider. Where httpExecutoris import { httpExecutor } from "electron-builder-util/out/nodeHttpExecutor"

but problem is that electron-updater must not depends on electron-builder-util. And nodeHttpExecutor depends on ini (I want to reduce number of deps for electron-updater)...

Yeach :(

@develar
Copy link
Member

develar commented Mar 16, 2017

Well, it seems there is a way to modify headers during redirect — https://github.com/electron/electron/blob/master/docs/api/web-request.md#webrequestonbeforesendheadersfilter-listener

@AlienHoboken
Copy link
Contributor

Good find! I think that might be serviceable. Let me test it out and if it works I will submit a PR.

@vinogradov-m
Copy link

vinogradov-m commented Mar 16, 2017

@develar @AlienHoboken Wouldn't it be better just to add "Authorization" header only to requests to api.github.com if auth token is defined instead of modifying headers during a redirect?
Something like this: vinogradov-m@99c2edd#diff-684ec0d65067e3c6a1376172edaf1ca3R50

I've just checked, it seems to be working.
Unfortunately, I faced with unavailability to validate UpdateInfo when I call method getLatestVersion() - the response isn't parsed and it is still a json-string, not an object. So I'll double check if I broke something with this code or not.

@develar
Copy link
Member

develar commented Mar 16, 2017

@vinogradov-m Looks correct and cool. Except that

details.requestHeaders['Authorization'] = token ${this.token};

Here we should not add Authorization if already presented.

So I'll double check if I broke something with this code or not.

The same difference between Electron and Node HttpExecutor.

Please

import { safeLoad } from "js-yaml"

if (typeof result === "string") {
        if (getCurrentPlatform() === "darwin") {
          result = JSON.parse(result)
        }
        else {
          result = safeLoad(result)
        }
      }

@AlienHoboken
Copy link
Contributor

Why the addition of the interface in this 2nd version vinogradov-m@3b0668d?

And saving original filename looks ok, there is also this check which was originally used (and develar mentioned): ce1df10#diff-684ec0d65067e3c6a1376172edaf1ca3R44 But saving filename looks cleaner

@vinogradov-m
Copy link

vinogradov-m commented Mar 16, 2017

@AlienHoboken Unfortunately, I've just tested this solution on Mac and it doesn't work. As I understood, we can't add headers to requests of native updater (electron.autoUpdater) via webRequest of the default session.

@AlienHoboken
Copy link
Contributor

I will look at it when I get some time today, maybe this evening. I will see if I can get it working.

Just to understand, @vinogradov-m your solution worked on Windows but not on Mac?

@develar
Copy link
Member

develar commented Mar 16, 2017

On macOS link to file is hardcoded in the update file, because Squirrel.Mac doesn't support relative links. You cannot change link in runtime.

@develar
Copy link
Member

develar commented Mar 16, 2017

@AlienHoboken have you tested your solution before PR on Mac?

@develar
Copy link
Member

develar commented Mar 16, 2017

Well... no one will stop us to start local web server to act as a proxy for stupid Squirrel.Mac. Or we should finally fix Squirrel.Mac. Guys, I hope you will continue to investigate, I don't have time currently for this task.

@AlienHoboken
Copy link
Contributor

@develar no, I don't have code signing certificate for mac so I could not test. Is there a workaround for this?

@AlienHoboken
Copy link
Contributor

Please try as @develar said.

Electron updater does not interact with publishing, only applying updates already published. If you're still having issues with publishing after trying electron-builder 15.6.1, consider opening an issue and providing errors you're seeing.

@derek-duncan
Copy link
Author

@AlienHoboken have you made any progress on the squirrel.mac blocker? anything I can do to help?

@develar
Copy link
Member

develar commented Mar 20, 2017

Why I don't care about users who want to use private GitHub repo and I don't fix this issue in a short term? Because there are number of existing solutions (yes, server is required) — e.g. https://github.com/kevinphelps/electron-update-server Or you can use https://github.com/GitbookIO/nuts.

You can set several publisher — first (generic) will be used for auto-update, second (GitHub) for publishing.

@develar
Copy link
Member

develar commented Mar 20, 2017

How this issue can be fixed for mac:

  • Use Sparkle under the hood. Result — I cannot believe, so awesome. e.g. additional zip will be not required. Well, I am not going to be a hero. Maybe in the future.
  • Start local web server and acts as a proxy for Squirrel.Mac Possible solution.

@develar develar changed the title Downloading update from private GitHub repository fails. Using wrong authentication format. Downloading update from private GitHub repository — mac support Mar 20, 2017
@develar develar added the bug label Mar 20, 2017
@AlienHoboken
Copy link
Contributor

@derek-duncan I've been pretty busy this weekend, didn't get time to work a lot on it. But I have a working approach (local web proxy as @develar said) that while not an ideal solution long term will get it working.

I'm going to look at Sparkle as well, though. But that's a bigger chore. For the meantime I may just get it working.

@sondremare
Copy link

sondremare commented May 16, 2017

@develar Hi, I am trying to use https://github.com/kevinphelps/electron-update-server for mac autoupdates. But I am struggling.

This is my publish setup:

"publish": [
    {
      "provider": "generic",
      "url": "http://localhost:5000/download/OwnerOfRepo/repo-name"
    }, {
      "provider": "github"
    }
  ]

When the GenericProvider tries to fetch the latest version (getLatestVersion()) it correctly uses http://localhost:5000/download/OwnerOfRepo/repo-name/latest-mac.json. However, this being only a proxy, the latest-mac.json file hosted in GitHub Releases contains the wrong url reference to the zip-file. It contains the direct link to the repo, not the proxied version.
I have created new releases and bumped the version after I added the generic provider, but the file that is uploaded still contains this direct link.

When I build locally, I can see in my dist folder that I have a latest-mac.json with the correct proxied url on root level, there is also another latest-mac.json inside a github-folder.

Any ideas on how I can publish the proxied version instead of the direct version?

EDIT: I am using electron-updater 1.15.0 and electron-publish 17.4.0

@develar
Copy link
Member

develar commented May 16, 2017

Any ideas on how I can publish the proxied version instead of the direct version?

Because Squirrel.Mac doesn't support relative urls. Work on custom proxy (local http server to fix Squirrel.Mac issues) is started. You can expect solution in one week.

@sondremare
Copy link

Ok, thank you for the quick response :)

@develar
Copy link
Member

develar commented Jun 5, 2017

Fixed in electron-update 2.0.0. Will be announced in the #1167.

@amok
Copy link

amok commented Dec 15, 2017

experiencing exactly the same issue

it seems like removeAuthHeader code has been moved around a lot during last versions and things got broken

@ghost
Copy link

ghost commented Dec 18, 2017

Same here,
Finally got Code Signing to work and now this.

@KrzysZG37
Copy link

@hgbloch, how did you solve the problem?

@ghost
Copy link

ghost commented Dec 19, 2017

@KrzysZG37 sorry I wasn't clear.
I didn't solve the problem, it still occurs.
I've just moved to use S3

@KrzysZG37
Copy link

@hgbloch unfortunately, i still got the problem even on S3, which i am currently using (Code signature at URL ... did not pass validation: code object is not signed at all)

@amok
Copy link

amok commented Dec 19, 2017

@hgbloch
I switched to s3 too
believe it's a bug and the issue should be reopened

@ghost
Copy link

ghost commented Dec 19, 2017

@amok it is a bug,
After looking at recent changes, about 13 days ago they moved the auth headers removal from PrivateGithubProvider to electronHttpExecuter, or at least it seems like that.

@KrzysZG37 I will be more than happy to help, I think you should open a new issue

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

Successfully merging a pull request may close this issue.

8 participants