Update security doc with full functionality and support for other transports

[rob42]

Updated and completed security doc to reflect a full set of functionality for web and other transports

[fabdrol]

PR looks good to me, but it may make sense to get @tkurki and/or @sbender9 opinion on this too, as they did a lot of work on security in the node server.

Something else I noticed, but not necessarily an issue with this commit:

Early in the document, it is stated that tokens can be of any type: The tokens can be of any type, but are typically JWT tokens.See https://jwt.io/. In the initial authentication requests, a server responds to the login by sending a response that contains the token type and the actual token. Later on, however, there are various examples of request authentication to the server that include a token, but no type (with the exception of the Authorization header approach).

How would a server know, e.g. from a request with the SK_COOKIE set, what kind of token it's looking it? JWT tokens are fairly easy to recognise, but other formats may not be. I realise that this is not such a big issue in practise, as most servers will only implement a single type of token and in most cases the token will come from that server in the first place. However, it seems like an oversight to me.

[fabdrol]

@rob42 @tkurki I have ran into various issues whilst working on https://github.com/fabdrol/signalk-js-client/tree/sdk; mostly to do with the way Node server implements security but also related to my observation in my first comment. I believe these concerns should be addressed, or at least discussed, before pulling this PR.

https://github.com/SignalK/signalk-server-node/issues/715

[rob42]

@fabdrol using 'Bearer' instead of 'JWT' seems much better. Removes the need for token 'type', which should not matter to the client anyway?

[fabdrol]

@rob42 unless a server, for some reason, needs to be able to accept multiple token types (federated ID server integration or whatever). So I’m not against the type lest we implement it across the board (I.e. in every auth method).

@sbender9 what do you reckon? Remove everywhere for now and go with default Bearer?

[sbender9]

Maybe we should should drop type completely? We're just making things overly complicated for clients. Let's keep it simple, if the server needs to include type in the token it can. We can just state that the token is a string, there's no need to specify what it looks like.

[sbender9]

Too many unrelated changes in this PR, don't see how we could come to consensus on all of them.

[tkurki]

We can just state that the token is a string, there's no need to specify what it looks like.

+1

[rob42]

Still need to add the new auth messages to the schema.

[rob42]

Are we ready to merge?

[sbender9]

My only issue is the requirement to include the token in every websocket message.

[rob42]

I removed that for ws.

[tkurki]

@rob42 the token type change is not backwards compatible with the security spec in 1.3.0. I think it should be rewritten so that it is.

[fabdrol]

@rob42 there are some more changes required here. Could you have a look and update?

[fabdrol]

Looks like all changes have been made, except the GET thing? Is it correct to remove the type of token and default to Bearer - because then I need to modify that in signalk-js-client.

[rob42]

I thought I changed GET to POST? Let me check my commits

[rob42]

Yes its changed. Its in commit d7dd6f0 above

[fabdrol]

Okay, @tkurki does this resolve your review? Please comment and merge if yes :)

[tkurki]

Yay, progress!

The schema file still uses expiry and none of the test samples use timeToLive anywhere, so maybe these should be corrected?

[fabdrol]

Looks like it's resolved, is this ready to merge @rob42 ?