feat: Improve API key validation with format-specific detection

Backend changes: - Detect provider from key format first (sk-ant- = Anthropic, sk- = OpenAI, etc.) - Only test the detected provider instead of all providers - Return structured error responses with suggestions - Add DetectProviderError struct with detailed feedback Frontend changes: - Handle new structured error responses - Show provider-specific error messages - Display helpful suggestions when validation fails - Improve loading state to show 'Testing API key...' Fixes issue where wrong keys would test against all providers. Now users get clear feedback about their specific key type.
2026-07-17 12:56:20 +02:00 · 2025-10-30 10:48:20 -04:00
parent c5cfcc9ff0
commit c9f2beecbd
6 changed files with 282 additions and 93 deletions
@@ -389,6 +389,7 @@ derive_utoipa!(Icon as IconSchema);
        super::routes::config_management::ConfigKeyQuery,
        super::routes::config_management::DetectProviderRequest,
        super::routes::config_management::DetectProviderResponse,
+        super::routes::config_management::DetectProviderError,
        super::routes::config_management::ConfigResponse,
        super::routes::config_management::ProvidersResponse,
        super::routes::config_management::ProviderDetails,
@@ -99,6 +99,13 @@ pub struct DetectProviderResponse {
    pub provider_name: String,
    pub models: Vec<String>,
 }
+
+#[derive(Serialize, ToSchema)]
+pub struct DetectProviderError {
+    pub error: String,
+    pub detected_format: Option<String>,
+    pub suggestions: Vec<String>,
+}
 #[utoipa::path(
    post,
    path = "/config/upsert",
@@ -533,19 +540,71 @@ pub async fn upsert_permissions(
    request_body = DetectProviderRequest,
    responses(
        (status = 200, description = "Provider detected successfully", body = DetectProviderResponse),
-        (status = 404, description = "No matching provider found"),
+        (status = 400, description = "Invalid API key format or key validation failed", body = DetectProviderError),
        (status = 500, description = "Internal server error")
    )
 )]
 pub async fn detect_provider(
    Json(detect_request): Json<DetectProviderRequest>,
-) -> Result<Json<DetectProviderResponse>, StatusCode> {
-    match auto_detect::detect_provider_from_api_key(&detect_request.api_key).await {
+) -> Result<Json<DetectProviderResponse>, (StatusCode, Json<DetectProviderError>)> {
+    let api_key = detect_request.api_key.trim();
+
+    // Detect the expected provider from key format
+    let detected_format = detect_key_format(api_key);
+
+    match auto_detect::detect_provider_from_api_key(api_key).await {
        Some((provider_name, models)) => Ok(Json(DetectProviderResponse {
            provider_name,
            models,
        })),
-        None => Err(StatusCode::NOT_FOUND),
+        None => {
+            let (error_msg, suggestions) = if let Some(format) = &detected_format {
+                (
+                    format!("API key appears to be for {} but validation failed", format),
+                    vec![
+                        "Check that your API key is correct and complete".to_string(),
+                        "Verify your account has sufficient credits or is active".to_string(),
+                        "Ensure the API key has the necessary permissions".to_string(),
+                    ],
+                )
+            } else {
+                (
+                    "Could not determine provider from API key format".to_string(),
+                    vec![
+                        "Supported formats: Anthropic (sk-ant-...), OpenAI (sk-...), Google (AIza...), Groq (gsk_...)".to_string(),
+                        "Make sure you copied the complete API key".to_string(),
+                        "Try setting up the provider manually in settings".to_string(),
+                    ]
+                )
+            };
+
+            Err((
+                StatusCode::BAD_REQUEST,
+                Json(DetectProviderError {
+                    error: error_msg,
+                    detected_format,
+                    suggestions,
+                }),
+            ))
+        }
+    }
+}
+
+fn detect_key_format(api_key: &str) -> Option<String> {
+    let trimmed_key = api_key.trim();
+
+    if trimmed_key.starts_with("sk-ant-") {
+        Some("Anthropic".to_string())
+    } else if trimmed_key.starts_with("sk-") {
+        Some("OpenAI".to_string())
+    } else if trimmed_key.starts_with("AIza") {
+        Some("Google".to_string())
+    } else if trimmed_key.starts_with("gsk_") {
+        Some("Groq".to_string())
+    } else if trimmed_key.starts_with("xai-") {
+        Some("xAI".to_string())
+    } else {
+        None
    }
 }

@@ -1,51 +1,94 @@
 use crate::model::ModelConfig;

-pub async fn detect_provider_from_api_key(api_key: &str) -> Option<(String, Vec<String>)> {
-    let provider_tests = vec![
-        ("anthropic", "ANTHROPIC_API_KEY"),
-        ("openai", "OPENAI_API_KEY"),
-        ("google", "GOOGLE_API_KEY"),
-        ("groq", "GROQ_API_KEY"),
-        ("xai", "XAI_API_KEY"),
-        ("ollama", "OLLAMA_API_KEY"),
-    ];
+/// Detect the provider based on API key format
+fn detect_provider_from_key_format(api_key: &str) -> Option<&'static str> {
+    let trimmed_key = api_key.trim();

-    let tasks: Vec<_> = provider_tests
-        .into_iter()
-        .map(|(provider_name, env_key)| {
-            let api_key = api_key.to_string();
-            tokio::spawn(async move {
-                let original_value = std::env::var(env_key).ok();
-                std::env::set_var(env_key, &api_key);
-
-                let result = match crate::providers::create(
-                    provider_name,
-                    ModelConfig::new_or_fail("default"),
-                )
-                .await
-                {
-                    Ok(provider) => match provider.fetch_supported_models().await {
-                        Ok(Some(models)) => Some((provider_name.to_string(), models)),
-                        _ => None,
-                    },
-                    Err(_) => None,
-                };
-
-                match original_value {
-                    Some(val) => std::env::set_var(env_key, val),
-                    None => std::env::remove_var(env_key),
-                }
-
-                result
-            })
-        })
-        .collect();
-
-    for task in tasks {
-        if let Ok(Some(result)) = task.await {
-            return Some(result);
-        }
+    // Anthropic keys start with sk-ant-
+    if trimmed_key.starts_with("sk-ant-") {
+        return Some("anthropic");
    }

+    // OpenAI keys start with sk- but not sk-ant-
+    if trimmed_key.starts_with("sk-") && !trimmed_key.starts_with("sk-ant-") {
+        return Some("openai");
+    }
+
+    // Google keys typically start with AIza
+    if trimmed_key.starts_with("AIza") {
+        return Some("google");
+    }
+
+    // Groq keys start with gsk_
+    if trimmed_key.starts_with("gsk_") {
+        return Some("groq");
+    }
+
+    // xAI keys start with xai-
+    if trimmed_key.starts_with("xai-") {
+        return Some("xai");
+    }
+
+    // If we can't detect the format, return None
    None
 }
+
+pub async fn detect_provider_from_api_key(api_key: &str) -> Option<(String, Vec<String>)> {
+    // First, try to detect the provider from the key format
+    if let Some(detected_provider) = detect_provider_from_key_format(api_key) {
+        // Test only the detected provider
+        let env_key = match detected_provider {
+            "anthropic" => "ANTHROPIC_API_KEY",
+            "openai" => "OPENAI_API_KEY",
+            "google" => "GOOGLE_API_KEY",
+            "groq" => "GROQ_API_KEY",
+            "xai" => "XAI_API_KEY",
+            _ => return None,
+        };
+
+        // Store original value and set the test key
+        let original_value = std::env::var(env_key).ok();
+        std::env::set_var(env_key, api_key);
+
+        let result =
+            match crate::providers::create(detected_provider, ModelConfig::new_or_fail("default"))
+                .await
+            {
+                Ok(provider) => match provider.fetch_supported_models().await {
+                    Ok(Some(models)) => Some((detected_provider.to_string(), models)),
+                    _ => None,
+                },
+                Err(_) => None,
+            };
+
+        // Restore original value
+        match original_value {
+            Some(val) => std::env::set_var(env_key, val),
+            None => std::env::remove_var(env_key),
+        }
+
+        return result;
+    }
+
+    // If we can't detect the format, try Ollama as a fallback
+    // (since Ollama keys don't have a standard format)
+    let original_value = std::env::var("OLLAMA_API_KEY").ok();
+    std::env::set_var("OLLAMA_API_KEY", api_key);
+
+    let result = match crate::providers::create("ollama", ModelConfig::new_or_fail("default")).await
+    {
+        Ok(provider) => match provider.fetch_supported_models().await {
+            Ok(Some(models)) => Some(("ollama".to_string(), models)),
+            _ => None,
+        },
+        Err(_) => None,
+    };
+
+    // Restore original value
+    match original_value {
+        Some(val) => std::env::set_var("OLLAMA_API_KEY", val),
+        None => std::env::remove_var("OLLAMA_API_KEY"),
+    }
+
+    result
+}
@@ -559,8 +559,15 @@
              }
            }
          },
-          "404": {
-            "description": "No matching provider found"
+          "400": {
+            "description": "Invalid API key format or key validation failed",
+            "content": {
+              "application/json": {
+                "schema": {
+                  "$ref": "#/components/schemas/DetectProviderError"
+                }
+              }
+            }
          },
          "500": {
            "description": "Internal server error"
@@ -2440,6 +2447,28 @@
          }
        }
      },
+      "DetectProviderError": {
+        "type": "object",
+        "required": [
+          "error",
+          "suggestions"
+        ],
+        "properties": {
+          "detected_format": {
+            "type": "string",
+            "nullable": true
+          },
+          "error": {
+            "type": "string"
+          },
+          "suggestions": {
+            "type": "array",
+            "items": {
+              "type": "string"
+            }
+          }
+        }
+      },
      "DetectProviderRequest": {
        "type": "object",
        "required": [
@@ -4950,4 +4979,4 @@
      }
    }
  }
-}
+}
@@ -118,6 +118,12 @@ export type DeleteRecipeRequest = {
    id: string;
 };

+export type DetectProviderError = {
+    detected_format?: string | null;
+    error: string;
+    suggestions: Array<string>;
+};
+
 export type DetectProviderRequest = {
    api_key: string;
 };
@@ -1342,15 +1348,17 @@ export type DetectProviderData = {

 export type DetectProviderErrors = {
    /**
-     * No matching provider found
+     * Invalid API key format or key validation failed
     */
-    404: unknown;
+    400: DetectProviderError;
    /**
     * Internal server error
     */
    500: unknown;
 };

+export type DetectProviderError2 = DetectProviderErrors[keyof DetectProviderErrors];
+
 export type DetectProviderResponses = {
    /**
     * Provider detected successfully
@@ -16,12 +16,17 @@ interface TestResult {
  model?: string;
  totalModels?: number;
  error?: string;
+  detectedFormat?: string;
+  suggestions?: string[];
 }

 interface ApiError {
  response?: {
+    status?: number;
    data?: {
-      message?: string;
+      error?: string;
+      detected_format?: string;
+      suggestions?: string[];
    };
  };
  message?: string;
@@ -90,21 +95,48 @@ export default function ApiKeyTester({ onSuccess }: ApiKeyTesterProps) {
      console.error('Detection failed:', error);
      
      const apiError = error as ApiError;
-      const errorMessage = apiError.response?.data?.message || 
-                          apiError.message || 
-                          'Could not detect provider. Please check your API key.';
      
-      setTestResults([{
-        provider: 'Unknown',
-        success: false,
-        error: errorMessage,
-      }]);
+      // Handle the new structured error response
+      if (apiError.response?.status === 400 && apiError.response.data) {
+        const errorData = apiError.response.data;
+        
+        setTestResults([{
+          provider: errorData.detected_format || 'Unknown',
+          success: false,
+          error: errorData.error || 'API key validation failed',
+          detectedFormat: errorData.detected_format,
+          suggestions: errorData.suggestions,
+        }]);

-      toastService.error({
-        title: 'Detection Failed',
-        msg: 'Could not validate API key. Please check the key and try again.',
-        traceback: error instanceof Error ? error.stack || '' : '',
-      });
+        // Show detailed error message
+        let errorMessage = errorData.error || 'Could not validate API key.';
+        if (errorData.suggestions && errorData.suggestions.length > 0) {
+          errorMessage += '\n\nSuggestions:\n' + errorData.suggestions.map(s => `• ${s}`).join('\n');
+        }
+
+        toastService.error({
+          title: errorData.detected_format ? `${errorData.detected_format} Key Invalid` : 'Detection Failed',
+          msg: errorMessage,
+          traceback: '',
+        });
+      } else {
+        // Fallback for other errors
+        const errorMessage = apiError.response?.data?.error || 
+                            apiError.message || 
+                            'Could not detect provider. Please check your API key.';
+        
+        setTestResults([{
+          provider: 'Unknown',
+          success: false,
+          error: errorMessage,
+        }]);
+
+        toastService.error({
+          title: 'Detection Failed',
+          msg: 'Could not validate API key. Please check the key and try again.',
+          traceback: error instanceof Error ? error.stack || '' : '',
+        });
+      }
    } finally {
      setIsLoading(false);
    }
@@ -167,14 +199,10 @@ export default function ApiKeyTester({ onSuccess }: ApiKeyTesterProps) {
          {/* Loading state */}
          {isLoading && (
            <div className="space-y-2">
-              <p className="text-sm text-text-muted">Testing providers...</p>
-              <div className="flex flex-wrap gap-2">
-                {['Anthropic', 'OpenAI', 'Google', 'Groq', 'xAI', 'Ollama'].map(provider => (
-                  <div key={provider} className="flex items-center gap-1 px-2 py-1 bg-background-muted rounded text-xs">
-                    <div className="w-2 h-2 border-2 border-current border-t-transparent rounded-full animate-spin"></div>
-                    <span>{provider}</span>
-                  </div>
-                ))}
+              <p className="text-sm text-text-muted">Testing API key...</p>
+              <div className="flex items-center gap-2 px-3 py-2 bg-background-muted rounded text-sm">
+                <div className="w-3 h-3 border-2 border-current border-t-transparent rounded-full animate-spin"></div>
+                <span>Validating key format and testing connection...</span>
              </div>
            </div>
          )}
@@ -185,24 +213,45 @@ export default function ApiKeyTester({ onSuccess }: ApiKeyTesterProps) {
              <h4 className="font-medium text-text-standard text-sm">Test Results:</h4>
              <div className="space-y-1">
                {testResults.map((result, index) => (
-                  <div
-                    key={index}
-                    className={`flex items-center gap-2 text-sm p-2 rounded ${
-                      result.success
-                        ? 'bg-green-50 text-green-800 border border-green-200 dark:bg-green-900/20 dark:text-green-200 dark:border-green-800'
-                        : 'bg-red-50 text-red-800 border border-red-200 dark:bg-red-900/20 dark:text-red-200 dark:border-red-800'
-                    }`}
-                  >
-                    <span>{result.success ? '✅' : '❌'}</span>
-                    <span className="font-medium">{result.provider}</span>
-                    {result.success && result.model && (
-                      <span className="text-green-600 dark:text-green-400">
-                        - {result.model}
-                        {result.totalModels && ` (${result.totalModels} models available)`}
-                      </span>
-                    )}
-                    {!result.success && result.error && (
-                      <span className="text-red-600 dark:text-red-400">- {result.error}</span>
+                  <div key={index} className="space-y-2">
+                    <div
+                      className={`flex items-center gap-2 text-sm p-3 rounded-lg ${
+                        result.success
+                          ? 'bg-green-50 text-green-800 border border-green-200 dark:bg-green-900/20 dark:text-green-200 dark:border-green-800'
+                          : 'bg-red-50 text-red-800 border border-red-200 dark:bg-red-900/20 dark:text-red-200 dark:border-red-800'
+                      }`}
+                    >
+                      <span>{result.success ? '✅' : '❌'}</span>
+                      <div className="flex-1">
+                        <div className="font-medium">
+                          {result.success ? `Detected ${result.provider}` : 
+                           result.detectedFormat ? `${result.detectedFormat} Key Invalid` : 
+                           'Detection Failed'}
+                        </div>
+                        {result.success && result.model && (
+                          <div className="text-green-600 dark:text-green-400 text-xs mt-1">
+                            Model: {result.model}
+                            {result.totalModels && ` (${result.totalModels} models available)`}
+                          </div>
+                        )}
+                        {!result.success && result.error && (
+                          <div className="text-red-600 dark:text-red-400 text-xs mt-1">
+                            {result.error}
+                          </div>
+                        )}
+                      </div>
+                    </div>
+                    
+                    {/* Show suggestions for failed attempts */}
+                    {!result.success && result.suggestions && result.suggestions.length > 0 && (
+                      <div className="ml-6 p-2 bg-yellow-50 dark:bg-yellow-900/20 border border-yellow-200 dark:border-yellow-800 rounded text-xs">
+                        <div className="font-medium text-yellow-800 dark:text-yellow-200 mb-1">Suggestions:</div>
+                        <ul className="text-yellow-700 dark:text-yellow-300 space-y-1">
+                          {result.suggestions.map((suggestion, i) => (
+                            <li key={i}>• {suggestion}</li>
+                          ))}
+                        </ul>
+                      </div>
                    )}
                  </div>
                ))}