Skip to content
This repository has been archived by the owner on Dec 15, 2022. It is now read-only.

Is it a no-no to get/set from an Electron renderer process? #250

Open
1 task done
aguynamedben opened this issue Feb 21, 2020 · 6 comments
Open
1 task done

Is it a no-no to get/set from an Electron renderer process? #250

aguynamedben opened this issue Feb 21, 2020 · 6 comments

Comments

@aguynamedben
Copy link

Prerequisites

Description

I've worked myself into a slight pickle with my keytar usage due to the fact that I was setting keys in the Electron renderer, and the Electron renderer process name changed formats in Electron 6.

More details here: electron/electron#22317

Summary:

  • In version A of my app, with Electron 5, the renderer set keytar values. macOS keychain allows access from the setting process, i.e. Command E Helper.
  • In version B of my app, with Electron 7, the renderer attempts to get keytar values, but the macOS keychain pops up a user permission dialog for every single get because the renderer process name has changed, i.e. Command E Helper (Renderer).

My main question is: Is is a no-no to keytar get/set from renderer process? I don't really see any warnings about this in the README.

Thanks for any feedback or ways out you can offer. I really appreciate keytar and Electron, thank you for the work you do on these excellent projects.

@aguynamedben aguynamedben changed the title Is it a no-no to get/set from the Electro renderer process? Is it a no-no to get/set from an Electron renderer process? Feb 21, 2020
@aguynamedben
Copy link
Author

aguynamedben commented Feb 21, 2020

Here are some screenshots that demonstrate what's going on in macOS keychain.

Keychain Access details, Version A of my app w/ Electron 5, after a keytar set from the renderer
image

Version B of my app w/ Electron 7, during a keytar get from the renderer. Note the new name, Command E Helper (Renderer)
image

Keychain Access details after typing my password clicking Always Allow
image

@shiftkey
Copy link
Contributor

shiftkey commented Feb 21, 2020

Is is a no-no to keytar get/set from renderer process?

This is fine, and a very common scenario. It's just that keytar doesn't care about where it gets run from - as long as its something resembling a Node runtime.

As @MarshallOfSound pointed out in the upstream issue, there were changes to how the app works on macOS with Electron 6 that mean the bundle ID for the renderer process has changed, and the operating system now sees it as different to the previous app that it granted permission to read and write to the keychain. Hence the new warning.

I'm not 100% sure of the fix here (I haven't kept up with the latest in the Electron space in the previous few months), but keytar doesn't have any process-specific checks that you can control. I'd recommend checking how your app is packaged, and what hooks you have to maintain compatibility across the Electron upgrade process.

Gonna leave this open for a bit because I suspect others may encounter the same problem.

@aguynamedben
Copy link
Author

@shiftkey Thank you for the very helpful reply. It's nice to know that I'm not just going insane. :)

You're exactly right. I'm packing my app with electron-builder, the bundleIdentifier of the renderer process has changed due to the way electron-builder integrated the Electron 6 helper process changes. I submitted a PR that allows electron-builder users keep a consistent bundleIdentifier value across Electron 5 and Electron 6+ if they specify values in the electron-builder config.

For those trying to understand this issue, here are some helpful commands that will help you introspect your packaged Electron app, each process' plist file, and the bundleIdentifier value macOS depends on when deciding if it will show a permissions dialog or not.

For Electron 5 and before, using electron-builder:

# Dump the .plist file to JSON
plutil -convert json -o /tmp/wat.json /Applications/Command\ E.app/Contents/Frameworks/Command\ E\ Helper.app/Contents/Info.plist

# View the JSON
cat /tmp/wat.json | python -mjson.tool

# (inspect the CFBundleIdentifier value)

If you do this with various installed builds of your app using Electron 5 and Electron 6, you'll see CFBundleIdentifier doesn't actually change. It will be something like com.acme.CoolApp.helper. The problem is that the Electron renderer process is now packaged up in a different process (i.e. CoolApp Helper (Renderer) instead of CoolApp Helper). Let's check out the bundleIdentifier of THAT process:

For Electron 6+, using electron-builder:

# Dump the .plist file to JSON
# Note the path is different! (Renderer)
plutil -convert json -o /tmp/wat.json /Applications/Command\ E.app/Contents/Frameworks/Command\ E\ Helper\ \(Renderer\).app/Contents/Info.plist

# View the JSON
cat /tmp/wat.json | python -mjson.tool

# (inspect the CFBundleIdentifier value)

THIS process has a CFBundleIdentifier value of com.acme.CoolApp.helper.Renderer. If you use keytar from the renderer process now, in Electron 6+, macOS freaks out because this value has "changed" (i.e. the new value doesn't match the value macOS stored when creating the data).

If this is affecting your keytar calls from the renderer in Electron 6+, see my PR. You need to:

  1. Make sure you're using a version of electron-builder with my fix.
  2. Set the "mac" section of your electron-builder config to include:
"helperBundleId": "io.hypertools.Command-E.helperGeneric",
"helperRendererBundleId": "io.hypertools.Command-E.helper",

Shameless plug: I'm the founder of Command E, if you like this helpful information and want to work on some fun Electron stuff, hit me up!!! :)

@bpasero
Copy link

bpasero commented Sep 22, 2020

Slightly related question to @shiftkey and maybe @BinaryMuse: so far in VSCode we have been using keytar from the renderer as well but think about moving it to the Electron main process or some other process that runs in the background. I had a brief look at the native code and feel that the execution is async, i.e. not blocking the main process if it was executed from there, is that true?

Basically I am trying to understand if it is save to have keytar do its business from the Electron main process or if anything could block the event loop there because keytar is doing something non-async. If that was the case I would rather have it in some forked node process off the main process.

@BinaryMuse
Copy link
Contributor

I had a brief look at the native code and feel that the execution is async, i.e. not blocking the main process if it was executed from there, is that true?

Correct, most of the work (calling platform specific systems calls etc) happens in async workers.

@bpasero
Copy link

bpasero commented Sep 22, 2020

Great thanks for clarifying 👍

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

No branches or pull requests

4 participants