feat(mcp): support pre-registered OAuth clients for streamable_http extensions (#11182)
Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: OpenAI Codex <codex@openai.com>
This commit is contained in:
@@ -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(),
|
||||
},
|
||||
|
||||
@@ -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(),
|
||||
|
||||
@@ -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![],
|
||||
}
|
||||
|
||||
@@ -361,7 +361,7 @@ pub enum GooseExtension {
|
||||
available_tools: Option<Vec<String>>,
|
||||
},
|
||||
Mcp {
|
||||
server: McpServer,
|
||||
server: Box<McpServer>,
|
||||
#[serde(default, rename = "envKeys", skip_serializing_if = "Vec::is_empty")]
|
||||
env_keys: Vec<String>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
@@ -370,6 +370,19 @@ pub enum GooseExtension {
|
||||
timeout: Option<u64>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
socket: Option<String>,
|
||||
/// Pre-registered OAuth client ID for the server's authorization server.
|
||||
#[serde(default, rename = "clientId", skip_serializing_if = "Option::is_none")]
|
||||
client_id: Option<String>,
|
||||
/// 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<String>,
|
||||
/// OAuth scopes to request with `client_id`.
|
||||
#[serde(default, skip_serializing_if = "Vec::is_empty")]
|
||||
scopes: Vec<String>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
bundled: Option<bool>,
|
||||
/// Tool allowlist for this extension. Omit this field to allow all tools.
|
||||
|
||||
@@ -223,6 +223,15 @@ pub enum RecipeExtensionDto {
|
||||
timeout: Option<u64>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
socket: Option<String>,
|
||||
/// Pre-registered OAuth client ID for the server's authorization server.
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
client_id: Option<String>,
|
||||
/// Name of the env/secret key holding the OAuth client secret.
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
client_secret_key: Option<String>,
|
||||
/// OAuth scopes to request with `client_id`.
|
||||
#[serde(default, skip_serializing_if = "Vec::is_empty")]
|
||||
scopes: Vec<String>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
bundled: Option<bool>,
|
||||
/// Tool allowlist for this extension. Omit this field to allow all tools.
|
||||
|
||||
@@ -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",
|
||||
|
||||
@@ -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![],
|
||||
};
|
||||
|
||||
@@ -397,6 +397,9 @@ fn mcp_server_to_extension_config(mcp_server: McpServer) -> Result<ExtensionConf
|
||||
.collect(),
|
||||
timeout,
|
||||
socket: None,
|
||||
client_id: None,
|
||||
client_secret_key: None,
|
||||
scopes: vec![],
|
||||
bundled: Some(false),
|
||||
available_tools: vec![],
|
||||
})
|
||||
@@ -2540,6 +2543,9 @@ extensions:
|
||||
)]),
|
||||
timeout: None,
|
||||
socket: None,
|
||||
client_id: None,
|
||||
client_secret_key: None,
|
||||
scopes: vec![],
|
||||
bundled: Some(false),
|
||||
available_tools: vec![],
|
||||
})
|
||||
|
||||
@@ -178,11 +178,16 @@ fn config_to_goose_extension(
|
||||
available_tools,
|
||||
..
|
||||
} => 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,
|
||||
};
|
||||
|
||||
@@ -344,6 +344,9 @@ impl TryFrom<RecipeExtensionDto> for ExtensionConfig {
|
||||
headers,
|
||||
timeout,
|
||||
socket,
|
||||
client_id,
|
||||
client_secret_key,
|
||||
scopes,
|
||||
bundled,
|
||||
available_tools,
|
||||
} => Self::StreamableHttp {
|
||||
@@ -355,6 +358,9 @@ impl TryFrom<RecipeExtensionDto> 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<ExtensionConfig> for RecipeExtensionDto {
|
||||
headers,
|
||||
timeout,
|
||||
socket,
|
||||
client_id,
|
||||
client_secret_key,
|
||||
scopes,
|
||||
bundled,
|
||||
available_tools,
|
||||
} => Self::StreamableHttp {
|
||||
@@ -438,6 +447,9 @@ impl TryFrom<ExtensionConfig> 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()]),
|
||||
},
|
||||
|
||||
@@ -249,6 +249,26 @@ pub enum ExtensionConfig {
|
||||
/// Use `@name` for Linux abstract sockets.
|
||||
#[serde(default)]
|
||||
socket: Option<String>,
|
||||
/// 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<String>,
|
||||
/// 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<String>,
|
||||
/// 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<String>,
|
||||
#[serde(default)]
|
||||
bundled: Option<bool>,
|
||||
#[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![],
|
||||
};
|
||||
|
||||
@@ -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<String, String>,
|
||||
config: &Config,
|
||||
) -> Result<Option<StaticOAuthClientConfig>, Box<ExtensionError>> {
|
||||
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<String, String>,
|
||||
config: &Config,
|
||||
) -> Result<String, Box<ExtensionError>> {
|
||||
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, String>) -> String {
|
||||
let mut result = value.to_string();
|
||||
@@ -679,6 +740,7 @@ async fn create_streamable_http_client(
|
||||
headers: &HashMap<String, String>,
|
||||
name: &str,
|
||||
socket: Option<&str>,
|
||||
static_oauth_client: Option<StaticOAuthClientConfig>,
|
||||
credential_store: Box<dyn CredentialStore>,
|
||||
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(),
|
||||
|
||||
+345
-26
@@ -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<String>,
|
||||
/// Scopes to request. When empty, scopes are selected from server
|
||||
/// metadata, which may be broader than the extension needs.
|
||||
pub scopes: Vec<String>,
|
||||
}
|
||||
|
||||
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<Vec<String>>,
|
||||
previous_granted_scopes: &[String],
|
||||
) -> Vec<String> {
|
||||
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<AuthorizationManager, anyhow::Error> {
|
||||
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<String> = 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<_>>(),
|
||||
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();
|
||||
|
||||
@@ -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<Vec<String>>,
|
||||
}
|
||||
|
||||
/// 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<Option<StoredCredentials>, AuthError> {
|
||||
fn load_persisted(&self) -> Result<Option<PersistedCredentials>, AuthError> {
|
||||
let config = Config::global();
|
||||
let key = self.secret_key();
|
||||
|
||||
match config.get_secret::<StoredCredentials>(&key) {
|
||||
match config.get_secret::<PersistedCredentials>(&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<Option<Vec<String>>, AuthError> {
|
||||
Ok(self
|
||||
.load_persisted()?
|
||||
.and_then(|credentials| credentials.requested_scopes))
|
||||
}
|
||||
|
||||
pub fn save_with_requested_scopes(
|
||||
&self,
|
||||
credentials: StoredCredentials,
|
||||
requested_scopes: Option<Vec<String>>,
|
||||
) -> Result<(), AuthError> {
|
||||
self.save_persisted(PersistedCredentials {
|
||||
credentials,
|
||||
requested_scopes,
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
#[async_trait::async_trait]
|
||||
impl CredentialStore for GooseCredentialStore {
|
||||
async fn load(&self) -> Result<Option<StoredCredentials>, 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"]);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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![],
|
||||
}],
|
||||
|
||||
@@ -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![],
|
||||
},
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -66,6 +66,12 @@ enum RecipeExtensionConfigInternal {
|
||||
#[serde(default)]
|
||||
socket: Option<String>,
|
||||
#[serde(default)]
|
||||
client_id: Option<String>,
|
||||
#[serde(default)]
|
||||
client_secret_key: Option<String>,
|
||||
#[serde(default)]
|
||||
scopes: Vec<String>,
|
||||
#[serde(default)]
|
||||
bundled: Option<bool>,
|
||||
#[serde(default)]
|
||||
available_tools: Vec<String>,
|
||||
@@ -146,6 +152,9 @@ impl From<RecipeExtensionConfigInternal> for ExtensionConfig {
|
||||
headers,
|
||||
timeout,
|
||||
socket,
|
||||
client_id,
|
||||
client_secret_key,
|
||||
scopes,
|
||||
bundled,
|
||||
available_tools
|
||||
},
|
||||
|
||||
@@ -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: <YOUR_REGISTERED_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
|
||||
|
||||
|
||||
@@ -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),
|
||||
};
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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(
|
||||
<RecipeExtensionSelector selectedExtensions={[]} onExtensionsChange={onExtensionsChange} />
|
||||
);
|
||||
|
||||
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'],
|
||||
}),
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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',
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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<string>;
|
||||
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<string>;
|
||||
bundled?: boolean | null;
|
||||
/**
|
||||
* Tool allowlist for this extension. Omit this field to allow all tools.
|
||||
|
||||
@@ -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()
|
||||
|
||||
Reference in New Issue
Block a user