Config: Add HTTP-client timeout (#2725)

Author: reinkrul

README.rst

@@ -156,7 +156,13 @@ func (auth *Auth) Configure(config core.ServerConfig) error {
 		return err
 	}
 
-	clientTimeout := time.Duration(auth.config.HTTPTimeout) * time.Second
+	var clientTimeout time.Duration
+	if auth.config.HTTPTimeout >= 0 {
+		clientTimeout = time.Duration(auth.config.HTTPTimeout) * time.Second
+	} else {
+		// auth.http.config got deprecated in favor of httpclient.timeout
+		clientTimeout = config.HTTPClient.Timeout
+	}
 	accessTokenLifeSpan := time.Duration(auth.config.AccessTokenLifeSpan) * time.Second
 	auth.authzServer = oauth.NewAuthorizationServer(auth.vdrInstance.Resolver(), auth.vcr, auth.vcr.Verifier(), auth.serviceResolver,
 		auth.keyStore, auth.contractNotary, auth.jsonldManager, accessTokenLifeSpan)

auth/auth.go

@@ -52,12 +52,29 @@ func TestAuth_Configure(t *testing.T) {
 
 		require.NoError(t, i.Configure(tlsServerConfig))
 	})
+	t.Run("use legacy auth.http.timeout config", func(t *testing.T) {
+		config := DefaultConfig()
+		config.HTTPTimeout = 10
+		config.ContractValidators = []string{"uzi"}
+		ctrl := gomock.NewController(t)
+		pkiMock := pki.NewMockProvider(ctrl)
+		pkiMock.EXPECT().AddTruststore(gomock.Any())   // uzi
+		pkiMock.EXPECT().CreateTLSConfig(gomock.Any()) // tlsConfig
+		vdrInstance := vdr.NewMockVDR(ctrl)
+		vdrInstance.EXPECT().Resolver().AnyTimes()
+
+		i := NewAuthInstance(config, vdrInstance, vcr.NewTestVCRInstance(t), crypto.NewMemoryCryptoInstance(), nil, nil, pkiMock)
+
+		require.NoError(t, i.Configure(tlsServerConfig))
+	})
 
 	t.Run("error - IRMA config failure", func(t *testing.T) {
 		authCfg := TestConfig()
 		authCfg.Irma.SchemeManager = "non-existing"
+		serverConfig := tlsServerConfig
+		serverConfig.Strictmode = false
 		i := testInstance(t, authCfg)
-		err := i.Configure(tlsServerConfig)
+		err := i.Configure(serverConfig)
 		require.NoError(t, err)
 	})
 
@@ -73,19 +90,14 @@ func TestAuth_Configure(t *testing.T) {
 		authCfg := TestConfig()
 		authCfg.Irma.SchemeManager = "demo"
 		i := testInstance(t, authCfg)
-		serverConfig := core.NewServerConfig()
-		serverConfig.Strictmode = true
-		err := i.Configure(*serverConfig)
+		err := i.Configure(tlsServerConfig)
 		assert.EqualError(t, err, "in strictmode the only valid irma-scheme-manager is 'pbdf'")
 	})
 
 	t.Run("error - TLS required in strict mode", func(t *testing.T) {
 		authCfg := TestConfig()
 		i := testInstance(t, authCfg)
-		serverConfig := core.NewServerConfig()
-		serverConfig.Strictmode = true
-		serverConfig.URL = "https://nuts.nl"
-		err := i.Configure(*serverConfig)
+		err := i.Configure(core.TestServerConfig())
 		assert.EqualError(t, err, "in strictmode TLS must be enabled")
 	})
 

auth/auth_test.go

@@ -57,6 +57,7 @@ func FlagSet() *pflag.FlagSet {
 	flags.StringSlice(ConfContractValidators, defs.ContractValidators, "sets the different contract validators to use")
 	flags.Bool(ConfV2APIEnabled, defs.V2APIEnabled, "enables experimental v2 API endpoints")
 	_ = flags.MarkHidden(ConfV2APIEnabled)
+	_ = flags.MarkDeprecated("auth.http.timeout", "use httpclient.timeout instead")
 
 	return flags
 }

auth/cmd/cmd.go

@@ -55,7 +55,6 @@ func TestFlagSet(t *testing.T) {
 func TestIrmaConfigInjection(t *testing.T) {
 	serverCfg := core.NewServerConfig()
 	serverCfg.Verbosity = "debug"
-	serverCfg.LoggerFormat = "text"
 	t.Setenv("NUTS_AUTH_IRMA_SCHEMEMANAGER", "irma-demo")
 	t.Setenv("NUTS_AUTH_IRMA_AUTOUPDATESCHEMES", "true")
 	err := serverCfg.Load(FlagSet())

auth/cmd/cmd_test.go

@@ -47,8 +47,7 @@ func DefaultConfig() Config {
 			SchemeManager:     "pbdf",
 			AutoUpdateSchemas: true,
 		},
-		HTTPTimeout: 30,
-		ClockSkew:   5000,
+		ClockSkew: 5000,
 		ContractValidators: []string{
 			string(services.IrmaFormat),
 			uzi.ContractFormat,

auth/config.go

@@ -33,6 +33,7 @@ import (
 	"net/url"
 	"reflect"
 	"strings"
+	"time"
 )
 
 const defaultConfigFile = "nuts.yaml"
@@ -58,13 +59,20 @@ type ServerConfig struct {
 	Strictmode          bool              `koanf:"strictmode"`
 	InternalRateLimiter bool              `koanf:"internalratelimiter"`
 	Datadir             string            `koanf:"datadir"`
+	HTTPClient          HTTPClientConfig  `koanf:"httpclient"`
 	TLS                 TLSConfig         `koanf:"tls"`
 	LegacyTLS           *NetworkTLSConfig `koanf:"network"`
 	// URL contains the base URL for public-facing HTTP services.
 	URL       string `koanf:"url"`
 	configMap *koanf.Koanf
 }
 
+// HTTPClientConfig contains settings for HTTP clients.
+type HTTPClientConfig struct {
+	// Timeout specifies the timeout for HTTP requests.
+	Timeout time.Duration `koanf:"timeout"`
+}
+
 // TLSConfig specifies how TLS should be configured for connections.
 // For v5, network.enabletls, network.truststorefile, network.certfile and network.certkeyfile must be moved to this struct.
 type TLSConfig struct {
@@ -177,10 +185,20 @@ const (
 func NewServerConfig() *ServerConfig {
 	legacyTLS := &NetworkTLSConfig{}
 	return &ServerConfig{
-		configMap: koanf.New(defaultDelimiter),
-		LegacyTLS: legacyTLS,
+		configMap:           koanf.New(defaultDelimiter),
+		LegacyTLS:           legacyTLS,
+		LoggerFormat:        "text",
+		Verbosity:           "info",
+		Strictmode:          true,
+		InternalRateLimiter: true,
+		Datadir:             "./data",
 		TLS: TLSConfig{
-			legacyTLS: legacyTLS,
+			legacyTLS:      legacyTLS,
+			TrustStoreFile: "truststore.pem",
+			Offload:        NoOffloading,
+		},
+		HTTPClient: HTTPClientConfig{
+			Timeout: 30 * time.Second,
 		},
 	}
 }
@@ -258,20 +276,23 @@ func resolveConfigFilePath(flags *pflag.FlagSet) string {
 // FlagSet returns the default server flags
 func FlagSet() *pflag.FlagSet {
 	flagSet := pflag.NewFlagSet("server", pflag.ContinueOnError)
+	defaultCfg := NewServerConfig()
+
 	flagSet.String(configFileFlag, defaultConfigFile, "Nuts config file")
 	flagSet.String("cpuprofile", "", "When set, a CPU profile is written to the given path. Ignored when strictmode is set.")
-	flagSet.String("verbosity", "info", "Log level (trace, debug, info, warn, error)")
-	flagSet.String("loggerformat", "text", "Log format (text, json)")
-	flagSet.Bool("strictmode", true, "When set, insecure settings are forbidden.")
-	flagSet.Bool("internalratelimiter", true, "When set, expensive internal calls are rate-limited to protect the network. Always enabled in strict mode.")
-	flagSet.String("datadir", "./data", "Directory where the node stores its files.")
-	flagSet.String("url", "", "Public facing URL of the server (required). Must be HTTPS when strictmode is set.")
-	flagSet.String("tls.certfile", "", "PEM file containing the certificate for the server (also used as client certificate).")
-	flagSet.String("tls.certkeyfile", "", "PEM file containing the private key of the server certificate.")
-	flagSet.String("tls.truststorefile", "truststore.pem", "PEM file containing the trusted CA certificates for authenticating remote servers.")
-	flagSet.String("tls.offload", string(NoOffloading), fmt.Sprintf("Whether to enable TLS offloading for incoming connections. "+
+	flagSet.String("verbosity", defaultCfg.Verbosity, "Log level (trace, debug, info, warn, error)")
+	flagSet.String("loggerformat", defaultCfg.LoggerFormat, "Log format (text, json)")
+	flagSet.Bool("strictmode", defaultCfg.Strictmode, "When set, insecure settings are forbidden.")
+	flagSet.Bool("internalratelimiter", defaultCfg.InternalRateLimiter, "When set, expensive internal calls are rate-limited to protect the network. Always enabled in strict mode.")
+	flagSet.String("datadir", defaultCfg.Datadir, "Directory where the node stores its files.")
+	flagSet.String("url", defaultCfg.URL, "Public facing URL of the server (required). Must be HTTPS when strictmode is set.")
+	flagSet.Duration("httpclient.timeout", defaultCfg.HTTPClient.Timeout, "Request time-out for HTTP clients, such as '10s'. Refer to Golang's 'time.Duration' syntax for a more elaborate description of the syntax.")
+	flagSet.String("tls.certfile", defaultCfg.TLS.CertFile, "PEM file containing the certificate for the server (also used as client certificate).")
+	flagSet.String("tls.certkeyfile", defaultCfg.TLS.CertKeyFile, "PEM file containing the private key of the server certificate.")
+	flagSet.String("tls.truststorefile", defaultCfg.TLS.TrustStoreFile, "PEM file containing the trusted CA certificates for authenticating remote servers.")
+	flagSet.String("tls.offload", string(defaultCfg.TLS.Offload), fmt.Sprintf("Whether to enable TLS offloading for incoming connections. "+
 		"Enable by setting it to '%s'. If enabled 'tls.certheader' must be configured as well.", OffloadIncomingTLS))
-	flagSet.String("tls.certheader", "", "Name of the HTTP header that will contain the client certificate when TLS is offloaded.")
+	flagSet.String("tls.certheader", defaultCfg.TLS.ClientCertHeaderName, "Name of the HTTP header that will contain the client certificate when TLS is offloaded.")
 
 	// Maxvaliditydays has been deprecated in v5.x
 	flagSet.Int("tls.crl.maxvaliditydays", 0, "The number of days a CRL can be outdated, after that it will hard-fail.")

core/server_config.go

@@ -34,13 +34,12 @@ type TestEngineConfig struct {
 }
 
 // TestServerConfig returns a new ServerConfig with the given template applied.
-func TestServerConfig(template ServerConfig) ServerConfig {
+func TestServerConfig(visitors ...func(*ServerConfig)) ServerConfig {
 	config := NewServerConfig()
-	// Most commonly used properties
-	config.Datadir = template.Datadir
-	config.Strictmode = template.Strictmode
-	config.InternalRateLimiter = template.InternalRateLimiter
-	config.URL = template.URL
+	config.URL = "https://nuts.nl"
+	for _, visitor := range visitors {
+		visitor(config)
+	}
 	return *config
 }
 

core/test.go

@@ -180,17 +180,18 @@ func TestCrypto_setupBackend(t *testing.T) {
 
 func TestCrypto_Configure(t *testing.T) {
 	directory := io.TestDirectory(t)
-	cfg := *core.NewServerConfig()
-	cfg.Datadir = directory
-	t.Run("ok", func(t *testing.T) {
+	cfg := core.TestServerConfig(func(config *core.ServerConfig) {
+		config.Datadir = directory
+	})
+	t.Run("default backend (fs) can be used in non-strictmode", func(t *testing.T) {
 		e := createCrypto(t)
+		cfg := cfg
+		cfg.Strictmode = false
 		err := e.Configure(cfg)
 		assert.NoError(t, err)
 	})
 	t.Run("error - no backend in strict mode is now allowed", func(t *testing.T) {
 		client := createCrypto(t)
-		cfg := cfg
-		cfg.Strictmode = true
 		err := client.Configure(cfg)
 		assert.EqualError(t, err, "backend must be explicitly set in strict mode", "expected error")
 	})

crypto/crypto_test.go

@@ -44,8 +44,9 @@ The following options apply to the server commands below:
       --http.default.cors.origin strings              When set, enables CORS from the specified origins on the default HTTP interface.
       --http.default.log string                       What to log about HTTP requests. Options are 'nothing', 'metadata' (log request method, URI, IP and response code), and 'metadata-and-body' (log the request and response body, in addition to the metadata). (default "metadata")
       --http.default.tls string                       Whether to enable TLS for the default interface, options are 'disabled', 'server', 'server-client'. Leaving it empty is synonymous to 'disabled',
+      --httpclient.timeout duration                   Request time-out for HTTP clients, such as '10s'. Refer to Golang's 'time.Duration' syntax for a more elaborate description of the syntax. (default 30s)
       --internalratelimiter                           When set, expensive internal calls are rate-limited to protect the network. Always enabled in strict mode. (default true)
-      --jsonld.contexts.localmapping stringToString   This setting allows mapping external URLs to local files for e.g. preventing external dependencies. These mappings have precedence over those in remoteallowlist. (default [https://schema.org=assets/contexts/schema-org-v13.ldjson,https://nuts.nl/credentials/v1=assets/contexts/nuts.ldjson,https://www.w3.org/2018/credentials/v1=assets/contexts/w3c-credentials-v1.ldjson,https://w3c-ccg.github.io/lds-jws2020/contexts/lds-jws2020-v1.json=assets/contexts/lds-jws2020-v1.ldjson,https://w3id.org/vc/status-list/2021/v1=assets/contexts/w3c-statuslist2021.ldjson])
+      --jsonld.contexts.localmapping stringToString   This setting allows mapping external URLs to local files for e.g. preventing external dependencies. These mappings have precedence over those in remoteallowlist. (default [https://nuts.nl/credentials/v1=assets/contexts/nuts.ldjson,https://www.w3.org/2018/credentials/v1=assets/contexts/w3c-credentials-v1.ldjson,https://w3id.org/vc/status-list/2021/v1=assets/contexts/w3c-statuslist2021.ldjson,https://w3c-ccg.github.io/lds-jws2020/contexts/lds-jws2020-v1.json=assets/contexts/lds-jws2020-v1.ldjson,https://schema.org=assets/contexts/schema-org-v13.ldjson])
       --jsonld.contexts.remoteallowlist strings       In strict mode, fetching external JSON-LD contexts is not allowed except for context-URLs listed here. (default [https://schema.org,https://www.w3.org/2018/credentials/v1,https://w3c-ccg.github.io/lds-jws2020/contexts/lds-jws2020-v1.json,https://w3id.org/vc/status-list/2021/v1])
       --loggerformat string                           Log format (text, json) (default "text")
       --network.bootstrapnodes strings                List of bootstrap nodes ('<host>:<port>') which the node initially connect to.