-
Notifications
You must be signed in to change notification settings - Fork 3
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
Separate context bridge for electron-auth #89
Comments
We should consider adding a different context bridge for electron-auth so that it doesn't accidentally share the same one as core-electron. However, we thought it would be safe since you'd have to merge the two if you wanted to have both. tl;dr We went back and forth on this a lot when the split happened during the 3.x release. The overriding goal was to avoid any explicit dependency in the core-electron (or any core package) on a specific type of authorization workflow and keep it as agnostic as possible. This was the main reason for not having a dependency on an OIDC/OAuth2 specific package for our default electron configuration. The reason for not having it the other way is to allow other Electron apps that we may have to use the package without needing to pull along all of our backend code which is required from core-electron. Given those two goals and the fact you can't import additional javascript files, we had two preload scripts to ensure the electron-auth package could stand on its own with the core-electron package and not rely on the user of it to have to add code specifically to their preload script if they didn't need to do so. As mentioned above, we could have a separate bridge that makes it clear it's for auth and not just the general auth. |
My two cents (not really what you guys were talking about, but related)... Our app has a problem with this package because @itwin/electron-authorization assumes authorization takes place on Electron's main process. What I suggest, is either:
I can submit a PR with the config change if you agree |
We have an electron preload script, that exposes some ipc message handling for itwinjs channels on the window, namely core-electron and electron-auth.
Luckily for us, its the same api rn in both and theres no collision, but if there ever arises a time when there is a difference in the apis, it might cause an issue for us. It also really bothers me that we have pretty much the exact same file duplicated across both pkgs. Wondering if we need to consolidate, have an electron-common pkg or some shim. Also wondering if maybe core-electron takes a dep on electron-auth or vice versa. Just food for thought, something Id like to discuss further with our electron experts
The text was updated successfully, but these errors were encountered: