Allow websocket server to take ssh host as query parameter…#175
Allow websocket server to take ssh host as query parameter…#175nlundquist wants to merge 2 commits intobillchurch:masterfrom
Conversation
…ws sockets to be opened without loading the client page first to set up the session. This simplifies use when using WebSSH2 with an alternative client. Cleaned up the derivation of configuration from defaults, static config file, request params & session
|
This looks good so far. Give me a day or two to play around with it but I like what you've done so far! |
|
Curious if you've had a chance to look at this yet @billchurch. Main reason I'm interested is that I'm relying on the changes made in this branch, and would prefer not to maintain a fork long-term if possible |
|
I hadn't yet but I am now. Sorry for the delay. |
|
@Mierdin for whatever reason the changes you have don't seem to be compatible with the build of the web client that's in your branch. I get this in the console.log: Chrome devtools look normal to me, don't see anything glaring there. So it seems like the socket is getting established, but it doesn't seem like the ssh connection is happening properly perhaps... If i check out commit 2289036 which seems to be the version of master you started with, everything seems to work properly... I also have some conflicts in the most recent master, but I can resolve those. Could you check out a "fresh" copy of your PR and test against the native client and let me know what you see? |
|
The PR actually belongs to @nlundquist, it was related to some work we were doing together early in the year. I may have some cycles in the near future to get this straightened out - thanks for your patience. |
|
I want to add this but having some issues getting this PR to work. I'm in the process of updating some modules for security and should be publishing a new version soon, afterwich I'd like to revisit this and get it in the code and turn it around quicker... |
|
I like where you're going with this, but I think there's a lot of change here at once, also some "todo" that I don't want to leave hanging out there. I agree that: 1- Websocket needs to be fully removed from client... You should be able to start a session from the websocket entirely 2- Config "stuff" should be out of app.js (just made that change in (f3a9478) in fact... Expect to clean it up a bit more 3- I could be doing a lot of things better What I'd like to do is add this to the 0.5.0 roadmap #244 and I'll make a separate issue #245 to discuss this feature because I could definitely use some help. Namely we need to define an API for that use-case and work backwards from there I think. Namely, what parameters do we want to expose to the user and which ones do we want to limit or control. The needs for Express in this project....as well as socket.io are probably outdated and there are many better ways to accomplish what I was trying to do years ago. |
3 participants
This allows sockets to be opened without requiring the user to load the client page first to set up the session. This simplifies use when using WebSSH2 with an alternative client.
Additionally cleaned up the derivation of configuration from defaults, static config file, request params & session. Some mild renaming and shuffling of code. If any of this is against the style sensibilities of the project I'd be glad to change it.
Note: This is a still a WIP, though I wanted a general review of it earlier rather than later. There's still a few unfinished TODOs to take care of in this before merging. A bit of previous functionality I commented out while making my changes that I have to reenable. Shouldn't be too difficult.