Dual Interface Support

[arrmo]

Hi,

To this issue, https://github.com/bsileo/SmartThings_Pentair/issues/5#issuecomment-348819940

Is it possible to have nodejs-poolController support two interfaces ... http on one port, https on another? https is needed for external access, but http for integration.

Thoughts?

Thanks!

[tagyoureit]

I had tried this a while ago... my idea was to have HTTP running and if the user wanted HTTPS to just redirect to port 443/SSL. I did run into some trouble getting both servers running at the same time.

Not sure what it would take to get both running at the same time with either my original idea above, or your desired solution of having both running simultaneously.

I'll try to find some time to look at this, but any help is appreciated!

[arrmo]

I'll try to dig into it! Really need it on two different ports, to expose one externally (HTTPS), the other internal only (HTTP).

I admit though, a bit lost since you restructured the code ... :-(. Where is the Express server set up now?

Thanks!

[arrmo]

Actually, found it, and I think I have it figured out ... we can set up both simultaneously.

I'm fine taking a run at this, but as you are changing code as well - don't want to throw away my changes ... :-). I need to modify config.json and server.js (I think ... LMAO). Safe to change these?

Thanks!

[tagyoureit]

If you can figure it out, we'll be able to get it running with my changes... Everything from container.settings.xyz will now be called by container.settings.get('xyz'). And looking at some of my old code an laughing. So updating a lot here and there.

[arrmo]

NP, will take a run at it tonight or tomorrow (changes aren't that extensive, but a bit tied up tonight). Thanks!

[arrmo]

OK, I think I know how to make this work, but I need to change part of config.json - something like below,

"web": {
    "httpEnabled": 1,
    "httpExpressPort": 3001,
    "httpsEnabled": 1,
    "httpsExpressPort": 3000,
    "httpsExpressAuth": 1,
    "httpsExpressAuthFile": "/data/users.htpasswd"
},

But ... it seems these variable names are sprinkled throughout, making it quite a bit more difficult. Thoughts of how to handle this?

Thanks!

[tagyoureit]

We can change them as needed... it's easy to modify the code. What if we get ever more descriptive?

"web": {
        "http": {
             "enabled": 1,
         "expressPort": 3001,
          },
          "https":{
            "enabled": 1,
            "expressPort": 3000,
            "expressAuth": 1,
            "expressAuthFile": "/data/users.htpasswd"
        }
},

I guess another question is... do we want to enable authentication on http in addition to https? And if so, should it be it's own section or one single section for each protocol?

EG, if someone just wants auth on http, should we allow that configuration?

[tagyoureit]

BTW, since we are re-doing the express interfaces, I'd like to add one more piece to the puzzle- a redirect from http to https. Often, when I have my https up and running I can't get to my server only to realize I'm not using the https interface. A redirect from one to another would be useful in many cases (but not the Smart Things integration). Maybe another variable under http like redirectToHttps:0/1?

[arrmo]

OK, too funny. I say that because I had it very similar to what you have above (when working on it here), but then ran into the variable issue. But I agree - yes, let's add Auth also to HTTP! I have an idea of how to make this work, but can't prove it out until the variables work. Are you able to fix that, then I can re-base and try the server fix.

BTW, just a thought (and by all means disagree!), but it would be nice if variables added to config.json get handled automatically. I was thinking / assuming that the variables above would just show up as settings.web.http.enabled (example). Then going forward is pretty easy.

Thanks!

[arrmo]

Digging in to this, and I think we need an array of servers (i.e. 2 of them) ... as each may or may not have Auth, so they need to be somewhat independent, agreed?

Some info on this - not sure it's too difficult, we just need to close first on having two servers. https://stackoverflow.com/questions/8355473/listen-on-http-and-https-for-a-single-express-app http://expressjs.com/en/api.html#app.listen

[arrmo]

A bit more on this, but it seems to me like setting up 2 servers is easiest - particularly given that they may not be the same in terms of Auth ... agreed? https://stackoverflow.com/questions/28755281/nodejs-express-separate-routes-on-two-ports

[tagyoureit]

Yes, I think we'll need to have two servers (http/https), and ideally share the same app. We can set one additional http-to-https forwarding if such a flag is set. We can use this module or use code like it for the redirect.

[tagyoureit]

I'm rethinking my approach to the settings file overall... I think it is over-complicated by having the settings in memory possibly be mis-aligned with the settings in the file. settings.js stores the settings, and config-editor.js reads/writes the settings, but you can set values separately in settings and not write them back. Just need to think about how to streamline the process.

Right now, if I want to know whether to output to a log, I call container.settings.get('logPumpMessages'). If I went with a straight 1:1 for the config file, that would change to container.settings.get('poolController.log.logPumpMessages'). (Before I changed it to .get() it was just container.settings.logPumpMessages so it already feels too verbose.)

Have to ponder this some more.

[arrmo]

Working through this, getting there! One odd thing though ... the path.join line inside opt_https seems to be outputting the following (and then breaking). Have you seen this before? Error: ENOENT: no such file or directory, open '/mnt/ProgSSD/nodejs-poolController[object Object]'

Thanks!

[tagyoureit]

Um, I'll give my standard answer... I don't see that problem. :) Make sure that you have your /data directory in nodejs-poolController and that it has your crt and key files in it. I used the ones from my specs directory (for testing) and copied this over to the root of the app and it worked fine.

This is something where we could offer some more flexibility (and probably some better error messages!) so something to consider with the next version of the config.json changes.

[tagyoureit]

Just pushed up a version that will check for the certificate and auth files so it will give you a better error message.

[arrmo]

OK, debugging here. Not sure I can pull in any updates - I made several changes to try to get this working, and it will likely blow them all away ... :-(.

[arrmo]

@$^&! ... OK, that was me. Sigh. Fat-fingered it, this part is working now. So I am able to (sort of) get two servers up and running, just a couple questions / issues, 1) After all this ... not sure it will work. We can't have 2 connections to socat - so how to make a single connection to socat work with 2 servers. Or will that be handled? 2) I'm not familiar with Promise - need to dig into it, but ... do we have to return just one? Looking at server.js, and not sure how (yet) to get around the Promise there.

Thanks!

[arrmo]

And one more ... I seem to have lost socket.io in my updates, but this is also not set in server.js. Any pointers on where / how this one is set up?

Thanks!

[arrmo]

OK, also got socket.io working, but (for now at least) I have to hard code only one of the servers. Not sure how to fix that. Hmmm ... thinking ... ;-)

[tagyoureit]

Nice job so far!

After all this ... not sure it will work. We can't have 2 connections to socat - so how to make a single connection to socat work with 2 servers. Or will that be handled?

We don't need 2 socat connections. Socat is just from the RS485 to poolController app. Only 2 web servers and Socket.IO connections.

I'm not familiar with Promise - need to dig into it, but ... do we have to return just one? Looking at server.js, and not sure how (yet) to get around the Promise there.

They are a pain in the butt, for the most part! We really don't need them for synchronous code. If you get the two servers working, I can integrate it with the rest of the app.

I'm not familiar with Promise - need to dig into it, but ... do we have to return just one? Looking at server.js, and not sure how (yet) to get around the Promise there.

Socket.io should be pretty easy to get two connections setup. If you get the two web servers working, I can wire in Socket.io pretty easily. If you want to try, this code is where it is done. We need to return one or two server objects and then create the appropriate connection based on that. Right now, it's only returning the one server object.

[arrmo]

We don't need 2 socat connections. Socat is just from the RS485 to poolController app. Only 2 web servers and Socket.IO connections.

Perfect! I was wondering that, and that's great news - means this will work.

They are a pain in the butt, for the most part! We really don't need them for synchronous code. If you get the two servers working, I can integrate it with the rest of the app.

OK, a bit more reading here, it makes sense now - and yes, it seems this is needed for asynchronous operation. But ... you seem to provide resolve, but no reject. Is there a way to reject if the server is not set up?

Also though - I'll have 2 servers ... just resolve after they are both set up (if enabled)?

Socket.io should be pretty easy to get two connections setup. If you get the two web servers working, I can wire in Socket.io pretty easily. If you want to try, this code is where it is done. We need to return one or two server objects and then create the appropriate connection based on that. Right now, it's only returning the one server object.

Yep, figured this out as well - it seems that I need to return an array from getServer, agreed? If I do, you can handle that?

[tagyoureit]

OK, a bit more reading here, it makes sense now - and yes, it seems this is needed for asynchronous operation. But ... you seem to provide resolve, but no reject. Is there a way to reject if the server is not set up?

I really don't need to wrap that whole thing in a Promise... there is no async code in there. But the format would otherwise be:

return new Promise(function(resolve,reject({
//do some code, if good...
resolve()
//if bad...
reject()
})

You could also do...

return new Promise(function(resolve,reject({
//do some code, if good...
resolve()
})
.catch(function(err){
// log my err, or fix it and continue
// a catch will 'resolve' the promise, but you could then 
// throw new Error('We really must exit. ' + err)
// if you want to reject the promise altogether
})

Yep, figured this out as well - it seems that I need to return an array from getServer, agreed? If I do, you can handle that?

Yes, that would work just fine.

[arrmo]

OK, I think I have it working! I can create two servers, http and https, and for each of them enable (independently) authentication. Two questions / issues ... 1) Do we want an enable / disable flag for each? So only one can be created if desired. 2) I can return an array - for now only returning one of them, so I don't break socketio-helper.js. I can select either array element (for http or https), and it works => do you want to change socketio-helper.js to accept an array (up to 2 elements, depending on enable / disable)?

I can create a PR, if that's easiest for you to test and work off of.

Thanks!

[tagyoureit]

Do we want an enable / disable flag for each? So only one can be created if desired.

Well, we always need at least one enabled... hmm, or do we. Maybe SmarthThings folks won't want my Web UI at all? No, that's not right because they need the http connection internally. So, I would say that we always need the first one, and the second is optional.

I can return an array - for now only returning one of them, so I don't break socketio-helper.js. I can select either array element (for http or https), and it works => do you want to change socketio-helper.js to accept an array (up to 2 elements, depending on enable / disable)?

Yes, this would be fine. Go ahead and create a PR and I'll clean up everything and setup some tests to make sure it all works. 👍

[arrmo]

Sounds good, PR coming!

I went ahead and added an independent enable for each - you'll see ... ;-). That way you can enable one or the other or both.

Seems to be working here, yell if you see issues!

[arrmo]

OK, PR #68. Do you want a separate one for the typo in write-packet.js?

In any case, you'll see where I pull the index [0] in socketio-helper.js => this is where you'll need an array.

BTW, server.js looks like more is changing than really is - because I moved the app.get items to a subroutine (function), just so I could call (set) them for each server. It really was just a move, I didn't change any of the app.get items, just shifted them (for reuse purposes).

Yell if you have any problems!

[tagyoureit]

Was the writepacket spelling error the loglLevel? If so, I fixed that already. Pull down the latest with your PR and let me know how it works.

[tagyoureit]

I think we're good here... re-open if there are any issues.