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

Rewrite to signalr #670

Open
wants to merge 15 commits into
base: master
Choose a base branch
from
Open

Rewrite to signalr #670

wants to merge 15 commits into from

Conversation

LORDofDOOM
Copy link

@LORDofDOOM LORDofDOOM commented Feb 28, 2022

Hi Guys,

I did a rewrite from SocketIO to signalr because there are a lot of drawbacks if there are 2 "server" run concurrent (SocketIO from nodejs and Kaestrel from dotnet). Besides of that I dislike the fact that one program needs 2 ports ;-)

I use Electron.NET different, so in my scenario I start electron though dotnet and not dotnet though electron (electronize). This fulfill my needs better because the UI is not needed in all situations (I want to e.g. access Kaestrel from a remote computer and want the UI only if I work locally on the same system), or want to show a traymenu while the main application is running (as some kind of crossplatform UI). I also work 99,9% in c# code, and dont want to attach a debugger all the time. "electronize" method should still work in this version.

I also want to worry about only one side in terms of security, IMHO dotnet with Kaestrel contains a lot of useful stuff for authorize and we can make sure that only "verified" commands run though.

Please DONT merge this as it is - It's it basically working but have a lot of drawbacks and untested areas (besides of the fact that it completely breaks the SocketIO version), so please let me know if you consider to use this as idea/base and take a review. If there are issues (they are^^) just write me.

Known Problems/Issues:

  • We wait 5s for each response from Node -> C# (see SignalrSerializeHelper.cs) , this is a problem if we wait for a Window result (like a MessageBox) - After 5s everything from SignalrSerializeHelper will return null, should be easy to implement.

  • Hybrid Support is forced to be true

  • Electron HostHook did not work (also unsure if needed)

  • SignalrSerializeHelper is a bit messy and could get replaced in net7 with [Epic]: Support returning values from client invocations dotnet/aspnetcore#5280

- IpcMain is not working (committed out multiple lines) - Unsure if this is needed because we're able to use signalr/razor/blazor for this. Fixed

- Demos are not converted yet Fixed

Expect a lot of bugs ;-)

Thank you

Known Problems/Issues:

- We wait 5s for each response from Node -> C#, this is a problem if we wait for a Window result (like a MessageBox) - After 5s everything from SignalrSerializeHelper will return null.

- IpcMain is not working (committed out multiple lines) - Unsure if this is needed because we're able to use signalr/razor/blazor for this

- Hybrid Support is forced to be true

- Electron HostHook dint works (also unsure if needed)

- Demos are not converted yet

Expect a lot of bugs ;-)
…rong in socketio version) -> Should fix most areas with system.text.json
@GregorBiswanger GregorBiswanger self-assigned this Apr 6, 2022
@GregorBiswanger
Copy link
Member

We have already received a PR #595 with a complete switch to another .NET library with Socket.IO support. Socket.IO runs natively in the backend and is better supported under Node.js than SignalR.

We are happy to be open to pros and cons when deciding on SignalR. Currently there would be no reason.

@LORDofDOOM
Copy link
Author

We have already received a PR #595 with a complete switch to another .NET library with Socket.IO support. Socket.IO runs natively in the backend and is better supported under Node.js than SignalR.

We are happy to be open to pros and cons when deciding on SignalR. Currently there would be no reason.

This PR is just to have a discussion :-)

From my point of view it depends on the use case what is a better fit. In my current project I use Electron.NET as completely optional component/plugin and in that scenario it makes sense to use dotnet as "master", I also use Electron.NET as GUI of a remote application that runs on a webserver for a different project - Both cases are not possible with the current implementation because Electron.NET and the main assembly needs to be on the same system and electron need to be the entry application what is not the case with the signalr implementation.

I also dislike the fact that we need to run 2 independent "server" implementations that use a separate port (dotnet backend, nodejs socket io) and there is no way to have a useful authorization - This problem dont exists with signalr/kaestrel as backend solution and is a show stopper in any real world scenario where security is a factor. Another thing that will make lazy people happy is that you dont need to attach a debugger - you can just debug from VS as you do in any other dotnet application :-)

@GregorBiswanger
Copy link
Member

The two processes communicating via web sockets is ugly. I'm with you there. With SignalR we would have the same communication as well. I haven't noticed any difference in your implementation.

As far as client application security is concerned, any desktop solution is easily vulnerable. Processes hosted in VMs can also be easily dismantled with a kernel debugger.

But yes, an alternative for Electron.NET would be great. We just haven't found a simple solution to this.

@LORDofDOOM
Copy link
Author

LORDofDOOM commented Apr 7, 2022

The two processes communicating via web sockets is ugly. I'm with you there. With SignalR we would have the same communication as well. I haven't noticed any difference in your implementation.

As far as client application security is concerned, any desktop solution is easily vulnerable. Processes hosted in VMs can also be easily dismantled with a kernel debugger.

But yes, an alternative for Electron.NET would be great. We just haven't found a simple solution to this.

Thats true - In terms of communication it's pretty similar, at the end of the day signalr is also a socket connection.

We need to run kaestrel (dotnet) in any scenario (with or without socketio) so the dotnet backend process will always be needed and with signalr it's just a additional endpoint on the same already existing backend and don't request a additional backend (socketio) that's hostet in a different process (electron)

In my PR Electron.NET (UI/Electron itself) is optional and not a base requirement that bootstraps the dotnet entry assembly, it's exactly the way around - dotnet will start the electron process if needed and setup/provide the communication with signalr. It shifts the complete server logic to one process (dotnet).

In terms of security concerns: It's way easier to exploit a application where I just need to send a packet to a socket without any kind of authorize than access the system memory. ATM every application that can communicate with localhost is able to send packets and use Electron.NET as puppet. Other exploits are for sure also possible but e.g. a memory injection would need at least elevated user rights and/or another level of knowledge.

With one backend the coder is able to use any level of protection/authorization aspnetcore offers (and that's a lot^^) and don't need to take care about two backend services.

Here a little sumup what the signalr implementation offers:

  • Only one backend (dotnet), not 2 (dotnet, socketio)
  • Need only one port
  • Electron and dotnet are independent processes that are no longer bound to each other
  • No need to bootstrap dotnet assembly from electron (even if this should still work)
  • Easier debugging (no need to attach debugger)
  • Electron is optional and can get started at runtime (electron can get started if it's needed but is not required to run the dotnet assembly)
  • Electron can run as client/UI only for a remote service on a webserver but still can communicate with the desktop (useful if you want to push notifications from the server to the client or any other kind of desktop interaction)
  • Authorization is possible though aspnetcore (e.g. JWT, cookies, OAuth, IdentityServer)
  • Encrypted communication is possible
  • Electron cant get triggered from anything else than the dotnet process (no option to send packages directly to electron -> Security)
  • With net7 (hopefully) it should be also possible to use signalr like a REST api where we no longer need to use the ugly workaround with SignalrSerializeHelper (what is also the case with socketIO, just different implemented)

@peter-bozovic
Copy link

This looks like interesting solution to me !

But if it is very much different from current Socket implementation I understand it could take a lot of time to fully test this approach and make it production ready.
I'm eager to get the new release that I hope is around the corner, so if we can get what is already stable with sockets would be great. ;)

And would it be possible to have like multiple different host models ? Users could choose if they wish to use the SocketHost library or the SignalRHost that could be the new one ?

I'm using currently Electron with Blazor Server. Would it use the same SignalR in this scenario or it would have two layers of SignalR within SignalR ? :)

And what about the third model that could work like BlazorWebView that Microsoft is shipping for MAUI and WPF ?
Can we manipulate directly the DOM elements within Electron Web View without having to use intermediate communication system ?

I'm really enjoying so far the use of electron.net and hoping for the update soon !

@LORDofDOOM
Copy link
Author

This looks like interesting solution to me !

But if it is very much different from current Socket implementation I understand it could take a lot of time to fully test this approach and make it production ready. I'm eager to get the new release that I hope is around the corner, so if we can get what is already stable with sockets would be great. ;)

And would it be possible to have like multiple different host models ? Users could choose if they wish to use the SocketHost library or the SignalRHost that could be the new one ?

I'm using currently Electron with Blazor Server. Would it use the same SignalR in this scenario or it would have two layers of SignalR within SignalR ? :)

And what about the third model that could work like BlazorWebView that Microsoft is shipping for MAUI and WPF ? Can we manipulate directly the DOM elements within Electron Web View without having to use intermediate communication system ?

I'm really enjoying so far the use of electron.net and hoping for the update soon !

Hi,

the solution works for what I need to get done pretty good - I use it already in production: In my productive version the code is a little bit modified from the code in repo because I need to have authorization and multiple security checks.

It should have implemented almost everything from the socketio solution but will break old code (if you have e.g. custom hooks/endpoints in electron JS).

The different hosting models are hard to implement - In the way it's currently written up we need to maintain both JS implementations (signalr works different from the socketio version and all JS files are changed to reflect this)

For Blazor: Thats exactly what I currently use in my productive solution too - Depending on what hosting model you use (Server/Clientside) it is easy to implement and you can directly bind blazor frontend to electron methods with the classes. I currently use a server side blazor hosting with electron.net as client side frontend.

For BlazorWebView: IMHO this is currently out of scope - The way how this is implemented is based on MS edge, not electron. To have a solution like this I suggest to use chromly or CEFSharp :-)

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

Successfully merging this pull request may close these issues.

4 participants