[FTF-468] Several docs changes to highlight token auth over API key auth, as well as more info on using JWTs #3085
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
paddybyers
left a comment
paddybyers
left a comment
There was a problem hiding this comment.
I've commented on much of this but stopped around half way through. I think it needs significant revision to make JWT the primary recommended route. The narrative and examples all need updating to reflect this.
A lot of this is slop - it's been AI-generated and not critically reviewed.
| ```javascript | ||
| // ALWAYS use token authentication in browsers | ||
| const realtime = new Ably.Realtime({ | ||
| authUrl: '/api/ably-token', |
There was a problem hiding this comment.
Why are we suggesting authURL instead of callback here? Does it not need at least some comment?
| ## Server auth endpoint template | ||
| Your server needs an endpoint that creates tokens. Here's a minimal example: |
There was a problem hiding this comment.
Why is this using native tokens, instead of generating a JWT?
| ``` | ||
| </Code> | ||
| ## Security comparison |
There was a problem hiding this comment.
I'm not convinced this section adds anything. It feels like slop.
There was a problem hiding this comment.
really a duplciate of https://ably-docs-authchanges-py2loc0j.herokuapp.com/docs/auth#selecting-auth. I do thinkg repetition is good at times, but why here?
| - [Token authentication reference](/docs/auth/token) - Full token auth documentation | ||
| - [Capabilities](/docs/auth/capabilities) - Fine-grained permission control | ||
| ## Troubleshooting |
There was a problem hiding this comment.
These are not the only things that could go wrong, or even the most likely things that could go wrong.
Error 40103: Token expired
Not according to https://github.com/ably/ably-common/blob/main/protocol/errors.json#L40
If we're going to retain this section we should also refer to:
40160: incorrect capabilities
80019: error in auth callback/url
40171: no means to renew token
Perhaps we should defer adding this section until we've got the more thorough error code documentation landed?
There was a problem hiding this comment.
Why do we need troubleshooting at all? We have error codes and paths. Do we think people are going to read this and pre-empty troubles?
src/pages/docs/auth/token.mdx
Outdated
|
|
||
| ### TokenRequest <a id="choosing-tokenrequest"/> | ||
|
|
||
| **Best for:** Standard web and mobile applications using Ably SDKs. |
There was a problem hiding this comment.
I don't agree with this. We recommend JWT in all circumstances that they can be used. There are only two situations where you would use native tokens:
- where the capabilities list is so large that a literal token is unworkable;
- where the capabilities or clientId are sensitive in some way, and there is a desired to keep them confidential, even if inspecting the token.
Of these, I'm not sure we should even mention the second one because it's such an unusual case.
| Occupancy generates messages on any client entering/leaving a room, and so increases the number of billable messages sent in a room - as such, it is disabled by default and needs to be [enabled](/docs/chat/rooms#create) when creating or retrieving a room. | ||
| </Aside> | ||
|
|
||
| <Aside data-type='note'> |
There was a problem hiding this comment.
This is irrelevant. I suggest removing it.
| The presence feature of a chat room enables online users to advertise to others that they are online. Use this feature to build online status indicators to display who is online in a chat room. Users can also share additional information such as avatar URLs or custom statuses. | ||
|
|
||
| <Aside data-type='note'> | ||
| **Authentication required:** Presence requires an authenticated client with a `clientId`. Clients without a `clientId` cannot enter the presence set. Use [token authentication](/docs/auth/token) with server-assigned client IDs for production applications. See the [Chat setup guide](/docs/chat/setup#authentication) for examples. |
|
|
||
| Room reactions are ephemeral and not stored or aggregated by Ably. The intention being that they show the overall sentiment of a room at a point in time. | ||
|
|
||
| <Aside data-type='note'> |
There was a problem hiding this comment.
This is incorrect advice. It should state instead that a client needs to be identified in order to use certain reaction types. It does not imply the use of token auth.
src/pages/docs/chat/rooms/typing.mdx
Outdated
| Typing indicators enable you to display which users are currently writing a message in a room. This feature can be used to display a message such as *Sandi is typing...*, or when a certain threshold is reached you could instead display *Multiple people are typing...* or *12 people are typing...*. Typing events are emitted whenever a user starts or stops typing. | ||
|
|
||
| <Aside data-type='note'> | ||
| **Authentication required:** Typing indicators display the `clientId` of users who are typing. Use [token authentication](/docs/auth/token) to ensure these identities are trustworthy. See the [Chat setup guide](/docs/chat/setup#authentication) for examples. |
There was a problem hiding this comment.
I don't understand why this comment is here. Doesn't the chat SDK ensure that a client is always identified?
There was a problem hiding this comment.
Yes it does, this is needless information as they cannot connect without a clientId
|
|
||
| Your server needs an endpoint that: | ||
| 1. Verifies the user is authenticated (check session, JWT, etc.) | ||
| 2. Creates an Ably TokenRequest with the user's identity |
There was a problem hiding this comment.
Again, we should be recommending JWT auth.
mattheworiordan
left a comment
mattheworiordan
left a comment
There was a problem hiding this comment.
Thanks for submitting. Good to see you take a stab at dealing with the LLM auth issues, but I think this PR needs a lot more human review by yourself before it's sent for others to review. Lots of obvious problems as it appears to have been pumped out directly from an LLM, which is great to move quickly, but thiks is our docs, and is representative of whether we care to customers.
|
|
||
| ## Approach 1: Separate Ably JWT <a id="separate-jwt"/> | ||
|
|
||
| Your existing auth flow validates the user, then you create an Ably JWT using any JWT library. No Ably SDK is required on your server. |
There was a problem hiding this comment.
I am not sure this is what we'd recommend, always sending both tokens instead of just the one you need? Why would a single endpoint be used for Ably and app auth, when both only need one of the two JWTs?
| authCallback: async (tokenParams, callback) => { | ||
| // On token refresh, fetch new Ably JWT | ||
| try { | ||
| const response = await fetch('/auth/ably-token', { |
There was a problem hiding this comment.
This URL doesn't match the server example you have above
| // On token refresh, fetch new Ably JWT | ||
| try { | ||
| const response = await fetch('/auth/ably-token', { | ||
| headers: { 'Authorization': `Bearer ${appToken}` }, |
There was a problem hiding this comment.
This approach is broken in that a new app token cannot be generated as it's cached at the time of login, instead of obtaining one at the time it's needed
| 'x-ably-token': ablyToken.token, | ||
| }, | ||
| process.env.APP_JWT_SECRET, | ||
| { expiresIn: '1h' } // Must not exceed Ably token expiry |
There was a problem hiding this comment.
If that's teh case, why are you not specifying a TTL when issuing the token?
| ``` | ||
| </Code> | ||
|
|
||
| ## Integrating with AWS Cognito <a id="cognito"/> |
There was a problem hiding this comment.
I am not sure we should be just listing out integrations with random providers like this. It doesn't really flow, at the start we're talking about issuing JWTs ,then we are showing random integrations where with this one we're just showing how to issue a token with Lambda, and then we move onto token renewal. It doesn' feel well thought through, why this order?
| If your `authUrl` endpoint is slow or unreliable, tokens may expire before refresh completes. Ensure your auth endpoint responds quickly (under 5 seconds) and is highly available. | ||
| ## Next steps |
There was a problem hiding this comment.
Why have you introduced Next Steps? We've not used this in the docs.
| The Ably SDK automatically calls your `authCallback` before the current token expires, ensuring seamless token rotation. | ||
| ## Next steps |
There was a problem hiding this comment.
Why have you introduced Next Steps? We've not used this in the docs.
| If your `authUrl` endpoint is slow or unreliable, tokens may expire before refresh completes. Ensure your auth endpoint responds quickly (under 5 seconds) and is highly available. | ||
| ## Next steps |
There was a problem hiding this comment.
Why have you introduced Next Steps? We've not used this in the docs.
src/pages/docs/chat/rooms/typing.mdx
Outdated
| Typing indicators enable you to display which users are currently writing a message in a room. This feature can be used to display a message such as *Sandi is typing...*, or when a certain threshold is reached you could instead display *Multiple people are typing...* or *12 people are typing...*. Typing events are emitted whenever a user starts or stops typing. | ||
|
|
||
| <Aside data-type='note'> | ||
| **Authentication required:** Typing indicators display the `clientId` of users who are typing. Use [token authentication](/docs/auth/token) to ensure these identities are trustworthy. See the [Chat setup guide](/docs/chat/setup#authentication) for examples. |
There was a problem hiding this comment.
Yes it does, this is needless information as they cannot connect without a clientId
src/pages/docs/spaces/avatar.mdx
Outdated
| Subscribe to the `space.members` namespace in order to keep your avatar stack updated in realtime. | ||
|
|
||
| <Aside data-type='note'> | ||
| **Authentication required:** This feature uses the client's `clientId` for identification. Use [token authentication](/docs/auth/token) with server-assigned client IDs to prevent identity spoofing. See the [Spaces setup guide](/docs/spaces/setup#authenticate) for examples. |
There was a problem hiding this comment.
This is not necessary. If this is needed, the problem of which auth mechanism t use is surely solved?
…ll as more info on using JWTs
umair-ably
force-pushed
the
authChanges
branch
from
8d4649e to
4e56d1a
Compare
|
Thanks both @mattheworiordan @paddybyers I should've made it clear I had not yet reviewed the LLM output, so this at the very least should've been a draft only! I mostly put it up to get a gauge on the kind of content we think we were missing and the placement of it. However, your comments have massively helped me in understanding the scope of what needs changing in our auth docs, as well as highlighting my limited understanding of exactly what we're trying to tell and recommend to our customers. I've got a session with Mark and Paddy shortly to understand exactly what our position is re: Auth with Ably, what we're trying to tell our customers, and some historical context of how auth has evolved to the point we seem to have deviations in our docs. I'm trying to solely use LLMs for this, so again, some of these changes are likely subpar as I get a better understanding of the problem later today. For the time-being, I've gone back and forth with Claude to "resolve" (or at least have a stab at resolving) your current comments. I would ignore this for now until I've had a chance to capture and implement the findings from our meeting later today - this is purely to move the needle and help my own understanding from what you've presented in the comments so far. Comments on auth/quick-reference.mdx (12 comments)
Comments on auth/token.mdx (8 comments)
Comments on auth/jwt-integration.mdx (8 comments)
Comments on chat/authentication.mdx (12 comments)
Comments on Chat Room Features (6 comments)
Comments on chat/setup.mdx (1 comment)
Comments on spaces/authentication.mdx & spaces/avatar.mdx (2 comments)
Key Changes Made
|
Description
Comprehensive overhaul of authentication documentation to help developers choose the right auth method and implement it securely.
Key additions:
Changes across 46 files:
Why this matters:
Checklist