feature: implement device access and new security spec

[sbender9]

TODO:

• Do a write up with screenshots
• UI to edit devices
• Create tests
• Add WS implementation
• Update plugin documentation on put requests
• Add tests for device access requests
• Add tests for put requests
• prune old requests

This PR provides a way for devices to request an access token from the server.

It also has changes to conform to the new request/response and security specifications.

The device will send a REST request to the server:

curl -k --header "Content-Type: application/json" --request POST --data '{"clientId":"1234-45653-343453","description": "My Awesome Sensor"}' https://localhost:3443/access/requests

The server will return a json response with a request identifier:

{"requestId":"60240a45-bbcb-46f0-8df6-c7a3e048bec0"}

A Signal K notification is sent out when this happens and In the admin ui, there is a list of devices that are requesting access:

In the mean time, a device should poll the server using the requestId in the response above to see if it has been granted access and get the token:

The request is pending:

$ curl -k https://localhost:3443/access/requests/ee759b2b-9051-4188-ab2d-56b97f2a58eb
{"status":"PENDING"}$ 

The request is denied:

$ curl -k https://localhost:3443/access/requests/ee759b2b-9051-4188-ab2d-56b97f2a58eb
{"status":"'DENIED'"}$ 

The request was approved:

$ curl -k https://localhost:3443/access/requests/ee759b2b-9051-4188-ab2d-56b97f2a58eb
{"status":"APPROVED","token":"eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJkZXZpY2UiOiIxMjM0LTQ1NjUzLTM0MzQ1MyIsImlhdCI6MTUzNjg4NDY5MSwiZXhwIjoxNTY4NDQyMjkxfQ.5wypdKin5Q-gsi9aQ8sN1XBAP8bt3tNBT1WiIttm3qM"}

On approval, the device would save the token in a secure way and use it when sending or requesting data. Subsequently, when a device gets an 'Access Denied' response, it should send a new request for access to get a new token.

In the same vane, this PR also provides a way for users to request access to the server.

The login page now shows a "Sign Up" link:

The user will then be presented with a page to enter an email address and password:

A notification is sent out when this happens and In the admin ui, there is a list of users that are requesting access:

Example of a client that could allow users to easily approve requests:

[inspirity]

Great idea! How would you handle the metadata info for the requesting device?
Is there a way for the new device to populate basic info or should this be done manually?

[sbender9]

Not sure exactly what your asking, but I have an open PR in the spec for meta data : SignalK/specification#454

And a PR on node server: #481

Is this the "metadata info" you are referring to?

[inspirity]

yes exactly, with the meta delta a new device could populate some of the meta after approval.

[tkurki]

What if the initial connection request had a way to include more info about the requester? Now there is just one string, how about including something like meta deltas?

[inspirity]

Yes this would help. We should be aware that some devices can have limitations to handle long Strings.
Specially some simple sensors. So there should still be a way to set the meta data in a separate message.

[tkurki]

Yep, meta deltas will be available for all devices.

[zubenubi]

What kind of sensors/devices are you imagining? All IP connected?

[sbender9]

Not sure I understand what the user case is here for meta data. Can you give me an example of what a device would supply?

[sbender9]

@zubenubi yes, this is for IP connected devices.

Really could be any kind of device, like a display, engine sensors, wind sensors, etc.

Clients could even use this. So for example, when your buddy gets on your boat, he can open WilhelmSK, it makes an access request, you get a notification on your phone and go give him read-only permission. No need to create a userid/password and then have to type it in the app.

[ba58smith]

@sbender9 Is this intended to replace the current CLI approach of getting a Security Token with signalk-generate-token? Want to make sure I understand the purpose.

[sbender9]

Yes, this removes the need to manually create the token and configure it on the device. The script won't go away, so it can still be done the old way.

[ba58smith]

Yes, this removes the need to manually create the token and configure it on the device. The script won't go away, so it can still be done the old way.

Excellent! That's what I was hoping. Thanks.

[fabdrol]

Looks good, I like this kind of approach. Some thoughts:

• Should requests expire?
• Should a requesting device have the option to expire a pending request?
• From the screenshots (haven't had time to read through the code yet) it seems that a request is fairly simple. What can a device ask for? I.e. to me it doesn't make sense that a device that asks for READ ONLY access is given WRITE access, even if the user grants it. Giving the user the option to set the kind of access (READ ONLY, WRITE or ADMIN) makes sense when granting access to another user, but doesn't make sense when it's a device (e.g. sensor) with specific capabilities IMHO

[sbender9]

@fabdrol

• Yes, requests should expire. I have not done that yet.
• I'm not sure I see the real world use case where the device would need to expire a pending request?
• Hmm. Good point. Currently the just asks for access and the admin decides what to give it. But I think I'll change to give the device the option of requesting specific permissions, in that case the admin would not be able to change it.

[fabdrol]

@sbender9 you'r right, I can't think of one. I guess I was thinking of proper "cleanup", on shutdown or retry, but why bother? It could just do a new request after startup and let server bother with the invalidation of the "old" request. If that expiry is implemented, it's never a problem.

How does the notification -> reply work? I.e. the notification is just a delta message (I assume), which disappears from the tree after the request has been granted, denied or has expired? The reply is just a HTTP request from a client?

[sbender9]

@fabdrol The client gets a uuid in the response when make the access request. It then polls via REST using that uuid to get the response.

[rob42]

I agree we need this functionality but all data needs to be in a signalk message, rather than depending on the url for context, eg

{ "accessRequest": {"uuid":"1234-45653-343453","description": "My Awesome Sensor"}}

The reason is simple, how do I make an access request (using a url) if I have a device on some other form of connection (serial, MQTT, STOMP, COAP)?

We need to ensure everything in signalk can be done without depending/assuming http/url/webbrowser

[sbender9]

@rob42 I agree that we need to support other transports, but I think for a REST interface, it should look like normal REST calls. You would not have "accessRequest", in the REST interface. Would you agree?

I think I'll go ahead and do a WS implementation of this. In that case I would do as you said, and then that could be used for other transports as well.

[rob42]

Agreed, the REST interface already 'knows' the context of the data.

This is also consistent with the other REST interfaces, when we GET .../navigation/position it just returns the lat/lon object, not the full tree.

In the artemis-server case the REST interface is a facade for the signalk message, it would just construct the full message and send into the signalk server anyway, and await and return the response.

[rob42]

It would be good to align the message format with the current norm for updates/GET/PUT/LIST messages
See https://github.com/SignalK/specification/wiki/Get,-Put-and-List-messages

I think we should have more tags at the context level, eg login, password, token etc, to allow security, and add access (? needs a better name), so

{ 
    "context": "vessels.self",
    "token": "gsgggsggsggsgsggsgsggsgsggsgsgsggsggsgsg",
    "access": {
         " request": {"uuid":"1234-45653-343453","description": "My Awesome Sensor"}
          "status": ....
          ....
       }
}

[sbender9]

@rob42 I've added a section on doing this via WS, can you review?

[rob42]

I had a look but Im not familiar with the code or the libs used so I cant make much of it Im afraid. Sorry I dont have the time to study up on them at the moment. We really need to make a schema change to allow a set of new keys to the delta format to allow this and other tasks but I'm buried next few days

[sbender9]

I just want you to look the at the ws messages. I can do a PR to the spec once this is ironed out.

[rob42]

sorry for being dense, I cant see any ws samples?

[sbender9]

It's in the PR description above, search for "Requests can also be made via web sock or other protocols"

[rob42]

When using stateless async messaging (eg ws, MQTT, etc) there is no way for the client to correlate a reply message to a previously sent request unless the messaging system uses a correlation id. For our case the best solution is to add a generated uuid to the message, since that avoids having to work with different correlation systems on different transports.

Hence the reply will have to have an identifier from the request. If that is "uuid": "1234-1245-2345-5555", simply including it in the replies would suffice.

In the example above its not clear (due to formatting) that the issued token should be inside the access object so it does not get confused with any possible token key in the header. Just for clarity it should look like this.

{
  "context":"vessels.urn:mrn:imo:mmsi:338184312",
  "access": {
    "uuid": "1234-1245-2345-5555", 
    "status":"APPROVED",
    "token":"eyJhbGci........vOJof0_10bzZICOfiFw47hEjQ"
  }
}

[rob42]

You might also add a WHITE_LIST, BLACK_LIST state so that devices dont keep asking every reboot, or rogue devices keep trying to get in. Dont really want notification DOS attack on the helm :-)
Accepting devices to the WHITE_LIST would auto-approve them next time, blacklist auto-reject, the server would need to monitor the replies from the helm.

[tkurki]

What is the role of context in the ws request? What values could it have in request/response? What should/could the client do with the context value?

Device access not allowed is not a good example of an ERROR, as that should result in DENIED.

The messages are not symmetric, request is access.request but response is just access. What do you guys think of just accessRequest and accessResponse?

If the client drops the ws connection before receiving confirmation how does it get the response if the request was granted/denied while it was not connected? What if the client never sees

I updated the messages in the description to include the uuid for request-response collation. This makes the uuid a client issued request id instead of a client id, which was probably what Scott suggested originally. Is this the way it should be? In REST things are different, as the server issues the request id. I think it would be better to not use the name uuid but something semantic like clientId and requestId, the value being uuid is sort of an implementation detail.

I think it would make sense to start working on this as a PR to specification in parallel with the implementation, if for nothing else than gaining Github versioning and diffing for the spec addition. Appendix or a new chapter? Lest the specification never happens...

BTW I would be happy with just getting this defined for REST, ws adds quite a bit of complication in spec and implementation.

[rob42]

The current delta format needs extension to include at least token in the header. We are all familiar with the format, and have code to create/process it, so it seems simplest to add an access object, and use the format for more than just deltas.

We could add an authenticate object and handle the login/passwd stuff too.

[rob42]

When giving permissions to a device the optimal way to do it is to provide RW based on keys, and using wildcards, just as we do via subscribe etc. So a device will request READ for a list of keys, and WRITE for another list. Bit like android permissions, you can grant/deny each.

Probably there would be defined 'profiles' to save a lot of detail, eg profile.anemometer etc.

I think we should do the definition of that later (a lot of work), after the basics are settled, but the design should be compatible with that concept.