From 116805acb3b59ced3ea78ea86de38dc49b9eb612 Mon Sep 17 00:00:00 2001 From: Caleb Doxsey Date: Wed, 14 Apr 2021 12:22:21 -0600 Subject: [PATCH] config: rename headers to set_response_headers (#2081) * config: rename headers to set_response_headers * Update config/options.go Co-authored-by: bobby <1544881+desimone@users.noreply.github.com> Co-authored-by: bobby <1544881+desimone@users.noreply.github.com> --- config/envoyconfig/listeners.go | 2 +- config/options.go | 35 ++++++++----- config/options_test.go | 81 +++++++++++++++++++++++++------ docs/reference/readme.md | 12 ++--- docs/reference/settings.yaml | 14 +++--- internal/autocert/manager_test.go | 4 +- pkg/grpc/config/config.proto | 2 +- 7 files changed, 106 insertions(+), 44 deletions(-) diff --git a/config/envoyconfig/listeners.go b/config/envoyconfig/listeners.go index fc64aa7dd..a96d5b9c4 100644 --- a/config/envoyconfig/listeners.go +++ b/config/envoyconfig/listeners.go @@ -336,7 +336,7 @@ func (b *Builder) buildMainHTTPConnectionManagerFilter( // if we're the proxy or authenticate service, add our global headers if config.IsProxy(options.Services) || config.IsAuthenticate(options.Services) { - vh.ResponseHeadersToAdd = toEnvoyHeaders(options.Headers) + vh.ResponseHeadersToAdd = toEnvoyHeaders(options.SetResponseHeaders) } if len(vh.Routes) > 0 { diff --git a/config/options.go b/config/options.go index 19bf8524a..5c13a03d3 100644 --- a/config/options.go +++ b/config/options.go @@ -170,9 +170,9 @@ type Options struct { SigningKey string `mapstructure:"signing_key" yaml:"signing_key,omitempty"` SigningKeyAlgorithm string `mapstructure:"signing_key_algorithm" yaml:"signing_key_algorithm,omitempty"` - // Headers to set on all proxied requests. Add a 'disable' key map to turn off. - HeadersEnv string `yaml:",omitempty"` - Headers map[string]string `yaml:",omitempty"` + HeadersEnv string `yaml:",omitempty"` + // SetResponseHeaders to set on all proxied requests. Add a 'disable' key map to turn off. + SetResponseHeaders map[string]string `yaml:",omitempty"` // List of JWT claims to insert as x-pomerium-claim-* headers on proxied requests JWTClaimsHeaders JWTClaimHeaders `mapstructure:"jwt_claims_headers" yaml:"jwt_claims_headers,omitempty"` @@ -305,7 +305,7 @@ var defaultOptions = Options{ CookieExpire: 14 * time.Hour, CookieName: "_pomerium", DefaultUpstreamTimeout: 30 * time.Second, - Headers: map[string]string{ + SetResponseHeaders: map[string]string{ "X-Frame-Options": "SAMEORIGIN", "X-XSS-Protection": "1; mode=block", "Strict-Transport-Security": "max-age=31536000; includeSubDomains; preload", @@ -445,12 +445,23 @@ func (o *Options) parseHeaders() error { } } - o.Headers = headers - } else if o.viperIsSet("headers") { - if err := o.viper.UnmarshalKey("headers", &headers); err != nil { - return fmt.Errorf("header %s failed to parse: %w", o.viper.Get("headers"), err) + o.SetResponseHeaders = headers + return nil + } + + if o.viperIsSet("headers") { + log.Warn().Msg("config: headers has been renamed to set_response_headers, it will be removed in v0.16") + } + + // option was renamed from `headers` to `set_response_headers`. Both config settings are supported. + headerKeys := []string{"headers", "set_response_headers"} + for _, headerKey := range headerKeys { + if o.viperIsSet(headerKey) { + if err := o.viper.UnmarshalKey(headerKey, &headers); err != nil { + return fmt.Errorf("header %s failed to parse: %w", o.viper.Get(headerKey), err) + } + o.SetResponseHeaders = headers } - o.Headers = headers } return nil } @@ -594,8 +605,8 @@ func (o *Options) Validate() error { return fmt.Errorf("config: failed to parse headers: %w", err) } - if _, disable := o.Headers[DisableHeaderKey]; disable { - o.Headers = make(map[string]string) + if _, disable := o.SetResponseHeaders[DisableHeaderKey]; disable { + o.SetResponseHeaders = make(map[string]string) } hasCert := false @@ -1034,7 +1045,7 @@ func (o *Options) ApplySettings(settings *config.Settings) { o.SigningKeyAlgorithm = settings.GetSigningKeyAlgorithm() } if settings.Headers != nil && len(settings.Headers) > 0 { - o.Headers = settings.Headers + o.SetResponseHeaders = settings.Headers } if len(settings.JwtClaimsHeaders) > 0 { o.JWTClaimsHeaders = settings.GetJwtClaimsHeaders() diff --git a/config/options_test.go b/config/options_test.go index d1dcc8568..bb6255cfb 100644 --- a/config/options_test.go +++ b/config/options_test.go @@ -116,18 +116,69 @@ func Test_bindEnvs(t *testing.T) { func Test_parseHeaders(t *testing.T) { // t.Parallel() tests := []struct { - name string - want map[string]string - envHeaders string - viperHeaders interface{} - wantErr bool + name string + want map[string]string + envHeaders string + viperHeaderKey string + viperHeaders interface{} + wantErr bool }{ - {"good env", map[string]string{"X-Custom-1": "foo", "X-Custom-2": "bar"}, `{"X-Custom-1":"foo", "X-Custom-2":"bar"}`, map[string]string{"X": "foo"}, false}, - {"good env not_json", map[string]string{"X-Custom-1": "foo", "X-Custom-2": "bar"}, `X-Custom-1:foo,X-Custom-2:bar`, map[string]string{"X": "foo"}, false}, - {"bad env", map[string]string{}, "xyyyy", map[string]string{"X": "foo"}, true}, - {"bad env not_json", map[string]string{"X-Custom-1": "foo", "X-Custom-2": "bar"}, `X-Custom-1:foo,X-Custom-2bar`, map[string]string{"X": "foo"}, true}, - {"bad viper", map[string]string{}, "", "notaheaderstruct", true}, - {"good viper", map[string]string{"X-Custom-1": "foo", "X-Custom-2": "bar"}, "", map[string]string{"X-Custom-1": "foo", "X-Custom-2": "bar"}, false}, + { + "good env", + map[string]string{"X-Custom-1": "foo", "X-Custom-2": "bar"}, + `{"X-Custom-1":"foo", "X-Custom-2":"bar"}`, + "headers", + map[string]string{"X": "foo"}, + false, + }, + { + "good env not_json", + map[string]string{"X-Custom-1": "foo", "X-Custom-2": "bar"}, + `X-Custom-1:foo,X-Custom-2:bar`, + "headers", + map[string]string{"X": "foo"}, + false, + }, + { + "bad env", + map[string]string{}, + "xyyyy", + "headers", + map[string]string{"X": "foo"}, + true, + }, + { + "bad env not_json", + map[string]string{"X-Custom-1": "foo", "X-Custom-2": "bar"}, + `X-Custom-1:foo,X-Custom-2bar`, + "headers", + map[string]string{"X": "foo"}, + true, + }, + { + "bad viper", + map[string]string{}, + "", + "headers", + "notaheaderstruct", + true, + }, + { + "good viper", + map[string]string{"X-Custom-1": "foo", "X-Custom-2": "bar"}, + "", + "headers", + map[string]string{"X-Custom-1": "foo", "X-Custom-2": "bar"}, + false, + }, + { + "new field name", + map[string]string{"X-Custom-1": "foo"}, + "", + "set_response_headers", + map[string]string{"X-Custom-1": "foo"}, + false, + }, } for _, tt := range tests { @@ -148,8 +199,8 @@ func Test_parseHeaders(t *testing.T) { t.Errorf("Error condition unexpected: err=%s", err) } - if !tt.wantErr && !cmp.Equal(tt.want, o.Headers) { - t.Errorf("Did get expected headers: %s", cmp.Diff(tt.want, o.Headers)) + if !tt.wantErr && !cmp.Equal(tt.want, o.SetResponseHeaders) { + t.Errorf("Did get expected headers: %s", cmp.Diff(tt.want, o.SetResponseHeaders)) } }) } @@ -259,7 +310,7 @@ func TestOptionsFromViper(t *testing.T) { GRPCServerMaxConnectionAge: 5 * time.Minute, GRPCServerMaxConnectionAgeGrace: 5 * time.Minute, AuthenticateCallbackPath: "/oauth2/callback", - Headers: map[string]string{ + SetResponseHeaders: map[string]string{ "Strict-Transport-Security": "max-age=31536000; includeSubDomains; preload", "X-Frame-Options": "SAMEORIGIN", "X-XSS-Protection": "1; mode=block", @@ -286,7 +337,7 @@ func TestOptionsFromViper(t *testing.T) { InsecureServer: true, GRPCServerMaxConnectionAge: 5 * time.Minute, GRPCServerMaxConnectionAgeGrace: 5 * time.Minute, - Headers: map[string]string{}, + SetResponseHeaders: map[string]string{}, RefreshDirectoryTimeout: 1 * time.Minute, RefreshDirectoryInterval: 10 * time.Minute, QPS: 1.0, diff --git a/docs/reference/readme.md b/docs/reference/readme.md index 20e54721b..ac2fbfbc3 100644 --- a/docs/reference/readme.md +++ b/docs/reference/readme.md @@ -821,9 +821,9 @@ Be sure to include the intermediary certificate. Default Upstream Timeout is the default timeout applied to a proxied route when no `timeout` key is specified by the policy. -### Headers -- Environmental Variable: `HEADERS` -- Config File Key: `headers` +### Set Response Headers +- Environmental Variable: `SET_RESPONSE_HEADERS` +- Config File Key: `set_response_headers` - Type: map of `strings` key value pairs - Examples: @@ -832,7 +832,7 @@ Default Upstream Timeout is the default timeout applied to a proxied route when - YAML: ```yaml - headers: + set_response_headers: X-Test: X-Value ``` @@ -847,7 +847,7 @@ Default Upstream Timeout is the default timeout applied to a proxied route when Strict-Transport-Security:max-age=31536000; includeSubDomains; preload, ``` -Headers specifies a mapping of [HTTP Header](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers) to be added globally to all managed routes and pomerium's authenticate service. +Set Response Headers specifies a mapping of [HTTP Header](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers) to be added globally to all managed routes and pomerium's authenticate service. By default, conservative [secure HTTP headers](https://www.owasp.org/index.php/OWASP_Secure_Headers_Project) are set. @@ -972,7 +972,7 @@ The connection string that the databroker service will use to connect to storage For `redis`, the following URL types are supported: -- simple: `redis://{username}:{password}@{host}:{port}/{db}` +- simple: `redis://[username:password@]host:port/[db]` - sentinel: `redis+sentinel://[:password@]host:port[,host2:port2,...]/[master_name[/db]][?param1=value1[¶m2=value2&...]]` - cluster: `redis+cluster://[username:password@]host:port[,host2:port2,...]/[?param1=value1[¶m2=value=2&...]]` diff --git a/docs/reference/settings.yaml b/docs/reference/settings.yaml index 5cdd41c1a..aafec36c1 100644 --- a/docs/reference/settings.yaml +++ b/docs/reference/settings.yaml @@ -931,11 +931,11 @@ settings: Default Upstream Timeout is the default timeout applied to a proxied route when no `timeout` key is specified by the policy. shortdoc: | Default Upstream Timeout is the default timeout applied to a proxied route when no timeout key is specified by the policy. - - name: "Headers" - keys: ["headers"] + - name: "Set Response Headers" + keys: ["set_response_headers"] attributes: | - - Environmental Variable: `HEADERS` - - Config File Key: `headers` + - Environmental Variable: `SET_RESPONSE_HEADERS` + - Config File Key: `set_response_headers` - Type: map of `strings` key value pairs - Examples: @@ -944,7 +944,7 @@ settings: - YAML: ```yaml - headers: + set_response_headers: X-Test: X-Value ``` @@ -959,7 +959,7 @@ settings: Strict-Transport-Security:max-age=31536000; includeSubDomains; preload, ``` doc: | - Headers specifies a mapping of [HTTP Header](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers) to be added globally to all managed routes and pomerium's authenticate service. + Set Response Headers specifies a mapping of [HTTP Header](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers) to be added globally to all managed routes and pomerium's authenticate service. By default, conservative [secure HTTP headers](https://www.owasp.org/index.php/OWASP_Secure_Headers_Project) are set. @@ -1038,7 +1038,7 @@ settings: - name: "The number of trusted hops" keys: ["xff_num_trusted_hops"] attributes: | - - Environmental Variable: `XFF_NUM_TRUSTED_HOPS + - Environmental Variable: `XFF_NUM_TRUSTED_HOPS` - Config File Key: `xff_num_trusted_hops` - Type: `uint32` - Default: `0` diff --git a/internal/autocert/manager_test.go b/internal/autocert/manager_test.go index e942a6c85..d9f0814e6 100644 --- a/internal/autocert/manager_test.go +++ b/internal/autocert/manager_test.go @@ -200,7 +200,7 @@ func TestRedirect(t *testing.T) { src := config.NewStaticSource(&config.Config{ Options: &config.Options{ HTTPRedirectAddr: addr, - Headers: map[string]string{ + SetResponseHeaders: map[string]string{ "X-Frame-Options": "SAMEORIGIN", "X-XSS-Protection": "1; mode=block", "Strict-Transport-Security": "max-age=31536000; includeSubDomains; preload", @@ -229,7 +229,7 @@ func TestRedirect(t *testing.T) { defer res.Body.Close() assert.Equal(t, http.StatusMovedPermanently, res.StatusCode, "should redirect to https") - for k, v := range src.GetConfig().Options.Headers { + for k, v := range src.GetConfig().Options.SetResponseHeaders { assert.NotEqual(t, v, res.Header.Get(k), "should ignore options header") } } diff --git a/pkg/grpc/config/config.proto b/pkg/grpc/config/config.proto index f135ca7eb..7d69d0803 100644 --- a/pkg/grpc/config/config.proto +++ b/pkg/grpc/config/config.proto @@ -149,7 +149,7 @@ message Settings { optional string certificate_authority_file = 35; optional string signing_key = 36; optional string signing_key_algorithm = 62; - map headers = 69; + map set_response_headers = 69; // repeated string jwt_claims_headers = 37; map jwt_claims_headers = 63; optional google.protobuf.Duration refresh_cooldown = 38;