1
0
Fork 0
mirror of https://github.com/imjasonh/krust synced 2026-07-07 22:35:25 +00:00

refactor: consolidate authentication systems

Remove the custom keychain implementation and keep only the simple wrapper
around oci-distribution's built-in auth. This change:

- Removes 300+ lines of duplicate authentication code
- Leverages well-tested oci-distribution auth functionality
- Simplifies the codebase and reduces maintenance burden
- Maintains full backward compatibility

The custom keychain implementation was never used in production code,
only the simple resolve_auth function was called. All tests have been
updated and are passing.

Fixes #17

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
Jason Hall 2025-06-08 16:49:35 -04:00
parent e497d78fea
commit 047d6de9ef
Failed to extract signature
4 changed files with 21 additions and 548 deletions

View file

@ -1,364 +0,0 @@
//! Keychain implementation for credential management
use super::{Anonymous, AuthConfig, Authenticator, DockerAuthEntry, DockerConfig};
use anyhow::{Context, Result};
use std::collections::HashMap;
use std::path::PathBuf;
use std::sync::{Arc, Mutex};
use tracing::{debug, warn};
/// Trait for types that can resolve authentication for a given resource
pub trait Keychain: Send + Sync {
/// Resolve authentication for a given resource (registry URL or image reference)
fn resolve(&self, resource: &str) -> Result<Box<dyn Authenticator>>;
}
/// Default keychain implementation that checks Docker config files
pub struct DefaultKeychain {
/// Cached config to avoid re-reading files
config_cache: Arc<Mutex<Option<DockerConfig>>>,
}
impl DefaultKeychain {
/// Create a new DefaultKeychain
pub fn new() -> Self {
Self {
config_cache: Arc::new(Mutex::new(None)),
}
}
/// Get paths to check for Docker config
fn config_paths() -> Vec<PathBuf> {
let mut paths = Vec::new();
// Check DOCKER_CONFIG environment variable
if let Ok(docker_config) = std::env::var("DOCKER_CONFIG") {
paths.push(PathBuf::from(docker_config).join("config.json"));
}
// Check REGISTRY_AUTH_FILE environment variable
if let Ok(auth_file) = std::env::var("REGISTRY_AUTH_FILE") {
paths.push(PathBuf::from(auth_file));
}
// Check XDG_RUNTIME_DIR for containers auth
if let Ok(xdg_runtime) = std::env::var("XDG_RUNTIME_DIR") {
paths.push(PathBuf::from(xdg_runtime).join("containers/auth.json"));
}
// Check default Docker config location
if let Some(home) = dirs::home_dir() {
paths.push(home.join(".docker/config.json"));
}
paths
}
/// Load Docker config from disk
fn load_config(&self) -> Result<DockerConfig> {
// Check cache first
{
let cache = self.config_cache.lock().unwrap();
if let Some(config) = cache.as_ref() {
return Ok(config.clone());
}
}
// Try each config path
for path in Self::config_paths() {
if path.exists() {
debug!("Checking Docker config at: {}", path.display());
match std::fs::read_to_string(&path) {
Ok(content) => {
match serde_json::from_str::<DockerConfig>(&content) {
Ok(config) => {
debug!("Loaded Docker config from: {}", path.display());
// Cache the config
let mut cache = self.config_cache.lock().unwrap();
*cache = Some(config.clone());
return Ok(config);
}
Err(e) => {
warn!("Failed to parse Docker config at {}: {}", path.display(), e);
}
}
}
Err(e) => {
warn!("Failed to read Docker config at {}: {}", path.display(), e);
}
}
}
}
// Return empty config if no valid config found
Ok(DockerConfig {
auths: HashMap::new(),
cred_helpers: HashMap::new(),
creds_store: None,
})
}
/// Extract registry from image reference
fn extract_registry(image_ref: &str) -> &str {
// Handle different image reference formats:
// - docker.io/library/ubuntu:latest -> docker.io
// - gcr.io/project/image:tag -> gcr.io
// - localhost:5000/image -> localhost:5000
// - ubuntu:latest -> docker.io (implicit)
if let Some(slash_pos) = image_ref.find('/') {
let registry_part = &image_ref[..slash_pos];
// Check if this looks like a registry (contains . or :)
if registry_part.contains('.') || registry_part.contains(':') {
return registry_part;
}
}
// Default to Docker Hub
"index.docker.io"
}
/// Normalize registry URL for matching
fn normalize_registry(registry: &str) -> Vec<String> {
let mut variants = vec![registry.to_string()];
// Add common variants
if registry == "docker.io" || registry == "index.docker.io" {
variants.push("docker.io".to_string());
variants.push("index.docker.io".to_string());
variants.push("https://index.docker.io/v1/".to_string());
variants.push("https://index.docker.io/v2/".to_string());
} else if !registry.starts_with("http://") && !registry.starts_with("https://") {
// Add protocol variants
variants.push(format!("https://{}", registry));
variants.push(format!("http://{}", registry));
// Add /v1/ and /v2/ variants
variants.push(format!("https://{}/v1/", registry));
variants.push(format!("https://{}/v2/", registry));
}
variants
}
/// Find auth entry for a registry
fn find_auth_entry(&self, config: &DockerConfig, registry: &str) -> Option<DockerAuthEntry> {
let variants = Self::normalize_registry(registry);
for variant in variants {
if let Some(entry) = config.auths.get(&variant) {
return Some(entry.clone());
}
}
None
}
/// Get credential helper for a registry
fn get_credential_helper(&self, config: &DockerConfig, registry: &str) -> Option<String> {
// Check specific credential helper for registry
if let Some(helper) = config.cred_helpers.get(registry) {
return Some(helper.clone());
}
// Check default credential store
config.creds_store.clone()
}
/// Execute credential helper to get credentials
fn execute_credential_helper(&self, helper: &str, registry: &str) -> Result<AuthConfig> {
use std::io::Write;
use std::process::{Command, Stdio};
let helper_name = format!("docker-credential-{}", helper);
debug!(
"Executing credential helper: {} for {}",
helper_name, registry
);
let mut child = Command::new(&helper_name)
.arg("get")
.stdin(Stdio::piped())
.stdout(Stdio::piped())
.stderr(Stdio::piped())
.spawn()
.context(format!(
"Failed to spawn credential helper: {}",
helper_name
))?;
// Write registry URL to stdin
if let Some(mut stdin) = child.stdin.take() {
stdin.write_all(registry.as_bytes())?;
stdin.write_all(b"\n")?;
}
let output = child.wait_with_output()?;
if !output.status.success() {
let stderr = String::from_utf8_lossy(&output.stderr);
anyhow::bail!("Credential helper {} failed: {}", helper_name, stderr);
}
// Parse output as JSON
#[derive(serde::Deserialize)]
struct HelperResponse {
#[serde(rename = "Username")]
username: Option<String>,
#[serde(rename = "Secret")]
secret: Option<String>,
#[serde(rename = "ServerURL")]
_server_url: Option<String>,
}
let response: HelperResponse = serde_json::from_slice(&output.stdout)
.context("Failed to parse credential helper response")?;
Ok(AuthConfig {
username: response.username,
password: response.secret,
..Default::default()
})
}
}
impl Default for DefaultKeychain {
fn default() -> Self {
Self::new()
}
}
impl Keychain for DefaultKeychain {
fn resolve(&self, resource: &str) -> Result<Box<dyn Authenticator>> {
let config = self.load_config()?;
let registry = Self::extract_registry(resource);
debug!(
"Resolving auth for resource: {} (registry: {})",
resource, registry
);
// Try to find auth entry in config
if let Some(auth_entry) = self.find_auth_entry(&config, registry) {
debug!("Found auth entry for {}", registry);
let auth_config = auth_entry.to_auth_config();
// Return appropriate authenticator based on auth type
if auth_config.is_anonymous() {
return Ok(Box::new(Anonymous));
}
return Ok(Box::new(ConfigAuthenticator {
config: auth_config,
}));
}
// Try credential helper
if let Some(helper) = self.get_credential_helper(&config, registry) {
debug!("Trying credential helper: {} for {}", helper, registry);
match self.execute_credential_helper(&helper, registry) {
Ok(auth_config) => {
return Ok(Box::new(ConfigAuthenticator {
config: auth_config,
}));
}
Err(e) => {
warn!("Credential helper failed: {}", e);
}
}
}
// Default to anonymous
debug!("No credentials found for {}, using anonymous", registry);
Ok(Box::new(Anonymous))
}
}
/// Authenticator that returns a fixed AuthConfig
struct ConfigAuthenticator {
config: AuthConfig,
}
impl Authenticator for ConfigAuthenticator {
fn authorization(&self) -> Result<AuthConfig> {
Ok(self.config.clone())
}
}
/// Multi-keychain that tries multiple keychains in order
pub struct MultiKeychain {
keychains: Vec<Box<dyn Keychain>>,
}
impl MultiKeychain {
/// Create a new MultiKeychain
#[allow(dead_code)]
pub fn new(keychains: Vec<Box<dyn Keychain>>) -> Self {
Self { keychains }
}
}
impl Keychain for MultiKeychain {
fn resolve(&self, resource: &str) -> Result<Box<dyn Authenticator>> {
for keychain in &self.keychains {
match keychain.resolve(resource) {
Ok(auth) => {
// Check if it's not anonymous
if let Ok(config) = auth.authorization() {
if !config.is_anonymous() {
return Ok(auth);
}
}
}
Err(e) => {
debug!("Keychain failed: {}", e);
}
}
}
// Default to anonymous
Ok(Box::new(Anonymous))
}
}
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn test_extract_registry() {
assert_eq!(
DefaultKeychain::extract_registry("docker.io/library/ubuntu:latest"),
"docker.io"
);
assert_eq!(
DefaultKeychain::extract_registry("gcr.io/project/image:tag"),
"gcr.io"
);
assert_eq!(
DefaultKeychain::extract_registry("localhost:5000/image"),
"localhost:5000"
);
assert_eq!(
DefaultKeychain::extract_registry("ubuntu:latest"),
"index.docker.io"
);
assert_eq!(
DefaultKeychain::extract_registry("user/image:tag"),
"index.docker.io"
);
}
#[test]
fn test_normalize_registry() {
let variants = DefaultKeychain::normalize_registry("docker.io");
assert!(variants.contains(&"docker.io".to_string()));
assert!(variants.contains(&"index.docker.io".to_string()));
let variants = DefaultKeychain::normalize_registry("gcr.io");
assert!(variants.contains(&"gcr.io".to_string()));
assert!(variants.contains(&"https://gcr.io".to_string()));
}
}

View file

@ -8,10 +8,8 @@ use base64::Engine;
use serde::{Deserialize, Serialize};
use std::collections::HashMap;
mod keychain;
mod simple;
pub use keychain::{DefaultKeychain, Keychain};
pub use simple::resolve_auth;
/// Authentication configuration containing credentials
@ -114,62 +112,6 @@ impl AuthConfig {
}
}
/// Trait for types that can provide authentication
pub trait Authenticator: Send + Sync {
/// Get the authentication configuration
fn authorization(&self) -> Result<AuthConfig>;
}
/// Anonymous authenticator
pub struct Anonymous;
impl Authenticator for Anonymous {
fn authorization(&self) -> Result<AuthConfig> {
Ok(AuthConfig::anonymous())
}
}
/// Basic authenticator with username and password
pub struct Basic {
username: String,
password: String,
}
impl Basic {
pub fn new(username: String, password: String) -> Self {
Self { username, password }
}
}
impl Authenticator for Basic {
fn authorization(&self) -> Result<AuthConfig> {
Ok(AuthConfig::new(
self.username.clone(),
self.password.clone(),
))
}
}
/// Bearer token authenticator
pub struct Bearer {
token: String,
}
impl Bearer {
pub fn new(token: String) -> Self {
Self { token }
}
}
impl Authenticator for Bearer {
fn authorization(&self) -> Result<AuthConfig> {
Ok(AuthConfig {
registry_token: Some(self.token.clone()),
..Default::default()
})
}
}
/// Docker config file structure
#[derive(Debug, Clone, Deserialize, Serialize)]
pub struct DockerConfig {

View file

@ -1,17 +1,6 @@
//! Tests for the auth module
use super::*;
use crate::auth::keychain::MultiKeychain;
use std::fs;
use std::path::{Path, PathBuf};
use tempfile::TempDir;
/// Helper to create a test Docker config file
fn create_test_config(dir: &Path, content: &str) -> PathBuf {
let config_path = dir.join("config.json");
fs::write(&config_path, content).unwrap();
config_path
}
#[test]
fn test_docker_config_parsing() {
@ -67,103 +56,3 @@ fn test_docker_auth_entry_to_auth_config() {
assert!(config.username.is_none());
assert!(config.password.is_none());
}
#[test]
fn test_default_keychain_config_paths() {
let temp_dir = TempDir::new().unwrap();
let config_content = r#"{
"auths": {
"test.registry.io": {
"auth": "dGVzdDp0ZXN0"
}
}
}"#;
// Test DOCKER_CONFIG env var
let docker_config_dir = temp_dir.path().join("docker");
fs::create_dir_all(&docker_config_dir).unwrap();
create_test_config(&docker_config_dir, config_content);
std::env::set_var("DOCKER_CONFIG", docker_config_dir.to_str().unwrap());
let keychain = DefaultKeychain::new();
let auth = keychain.resolve("test.registry.io/image:tag").unwrap();
let auth_config = auth.authorization().unwrap();
assert!(!auth_config.is_anonymous());
assert_eq!(auth_config.auth, Some("dGVzdDp0ZXN0".to_string()));
std::env::remove_var("DOCKER_CONFIG");
}
// Registry extraction tests are skipped as the method is private
// The functionality is tested through the public resolve() method
#[test]
fn test_keychain_resolve_different_registries() {
// Test that keychain can resolve different registry formats
let keychain = DefaultKeychain::new();
// Should return anonymous for unknown registries
let auth = keychain.resolve("unknown.registry.io/image:tag").unwrap();
let config = auth.authorization().unwrap();
assert!(config.is_anonymous());
}
#[test]
fn test_anonymous_authenticator() {
let anon = Anonymous;
let auth = anon.authorization().unwrap();
assert!(auth.is_anonymous());
assert!(auth.to_authorization_header().unwrap().is_none());
}
#[test]
fn test_basic_authenticator() {
let basic = Basic::new("user".to_string(), "pass".to_string());
let auth = basic.authorization().unwrap();
assert!(!auth.is_anonymous());
let header = auth.to_authorization_header().unwrap().unwrap();
assert!(header.starts_with("Basic "));
}
#[test]
fn test_bearer_authenticator() {
let bearer = Bearer::new("token123".to_string());
let auth = bearer.authorization().unwrap();
assert!(!auth.is_anonymous());
let header = auth.to_authorization_header().unwrap().unwrap();
assert_eq!(header, "Bearer token123");
}
#[test]
fn test_multi_keychain() {
// Create a keychain that always returns anonymous
struct AlwaysAnonymous;
impl Keychain for AlwaysAnonymous {
fn resolve(&self, _: &str) -> Result<Box<dyn Authenticator>> {
Ok(Box::new(Anonymous))
}
}
// Create a keychain that returns basic auth
struct AlwaysBasic;
impl Keychain for AlwaysBasic {
fn resolve(&self, _: &str) -> Result<Box<dyn Authenticator>> {
Ok(Box::new(Basic::new("user".to_string(), "pass".to_string())))
}
}
// Test that MultiKeychain returns the first non-anonymous auth
let multi = MultiKeychain::new(vec![
Box::new(AlwaysAnonymous) as Box<dyn Keychain>,
Box::new(AlwaysBasic) as Box<dyn Keychain>,
]);
let auth = multi.resolve("any.registry.io").unwrap();
let config = auth.authorization().unwrap();
assert!(!config.is_anonymous());
assert_eq!(config.username, Some("user".to_string()));
}

View file

@ -1,7 +1,8 @@
//! Integration tests for authentication
use anyhow::Result;
use krust::auth::{AuthConfig, DefaultKeychain, Keychain};
use krust::auth::{resolve_auth, AuthConfig};
use oci_distribution::secrets::RegistryAuth;
use std::fs;
use tempfile::TempDir;
@ -30,25 +31,30 @@ fn test_auth_integration_with_docker_config() -> Result<()> {
// Set HOME to temp directory
std::env::set_var("HOME", temp_dir.path());
let keychain = DefaultKeychain::new();
// Test GitHub Container Registry auth
let ghcr_auth = keychain.resolve("ghcr.io/user/image:tag")?;
let ghcr_config = ghcr_auth.authorization()?;
assert!(!ghcr_config.is_anonymous());
assert_eq!(ghcr_config.auth, Some("dGVzdDp0ZXN0MTIz".to_string()));
let ghcr_auth = resolve_auth("ghcr.io/user/image:tag")?;
match ghcr_auth {
RegistryAuth::Basic(user, pass) => {
// The base64 "dGVzdDp0ZXN0MTIz" decodes to "test:test123"
assert_eq!(user, "test");
assert_eq!(pass, "test123");
}
_ => panic!("Expected Basic auth for ghcr.io"),
}
// Test Docker Hub auth
let docker_auth = keychain.resolve("docker.io/library/ubuntu:latest")?;
let docker_config = docker_auth.authorization()?;
assert!(!docker_config.is_anonymous());
assert_eq!(docker_config.username, Some("testuser".to_string()));
assert_eq!(docker_config.password, Some("testpass".to_string()));
let docker_auth = resolve_auth("docker.io/library/ubuntu:latest")?;
match docker_auth {
RegistryAuth::Basic(user, pass) => {
assert_eq!(user, "testuser");
assert_eq!(pass, "testpass");
}
_ => panic!("Expected Basic auth for docker.io"),
}
// Test unknown registry returns anonymous
let unknown_auth = keychain.resolve("unknown.registry.io/image:tag")?;
let unknown_config = unknown_auth.authorization()?;
assert!(unknown_config.is_anonymous());
let unknown_auth = resolve_auth("unknown.registry.io/image:tag")?;
assert!(matches!(unknown_auth, RegistryAuth::Anonymous));
// Clean up
std::env::remove_var("HOME");