feature: define process for devices to request access to a server #505

sbender9 · 2018-09-18T13:01:38Z

TODO:

sync up with feature: define request reponse semantics #508

tkurki · 2018-09-28T18:05:19Z

I am happy with this and would be ready to move forward with it.

fabdrol · 2018-10-01T08:18:53Z

I feel that this should be extended with the kind of access a client can request (i.e. read, read/write or admin - potentially with group modifiers)

tkurki · 2018-10-01T17:15:19Z

The problem that this PR is solving is how do you grant access to a device that has no practical input methods and you can not enter any kind of credentials to it.

I think we end up stalled if we try to specify the kind of access. For example:

node server has ACLs by path, not just a global read/readwrite - how do you specify that?
there is admin in the specification - what does that mean?

There is value in adding this to the spec as it is.

Issues and PRs are easy to open, hard to close.

sbender9 · 2018-10-01T17:57:58Z

I actually did intend this to be used for any kind of device. For example, WilhelmSK now supports requesting access this way.

For example, buddy can come on your boat. Try to connect with WilhelmSK and WilhelmSK can request access. You hit a button, done. No need to fuss with typing in username and password.

tkurki · 2018-10-01T18:05:37Z

Is this the my crew can hardly write use case?

fabdrol · 2018-10-01T20:13:03Z

What about this situation: I develop a native Signal K instrument. The instrument can display a certain number of values (i.e. paths), and needs only read access. The access request will be build into the software, i.e. hard-coded. I believe it makes sense if my screen only requests read access for the paths I need.

sbender9 · 2018-10-02T00:09:39Z

What about this situation: I develop a native Signal K instrument. The instrument can display a certain number of values (i.e. paths), and needs only read access. The access request will be build into the software, i.e. hard-coded. I believe it makes sense if my screen only requests read access for the paths I need.

My thought was that we could add this in the future. We currently don't have anything spec a providing path level permissions. Once we have that we could add something here.

fabdrol · 2018-10-02T07:02:27Z

My thought was that we could add this in the future. We currently don't have anything spec a providing path level permissions. Once we have that we could add something here

Alright, that makes sense. No more comments then ;)

Wrapped the text at 119 columns which makes for nicer diffs later and easier reading in a terminal now. Also, replaced some quoted text with bold and cleaned up the code examples.

timmathews

This looks really good. I'm not quite sure I understand the need for a result code in responses since that exists in the HTTP header. If it accomplishes something the header cannot, shouldn't it be in all of the responses?

timmathews · 2018-10-03T15:36:44Z

gitbook-docs/access_requests.md

+
+## Definitions
+
+* `clientId` is a string that uniquely identifies a device. This could be a UUID, a combination of model number and


We should require that client IDs be UUIDs and specify that the UUID will be converted to a Signal K URN. Or else require that the client provide them as such. e.g. urn:mrn:signalk:uuid:705f5f1a-efaf-44aa-9cb8-a0fd6305567c. If the client is responsible for generating the ID, UUID provides stronger uniqueness guarantees than letting manufacturers arbitrarily decide what identifier is unique enough.

I'm good with that. They could put model, serial, etc. in the description.

gitbook-docs/access_requests.md

timmathews · 2018-10-03T15:42:39Z

gitbook-docs/access_requests.md

+$ curl -k https://localhost:3443/signalk/v1/access/requests/358b5f32-76bf-4b33-8b23-10a330827185
+{
+  "state": "COMPLETED",
+  "result": 200,


If access is denied, shouldn't the result code be 401?

I'm a little conflicted on this.

That could mean that they didn't have permission to make the request in the first place.

The request for access was carried out successfully. It's just that the admin denied it.

I think 200 is correct here.

I'm not sure I see a difference between "the admin denied the request" and "a security policy/configuration denied the request".

Lets define result as the outcome of the request/response layer, and state the outcome of the requested action, eg getting access.
So result=200 means the request was received/processed successfully, the outcome of the request was 'DENIED'

The definition of state is in the generic request/response document and I don't think DENIED would apply to any other responses. Thus it is in the response specific data.

sbender9 · 2018-10-17T18:12:05Z

@timmathews can you please review my responses above and let me know if this needs any more changes.

timmathews · 2018-10-21T15:55:01Z

The only thing we need to resolve for this, IMO is the mechanism for repeated access requests. And perhaps also, what they should do if access was previously granted and then revoked. That could happen when a client device is moved to another server (e.g. the device is sold or the server is replaced) or when someone manually revokes the grant.

tkurki · 2018-10-28T07:16:30Z

@timmathews please elaborate what problems you foresee (see my comment inline above)

As your comment sounds like possible additions I feel we can merge this as is and add more specific instructions if needed.

feature: device process for devices to request access to a server

58f224e

sbender9 changed the title ~~feature: device process for devices to request access to a server~~ feature: define process for devices to request access to a server Sep 18, 2018

sbender9 mentioned this pull request Sep 18, 2018

feature: implement device access and new security spec SignalK/signalk-server#620

Merged

8 tasks

sbender9 changed the title ~~feature: define process for devices to request access to a server~~ [WIP] feature: define process for devices to request access to a server Sep 18, 2018

sbender9 added 2 commits September 18, 2018 09:08

docs: remove references to new user signup and node server specifics

3983d78

docs: add an example of an ERROR response

ff9b4c8

rob42 mentioned this pull request Sep 18, 2018

feature: define Signal K security protocols #506

Merged

2 tasks

sbender9 mentioned this pull request Sep 20, 2018

feature: PUT Requests #507

Merged

1 task

docs: sync up with request/response PR

f899d0c

sbender9 changed the title ~~[WIP] feature: define process for devices to request access to a server~~ feature: define process for devices to request access to a server Sep 20, 2018

tkurki added 3 commits September 22, 2018 21:16

minor tweaks, add 202 not implemented

52c7f8a

doc: fix typo

7cd674b

Minor changes

c303606

sbender9 added 2 commits September 28, 2018 14:26

docs: sync up with #508 and add link to it

076814d

docs: put request specific info under accessRequest object

0ce50b7

Tim Mathews added 3 commits October 3, 2018 09:35

Spelling

ee634bd

Formatting (Mostly)

7e80470

Wrapped the text at 119 columns which makes for nicer diffs later and easier reading in a terminal now. Also, replaced some quoted text with bold and cleaned up the code examples.

Add missing quotes

68c8c57

timmathews reviewed Oct 3, 2018

View reviewed changes

sbender9 added 3 commits October 3, 2018 14:13

remove result from http responses

a2e23c6

put result back in

f667b73

result updates

8b325a8

sbender9 added 3 commits October 3, 2018 18:00

add state=PENDING to the initial response to the request

bf2da4e

specify that clientId must be a UUID

57d7af0

Rename result to statusCode

a474c0b

Define how a device should handle subsequent 403 errors

db7e785

tkurki merged commit 7aacc42 into master Oct 29, 2018

tkurki deleted the acess-requests branch October 29, 2018 18:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feature: define process for devices to request access to a server #505

feature: define process for devices to request access to a server #505

sbender9 commented Sep 18, 2018 •

edited

tkurki commented Sep 28, 2018

fabdrol commented Oct 1, 2018

tkurki commented Oct 1, 2018 •

edited

sbender9 commented Oct 1, 2018

tkurki commented Oct 1, 2018

fabdrol commented Oct 1, 2018

sbender9 commented Oct 2, 2018

fabdrol commented Oct 2, 2018

timmathews left a comment

timmathews Oct 3, 2018

sbender9 Oct 3, 2018

timmathews Oct 3, 2018

sbender9 Oct 3, 2018

tkurki Oct 3, 2018

timmathews Oct 3, 2018

rob42 Oct 3, 2018

sbender9 Oct 3, 2018

sbender9 commented Oct 17, 2018

timmathews commented Oct 21, 2018

tkurki commented Oct 28, 2018


		## Definitions

		* `clientId` is a string that uniquely identifies a device. This could be a UUID, a combination of model number and

feature: define process for devices to request access to a server #505

feature: define process for devices to request access to a server #505

Conversation

sbender9 commented Sep 18, 2018 • edited

tkurki commented Sep 28, 2018

fabdrol commented Oct 1, 2018

tkurki commented Oct 1, 2018 • edited

sbender9 commented Oct 1, 2018

tkurki commented Oct 1, 2018

fabdrol commented Oct 1, 2018

sbender9 commented Oct 2, 2018

fabdrol commented Oct 2, 2018

timmathews left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sbender9 commented Oct 17, 2018

timmathews commented Oct 21, 2018

tkurki commented Oct 28, 2018

sbender9 commented Sep 18, 2018 •

edited

tkurki commented Oct 1, 2018 •

edited