Refactored RTN22, added missing details #259
Conversation
sacOO7
commented
Dec 18, 2024
•
sacOO7
commented
- Link to internal conversation -> https://ably-real-time.slack.com/archives/C8SPU4589/p1724236742837859
- Server auth wrt. token timeout is useful for testing and future wrapper SDKs.
- Current use-case [ECO-5171][RTN22] Server initiated auth/ renew token laravel-broadcaster#56
sacOO7
changed the title
sacOO7
changed the title
| @@ -502,7 +502,7 @@ h3(#realtime-connection). Connection | |||
| ** @(RTN7b)@ Every @ProtocolMessage@ that expects an @ACK@ or @NACK@ sent must contain a unique serially incrementing @msgSerial@ integer value starting at zero. The @msgSerial@ along with the @count@ for incoming @ACK@ and @NACK@ @ProtocolMessages@ indicates which messages succeeded or failed to be delivered | |||
| ** @(RTN7c)@ If a connection enters the @SUSPENDED@, @CLOSED@ or @FAILED@ state, or if the connection state is lost, and an @ACK@ or @NACK@ has not yet been received for a message, the client should consider the delivery of those messages as failed, meaning their callback (or language equivalent) should be called with an error representing the reason for the state change, and they should be removed from any @RTN9a@ retry queue | |||
| ** @(RTN7d)@ If the @queueMessages@ client option (@TO3g@) has been set to @false@, then when a connection enters the @DISCONNECTED@ state, any messages which have not yet been @ACK@d should be considered to have failed, with the same effect as in @RTN7c@ | |||
| * @(RTN22)@ Ably can request that a connected client re-authenticates by sending the client an @AUTH@ @ProtocolMessage@. The client must then immediately start a new authentication process as described in "RTC8":#RTC8 | |||
| * @(RTN22)@ Ably can request that a connected client re-authenticates by sending the client an @AUTH@ @ProtocolMessage@. The client must then immediately start a new authentication process as described in "RTC8":#RTC8. Ably sends @AUTH@ @ProtocolMessage@ 30 seconds before the token expires. Note: If the token has a ttl less than 30 seconds, e.g. 5 seconds, ably won't be able to send @AUTH@ @ProtocolMessage@. | |||
There was a problem hiding this comment.
Ably sends
AUTHProtocolMessage30 seconds before the token expires. Note: If the token has a ttl less than 30 seconds, e.g. 5 seconds, ably won't be able to sendAUTHProtocolMessage
The spec specifies SDK behaviour. The added sentences don't have any requirements, they're background information. What are SDK devs expected to do differently as a result of this information?
Like: currently, the server does not guarantee that it sends an AUTH exactly 30 seconds before expiry, nor does it guarantee that it won't send one if you connect with a <30s token. Both things are at the server's discretion, and might change at any point. By putting this in the spec, we are saying that SDK devs should be able to rely on the current behaviour. Is that necessary? If so, for what purpose are they relying on it?
There was a problem hiding this comment.
- I believe this won't affect core SDKs, but currently we added check as a part of laravel token refresh PR
- I had a hunch about value change, so the PR. Btw, few questions regarding this
- How frequently do we change this value? or when was the last time value was changed ?
- Currently,
laravel-echoclient is restricted to use protocol 2 or [email protected], can we still change the value from server side?
There was a problem hiding this comment.
If value can change in the future ( with certain max limit, say 1 minute ), I can update PR accordingly
There was a problem hiding this comment.
How frequently do we change this value? or when was the last time value was changed ?
It's never changed. But that's not really the point; the server has behaviours that it guarantees as part of its protocol and behaviours that are at its discretion, and we should only move something from the latter category to the former if there's a good reason to do so.
I'd like to understand why laravel-broadcaster needs to know how long before expiry it'll get an AUTH. I've looked at the linked PR but I don't have the context to understand what it's doing or why without that it's requesting a token without all the required capabilities. Could you explain?
There was a problem hiding this comment.
Okay current laravel impl. goes as follows
- Each requested token has default 1 hr expiry. Each channel capability can be configured at server side using configure channel capability.
- For each private/presence channel join, internally new jwt token is requested. As a part of request, it sends previous token. Server validates the previous token, checks if it is valid, and has previous private/presence channels. If it has, token is upgraded with the latest requested channel capabilities while maintaining old channels. While token is upgraded, we set new token expiry same as old token. So, after token expires ( default 1 hr expiry ), the client loses access to all channels along with their capabilities.
- As a part of new token request after token expiry, although old token is provided ( since it's expired ), we return token with only
publicchannels access. This means after token expiry, each channel will detach and will re-request token. This will cause presence enter/leave as customer explained -> https://ably-real-time.slack.com/archives/CURL4U2FP/p1733135454672739
There was a problem hiding this comment.
Also, about statement Ably sends @AUTH@ @ProtocolMessage@ 30 seconds before the token expires. Note: If the token has a ttl less than 30 seconds, e.g. 5 seconds, ably won't be able to send @AUTH@ @ProtocolMessage@., This is useful for testing purposes ( mainly integration tests ).
But if you feel, the value can change in the future, we can skip the change or mention that value can change in the future, WDYT
There was a problem hiding this comment.
We can upgrade token expiry to new value every time user joins a new channel, but then there won't be any point of having token expiry
It seems to me that this claim contradicts the change you've made in the PR.
If the point of having tokens expire is that the customer, via their auth server, must approve (i.e. have the ability to disapprove) every user's right to be in each private channel at least once per hour (or whatever period), then surely having the auth server automatically re-approve all permissions if the user's token is about to expire (without asking the customer) defeats that point.
(And if we don't believe that, if we think it's acceptable for the auth server to automatically re-issue the token with all its previous permissions if it's near expiry, then why not extend the expiry every time a new channel is added? But I don't think we do believe this; as you say that would defeat the point of token expiry).
Should the broadcaster not re-ask the customer to confirm permissions (using verifyUserCanAccessChannel) for each non-public channel in the token if the token is near expiry (which would be something like, on any auth request with < 10 minutes to go, so including channel access requests made towards the end, not just 30 seconds), and then it can extend the expiry time without breaking the auth model?
There was a problem hiding this comment.
Okay, I will give a thought about this and will get back 👍
There was a problem hiding this comment.
It seems sensible to verify for each non-public channel if the token is near expiry, before refreshing to new token with upgraded expiry. Will update PR accordingly in the future 👍
There was a problem hiding this comment.
Added TODO for the same ably/laravel-broadcaster#56 (comment)
