-
Notifications
You must be signed in to change notification settings - Fork 5.4k
feat: Socketio server add auth token #6144
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Socketio server add auth token #6144
Conversation
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. |
|
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 ✨ Finishing touches🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. 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 |
standujar
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello @nguyennk92 thanks for your contribution ! Could please re-use ELIZA_SERVER_AUTH_TOKEN instead of creating another one ? Thanks a lot !
Sure, but I'm a little bit worry about backward compatibility if we re-use ELIZA_SERVER_AUTH_TOKEN, it would break old code as they are not expecting to have to auth socketio connection on setting ELIZA_SERVER_AUTH_TOKEN. What's your opinion about this? |
|
@nguyennk92 I’ll let others share their thoughts. |
|
I'm adding this on #6107 too. |
4052d82 to
1d8ca86
Compare
|
@standujar I updated the env to reuse ELIZA_SERVER_AUTH_TOKEN as well as the description |
|
Hi @nguyennk92 ! Thanks. I think that we can close this one since #6107 will handle more complexe security level. Including this. |
|
@standujar Sure, that's great to hear |
Risks
Low
Background
What does this PR do?
Authenticate socketio connection using ELIZA_SERVER_AUTH_TOKEN
What kind of change is this?
Improvements (misc. changes to existing features)
Why are we doing this? Any context or related work?
Add a layer of security
Backward compatibility
This breaks older clients as they were not expecting SocketIO authentication. Doc will need to be updated as they only mention API authen with X-API-KEY header