electron-userland / electron-builder

A complete solution to package and build a ready for distribution Electron app with “auto update” support out of the box
https://www.electron.build
MIT License
13.7k stars 1.74k forks source link

electron-builder updater not using authorization (or any headers) in 'generic' provider #6192

Closed jbool24 closed 3 years ago

jbool24 commented 3 years ago

Issues:

  1. Using a 'generic' provider configuration to download update does not include the request headers. The request fails because the headers are ignored and not passed along to the request.
  2. If you try letting autoUpdate configure itself based on the provider value in electron-builder configs, it does not product the app-update.yml file. Therefor no update checks.
  3. Using a 'custom' provider for publisher requires 'custom' provider in updater which can be passed in AllPublishOptions however there is no documentation on what to subclass for creating a custom updater class.

The 'custom' publisher also completes without reporting errors but does not upload the file. The Basic Auth credentials were tested with curl and worked correctly. But the files do not upload even though electron-publish reports it did.

Both the custom Publisher and the Updater setup are below...

Here are the configs for setting up the Updater in the main process

import type { AllPublishOptions } from 'builder-util-runtime'
import type { AppUpdater } from 'electron-updater'
import { AppImageUpdater, MacUpdater, NsisUpdater } from 'electron-updater'

let autoUpdater: AppUpdater
const options = {
  provider: 'generic',
  requestHeaders: {
    Authorization: 'Basic XXXXXXXXXXXXXXXXXXXXXXXXX,
  },
  url: 'https://api.bitbucket.org/2.0/repositories/{{REPO_OWNER}}/{{REPO_SLUG}}/downloads',
} as AllPublishOptions

if (process.platform === 'win32') {
  autoUpdater = new NsisUpdater(options)
}
else if (process.platform === 'darwin') {
  autoUpdater = new MacUpdater(options)
}
else {
  autoUpdater = new AppImageUpdater(options)
}

Here is the publisher for build time

const { basename, join } = require('path');

const { Arch } = require('builder-util');
const { HttpPublisher } = require('electron-publish/out/publisher');
const { httpExecutor } = require('builder-util/out/nodeHttpExecutor');
const { configureRequestOptions, configureRequestOptionsFromUrl } = require('builder-util-runtime');
const { createReadStream, stat } = require('fs-extra');
const mime = require('mime');
const { Console } = require('console');

class CustomPublisher extends HttpPublisher {
  constructor(context, info) {
    super(context, false);
    console.log(info);
    this.options = info;
  }

  get providerName() {
    return this.options.publisher || 'Custom Publisher';
  }

  // Create string for cli output publisher type build step
  toString() {
    const version = this.context.packager.appInfo.buildVersion;
    return `Custom (publish: ${new URL(this.options.url).host}, version: ${version})`;
  }

  // Override base class upload function
  async upload(task) {
    const fileName =
      (this.useSafeArtifactName ? task.safeArtifactName : null) ||
      basename(task.file);

    if (task.fileContent != null) {
      console.log('upload with content')
      await this.doUpload(
        fileName,
        task.arch || Arch.x64,
        task.fileContent.length,
        (it) => it.end(task.fileContent),
      );
      return;
    }

    const fileStat = await stat(task.file);

    const progressBar = this.createProgressBar(fileName, fileStat.size);
    console.log('upload from filepath')
    await this.doUpload(
      fileName,
      task.arch || Arch.x64,
      fileStat.size,
      (request, reject) => {
        if (progressBar != null) {
          // reset (because can be called several times (several attempts)
          progressBar.update(0);
        }
        return this.createReadStreamAndProgressBar(task.file, fileStat, progressBar, reject).pipe(request);
      },
    );
  }

  async doUpload(fileName, arch, dataLength, requestProcessor) {
    const headers = this.options.headers || {};

    const opts = configureRequestOptionsFromUrl(
      this.options.url,
      {
        headers: {
          ...headers,
          'X-File-Name': fileName,
          'Content-Type': mime.getType(fileName) || 'application/octet-stream',
          'Content-Length': dataLength,
        },
      },
    );

    return await httpExecutor.doApiRequest(opts, this.context.cancellationToken, requestProcessor);
  }
}

module.exports = {
  default: CustomPublisher,
};

**NOTE: I would be willing to sponsor updating of docs for more clear and correct instructions on custom extending Publisher and Updater if in fact this is not an issue and the custom features are actually working and this is my development error for incorrectly implementing.

mmaietta commented 3 years ago

Tbh, I still don't understand the "custom" provider

That being said, I actually just wrote an official Publisher and Provider for Keygen https://github.com/electron-userland/electron-builder/pull/6167 As part of it, I added a new way to authenticate the updater https://github.com/electron-userland/electron-builder/blob/a755c822e1ec977af529fe589be72d0657f0b9ab/packages/electron-updater/src/AppUpdater.ts#L97-L101

For creating an updater, you only need to use prop autoUpdater, it'll handle the OS logic for you https://github.com/electron-userland/electron-builder/blob/dc359de5019807a014c62468385dfb14bbb5bd83/packages/electron-updater/src/main.ts#L18-L30

I'm not sure why your provider headers are not working though unless there's an authorization header being provided elsewhere which I guess is very unlikely https://github.com/electron-userland/electron-builder/blob/dc359de5019807a014c62468385dfb14bbb5bd83/packages/electron-updater/src/providers/Provider.ts#L69-L75 I see Bintray provider has header logic though, maybe try copying that to your provider and setting the auth header through there? https://github.com/electron-userland/electron-builder/blob/dc359de5019807a014c62468385dfb14bbb5bd83/packages/electron-updater/src/providers/BintrayProvider.ts#L19-L22

Hope that helps! (and that I didn't misunderstand any parts)

jbool24 commented 3 years ago

@mmaietta Thanks for this. I'm just digging into the src code because I need to implement publish/auto-update with Atlassian products (so bitbucket) and private repos. And I'm having a hard time following the trail of what core code calls and where to hook in custom code. Also, We don't want to monkey-patch or fork this repo to add custom code to packages. Is that what you've done? I'm asking because of the file path you listed in under electron-builder. Or is there a way to drop updater code somewhere that core code will know to grab like the Publisher does with electron-publisher-{whatever}.js files in buildResources dir.

jbool24 commented 3 years ago

@develar is there someone from the core team that can be commissioned :moneybag: to create/update documentation on how to create both custom Publisher and Updater/Provider? The end goal specifically is to implement Bitbucket.org private repo integration for a private internal company project.

mmaietta commented 3 years ago

Also, We don't want to monkey-patch or fork this repo to add custom code to packages. Is that what you've done? I'm asking because of the file path you listed in under electron-builder

@jbool24 I'm not sure I follow. The is a monorepo, so each module resides under the path ./electron-builder/packages/*.

Would you be interested in contributing to this project for electron-builder to officially support bitbucket? I'd be more than happy to assist in getting it integrated. It'd be cool to have a BitbucketPublisher and BitbucketProvider

jbool24 commented 3 years ago

@mmaietta if we were to implement custom code we didn't want to have to keep a local package to inject into 'electron-updater' and we def did not want to modify the package after download (monkey-patch). Since we will need to do it either way I wouldn't mind making it a Bitbucket provider. I however I'd need a good alley oop since that part of the code is hard for me to follow. I looked at your Keygen a bit but still confused at which functions are hooked by the core and which are your internals. Also the current Publisher above is not actually publishing to the repo but doesn' throw errors in code or network response side. So that still needs work also.

mmaietta commented 3 years ago

Happy to help!

For the publisher, only protected doUpload(...) is required when extending HttpPublisher

For the provider only these are needed

  abstract getLatestVersion(): Promise<T>
  abstract resolveFiles(updateInfo: T): Array<ResolvedUpdateFileInfo>
  toString(): string

For integration testing, here are two skeletons you could easily reuse https://github.com/electron-userland/electron-builder/blob/0880d1bada5f626f0baf1a988e8f37e3647f398e/test/src/updater/nsisUpdaterTest.ts#L49-L58 https://github.com/electron-userland/electron-builder/blob/0880d1bada5f626f0baf1a988e8f37e3647f398e/test/src/ArtifactPublisherTest.ts#L128-L142

Notes:

jbool24 commented 3 years ago

@mmaietta OK, I'll start to dive in and go from there. For my publisher it looks as though upload is the called function at runtime by core code. Are you sure its doUpload to implement/extend? I had to call doUpload from my upload function to get any sort of messages or errors during builder runtime. However, right now that publisher no longer gives back 403 errors from missing headers because it properly sets Authorization token but it doesn't actually DO the call (I think) because it silently passes with a Done message at the end with no files in my Bitbucket.

@develar Can you provide an background or any suggestions/ideas??

mmaietta commented 3 years ago

If you extend HttpPublisher for simplicity, then yes, doUpload would be the correct route for a custom publisher (similarly to Bintray, Github, and Keygen). They all have custom uploads to be executed for each file (keygen for instance requires a PUT to return a unique artifact URL, then a secondary upload of the artifact to that generated URL for each individual artifact) https://github.com/electron-userland/electron-builder/blob/0880d1bada5f626f0baf1a988e8f37e3647f398e/packages/electron-publish/src/publisher.ts#L102-L108

Ref: https://github.com/electron-userland/electron-builder/search?q=doUpload

jbool24 commented 3 years ago

@mmaietta Ok. I found that spot. I will certainly need to override upload because the Bitbucket REST api requires FormData content-type for uploads. So a FormData stream will need to be piped in to the request writable. I have something quickly added but the api keeps responding that the field value of files is missing.

{
  "type": "error",
  "error": {
    "fields": {
      "files": [
        "This field is required."
      ]
    },
    "message": "files: This field is required."
  }
}

Here is a snippet of that code. I'm wondering, has the request already started the stream with data elsewhere before I start piping so that the request will be malformed? Currently I've only been testing the latest.yml file which shows up as a Buffer on task.fileContent and immediately returning to skip the packaged artifact upload.

const FormData = require('form-data')
//...
class CustomPublisher extends HttpPublisher {
//...

  // Override base class function
  async upload(task) {
    const fileName = (this.useSafeArtifactName ? task.safeArtifactName : null) || basename(task.file);

    if (task.fileContent != null) {
      console.log('upload with content')

      const form = new FormData();
      form.append('files', task.fileContent, { fileName })

      this.options.headers = { ...this.options.headers, ...form.getHeaders() }

      await this.doUpload(
        fileName,
        task.arch || Arch.x64,
        task.fileContent.length,
        (request, reject) => {
          form.on('end', () => console.log('ALL FORM DATA READ'))
          form.pipe(request);
        },
      );
      return;
    }
//...
}
jbool24 commented 3 years ago

After further inspection I found that form.pipe(request) is piping the data stream in correctly but something weird is going on in the request. Using nc util on linux to view the payload for curl (which correctly uploads the test file) I can match the output of the formdata stream and they are mostly identical with the variation being the boundry. example outputs:

With NetCat listening on port 8888 of local host

nc -l localhost 8888

This is output from curl to netcat. Importantly observe FormData output between boundry

curl -X POST localhost:8888 -v -H "Authorization: Basic ***REDACTED***" -v  -F files=@dist/latest-linux.yml 
POST / HTTP/1.1
Host: localhost:8888
User-Agent: curl/7.68.0
Accept: */*
Authorization: Basic ***REDACTED***
Content-Length: 620
Content-Type: multipart/form-data; boundary=------------------------39f4731450e9571b

--------------------------39f4731450e9571b
Content-Disposition: form-data; name="files"; filename="latest-linux.yml"
Content-Type: application/octet-stream

version: 21.9.1-1630512963907
files:
  - url: TESTAPP-21.9.1-1630512963907.AppImage
    sha512: 01ReKGhqGcoF4iQRGZvq8s60qCSHuFCGQqMYBAojMH0uhqUIzEON5Y/cTtEsLxpA45/olIQ71QiJKtJJzHYR6w==
    size: 84799057
    blockMapSize: 89701
path: TESTAPP-21.9.1-1630512963907.AppImage
sha512: 01ReKGhqGcoF4iQRGZvq8s60qCSHuFCGQqMYBAojMH0uhqUIzEON5Y/cTtEsLxpA45/olIQ71QiJKtJJzHYR6w==
releaseDate: '2021-09-01T16:16:11.019Z'

--------------------------39f4731450e9571b--

Now here is spying on the stream with a passthrough Transform stream to print the contents of FormData

async upload(task) {
    const fileName = (this.useSafeArtifactName ? task.safeArtifactName : null) ||  basename(task.file);

    if (task.fileContent != null) {
      console.log('upload with content')

      const form = new FormData();
      form.append(
        'files',
        task.fileContent,
        {
          filename: fileName,
          header: `\r\n--${form.getBoundary()}\r\n` +
          `Content-Disposition: form-data; name="files"; filename="${fileName}"\r\n` +
          `Content-Type: ${mime.getType(fileName)}\r\n`,
        },
      );

      this.options.headers = { ...this.options.headers, ...form.getHeaders() };

      // for debugging the stream
      const passthrough = new Transform({
        transform: (chunk, encoding, cb) => {
          console.log(chunk.toString())
          cb(null, chunk)
        }
      })

      await this.doUpload(
        fileName,
        task.arch || Arch.x64,
        task.fileContent.length,
        (request, reject) => {
          request.on('end', () => console.log('ALL DONE'))
          request.on('pipe', (src) => console.log('Pipe from', src.constructor.name))
          request.on('error', (e) => {console.error(e); reject(e)})
          passthrough.on('end', () => { console.log('FORM SENT'); request.end(); });
          form.pipe(passthrough).pipe(request, {end: false}); // manually calling end for request stream writable
          // form.pipe(request);
        },
      );
      return;
    }

and here is the output

upload with content
{                                                                                              
  hostname: 'api.bitbucket.org',                                                               
  path: '{{REDACTED PRIVATE REPO PATH}}',                                                                                                                       
  protocol: 'https:',                                                                          
  method: 'POST',                                                                              
  headers: {                                                                                   
    Host: 'api.bitbucket.org',                                                                 
    Accept: '*/*',                                                                             
    'Content-Length': 411,                                                                     
    'content-type': 'multipart/form-data; boundary=--------------------------581743681975947702003689',                                                                                       
    authorization: 'Basic {{**REDACTED**}}',                           
    'User-Agent': 'electron-builder',                                                          
    'Cache-Control': 'no-cache'                                                                
  }                                                                                            
}                                                                                              

----------------------------581743681975947702003689                                           
Content-Disposition: form-data; name="files"; filename="latest-linux.yml"                                                                                                                     
Content-Type: text/yaml                                                                        

version: 21.9.1-1630515079265                                                                  
files:                                                                                         
  - url: TESTAPP-21.9.1-1630515079265.AppImage                                                
    sha512: vs5TFNf3zt+CzQAxPt+NimNDUKHHafcXmgkYQ7IyrhtYcK5r9x8rNyNqjLUALkHV1zhH9AQH3uYwteC1SpsSnQ==                                                                                          
    size: 84799057                                                                             
    blockMapSize: 89701                                                                        
path: TESTAPP-21.9.1-1630515079265.AppImage                                                   
sha512: vs5TFNf3zt+CzQAxPt+NimNDUKHHafcXmgkYQ7IyrhtYcK5r9x8rNyNqjLUALkHV1zhH9AQH3uYwteC1SpsSnQ==                                                                                              
releaseDate: '2021-09-01T16:51:26.899Z'                                                        

----------------------------581743681975947702003689--                                         

Pipe from Transform                                                                            
FORM SENT                                                                                      
Status Code: 400                                                                               
Message: 400 Bad Request                                                                       
{                                                                                              
  "type": "error",                                                                             
  "error": {                                                                                   
    "fields": {                                                                                
      "files": [                                                                               
        "This field is required."                                                              
      ]                                                                                        
    },                                                                                         
    "message": "files: This field is required."                                                
  }                                                                                            
}                                                                                              
Headers: {                                                                                     
  "server": "nginx",                                                                           
  "vary": "Authorization, Origin",                                                             
  "cache-control": "no-cache, no-store, must-revalidate, max-age=0",                                                                                                                          
  "content-type": "application/json; charset=utf-8",                                           
  "x-b3-traceid": "66c1242c919e4503",                                                          
  "x-oauth-scopes": "repository:write",                                                        
  "x-usage-output-ops": "0",                                                                   
  "x-dc-location": "Micros",                                                                   
  "strict-transport-security": "max-age=31536000; includeSubDomains; preload",                                                                                                                
  "date": "Wed, 01 Sep 2021 16:51:27 GMT",                                                     
  "x-usage-user-time": "0.120622",                                                             
  "x-usage-system-time": "0.001988",                                                           
  "expires": "Wed, 01 Sep 2021 16:51:27 GMT",                                                  
  "x-served-by": "ab58f15e475f",                                                               
  "x-view-name": "bitbucket.apps.downloads.api.v20.handlers.DownloadsHandler",                                                                                                                
  "x-static-version": "12fbe411e51b",                                                          
  "x-credential-type": "apppassword",                                                          
  "x-render-time": "0.0714159011841",                                                          
  "x-accepted-oauth-scopes": "repository:write",                                               
  "connection": "close",                                                                       
  "x-usage-input-ops": "0",                                                                    
  "x-request-count": "3833",                                                                   
  "x-frame-options": "SAMEORIGIN",                                                             
  "x-version": "12fbe411e51b",                                                                 
  "content-length": "123"                                                                      
}   

If I try to send the request to netcat by changing the url in electron-configs to http://localhost:8888 I get errors about the self signed certificate. This should not happen for http requests so somewhere the ClientRequest object is not being setup correctly.

{                                    
  hostname: 'localhost',                                                                                                                                      
  path: '/',                                                        
  protocol: 'http:',                 
  method: 'POST',                                                              
  headers: {                                        
    Host: 'localhost:8888',                                                                                                             
    Accept: '*/*',                                                  
    'Content-Length': 411,                                                     
    'content-type': 'multipart/form-data; boundary=--------------------------375342512532304869982889',                                                       
    authorization: 'Basic ***REDACTED***',
    'User-Agent': 'electron-builder',                               
    'Cache-Control': 'no-cache'                                                                                                         
  }               
}                                              

----------------------------375342512532304869982889                                                                                    
Content-Disposition: form-data; name="files"; filename="latest-linux.yml"                                                                                                                     
Content-Type: text/yaml                                                        

version: 21.9.1-1630525623110                         
files:                                                                                         
  - url: TESTAPP-21.9.1-1630525623110.AppImage                     
    sha512: VD+S1hnQ5KQOQOr5OcugG2Q6oJ5fX4YJB5p/q8U741exMw5m9ifD6A84n6+T5HimEVlNoOt8LNLtoaAJzXuQgA==                                                                                          
    size: 84799039                                                  
    blockMapSize: 89683       
path: TESTAPP-21.9.1-1630525623110.AppImage           
sha512: VD+S1hnQ5KQOQOr5OcugG2Q6oJ5fX4YJB5p/q8U741exMw5m9ifD6A84n6+T5HimEVlNoOt8LNLtoaAJzXuQgA==                                                                                              
releaseDate: '2021-09-01T19:47:10.117Z'          

----------------------------375342512532304869982889--

Pipe from Transform                  
FORM SENT     
Pipe from Transform                                                                                                                     
Error: self signed certificate                                                 
    at TLSSocket.onConnectSecure (_tls_wrap.js:1497:34)
    at TLSSocket.emit (events.js:315:20)                                                                                                                                                                                                                                        
    at TLSSocket._finishInit (_tls_wrap.js:932:8)                                              
    at TLSWrap.ssl.onhandshakedone (_tls_wrap.js:706:12) {                     
  code: 'DEPTH_ZERO_SELF_SIGNED_CERT'                                                                                                                                                                                                                                                                                        
}                                                                                              
Status Code: undefined                                                                         
Message: self signed certificate                                                               
Done in 7.56s.   
mmaietta commented 3 years ago

Do you have a sample branch that I could checkout and can you provide any instructions for me to get set up locally? This is a bit outside of my league, per se, but maybe I can debug it locally and help you out in an easier manner.

jbool24 commented 3 years ago

@mmaietta I haven't forked this project yet. I'm currently only using 'custom' to validate the publisher logic in our own application repo. The custom code just gets dropped into the buildResources directory of the project root as electron-publisher-${someName}.js and the builder picks it up as an override if its custom. I can give you the source of that file. With the publisher key of the electron-builder.config.js object set to custom keys.

// in electron-builder.config.js

//...
publish: [
    {
      provider: "custom",
      publisher: "Bitbucket.org",
      channel: "latest",
      auth: 'Basic ***REDACTED***',
      url: `https://api.bitbucket.org/2.0/repositories/${BITBUCKET_REPO_OWNER}/${BITBUCKET_REPO_SLUG}/downloads`
      // url: 'http://localhost:8888/'
    },
  ],
// ...

You'll need to create an app password token for your personal account in Bitbucket to replace the Basic Auth header

This is where I'm at currently. This code is in a file called electron-publisher-custom.js in ./{{PROJECT_ROOT}}/buildResources

const { basename } = require('path');
const { Transform } = require('stream');

const mime = require('mime');
const FormData = require('form-data');
const { Arch } = require('builder-util');
const { HttpPublisher } = require('electron-publish/out/publisher');
const { httpExecutor } = require('builder-util/out/nodeHttpExecutor');
const { configureRequestOptions } = require('builder-util-runtime');
const { stat } = require('fs-extra');

class CustomPublisher extends HttpPublisher {
  constructor(context, info) {
    super(context, false);
    console.log(info);
    this.options = info;
  }

  get providerName() {
    return this.options.publisher || 'Custom Publisher';
  }

  // Create string for cli output publisher type build step
  toString() {
    const version = this.context.packager.appInfo.buildVersion;
    return `Custom (publish: ${new URL(this.options.url).host}, version: ${version})`;
  }

  // Override base class upload function
  async upload(task) {
    const fileName =
      (this.useSafeArtifactName ? task.safeArtifactName : null) ||
      basename(task.file);

    if (task.fileContent != null) {
      console.log('upload with content');

      const form = new FormData();
      form.append(
        'files',
        task.fileContent,
        {
          filename: fileName,
          contentType: mime.getType(fileName),
          header: `\r\n--${form.getBoundary()}\r\n` +
          `Content-Disposition: form-data; name="files"; filename="${fileName}"\r\n` +
          `Content-Type: ${mime.getType(fileName)}\r\n`,
        },
      );

      this.options.headers = { ...this.options.headers, ...form.getHeaders() };

      console.log(form);

      const passthrough = new Transform({
        transform: (chunk, encoding, cb) => {
          console.log(chunk.toString());
          cb(null, chunk);
        },
      });

      await this.doUpload(
        fileName,
        task.arch || Arch.x64,
        task.fileContent.length,
        (request, reject) => {
          request.on('end', () => console.log('ALL DONE'));
          request.on('pipe', (src) => console.log('Pipe from', src.constructor.name));
          request.on('error', (e) => {console.error(e); reject(e); });

          passthrough.on('end', () => { console.log('FORM SENT'); request.end(); });

          form.pipe(passthrough).pipe(request, {end: false});
          // form.pipe(request);
        },
      );
      return;
    }
    return

    // const fileStat = await stat(task.file);

    // const progressBar = this.createProgressBar(fileName, fileStat.size);
    // console.log('upload from filepath')

    // await this.doUpload(
    //   fileName,
    //   task.arch || Arch.x64,
    //   fileStat.size,
    //   (request, reject) => {
    //     if (progressBar != null) {
    //       // reset (because can be called several times (several attempts)
    //       progressBar.update(0);
    //     }

    //     const form = new FormData();
    //     form.append(
    //       'files',
    //       task.fileContent,
    //       {
    //         filename: fileName,
    //         contentType: mime.getType(fileName),
    //         header: `\r\n--${form.getBoundary()}\r\n` +
    //         `Content-Disposition: form-data; name="files"; filename="${fileName}"\r\n` +
    //         `Content-Type: ${mime.getType(fileName)}\r\n`,
    //       },
    //     );

    //     return this.createReadStreamAndProgressBar(task.file, fileStat, progressBar, reject).pipe(request);
    //   },
    // );
  }

  async doUpload(fileName, arch, dataLength, requestProcessor) {
    const headers = this.options.headers || {};

    const url = new URL(this.options.url);

    const opts = configureRequestOptions(
      {
        hostname: url.hostname,
        path: url.pathname,
        protocol: url.protocol,
        method: 'POST',
        headers: {
          'Host': url.host,
          'Accept': '*/*',
          // 'X-File-Name': fileName,
          // 'Content-Type': mime.getType(fileName) || 'application/octet-stream',
          'Content-Length': dataLength,
          ...headers,
        },
      },
      this.options.auth,
    );
    console.log(opts)
    return await httpExecutor
      .doApiRequest(opts, this.context.cancellationToken, requestProcessor)
      .catch(e => {
        console.error(`Status Code: ${e.statusCode}`);
        console.error(`Message: ${e.message}`);
      });
  }
}

module.exports = {
  default: CustomPublisher,
};
mmaietta commented 3 years ago

From what I've read, don't set the headers, it's automatically created through getHeaders and the form data is prefixed when it's piped. So you can recreate the requestProcessor and just use it => form.pipe(it) AFAICT

[EDIT] I was able to get it publishing + integration test passing. Here's the draft PR https://github.com/electron-userland/electron-builder/pull/6228 Key part, we re-read the file into a Buffer and append that to files.

    return HttpExecutor.retryOnServerError(async () => {
      const fileContent = await readFile(file)
      const form = new FormData()
      form.append("files", fileContent, fileName)
      const upload: RequestOptions = {
        hostname: this.hostname,
        path: this.basePath,
        headers: form.getHeaders(),
      }
      await httpExecutor.doApiRequest(configureRequestOptions(upload, this.auth, "POST"), this.context.cancellationToken, it => form.pipe(it))
      return fileName
    })
mmaietta commented 3 years ago

Added Auto-Updater functionality, BitbucketProvider, with an integration test. Still need to test E2E Authorization for auto-updates is added through AppUpdater::addAuthHeader, I'd imagine ideally your users will be using an oauth2 Bearer token For app passwords, they're converted to a Basic token via BitbucketPublisher.convertAppPassword(...) For publishing, you just need to set process.env.BITBUCKET_TOKEN (app password) though and the class will auto-convert/process it into the correct header.

jbool24 commented 3 years ago

@mmaietta Awesome!! YOU ROCK FOR THIS!! But what did you mean for updates using Bearer token? Consumers of this new Provider in general or our specific use case? We chose to use Bitbucket App tokens over oauth flow because the electron app is distributed internally only but to many workstations. We can control revoke and re-creation an app token for distribution without bothering end-users to authorize updates for electron app. We will just do it for them any time there is a update published. But does that effect community users of this Publisher if they would rather use OAuth and Bearer tokens?

Also, whats the approval workflow for this project? Who reviews the pull requests?

mmaietta commented 3 years ago

Oh, none to worry about then. You're welcome to choose whatever auth mechanism you want. Just need to set the Authorization header via AppUpdater::addAuthHeader with however you choose to proceed.

It's just develar and myself as maintainers in terms of workflow. I always do encourage community contributors and PR reviewers though 🙂

jbool24 commented 3 years ago

@mmaietta Thanks so much for assistance with this! As my workload reduces I can totally stay more on top of this project and potentially chip in where I can now that you have helped orient me to this code.

So what needs to happen to get this merged and published to npm? Anything from me?

mmaietta commented 3 years ago

Merged. Released as part of electron-builder 22.14.0 and electron-updater 4.6.0 @ next

jbool24 commented 3 years ago

Seems like there is a dependency issue on dmg-license package for linux on that release of electron-builder@next. Is that a forced dependency?

[1/5] Validating package.json...
[2/5] Resolving packages...
[3/5] Fetching packages...
info fsevents@2.3.2: The platform "linux" is incompatible with this module.
info "fsevents@2.3.2" is an optional dependency and failed compatibility check. Excluding it from installation.
error dmg-license@1.0.9: The platform "linux" is incompatible with this module.
error Found incompatible module.

electron-upload@next downloaded without issue on linux

mmaietta commented 3 years ago

Should be fixed via https://github.com/electron-userland/electron-builder/pull/6244. Can you try with 22.14.1 now?

xueqingxiao commented 3 years ago

@mmaietta I'm trying with 22.14.1, it's still thrown same error, you can see here

jbool24 commented 3 years ago

@mmaietta confirming on Linux still receiving that same error with 22.14.1

mmaietta commented 3 years ago

I've deprecated 22.14.0 and 22.14.1 to prevent impact on other users. next should still be 22.13.1 now.

Working on fixing build asap. Seems there's some mixup with optional dependencies

mmaietta commented 3 years ago

Please try the next release 22.14.2. It's reverted specific package.json changes back to 22.13.1.

jbool24 commented 3 years ago

@mmaietta Well, the dependencies are good now. No more errors on forced packages. But getting 401 unauthorized on the Bitbucket API POST request. I have BITBUCKET_TOKEN in my env variables with the app token. And I checked with curl that token is still valid. Is a token field supposed to be specified on the config like GithubPublisherOptions?

mmaietta commented 3 years ago

Hmmm are you just using an App Password? That's supposed to be the BITBUCKET_TOKEN, no transforms or "Basic" prefix modifications. I have an integration test specifically for Bitbucket uploads: https://github.com/electron-userland/electron-builder/blob/a9ec90d539fdbb5786692629275b1a89bfd7aec4/test/src/ArtifactPublisherTest.ts#L145-L153

jbool24 commented 3 years ago

Hmmm are you just using an App Password? That's supposed to be the BITBUCKET_TOKEN, no transforms or "Basic" prefix modifications. I have an integration test specifically for Bitbucket uploads:

https://github.com/electron-userland/electron-builder/blob/a9ec90d539fdbb5786692629275b1a89bfd7aec4/test/src/ArtifactPublisherTest.ts#L145-L153

Yup the value I have stored in the Env variable is just a 20 char string of alphanumerics. No prefixes. And my config options objects looks the same as your test except i specify a channel as well.

jbool24 commented 3 years ago

@mmaietta Also, do you think we should supply a token field on the Options object instead of coding an env variable directly in the concrete publisher class. It might be a little more transparent that way. Same with the URL in case some consumers are using a self-hosted version of Bitbucket instead of the cloud. And then configs can be responsible for setting up the value via Env variables.

export class BitbucketPublisher extends HttpPublisher {
  readonly providerName = "bitbucket"
  readonly hostname = this.info.hostname || "api.bitbucket.org" // <-- might be a  self hosted domain

  private readonly info: BitbucketOptions
  private readonly auth: string
  private readonly basePath: string

  constructor(context: PublishContext, info: BitbucketOptions) {
    super(context)

    // So this instead of this
    // const token = process.env.BITBUCKET_TOKEN

    const token = this.info.token // <-- use this instead
   // Or at least check the config value first then default to checking for ENV. 
    const token = (this.info.token || process.env.BITBUCKET_TOKEN) || null
    if (isEmptyOrSpaces(token)) {
      throw new InvalidConfigurationError(`Bitbucket token is not set in publisher config options or environment variable 'BITBUCKET_TOKEN' (see https://www.electron.build/configuration/publish#BitbucketOptions)`)
    }
    this.info = info
    this.auth = BitbucketPublisher.convertAppPassword(this.info.owner, token)
    this.basePath = `/2.0/repositories/${this.info.owner}/${this.info.slug}/downloads`
  }

That way we can transparently set it in an config file

    {
      provider: "bitbucket",
      channel: "latest",
      // hostname: 'api.self-hosted-domain.com', (optionally)
      token: BITBUCKET_TOKEN,
      owner: BITBUCKET_REPO_OWNER,
      slug: BITBUCKET_REPO_SLUG,
    },
mmaietta commented 3 years ago

Yup the value I have stored in the Env variable is just a 20 char string of alphanumerics. No prefixes. And my config options objects look the same as your test except I specify a channel as well.

Can you confirm what permissions you have set on the token? I think all I have on mine are read/write Repository permissions.

Re: Token via Config files

The whole point of having it as an env var is so that security vulnerabilities aren't inadvertently created by unsuspecting users who have:

https://github.com/electron-userland/electron-builder/blob/f4c2a19778c6d95c98fd018b2c7397b77b91f31d/packages/builder-util-runtime/src/publishOptions.ts#L108-L111 Even the comment for github says not to store it in the config. Rather, it was provided as a manner with which to programmatically set the token when using setFeedUrl. IMO, the token property should be removed and we require all users to utilize AppUpdater::addAuthHeader instead for setting the token, but that's a breaking semver change and might be unnecessary for the interim.

Basically, the blanket rule is to not commit tokens in a config or env file.

Side note, I like the idea of allowing a self-hosted Bitbucket domain to be provided. Great callout. Happy to review a PR if you're interested in adding it 😉

jbool24 commented 3 years ago

@mmaietta

Can you confirm what permissions you have set on the token? I think all I have on mine are read/write Repository permissions.

Yes the token I have has proper permissions (I just re verified this moment) both a request with curl and Postman using that token. The token has read/write permission on that repository. I just uploaded a test.txt file with both curl and Postman successfully using the same token

Re: Token via Config files

The whole point of having it as an env var is so that security vulnerabilities aren't inadvertently created by unsuspecting users who have:

  • stored the token in their config and committed it to a public repo
  • have the token injected in the config but then reveals it in their public build/deploy logs

https://github.com/electron-userland/electron-builder/blob/f4c2a19778c6d95c98fd018b2c7397b77b91f31d/packages/builder-util-runtime/src/publishOptions.ts#L108-L111

Even the comment for github says not to store it in the config. Rather, it was provided as a manner with which to programmatically set the token when using setFeedUrl. IMO, the token property should be removed and we require all users to utilize AppUpdater::addAuthHeader instead for setting the token, but that's a breaking semver change and might be unnecessary for the interim. Basically, the blanket rule is to not commit tokens in a config or env file.

Side note, I like the idea of allowing a self-hosted Bitbucket domain to be provided. Great callout. Happy to review a PR if you're interested in adding it

I politely disagree that there is a very good reason to have the use case as in the GithubOptions object. Yes, I acknowledge and agree that committing configs with secrets is irresponsible. Which is why I use a factory function to compose the configs with Env variables and deep freeze such objects at runtime. My configs are NEVER added to any repository with secrets and this is not the use case I'm suggesting. The builder cmd allows for a --config flag to provide a filepath. I use a *.js (just like webpack) to compose configs as I mentioned.

None of what I've suggested should imply committing token values (or any secrets) to a repository directly. If a developer consuming this library does that!? it's on them and their team, not the fault of library code or the authors.

Having the choice to use the field value in programmatic files allows a project to communicate to other team members that a value dependency needs to be there without having to dive into library source or rely on error message instructions from library source. Anyone on the team can see its usage inside the config.

jbool24 commented 3 years ago

@mmaietta Where is this new code located in the packages? I cannot find the bitbucket provider or any of the new stuff. Also did testing exporting the value directly and still not working. I have verified this particular token is valid, has admin rights, and certain it works on all GET|POST|PUT|DELETE requests. This has been tested with Postman and curl.

# 201 in this case BITBUCKET_TOKEN is prehashed using user:passkey 
[ $BITBUCKET_TOKEN ==  `echo -n "${OWNER}:${APP-TOKEN}" | base64` ] # true
# comes out to something like SkJlbGxlcstf45gsdfkROTM2c0xTZkRuS1FqNEo=
BITBUCKET_TOKEN={{token_value}}; curl -iX GET https://api.bitbucket.org/2.0/repositories/{{OWNER}}/{{SLUG}}/downloads -H "Authorization: Basic ${BITBUCKET_TOKEN}

HTTP/2 201 
server: nginx
vary: Authorization, Origin
cache-control: no-cache, no-store, must-revalidate, max-age=0
content-type: text/html; charset=utf-8
...
# this fails 401 -- In this case the BITBUCKET_TOKEN is just the passkey value direct from bitbucket since your code uses the helper static to hash the string. 
BITBUCKET_TOKEN={{token_value}}; yarn electron-build --config electron-builder.config.js --publish always

⨯ 401 Unauthorized
"method: POST url: https://api.bitbucket.org/2.0/repositories/[[REDACTED_OWNER]]/[[REDACTED_SLUG]]/downloads\n\n          Data:\n          \n          "
Headers: {
  "server": "nginx",
  "vary": "Origin",
  "www-authenticate": "Basic realm=\"Bitbucket.org HTTP\"",
  "cache-control": "no-cache, no-store, must-revalidate, max-age=0",
  "content-type": "text/html; charset=utf-8",
  "x-b3-traceid": "5237834340b72510",
  "x-usage-output-ops": "0",
  "x-dc-location": "Micros",

I think something is wrong in the final URL string that gets used in the request. 🤷


Regarding the token as an optional config input field... Another use case would be a very strict corporate setting with very tight security policies. Development teams may not have access to actual values to export and so they might need to employ client code from a platform service such as Google::Secrets Manger or AWS::Hey Management Service or Hashicorp::Vault. In those cases, only the client code would have access to the token key at runtime so a config script would need to inject the private token value that way. If this all happens during a CI pipeline it makes the setFeedUrl flow pointless. setFeedURL with token values only makes sense in user code during electron startup. I agree to disagree so long as the token value is not pulled altogether as an optional field. It would be a shortsighted mistake of Enterprise consumer use-cases.

mmaietta commented 3 years ago

@jbool24 it's in the latest next version and it should be the pre-hashed app password

BITBUCKET_TOKEN=${APP-TOKEN}

electron-builder currently pulls the owner of the repo and converts the BITBUCKET_TOKEN into a Basic <auth> header, as I thought that the owner:app-password hashing may not be apparent to most users.

Maybe I should rename the env vars to BITBUCKET_APP_PASSWORD and add a BITBUCKET_USERNAME to make it more explicit?

Also, I'm not sure I understand your description of setFeedUrl. Could you elaborate more on this? Are you suggesting keeping the token as available in the electron-builder config or for removing it?

I agree to disagree so long as the token value is not pulled altogether as an optional field. It would be a shortsighted mistake of Enterprise consumer use-cases.

jbool24 commented 3 years ago

@mmaietta

electron-builder currently pulls the owner of the repo and converts the BITBUCKET_TOKEN into a Basic header, as I thought that the owner:app-password hashing may not be apparent to most users.

Yes I understood that and I think it makes sense to leave the name as you have it. I was explaining above that I had to hash the user:token myself in order to test with curl only. This was to sanity check that my token was authorized to make both GET and POST requests to the downloads endpoint. When I exported the value in BITBUCKET_TOKEN for use with electron-builder command, the value is the pre-hashed token directly out of bitbucket just as you mention it should be. However, the output of that operation is still 401 unauthorized from within one of the internal requests from electron-builder. Something is not being setup properly in one of the requests from electron-builder.

so, I'm not sure I understand your description of setFeedUrl. Could you elaborate more on this? Are you suggesting keeping the token as available in the electron-builder config or for removing it?

I'm requesting that we keep it (and add optional 'token' field to the schema for BitbucketOptions) so that Enterprise consumers who have a specific use-case where all secrets must be stored in Key Management Services that are controlled by other departments outside the development team can use client-code inside a config.js file and let the client code supply the token via options that way. Does that make sense? Its a very specific scenario but exactly the scenario we have :smile:

In Pre-Production CI, I have no control over what 3rd party tokens/keys were created and stored (SecOps does this) so we let the config script pull in the value using Google/AWS SDKs to pass the returned key into our pre-build operation. We would need the line below added to the BitbucketPublisher as well as the field added to schema validator template to allow for this use-case. This is exactly how I provided a token for GithubPublisher before migrating our repository to bitbucket.

// BitbucketPublisher.ts @ line 20
const token = (this.info.token || process.env.BITBUCKET_TOKEN) || null
jbool24 commented 3 years ago

@mmaietta So, can we get line 20 changed to the previous mentioned?

// BitbucketPublisher.ts @ line 20 and 21
const token = (this.info.token || process.env.BITBUCKET_TOKEN) || null
const username =  (this.info.username || process.env.BITBUCKET_USERNAME) || null

I hope its understood what this would allow me to do in configs. What I would like to have the flexibility to do is run

> electron-builder --config=electron-builder.config.js -p always

with the config as below

// electron-config.js
const package = require("./package.json");
const now = new Date();
const buildVersion = `${now.getFullYear() - 2000}.${now.getMonth() +
  1}.${now.getDate()}-${now.getTime()}`;

const BITBUCKET_API_USER = process.env.BITBUCKET_API_USERNAME;
const BITBUCKET_API_SECRET = process.env.BITBUCKET_API_PASSWORD; // <-- notice still secret 
// but user defined env var name. Not forced to use name BITBUCKET_TOKEN
const BITBUCKET_REPO_OWNER = process.env.BITBUCKET_REPO_OWNER;
const BITBUCKET_REPO_SLUG = process.env.BITBUCKET_REPO_SLUG;

/**
 * @type {import('electron-builder').Configuration}
 * @see https://www.electron.build/configuration/configuration
 */
const config = {
  appId: `com.example.${package.name}`,
  productName: "CoolApp",
  copyright: `Copyright © year ${new Date().getFullYear()}`,
  publish: [
    {
      provider: "bitbucket",
      channel: "latest",
      owner: BITBUCKET_REPO_OWNER,
      slug: BITBUCKET_REPO_SLUG,
      token: BITBUCKET_API_SECRET, //<-- I would rather do this. Now I could use a user defined env var name
      username: BITBUCKET_API_USER, //<--- Here Too
    },
  ],
//... rest of the config
}

This setup makes the builder config (publish settings) transparent when someone else on the team needs to see what is required in the ENV for CI builder and Orchestrated Deployments. ALSO, this way I now would have the ability to use a Secrets Manager service client library in cloud CI/CD environments (AWS, Google, Azure)

mmaietta commented 3 years ago

Fair points. Happy to accept a PR 🙂

jbool24 commented 3 years ago

@mmaietta OK. I'm also going to toss in one more thing to the PR. Using fs-extra module to support fs/promises. I'm stuck on an old version (electron 11) for now to make sure electron print functions work (currently broken in Electron > 11.4). So I can't use naitive (Node 14) fs/promises which you use in BitbucketProvider.js. Its causing runtime errors for the updater.

jbool24 commented 2 years ago

@mmaietta still some issues on the Updater runtime side. Getting the following:

Error: Error: Unable to find latest version on Bitbucket (owner: {{REDACTED}}, slug: {{REDACTED}}, channel: latest), please ensure release exists: HttpError: 403 
"method: GET url: https://api.bitbucket.org/2.0/repositories/{{REDACTED}}/{{REDACTED}}/downloads/latest-linux.yml?noCache=1fs50trpr
Data:
   Access denied. You must have write or admin access."          

Looks like we need that ApiKey bundled with the other values in app-update.yml in order for the Provider to use on GET request. How does the Github provider handle this since it also needs the key at client runtime? NOTE: The key we used to Publish works and does have read/write access to the repository on Bitbucket.

mmaietta commented 2 years ago

Hi @jbool24 , apologies for the delayed response. For electron-updater, I swapped out fs/promises and put back fs-extra so that it has broader electron support. That's in the v23-alpha versions right now.

Regarding the token. You can add an authentication header to the appUpdater via
https://github.com/electron-userland/electron-builder/blob/86e6d1509f9b9c76c559e9c3a12b7a1595fe3ac4/packages/electron-updater/src/AppUpdater.ts#L104-L111 Usage in the docs: https://github.com/electron-userland/electron-builder/blob/b01d5225631115f6f301cb113b044fd10ebb5256/docs/auto-update.md#custom-options-instantiating-updater-directly

jbool24 commented 2 years ago

@mmaietta Awesome I can confirm that worked. Thank you for that. BUT now I get another error because the Bitbucket API redirects to an Amazon s3 bucket for object storage and there seems to be a problem with the underlying request made by the Updater to follow the redirect to s3.

The link that prints out in my console does works if I open in a browser so I can confirm that its a valid link and I can download the latest.yml file from Bitbucket's s3 bucket. But I get something like below at runtime during the Update check flow...

Error: Error: Unable to find latest version on Bitbucket (owner: {{REDACTED}}, slug: {{REDACTED}}, channel: latest), please ensure release exists: HttpError: 400 Bad Request                                                                                            
"method: GET url: https://bbuseruploads.s3.amazonaws.com/{{REDACTED}}/downloads/{{REDACTED}}/latest-linux.yml?Signature={{REDACTED}}%3D&Expires=1645150556&AWSAccessKeyId={{REDACTED}}&versionId=vk00jQdd
y3wvZPtsK80aZB0.y25XBi72&response-content-disposition=attachment%3B%20filename%3D%22latest-linux.yml%22\n\n"

Again, if I manually follow that link that prints it does download that file so it does exist. Any Ideas??

mmaietta commented 2 years ago

Maybe there are some headers missing? I don't have a test environment to work with currently. Can you trace the network request and compare web browser vs electron-updater's request?

jbool24 commented 2 years ago

@mmaietta not sure how to capture the trace from electron-updater. How can I do that?

I can do tcpdump but that won't show the header data like curl or the browser

jbool24 commented 2 years ago

@mmaietta Any Ideas here? I don't think whatever client electron-updater is using is passing the Auth headers on through the 301 redirect to AWS s3.

This is the only trace I can see from the console

/tmp/.mount_PushPaVnJtgc/resources/app.asar/packages/main/dist/updater.js [UPDATER] Provider Options:  {
  provider: 'bitbucket',
  owner: '{{REDACTED}}',
  slug: '{{REDACTED}},
  channel: 'latest',
  requestHeaders: { Authorization: 'Basic {{REDACTED}}' }
}
# ...

  <?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>Basic {{REDACTED}}</ArgumentValue><RequestId>XFZ7KE661TCYTGD0</RequestId><HostId>CneBQ7oyQIk9PciPQsWzuF65kU5DpY1y6eM7OjL3W5ayRaEu0XrQ5g0D61RlQ8ZMmmeA2BDEoJE=</HostId></Error>\n          "
Headers: {
  "x-amz-request-id": "XFZ7KE661TCYTGD0",
  "x-amz-id-2": "CneBQ7oyQIk9PciPQsWzuF65kU5DpY1y6eM7OjL3W5ayRaEu0XrQ5g0D61RlQ8ZMmmeA2BDEoJE=",
  "content-type": "application/xml",
  "transfer-encoding": "chunked",
  "date": "Fri, 25 Feb 2022 15:58:38 GMT",
  "server": "AmazonS3",
  "connection": "close"
}
    at ye (/tmp/.mount_PushPaVnJtgc/resources/app.asar/packages/main/dist/updater.js:1:9819)
    at IncomingMessage.<anonymous> (/tmp/.mount_PushPaVnJtgc/resources/app.asar/packages/main/dist/updater.js:1:12648)
    at IncomingMessage.emit (node:events:394:28)
    at endReadableNT (node:internal/streams/readable:1343:12)
    at processTicksAndRejections (node:internal/process/task_queues:83:21)
    at Object.e.newError (/tmp/.mount_PushPaVnJtgc/resources/app.asar/packages/main/dist/updater.js:2:15129)
    at ms.getLatestVersion (/tmp/.mount_PushPaVnJtgc/resources/app.asar/packages/main/dist/updater.js:2:109242)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)
    at async uc.getUpdateInfoAndProvider (/tmp/.mount_PushPaVnJtgc/resources/app.asar/packages/main/dist/updater.js:2:122415)
    at async uc.doCheckForUpdates (/tmp/.mount_PushPaVnJtgc/resources/app.asar/packages/main/dist/updater.js:2:122701)
mmaietta commented 2 years ago

This looks like a solid culprit

Only one auth mechanism allowed; only the X-Amz-Algorithm query parameter, Signature query string parameter or the Authorization header should be specified

Also, I think you may have left an additional auth token in the error response still needed to be edited out (<ArgumentValue>Basic ____)

My best guess is that the authorization header is supposed to be removed on redirects?

jbool24 commented 2 years ago

@mmaietta

Also, I think you may have left an additional auth token in the error response still needed to be edited out (Basic ____)

Thanks :pray:! Removed although these tokens are not for production and get cleared often. :smile:

Hmm so what do you suggest I can do to fudge around with the updater? Can you point me to where I might dig in to source code. There is a lot of inheritance happening spread across modules withing electron-builder so I'm having a little difficulty sourcing the responsible functionality :face_with_spiral_eyes:

I have to figure out how to make the updater work with bitbucket asap to complete deployment of a project so any help you can provide is crazy appreciated :pray: :pray: :pray:

mmaietta commented 2 years ago

This is how the auth header is being added in the integration test: https://github.com/electron-userland/electron-builder/blob/bf0382e8a142a365848021319dd51dba4d7617ae/test/src/updater/nsisUpdaterTest.ts#L64-L74

updater.addAuthHeader(BitbucketPublisher.convertAppPassword(options.owner, process.env.BITBUCKET_TOKEN!))

Could you confirm that you're converting your app password before adding it to the header? The integration test passes for me with my bitbucket token for the updater's validateDownload It's mentioned here in the Bitbucket section https://www.electron.build/configuration/publish#publishers

convertAppPassword(owner: string, token: string) {
    const base64encodedData = Buffer.from(`${owner}:${token.trim()}`).toString("base64")
    return `Basic ${base64encodedData}`
}
jbool24 commented 2 years ago

@mmaietta yes I can confirm the password is encoded as Basic Auth base64 string. Right now for testing in fact I have the string already converted and hard-coded in.

const options = {
  provider: 'bitbucket',
  owner: import.meta.env.VITE_BITBUCKET_REPO_OWNER,
  slug: import.meta.env.VITE_BITBUCKET_REPO_SLUG,
  channel: 'latest',
  requestHeaders: {
    Authorization: 'Basic {{REDACTED}}',
  },
} as BitbucketOptions;

Where what has been redacted looks like U29tZW5hbWU6c29tZXNlY3JldA==. Is that the issue? Should I use the class method to set the auth value instead? Does this change the way the request headers are sent on the redirect trip once the http client gets a 301?

let autoUpdater: AppUpdater;

const options = {
  provider: 'bitbucket',
  owner: import.meta.env.VITE_BITBUCKET_REPO_OWNER,
  slug: import.meta.env.VITE_BITBUCKET_REPO_SLUG,
  channel: 'latest',
} as BitbucketOptions;

console.log(`${__filename} [UPDATER] Provider Options: `, options);

if (process.platform === 'win32') {
  autoUpdater = new NsisUpdater(options);
} else if (process.platform === 'darwin') {
  autoUpdater = new MacUpdater(options);
} else {
  autoUpdater = new AppImageUpdater(options);
}

// Again using the hardcoded for testing but will change to the class method `convertAppPassword` for PROD
autoUpdater.addAuthHeader('Basic {{REDACTED}}'); <-- should this function be used instead of passing in args to contructor?
//...

Note 02/28/2022 - I tested as above as well and request seems to download and evaluate the latest.yml file. However the download cannot complete in the rest of the updater flow because then next request is again malformed (missing headers I believe) See below logs

Loading autoloader...
/tmp/.mount_PushPabA0c1x/resources/app.asar/packages/main/dist/updater.js [UPDATER] Provider Options:  {
  provider: 'bitbucket',
  owner: '{{REDACTED}}',
  slug: '{{REDACTED}}',
  channel: 'latest'
}
Checking for update
Checking for update...
[940251:0228/121712.322575:ERROR:nss_util.cc(286)] After loading Root Certs, loaded==false: NSS error code: -8018
Found version 22.2.28-1646068495920 (url: {{APPNAME REDACTED}}.AppImage)
Update available.
Downloading update from {{APPNAME REDACTED}}.AppImage
{
  version: '22.2.28-1646068495920',
  files: [
    {
      url: '{{APPNAME REDACTED}}.AppImage',
      sha512: {{REDACTED}},
      size: 143067188,
      blockMapSize: 149576
    }
  ],
  path: '{{APPNAME REDACTED}}.AppImage',
  sha512: {{REDACTED}},
  releaseDate: '2022-02-28T17:16:52.521Z'
}
updater cache dir: /home/justin/.cache/push-pass-updater
Cannot download differentially, fallback to full download: HttpError: 400 Bad Request
Headers: {
  "x-amz-request-id": {{REDACTED}},
  "x-amz-id-2": {{REDACTED}},
  "content-type": "application/xml",
  "transfer-encoding": "chunked",
  "date": "Mon, 28 Feb 2022 17:17:13 GMT",
  "server": "AmazonS3",
}
    at Object.ye (/tmp/.mount_PushPabA0c1x/resources/app.asar/packages/main/dist/updater.js:1:9819)
    at Object.Ul [as checkIsRangesSupported] (/tmp/.mount_PushPabA0c1x/resources/app.asar/packages/main/dist/updater.js:2:134964)
    at ClientRequest.<anonymous> (/tmp/.mount_PushPabA0c1x/resources/app.asar/packages/main/dist/updater.js:2:142835)
    at ClientRequest.emit (node:events:394:28)
    at SimpleURLLoaderWrapper.<anonymous> (node:electron/js2c/browser_init:101:6816)
    at SimpleURLLoaderWrapper.emit (node:events:394:28)
mmaietta commented 2 years ago

Is that the issue? Should I use the class method to set the auth value instead? Does this change the way the request headers are sent on the redirect trip once the http client gets a 301?

autoUpdater.addAuthHeader('Basic {{REDACTED}}'); <-- should this function be used instead of passing in args to constructor?

I think technically both routes would work. Passing an auth header requestHeaders in options is the same as the helper method addAuthHeader https://github.com/electron-userland/electron-builder/blob/6fcd47767af8a95ab018fe0d8a07d2c53a72067d/packages/electron-updater/src/AppUpdater.ts#L194-L196 https://github.com/electron-userland/electron-builder/blob/6fcd47767af8a95ab018fe0d8a07d2c53a72067d/packages/electron-updater/src/AppUpdater.ts#L108-L110

I'm legitimately confused on this. I can't even find where the error is being through, as it'd be great to have the error also log what the URL it's trying to download from. I'm wondering if the full download URL is just the blockmap URL with .blockmap removed, which is causing the failure because the base url would be different?

Side note: I think you can just use const { autoUpdater } from 'electron-updater'. It automatically handles the per-OS updater logic https://github.com/electron-userland/electron-builder/blob/6fcd47767af8a95ab018fe0d8a07d2c53a72067d/packages/electron-updater/src/main.ts#L18-L30

mmaietta commented 2 years ago

Can you try applying this patch so that we can get more data on where it's failing? electron-updater+5.0.0.patch

diff --git a/node_modules/electron-updater/out/differentialDownloader/DifferentialDownloader.js b/node_modules/electron-updater/out/differentialDownloader/DifferentialDownloader.js
index 6cabae3..7bb8cdd 100644
--- a/node_modules/electron-updater/out/differentialDownloader/DifferentialDownloader.js
+++ b/node_modules/electron-updater/out/differentialDownloader/DifferentialDownloader.js
@@ -28,6 +28,7 @@ class DifferentialDownloader {
         builder_util_runtime_1.configureRequestUrl(this.options.newUrl, result);
         // user-agent, cache-control and other common options
         builder_util_runtime_1.configureRequestOptions(result);
+        this.logger.info(result);
         return result;
     }
     doDownload(oldBlockMap, newBlockMap) {
diff --git a/node_modules/electron-updater/out/differentialDownloader/FileWithEmbeddedBlockMapDifferentialDownloader.js b/node_modules/electron-updater/out/differentialDownloader/FileWithEmbeddedBlockMapDifferentialDownloader.js
index c53d2e6..2fabf2d 100644
--- a/node_modules/electron-updater/out/differentialDownloader/FileWithEmbeddedBlockMapDifferentialDownloader.js
+++ b/node_modules/electron-updater/out/differentialDownloader/FileWithEmbeddedBlockMapDifferentialDownloader.js
@@ -10,28 +10,31 @@ class FileWithEmbeddedBlockMapDifferentialDownloader extends DifferentialDownloa
         const fileSize = packageInfo.size;
         const offset = fileSize - (packageInfo.blockMapSize + 4);
         this.fileMetadataBuffer = await this.readRemoteBytes(offset, fileSize - 1);
-        const newBlockMap = readBlockMap(this.fileMetadataBuffer.slice(0, this.fileMetadataBuffer.length - 4));
-        await this.doDownload(await readEmbeddedBlockMapData(this.options.oldFile), newBlockMap);
+        this.logger.info('readRemoteBytes succeeded');
+        const newBlockMap = this.readBlockMap(this.fileMetadataBuffer.slice(0, this.fileMetadataBuffer.length - 4));
+        await this.doDownload(await this.readEmbeddedBlockMapData(this.options.oldFile), newBlockMap);
+        this.logger.info('doDownload succeeded');
     }
-}
-exports.FileWithEmbeddedBlockMapDifferentialDownloader = FileWithEmbeddedBlockMapDifferentialDownloader;
-function readBlockMap(data) {
-    return JSON.parse(zlib_1.inflateRawSync(data).toString());
-}
-async function readEmbeddedBlockMapData(file) {
-    const fd = await fs_extra_1.open(file, "r");
-    try {
-        const fileSize = (await fs_extra_1.fstat(fd)).size;
-        const sizeBuffer = Buffer.allocUnsafe(4);
-        await fs_extra_1.read(fd, sizeBuffer, 0, sizeBuffer.length, fileSize - sizeBuffer.length);
-        const dataBuffer = Buffer.allocUnsafe(sizeBuffer.readUInt32BE(0));
-        await fs_extra_1.read(fd, dataBuffer, 0, dataBuffer.length, fileSize - sizeBuffer.length - dataBuffer.length);
-        await fs_extra_1.close(fd);
-        return readBlockMap(dataBuffer);
+    readBlockMap(data) {
+        return JSON.parse(zlib_1.inflateRawSync(data).toString());
     }
-    catch (e) {
-        await fs_extra_1.close(fd);
-        throw e;
+    async readEmbeddedBlockMapData(file) {
+        const fd = await fs_extra_1.open(file, "r");
+        try {
+            const fileSize = (await fs_extra_1.fstat(fd)).size;
+            const sizeBuffer = Buffer.allocUnsafe(4);
+            await fs_extra_1.read(fd, sizeBuffer, 0, sizeBuffer.length, fileSize - sizeBuffer.length);
+            const dataBuffer = Buffer.allocUnsafe(sizeBuffer.readUInt32BE(0));
+            await fs_extra_1.read(fd, dataBuffer, 0, dataBuffer.length, fileSize - sizeBuffer.length - dataBuffer.length);
+            await fs_extra_1.close(fd);
+            return this.readBlockMap(dataBuffer);
+        }
+        catch (e) {
+            await fs_extra_1.close(fd);
+            this.logger.error(e);
+            throw e;
+        }
     }
 }
+exports.FileWithEmbeddedBlockMapDifferentialDownloader = FileWithEmbeddedBlockMapDifferentialDownloader;
 //# sourceMappingURL=FileWithEmbeddedBlockMapDifferentialDownloader.js.map
\ No newline at end of file
jbool24 commented 2 years ago

@mmaietta

Side note: I think you can just use const { autoUpdater } from 'electron-updater'. It automatically handles the per-OS updater logic

Updated my code to your suggestion above. I updated electron-updater to version 5.0.0 and also patched both FileWithEmbeddedBlockMapDifferentialDownloader.js and DifferentialDownloader.js with logging statements as listed above but I do not see any of those coming to stdout.

After patching electron-builder, I did the following

  1. rebundle and recompile app code and let it publish to bitbucket
    yarn build && yarn electron-builder --config electron-builder.config.js -p always
  2. repeat above once more
  3. then run the first packaged executable so there is a hit on the diff check between current and latest registered in the latest.yml

This is the output at runtime

> ./dist/{{REDACTED}}-22.3.2-1646245898359.AppImage                                                                                                                                                                                          
Loading Printer Module
[1321323:0302/134632.836679:ERROR:sandbox_linux.cc(376)] InitializeSandbox() called with multiple threads in process gpu-process.
Loading autoupdater
Checking for update
[1321287:0302/134634.091647:ERROR:nss_util.cc(286)] After loading Root Certs, loaded==false: NSS error code: -8018
Found version 22.3.2-1646246571369 (url: {{REDACTED}}-22.3.2-1646246571369.AppImage)
Downloading update from {{REDACTED}}-22.3.2-1646246571369.AppImage
updater cache dir: /home/justin/.cache/{{REDACTED}}-updater
{
  version: '22.3.2-1646246571369',
  files: [
    {
      url: '{{REDACTED}}-22.3.2-1646246571369.AppImage',
      sha512: 'N4/h4doVl0diYq3qtQUEYbjYTxXYia/hB2fczLmynTSXdtylN8P/zqNX1Ncf/p9a1Kjbd6N+z+XnMS9mb2YxZg==',
      size: 143058296,
      blockMapSize: 148876
    }
  ],
  path: '{{REDACTED}}-22.3.2-1646246571369.AppImage',
  sha512: 'N4/h4doVl0diYq3qtQUEYbjYTxXYia/hB2fczLmynTSXdtylN8P/zqNX1Ncf/p9a1Kjbd6N+z+XnMS9mb2YxZg==',
  releaseDate: '2022-03-02T18:44:49.044Z'
}
Cached update sha512 checksum doesn't match the latest available update. New update must be downloaded. Cached: wIhWTc1w2DnfM11NWhE89w4lFDxg8zzzwaJPBj2ZMqii7nyNTnkK6+HuCONiSrszYGFosGw0BiFPgS8WomLLTA==, expected: N4/h4doVl0diYq3qtQUEYbjYTxXYia/hB2fczLmynTSXdtylN8P/zqNX1Ncf/p9a1Kjbd6N+z+XnMS9mb2YxZg==. Directory for cached update will be cleaned
{
  headers: {
    accept: '*/*',
    authorization: 'Basic {{REDACTED}}',
    'User-Agent': 'electron-builder',
    'Cache-Control': 'no-cache'
  },
  protocol: 'https:',
  hostname: 'api.bitbucket.org',
  path: '/2.0/repositories/{{REDACTED}}/{{REDACTED}}/downloads/{{REDACTED}}-22.3.2-1646246571369.AppImage'
}
Cannot download differentially, fallback to full download: HttpError: 400 Bad Request
Headers: {
  "x-amz-request-id": "{{REDACTED}}",
  "x-amz-id-2": "{{REDACTED}}",
  "content-type": "application/xml",
  "transfer-encoding": "chunked",
  "date": "Wed, 02 Mar 2022 18:46:34 GMT",
  "server": "AmazonS3",
  "connection": "close"
}
    at Object.zl (/tmp/.mount_PushPaDhLcjI/resources/app.asar/packages/main/dist/index.cjs:8:200550)
    at Object.Wy [as checkIsRangesSupported] (/tmp/.mount_PushPaDhLcjI/resources/app.asar/packages/main/dist/index.cjs:9:132644)
    at ClientRequest.<anonymous> (/tmp/.mount_PushPaDhLcjI/resources/app.asar/packages/main/dist/index.cjs:9:140536)
    at ClientRequest.emit (node:events:394:28)
    at SimpleURLLoaderWrapper.<anonymous> (node:electron/js2c/browser_init:101:6816)
    at SimpleURLLoaderWrapper.emit (node:events:394:28)
New version 22.3.2-1646246571369 has been downloaded to /home/justin/.cache/{{REDACTED}}-updater/pending/{{REDACTED}}-22.3.2-1646246571369.AppImage
Auto install update on quit
Install: isSilent: true, isForceRunAfter: false
./dist/{{REDACTED}}-22.3.2-1646245898359.AppImage  8.90s user 3.58s system 2% cpu 8:21.50 total

I think it worked but I do not see any of those logging statements patched in. Does that mean that code never executed? Is that a problem?

mmaietta commented 2 years ago

Maybe this needs to be added in order for the logging to appear? https://www.electron.build/auto-update#debugging