Repository navigation
[proposed spec change] Mention read caps in Updates-Via - #179
michielbdejong wants to merge 2 commits into
Conversation
|
Looks mostly good to me. I wonder if it's possible/a good idea to also have the server notify the client if it's subscribing to too many updates before it closes the connection? |
|
Yeah, that's probably what I would implement - send an informational error message 'too many subsriptions', then close the connections. thanks! We should discuss this in https://www.w3.org/community/solid/wiki/Meetings#20190530_1000CEST |
| HTTP/1.1 200 OK | ||
| ... | ||
| Updates-Via: wss://example.org/ | ||
| Updates-Via: wss://example.org/?access_token=Ne3jiZ1Mei6Air6iefoh |
There was a problem hiding this comment.
I don't think we want to tie auth into the resource address. What are the other options?
There was a problem hiding this comment.
Is this just an example, or MUST auth be connected to the Updates-Via header like this? This also suggests a high level of coordination between the resource server and the websocket server, which could run on entirely independent systems. In fact, I have a lot of questions about this sort of token structure: are these one-time tokens? how do they expire? are they revokable? are they susceptible to replay attacks?
There was a problem hiding this comment.
This PR just updates the text from using NSS's insecure implementation as an example to using inrupt's pod-server / wac-ldp / websockets-pubsub implementation as the example.
@RubenVerborgh I don't think there are many other options, particularly Authorization headers on the upgrade request aren't a good option, because they can't be set through the web api for WebSockets. If you can think of other/better options, then please join our discussion.
@acoburn I'm currently using the PoP token as-is, so it expires for the WebSocket simply when it expires for the storage server. I think they should have an expiry of like 10 minutes, so that gives some protection against replay attacks.
There was a problem hiding this comment.
@RubenVerborgh I don't think there are many other options
There are, and we should definitely discuss: inrupt-archived/websockets-pubsub#5 (spun off from the discussion you linked to)
| ``` | ||
|
|
||
| If a change occurs and the client is subscribed to that resource, it will | ||
| If a change occurs and the client is subscribed to that resource and has Read access to it, it will |
There was a problem hiding this comment.
I would love that, but I'm afraid this is not how NSS works (and that is a security hole).
There was a problem hiding this comment.
I know, and part of the reason that that wasn't fixed yet is the NSS team didn't have good way to do it, since WebSockets can't have Authorization headers when they are opened from a browser. So this is a proposal to describe a solution in the spec, and once we have a good understanding of how this can work, we should implement the same in NSS.
|
|
||
| The server may send additional informational messages for e.g. error reporting, | ||
| as long as they don't start with `pub`. | ||
| If a client subscribes to too many updates, the server may close the socket. |
There was a problem hiding this comment.
How will the client know that this is the cause for closing?
Not in favor of the current phrasing; too open-ended. If we don't have a more explicit protection mechanism, let's not list a quick workaround.
There was a problem hiding this comment.
If there will be informational messages, then I think we should suggest what the keyword for these would be (e.g. log or info).
There was a problem hiding this comment.
You're right that it would make sense to have such a keyword, but I don't think it's a priority.
The current spec sort of silently allows messages that don't start with pub, that's why I saw an opportunity to make that possibility more explicit here. That's a clarification, not a spec change.
If we want to introduce a new keyword, that would be a spec change, and I think we should postpone all proposed spec changes until we have first fixed two things that are more urgent for Solid app developers. The first one being clarifications (which I see as version 0.8), and then I think the second one would probably be inventing a granular sharing mechanism for apps (which I would see as version 0.9). And then maybe in version 0.10 we could add a spec change like that. Until then, my personal opinion is that such a spec change is not urgent; especially because it's already possible to start your debug messages with log or info (they would be intended to be ignored by the code but seen by the developer).
|
At present there are two keywords involved in this interaction: If this specification defines a |
|
^ sorry, didn't want to downvote your proposal on its merits as such, you're right that there are more beautiful ways to design the protocol we have. But it's what we have, and I like that it's supersimple. The reason I downvoted is just that I think that changing the websockets-pubsub protocol to something different but functionally equivalent (i.e., a 'mostly sideways change') would have a tiny benefit to us as Solid app developers, and a huge cost. |
|
I'm concerned that we are making medium changes to the spec here without sufficiently stepping back and thinking about the design. This has all grown organically; no one has ever considered whether even the Why don't we take a step back, do requirements engineering for the sockets, and then come back with a full solution instead of putting duct tape on duct tape? |
|
It's now implemented in IPS, but no plans (yet) to implement it in NSS. Will leave this as a proposed spec change. |
michielbdejong
added
the
proposed-spec-change
michielbdejong
changed the title
|
Do not merge unless NSS also implements this. |
Uh oh!
There was an error while loading. Please reload this page.