mirror of
https://github.com/outbackdingo/optimclaw.git
synced 2026-08-25 14:53:34 +00:00
feat(extensions): improve auth UX and add load-time validation (#536)
* feat(extensions): add load-time validation for auth capabilities Catch common misconfigurations (missing auth section, missing setup_url, short prompts) at startup via tracing::warn instead of silently failing at auth time. * feat(extensions): improve auth prompts, setup_url, and showAuthCard Add setup_url and descriptive prompts to channel and tool capabilities files. Fix showAuthCard in web gateway and improve extension manager auth flow messaging. * refactor(extensions): extract MIN_PROMPT_LENGTH constant in validate() Address review feedback: replace magic number 30 with a named constant for readability and maintainability. [skip-regression-check] Co-Authored-By: Claude Opus 4.6 <[email protected]> --------- Co-authored-by: Claude Opus 4.6 <[email protected]>
This commit is contained in:
Pierre LE GUEN
co-authored by
Claude Opus 4.6
Claude Opus 4.6
parent
cbcd5adcc0
commit
13697976db
@@ -6,15 +6,16 @@
|
||||
"required_secrets": [
|
||||
{
|
||||
"name": "discord_bot_token",
|
||||
"prompt": "Enter your Discord Bot Token (from Developer Portal)",
|
||||
"prompt": "Enter your Discord Bot Token. Find it under Bot > Token in your Discord Application settings.",
|
||||
"optional": false
|
||||
},
|
||||
{
|
||||
"name": "discord_public_key",
|
||||
"prompt": "Enter your Discord Application Public Key (from Developer Portal > General Information)",
|
||||
"prompt": "Enter your Discord Application Public Key (found under General Information in your Discord Application settings).",
|
||||
"optional": false
|
||||
}
|
||||
]
|
||||
],
|
||||
"setup_url": "https://discord.com/developers/applications"
|
||||
},
|
||||
"capabilities": {
|
||||
"http": {
|
||||
|
||||
@@ -6,15 +6,16 @@
|
||||
"required_secrets": [
|
||||
{
|
||||
"name": "slack_bot_token",
|
||||
"prompt": "Enter your Slack Bot OAuth Token (xoxb-...)",
|
||||
"prompt": "Enter your Slack Bot User OAuth Token (starts with xoxb-). Find it under OAuth & Permissions in your Slack App settings.",
|
||||
"optional": false
|
||||
},
|
||||
{
|
||||
"name": "slack_signing_secret",
|
||||
"prompt": "Enter your Slack Signing Secret (from App Credentials)",
|
||||
"prompt": "Enter your Slack App Signing Secret (found under Basic Information > App Credentials in your Slack App settings).",
|
||||
"optional": false
|
||||
}
|
||||
]
|
||||
],
|
||||
"setup_url": "https://api.slack.com/apps"
|
||||
},
|
||||
"capabilities": {
|
||||
"http": {
|
||||
|
||||
@@ -9,7 +9,8 @@
|
||||
"prompt": "Enter your Telegram Bot API token (from @BotFather)",
|
||||
"optional": false
|
||||
}
|
||||
]
|
||||
],
|
||||
"setup_url": "https://t.me/BotFather"
|
||||
},
|
||||
"capabilities": {
|
||||
"http": {
|
||||
|
||||
@@ -6,7 +6,7 @@
|
||||
"required_secrets": [
|
||||
{
|
||||
"name": "whatsapp_access_token",
|
||||
"prompt": "Enter your WhatsApp Cloud API access token (from Meta Developer Portal)",
|
||||
"prompt": "Enter your WhatsApp Cloud API permanent access token (from the Meta Developer Portal under your app's WhatsApp > API Setup).",
|
||||
"validation": "^[A-Za-z0-9_-]+$"
|
||||
},
|
||||
{
|
||||
@@ -16,7 +16,8 @@
|
||||
"auto_generate": { "length": 32 }
|
||||
}
|
||||
],
|
||||
"validation_endpoint": "https://graph.facebook.com/v18.0/me?access_token={whatsapp_access_token}"
|
||||
"validation_endpoint": "https://graph.facebook.com/v18.0/me?access_token={whatsapp_access_token}",
|
||||
"setup_url": "https://developers.facebook.com/apps"
|
||||
},
|
||||
"capabilities": {
|
||||
"http": {
|
||||
|
||||
@@ -81,6 +81,7 @@ impl WasmChannelLoader {
|
||||
let cap_bytes = fs::read(cap_path).await?;
|
||||
let cap_file = ChannelCapabilitiesFile::from_bytes(&cap_bytes)
|
||||
.map_err(|e| WasmChannelError::InvalidCapabilities(e.to_string()))?;
|
||||
cap_file.validate();
|
||||
|
||||
// Debug: log raw capabilities
|
||||
tracing::debug!(
|
||||
|
||||
@@ -90,6 +90,37 @@ impl ChannelCapabilitiesFile {
|
||||
serde_json::from_slice(bytes)
|
||||
}
|
||||
|
||||
/// Validate the capabilities file and emit warnings for common misconfigurations.
|
||||
///
|
||||
/// Called once at load time to catch issues early. Warnings are emitted via
|
||||
/// `tracing::warn` so they show up in startup logs without blocking loading.
|
||||
pub fn validate(&self) {
|
||||
const MIN_PROMPT_LENGTH: usize = 30;
|
||||
|
||||
// Check for short prompts in required_secrets
|
||||
for secret in &self.setup.required_secrets {
|
||||
if secret.prompt.len() < MIN_PROMPT_LENGTH {
|
||||
tracing::warn!(
|
||||
channel = self.name,
|
||||
secret = secret.name,
|
||||
prompt = secret.prompt,
|
||||
"setup.required_secrets prompt is shorter than {} chars — \
|
||||
consider a more descriptive prompt that tells the user where to find this value",
|
||||
MIN_PROMPT_LENGTH
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
// Has required_secrets but no setup_url
|
||||
if !self.setup.required_secrets.is_empty() && self.setup.setup_url.is_none() {
|
||||
tracing::warn!(
|
||||
channel = self.name,
|
||||
"setup.required_secrets defined but no setup.setup_url — \
|
||||
user has no link to obtain credentials"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/// Convert to runtime ChannelCapabilities.
|
||||
pub fn to_capabilities(&self) -> ChannelCapabilities {
|
||||
self.capabilities.to_channel_capabilities(&self.name)
|
||||
@@ -262,6 +293,10 @@ pub struct SetupSchema {
|
||||
/// Placeholders like {secret_name} are replaced with actual values.
|
||||
#[serde(default)]
|
||||
pub validation_endpoint: Option<String>,
|
||||
|
||||
/// User-facing URL where they can create/manage credentials.
|
||||
#[serde(default)]
|
||||
pub setup_url: Option<String>,
|
||||
}
|
||||
|
||||
/// Configuration for a secret required during setup.
|
||||
@@ -605,6 +640,65 @@ mod tests {
|
||||
|
||||
// ── Category 5: Discord Capabilities Setup & Configuration ──────────
|
||||
|
||||
#[test]
|
||||
fn test_validate_channel_short_prompt() {
|
||||
// prompt < 30 chars — should not panic
|
||||
let json = r#"{
|
||||
"name": "test-channel",
|
||||
"setup": {
|
||||
"required_secrets": [
|
||||
{ "name": "bot_token", "prompt": "Bot token" }
|
||||
],
|
||||
"setup_url": "https://example.com"
|
||||
}
|
||||
}"#;
|
||||
|
||||
let file = ChannelCapabilitiesFile::from_json(json).unwrap();
|
||||
// Should not panic; warning emitted for short prompt
|
||||
file.validate();
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_validate_channel_missing_setup_url() {
|
||||
// required_secrets without setup_url — should not panic
|
||||
let json = r#"{
|
||||
"name": "test-channel",
|
||||
"setup": {
|
||||
"required_secrets": [
|
||||
{
|
||||
"name": "bot_token",
|
||||
"prompt": "Enter your bot token from the developer portal settings"
|
||||
}
|
||||
]
|
||||
}
|
||||
}"#;
|
||||
|
||||
let file = ChannelCapabilitiesFile::from_json(json).unwrap();
|
||||
// Should not panic; warning emitted for missing setup_url
|
||||
file.validate();
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_validate_clean_channel() {
|
||||
// Well-configured channel — should not panic or warn
|
||||
let json = r#"{
|
||||
"name": "good-channel",
|
||||
"setup": {
|
||||
"required_secrets": [
|
||||
{
|
||||
"name": "bot_token",
|
||||
"prompt": "Enter your bot token from https://example.com/bot-settings"
|
||||
}
|
||||
],
|
||||
"setup_url": "https://example.com/bot-settings"
|
||||
}
|
||||
}"#;
|
||||
|
||||
let file = ChannelCapabilitiesFile::from_json(json).unwrap();
|
||||
// Should not panic and emits no warnings
|
||||
file.validate();
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_discord_capabilities_has_public_key_secret() {
|
||||
let json = include_str!("../../../channels-src/discord/discord.capabilities.json");
|
||||
|
||||
@@ -874,7 +874,7 @@ function showAuthCard(data) {
|
||||
|
||||
const tokenInput = document.createElement('input');
|
||||
tokenInput.type = 'password';
|
||||
tokenInput.placeholder = 'Paste your API key or token';
|
||||
tokenInput.placeholder = data.instructions || 'Paste your API key or token';
|
||||
tokenInput.addEventListener('keydown', (e) => {
|
||||
if (e.key === 'Enter') submitAuthToken(data.extension_name, tokenInput.value);
|
||||
});
|
||||
|
||||
@@ -2110,7 +2110,7 @@ impl ExtensionManager {
|
||||
auth_url: None,
|
||||
callback_type: None,
|
||||
instructions: Some(next.prompt.clone()),
|
||||
setup_url: cap_file.setup.validation_endpoint.clone(),
|
||||
setup_url: cap_file.setup.setup_url.clone(),
|
||||
awaiting_token: true,
|
||||
status: "awaiting_token".to_string(),
|
||||
});
|
||||
@@ -2124,7 +2124,7 @@ impl ExtensionManager {
|
||||
auth_url: None,
|
||||
callback_type: None,
|
||||
instructions: Some(secret.prompt.clone()),
|
||||
setup_url: cap_file.setup.validation_endpoint.clone(),
|
||||
setup_url: cap_file.setup.setup_url.clone(),
|
||||
awaiting_token: true,
|
||||
status: "awaiting_token".to_string(),
|
||||
})
|
||||
|
||||
@@ -105,6 +105,59 @@ impl CapabilitiesFile {
|
||||
self
|
||||
}
|
||||
|
||||
/// Validate the capabilities file and emit warnings for common misconfigurations.
|
||||
///
|
||||
/// Called once at load time to catch issues early. Warnings are emitted via
|
||||
/// `tracing::warn` so they show up in startup logs without blocking loading.
|
||||
pub fn validate(&self, name: &str) {
|
||||
const MIN_PROMPT_LENGTH: usize = 30;
|
||||
|
||||
// setup.required_secrets present but no auth section → auth card won't display
|
||||
if let Some(setup) = &self.setup {
|
||||
if !setup.required_secrets.is_empty() && self.auth.is_none() {
|
||||
tracing::warn!(
|
||||
tool = name,
|
||||
"setup.required_secrets defined but no 'auth' section — \
|
||||
chat-based auth card will not display for this tool"
|
||||
);
|
||||
}
|
||||
|
||||
// Check for short prompts
|
||||
for secret in &setup.required_secrets {
|
||||
if secret.prompt.len() < MIN_PROMPT_LENGTH {
|
||||
tracing::warn!(
|
||||
tool = name,
|
||||
secret = secret.name,
|
||||
prompt = secret.prompt,
|
||||
"setup.required_secrets prompt is shorter than {} chars — \
|
||||
consider a more descriptive prompt that tells the user where to find this value",
|
||||
MIN_PROMPT_LENGTH
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Manual auth (no OAuth) checks
|
||||
if let Some(auth) = &self.auth
|
||||
&& auth.oauth.is_none()
|
||||
{
|
||||
if auth.setup_url.is_none() {
|
||||
tracing::warn!(
|
||||
tool = name,
|
||||
"auth section has no OAuth and no setup_url — \
|
||||
user has no link to obtain credentials"
|
||||
);
|
||||
}
|
||||
if auth.instructions.is_none() {
|
||||
tracing::warn!(
|
||||
tool = name,
|
||||
"auth section has no OAuth and no instructions — \
|
||||
user has no guidance on how to obtain credentials"
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Convert to runtime Capabilities.
|
||||
pub fn to_capabilities(&self) -> Capabilities {
|
||||
let mut caps = Capabilities::default();
|
||||
@@ -1056,6 +1109,60 @@ mod tests {
|
||||
assert_eq!(caps.setup.unwrap().required_secrets[0].name, "my_secret");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_validate_setup_without_auth_warns() {
|
||||
// setup.required_secrets with no auth section — should not panic
|
||||
let json = r#"{
|
||||
"setup": {
|
||||
"required_secrets": [
|
||||
{ "name": "api_key", "prompt": "Enter your API key from the provider dashboard settings page" }
|
||||
]
|
||||
}
|
||||
}"#;
|
||||
|
||||
let caps = CapabilitiesFile::from_json(json).unwrap();
|
||||
// Should not panic; warning is emitted via tracing
|
||||
caps.validate("test-tool");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_validate_manual_auth_missing_fields() {
|
||||
// auth without OAuth, missing setup_url and instructions
|
||||
let json = r#"{
|
||||
"auth": {
|
||||
"secret_name": "my_api_key"
|
||||
}
|
||||
}"#;
|
||||
|
||||
let caps = CapabilitiesFile::from_json(json).unwrap();
|
||||
// Should not panic; warnings emitted for missing setup_url and instructions
|
||||
caps.validate("test-tool");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_validate_clean_tool() {
|
||||
// Well-configured tool with auth, setup_url, instructions, and good prompts
|
||||
let json = r#"{
|
||||
"auth": {
|
||||
"secret_name": "my_api_key",
|
||||
"setup_url": "https://example.com/api-keys",
|
||||
"instructions": "Go to example.com/api-keys and create a new key"
|
||||
},
|
||||
"setup": {
|
||||
"required_secrets": [
|
||||
{
|
||||
"name": "my_api_key",
|
||||
"prompt": "Enter your API key from https://example.com/api-keys"
|
||||
}
|
||||
]
|
||||
}
|
||||
}"#;
|
||||
|
||||
let caps = CapabilitiesFile::from_json(json).unwrap();
|
||||
// Should not panic and emits no warnings (has auth, setup_url, instructions, long prompt)
|
||||
caps.validate("clean-tool");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_resolve_nested_empty_capabilities_noop() {
|
||||
// Empty inner capabilities should not clobber outer http
|
||||
|
||||
@@ -126,6 +126,7 @@ impl WasmToolLoader {
|
||||
let cap_bytes = fs::read(cap_path).await?;
|
||||
let cap_file = CapabilitiesFile::from_bytes(&cap_bytes)
|
||||
.map_err(|e| WasmLoadError::InvalidCapabilities(e.to_string()))?;
|
||||
cap_file.validate(name);
|
||||
let caps = cap_file.to_capabilities();
|
||||
let oauth = resolve_oauth_refresh_config(&cap_file);
|
||||
(caps, oauth)
|
||||
|
||||
@@ -34,11 +34,19 @@
|
||||
]
|
||||
}
|
||||
},
|
||||
"auth": {
|
||||
"secret_name": "github_token",
|
||||
"display_name": "GitHub",
|
||||
"instructions": "Create a Personal Access Token at github.com/settings/tokens with repo scope, then paste it here.",
|
||||
"setup_url": "https://github.com/settings/tokens",
|
||||
"token_hint": "Starts with 'ghp_' or 'github_pat_'",
|
||||
"env_var": "GITHUB_TOKEN"
|
||||
},
|
||||
"setup": {
|
||||
"required_secrets": [
|
||||
{
|
||||
"name": "github_token",
|
||||
"prompt": "GitHub Personal Access Token (from github.com/settings/tokens)"
|
||||
"prompt": "GitHub Personal Access Token (create one at github.com/settings/tokens with 'repo' scope)"
|
||||
}
|
||||
]
|
||||
},
|
||||
|
||||
Reference in New Issue
Block a user