diff --git a/crates/goose-cli/src/commands/configure.rs b/crates/goose-cli/src/commands/configure.rs index b0c4da536..ed0c20a29 100644 --- a/crates/goose-cli/src/commands/configure.rs +++ b/crates/goose-cli/src/commands/configure.rs @@ -1321,6 +1321,9 @@ fn configure_streamable_http_extension() -> anyhow::Result<()> { description, timeout: Some(timeout), socket: None, + client_id: None, + client_secret_key: None, + scopes: vec![], bundled: None, available_tools: Vec::new(), }, diff --git a/crates/goose-cli/src/recipes/secret_discovery.rs b/crates/goose-cli/src/recipes/secret_discovery.rs index 29a102ee3..56f20a47a 100644 --- a/crates/goose-cli/src/recipes/secret_discovery.rs +++ b/crates/goose-cli/src/recipes/secret_discovery.rs @@ -51,13 +51,18 @@ fn extract_secrets_from_extensions( let mut secrets = Vec::new(); for ext in extensions { - let (extension_name, env_keys) = match ext { - ExtensionConfig::Stdio { name, env_keys, .. } => (name, env_keys), - ExtensionConfig::StreamableHttp { name, env_keys, .. } => (name, env_keys), - ExtensionConfig::Builtin { name, .. } => (name, &Vec::new()), - ExtensionConfig::Platform { name, .. } => (name, &Vec::new()), - ExtensionConfig::Frontend { name, .. } => (name, &Vec::new()), - ExtensionConfig::InlinePython { name, .. } => (name, &Vec::new()), + let (extension_name, env_keys, client_secret_key) = match ext { + ExtensionConfig::Stdio { name, env_keys, .. } => (name, env_keys, None), + ExtensionConfig::StreamableHttp { + name, + env_keys, + client_secret_key, + .. + } => (name, env_keys, client_secret_key.as_ref()), + ExtensionConfig::Builtin { name, .. } => (name, &Vec::new(), None), + ExtensionConfig::Platform { name, .. } => (name, &Vec::new(), None), + ExtensionConfig::Frontend { name, .. } => (name, &Vec::new(), None), + ExtensionConfig::InlinePython { name, .. } => (name, &Vec::new(), None), // SSE is unsupported - skip ExtensionConfig::Sse { name, .. } => { tracing::warn!(name = %name, "SSE is unsupported, skipping"); @@ -65,7 +70,7 @@ fn extract_secrets_from_extensions( } }; - for key in env_keys { + for key in env_keys.iter().chain(client_secret_key) { if seen_keys.insert(key.clone()) { let secret_req = SecretRequirement::new(extension_name.clone(), key.clone()); secrets.push(secret_req); @@ -167,6 +172,9 @@ mod tests { description: "github-mcp".to_string(), timeout: None, socket: None, + client_id: None, + client_secret_key: None, + scopes: vec![], bundled: None, available_tools: Vec::new(), headers: HashMap::new(), @@ -265,6 +273,9 @@ mod tests { description: "service-a".to_string(), timeout: None, socket: None, + client_id: None, + client_secret_key: None, + scopes: vec![], bundled: None, available_tools: Vec::new(), headers: HashMap::new(), @@ -299,6 +310,50 @@ mod tests { assert!(api_key.extension_name == "service-a" || api_key.extension_name == "service-b"); } + #[test] + fn test_discover_recipe_secrets_includes_client_secret_key() { + let recipe = Recipe { + version: "1.0.0".to_string(), + title: "OAuth Recipe".to_string(), + description: "A recipe with a pre-registered OAuth client".to_string(), + instructions: Some("Test instructions".to_string()), + prompt: None, + extensions: Some(vec![ExtensionConfig::StreamableHttp { + name: "oauth-ext".to_string(), + uri: "http://localhost:8080/mcp".to_string(), + envs: Envs::new(HashMap::new()), + env_keys: vec!["API_TOKEN".to_string()], + description: "oauth-ext".to_string(), + timeout: None, + socket: None, + client_id: Some("registered-client".to_string()), + client_secret_key: Some("OAUTH_CLIENT_SECRET".to_string()), + scopes: vec![], + bundled: None, + available_tools: Vec::new(), + headers: HashMap::new(), + }]), + sub_recipes: None, + settings: None, + activities: None, + author: None, + parameters: None, + response: None, + retry: None, + }; + + let secrets = discover_recipe_secrets(&recipe); + let keys: Vec<&str> = secrets.iter().map(|s| s.key.as_str()).collect(); + + assert!(keys.contains(&"API_TOKEN")); + assert!(keys.contains(&"OAUTH_CLIENT_SECRET")); + let client_secret = secrets + .iter() + .find(|s| s.key == "OAUTH_CLIENT_SECRET") + .unwrap(); + assert_eq!(client_secret.extension_name, "oauth-ext"); + } + #[test] fn test_secret_requirement_creation() { let req = SecretRequirement::new("test-ext".to_string(), "API_TOKEN".to_string()); @@ -326,6 +381,9 @@ mod tests { description: "parent-ext".to_string(), timeout: None, socket: None, + client_id: None, + client_secret_key: None, + scopes: vec![], bundled: None, available_tools: Vec::new(), headers: HashMap::new(), diff --git a/crates/goose-cli/src/session/mod.rs b/crates/goose-cli/src/session/mod.rs index a93820917..c5321cb31 100644 --- a/crates/goose-cli/src/session/mod.rs +++ b/crates/goose-cli/src/session/mod.rs @@ -407,6 +407,9 @@ impl CliSession { description: goose::config::DEFAULT_EXTENSION_DESCRIPTION.to_string(), timeout: Some(timeout), socket: None, + client_id: None, + client_secret_key: None, + scopes: vec![], bundled: None, available_tools: Vec::new(), } @@ -3056,6 +3059,9 @@ mod tests { description: goose::config::DEFAULT_EXTENSION_DESCRIPTION.to_string(), timeout: Some(300), socket: None, + client_id: None, + client_secret_key: None, + scopes: vec![], bundled: None, available_tools: vec![], } @@ -3072,6 +3078,9 @@ mod tests { description: goose::config::DEFAULT_EXTENSION_DESCRIPTION.to_string(), timeout: Some(300), socket: None, + client_id: None, + client_secret_key: None, + scopes: vec![], bundled: None, available_tools: vec![], } @@ -3088,6 +3097,9 @@ mod tests { description: goose::config::DEFAULT_EXTENSION_DESCRIPTION.to_string(), timeout: Some(300), socket: None, + client_id: None, + client_secret_key: None, + scopes: vec![], bundled: None, available_tools: vec![], } diff --git a/crates/goose-sdk-types/src/custom_requests.rs b/crates/goose-sdk-types/src/custom_requests.rs index b3911964e..cb800d249 100644 --- a/crates/goose-sdk-types/src/custom_requests.rs +++ b/crates/goose-sdk-types/src/custom_requests.rs @@ -361,7 +361,7 @@ pub enum GooseExtension { available_tools: Option>, }, Mcp { - server: McpServer, + server: Box, #[serde(default, rename = "envKeys", skip_serializing_if = "Vec::is_empty")] env_keys: Vec, #[serde(default, skip_serializing_if = "Option::is_none")] @@ -370,6 +370,19 @@ pub enum GooseExtension { timeout: Option, #[serde(default, skip_serializing_if = "Option::is_none")] socket: Option, + /// Pre-registered OAuth client ID for the server's authorization server. + #[serde(default, rename = "clientId", skip_serializing_if = "Option::is_none")] + client_id: Option, + /// Name of the env/secret key holding the OAuth client secret. + #[serde( + default, + rename = "clientSecretKey", + skip_serializing_if = "Option::is_none" + )] + client_secret_key: Option, + /// OAuth scopes to request with `client_id`. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + scopes: Vec, #[serde(default, skip_serializing_if = "Option::is_none")] bundled: Option, /// Tool allowlist for this extension. Omit this field to allow all tools. diff --git a/crates/goose-sdk-types/src/custom_requests/recipe.rs b/crates/goose-sdk-types/src/custom_requests/recipe.rs index 7c0003fc3..10b9a0e3d 100644 --- a/crates/goose-sdk-types/src/custom_requests/recipe.rs +++ b/crates/goose-sdk-types/src/custom_requests/recipe.rs @@ -223,6 +223,15 @@ pub enum RecipeExtensionDto { timeout: Option, #[serde(default, skip_serializing_if = "Option::is_none")] socket: Option, + /// Pre-registered OAuth client ID for the server's authorization server. + #[serde(default, skip_serializing_if = "Option::is_none")] + client_id: Option, + /// Name of the env/secret key holding the OAuth client secret. + #[serde(default, skip_serializing_if = "Option::is_none")] + client_secret_key: Option, + /// OAuth scopes to request with `client_id`. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + scopes: Vec, #[serde(default, skip_serializing_if = "Option::is_none")] bundled: Option, /// Tool allowlist for this extension. Omit this field to allow all tools. diff --git a/crates/goose/acp-schema.json b/crates/goose/acp-schema.json index 4fcf9c93c..edd57aa70 100644 --- a/crates/goose/acp-schema.json +++ b/crates/goose/acp-schema.json @@ -148,6 +148,27 @@ "null" ] }, + "clientId": { + "type": [ + "string", + "null" + ], + "description": "Pre-registered OAuth client ID for the server's authorization server." + }, + "clientSecretKey": { + "type": [ + "string", + "null" + ], + "description": "Name of the env/secret key holding the OAuth client secret." + }, + "scopes": { + "type": "array", + "items": { + "type": "string" + }, + "description": "OAuth scopes to request with `client_id`." + }, "bundled": { "type": [ "boolean", @@ -3976,6 +3997,27 @@ "null" ] }, + "client_id": { + "type": [ + "string", + "null" + ], + "description": "Pre-registered OAuth client ID for the server's authorization server." + }, + "client_secret_key": { + "type": [ + "string", + "null" + ], + "description": "Name of the env/secret key holding the OAuth client secret." + }, + "scopes": { + "type": "array", + "items": { + "type": "string" + }, + "description": "OAuth scopes to request with `client_id`." + }, "bundled": { "type": [ "boolean", diff --git a/crates/goose/src/acp/provider.rs b/crates/goose/src/acp/provider.rs index 160064e20..b0f53c66f 100644 --- a/crates/goose/src/acp/provider.rs +++ b/crates/goose/src/acp/provider.rs @@ -2915,6 +2915,9 @@ mod tests { headers: HashMap::from([("Authorization".into(), "Bearer ghp_xxxxxxxxxxxx".into())]), timeout: None, socket: None, + client_id: None, + client_secret_key: None, + scopes: vec![], bundled: Some(false), available_tools: vec![], }, @@ -2969,6 +2972,9 @@ mod tests { headers: HashMap::from([("Authorization".into(), "Bearer ghp_xxxxxxxxxxxx".into())]), timeout: None, socket: None, + client_id: None, + client_secret_key: None, + scopes: vec![], bundled: Some(false), available_tools: vec![], }; diff --git a/crates/goose/src/acp/server.rs b/crates/goose/src/acp/server.rs index 3606088c6..f185789e2 100644 --- a/crates/goose/src/acp/server.rs +++ b/crates/goose/src/acp/server.rs @@ -397,6 +397,9 @@ fn mcp_server_to_extension_config(mcp_server: McpServer) -> Result GooseExtension::Mcp { - server: McpServer::Stdio(McpServerStdio::new(name, cmd).args(args.clone())), + server: Box::new(McpServer::Stdio( + McpServerStdio::new(name, cmd).args(args.clone()), + )), env_keys: env_keys.clone(), description: empty_string_to_none(description), timeout: *timeout, socket: None, + client_id: None, + client_secret_key: None, + scopes: vec![], bundled: *bundled, available_tools: available_tools_to_wire(available_tools), }, @@ -194,6 +199,9 @@ fn config_to_goose_extension( headers, timeout, socket, + client_id, + client_secret_key, + scopes, bundled, available_tools, .. @@ -203,11 +211,16 @@ fn config_to_goose_extension( .map(|(key, value)| HttpHeader::new(key, value)) .collect(); GooseExtension::Mcp { - server: McpServer::Http(McpServerHttp::new(name, uri).headers(headers)), + server: Box::new(McpServer::Http( + McpServerHttp::new(name, uri).headers(headers), + )), env_keys: env_keys.clone(), description: empty_string_to_none(description), timeout: *timeout, socket: socket.clone(), + client_id: client_id.clone(), + client_secret_key: client_secret_key.clone(), + scopes: scopes.clone(), bundled: *bundled, available_tools: available_tools_to_wire(available_tools), } @@ -263,14 +276,22 @@ fn goose_extension_to_config( description, timeout, socket, + client_id, + client_secret_key, + scopes, bundled, available_tools, - } => match server { + } => match *server { McpServer::Stdio(stdio) => { if socket.is_some() { return Err(agent_client_protocol::Error::invalid_params() .data("socket is only supported for streamable_http MCP extensions")); } + if client_id.is_some() || client_secret_key.is_some() || !scopes.is_empty() { + return Err(agent_client_protocol::Error::invalid_params().data( + "OAuth client fields are only supported for streamable_http MCP extensions", + )); + } let mut env_keys = env_keys; for env in stdio.env { if !env_keys.contains(&env.name) { @@ -304,6 +325,9 @@ fn goose_extension_to_config( .collect(), timeout, socket, + client_id, + client_secret_key, + scopes, bundled, available_tools: available_tools.unwrap_or_default(), }, @@ -477,6 +501,9 @@ mod tests { description, timeout, socket, + client_id, + client_secret_key, + scopes, bundled, available_tools, } = extension @@ -488,10 +515,13 @@ mod tests { assert_eq!(description.as_deref(), Some("Test stdio")); assert_eq!(timeout, Some(42)); assert_eq!(socket, None); + assert_eq!(client_id, None); + assert_eq!(client_secret_key, None); + assert!(scopes.is_empty()); assert_eq!(bundled, None); assert_eq!(available_tools, Some(vec!["run".to_string()])); - let McpServer::Stdio(stdio) = server else { + let McpServer::Stdio(stdio) = *server else { panic!("expected stdio server"); }; @@ -518,6 +548,9 @@ mod tests { )]), timeout: Some(99), socket: Some("@egress.sock".to_string()), + client_id: Some("registered-client".to_string()), + client_secret_key: Some("OAUTH_CLIENT_SECRET".to_string()), + scopes: vec!["scope.read".to_string()], bundled: None, available_tools: vec!["fetch".to_string()], }; @@ -532,6 +565,9 @@ mod tests { description, timeout, socket, + client_id, + client_secret_key, + scopes, bundled, available_tools, } = extension @@ -543,10 +579,13 @@ mod tests { assert_eq!(description.as_deref(), Some("Test HTTP")); assert_eq!(timeout, Some(99)); assert_eq!(socket.as_deref(), Some("@egress.sock")); + assert_eq!(client_id.as_deref(), Some("registered-client")); + assert_eq!(client_secret_key.as_deref(), Some("OAUTH_CLIENT_SECRET")); + assert_eq!(scopes, vec!["scope.read"]); assert_eq!(bundled, None); assert_eq!(available_tools, Some(vec!["fetch".to_string()])); - let McpServer::Http(http) = server else { + let McpServer::Http(http) = *server else { panic!("expected http server"); }; @@ -618,14 +657,17 @@ mod tests { #[test] fn goose_mcp_stdio_extension_converts_to_config_without_literal_envs() { let extension = GooseExtension::Mcp { - server: McpServer::Stdio( + server: Box::new(McpServer::Stdio( McpServerStdio::new("test-stdio", "test-command") .args(vec!["--flag".to_string(), "value".to_string()]), - ), + )), env_keys: vec!["SECRET_TOKEN".to_string()], description: Some("Test stdio".to_string()), timeout: Some(42), socket: None, + client_id: None, + client_secret_key: None, + scopes: vec![], bundled: Some(true), available_tools: Some(vec!["run".to_string()]), }; @@ -666,17 +708,25 @@ mod tests { #[test] fn goose_mcp_stdio_extension_extracts_literal_envs_for_config_add() { let extension = GooseExtension::Mcp { - server: McpServer::Stdio(McpServerStdio::new("test-stdio", "test-command").env(vec![ - agent_client_protocol::schema::v1::EnvVariable::new( - "SECRET_TOKEN", - "literal-secret", - ), - agent_client_protocol::schema::v1::EnvVariable::new("OTHER_TOKEN", "other-secret"), - ])), + server: Box::new(McpServer::Stdio( + McpServerStdio::new("test-stdio", "test-command").env(vec![ + agent_client_protocol::schema::v1::EnvVariable::new( + "SECRET_TOKEN", + "literal-secret", + ), + agent_client_protocol::schema::v1::EnvVariable::new( + "OTHER_TOKEN", + "other-secret", + ), + ]), + )), env_keys: vec!["SECRET_TOKEN".to_string()], description: Some("Test stdio".to_string()), timeout: Some(42), socket: None, + client_id: None, + client_secret_key: None, + scopes: vec![], bundled: Some(true), available_tools: None, }; @@ -711,15 +761,18 @@ mod tests { #[test] fn goose_mcp_streamable_http_extension_converts_to_config_without_literal_envs() { let extension = GooseExtension::Mcp { - server: McpServer::Http( + server: Box::new(McpServer::Http( McpServerHttp::new("test-http", "https://example.com/mcp").headers(vec![ HttpHeader::new("Authorization", "Bearer ${API_TOKEN}"), ]), - ), + )), env_keys: vec!["API_TOKEN".to_string()], description: Some("Test HTTP".to_string()), timeout: Some(99), socket: Some("@egress.sock".to_string()), + client_id: Some("registered-client".to_string()), + client_secret_key: Some("OAUTH_CLIENT_SECRET".to_string()), + scopes: vec!["scope.read".to_string()], bundled: Some(true), available_tools: Some(vec!["fetch".to_string()]), }; @@ -736,6 +789,9 @@ mod tests { headers, timeout, socket, + client_id, + client_secret_key, + scopes, bundled, available_tools, } = conversion.config @@ -760,6 +816,9 @@ mod tests { ); assert_eq!(timeout, Some(99)); assert_eq!(socket.as_deref(), Some("@egress.sock")); + assert_eq!(client_id.as_deref(), Some("registered-client")); + assert_eq!(client_secret_key.as_deref(), Some("OAUTH_CLIENT_SECRET")); + assert_eq!(scopes, vec!["scope.read"]); assert_eq!(bundled, Some(true)); assert_eq!(available_tools, vec!["fetch"]); } @@ -832,11 +891,17 @@ mod tests { #[test] fn goose_mcp_sse_extension_is_rejected_for_config_add() { let extension = GooseExtension::Mcp { - server: McpServer::Sse(McpServerSse::new("legacy-sse", "https://example.com/sse")), + server: Box::new(McpServer::Sse(McpServerSse::new( + "legacy-sse", + "https://example.com/sse", + ))), env_keys: Vec::new(), description: None, timeout: None, socket: None, + client_id: None, + client_secret_key: None, + scopes: vec![], bundled: None, available_tools: None, }; diff --git a/crates/goose/src/acp/server/recipe/conversions.rs b/crates/goose/src/acp/server/recipe/conversions.rs index 0a41a0594..82b183529 100644 --- a/crates/goose/src/acp/server/recipe/conversions.rs +++ b/crates/goose/src/acp/server/recipe/conversions.rs @@ -344,6 +344,9 @@ impl TryFrom for ExtensionConfig { headers, timeout, socket, + client_id, + client_secret_key, + scopes, bundled, available_tools, } => Self::StreamableHttp { @@ -355,6 +358,9 @@ impl TryFrom for ExtensionConfig { headers, timeout, socket, + client_id, + client_secret_key, + scopes, bundled, available_tools: available_tools.unwrap_or_default(), }, @@ -427,6 +433,9 @@ impl TryFrom for RecipeExtensionDto { headers, timeout, socket, + client_id, + client_secret_key, + scopes, bundled, available_tools, } => Self::StreamableHttp { @@ -438,6 +447,9 @@ impl TryFrom for RecipeExtensionDto { headers, timeout, socket, + client_id, + client_secret_key, + scopes, bundled, available_tools: available_tools_to_wire(available_tools), }, @@ -523,6 +535,9 @@ mod tests { headers: HashMap::from([("X-Test".to_string(), "true".to_string())]), timeout: Some(30), socket: None, + client_id: None, + client_secret_key: None, + scopes: vec![], bundled: Some(false), available_tools: Some(vec!["fetch".to_string()]), }, diff --git a/crates/goose/src/agents/extension.rs b/crates/goose/src/agents/extension.rs index 4f6450cfc..b9c1aa511 100644 --- a/crates/goose/src/agents/extension.rs +++ b/crates/goose/src/agents/extension.rs @@ -249,6 +249,26 @@ pub enum ExtensionConfig { /// Use `@name` for Linux abstract sockets. #[serde(default)] socket: Option, + /// OAuth client ID pre-registered with the server's authorization + /// server. When set, it is used directly for the authorization flow + /// instead of Client ID Metadata Documents or Dynamic Client + /// Registration — required for authorization servers that support + /// neither. Supports `$VAR`/`${VAR}` substitution. + #[serde(default)] + #[serde(skip_serializing_if = "Option::is_none")] + client_id: Option, + /// Name of the env/secret key holding the OAuth client secret paired + /// with `client_id`. The value is resolved from `envs`/`env_keys` or + /// the config secret store — never stored inline. Optional: public + /// clients using PKCE have no secret. + #[serde(default)] + #[serde(skip_serializing_if = "Option::is_none")] + client_secret_key: Option, + /// OAuth scopes to request with `client_id`. When empty, scopes are + /// selected from server metadata, which may be broader than needed. + #[serde(default)] + #[serde(skip_serializing_if = "Vec::is_empty")] + scopes: Vec, #[serde(default)] bundled: Option, #[serde(default)] @@ -323,6 +343,9 @@ impl ExtensionConfig { description: description.into(), timeout: Some(timeout.into()), socket: None, + client_id: None, + client_secret_key: None, + scopes: Vec::new(), bundled: None, available_tools: Vec::new(), } @@ -482,10 +505,22 @@ impl ExtensionConfig { headers, timeout, socket, + client_id, + client_secret_key, + scopes, bundled, available_tools, } => { - let merged = merge_environments(&envs, &env_keys, &name, config).await?; + // Resolve the OAuth client secret alongside env_keys so that + // rotating it changes the resolved config, which is what + // add_extension compares to decide whether to restart. + let mut secret_keys = env_keys; + if let Some(key) = &client_secret_key { + if !secret_keys.contains(key) { + secret_keys.push(key.clone()); + } + } + let merged = merge_environments(&envs, &secret_keys, &name, config).await?; let headers = headers .into_iter() .map(|(k, v)| { @@ -494,6 +529,7 @@ impl ExtensionConfig { }) .collect(); let socket = socket.map(|s| substitute_env_vars(&s, &merged)); + let client_id = client_id.map(|c| substitute_env_vars(&c, &merged)); Ok(Self::StreamableHttp { name, description, @@ -503,6 +539,9 @@ impl ExtensionConfig { headers, timeout, socket, + client_id, + client_secret_key, + scopes, bundled, available_tools, }) @@ -671,6 +710,77 @@ available_tools: [] } } + #[test] + fn test_deserialize_streamable_http_oauth_client_fields() { + let config: ExtensionConfig = serde_yaml::from_str( + "type: streamable_http +name: remote +uri: https://example.com/mcp +client_id: registered-client.example +client_secret_key: OAUTH_CLIENT_SECRET +scopes: + - scope.read + - scope.write +timeout: 300", + ) + .unwrap(); + + let ExtensionConfig::StreamableHttp { + client_id, + client_secret_key, + scopes, + .. + } = config + else { + panic!("expected streamable_http config"); + }; + + assert_eq!(client_id.as_deref(), Some("registered-client.example")); + assert_eq!(client_secret_key.as_deref(), Some("OAUTH_CLIENT_SECRET")); + assert_eq!(scopes, vec!["scope.read", "scope.write"]); + } + + #[test] + fn test_deserialize_streamable_http_without_oauth_client_fields() { + let config: ExtensionConfig = serde_yaml::from_str( + "type: streamable_http +name: remote +uri: https://example.com/mcp +timeout: 300", + ) + .unwrap(); + + let ExtensionConfig::StreamableHttp { + client_id, + client_secret_key, + scopes, + .. + } = config + else { + panic!("expected streamable_http config"); + }; + + assert_eq!(client_id, None); + assert_eq!(client_secret_key, None); + assert!(scopes.is_empty()); + } + + #[test] + fn serialization_omits_unset_oauth_client_fields() { + let config = ExtensionConfig::streamable_http( + "remote", + "https://example.com/mcp", + "remote extension", + 300u64, + ); + + let yaml = serde_yaml::to_string(&config).unwrap(); + + assert!(!yaml.contains("client_id")); + assert!(!yaml.contains("client_secret_key")); + assert!(!yaml.contains("scopes")); + } + #[test] fn envs_deserialization_filters_disallowed_keys() { let envs: extension::Envs = @@ -752,6 +862,9 @@ available_tools: [] .collect(), timeout: None, socket: None, + client_id: None, + client_secret_key: None, + scopes: vec![], bundled: None, available_tools: vec![], }, @@ -773,6 +886,9 @@ available_tools: [] .collect(), timeout: None, socket: None, + client_id: None, + client_secret_key: None, + scopes: vec![], bundled: None, available_tools: vec![], } @@ -851,6 +967,9 @@ available_tools: [] .collect(), timeout: None, socket: None, + client_id: None, + client_secret_key: None, + scopes: vec![], bundled: None, available_tools: vec![], }, @@ -869,6 +988,9 @@ available_tools: [] .collect(), timeout: None, socket: None, + client_id: None, + client_secret_key: None, + scopes: vec![], bundled: None, available_tools: vec![], } @@ -884,6 +1006,9 @@ available_tools: [] headers: std::collections::HashMap::new(), timeout: None, socket: None, + client_id: None, + client_secret_key: None, + scopes: vec![], bundled: None, available_tools: vec![], }, @@ -900,6 +1025,9 @@ available_tools: [] headers: std::collections::HashMap::new(), timeout: None, socket: None, + client_id: None, + client_secret_key: None, + scopes: vec![], bundled: None, available_tools: vec![], } @@ -940,6 +1068,80 @@ available_tools: [] } ; "env_key_skipped_when_already_in_envs" )] + #[test_case( + ExtensionConfig::StreamableHttp { + name: "test".into(), + description: String::new(), + uri: "https://example.com/mcp".into(), + envs: extension::Envs::default(), + env_keys: vec!["MY_SECRET".into()], + headers: std::collections::HashMap::new(), + timeout: None, + socket: None, + client_id: Some("${MY_SECRET}".into()), + client_secret_key: Some("MY_SECRET".into()), + scopes: vec!["scope.read".into()], + bundled: None, + available_tools: vec![], + }, + ExtensionConfig::StreamableHttp { + name: "test".into(), + description: String::new(), + uri: "https://example.com/mcp".into(), + envs: extension::Envs::new({ + let mut m = std::collections::HashMap::new(); + m.insert("MY_SECRET".to_string(), "secret_value".to_string()); + m + }), + env_keys: vec![], + headers: std::collections::HashMap::new(), + timeout: None, + socket: None, + client_id: Some("secret_value".into()), + client_secret_key: Some("MY_SECRET".into()), + scopes: vec!["scope.read".into()], + bundled: None, + available_tools: vec![], + } + ; "http_client_id_substitution_and_oauth_fields_preserved" + )] + #[test_case( + ExtensionConfig::StreamableHttp { + name: "test".into(), + description: String::new(), + uri: "https://example.com/mcp".into(), + envs: extension::Envs::default(), + env_keys: vec![], + headers: std::collections::HashMap::new(), + timeout: None, + socket: None, + client_id: Some("registered-client".into()), + client_secret_key: Some("MY_SECRET".into()), + scopes: vec![], + bundled: None, + available_tools: vec![], + }, + ExtensionConfig::StreamableHttp { + name: "test".into(), + description: String::new(), + uri: "https://example.com/mcp".into(), + envs: extension::Envs::new({ + let mut m = std::collections::HashMap::new(); + m.insert("MY_SECRET".to_string(), "secret_value".to_string()); + m + }), + env_keys: vec![], + headers: std::collections::HashMap::new(), + timeout: None, + socket: None, + client_id: Some("registered-client".into()), + client_secret_key: Some("MY_SECRET".into()), + scopes: vec![], + bundled: None, + available_tools: vec![], + } + ; "http_client_secret_key_resolved_without_env_keys_entry" + )] #[tokio::test] async fn test_resolve(config: ExtensionConfig, expected: ExtensionConfig) { let dir = tempfile::tempdir().unwrap(); @@ -963,6 +1165,9 @@ available_tools: [] headers: std::collections::HashMap::new(), timeout: None, socket: Some("@egress.sock".to_string()), + client_id: None, + client_secret_key: None, + scopes: vec![], bundled: None, available_tools: vec![], }; @@ -983,6 +1188,9 @@ available_tools: [] headers: std::collections::HashMap::new(), timeout: None, socket: None, + client_id: None, + client_secret_key: None, + scopes: vec![], bundled: None, available_tools: vec![], }; diff --git a/crates/goose/src/agents/extension_manager.rs b/crates/goose/src/agents/extension_manager.rs index 6f6fa0c9e..fa18d2b46 100644 --- a/crates/goose/src/agents/extension_manager.rs +++ b/crates/goose/src/agents/extension_manager.rs @@ -45,7 +45,7 @@ use crate::builtin_extension::get_builtin_extension; use crate::config::extensions::name_to_key; use crate::config::search_path::SearchPaths; use crate::config::{get_all_extensions, Config}; -use crate::oauth::{oauth_flow, GooseCredentialStore}; +use crate::oauth::{oauth_flow, GooseCredentialStore, StaticOAuthClientConfig}; use crate::prompt_template; use crate::subprocess::configure_subprocess; use rmcp::model::{ @@ -589,6 +589,67 @@ pub(crate) async fn merge_environments( Ok(Envs::new(all_envs).get_env()) } +/// Build the pre-registered OAuth client config for a streamable_http +/// extension. The secret is referenced by key and resolved from the merged +/// environment or the config secret store, so it is never stored inline in +/// the extension config. +fn resolve_static_oauth_client( + client_id: Option<&str>, + client_secret_key: Option<&str>, + scopes: &[String], + envs: &HashMap, + config: &Config, +) -> Result, Box> { + let Some(client_id) = client_id else { + if client_secret_key.is_some() { + return Err(Box::new(ExtensionError::ConfigError( + "client_secret_key requires client_id".to_string(), + ))); + } + if !scopes.is_empty() { + return Err(Box::new(ExtensionError::ConfigError( + "scopes requires client_id".to_string(), + ))); + } + return Ok(None); + }; + + let client_secret = match client_secret_key { + Some(key) => Some(resolve_secret_value(key, envs, config)?), + None => None, + }; + + Ok(Some(StaticOAuthClientConfig { + client_id: substitute_env_vars(client_id, envs), + client_secret, + scopes: scopes.to_vec(), + })) +} + +fn resolve_secret_value( + key: &str, + envs: &HashMap, + config: &Config, +) -> Result> { + if let Some(value) = envs.get(key) { + return Ok(value.clone()); + } + + let value = config.get(key, true).map_err(|error| { + Box::new(ExtensionError::ConfigError(format!( + "Failed to fetch secret '{}' from config: {}", + key, error + ))) + })?; + + value.as_str().map(str::to_string).ok_or_else(|| { + Box::new(ExtensionError::ConfigError(format!( + "Secret '{}' is not a string", + key + ))) + }) +} + /// Substitute environment variables in a string. Supports both ${VAR} and $VAR syntax. pub(crate) fn substitute_env_vars(value: &str, env_map: &HashMap) -> String { let mut result = value.to_string(); @@ -679,6 +740,7 @@ async fn create_streamable_http_client( headers: &HashMap, name: &str, socket: Option<&str>, + static_oauth_client: Option, credential_store: Box, provider: SharedProvider, client_name: String, @@ -745,7 +807,13 @@ async fn create_streamable_http_client( // If we have stored OAuth credentials, try refreshing and connecting directly. // This avoids the unnecessary 401 → browser re-auth cycle on every new session. if credential_store.load().await.is_ok_and(|c| c.is_some()) { - match oauth_flow(&uri.to_string(), &name.to_string()).await { + match oauth_flow( + &uri.to_string(), + &name.to_string(), + static_oauth_client.as_ref(), + ) + .await + { Ok(auth_manager) => { let auth_result = connect_with_auth( auth_manager, @@ -802,7 +870,13 @@ async fn create_streamable_http_client( .await; if should_attempt_oauth_fallback(&client_res) { - match oauth_flow(&uri.to_string(), &name.to_string()).await { + match oauth_flow( + &uri.to_string(), + &name.to_string(), + static_oauth_client.as_ref(), + ) + .await + { Ok(auth_manager) => { connect_with_auth( auth_manager, @@ -1010,6 +1084,9 @@ impl ExtensionManager { envs, env_keys, socket, + client_id, + client_secret_key, + scopes, .. } => { let config = Config::global(); @@ -1020,12 +1097,21 @@ impl ExtensionManager { .map(|(k, v)| (k.clone(), substitute_env_vars(v, &all_envs))) .collect(); let resolved_socket = socket.as_ref().map(|s| substitute_env_vars(s, &all_envs)); + let static_oauth_client = resolve_static_oauth_client( + client_id.as_deref(), + client_secret_key.as_deref(), + scopes, + &all_envs, + config, + ) + .map_err(|error| *error)?; create_streamable_http_client( &resolved_uri, *timeout, &resolved_headers, name, resolved_socket.as_deref(), + static_oauth_client, Box::new(GooseCredentialStore::new(name.to_string())), self.provider.clone(), self.client_name.clone(), @@ -2185,6 +2271,160 @@ impl ExtensionManager { mod tests { use super::*; use rmcp::model::CallToolResult; + + mod static_oauth_client { + use super::*; + + fn test_config(dir: &tempfile::TempDir) -> Config { + Config::new_with_file_secrets( + dir.path().join("config.yaml"), + dir.path().join("secrets.yaml"), + ) + .unwrap() + } + + #[test] + fn absent_client_id_yields_no_static_client() { + let dir = tempfile::tempdir().unwrap(); + let config = test_config(&dir); + + let resolved = + resolve_static_oauth_client(None, None, &[], &HashMap::new(), &config).unwrap(); + + assert_eq!(resolved, None); + } + + #[test] + fn client_id_without_secret_resolves_public_client() { + let dir = tempfile::tempdir().unwrap(); + let config = test_config(&dir); + + let resolved = resolve_static_oauth_client( + Some("registered-client"), + None, + &["scope.read".to_string()], + &HashMap::new(), + &config, + ) + .unwrap() + .unwrap(); + + assert_eq!(resolved.client_id, "registered-client"); + assert_eq!(resolved.client_secret, None); + assert_eq!(resolved.scopes, vec!["scope.read"]); + } + + #[test] + fn client_id_supports_env_substitution() { + let dir = tempfile::tempdir().unwrap(); + let config = test_config(&dir); + let envs = HashMap::from([( + "OAUTH_CLIENT_ID".to_string(), + "registered-client".to_string(), + )]); + + let resolved = + resolve_static_oauth_client(Some("${OAUTH_CLIENT_ID}"), None, &[], &envs, &config) + .unwrap() + .unwrap(); + + assert_eq!(resolved.client_id, "registered-client"); + } + + #[test] + fn client_secret_resolves_from_envs() { + let dir = tempfile::tempdir().unwrap(); + let config = test_config(&dir); + let envs = HashMap::from([( + "OAUTH_CLIENT_SECRET".to_string(), + "secret-value".to_string(), + )]); + + let resolved = resolve_static_oauth_client( + Some("registered-client"), + Some("OAUTH_CLIENT_SECRET"), + &[], + &envs, + &config, + ) + .unwrap() + .unwrap(); + + assert_eq!(resolved.client_secret.as_deref(), Some("secret-value")); + } + + #[test] + fn client_secret_falls_back_to_config_secret_store() { + let dir = tempfile::tempdir().unwrap(); + let config = test_config(&dir); + config + .set("OAUTH_CLIENT_SECRET", &"stored-secret", true) + .unwrap(); + + let resolved = resolve_static_oauth_client( + Some("registered-client"), + Some("OAUTH_CLIENT_SECRET"), + &[], + &HashMap::new(), + &config, + ) + .unwrap() + .unwrap(); + + assert_eq!(resolved.client_secret.as_deref(), Some("stored-secret")); + } + + #[test] + fn client_secret_key_without_client_id_is_rejected() { + let dir = tempfile::tempdir().unwrap(); + let config = test_config(&dir); + + let error = resolve_static_oauth_client( + None, + Some("OAUTH_CLIENT_SECRET"), + &[], + &HashMap::new(), + &config, + ) + .unwrap_err(); + + assert!(matches!(*error, ExtensionError::ConfigError(_))); + } + + #[test] + fn scopes_without_client_id_are_rejected() { + let dir = tempfile::tempdir().unwrap(); + let config = test_config(&dir); + + let error = resolve_static_oauth_client( + None, + None, + &["scope.read".to_string()], + &HashMap::new(), + &config, + ) + .unwrap_err(); + + assert!(matches!(*error, ExtensionError::ConfigError(_))); + } + + #[test] + fn missing_client_secret_key_is_an_error() { + let dir = tempfile::tempdir().unwrap(); + let config = test_config(&dir); + + let error = resolve_static_oauth_client( + Some("registered-client"), + Some("MISSING_KEY"), + &[], + &HashMap::new(), + &config, + ) + .unwrap_err(); + + assert!(matches!(*error, ExtensionError::ConfigError(_))); + } + } use rmcp::model::{CustomNotification, InitializeResult, JsonObject}; use rmcp::{object, ServiceError as Error}; @@ -3367,6 +3607,7 @@ mod tests { &headers, "test-ext", None, + None, Box::new(rmcp::transport::auth::InMemoryCredentialStore::new()), provider, "goose-test".to_string(), @@ -3404,6 +3645,7 @@ mod tests { &headers, "test-ext", None, + None, Box::new(rmcp::transport::auth::InMemoryCredentialStore::new()), provider, "goose-test".to_string(), @@ -3452,6 +3694,7 @@ mod tests { &headers, "test-ext", None, + None, Box::new(rmcp::transport::auth::InMemoryCredentialStore::new()), provider, "goose-test".to_string(), diff --git a/crates/goose/src/oauth/mod.rs b/crates/goose/src/oauth/mod.rs index 8e5c3289c..3846fdde5 100644 --- a/crates/goose/src/oauth/mod.rs +++ b/crates/goose/src/oauth/mod.rs @@ -7,10 +7,14 @@ use axum::response::Html; use axum::routing::get; use axum::Router; use minijinja::render; -use oauth2::TokenResponse; -use rmcp::transport::auth::{AuthorizationRequest, CredentialStore, OAuthState, StoredCredentials}; +use oauth2::{Scope, TokenResponse}; +use rmcp::transport::auth::{ + AuthError, AuthorizationRequest, CredentialStore, OAuthClientConfig, OAuthState, + OAuthTokenResponse, StoredCredentials, +}; use rmcp::transport::AuthorizationManager; use serde::Deserialize; +use std::collections::BTreeSet; use std::net::SocketAddr; use std::sync::Arc; use std::time::Duration; @@ -83,24 +87,182 @@ async fn wait_for_callback( } } +/// OAuth client credentials registered with the authorization server out of +/// band, for servers whose authorization server supports neither Dynamic +/// Client Registration nor Client ID Metadata Documents. +#[derive(Clone, Debug, PartialEq)] +pub struct StaticOAuthClientConfig { + pub client_id: String, + /// Secret paired with the client ID. Optional: public clients using PKCE + /// have no secret. + pub client_secret: Option, + /// Scopes to request. When empty, scopes are selected from server + /// metadata, which may be broader than the extension needs. + pub scopes: Vec, +} + +fn scope_set(scopes: &[String]) -> BTreeSet<&str> { + scopes.iter().map(String::as_str).collect() +} + +fn configured_scopes_changed( + static_client: Option<&StaticOAuthClientConfig>, + previous_requested_scopes: Option<&[String]>, + granted_scopes: &[String], +) -> bool { + let Some(client) = static_client else { + return previous_requested_scopes.is_some(); + }; + + match previous_requested_scopes { + Some(previous) => scope_set(previous) != scope_set(&client.scopes), + None => !scope_set(&client.scopes).is_subset(&scope_set(granted_scopes)), + } +} + +fn configured_client_changed( + static_client: Option<&StaticOAuthClientConfig>, + stored_client_id: &str, +) -> bool { + static_client.is_some_and(|client| client.client_id != stored_client_id) +} + +fn resolve_refreshed_granted_scopes( + token_scopes: Option>, + previous_granted_scopes: &[String], +) -> Vec { + token_scopes.unwrap_or_else(|| previous_granted_scopes.to_vec()) +} + +fn configure_static_client( + auth_manager: &mut AuthorizationManager, + static_client: Option<&StaticOAuthClientConfig>, + redirect_uri: &str, +) -> Result<(), AuthError> { + let Some(client) = static_client else { + return Ok(()); + }; + + let mut config = OAuthClientConfig::new(client.client_id.clone(), redirect_uri.to_string()); + if let Some(secret) = &client.client_secret { + config = config.with_client_secret(secret.clone()); + } + auth_manager.configure_client(config) +} + +fn restore_omitted_scopes( + token_response: &mut OAuthTokenResponse, + granted_scopes: &[String], +) -> bool { + if token_response.scopes().is_some() || granted_scopes.is_empty() { + return false; + } + + token_response.set_scopes(Some( + granted_scopes.iter().cloned().map(Scope::new).collect(), + )); + true +} + +fn build_authorization_request( + redirect_uri: String, + static_client: Option<&StaticOAuthClientConfig>, +) -> AuthorizationRequest { + let mut request = AuthorizationRequest::new(redirect_uri).with_client_name("goose"); + match static_client { + Some(client) => { + request = request.with_preregistered_client(client.client_id.clone()); + if let Some(secret) = &client.client_secret { + request = request.with_client_secret(secret.clone()); + } + if !client.scopes.is_empty() { + request = request.with_scopes(client.scopes.clone()); + } + } + None => { + request = request.with_client_metadata_url(CLIENT_METADATA_URL); + } + } + request +} + pub async fn oauth_flow( mcp_server_url: &String, name: &String, + static_client: Option<&StaticOAuthClientConfig>, ) -> Result { let credential_store = GooseCredentialStore::new(name.clone()); let mut auth_manager = AuthorizationManager::new(mcp_server_url).await?; auth_manager.set_credential_store(credential_store.clone()); + let stored_credentials = credential_store.load().await?; + let previous_requested_scopes = credential_store.load_requested_scopes()?; + if auth_manager.initialize_from_store().await? { - match auth_manager.refresh_token().await { - Ok(_) => { - return Ok(auth_manager); - } - Err(e) => { - warn!( - "[OAuth:{}] Token refresh failed: {} - clearing stored credentials and falling back to browser auth", - name, e - ); + let stored_credentials = stored_credentials + .as_ref() + .ok_or_else(|| anyhow::anyhow!("OAuth credentials disappeared during startup"))?; + let previous_granted_scopes = stored_credentials.granted_scopes.as_slice(); + let scopes_changed = configured_scopes_changed( + static_client, + previous_requested_scopes.as_deref(), + previous_granted_scopes, + ); + let client_changed = + configured_client_changed(static_client, &stored_credentials.client_id); + + if !scopes_changed && !client_changed { + // initialize_from_store configures the client from the stored + // client_id alone; a confidential client must present its secret + // at the token endpoint for the refresh to succeed. + configure_static_client(&mut auth_manager, static_client, mcp_server_url)?; + match auth_manager.refresh_token().await { + Ok(mut token_response) => { + let restored_omitted_scopes = + restore_omitted_scopes(&mut token_response, previous_granted_scopes); + let mut refreshed_credentials = + credential_store.load().await?.ok_or_else(|| { + anyhow::anyhow!("OAuth refresh did not persist credentials") + })?; + let refreshed_client_id = refreshed_credentials.client_id.clone(); + refreshed_credentials.token_response = Some(token_response.clone()); + refreshed_credentials.granted_scopes = resolve_refreshed_granted_scopes( + token_response + .scopes() + .map(|scopes| scopes.iter().map(|scope| scope.to_string()).collect()), + previous_granted_scopes, + ); + let requested_scopes = static_client + .map(|client| client.scopes.clone()) + .or(previous_requested_scopes); + credential_store + .save_with_requested_scopes(refreshed_credentials, requested_scopes)?; + + if restored_omitted_scopes { + let mut oauth_state = OAuthState::new(mcp_server_url, None).await?; + oauth_state + .set_credentials(&refreshed_client_id, token_response) + .await?; + let mut restored_manager = + oauth_state.into_authorization_manager().ok_or_else(|| { + anyhow::anyhow!("Failed to restore OAuth authorization manager") + })?; + configure_static_client( + &mut restored_manager, + static_client, + mcp_server_url, + )?; + restored_manager.set_credential_store(credential_store); + return Ok(restored_manager); + } + return Ok(auth_manager); + } + Err(e) => { + warn!( + "[OAuth:{}] Token refresh failed: {} - clearing stored credentials and falling back to browser auth", + name, e + ); + } } } @@ -147,11 +309,7 @@ pub async fn oauth_flow( let redirect_uri = format!("http://127.0.0.1:{}/oauth_callback", used_addr.port()); oauth_state - .start_authorization( - AuthorizationRequest::new(redirect_uri) - .with_client_name("goose") - .with_client_metadata_url(CLIENT_METADATA_URL), - ) + .start_authorization(build_authorization_request(redirect_uri, static_client)) .await?; let authorization_url = oauth_state.get_authorization_url().await?; @@ -186,14 +344,9 @@ pub async fn oauth_flow( .into_authorization_manager() .ok_or_else(|| anyhow::anyhow!("Failed to get authorization manager"))?; - let granted_scopes: Vec = token_response - .as_ref() - .and_then(|tr| tr.scopes()) - .map(|scopes| scopes.iter().map(|s| s.to_string()).collect()) - .unwrap_or_default(); - - credential_store - .save(StoredCredentials::new( + let granted_scopes = auth_manager.get_current_scopes().await; + credential_store.save_with_requested_scopes( + StoredCredentials::new( client_id, token_response, granted_scopes, @@ -203,8 +356,9 @@ pub async fn oauth_flow( .map(|duration| duration.as_secs()) .unwrap_or(0), ), - )) - .await?; + ), + static_client.map(|client| client.scopes.clone()), + )?; auth_manager.set_credential_store(credential_store); @@ -288,6 +442,171 @@ mod tests { assert_eq!(params.iss, None); } + #[test] + fn unchanged_scope_request_preserves_a_narrowed_grant() { + let static_client = StaticOAuthClientConfig { + client_id: "registered-client".to_string(), + client_secret: None, + scopes: vec!["scope.read".to_string(), "scope.write".to_string()], + }; + + assert!(!configured_scopes_changed( + Some(&static_client), + Some(&["scope.read".to_string(), "scope.write".to_string()]), + &["scope.read".to_string()], + )); + } + + #[test] + fn changed_scope_request_requires_reauthorization() { + let static_client = StaticOAuthClientConfig { + client_id: "registered-client".to_string(), + client_secret: None, + scopes: vec!["scope.read".to_string(), "scope.write".to_string()], + }; + + assert!(configured_scopes_changed( + Some(&static_client), + Some(&["scope.read".to_string()]), + &["scope.read".to_string()], + )); + } + + #[test] + fn changed_static_client_requires_reauthorization() { + let static_client = StaticOAuthClientConfig { + client_id: "new-client".to_string(), + client_secret: None, + scopes: vec![], + }; + + assert!(configured_client_changed( + Some(&static_client), + "old-client" + )); + assert!(!configured_client_changed( + Some(&static_client), + "new-client" + )); + assert!(!configured_client_changed(None, "old-client")); + } + + #[test] + fn legacy_grant_reauthorizes_once_only_when_scopes_are_missing() { + let static_client = StaticOAuthClientConfig { + client_id: "registered-client".to_string(), + client_secret: None, + scopes: vec!["scope.read".to_string(), "scope.write".to_string()], + }; + + assert!(configured_scopes_changed( + Some(&static_client), + None, + &["scope.read".to_string()], + )); + assert!(!configured_scopes_changed( + Some(&static_client), + None, + &["scope.read".to_string(), "scope.write".to_string()], + )); + } + + #[test] + fn removing_static_client_configuration_requires_reauthorization() { + assert!(configured_scopes_changed( + None, + Some(&["scope.read".to_string()]), + &["scope.read".to_string()], + )); + assert!(!configured_scopes_changed( + None, + None, + &["scope.read".to_string()], + )); + } + + #[test] + fn omitted_refresh_scope_preserves_the_previous_grant() { + use oauth2::{basic::BasicTokenType, AccessToken}; + use rmcp::transport::auth::VendorExtraTokenFields; + + let previous = vec!["scope.read".to_string()]; + let mut token_response = OAuthTokenResponse::new( + AccessToken::new("access-token".to_string()), + BasicTokenType::Bearer, + VendorExtraTokenFields::default(), + ); + + assert_eq!(resolve_refreshed_granted_scopes(None, &previous), previous); + assert!(restore_omitted_scopes(&mut token_response, &previous)); + assert_eq!( + token_response + .scopes() + .unwrap() + .iter() + .map(|scope| scope.as_str()) + .collect::>(), + vec!["scope.read"] + ); + assert!(!restore_omitted_scopes(&mut token_response, &previous)); + assert_eq!( + resolve_refreshed_granted_scopes(Some(vec!["scope.other".to_string()]), &previous), + vec!["scope.other"] + ); + } + + #[test] + fn authorization_request_uses_client_metadata_url_without_static_client() { + let request = + build_authorization_request("http://127.0.0.1:1234/oauth_callback".to_string(), None); + + assert_eq!(request.client_id, None); + assert_eq!(request.client_secret, None); + assert_eq!( + request.client_metadata_url.as_deref(), + Some(CLIENT_METADATA_URL) + ); + assert!(request.scopes.is_empty()); + } + + #[test] + fn authorization_request_prefers_static_client_over_client_metadata_url() { + let static_client = StaticOAuthClientConfig { + client_id: "registered-client".to_string(), + client_secret: Some("registered-secret".to_string()), + scopes: vec!["scope.read".to_string(), "scope.write".to_string()], + }; + + let request = build_authorization_request( + "http://127.0.0.1:1234/oauth_callback".to_string(), + Some(&static_client), + ); + + assert_eq!(request.client_id.as_deref(), Some("registered-client")); + assert_eq!(request.client_secret.as_deref(), Some("registered-secret")); + assert_eq!(request.client_metadata_url, None); + assert_eq!(request.scopes, vec!["scope.read", "scope.write"]); + } + + #[test] + fn authorization_request_omits_secret_and_scopes_for_public_static_client() { + let static_client = StaticOAuthClientConfig { + client_id: "registered-client".to_string(), + client_secret: None, + scopes: vec![], + }; + + let request = build_authorization_request( + "http://127.0.0.1:1234/oauth_callback".to_string(), + Some(&static_client), + ); + + assert_eq!(request.client_id.as_deref(), Some("registered-client")); + assert_eq!(request.client_secret, None); + assert_eq!(request.client_metadata_url, None); + assert!(request.scopes.is_empty()); + } + #[tokio::test] async fn wait_for_callback_times_out_with_authorization_url() { let (_sender, receiver) = oneshot::channel(); diff --git a/crates/goose/src/oauth/persist.rs b/crates/goose/src/oauth/persist.rs index b0c4155ec..293afd289 100644 --- a/crates/goose/src/oauth/persist.rs +++ b/crates/goose/src/oauth/persist.rs @@ -1,7 +1,16 @@ use rmcp::transport::auth::{AuthError, CredentialStore, StoredCredentials}; +use serde::{Deserialize, Serialize}; use crate::config::Config; +#[derive(Serialize, Deserialize)] +struct PersistedCredentials { + #[serde(flatten)] + credentials: StoredCredentials, + #[serde(default, skip_serializing_if = "Option::is_none")] + requested_scopes: Option>, +} + /// Goose-specific credential store that uses the Config system /// /// This implementation stores OAuth credentials in the goose configuration @@ -20,21 +29,18 @@ impl GooseCredentialStore { fn secret_key(&self) -> String { format!("oauth_creds_{}", self.name) } -} -#[async_trait::async_trait] -impl CredentialStore for GooseCredentialStore { - async fn load(&self) -> Result, AuthError> { + fn load_persisted(&self) -> Result, AuthError> { let config = Config::global(); let key = self.secret_key(); - match config.get_secret::(&key) { + match config.get_secret::(&key) { Ok(credentials) => Ok(Some(credentials)), - Err(_) => Ok(None), // No credentials found + Err(_) => Ok(None), } } - async fn save(&self, credentials: StoredCredentials) -> Result<(), AuthError> { + fn save_persisted(&self, credentials: PersistedCredentials) -> Result<(), AuthError> { let config = Config::global(); let key = self.secret_key(); @@ -43,6 +49,37 @@ impl CredentialStore for GooseCredentialStore { .map_err(|e| AuthError::InternalError(format!("Failed to save credentials: {}", e))) } + pub fn load_requested_scopes(&self) -> Result>, AuthError> { + Ok(self + .load_persisted()? + .and_then(|credentials| credentials.requested_scopes)) + } + + pub fn save_with_requested_scopes( + &self, + credentials: StoredCredentials, + requested_scopes: Option>, + ) -> Result<(), AuthError> { + self.save_persisted(PersistedCredentials { + credentials, + requested_scopes, + }) + } +} + +#[async_trait::async_trait] +impl CredentialStore for GooseCredentialStore { + async fn load(&self) -> Result, AuthError> { + Ok(self + .load_persisted()? + .map(|credentials| credentials.credentials)) + } + + async fn save(&self, credentials: StoredCredentials) -> Result<(), AuthError> { + let requested_scopes = self.load_requested_scopes()?; + self.save_with_requested_scopes(credentials, requested_scopes) + } + async fn clear(&self) -> Result<(), AuthError> { let config = Config::global(); let key = self.secret_key(); @@ -52,3 +89,40 @@ impl CredentialStore for GooseCredentialStore { .map_err(|e| AuthError::InternalError(format!("Failed to clear credentials: {}", e))) } } + +#[cfg(test)] +mod tests { + use super::*; + + fn credentials() -> StoredCredentials { + StoredCredentials::new( + "client-id".to_string(), + None, + vec!["scope.read".to_string()], + Some(123), + ) + } + + #[test] + fn persisted_credentials_read_the_legacy_shape() { + let legacy = serde_json::to_value(credentials()).unwrap(); + let persisted: PersistedCredentials = serde_json::from_value(legacy).unwrap(); + + assert_eq!(persisted.credentials.client_id, "client-id"); + assert_eq!(persisted.credentials.granted_scopes, vec!["scope.read"]); + assert_eq!(persisted.requested_scopes, None); + } + + #[test] + fn persisted_credentials_remain_readable_as_stored_credentials() { + let persisted = PersistedCredentials { + credentials: credentials(), + requested_scopes: Some(vec!["scope.read".to_string(), "scope.write".to_string()]), + }; + let value = serde_json::to_value(persisted).unwrap(); + let credentials: StoredCredentials = serde_json::from_value(value).unwrap(); + + assert_eq!(credentials.client_id, "client-id"); + assert_eq!(credentials.granted_scopes, vec!["scope.read"]); + } +} diff --git a/crates/goose/src/providers/claude_code.rs b/crates/goose/src/providers/claude_code.rs index febb7896c..b304e2d39 100644 --- a/crates/goose/src/providers/claude_code.rs +++ b/crates/goose/src/providers/claude_code.rs @@ -1252,6 +1252,9 @@ mod tests { headers: HashMap::from([("Authorization".into(), "Bearer token".into())]), timeout: None, socket: None, + client_id: None, + client_secret_key: None, + scopes: vec![], bundled: Some(false), available_tools: vec![], }], @@ -1274,6 +1277,9 @@ mod tests { headers: HashMap::new(), timeout: None, socket: None, + client_id: None, + client_secret_key: None, + scopes: vec![], bundled: None, available_tools: vec![], }], diff --git a/crates/goose/src/providers/codex.rs b/crates/goose/src/providers/codex.rs index 24d503806..468fd7fee 100644 --- a/crates/goose/src/providers/codex.rs +++ b/crates/goose/src/providers/codex.rs @@ -802,6 +802,9 @@ mod tests { headers: HashMap::from([("Authorization".into(), "Bearer token".into())]), timeout: None, socket: None, + client_id: None, + client_secret_key: None, + scopes: vec![], bundled: Some(false), available_tools: vec![], }, @@ -821,6 +824,9 @@ mod tests { headers: HashMap::new(), timeout: None, socket: None, + client_id: None, + client_secret_key: None, + scopes: vec![], bundled: None, available_tools: vec![], }, diff --git a/crates/goose/src/providers/oauth.rs b/crates/goose/src/providers/oauth.rs index 1c207230a..692dd292b 100644 --- a/crates/goose/src/providers/oauth.rs +++ b/crates/goose/src/providers/oauth.rs @@ -558,11 +558,10 @@ mod tests { #[test] fn test_token_cache() -> Result<()> { - let cache = TokenCache::new( - "https://example.com", - "test-client", - &["scope1".to_string()], - ); + let directory = tempfile::tempdir()?; + let cache = TokenCache { + cache_path: directory.path().join("token.json"), + }; // Test with expiration time let token_data = TokenData { diff --git a/crates/goose/src/recipe/recipe_extension_adapter.rs b/crates/goose/src/recipe/recipe_extension_adapter.rs index 0372f4a5d..eff98ab22 100644 --- a/crates/goose/src/recipe/recipe_extension_adapter.rs +++ b/crates/goose/src/recipe/recipe_extension_adapter.rs @@ -66,6 +66,12 @@ enum RecipeExtensionConfigInternal { #[serde(default)] socket: Option, #[serde(default)] + client_id: Option, + #[serde(default)] + client_secret_key: Option, + #[serde(default)] + scopes: Vec, + #[serde(default)] bundled: Option, #[serde(default)] available_tools: Vec, @@ -146,6 +152,9 @@ impl From for ExtensionConfig { headers, timeout, socket, + client_id, + client_secret_key, + scopes, bundled, available_tools }, diff --git a/documentation/docs/getting-started/using-extensions.md b/documentation/docs/getting-started/using-extensions.md index 73ddd7cbf..4c0b79b9f 100644 --- a/documentation/docs/getting-started/using-extensions.md +++ b/documentation/docs/getting-started/using-extensions.md @@ -330,7 +330,43 @@ extensions: type: stdio timeout: 300 ``` - + +#### Remote extensions with a pre-registered OAuth client + +Remote (`streamable_http`) extensions that require OAuth normally obtain a +client ID automatically, using Client ID Metadata Documents or Dynamic Client +Registration. Some authorization servers support neither and instead require a +client that was registered out of band. For those servers, set `client_id` — +and, for confidential clients, `client_secret_key` — on the extension: + +```yaml +extensions: + remote-example: + name: Remote Example + type: streamable_http + uri: https://example.com/mcp + client_id: + client_secret_key: REMOTE_EXAMPLE_OAUTH_SECRET + scopes: + - example.readonly + enabled: true + timeout: 300 +``` + +- `client_id`: the OAuth client ID registered with the server's authorization + server. When set, it takes priority over Client ID Metadata Documents and + Dynamic Client Registration. Supports `$VAR`/`${VAR}` substitution. +- `client_secret_key`: the name of an env/secret key holding the client + secret, resolved from `envs`/`env_keys` or goose's secret store (`goose + configure` > extension secrets). The secret value itself never goes in the + config file. Omit it for public clients that authenticate with PKCE alone. +- `scopes`: the OAuth scopes to request. When omitted, scopes are selected + from the server's advertised metadata, which may be broader than the + extension needs. + +The OAuth callback is served on `127.0.0.1` with an ephemeral port; set +`GOOSE_OAUTH_CALLBACK_PORT` if the authorization server only allows a +pre-registered redirect URI with a fixed port. ## Enabling/Disabling Extensions diff --git a/ui/desktop/src/acp/extensions.ts b/ui/desktop/src/acp/extensions.ts index 72e1efd2b..8761d6291 100644 --- a/ui/desktop/src/acp/extensions.ts +++ b/ui/desktop/src/acp/extensions.ts @@ -55,6 +55,9 @@ export function gooseExtensionToExtensionConfig(extension: GooseExtension): Exte env_keys: extension.envKeys ?? [], timeout: extension.timeout, socket: extension.socket, + client_id: extension.clientId, + client_secret_key: extension.clientSecretKey, + scopes: extension.scopes ?? [], bundled: extension.bundled, available_tools: availableToolsOrUndefined(extension.available_tools), }; @@ -135,6 +138,9 @@ export function extensionConfigToGooseExtension(config: ExtensionConfig): GooseE description: config.description, timeout: config.timeout, socket: config.socket, + clientId: config.client_id, + clientSecretKey: config.client_secret_key, + scopes: config.scopes ?? [], bundled: config.bundled, available_tools: availableToolsOrUndefined(config.available_tools), }; diff --git a/ui/desktop/src/components/recipes/shared/RecipeExtensionSelector.tsx b/ui/desktop/src/components/recipes/shared/RecipeExtensionSelector.tsx index 9f0af6eec..14acd4f35 100644 --- a/ui/desktop/src/components/recipes/shared/RecipeExtensionSelector.tsx +++ b/ui/desktop/src/components/recipes/shared/RecipeExtensionSelector.tsx @@ -115,6 +115,9 @@ function toRecipeExtension( headers, timeout, socket, + client_id, + client_secret_key, + scopes, bundled, available_tools, type, @@ -128,6 +131,9 @@ function toRecipeExtension( headers, timeout, socket, + client_id, + client_secret_key, + scopes, bundled, ...availableToolsProps(available_tools), type, diff --git a/ui/desktop/src/components/recipes/shared/__tests__/RecipeExtensionSelector.test.tsx b/ui/desktop/src/components/recipes/shared/__tests__/RecipeExtensionSelector.test.tsx index 353e6a8fe..81140fa5a 100644 --- a/ui/desktop/src/components/recipes/shared/__tests__/RecipeExtensionSelector.test.tsx +++ b/ui/desktop/src/components/recipes/shared/__tests__/RecipeExtensionSelector.test.tsx @@ -51,4 +51,37 @@ describe('RecipeExtensionSelector', () => { }), ]); }); + + it('preserves static OAuth fields when selecting a configured extension', async () => { + const user = userEvent.setup(); + const onExtensionsChange = vi.fn(); + configContextMock.extensionsList = [ + { + type: 'streamable_http', + name: 'google_workspace', + description: 'Google Workspace', + uri: 'https://example.com/mcp', + client_id: 'registered-client', + client_secret_key: 'GOOGLE_OAUTH_SECRET', + scopes: ['drive.readonly'], + enabled: true, + }, + ]; + + renderWithIntl( + + ); + + await user.click(screen.getByTitle('Google Workspace')); + + expect(onExtensionsChange).toHaveBeenCalledWith([ + expect.objectContaining({ + type: 'streamable_http', + name: 'google_workspace', + client_id: 'registered-client', + client_secret_key: 'GOOGLE_OAUTH_SECRET', + scopes: ['drive.readonly'], + }), + ]); + }); }); diff --git a/ui/desktop/src/components/settings/extensions/utils.test.ts b/ui/desktop/src/components/settings/extensions/utils.test.ts index b364d019c..051b332d3 100644 --- a/ui/desktop/src/components/settings/extensions/utils.test.ts +++ b/ui/desktop/src/components/settings/extensions/utils.test.ts @@ -111,6 +111,32 @@ describe('Extension Utils', () => { }); }); + it('should preserve streamable_http OAuth client fields and socket through a form round-trip', () => { + const extension: FixedExtensionEntry = { + type: 'streamable_http', + name: 'oauth-extension', + description: 'OAuth description', + uri: 'http://api.example.com', + enabled: true, + headers: {}, + socket: '@egress.sock', + client_id: 'registered-client', + client_secret_key: 'OAUTH_CLIENT_SECRET', + scopes: ['scope.read'], + }; + + const formData = extensionToFormData(extension); + const config = createExtensionConfig(formData); + + expect(config).toMatchObject({ + type: 'streamable_http', + socket: '@egress.sock', + client_id: 'registered-client', + client_secret_key: 'OAUTH_CLIENT_SECRET', + scopes: ['scope.read'], + }); + }); + it('should handle legacy envs field', () => { const extension: FixedExtensionEntry = { type: 'stdio', diff --git a/ui/desktop/src/components/settings/extensions/utils.ts b/ui/desktop/src/components/settings/extensions/utils.ts index 34d8ddcbe..d3fcd6b89 100644 --- a/ui/desktop/src/components/settings/extensions/utils.ts +++ b/ui/desktop/src/components/settings/extensions/utils.ts @@ -38,6 +38,12 @@ export interface ExtensionFormData { }[]; installation_notes?: string; available_tools?: string[]; + // streamable_http fields with no form input yet; carried through so an + // unrelated edit does not strip them from the saved config. + socket?: string | null; + client_id?: string | null; + client_secret_key?: string | null; + scopes?: string[]; } export function getDefaultFormData(): ExtensionFormData { @@ -125,6 +131,14 @@ export function extensionToFormData(extension: FixedExtensionEntry): ExtensionFo | string | undefined, ...(availableTools ? { available_tools: availableTools } : {}), + ...(extension.type === 'streamable_http' + ? { + socket: extension.socket, + client_id: extension.client_id, + client_secret_key: extension.client_secret_key, + scopes: extension.scopes, + } + : {}), }; } @@ -176,6 +190,12 @@ export function createExtensionConfig(formData: ExtensionFormData): ExtensionCon ...(env_keys.length > 0 ? { env_keys } : {}), headers, ...availableToolsConfig(formData.available_tools), + ...(formData.socket != null ? { socket: formData.socket } : {}), + ...(formData.client_id != null ? { client_id: formData.client_id } : {}), + ...(formData.client_secret_key != null + ? { client_secret_key: formData.client_secret_key } + : {}), + ...(formData.scopes?.length ? { scopes: formData.scopes } : {}), }; } else if (formData.type === 'builtin') { return { diff --git a/ui/sdk/src/generated/types.gen.ts b/ui/sdk/src/generated/types.gen.ts index 03d7cd847..e80281d74 100644 --- a/ui/sdk/src/generated/types.gen.ts +++ b/ui/sdk/src/generated/types.gen.ts @@ -36,6 +36,18 @@ export type GooseExtension = { description?: string | null; timeout?: number | null; socket?: string | null; + /** + * Pre-registered OAuth client ID for the server's authorization server. + */ + clientId?: string | null; + /** + * Name of the env/secret key holding the OAuth client secret. + */ + clientSecretKey?: string | null; + /** + * OAuth scopes to request with `client_id`. + */ + scopes?: Array; bundled?: boolean | null; /** * Tool allowlist for this extension. Omit this field to allow all tools. @@ -1634,6 +1646,18 @@ export type RecipeExtensionDto = { }; timeout?: number | null; socket?: string | null; + /** + * Pre-registered OAuth client ID for the server's authorization server. + */ + client_id?: string | null; + /** + * Name of the env/secret key holding the OAuth client secret. + */ + client_secret_key?: string | null; + /** + * OAuth scopes to request with `client_id`. + */ + scopes?: Array; bundled?: boolean | null; /** * Tool allowlist for this extension. Omit this field to allow all tools. diff --git a/ui/sdk/src/generated/zod.gen.ts b/ui/sdk/src/generated/zod.gen.ts index c3f0807c8..8234aa5bc 100644 --- a/ui/sdk/src/generated/zod.gen.ts +++ b/ui/sdk/src/generated/zod.gen.ts @@ -175,6 +175,15 @@ export const zGooseExtension = z.union([ z.string(), z.null() ]).optional(), + clientId: z.union([ + z.string(), + z.null() + ]).optional(), + clientSecretKey: z.union([ + z.string(), + z.null() + ]).optional(), + scopes: z.array(z.string()).optional(), bundled: z.union([ z.boolean(), z.null() @@ -1586,6 +1595,15 @@ export const zRecipeExtensionDto = z.union([ z.string(), z.null() ]).optional(), + client_id: z.union([ + z.string(), + z.null() + ]).optional(), + client_secret_key: z.union([ + z.string(), + z.null() + ]).optional(), + scopes: z.array(z.string()).optional(), bundled: z.union([ z.boolean(), z.null()