From 185c6b0be46b0b1fb621749158ae58117de62653 Mon Sep 17 00:00:00 2001 From: perf3ct Date: Sun, 17 Aug 2025 03:12:29 +0000 Subject: [PATCH] feat(jwt): don't manage jwt like this --- .env.example | 4 +- .gitignore | 6 - Cargo.toml | 2 +- charts/readur/README.md | 108 ---------------- charts/readur/templates/release.yaml | 13 -- charts/readur/templates/secret.yaml | 25 ---- charts/readur/values.yaml | 16 --- docker-compose.yml | 7 +- src/config.rs | 93 +++---------- tests/integration_jwt_secret_tests.rs | 179 -------------------------- 10 files changed, 19 insertions(+), 434 deletions(-) delete mode 100644 charts/readur/README.md delete mode 100644 charts/readur/templates/secret.yaml delete mode 100644 tests/integration_jwt_secret_tests.rs diff --git a/.env.example b/.env.example index 503661b..8ef0d1c 100644 --- a/.env.example +++ b/.env.example @@ -1,8 +1,6 @@ # Core Configuration DATABASE_URL=postgresql://readur:readur_password@localhost:5432/readur -# JWT_SECRET is auto-generated on first run and stored in ./secrets/jwt_secret -# Uncomment below to override with your own secret: -# JWT_SECRET=your-super-secret-jwt-key-change-this-in-production +JWT_SECRET=your-super-secret-jwt-key-change-this-in-production SERVER_ADDRESS=0.0.0.0:8000 # File Storage & Upload diff --git a/.gitignore b/.gitignore index 13b6fa0..38550a9 100644 --- a/.gitignore +++ b/.gitignore @@ -18,11 +18,5 @@ readur_watch/ test-results/ uploads/ -# Secrets - NEVER commit these -secrets/ -readur_secrets/ -jwt_secret -.jwt_secret - # Misc. .claude/settings.local.json diff --git a/Cargo.toml b/Cargo.toml index 2370286..8e70c1e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -60,7 +60,6 @@ sha2 = "0.10" utoipa-swagger-ui = { version = "9", features = ["axum"] } testcontainers = { version = "0.24", optional = true } testcontainers-modules = { version = "0.12", features = ["postgres"], optional = true } -rand = "0.8" [features] default = ["ocr", "s3"] @@ -73,6 +72,7 @@ tempfile = "3" wiremock = "0.6" tokio-test = "0.4" futures = "0.3" +rand = "0.8" # Database testing dependencies testcontainers = "0.24" testcontainers-modules = { version = "0.12", features = ["postgres"] } diff --git a/charts/readur/README.md b/charts/readur/README.md deleted file mode 100644 index b4b1206..0000000 --- a/charts/readur/README.md +++ /dev/null @@ -1,108 +0,0 @@ -# Readur Helm Chart - -This Helm chart deploys Readur on Kubernetes using the [bjw-s common library chart](https://github.com/bjw-s/helm-charts/tree/main/charts/library/common). - -## Installation - -```bash -helm repo add readur https://readur.github.io/charts -helm install readur readur/readur -``` - -## Configuration - -### JWT Secret - -The JWT secret is automatically generated and persisted if not provided. You have three options: - -1. **Auto-generation (Recommended)**: Don't set any JWT configuration, and a secure secret will be auto-generated -2. **Custom value**: Set `jwtSecret.value` in your values -3. **Existing secret**: Reference an existing Kubernetes secret with `jwtSecret.existingSecret` - -```yaml -# Option 1: Auto-generate (default) -jwtSecret: - existingSecret: "" - value: "" - -# Option 2: Provide custom value -jwtSecret: - value: "your-secure-secret-here" - -# Option 3: Use existing Kubernetes secret -jwtSecret: - existingSecret: "my-jwt-secret" -``` - -The auto-generated secret is preserved across upgrades using the `helm.sh/resource-policy: keep` annotation. - -### Database Configuration - -Configure the database connection using either a direct URL or an existing secret: - -```yaml -# Option 1: Direct URL (not recommended for production) -database: - url: "postgresql://user:password@postgres/readur" - -# Option 2: Use existing secret (recommended) -database: - existingSecret: "readur-database-secret" -``` - -If using an existing secret, it should contain a `DATABASE_URL` key. - -### Persistence - -The chart configures two persistent volumes: - -```yaml -persistence: - uploads: - enabled: true - size: 10Gi - storageClass: "" # Uses default if not specified - - watch: - enabled: true - size: 5Gi - storageClass: "" -``` - -### Ingress - -Enable ingress to expose Readur: - -```yaml -ingress: - main: - enabled: true - className: nginx - hosts: - - host: readur.example.com - paths: - - path: / - pathType: Prefix - tls: - - secretName: readur-tls - hosts: - - readur.example.com -``` - -## Security Considerations - -1. **JWT Secret**: The auto-generated JWT secret is stored in a Kubernetes Secret and persists across upgrades -2. **Database Credentials**: Use Kubernetes Secrets for database credentials in production -3. **File Permissions**: An init container sets proper permissions for upload/watch directories -4. **Non-root User**: The container runs as UID 1000 (non-root) for security - -## Upgrading - -When upgrading the chart, the JWT secret is preserved automatically. If you need to rotate the secret: - -1. Delete the existing secret: `kubectl delete secret -jwt` -2. Upgrade the chart: `helm upgrade readur readur/readur` - -## Full Configuration - -See [values.yaml](values.yaml) for all available configuration options. \ No newline at end of file diff --git a/charts/readur/templates/release.yaml b/charts/readur/templates/release.yaml index 0b90f93..1a4f3ff 100644 --- a/charts/readur/templates/release.yaml +++ b/charts/readur/templates/release.yaml @@ -31,19 +31,6 @@ controllers: tag: latest pullPolicy: IfNotPresent - env: - {{- if not .Values.database.existingSecret }} - DATABASE_URL: {{ .Values.database.url | quote }} - {{- end }} - - envFrom: - - secretRef: - name: {{ .Values.jwtSecret.existingSecret | default (printf "%s-jwt" (include "bjw-s.common.lib.chart.names.fullname" .)) }} - {{- if .Values.database.existingSecret }} - - secretRef: - name: {{ .Values.database.existingSecret }} - {{- end }} - securityContext: runAsUser: 1000 runAsGroup: 1000 diff --git a/charts/readur/templates/secret.yaml b/charts/readur/templates/secret.yaml deleted file mode 100644 index dc4aef7..0000000 --- a/charts/readur/templates/secret.yaml +++ /dev/null @@ -1,25 +0,0 @@ -{{- if not .Values.jwtSecret.existingSecret }} ---- -apiVersion: v1 -kind: Secret -metadata: - name: {{ include "bjw-s.common.lib.chart.names.fullname" . }}-jwt - labels: - {{- include "bjw-s.common.lib.controller.metadata.labels" . | nindent 4 }} - annotations: - "helm.sh/resource-policy": keep -type: Opaque -data: - {{- if .Values.jwtSecret.value }} - JWT_SECRET: {{ .Values.jwtSecret.value | b64enc | quote }} - {{- else }} - # Generate a random JWT secret if not provided - # This uses a lookup to preserve the secret across upgrades - {{- $existingSecret := lookup "v1" "Secret" .Release.Namespace (printf "%s-jwt" (include "bjw-s.common.lib.chart.names.fullname" .)) }} - {{- if $existingSecret }} - JWT_SECRET: {{ index $existingSecret.data "JWT_SECRET" | quote }} - {{- else }} - JWT_SECRET: {{ randAlphaNum 43 | b64enc | quote }} - {{- end }} - {{- end }} -{{- end }} \ No newline at end of file diff --git a/charts/readur/values.yaml b/charts/readur/values.yaml index feeb432..c7b1901 100644 --- a/charts/readur/values.yaml +++ b/charts/readur/values.yaml @@ -3,22 +3,6 @@ ## Refer there for more detail about the supported values. ## Any values that you find in the above `values.yaml` can be provided to this chart and are then rendered. -# JWT Secret Configuration -jwtSecret: - # Set to use an existing Kubernetes secret containing JWT_SECRET - # If not set, a secret will be auto-generated - existingSecret: "" - # Optionally provide your own JWT secret value - # If not provided, a secure random secret will be generated - value: "" - -# Database Configuration -database: - # Reference to existing secret containing DATABASE_URL - existingSecret: "" - # Or provide database URL directly (not recommended for production) - url: "postgresql://readur:readur@postgres/readur" - controllers: main: containers: diff --git a/docker-compose.yml b/docker-compose.yml index daba8e1..ed2c6b0 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -29,8 +29,8 @@ services: SERVER_HOST: 0.0.0.0 SERVER_PORT: 8000 - # Security - JWT_SECRET will be auto-generated on first run if not provided - # JWT_SECRET: your-custom-secret-here # Optional: override auto-generated secret + # Security + JWT_SECRET: your-secret-key-change-this-in-production # File paths UPLOAD_PATH: /app/uploads @@ -64,9 +64,6 @@ services: # Watch folder - can be mapped to a host directory - ./readur_watch:/app/watch - # Secrets directory for JWT secret persistence - - ./readur_secrets:/app/secrets - # Or use a named volume for watch folder # - readur_watch:/app/watch diff --git a/src/config.rs b/src/config.rs index 428db2d..5ffa949 100644 --- a/src/config.rs +++ b/src/config.rs @@ -1,8 +1,5 @@ use anyhow::Result; use std::env; -use std::fs; -use std::path::Path; -use rand::Rng; use crate::models::S3SourceConfig; use crate::cpu_allocation::CpuAllocation; @@ -47,80 +44,6 @@ pub struct Config { } impl Config { - fn get_or_generate_jwt_secret() -> String { - // First check environment variable - if let Ok(secret) = env::var("JWT_SECRET") { - if !secret.is_empty() && secret != "your-secret-key-change-this-in-production" { - println!("✅ JWT_SECRET: ***hidden*** (loaded from env, {} chars)", secret.len()); - return secret; - } - } - - // Path for persistent JWT secret (in /app/secrets for Docker, or local for development) - let secret_dir = if Path::new("/app/secrets").exists() { - "/app/secrets" - } else { - "./secrets" - }; - - // Create directory if it doesn't exist - if let Err(e) = fs::create_dir_all(secret_dir) { - println!("⚠️ Could not create secrets directory: {}", e); - } - - let secret_file = format!("{}/jwt_secret", secret_dir); - - // Check if we have a persisted secret - if Path::new(&secret_file).exists() { - if let Ok(saved_secret) = fs::read_to_string(&secret_file) { - let trimmed = saved_secret.trim(); - if !trimmed.is_empty() { - println!("✅ JWT_SECRET: ***hidden*** (loaded from {} file, {} chars)", secret_file, trimmed.len()); - return trimmed.to_string(); - } - } - } - - // Generate a new secure secret (256 bits of entropy) - let mut rng = rand::thread_rng(); - let secret: String = (0..43) // 43 chars in base64 = ~256 bits - .map(|_| { - let idx = rng.gen_range(0..64); - match idx { - 0..26 => (b'A' + idx) as char, - 26..52 => (b'a' + idx - 26) as char, - 52..62 => (b'0' + idx - 52) as char, - 62 => '+', - 63 => '/', - _ => unreachable!(), - } - }) - .collect(); - - // Try to save it for next time - match fs::write(&secret_file, &secret) { - Ok(_) => { - // Set restrictive permissions on Unix systems - #[cfg(unix)] - { - use std::os::unix::fs::PermissionsExt; - if let Ok(metadata) = fs::metadata(&secret_file) { - let mut perms = metadata.permissions(); - perms.set_mode(0o600); // Read/write for owner only - let _ = fs::set_permissions(&secret_file, perms); - } - } - println!("✅ JWT_SECRET: Generated and saved new secure secret to {}", secret_file); - } - Err(e) => { - println!("⚠️ JWT_SECRET: Generated new secret but couldn't save to {}: {}", secret_file, e); - println!(" The secret will be regenerated on restart unless you set JWT_SECRET env var"); - } - } - - secret - } - pub fn from_env() -> Result { // Load .env file if present match dotenvy::dotenv() { @@ -186,7 +109,21 @@ impl Config { } } }, - jwt_secret: Self::get_or_generate_jwt_secret(), + jwt_secret: match env::var("JWT_SECRET") { + Ok(secret) => { + if secret == "your-secret-key" { + println!("⚠️ JWT_SECRET: Using default value (SECURITY RISK in production!)"); + } else { + println!("✅ JWT_SECRET: ***hidden*** (loaded from env, {} chars)", secret.len()); + } + secret + } + Err(_) => { + let default_secret = "your-secret-key".to_string(); + println!("⚠️ JWT_SECRET: Using default value (SECURITY RISK - env var not set!)"); + default_secret + } + }, upload_path: match env::var("UPLOAD_PATH") { Ok(path) => { println!("✅ UPLOAD_PATH: {} (loaded from env)", path); diff --git a/tests/integration_jwt_secret_tests.rs b/tests/integration_jwt_secret_tests.rs deleted file mode 100644 index 2449462..0000000 --- a/tests/integration_jwt_secret_tests.rs +++ /dev/null @@ -1,179 +0,0 @@ -#[cfg(test)] -mod tests { - use readur::config::Config; - use std::env; - use std::fs; - use std::path::Path; - use tempfile::TempDir; - use std::sync::Mutex; - - // Mutex to ensure JWT tests run sequentially to avoid race conditions - static JWT_TEST_MUTEX: Mutex<()> = Mutex::new(()); - - // Helper to run tests with isolated environment - fn run_with_clean_env(test_fn: F) -> R - where - F: FnOnce() -> R, - { - let _guard = JWT_TEST_MUTEX.lock().unwrap(); - - // Store and clear JWT_SECRET - let original_jwt = env::var("JWT_SECRET").ok(); - env::remove_var("JWT_SECRET"); - - // Run the test - let result = test_fn(); - - // Restore original - if let Some(value) = original_jwt { - env::set_var("JWT_SECRET", value); - } else { - env::remove_var("JWT_SECRET"); - } - - result - } - - #[test] - fn test_jwt_secret_from_env_var() { - run_with_clean_env(|| { - // Set a custom JWT secret - let custom_secret = "my-custom-test-secret-123456789"; - env::set_var("JWT_SECRET", custom_secret); - env::set_var("DATABASE_URL", "postgresql://test:test@localhost/test"); - - let config = Config::from_env().unwrap(); - assert_eq!(config.jwt_secret, custom_secret); - }); - } - - #[test] - fn test_jwt_secret_generation_when_no_env() { - run_with_clean_env(|| { - // Create a temp directory for secrets - let temp_dir = TempDir::new().unwrap(); - let secrets_dir = temp_dir.path().join("secrets"); - fs::create_dir_all(&secrets_dir).unwrap(); - - // Temporarily change working directory or use a test path - env::set_var("DATABASE_URL", "postgresql://test:test@localhost/test"); - - let config = Config::from_env().unwrap(); - - // Should have generated a non-empty secret - assert!(!config.jwt_secret.is_empty()); - // Should be a reasonable length (we generate 43 chars) - assert_eq!(config.jwt_secret.len(), 43); - // Should only contain base64 characters - assert!(config.jwt_secret.chars().all(|c| - c.is_ascii_alphanumeric() || c == '+' || c == '/' - )); - }); - } - - #[test] - fn test_jwt_secret_persistence() { - run_with_clean_env(|| { - // Create a temp directory for secrets - let temp_dir = TempDir::new().unwrap(); - let secrets_dir = temp_dir.path().join("secrets"); - fs::create_dir_all(&secrets_dir).unwrap(); - let secret_file = secrets_dir.join("jwt_secret"); - - // Write a known secret to the file - let known_secret = "persistent-test-secret-42"; - fs::write(&secret_file, known_secret).unwrap(); - - // Set DATABASE_URL for config - env::set_var("DATABASE_URL", "postgresql://test:test@localhost/test"); - - // Note: Since get_or_generate_jwt_secret checks /app/secrets or ./secrets, - // we'd need to adjust the test or make the path configurable for testing - // For now, this test validates the concept - - // Verify the file was created with content - assert!(secret_file.exists()); - let saved_content = fs::read_to_string(&secret_file).unwrap(); - assert_eq!(saved_content, known_secret); - }); - } - - #[test] - fn test_jwt_secret_ignores_default_value() { - run_with_clean_env(|| { - // Set the default/placeholder value that should be ignored - env::set_var("JWT_SECRET", "your-secret-key-change-this-in-production"); - env::set_var("DATABASE_URL", "postgresql://test:test@localhost/test"); - - let config = Config::from_env().unwrap(); - - // Should have generated a new secret, not used the default - assert_ne!(config.jwt_secret, "your-secret-key-change-this-in-production"); - assert!(!config.jwt_secret.is_empty()); - }); - } - - #[test] - fn test_jwt_secret_empty_string_generates_new() { - run_with_clean_env(|| { - // Set empty string - env::set_var("JWT_SECRET", ""); - env::set_var("DATABASE_URL", "postgresql://test:test@localhost/test"); - - let config = Config::from_env().unwrap(); - - // Should have generated a new secret - assert!(!config.jwt_secret.is_empty()); - assert_eq!(config.jwt_secret.len(), 43); - }); - } - - #[test] - #[cfg(unix)] - fn test_jwt_secret_file_permissions() { - use std::os::unix::fs::PermissionsExt; - - run_with_clean_env(|| { - // Create a temp directory for testing - let temp_dir = TempDir::new().unwrap(); - let secret_file = temp_dir.path().join("jwt_secret"); - - // Write a test secret - fs::write(&secret_file, "test-secret").unwrap(); - - // Set restrictive permissions like our code does - let metadata = fs::metadata(&secret_file).unwrap(); - let mut perms = metadata.permissions(); - perms.set_mode(0o600); - fs::set_permissions(&secret_file, perms).unwrap(); - - // Verify permissions are 0600 (owner read/write only) - let updated_metadata = fs::metadata(&secret_file).unwrap(); - let mode = updated_metadata.permissions().mode(); - assert_eq!(mode & 0o777, 0o600, "File should have 0600 permissions"); - }); - } - - #[test] - fn test_jwt_secret_randomness() { - run_with_clean_env(|| { - env::set_var("DATABASE_URL", "postgresql://test:test@localhost/test"); - - // Generate two configs without env var set - let config1 = Config::from_env().unwrap(); - - // Clear any saved secret to force regeneration - env::remove_var("JWT_SECRET"); - - let config2 = Config::from_env().unwrap(); - - // The secrets should be different (extremely unlikely to be the same) - // Note: In practice, the second call might load from file, - // so this test might need adjustment based on implementation - - // At minimum, verify they're valid secrets - assert_eq!(config1.jwt_secret.len(), 43); - assert_eq!(config2.jwt_secret.len(), 43); - }); - } -} \ No newline at end of file