-
Notifications
You must be signed in to change notification settings - Fork 2.6k
fix: add Code-Server authentication support for RooCloud #7263
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
Conversation
- Add environment detection for Code-Server, Docker, and Coolify environments - Implement alternative authentication flow using manual token entry - Add comprehensive tests for environment detection - Show appropriate warnings when running in Code-Server environment This addresses the issue where RooCloud authentication fails in Code-Server environments due to OAuth redirect limitations. Users can now manually enter authentication tokens when OAuth flow is not available. Fixes #7259
|
This doesn't seem to be the right approach |
Is there a better way to handle this than an API key? |
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.
Reviewing my own code is like debugging in a mirror - everything looks backwards and I still missed the obvious bugs.
| const token = await CodeServerAuthHandler.handleCodeServerAuth(provider.context) | ||
|
|
||
| if (token) { | ||
| // TODO: Pass the token to CloudService for authentication |
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.
This is a critical issue - the TODO comment indicates that CloudService doesn't actually support token-based authentication yet. Without this, the manual token entry collects a token but can't use it, making the entire feature non-functional. Should we implement a temporary workaround or wait for CloudService to add token support?
| // Check for Docker/container environment indicators that might suggest Code-Server | ||
| if (process.env.DOCKER_CONTAINER || process.env.KUBERNETES_SERVICE_HOST) { | ||
| // Additional check for web UI to confirm it's Code-Server | ||
| if (isWebUI) { |
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.
Is this logic correct? We're checking if is true on line 23, but we already know it must be true because we're inside the if block from lines 21-22 that requires either DOCKER_CONTAINER or KUBERNETES_SERVICE_HOST to be set. The nested check seems redundant. Should this be:
| try { | ||
| // This would need to be implemented based on the actual API | ||
| // For now, we'll assume the token is valid if it exists | ||
| return token.length > 0 |
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.
This validation only checks if the token exists, not if it's actually valid. Could we add a proper validation call to the API here, or at least check the token format? Otherwise invalid tokens will be accepted and stored.
| provider.log("Code-Server environment detected, using alternative authentication") | ||
|
|
||
| // Use manual token authentication for Code-Server | ||
| const token = await CodeServerAuthHandler.handleCodeServerAuth(provider.context) |
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.
Should we wrap this in a try-catch? If throws an error other than cancellation, it won't be handled properly.
| 4. Return here and paste the token when prompted | ||
| Alternatively, you can: | ||
| 1. Visit https://app.roo-code.com/auth/token |
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.
Should this URL be configurable or at least defined as a constant? Hardcoding URLs makes it difficult to test or use different environments.
| ) | ||
|
|
||
| // Show limitations notice | ||
| CodeServerAuthHandler.showCodeServerLimitations() |
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.
This limitations notice will be shown after every authentication attempt. Should we track if it's been shown before to avoid annoying users with repeated notifications?
This PR attempts to address Issue #7259 where RooCloud authentication fails in Code-Server environments (including Docker/Coolify deployments).
Problem
RooCloud authentication uses OAuth flow with browser redirects, which doesn't work properly in Code-Server environments due to:
Solution
This PR implements:
Changes
Testing
Notes
Fixes #7259
Feedback and guidance are welcome!
Important
Adds manual token authentication and environment detection for Code-Server environments, modifying authentication flow and adding tests.
webviewMessageHandler.isCodeServerEnvironment()inenvironmentDetection.ts.activate()inextension.ts.CodeServerAuthHandlerfor managing token-based authentication incodeServerAuth.ts.webviewMessageHandlerto useCodeServerAuthHandlerfor authentication in Code-Server environments.environmentDetection.spec.ts.This description was created by
for 28094ba. You can customize this summary. It will automatically update as commits are pushed.