Repository navigation
add session based authentication for rpc - #175
Conversation
add code comments wherever necessary for changes made as part of GSoC '18 work
used express-session middleware to handle the session(added as dependency) used basic-auth to handle authentication(added as dependency) modify the startclient to send cookie data modify the startServer to accept credentials tested both from browser and via node, works well
jadonk
left a comment
There was a problem hiding this comment.
I need to test this in the buster-iot images as well. First thing to fix is to pass the login and socket.io connections over to this client. To simplify things, would it make better sense if the security was done WITHIN the socket.io connection, rather than at the Express server level?
| _bonescript.on.connect_failed = function () {}; | ||
| _bonescript.on.error = function () {}; | ||
| _bonescript.on.error = function (err) { | ||
| throw (new Error(err)) |
There was a problem hiding this comment.
This runs asynchronously on load when loaded by a browser? How do we catch this error?
There was a problem hiding this comment.
I am not very sure how we will be able to catch this error from the browser , however if the current implementation is used, then a 401 Unauthorized response could be sent for unauthorized sessions from browser.
|
|
||
| exports.setUp = function (callback) { | ||
| server.serverStart(8000, process.cwd(), { // create a secure server by supplying credentials | ||
| username: 'testuser', |
There was a problem hiding this comment.
For "production" use, these need to be stored statically in a file. Where is the associated update to server.js (not src/server.js)? That file should be updated to read all parameters from a configuration file. Perhaps use /etc/default/bonescript?
There was a problem hiding this comment.
Sure, i will modifications for that, will use /etc/default/bonescript for the default config. I did not modify the server.js till now as i noticed that it starts the server with default settings, will make changes to it soon.
|
The coverage check should be considered a requirement now. We have a lot of untested code, so it shouldn't be hard to increase the coverage %. |
|
Sure, I will concentrate on increasing the coverage too when making the modifications |
authentication at express level was removed and was implemented at
the socket.io connection through extraheaders, corresponding modification
was made at server and client
* username & password send as basic authentication headers
* modified server.js to read config file from /etc/default/bonescript
the config file format used was :
{
"username":"debian",
"password":"temppwd",
"port": 8000 ,
"directory": "/usr/share/bone101"
}
* need to add more test cases and make other improvements
expression session dependency was removed and simple socket.io middleware
hash-challenge was used for authentication, modified the client and
server for the changes the passphrase can be supplied to serverstart already
as hash or as plaintext: the configuration files used for testing server.js were:
{
"hash": true,
"passphrase":"9f735e0df9a1ddc702bf0a1a7b83033f9f7153a00c29de82cedadc9957289b05",
"port": 8000 ,
"directory": "/usr/share/bone101"
}
and
{
"hash": false,
"passphrase":"testpassword",
"port": 8000 ,
"directory": "/usr/share/bone101"
}
also other unnecessary dependencies were removed, need to add test cases
f805590 to
9c2284b
Compare
modified rpc tests to increase converage added analogWrite(), digitalWrite() and pinMode() tests modified test-ffi to include callback cases added test-math based on pattern of math functions added test-digitalRead() and test-analogRead()
in the case of analogWrite() on digital out, the callbacks were invoked twice everytime, found while writing a test-case
b458da2 to
f149b56
Compare
the callback on digitalWrite on AnalogOut was never invoked due to error in arguments supplied to analogWrite()
jadonk
left a comment
There was a problem hiding this comment.
Good enough. I think we need to change the /etc/default/bonescript format to be more like a typical 'variable = value\n' style text document, but we can work towards that. I'm counting on your testing here!
also add some comments for previous modifications made