-
Notifications
You must be signed in to change notification settings - Fork 119
Device code auth support #281
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
Draft
anvilpete
wants to merge
1
commit into
zed-industries:main
Choose a base branch
from
anvilpete:device-code-auth
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+111
−17
Draft
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,9 +2,9 @@ use acp::schema::{ | |
| AgentAuthCapabilities, AgentCapabilities, AuthEnvVar, AuthMethod, AuthMethodAgent, | ||
| AuthMethodEnvVar, AuthMethodId, AuthenticateRequest, AuthenticateResponse, CancelNotification, | ||
| ClientCapabilities, CloseSessionRequest, CloseSessionResponse, Implementation, | ||
| InitializeRequest, InitializeResponse, ListSessionsRequest, ListSessionsResponse, | ||
| InitializeRequest, InitializeResponse, IntoOption, ListSessionsRequest, ListSessionsResponse, | ||
| LoadSessionRequest, LoadSessionResponse, LogoutCapabilities, LogoutRequest, LogoutResponse, | ||
| McpCapabilities, McpServer, McpServerHttp, McpServerStdio, NewSessionRequest, | ||
| McpCapabilities, McpServer, McpServerHttp, McpServerStdio, Meta, NewSessionRequest, | ||
| NewSessionResponse, PromptCapabilities, PromptRequest, PromptResponse, ProtocolVersion, | ||
| SessionCapabilities, SessionCloseCapabilities, SessionId, SessionInfo, SessionListCapabilities, | ||
| SetSessionConfigOptionRequest, SetSessionConfigOptionResponse, SetSessionModeRequest, | ||
|
|
@@ -32,7 +32,7 @@ use std::{ | |
| path::{Path, PathBuf}, | ||
| sync::{Arc, Mutex}, | ||
| }; | ||
| use tracing::{debug, info}; | ||
| use tracing::{debug, error, info}; | ||
| use unicode_segmentation::UnicodeSegmentation; | ||
|
|
||
| use crate::thread::Thread; | ||
|
|
@@ -58,6 +58,7 @@ pub struct CodexAgent { | |
| session_roots: Arc<Mutex<HashMap<SessionId, PathBuf>>>, | ||
| } | ||
|
|
||
| const CODEX_ACP_NAMESPACE: &str = "codex-acp"; | ||
| const SESSION_LIST_PAGE_SIZE: usize = 25; | ||
| const SESSION_TITLE_MAX_GRAPHEMES: usize = 120; | ||
|
|
||
|
|
@@ -133,8 +134,10 @@ impl CodexAgent { | |
| responder, | ||
| cx: ConnectionTo<Client>| { | ||
| let agent = agent.clone(); | ||
| let auth_cx = cx.clone(); | ||
| cx.spawn(async move { | ||
| responder.respond_with_result(agent.authenticate(request).await) | ||
| responder | ||
| .respond_with_result(agent.authenticate(request, auth_cx).await) | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| })?; | ||
| Ok(()) | ||
| } | ||
|
|
@@ -234,7 +237,7 @@ impl CodexAgent { | |
| let agent = agent.clone(); | ||
| cx.spawn(async move { | ||
| if let Err(e) = agent.cancel(notification).await { | ||
| tracing::error!("Error handling cancel: {:?}", e); | ||
| error!("Error handling cancel: {:?}", e); | ||
| } | ||
| Ok(()) | ||
| })?; | ||
|
|
@@ -432,14 +435,18 @@ impl CodexAgent { | |
| } = request; | ||
| debug!("Received initialize request with protocol version {protocol_version:?}",); | ||
| let protocol_version = ProtocolVersion::V1; | ||
| let client_supports_device_code_auth = | ||
| CodexAcpCapabilities::from_meta(client_capabilities.meta.as_ref()) | ||
| .is_some_and(|capabilities| capabilities.device_code_auth); | ||
|
|
||
| *self.client_capabilities.lock().unwrap() = client_capabilities; | ||
|
|
||
| let mut agent_capabilities = AgentCapabilities::new() | ||
| .prompt_capabilities(PromptCapabilities::new().embedded_context(true).image(true)) | ||
| .mcp_capabilities(McpCapabilities::new().http(true)) | ||
| .load_session(true) | ||
| .auth(AgentAuthCapabilities::new().logout(LogoutCapabilities::new())); | ||
| .auth(AgentAuthCapabilities::new().logout(LogoutCapabilities::new())) | ||
| .meta(CodexAcpCapabilities::new().device_code_auth(true)); | ||
|
|
||
| agent_capabilities.session_capabilities = SessionCapabilities::new() | ||
| .close(SessionCloseCapabilities::new()) | ||
|
|
@@ -450,8 +457,7 @@ impl CodexAgent { | |
| CodexAuthMethod::CodexApiKey.into(), | ||
| CodexAuthMethod::OpenAiApiKey.into(), | ||
| ]; | ||
| // Until codex device code auth works, we can't use this in remote ssh projects | ||
| if std::env::var("NO_BROWSER").is_ok() { | ||
| if std::env::var("NO_BROWSER").is_ok() && !client_supports_device_code_auth { | ||
| auth_methods.remove(0); | ||
| } | ||
|
|
||
|
|
@@ -464,6 +470,7 @@ impl CodexAgent { | |
| async fn authenticate( | ||
| &self, | ||
| request: AuthenticateRequest, | ||
| cx: ConnectionTo<Client>, | ||
| ) -> Result<AuthenticateResponse, Error> { | ||
| let auth_method = CodexAuthMethod::try_from(request.method_id)?; | ||
|
|
||
|
|
@@ -483,21 +490,25 @@ impl CodexAgent { | |
|
|
||
| match auth_method { | ||
| CodexAuthMethod::ChatGpt => { | ||
| // Perform browser/device login via codex-rs, then report success/failure to the client. | ||
| let opts = codex_login::ServerOptions::new( | ||
| self.config.codex_home.to_path_buf(), | ||
| codex_login::auth::CLIENT_ID.to_string(), | ||
| None, | ||
| self.config.cli_auth_credentials_store_mode, | ||
| ); | ||
|
|
||
| let server = | ||
| codex_login::run_login_server(opts).map_err(Error::into_internal_error)?; | ||
|
|
||
| server | ||
| .block_until_done() | ||
| .await | ||
| .map_err(Error::into_internal_error)?; | ||
| if std::env::var("NO_BROWSER").is_ok() { | ||
| return Self::device_code_auth(opts, self.auth_manager.clone(), cx).await; | ||
| } else { | ||
| // Perform browser/device login via codex-rs, then report success/failure to the client. | ||
| let server = | ||
| codex_login::run_login_server(opts).map_err(Error::into_internal_error)?; | ||
|
|
||
| server | ||
| .block_until_done() | ||
| .await | ||
| .map_err(Error::into_internal_error)?; | ||
| } | ||
| } | ||
| CodexAuthMethod::CodexApiKey => { | ||
| let api_key = read_codex_api_key_from_env().ok_or_else(|| { | ||
|
|
@@ -528,6 +539,41 @@ impl CodexAgent { | |
| Ok(AuthenticateResponse::new()) | ||
| } | ||
|
|
||
| async fn device_code_auth( | ||
| opts: codex_login::ServerOptions, | ||
| auth_manager: Arc<AuthManager>, | ||
| cx: ConnectionTo<Client>, | ||
| ) -> Result<AuthenticateResponse, Error> { | ||
| let device_code = codex_login::request_device_code(&opts) | ||
| .await | ||
| .map_err(Error::into_internal_error)?; | ||
|
|
||
| let url = device_code.verification_url.clone(); | ||
| let user_code = device_code.user_code.clone(); | ||
|
|
||
| // Complete device code auth in the background. | ||
| cx.spawn(async move { | ||
| if let Err(err) = codex_login::complete_device_code_login(opts, device_code).await { | ||
| error!("Device code auth failed: {err:?}"); | ||
| } else { | ||
| auth_manager.reload().await; | ||
| } | ||
| Ok(()) | ||
| })?; | ||
|
|
||
| let message = format!( | ||
| "Follow these steps to sign in with ChatGPT using device code authorization:\n\ | ||
| \n1. Open this link in your browser and sign in to your account\n {url}\n\ | ||
| \n2. Enter this one-time code (expires in 15 minutes)\n {user_code}\n\ | ||
| \nDevice codes are a common phishing target. Never share this code.\n" | ||
| ); | ||
| Err(Error::auth_required().data(DeviceCodeAuthRequiredData { | ||
| message, | ||
| url, | ||
| user_code, | ||
| })) | ||
| } | ||
|
|
||
| async fn logout(&self, _request: LogoutRequest) -> Result<LogoutResponse, Error> { | ||
| self.auth_manager | ||
| .logout() | ||
|
|
@@ -873,6 +919,52 @@ impl TryFrom<AuthMethodId> for CodexAuthMethod { | |
| } | ||
| } | ||
|
|
||
| #[derive(Debug, serde::Serialize)] | ||
| #[serde(rename_all = "camelCase")] | ||
| struct DeviceCodeAuthRequiredData { | ||
| message: String, | ||
| url: String, | ||
| user_code: String, | ||
| } | ||
|
|
||
| impl IntoOption<serde_json::Value> for DeviceCodeAuthRequiredData { | ||
| fn into_option(self) -> Option<serde_json::Value> { | ||
| Some(serde_json::to_value(self).expect("DeviceCodeAuthRequiredData should serialize")) | ||
| } | ||
| } | ||
|
|
||
| #[derive(Default, Debug, Clone, serde::Deserialize, serde::Serialize)] | ||
| #[serde(rename_all = "camelCase")] | ||
| struct CodexAcpCapabilities { | ||
| #[serde(default)] | ||
| device_code_auth: bool, | ||
| } | ||
|
|
||
| impl CodexAcpCapabilities { | ||
| fn new() -> Self { | ||
| Self::default() | ||
| } | ||
|
|
||
| fn device_code_auth(mut self, device_code_auth: bool) -> Self { | ||
| self.device_code_auth = device_code_auth; | ||
| self | ||
| } | ||
|
|
||
| fn from_meta(meta: Option<&Meta>) -> Option<Self> { | ||
| meta.and_then(|meta| meta.get(CODEX_ACP_NAMESPACE)) | ||
| .and_then(|value| serde_json::from_value::<Self>(value.clone()).ok()) | ||
| } | ||
| } | ||
|
|
||
| impl IntoOption<Meta> for CodexAcpCapabilities { | ||
| fn into_option(self) -> Option<Meta> { | ||
| Some(serde_json::Map::from_iter([( | ||
| CODEX_ACP_NAMESPACE.to_string(), | ||
| serde_json::to_value(self).expect("CodexAcpCapabilities should serialize"), | ||
| )])) | ||
| } | ||
| } | ||
|
|
||
| fn truncate_graphemes(text: &str, max_graphemes: usize) -> String { | ||
| let mut graphemes = text.grapheme_indices(true); | ||
|
|
||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Paid subscriptions aren't required.. maybe they were at some point in the past?