refactor(config): move cross-field validation into config parser

Move option combination checks from cli.go into an exported
Validate() method on configOptions, called after both config file
and environment variable parsing complete. Add new checks for
TLS, metrics auth, database pool, and scheduler interval consistency.

internal/cli/cli.go

@@ -4,7 +4,6 @@
 package cli // import "miniflux.app/v2/internal/cli"
 
 import (
-	"errors"
 	"flag"
 	"fmt"
 	"io"
@@ -92,25 +91,8 @@ func Parse() {
 		printErrorAndExit(err)
 	}
 
-	if config.Opts.OAuth2Provider() == "oidc" && config.Opts.OAuth2OIDCDiscoveryEndpoint() == "" {
-		printErrorAndExit(errors.New("OAUTH2_OIDC_DISCOVERY_ENDPOINT must be configured when using the OIDC provider"))
-	}
-
-	if config.Opts.DisableLocalAuth() {
-		switch {
-		case config.Opts.OAuth2Provider() == "" && config.Opts.AuthProxyHeader() == "":
-			printErrorAndExit(errors.New("DISABLE_LOCAL_AUTH is enabled but neither OAUTH2_PROVIDER nor AUTH_PROXY_HEADER is not set. Please enable at least one authentication source"))
-		case config.Opts.OAuth2Provider() != "" && !config.Opts.IsOAuth2UserCreationAllowed():
-			printErrorAndExit(errors.New("DISABLE_LOCAL_AUTH is enabled and an OAUTH2_PROVIDER is configured, but OAUTH2_USER_CREATION is not enabled"))
-		case config.Opts.AuthProxyHeader() != "" && !config.Opts.IsAuthProxyUserCreationAllowed():
-			printErrorAndExit(errors.New("DISABLE_LOCAL_AUTH is enabled and an AUTH_PROXY_HEADER is configured, but AUTH_PROXY_USER_CREATION is not enabled"))
-		}
-	}
-
-	if config.Opts.AuthProxyHeader() != "" {
-		if len(config.Opts.TrustedReverseProxyNetworks()) == 0 {
-			printErrorAndExit(errors.New("TRUSTED_REVERSE_PROXY_NETWORKS must be configured when AUTH_PROXY_HEADER is used"))
-		}
+	if err := config.Opts.Validate(); err != nil {
+		printErrorAndExit(err)
 	}
 
 	if flagConfigDump {

internal/config/options_parsing_test.go

@@ -1752,3 +1752,294 @@ func TestConfigMapWithRedactedSecrets(t *testing.T) {
 		t.Fatalf("Expected ADMIN_PASSWORD value to be redacted, got '%s'", configMap[0].Value)
 	}
 }
+
+func TestValidateOIDCProviderRequiresDiscoveryEndpoint(t *testing.T) {
+	configParser := NewConfigParser()
+	if err := configParser.parseLines([]string{"OAUTH2_PROVIDER=oidc"}); err != nil {
+		t.Fatalf("Unexpected parse error: %v", err)
+	}
+	err := configParser.options.Validate()
+	if err == nil {
+		t.Fatal("Expected error when OIDC provider is set without discovery endpoint")
+	}
+	if err.Error() != "OAUTH2_OIDC_DISCOVERY_ENDPOINT must be configured when using the OIDC provider" {
+		t.Fatalf("Unexpected error message: %v", err)
+	}
+}
+
+func TestValidateOIDCProviderWithDiscoveryEndpoint(t *testing.T) {
+	configParser := NewConfigParser()
+	if err := configParser.parseLines([]string{
+		"OAUTH2_PROVIDER=oidc",
+		"OAUTH2_OIDC_DISCOVERY_ENDPOINT=https://example.com/.well-known/openid-configuration",
+	}); err != nil {
+		t.Fatalf("Unexpected parse error: %v", err)
+	}
+	if err := configParser.options.Validate(); err != nil {
+		t.Fatalf("Unexpected error: %v", err)
+	}
+}
+
+func TestValidateDisableLocalAuthWithoutAlternative(t *testing.T) {
+	configParser := NewConfigParser()
+	if err := configParser.parseLines([]string{"DISABLE_LOCAL_AUTH=1"}); err != nil {
+		t.Fatalf("Unexpected parse error: %v", err)
+	}
+	if err := configParser.options.Validate(); err == nil {
+		t.Fatal("Expected error when local auth is disabled without alternative")
+	}
+}
+
+func TestValidateDisableLocalAuthWithOAuth2ButNoUserCreation(t *testing.T) {
+	configParser := NewConfigParser()
+	if err := configParser.parseLines([]string{
+		"DISABLE_LOCAL_AUTH=1",
+		"OAUTH2_PROVIDER=oidc",
+		"OAUTH2_OIDC_DISCOVERY_ENDPOINT=https://example.com/.well-known/openid-configuration",
+	}); err != nil {
+		t.Fatalf("Unexpected parse error: %v", err)
+	}
+	if err := configParser.options.Validate(); err == nil {
+		t.Fatal("Expected error when local auth is disabled with OAuth2 but without user creation")
+	}
+}
+
+func TestValidateDisableLocalAuthWithOAuth2AndUserCreation(t *testing.T) {
+	configParser := NewConfigParser()
+	if err := configParser.parseLines([]string{
+		"DISABLE_LOCAL_AUTH=1",
+		"OAUTH2_PROVIDER=oidc",
+		"OAUTH2_OIDC_DISCOVERY_ENDPOINT=https://example.com/.well-known/openid-configuration",
+		"OAUTH2_USER_CREATION=1",
+	}); err != nil {
+		t.Fatalf("Unexpected parse error: %v", err)
+	}
+	if err := configParser.options.Validate(); err != nil {
+		t.Fatalf("Unexpected error: %v", err)
+	}
+}
+
+func TestValidateDisableLocalAuthWithAuthProxyButNoUserCreation(t *testing.T) {
+	configParser := NewConfigParser()
+	if err := configParser.parseLines([]string{
+		"DISABLE_LOCAL_AUTH=1",
+		"AUTH_PROXY_HEADER=X-Forwarded-User",
+		"AUTH_PROXY_USER_CREATION=0",
+		"TRUSTED_REVERSE_PROXY_NETWORKS=10.0.0.0/8",
+	}); err != nil {
+		t.Fatalf("Unexpected parse error: %v", err)
+	}
+	if err := configParser.options.Validate(); err == nil {
+		t.Fatal("Expected error when local auth is disabled with auth proxy but without user creation")
+	}
+}
+
+func TestValidateDisableLocalAuthWithAuthProxyAndUserCreation(t *testing.T) {
+	configParser := NewConfigParser()
+	if err := configParser.parseLines([]string{
+		"DISABLE_LOCAL_AUTH=1",
+		"AUTH_PROXY_HEADER=X-Forwarded-User",
+		"AUTH_PROXY_USER_CREATION=1",
+		"TRUSTED_REVERSE_PROXY_NETWORKS=10.0.0.0/8",
+	}); err != nil {
+		t.Fatalf("Unexpected parse error: %v", err)
+	}
+	if err := configParser.options.Validate(); err != nil {
+		t.Fatalf("Unexpected error: %v", err)
+	}
+}
+
+func TestValidateAuthProxyRequiresTrustedNetworks(t *testing.T) {
+	configParser := NewConfigParser()
+	if err := configParser.parseLines([]string{"AUTH_PROXY_HEADER=X-Forwarded-User"}); err != nil {
+		t.Fatalf("Unexpected parse error: %v", err)
+	}
+	err := configParser.options.Validate()
+	if err == nil {
+		t.Fatal("Expected error when auth proxy header is set without trusted networks")
+	}
+	if err.Error() != "TRUSTED_REVERSE_PROXY_NETWORKS must be configured when AUTH_PROXY_HEADER is used" {
+		t.Fatalf("Unexpected error message: %v", err)
+	}
+}
+
+func TestValidateAuthProxyWithTrustedNetworks(t *testing.T) {
+	configParser := NewConfigParser()
+	if err := configParser.parseLines([]string{
+		"AUTH_PROXY_HEADER=X-Forwarded-User",
+		"TRUSTED_REVERSE_PROXY_NETWORKS=10.0.0.0/8",
+	}); err != nil {
+		t.Fatalf("Unexpected parse error: %v", err)
+	}
+	if err := configParser.options.Validate(); err != nil {
+		t.Fatalf("Unexpected error: %v", err)
+	}
+}
+
+func TestValidateCertFileMissingKeyFile(t *testing.T) {
+	configParser := NewConfigParser()
+	if err := configParser.parseLines([]string{"CERT_FILE=/path/to/cert.pem"}); err != nil {
+		t.Fatalf("Unexpected parse error: %v", err)
+	}
+	if err := configParser.options.Validate(); err == nil {
+		t.Fatal("Expected error when CERT_FILE is set without KEY_FILE")
+	}
+}
+
+func TestValidateKeyFileMissingCertFile(t *testing.T) {
+	configParser := NewConfigParser()
+	if err := configParser.parseLines([]string{"KEY_FILE=/path/to/key.pem"}); err != nil {
+		t.Fatalf("Unexpected parse error: %v", err)
+	}
+	if err := configParser.options.Validate(); err == nil {
+		t.Fatal("Expected error when KEY_FILE is set without CERT_FILE")
+	}
+}
+
+func TestValidateCertFileAndKeyFile(t *testing.T) {
+	configParser := NewConfigParser()
+	if err := configParser.parseLines([]string{
+		"CERT_FILE=/path/to/cert.pem",
+		"KEY_FILE=/path/to/key.pem",
+	}); err != nil {
+		t.Fatalf("Unexpected parse error: %v", err)
+	}
+	if err := configParser.options.Validate(); err != nil {
+		t.Fatalf("Unexpected error: %v", err)
+	}
+}
+
+func TestValidateCertDomainAndCertFileMutuallyExclusive(t *testing.T) {
+	configParser := NewConfigParser()
+	if err := configParser.parseLines([]string{
+		"CERT_DOMAIN=example.com",
+		"CERT_FILE=/path/to/cert.pem",
+		"KEY_FILE=/path/to/key.pem",
+	}); err != nil {
+		t.Fatalf("Unexpected parse error: %v", err)
+	}
+	if err := configParser.options.Validate(); err == nil {
+		t.Fatal("Expected error when both CERT_DOMAIN and CERT_FILE are set")
+	}
+}
+
+func TestValidateCertDomainAlone(t *testing.T) {
+	configParser := NewConfigParser()
+	if err := configParser.parseLines([]string{"CERT_DOMAIN=example.com"}); err != nil {
+		t.Fatalf("Unexpected parse error: %v", err)
+	}
+	if err := configParser.options.Validate(); err != nil {
+		t.Fatalf("Unexpected error: %v", err)
+	}
+}
+
+func TestValidateMetricsUsernameWithoutPassword(t *testing.T) {
+	configParser := NewConfigParser()
+	if err := configParser.parseLines([]string{"METRICS_USERNAME=admin"}); err != nil {
+		t.Fatalf("Unexpected parse error: %v", err)
+	}
+	if err := configParser.options.Validate(); err == nil {
+		t.Fatal("Expected error when METRICS_USERNAME is set without METRICS_PASSWORD")
+	}
+}
+
+func TestValidateMetricsPasswordWithoutUsername(t *testing.T) {
+	configParser := NewConfigParser()
+	if err := configParser.parseLines([]string{"METRICS_PASSWORD=secret"}); err != nil {
+		t.Fatalf("Unexpected parse error: %v", err)
+	}
+	if err := configParser.options.Validate(); err == nil {
+		t.Fatal("Expected error when METRICS_PASSWORD is set without METRICS_USERNAME")
+	}
+}
+
+func TestValidateMetricsUsernameAndPassword(t *testing.T) {
+	configParser := NewConfigParser()
+	if err := configParser.parseLines([]string{
+		"METRICS_USERNAME=admin",
+		"METRICS_PASSWORD=secret",
+	}); err != nil {
+		t.Fatalf("Unexpected parse error: %v", err)
+	}
+	if err := configParser.options.Validate(); err != nil {
+		t.Fatalf("Unexpected error: %v", err)
+	}
+}
+
+func TestValidateDatabaseMinConnsGreaterThanMaxConns(t *testing.T) {
+	configParser := NewConfigParser()
+	if err := configParser.parseLines([]string{
+		"DATABASE_MIN_CONNS=25",
+		"DATABASE_MAX_CONNS=10",
+	}); err != nil {
+		t.Fatalf("Unexpected parse error: %v", err)
+	}
+	if err := configParser.options.Validate(); err == nil {
+		t.Fatal("Expected error when DATABASE_MIN_CONNS > DATABASE_MAX_CONNS")
+	}
+}
+
+func TestValidateDatabaseMinConnsEqualToMaxConns(t *testing.T) {
+	configParser := NewConfigParser()
+	if err := configParser.parseLines([]string{
+		"DATABASE_MIN_CONNS=10",
+		"DATABASE_MAX_CONNS=10",
+	}); err != nil {
+		t.Fatalf("Unexpected parse error: %v", err)
+	}
+	if err := configParser.options.Validate(); err != nil {
+		t.Fatalf("Unexpected error: %v", err)
+	}
+}
+
+func TestValidateSchedulerRoundRobinMinGreaterThanMax(t *testing.T) {
+	configParser := NewConfigParser()
+	if err := configParser.parseLines([]string{
+		"SCHEDULER_ROUND_ROBIN_MIN_INTERVAL=1440",
+		"SCHEDULER_ROUND_ROBIN_MAX_INTERVAL=60",
+	}); err != nil {
+		t.Fatalf("Unexpected parse error: %v", err)
+	}
+	if err := configParser.options.Validate(); err == nil {
+		t.Fatal("Expected error when SCHEDULER_ROUND_ROBIN_MIN_INTERVAL > SCHEDULER_ROUND_ROBIN_MAX_INTERVAL")
+	}
+}
+
+func TestValidateSchedulerRoundRobinMinLessThanMax(t *testing.T) {
+	configParser := NewConfigParser()
+	if err := configParser.parseLines([]string{
+		"SCHEDULER_ROUND_ROBIN_MIN_INTERVAL=60",
+		"SCHEDULER_ROUND_ROBIN_MAX_INTERVAL=1440",
+	}); err != nil {
+		t.Fatalf("Unexpected parse error: %v", err)
+	}
+	if err := configParser.options.Validate(); err != nil {
+		t.Fatalf("Unexpected error: %v", err)
+	}
+}
+
+func TestValidateSchedulerEntryFrequencyMinGreaterThanMax(t *testing.T) {
+	configParser := NewConfigParser()
+	if err := configParser.parseLines([]string{
+		"SCHEDULER_ENTRY_FREQUENCY_MIN_INTERVAL=1440",
+		"SCHEDULER_ENTRY_FREQUENCY_MAX_INTERVAL=5",
+	}); err != nil {
+		t.Fatalf("Unexpected parse error: %v", err)
+	}
+	if err := configParser.options.Validate(); err == nil {
+		t.Fatal("Expected error when SCHEDULER_ENTRY_FREQUENCY_MIN_INTERVAL > SCHEDULER_ENTRY_FREQUENCY_MAX_INTERVAL")
+	}
+}
+
+func TestValidateSchedulerEntryFrequencyMinLessThanMax(t *testing.T) {
+	configParser := NewConfigParser()
+	if err := configParser.parseLines([]string{
+		"SCHEDULER_ENTRY_FREQUENCY_MIN_INTERVAL=5",
+		"SCHEDULER_ENTRY_FREQUENCY_MAX_INTERVAL=1440",
+	}); err != nil {
+		t.Fatalf("Unexpected parse error: %v", err)
+	}
+	if err := configParser.options.Validate(); err != nil {
+		t.Fatalf("Unexpected error: %v", err)
+	}
+}

internal/config/parser.go

@@ -50,6 +50,54 @@ func (cp *configParser) ParseFile(filename string) (*configOptions, error) {
 	return cp.options, nil
 }
 
+// Validate checks for invalid or incomplete option combinations.
+func (c *configOptions) Validate() error {
+	if c.OAuth2Provider() == "oidc" && c.OAuth2OIDCDiscoveryEndpoint() == "" {
+		return errors.New("OAUTH2_OIDC_DISCOVERY_ENDPOINT must be configured when using the OIDC provider")
+	}
+
+	if c.DisableLocalAuth() {
+		switch {
+		case c.OAuth2Provider() == "" && c.AuthProxyHeader() == "":
+			return errors.New("DISABLE_LOCAL_AUTH is enabled but neither OAUTH2_PROVIDER nor AUTH_PROXY_HEADER is set. Please enable at least one authentication source")
+		case c.OAuth2Provider() != "" && !c.IsOAuth2UserCreationAllowed():
+			return errors.New("DISABLE_LOCAL_AUTH is enabled and an OAUTH2_PROVIDER is configured, but OAUTH2_USER_CREATION is not enabled")
+		case c.AuthProxyHeader() != "" && !c.IsAuthProxyUserCreationAllowed():
+			return errors.New("DISABLE_LOCAL_AUTH is enabled and an AUTH_PROXY_HEADER is configured, but AUTH_PROXY_USER_CREATION is not enabled")
+		}
+	}
+
+	if c.AuthProxyHeader() != "" && len(c.TrustedReverseProxyNetworks()) == 0 {
+		return errors.New("TRUSTED_REVERSE_PROXY_NETWORKS must be configured when AUTH_PROXY_HEADER is used")
+	}
+
+	if (c.CertFile() != "") != (c.CertKeyFile() != "") {
+		return errors.New("CERT_FILE and KEY_FILE must both be provided")
+	}
+
+	if c.CertDomain() != "" && c.CertFile() != "" {
+		return errors.New("CERT_DOMAIN and CERT_FILE/KEY_FILE are mutually exclusive")
+	}
+
+	if (c.MetricsUsername() != "") != (c.MetricsPassword() != "") {
+		return errors.New("METRICS_USERNAME and METRICS_PASSWORD must both be provided")
+	}
+
+	if c.DatabaseMinConns() > c.DatabaseMaxConns() {
+		return errors.New("DATABASE_MIN_CONNS must be less than or equal to DATABASE_MAX_CONNS")
+	}
+
+	if c.SchedulerRoundRobinMinInterval() > c.SchedulerRoundRobinMaxInterval() {
+		return errors.New("SCHEDULER_ROUND_ROBIN_MIN_INTERVAL must be less than or equal to SCHEDULER_ROUND_ROBIN_MAX_INTERVAL")
+	}
+
+	if c.SchedulerEntryFrequencyMinInterval() > c.SchedulerEntryFrequencyMaxInterval() {
+		return errors.New("SCHEDULER_ENTRY_FREQUENCY_MIN_INTERVAL must be less than or equal to SCHEDULER_ENTRY_FREQUENCY_MAX_INTERVAL")
+	}
+
+	return nil
+}
+
 func (cp *configParser) postParsing() error {
 	// Parse basePath and rootURL based on BASE_URL
 	baseURL := cp.options.options["BASE_URL"].parsedStringValue