Skip to content

[Refactor] Create api/auth.rs and centralize API key validation #65

@oleander

Description

@oleander

Context

API key validation logic is duplicated in commit.rs and openai.rs. This creates a single source of truth.

Priority

🟠 MEDIUM - Removes duplication, improves maintainability

Steps

1. Create new file src/api/auth.rs

//! API authentication and key validation.

use anyhow::{bail, Result};
use crate::config::AppConfig;

/// Validates an API key is present and not a placeholder.
///
/// # Arguments
/// * `key` - The API key to validate
///
/// # Returns
/// * `Result<String>` - The validated key or an error
///
/// # Errors
/// Returns error if key is empty or is the placeholder value
pub fn validate_api_key(key: &str) -> Result<String> {
    if key.is_empty() || key == "<PLACE HOLDER FOR YOUR API KEY>" {
        bail!("Invalid or placeholder API key")
    }
    Ok(key.to_string())
}

/// Gets API key from config or environment, with validation.
///
/// # Arguments
/// * `config` - Application configuration
///
/// # Returns
/// * `Result<String>` - Validated API key
///
/// # Errors
/// Returns error if:
/// - No API key found in config or environment
/// - API key is invalid or placeholder
pub fn get_api_key(config: &AppConfig) -> Result<String> {
    // Try config first
    if let Some(key) = &config.openai_api_key {
        return validate_api_key(key);
    }
    
    // Try environment variable
    if let Ok(key) = std::env::var("OPENAI_API_KEY") {
        return validate_api_key(&key);
    }
    
    bail!(
        "OpenAI API key not found. Set via:\n\
         1. git-ai config set openai-api-key <your-key>\n\
         2. OPENAI_API_KEY environment variable"
    )
}

#[cfg(test)]
mod tests {
    use super::*;

    #[test]
    fn test_validate_valid_key() {
        assert!(validate_api_key("sk-valid-key-here").is_ok());
    }

    #[test]
    fn test_validate_empty_key() {
        assert!(validate_api_key("").is_err());
    }

    #[test]
    fn test_validate_placeholder() {
        assert!(validate_api_key("<PLACE HOLDER FOR YOUR API KEY>").is_err());
    }
}

2. Create src/api/mod.rs

pub mod auth;
pub mod openai;
pub mod ollama;

pub use auth::{get_api_key, validate_api_key};

3. Update src/lib.rs

// Add:
pub mod api;

// Keep existing:
pub mod openai;  // Re-export for compatibility
pub use api::openai;

4. Replace validation in src/commit.rs

Find and replace lines 86-106 with:

use crate::api::auth;

// In generate function:
let api_key = auth::get_api_key(config)
    .context("Failed to get API key")?;

5. Replace validation in src/openai.rs

Find and replace lines 113-126 with:

use crate::api::auth;

pub fn create_openai_config(settings: &AppConfig) -> Result<OpenAIConfig> {
    let api_key = auth::get_api_key(settings)?;
    Ok(OpenAIConfig::new().with_api_key(api_key))
}

6. Remove duplicate validation code

Delete all the old validation logic from both files.

Verification Criteria

Pass:

  • src/api/auth.rs created with validation functions
  • src/api/mod.rs created and exports auth module
  • All API key validation goes through auth::get_api_key()
  • No duplicate validation logic remains in commit.rs or openai.rs
  • Unit tests in auth.rs pass
  • cargo test passes all tests
  • cargo clippy shows no warnings
  • Error messages are clear and actionable

Test the change

# Test with no API key
unset OPENAI_API_KEY
cargo run -- config reset
# Should show helpful error message

# Test with valid key
cargo run -- config set openai-api-key sk-test-key
# Should work

# Test with placeholder
cargo run -- config set openai-api-key "<PLACE HOLDER FOR YOUR API KEY>"
# Should show error when trying to use

Estimated Time

2-3 hours

Dependencies

Labels

  • refactor
  • api
  • validation

Metadata

Metadata

Assignees

No one assigned

    Labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions