refactor(auth): reject auth.type=password, remove stale config fields
resolveAuthType now returns an error for unsupported auth types. Removed Password and PasswordHash fields from the Config struct. Added tests for auth type validation.
- commit
- a5ba9cabece1f44df08f2fbce279c8e2ceea742e
- author
- brice · · 2026-03-22 06:39 UTC
- committer
- brice · · 2026-03-22 17:35 UTC
- parent
- 4a0fe8f
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
2 files · +46 -12
M internal/app/run.go +11 -12
| @@ -127,11 +127,9 @@ type Config struct { | |||
| 127 | 127 | ||
| 128 | 128 | Auth struct { | |
| 129 | 129 | Enabled bool `mapstructure:"enabled"` | |
| 130 | - | Type string `mapstructure:"type"` // "password" or "token" | |
| 130 | + | Type string `mapstructure:"type"` // only "token" is supported | |
| 131 | 131 | SecretsBackend string `mapstructure:"secrets_backend"` // "pass", "sops", or "unsafe" | |
| 132 | 132 | Username string `mapstructure:"username"` | |
| 133 | - | Password string `mapstructure:"password"` // deprecated: use password_hash | |
| 134 | - | PasswordHash string `mapstructure:"password_hash"` // path in secrets backend | |
| 135 | 133 | TokenSecret string `mapstructure:"token_secret"` // path in secrets backend | |
| 136 | 134 | TokenExpiry string `mapstructure:"token_expiry"` // e.g., "720h", "30d" | |
| 137 | 135 | AccessTokenTTL string `mapstructure:"access_token_ttl"` // e.g., "15m", "30m" (default: 15m) | |
| @@ -997,7 +995,10 @@ func createAuthService(ctx context.Context, cfg Config, log zerowrap.Logger) (ou | |||
| 997 | 995 | return nil, nil, nil | |
| 998 | 996 | } | |
| 999 | 997 | ||
| 1000 | - | authType := resolveAuthType(cfg) | |
| 998 | + | authType, err := resolveAuthType(cfg) | |
| 999 | + | if err != nil { | |
| 1000 | + | return nil, nil, err | |
| 1001 | + | } | |
| 1001 | 1002 | backend, err := resolveSecretsBackend(cfg.Auth.SecretsBackend) | |
| 1002 | 1003 | if err != nil { | |
| 1003 | 1004 | return nil, nil, log.WrapErr(err, "failed to resolve secrets backend") | |
| @@ -1026,8 +1027,11 @@ func createAuthService(ctx context.Context, cfg Config, log zerowrap.Logger) (ou | |||
| 1026 | 1027 | ||
| 1027 | 1028 | // resolveAuthType determines the auth type from config. | |
| 1028 | 1029 | // Token-only authentication is the only supported mode. | |
| 1029 | - | func resolveAuthType(_ Config) domain.AuthType { | |
| 1030 | - | return domain.AuthTypeToken | |
| 1030 | + | func resolveAuthType(cfg Config) (domain.AuthType, error) { | |
| 1031 | + | if cfg.Auth.Type != "" && cfg.Auth.Type != "token" { | |
| 1032 | + | return "", fmt.Errorf("unsupported auth.type %q; only \"token\" is supported", cfg.Auth.Type) | |
| 1033 | + | } | |
| 1034 | + | return domain.AuthTypeToken, nil | |
| 1031 | 1035 | } | |
| 1032 | 1036 | ||
| 1033 | 1037 | func resolveSecretsBackend(backend string) (domain.SecretsBackend, error) { | |
| @@ -1071,10 +1075,6 @@ func createTokenStore(backend domain.SecretsBackend, dataDir string, log zerowra | |||
| 1071 | 1075 | } | |
| 1072 | 1076 | ||
| 1073 | 1077 | func buildAuthConfig(ctx context.Context, cfg Config, authType domain.AuthType, backend domain.SecretsBackend, dataDir string, log zerowrap.Logger) (auth.Config, error) { | |
| 1074 | - | if cfg.Auth.Password != "" { | |
| 1075 | - | return auth.Config{}, fmt.Errorf("auth.password is no longer supported; use auth.password_hash with a secrets backend instead") | |
| 1076 | - | } | |
| 1077 | - | ||
| 1078 | 1078 | authConfig := auth.Config{ | |
| 1079 | 1079 | Enabled: cfg.Auth.Enabled, | |
| 1080 | 1080 | AuthType: authType, | |
| @@ -2457,8 +2457,7 @@ func loadConfig(v *viper.Viper, configPath string) error { | |||
| 2457 | 2457 | v.SetDefault("logging.container_logs.max_age", 28) | |
| 2458 | 2458 | v.SetDefault("env.dir", "") // defaults to {data_dir}/env when empty | |
| 2459 | 2459 | v.SetDefault("auth.enabled", true) | |
| 2460 | - | // Note: auth.type is intentionally not set - it's inferred from config | |
| 2461 | - | // If password_hash is set -> password mode, otherwise -> token mode | |
| 2460 | + | // Note: auth.type defaults to "token" (the only supported mode) | |
| 2462 | 2461 | v.SetDefault("auth.secrets_backend", "") | |
| 2463 | 2462 | v.SetDefault("auth.token_expiry", "720h") | |
| 2464 | 2463 | v.SetDefault("api.rate_limit.enabled", true) | |
| @@ -127,11 +127,9 @@ type Config struct { | |||||
| 127 | 127 | ||||
| 128 | Auth struct { | 128 | Auth struct { | ||
| 129 | Enabled bool `mapstructure:"enabled"` | 129 | Enabled bool `mapstructure:"enabled"` | ||
| 130 | - | Type string `mapstructure:"type"` // "password" or "token" | 130 | + | Type string `mapstructure:"type"` // only "token" is supported |
| 131 | SecretsBackend string `mapstructure:"secrets_backend"` // "pass", "sops", or "unsafe" | 131 | SecretsBackend string `mapstructure:"secrets_backend"` // "pass", "sops", or "unsafe" | ||
| 132 | Username string `mapstructure:"username"` | 132 | Username string `mapstructure:"username"` | ||
| 133 | - | Password string `mapstructure:"password"` // deprecated: use password_hash | |||
| 134 | - | PasswordHash string `mapstructure:"password_hash"` // path in secrets backend | |||
| 135 | TokenSecret string `mapstructure:"token_secret"` // path in secrets backend | 133 | TokenSecret string `mapstructure:"token_secret"` // path in secrets backend | ||
| 136 | TokenExpiry string `mapstructure:"token_expiry"` // e.g., "720h", "30d" | 134 | TokenExpiry string `mapstructure:"token_expiry"` // e.g., "720h", "30d" | ||
| 137 | AccessTokenTTL string `mapstructure:"access_token_ttl"` // e.g., "15m", "30m" (default: 15m) | 135 | AccessTokenTTL string `mapstructure:"access_token_ttl"` // e.g., "15m", "30m" (default: 15m) | ||
| @@ -997,7 +995,10 @@ func createAuthService(ctx context.Context, cfg Config, log zerowrap.Logger) (ou | |||||
| 997 | return nil, nil, nil | 995 | return nil, nil, nil | ||
| 998 | } | 996 | } | ||
| 999 | 997 | ||||
| 1000 | - | authType := resolveAuthType(cfg) | 998 | + | authType, err := resolveAuthType(cfg) |
| 999 | + | if err != nil { | |||
| 1000 | + | return nil, nil, err | |||
| 1001 | + | } | |||
| 1001 | backend, err := resolveSecretsBackend(cfg.Auth.SecretsBackend) | 1002 | backend, err := resolveSecretsBackend(cfg.Auth.SecretsBackend) | ||
| 1002 | if err != nil { | 1003 | if err != nil { | ||
| 1003 | return nil, nil, log.WrapErr(err, "failed to resolve secrets backend") | 1004 | return nil, nil, log.WrapErr(err, "failed to resolve secrets backend") | ||
| @@ -1026,8 +1027,11 @@ func createAuthService(ctx context.Context, cfg Config, log zerowrap.Logger) (ou | |||||
| 1026 | 1027 | ||||
| 1027 | // resolveAuthType determines the auth type from config. | 1028 | // resolveAuthType determines the auth type from config. | ||
| 1028 | // Token-only authentication is the only supported mode. | 1029 | // Token-only authentication is the only supported mode. | ||
| 1029 | - | func resolveAuthType(_ Config) domain.AuthType { | 1030 | + | func resolveAuthType(cfg Config) (domain.AuthType, error) { |
| 1030 | - | return domain.AuthTypeToken | 1031 | + | if cfg.Auth.Type != "" && cfg.Auth.Type != "token" { |
| 1032 | + | return "", fmt.Errorf("unsupported auth.type %q; only \"token\" is supported", cfg.Auth.Type) | |||
| 1033 | + | } | |||
| 1034 | + | return domain.AuthTypeToken, nil | |||
| 1031 | } | 1035 | } | ||
| 1032 | 1036 | ||||
| 1033 | func resolveSecretsBackend(backend string) (domain.SecretsBackend, error) { | 1037 | func resolveSecretsBackend(backend string) (domain.SecretsBackend, error) { | ||
| @@ -1071,10 +1075,6 @@ func createTokenStore(backend domain.SecretsBackend, dataDir string, log zerowra | |||||
| 1071 | } | 1075 | } | ||
| 1072 | 1076 | ||||
| 1073 | func buildAuthConfig(ctx context.Context, cfg Config, authType domain.AuthType, backend domain.SecretsBackend, dataDir string, log zerowrap.Logger) (auth.Config, error) { | 1077 | func buildAuthConfig(ctx context.Context, cfg Config, authType domain.AuthType, backend domain.SecretsBackend, dataDir string, log zerowrap.Logger) (auth.Config, error) { | ||
| 1074 | - | if cfg.Auth.Password != "" { | |||
| 1075 | - | return auth.Config{}, fmt.Errorf("auth.password is no longer supported; use auth.password_hash with a secrets backend instead") | |||
| 1076 | - | } | |||
| 1077 | - | ||||
| 1078 | authConfig := auth.Config{ | 1078 | authConfig := auth.Config{ | ||
| 1079 | Enabled: cfg.Auth.Enabled, | 1079 | Enabled: cfg.Auth.Enabled, | ||
| 1080 | AuthType: authType, | 1080 | AuthType: authType, | ||
| @@ -2457,8 +2457,7 @@ func loadConfig(v *viper.Viper, configPath string) error { | |||||
| 2457 | v.SetDefault("logging.container_logs.max_age", 28) | 2457 | v.SetDefault("logging.container_logs.max_age", 28) | ||
| 2458 | v.SetDefault("env.dir", "") // defaults to {data_dir}/env when empty | 2458 | v.SetDefault("env.dir", "") // defaults to {data_dir}/env when empty | ||
| 2459 | v.SetDefault("auth.enabled", true) | 2459 | v.SetDefault("auth.enabled", true) | ||
| 2460 | - | // Note: auth.type is intentionally not set - it's inferred from config | 2460 | + | // Note: auth.type defaults to "token" (the only supported mode) |
| 2461 | - | // If password_hash is set -> password mode, otherwise -> token mode | |||
| 2462 | v.SetDefault("auth.secrets_backend", "") | 2461 | v.SetDefault("auth.secrets_backend", "") | ||
| 2463 | v.SetDefault("auth.token_expiry", "720h") | 2462 | v.SetDefault("auth.token_expiry", "720h") | ||
| 2464 | v.SetDefault("api.rate_limit.enabled", true) | 2463 | v.SetDefault("api.rate_limit.enabled", true) | ||
M internal/app/run_auth_config_test.go +35 -0
| @@ -46,6 +46,41 @@ func TestResolveSecretsBackend_RejectsUnknownBackend(t *testing.T) { | |||
| 46 | 46 | } | |
| 47 | 47 | } | |
| 48 | 48 | ||
| 49 | + | func TestResolveAuthType_RejectsPassword(t *testing.T) { | |
| 50 | + | cfg := Config{} | |
| 51 | + | cfg.Auth.Type = "password" | |
| 52 | + | _, err := resolveAuthType(cfg) | |
| 53 | + | if err == nil { | |
| 54 | + | t.Fatal("expected error for auth.type=password") | |
| 55 | + | } | |
| 56 | + | if !strings.Contains(err.Error(), "unsupported auth.type") { | |
| 57 | + | t.Fatalf("unexpected error: %v", err) | |
| 58 | + | } | |
| 59 | + | } | |
| 60 | + | ||
| 61 | + | func TestResolveAuthType_AcceptsToken(t *testing.T) { | |
| 62 | + | cfg := Config{} | |
| 63 | + | cfg.Auth.Type = "token" | |
| 64 | + | authType, err := resolveAuthType(cfg) | |
| 65 | + | if err != nil { | |
| 66 | + | t.Fatalf("unexpected error: %v", err) | |
| 67 | + | } | |
| 68 | + | if authType != domain.AuthTypeToken { | |
| 69 | + | t.Fatalf("expected AuthTypeToken, got %v", authType) | |
| 70 | + | } | |
| 71 | + | } | |
| 72 | + | ||
| 73 | + | func TestResolveAuthType_AcceptsEmpty(t *testing.T) { | |
| 74 | + | cfg := Config{} | |
| 75 | + | authType, err := resolveAuthType(cfg) | |
| 76 | + | if err != nil { | |
| 77 | + | t.Fatalf("unexpected error: %v", err) | |
| 78 | + | } | |
| 79 | + | if authType != domain.AuthTypeToken { | |
| 80 | + | t.Fatalf("expected AuthTypeToken, got %v", authType) | |
| 81 | + | } | |
| 82 | + | } | |
| 83 | + | ||
| 49 | 84 | func TestLoadConfig_DoesNotDefaultSecretsBackendToUnsafe(t *testing.T) { | |
| 50 | 85 | v := viper.New() | |
| 51 | 86 | ||
| @@ -46,6 +46,41 @@ func TestResolveSecretsBackend_RejectsUnknownBackend(t *testing.T) { | |||||
| 46 | } | 46 | } | ||
| 47 | } | 47 | } | ||
| 48 | 48 | ||||
| 49 | + | func TestResolveAuthType_RejectsPassword(t *testing.T) { | |||
| 50 | + | cfg := Config{} | |||
| 51 | + | cfg.Auth.Type = "password" | |||
| 52 | + | _, err := resolveAuthType(cfg) | |||
| 53 | + | if err == nil { | |||
| 54 | + | t.Fatal("expected error for auth.type=password") | |||
| 55 | + | } | |||
| 56 | + | if !strings.Contains(err.Error(), "unsupported auth.type") { | |||
| 57 | + | t.Fatalf("unexpected error: %v", err) | |||
| 58 | + | } | |||
| 59 | + | } | |||
| 60 | + | ||||
| 61 | + | func TestResolveAuthType_AcceptsToken(t *testing.T) { | |||
| 62 | + | cfg := Config{} | |||
| 63 | + | cfg.Auth.Type = "token" | |||
| 64 | + | authType, err := resolveAuthType(cfg) | |||
| 65 | + | if err != nil { | |||
| 66 | + | t.Fatalf("unexpected error: %v", err) | |||
| 67 | + | } | |||
| 68 | + | if authType != domain.AuthTypeToken { | |||
| 69 | + | t.Fatalf("expected AuthTypeToken, got %v", authType) | |||
| 70 | + | } | |||
| 71 | + | } | |||
| 72 | + | ||||
| 73 | + | func TestResolveAuthType_AcceptsEmpty(t *testing.T) { | |||
| 74 | + | cfg := Config{} | |||
| 75 | + | authType, err := resolveAuthType(cfg) | |||
| 76 | + | if err != nil { | |||
| 77 | + | t.Fatalf("unexpected error: %v", err) | |||
| 78 | + | } | |||
| 79 | + | if authType != domain.AuthTypeToken { | |||
| 80 | + | t.Fatalf("expected AuthTypeToken, got %v", authType) | |||
| 81 | + | } | |||
| 82 | + | } | |||
| 83 | + | ||||
| 49 | func TestLoadConfig_DoesNotDefaultSecretsBackendToUnsafe(t *testing.T) { | 84 | func TestLoadConfig_DoesNotDefaultSecretsBackendToUnsafe(t *testing.T) { | ||
| 50 | v := viper.New() | 85 | v := viper.New() | ||
| 51 | 86 | ||||