fix(nsis): If window service needs to run installer for update, the installer must have admin previlege. #6786

[HwangTaehyun]

If the electron app needs a custom NSIS installer script (aka. oneClick option is false), the electron installer cannot get isAdminRightsRequired and failed to update when the electron app is running on window service because it requires installer that being admin rights.
It seems that any other reason does not exist when the electron builder NSIS oneClick option is false, update-info's isAdminRightsRequired is false.
Thank you!

[changeset-bot]

🦋 Changeset detected

Latest commit: bb2aef5

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 8 packages

Name	Type
app-builder-lib	Patch
dmg-builder	Patch
electron-builder-squirrel-windows	Patch
electron-builder	Patch
electron-forge-maker-appimage	Patch
electron-forge-maker-nsis-web	Patch
electron-forge-maker-nsis	Patch
electron-forge-maker-snap	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[netlify]

✅ Deploy Preview for car-park-attendant-cleat-11576 ready!

Name	Link
🔨 Latest commit	bb2aef5
🔍 Latest deploy log	https://app.netlify.com/sites/car-park-attendant-cleat-11576/deploys/625da41e4a8a7100085191a8
😎 Deploy Preview	https://deploy-preview-6787--car-park-attendant-cleat-11576.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site settings.

[mmaietta]

Instead of removing && oneClick, is it possible to detect for this edge case?

when the electron app is running on window service

This change breaks a snapshot test and I'm unsure if all assisted installers should have "isAdminRightsRequired": true,

FAIL  src/windows/assistedInstallerTest.ts (149.916 s)
  √ assisted (45469 ms)
  √ allowElevation false, app requestedExecutionLevel admin (6587 ms)
  × assisted, only perMachine (39978 ms)

  ● assisted, only perMachine

    expect(received).toMatchSnapshot()

    Snapshot name: `assisted, only perMachine 1`

    - Snapshot  - 0
    + Received  + 2

    @@ -3,18 +3,20 @@
          Object {
            "arch": "x64",
            "file": "Test App ßW Setup 1.1.0.exe",
            "safeArtifactName": "TestApp-Setup-1.1.0.exe",
            "updateInfo": Object {
    +         "isAdminRightsRequired": true,
              "sha512": "@sha512",
              "size": "@size",
            },
          },
          Object {
            "file": "Test App ßW Setup 1.1.0.exe.blockmap",
            "safeArtifactName": "TestApp-Setup-1.1.0.exe.blockmap",
            "updateInfo": Object {
    +         "isAdminRightsRequired": true,
              "sha512": "@sha512",
              "size": "@size",
            },
          },
        ],

[HwangTaehyun]

@mmaietta Thank you for review. I'll try to detect for this edge case and change my code!

[HwangTaehyun]

I detected an edge case for using the allowServiceToInstall boolean option of nsis in the electron builder config

[mmaietta]

Are you already using packElevateHelper = true by chance? Would this work instead?

(updateInfo != null && isPerMachine && (oneClick || options.packElevateHelper))

[HwangTaehyun]

Yes, the elevate.exe file is already included in my electron app because of perMachine true option.

Electron updater decides to elevate or not using isAdminRightsRequired in the update-info.json file.
(https://github.com/electron-userland/electron-builder/blob/5e381c556d12ce185bb7ea720380509c1ddc5cf7/packages/electron-updater/src/NsisUpdater.ts#:~:text=if%20(options.isAdminRightsRequired,%7D)

So, it's seamless using packElevateHelper option beacuse if elevate.exe packed, then electron updater must using this to elevate when update.

I'll edit this based on your feedback!

[mmaietta]

Just to play it safe, can you please also add this test to the assistedInstallerTest.ts?

test.ifNotCiMac(
  "assisted, only perMachine and elevated",
  app({
    targets: nsisTarget,
    config: {
      nsis: {
        oneClick: false,
        perMachine: true,
        packElevateHelper: true
      },
    },
  })
)

You can update the snapshot via:

TEST_FILES="assistedInstallerTest" UPDATE_SNAPSHOT=true pnpm test 

After that, I think we're good to merge!

[HwangTaehyun]

I added the test code to this commit!
Lastly, I will test the update in the Window service and tell you again.

[HwangTaehyun]

I checked it.

below nsis option's generate the following latest.yml.

nsis:
  oneClick: false
  perMachine: true
  packElevateHelper: true
  ...
  include: './scripts/nsis/installer.nsh'

...
files:
  - url: xxx-Setup.exe
    sha512: NZvMh7iNTT9/Uz5WsEVR2YS6+KFI7dAHerTJH743tJkeS/jNJr3sUp6OT8f4y3MyANQ6FgUsb2qRd99HjEAhug==
    size: 152614338
    isAdminRightsRequired: true // generated by nsis's packElevateHelper true option despite oneClick false option
path: xxx-Setup.exe
sha512: NZvMh7iNTT9/Uz5WsEVR2YS6+KFI7dAHerTJH743tJkeS/jNJr3sUp6OT8f4y3MyANQ6FgUsb2qRd99HjEAhug==
releaseDate: '2022-04-19T02:54:40.490Z'