From 1251c8cd21034c10c2867707470b7fe84120c988 Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Tue, 15 Nov 2022 13:02:20 +0100 Subject: [PATCH 1/6] WIP: Introduce interface to ease forwarding of HTTP headers --- backend/common.go | 30 ++++++++++++++++++++++++++++++ backend/data.go | 40 ++++++++++++++++++++++++++++++++++++++++ backend/diagnostics.go | 38 ++++++++++++++++++++++++++++++++++++++ backend/resource.go | 26 ++++++++++++++++++++++++++ 4 files changed, 134 insertions(+) diff --git a/backend/common.go b/backend/common.go index dcc04e04d..f1ec44acc 100644 --- a/backend/common.go +++ b/backend/common.go @@ -2,6 +2,7 @@ package backend import ( "encoding/json" + "net/http" "time" "github.com/grafana/grafana-plugin-sdk-go/backend/httpclient" @@ -205,3 +206,32 @@ func SecureJSONDataFromHTTPClientOptions(opts httpclient.Options) (res map[strin return secureJSONData } + +// ForwardHTTPHeaders interface marking that forward of HTTP headers is supported. +type ForwardHTTPHeaders interface { + // SetHTTPHeader sets the header entries associated with key to the + // single element value. It replaces any existing values + // associated with key. The key is case insensitive; it is + // canonicalized by textproto.CanonicalMIMEHeaderKey. + SetHTTPHeader(key, value string) + + // GetHTTPHeader gets the first value associated with the given key. If + // there are no values associated with the key, Get returns "". + // It is case insensitive; textproto.CanonicalMIMEHeaderKey is + // used to canonicalize the provided key. Get assumes that all + // keys are stored in canonical form. + GetHTTPHeader(key string) string + + // GetHTTPHeaders returns forwarded HTTP headers, if any. + GetHTTPHeaders() http.Header +} + +const ( + // OAuthIdentityTokenHeaderName the header name used for forwarding + // OAuth Identity access token. + OAuthIdentityTokenHeaderName = "Authorization" + + // OAuthIdentityIDTokenHeaderName the header name used for forwarding + // OAuth Identity ID token. + OAuthIdentityIDTokenHeaderName = "X-ID-Token" +) diff --git a/backend/data.go b/backend/data.go index a26afc70e..3e85d1257 100644 --- a/backend/data.go +++ b/backend/data.go @@ -4,6 +4,10 @@ import ( "context" "encoding/json" "errors" + "fmt" + "net/http" + "net/textproto" + "strings" "time" "github.com/grafana/grafana-plugin-sdk-go/data" @@ -37,11 +41,45 @@ func (fn QueryDataHandlerFunc) QueryData(ctx context.Context, req *QueryDataRequ // QueryDataRequest contains a single request which contains multiple queries. // It is the input type for a QueryData call. type QueryDataRequest struct { + ForwardHTTPHeaders PluginContext PluginContext Headers map[string]string Queries []DataQuery } +func (req *QueryDataRequest) SetHTTPHeader(key, value string) { + if req.Headers == nil { + req.Headers = map[string]string{} + } + + req.Headers[fmt.Sprintf("http_%s", key)] = value +} + +func (req QueryDataRequest) GetHTTPHeader(key string) string { + return req.GetHTTPHeaders().Get(key) +} + +func (req QueryDataRequest) GetHTTPHeaders() http.Header { + httpHeaders := http.Header{} + + for k, v := range req.Headers { + if textproto.CanonicalMIMEHeaderKey(k) == OAuthIdentityTokenHeaderName { + httpHeaders.Set(k, v) + } + + if textproto.CanonicalMIMEHeaderKey(k) == OAuthIdentityIDTokenHeaderName { + httpHeaders.Set(k, v) + } + + if strings.HasPrefix(k, "http_") { + hKey := strings.TrimPrefix(k, "http_") + httpHeaders.Set(hKey, v) + } + } + + return httpHeaders +} + // DataQuery represents a single query as sent from the frontend. // A slice of DataQuery makes up the Queries property of a QueryDataRequest. type DataQuery struct { @@ -99,12 +137,14 @@ func NewQueryDataResponse() *QueryDataResponse { // Responses is a map of RefIDs (Unique Query ID) to DataResponses. // The QueryData method the QueryDataHandler method will set the RefId // property on the DataResponses' frames based on these RefIDs. +// //swagger:model type Responses map[string]DataResponse // DataResponse contains the results from a DataQuery. // A map of RefIDs (unique query identifiers) to this type makes up the Responses property of a QueryDataResponse. // The Error property is used to allow for partial success responses from the containing QueryDataResponse. +// //swagger:model type DataResponse struct { // The data returned from the Query. Each Frame repeats the RefID. diff --git a/backend/diagnostics.go b/backend/diagnostics.go index 5631aad12..1fcb615af 100644 --- a/backend/diagnostics.go +++ b/backend/diagnostics.go @@ -2,7 +2,11 @@ package backend import ( "context" + "fmt" + "net/http" + "net/textproto" "strconv" + "strings" ) // CheckHealthHandler enables users to send health check @@ -53,10 +57,44 @@ func (hs HealthStatus) String() string { // CheckHealthRequest contains the healthcheck request type CheckHealthRequest struct { + ForwardHTTPHeaders PluginContext PluginContext Headers map[string]string } +func (req *CheckHealthRequest) SetHTTPHeader(key, value string) { + if req.Headers == nil { + req.Headers = map[string]string{} + } + + req.Headers[fmt.Sprintf("http_%s", key)] = value +} + +func (req CheckHealthRequest) GetHTTPHeader(key string) string { + return req.GetHTTPHeaders().Get(key) +} + +func (req CheckHealthRequest) GetHTTPHeaders() http.Header { + httpHeaders := http.Header{} + + for k, v := range req.Headers { + if textproto.CanonicalMIMEHeaderKey(k) == OAuthIdentityTokenHeaderName { + httpHeaders.Set(k, v) + } + + if textproto.CanonicalMIMEHeaderKey(k) == OAuthIdentityIDTokenHeaderName { + httpHeaders.Set(k, v) + } + + if strings.HasPrefix(k, "http_") { + hKey := strings.TrimPrefix(k, "http_") + httpHeaders.Set(hKey, v) + } + } + + return httpHeaders +} + // CheckHealthResult contains the healthcheck response type CheckHealthResult struct { Status HealthStatus diff --git a/backend/resource.go b/backend/resource.go index 248f91670..9345332b5 100644 --- a/backend/resource.go +++ b/backend/resource.go @@ -2,10 +2,12 @@ package backend import ( "context" + "net/http" ) // CallResourceRequest represents a request for a resource call. type CallResourceRequest struct { + ForwardHTTPHeaders PluginContext PluginContext Path string Method string @@ -14,6 +16,30 @@ type CallResourceRequest struct { Body []byte } +func (req *CallResourceRequest) SetHTTPHeader(key, value string) { + if req.Headers == nil { + req.Headers = map[string][]string{} + } + + req.Headers[key] = []string{value} +} + +func (req CallResourceRequest) GetHTTPHeader(key string) string { + return req.GetHTTPHeaders().Get(key) +} + +func (req CallResourceRequest) GetHTTPHeaders() http.Header { + httpHeaders := http.Header{} + + for k, v := range req.Headers { + for _, strVal := range v { + httpHeaders.Add(k, strVal) + } + } + + return httpHeaders +} + // CallResourceResponse represents a response from a resource call. type CallResourceResponse struct { Status int From 9ff654405645cd452a7a22b840ef6d9b5a27bc9e Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Mon, 5 Dec 2022 13:36:41 -0800 Subject: [PATCH 2/6] changes after review refactor a bit, adds tests, adds proper documentation of methods and fields. --- backend/common.go | 30 ------------- backend/data.go | 49 +++++++++++----------- backend/data_test.go | 81 ++++++++++++++++++++++++++++++++++++ backend/diagnostics.go | 55 +++++++++++++----------- backend/diagnostics_test.go | 81 ++++++++++++++++++++++++++++++++++++ backend/http_headers.go | 67 +++++++++++++++++++++++++++++ backend/http_headers_test.go | 57 +++++++++++++++++++++++++ backend/resource.go | 47 +++++++++++++++++---- backend/resource_test.go | 81 ++++++++++++++++++++++++++++++++++++ 9 files changed, 461 insertions(+), 87 deletions(-) create mode 100644 backend/data_test.go create mode 100644 backend/diagnostics_test.go create mode 100644 backend/http_headers.go create mode 100644 backend/http_headers_test.go create mode 100644 backend/resource_test.go diff --git a/backend/common.go b/backend/common.go index f1ec44acc..dcc04e04d 100644 --- a/backend/common.go +++ b/backend/common.go @@ -2,7 +2,6 @@ package backend import ( "encoding/json" - "net/http" "time" "github.com/grafana/grafana-plugin-sdk-go/backend/httpclient" @@ -206,32 +205,3 @@ func SecureJSONDataFromHTTPClientOptions(opts httpclient.Options) (res map[strin return secureJSONData } - -// ForwardHTTPHeaders interface marking that forward of HTTP headers is supported. -type ForwardHTTPHeaders interface { - // SetHTTPHeader sets the header entries associated with key to the - // single element value. It replaces any existing values - // associated with key. The key is case insensitive; it is - // canonicalized by textproto.CanonicalMIMEHeaderKey. - SetHTTPHeader(key, value string) - - // GetHTTPHeader gets the first value associated with the given key. If - // there are no values associated with the key, Get returns "". - // It is case insensitive; textproto.CanonicalMIMEHeaderKey is - // used to canonicalize the provided key. Get assumes that all - // keys are stored in canonical form. - GetHTTPHeader(key string) string - - // GetHTTPHeaders returns forwarded HTTP headers, if any. - GetHTTPHeaders() http.Header -} - -const ( - // OAuthIdentityTokenHeaderName the header name used for forwarding - // OAuth Identity access token. - OAuthIdentityTokenHeaderName = "Authorization" - - // OAuthIdentityIDTokenHeaderName the header name used for forwarding - // OAuth Identity ID token. - OAuthIdentityIDTokenHeaderName = "X-ID-Token" -) diff --git a/backend/data.go b/backend/data.go index 3e85d1257..b9c67df58 100644 --- a/backend/data.go +++ b/backend/data.go @@ -6,8 +6,6 @@ import ( "errors" "fmt" "net/http" - "net/textproto" - "strings" "time" "github.com/grafana/grafana-plugin-sdk-go/data" @@ -41,43 +39,43 @@ func (fn QueryDataHandlerFunc) QueryData(ctx context.Context, req *QueryDataRequ // QueryDataRequest contains a single request which contains multiple queries. // It is the input type for a QueryData call. type QueryDataRequest struct { - ForwardHTTPHeaders + // PluginContext the contextual information for the request. PluginContext PluginContext - Headers map[string]string - Queries []DataQuery + + // Headers the environment/metadata information for the request. + // + // To access forwarded HTTP headers please use + // GetHTTPHeaders or GetHTTPHeader. + Headers map[string]string + + // Queries the data queries for the request. + Queries []DataQuery } +// SetHTTPHeader sets the header entries associated with key to the +// single element value. It replaces any existing values +// associated with key. The key is case insensitive; it is +// canonicalized by textproto.CanonicalMIMEHeaderKey. func (req *QueryDataRequest) SetHTTPHeader(key, value string) { if req.Headers == nil { req.Headers = map[string]string{} } - req.Headers[fmt.Sprintf("http_%s", key)] = value + req.Headers[fmt.Sprintf("%s%s", httpHeaderPrefix, key)] = value } +// GetHTTPHeader gets the first value associated with the given key. If +// there are no values associated with the key, Get returns "". +// It is case insensitive; textproto.CanonicalMIMEHeaderKey is +// used to canonicalize the provided key. Get assumes that all +// keys are stored in canonical form. func (req QueryDataRequest) GetHTTPHeader(key string) string { return req.GetHTTPHeaders().Get(key) } +// GetHTTPHeaders returns HTTP headers. func (req QueryDataRequest) GetHTTPHeaders() http.Header { - httpHeaders := http.Header{} - - for k, v := range req.Headers { - if textproto.CanonicalMIMEHeaderKey(k) == OAuthIdentityTokenHeaderName { - httpHeaders.Set(k, v) - } - - if textproto.CanonicalMIMEHeaderKey(k) == OAuthIdentityIDTokenHeaderName { - httpHeaders.Set(k, v) - } - - if strings.HasPrefix(k, "http_") { - hKey := strings.TrimPrefix(k, "http_") - httpHeaders.Set(hKey, v) - } - } - - return httpHeaders + return getHTTPHeadersFromStringMap(req.Headers) } // DataQuery represents a single query as sent from the frontend. @@ -157,6 +155,7 @@ type DataResponse struct { Status Status } +// ErrDataResponse returns an error DataResponse given status and message. func ErrDataResponse(status Status, message string) DataResponse { return DataResponse{ Error: errors.New(message), @@ -187,3 +186,5 @@ type TimeRange struct { func (tr TimeRange) Duration() time.Duration { return tr.To.Sub(tr.From) } + +var _ ForwardHTTPHeaders = (*QueryDataRequest)(nil) diff --git a/backend/data_test.go b/backend/data_test.go new file mode 100644 index 000000000..f24329eca --- /dev/null +++ b/backend/data_test.go @@ -0,0 +1,81 @@ +package backend + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestQueryDataRequest(t *testing.T) { + req := &QueryDataRequest{} + const customHeaderName = "X-Custom" + + t.Run("Legacy headers", func(t *testing.T) { + req.Headers = map[string]string{ + "Authorization": "a", + "X-ID-Token": "b", + "Cookies": "c", + customHeaderName: "d", + } + + t.Run("GetHTTPHeaders canonical form", func(t *testing.T) { + headers := req.GetHTTPHeaders() + require.Equal(t, "a", headers.Get(OAuthIdentityTokenHeaderName)) + require.Equal(t, "b", headers.Get(OAuthIdentityIDTokenHeaderName)) + require.Equal(t, "c", headers.Get(CookiesHeaderName)) + require.Empty(t, headers.Get(customHeaderName)) + }) + + t.Run("GetHTTPHeader canonical form", func(t *testing.T) { + require.Equal(t, "a", req.GetHTTPHeader(OAuthIdentityTokenHeaderName)) + require.Equal(t, "b", req.GetHTTPHeader(OAuthIdentityIDTokenHeaderName)) + require.Equal(t, "c", req.GetHTTPHeader(CookiesHeaderName)) + require.Empty(t, req.GetHTTPHeader(customHeaderName)) + }) + }) + + t.Run("SetHTTPHeader canonical form", func(t *testing.T) { + req.SetHTTPHeader(OAuthIdentityTokenHeaderName, "a") + req.SetHTTPHeader(OAuthIdentityIDTokenHeaderName, "b") + req.SetHTTPHeader(CookiesHeaderName, "c") + req.SetHTTPHeader(customHeaderName, "d") + + t.Run("GetHTTPHeaders canonical form", func(t *testing.T) { + headers := req.GetHTTPHeaders() + require.Equal(t, "a", headers.Get(OAuthIdentityTokenHeaderName)) + require.Equal(t, "b", headers.Get(OAuthIdentityIDTokenHeaderName)) + require.Equal(t, "c", headers.Get(CookiesHeaderName)) + require.Equal(t, "d", headers.Get(customHeaderName)) + }) + + t.Run("GetHTTPHeader canonical form", func(t *testing.T) { + require.Equal(t, "a", req.GetHTTPHeader(OAuthIdentityTokenHeaderName)) + require.Equal(t, "b", req.GetHTTPHeader(OAuthIdentityIDTokenHeaderName)) + require.Equal(t, "c", req.GetHTTPHeader(CookiesHeaderName)) + require.Equal(t, "d", req.GetHTTPHeader(customHeaderName)) + }) + }) + + t.Run("SetHTTPHeader non-canonical form", func(t *testing.T) { + req.SetHTTPHeader(strings.ToLower(OAuthIdentityTokenHeaderName), "a") + req.SetHTTPHeader(strings.ToLower(OAuthIdentityIDTokenHeaderName), "b") + req.SetHTTPHeader(strings.ToLower(CookiesHeaderName), "c") + req.SetHTTPHeader(strings.ToLower(customHeaderName), "d") + + t.Run("GetHTTPHeaders non-canonical form", func(t *testing.T) { + headers := req.GetHTTPHeaders() + require.Equal(t, "a", headers.Get(strings.ToLower(OAuthIdentityTokenHeaderName))) + require.Equal(t, "b", headers.Get(strings.ToLower(OAuthIdentityIDTokenHeaderName))) + require.Equal(t, "c", headers.Get(strings.ToLower(CookiesHeaderName))) + require.Equal(t, "d", headers.Get(strings.ToLower(customHeaderName))) + }) + + t.Run("GetHTTPHeader non-canonical form", func(t *testing.T) { + require.Equal(t, "a", req.GetHTTPHeader(strings.ToLower(OAuthIdentityTokenHeaderName))) + require.Equal(t, "b", req.GetHTTPHeader(strings.ToLower(OAuthIdentityIDTokenHeaderName))) + require.Equal(t, "c", req.GetHTTPHeader(strings.ToLower(CookiesHeaderName))) + require.Equal(t, "d", req.GetHTTPHeader(strings.ToLower(customHeaderName))) + }) + }) +} diff --git a/backend/diagnostics.go b/backend/diagnostics.go index 1fcb615af..ea7ebd004 100644 --- a/backend/diagnostics.go +++ b/backend/diagnostics.go @@ -4,9 +4,7 @@ import ( "context" "fmt" "net/http" - "net/textproto" "strconv" - "strings" ) // CheckHealthHandler enables users to send health check @@ -57,48 +55,51 @@ func (hs HealthStatus) String() string { // CheckHealthRequest contains the healthcheck request type CheckHealthRequest struct { - ForwardHTTPHeaders + // PluginContext the contextual information for the request. PluginContext PluginContext - Headers map[string]string + + // Headers the environment/metadata information for the request. + // + // To access forwarded HTTP headers please use + // GetHTTPHeaders or GetHTTPHeader. + Headers map[string]string } +// SetHTTPHeader sets the header entries associated with key to the +// single element value. It replaces any existing values +// associated with key. The key is case insensitive; it is +// canonicalized by textproto.CanonicalMIMEHeaderKey. func (req *CheckHealthRequest) SetHTTPHeader(key, value string) { if req.Headers == nil { req.Headers = map[string]string{} } - req.Headers[fmt.Sprintf("http_%s", key)] = value + req.Headers[fmt.Sprintf("%s%s", httpHeaderPrefix, key)] = value } +// GetHTTPHeader gets the first value associated with the given key. If +// there are no values associated with the key, Get returns "". +// It is case insensitive; textproto.CanonicalMIMEHeaderKey is +// used to canonicalize the provided key. Get assumes that all +// keys are stored in canonical form. func (req CheckHealthRequest) GetHTTPHeader(key string) string { return req.GetHTTPHeaders().Get(key) } +// GetHTTPHeaders returns HTTP headers. func (req CheckHealthRequest) GetHTTPHeaders() http.Header { - httpHeaders := http.Header{} - - for k, v := range req.Headers { - if textproto.CanonicalMIMEHeaderKey(k) == OAuthIdentityTokenHeaderName { - httpHeaders.Set(k, v) - } - - if textproto.CanonicalMIMEHeaderKey(k) == OAuthIdentityIDTokenHeaderName { - httpHeaders.Set(k, v) - } - - if strings.HasPrefix(k, "http_") { - hKey := strings.TrimPrefix(k, "http_") - httpHeaders.Set(hKey, v) - } - } - - return httpHeaders + return getHTTPHeadersFromStringMap(req.Headers) } // CheckHealthResult contains the healthcheck response type CheckHealthResult struct { - Status HealthStatus - Message string + // Status the HealthStatus of the healthcheck. + Status HealthStatus + + // Message the message of the healthcheck, if any. + Message string + + // JSONDetails the details of the healthcheck, if any, encoded as JSON bytes. JSONDetails []byte } @@ -120,10 +121,14 @@ func (fn CollectMetricsHandlerFunc) CollectMetrics(ctx context.Context, req *Col // CollectMetricsRequest contains the metrics request type CollectMetricsRequest struct { + // PluginContext the contextual information for the request. PluginContext PluginContext } // CollectMetricsResult collect metrics result. type CollectMetricsResult struct { + // PrometheusMetrics the Prometheus metrics encoded as bytes. PrometheusMetrics []byte } + +var _ ForwardHTTPHeaders = (*CheckHealthRequest)(nil) diff --git a/backend/diagnostics_test.go b/backend/diagnostics_test.go new file mode 100644 index 000000000..00a802888 --- /dev/null +++ b/backend/diagnostics_test.go @@ -0,0 +1,81 @@ +package backend + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestCheckHealthRequest(t *testing.T) { + req := &CheckHealthRequest{} + const customHeaderName = "X-Custom" + + t.Run("Legacy headers", func(t *testing.T) { + req.Headers = map[string]string{ + "Authorization": "a", + "X-ID-Token": "b", + "Cookies": "c", + customHeaderName: "d", + } + + t.Run("GetHTTPHeaders canonical form", func(t *testing.T) { + headers := req.GetHTTPHeaders() + require.Equal(t, "a", headers.Get(OAuthIdentityTokenHeaderName)) + require.Equal(t, "b", headers.Get(OAuthIdentityIDTokenHeaderName)) + require.Equal(t, "c", headers.Get(CookiesHeaderName)) + require.Empty(t, headers.Get(customHeaderName)) + }) + + t.Run("GetHTTPHeader canonical form", func(t *testing.T) { + require.Equal(t, "a", req.GetHTTPHeader(OAuthIdentityTokenHeaderName)) + require.Equal(t, "b", req.GetHTTPHeader(OAuthIdentityIDTokenHeaderName)) + require.Equal(t, "c", req.GetHTTPHeader(CookiesHeaderName)) + require.Empty(t, req.GetHTTPHeader(customHeaderName)) + }) + }) + + t.Run("SetHTTPHeader canonical form", func(t *testing.T) { + req.SetHTTPHeader(OAuthIdentityTokenHeaderName, "a") + req.SetHTTPHeader(OAuthIdentityIDTokenHeaderName, "b") + req.SetHTTPHeader(CookiesHeaderName, "c") + req.SetHTTPHeader(customHeaderName, "d") + + t.Run("GetHTTPHeaders canonical form", func(t *testing.T) { + headers := req.GetHTTPHeaders() + require.Equal(t, "a", headers.Get(OAuthIdentityTokenHeaderName)) + require.Equal(t, "b", headers.Get(OAuthIdentityIDTokenHeaderName)) + require.Equal(t, "c", headers.Get(CookiesHeaderName)) + require.Equal(t, "d", headers.Get(customHeaderName)) + }) + + t.Run("GetHTTPHeader canonical form", func(t *testing.T) { + require.Equal(t, "a", req.GetHTTPHeader(OAuthIdentityTokenHeaderName)) + require.Equal(t, "b", req.GetHTTPHeader(OAuthIdentityIDTokenHeaderName)) + require.Equal(t, "c", req.GetHTTPHeader(CookiesHeaderName)) + require.Equal(t, "d", req.GetHTTPHeader(customHeaderName)) + }) + }) + + t.Run("SetHTTPHeader non-canonical form", func(t *testing.T) { + req.SetHTTPHeader(strings.ToLower(OAuthIdentityTokenHeaderName), "a") + req.SetHTTPHeader(strings.ToLower(OAuthIdentityIDTokenHeaderName), "b") + req.SetHTTPHeader(strings.ToLower(CookiesHeaderName), "c") + req.SetHTTPHeader(strings.ToLower(customHeaderName), "d") + + t.Run("GetHTTPHeaders non-canonical form", func(t *testing.T) { + headers := req.GetHTTPHeaders() + require.Equal(t, "a", headers.Get(strings.ToLower(OAuthIdentityTokenHeaderName))) + require.Equal(t, "b", headers.Get(strings.ToLower(OAuthIdentityIDTokenHeaderName))) + require.Equal(t, "c", headers.Get(strings.ToLower(CookiesHeaderName))) + require.Equal(t, "d", headers.Get(strings.ToLower(customHeaderName))) + }) + + t.Run("GetHTTPHeader non-canonical form", func(t *testing.T) { + require.Equal(t, "a", req.GetHTTPHeader(strings.ToLower(OAuthIdentityTokenHeaderName))) + require.Equal(t, "b", req.GetHTTPHeader(strings.ToLower(OAuthIdentityIDTokenHeaderName))) + require.Equal(t, "c", req.GetHTTPHeader(strings.ToLower(CookiesHeaderName))) + require.Equal(t, "d", req.GetHTTPHeader(strings.ToLower(customHeaderName))) + }) + }) +} diff --git a/backend/http_headers.go b/backend/http_headers.go new file mode 100644 index 000000000..3340e734b --- /dev/null +++ b/backend/http_headers.go @@ -0,0 +1,67 @@ +package backend + +import ( + "net/http" + "net/textproto" + "strings" +) + +const ( + // OAuthIdentityTokenHeaderName the header name used for forwarding + // OAuth Identity access token. + OAuthIdentityTokenHeaderName = "Authorization" + + // OAuthIdentityIDTokenHeaderName the header name used for forwarding + // OAuth Identity ID token. + OAuthIdentityIDTokenHeaderName = "X-Id-Token" + + // CookiesHeaderName the header name used for forwarding + // cookies. + CookiesHeaderName = "Cookies" + + httpHeaderPrefix = "http_" +) + +// ForwardHTTPHeaders interface marking that forward of HTTP headers is supported. +type ForwardHTTPHeaders interface { + // SetHTTPHeader sets the header entries associated with key to the + // single element value. It replaces any existing values + // associated with key. The key is case insensitive; it is + // canonicalized by textproto.CanonicalMIMEHeaderKey. + SetHTTPHeader(key, value string) + + // GetHTTPHeader gets the first value associated with the given key. If + // there are no values associated with the key, Get returns "". + // It is case insensitive; textproto.CanonicalMIMEHeaderKey is + // used to canonicalize the provided key. Get assumes that all + // keys are stored in canonical form. + GetHTTPHeader(key string) string + + // GetHTTPHeaders returns HTTP headers. + GetHTTPHeaders() http.Header +} + +func getHTTPHeadersFromStringMap(headers map[string]string) http.Header { + httpHeaders := http.Header{} + + for k, v := range headers { + if textproto.CanonicalMIMEHeaderKey(k) == OAuthIdentityTokenHeaderName { + httpHeaders.Set(k, v) + } + + if textproto.CanonicalMIMEHeaderKey(k) == OAuthIdentityIDTokenHeaderName { + httpHeaders.Set(k, v) + } + + if textproto.CanonicalMIMEHeaderKey(k) == CookiesHeaderName { + httpHeaders.Set(k, v) + } + + if strings.HasPrefix(k, httpHeaderPrefix) { + hKey := strings.TrimPrefix(k, httpHeaderPrefix) + httpHeaders.Set(hKey, v) + } + } + + return httpHeaders +} diff --git a/backend/http_headers_test.go b/backend/http_headers_test.go new file mode 100644 index 000000000..b2c2afdab --- /dev/null +++ b/backend/http_headers_test.go @@ -0,0 +1,57 @@ +package backend + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestGetHTTPHeadersFromStringMap(t *testing.T) { + tcs := []struct { + input map[string]string + expected map[string]string + }{ + { + input: map[string]string{ + "authorization": "a", + "x-id-token": "b", + "cookies": "c", + }, + expected: map[string]string{ + "": "", + "a": "", + "authorization": "a", + "Authorization": "a", + "x-id-token": "b", + "X-Id-Token": "b", + "cookies": "c", + "Cookies": "c", + }, + }, + { + input: map[string]string{ + "Authorization": "a", + "X-ID-Token": "b", + "Cookies": "c", + }, + expected: map[string]string{ + "": "", + "a": "", + "authorization": "a", + "Authorization": "a", + "x-id-token": "b", + "X-Id-Token": "b", + "cookies": "c", + "Cookies": "c", + }, + }, + } + + for _, tc := range tcs { + headers := getHTTPHeadersFromStringMap(tc.input) + + for k, v := range tc.expected { + require.Equal(t, v, headers.Get(k)) + } + } +} diff --git a/backend/resource.go b/backend/resource.go index 9345332b5..e12ac7226 100644 --- a/backend/resource.go +++ b/backend/resource.go @@ -7,15 +7,33 @@ import ( // CallResourceRequest represents a request for a resource call. type CallResourceRequest struct { - ForwardHTTPHeaders + // PluginContext the contextual information for the request. PluginContext PluginContext - Path string - Method string - URL string - Headers map[string][]string - Body []byte + + // Path the forwarded HTTP path for the request. + Path string + + // Method the forwarded HTTP method for the request. + Method string + + // URL the forwarded HTTP URL for the request. + URL string + + // Headers the forwarded HTTP headers for the request, if any. + // + // Recommended to use GetHTTPHeaders or GetHTTPHeader + // since it automatically handles canonicalization of + // HTTP header keys. + Headers map[string][]string + + // Body the forwarded HTTP body for the request, if any. + Body []byte } +// SetHTTPHeader sets the header entries associated with key to the +// single element value. It replaces any existing values +// associated with key. The key is case insensitive; it is +// canonicalized by textproto.CanonicalMIMEHeaderKey. func (req *CallResourceRequest) SetHTTPHeader(key, value string) { if req.Headers == nil { req.Headers = map[string][]string{} @@ -24,10 +42,16 @@ func (req *CallResourceRequest) SetHTTPHeader(key, value string) { req.Headers[key] = []string{value} } +// GetHTTPHeader gets the first value associated with the given key. If +// there are no values associated with the key, Get returns "". +// It is case insensitive; textproto.CanonicalMIMEHeaderKey is +// used to canonicalize the provided key. Get assumes that all +// keys are stored in canonical form. func (req CallResourceRequest) GetHTTPHeader(key string) string { return req.GetHTTPHeaders().Get(key) } +// GetHTTPHeaders returns HTTP headers. func (req CallResourceRequest) GetHTTPHeaders() http.Header { httpHeaders := http.Header{} @@ -42,9 +66,14 @@ func (req CallResourceRequest) GetHTTPHeaders() http.Header { // CallResourceResponse represents a response from a resource call. type CallResourceResponse struct { - Status int + // Status the HTTP response status. + Status int + + // Headers the HTTP response headers. Headers map[string][]string - Body []byte + + // Body the HTTP response body. + Body []byte } // CallResourceResponseSender is used for sending resource call responses. @@ -67,3 +96,5 @@ type CallResourceHandlerFunc func(ctx context.Context, req *CallResourceRequest, func (fn CallResourceHandlerFunc) CallResource(ctx context.Context, req *CallResourceRequest, sender CallResourceResponseSender) error { return fn(ctx, req, sender) } + +var _ ForwardHTTPHeaders = (*CallResourceRequest)(nil) diff --git a/backend/resource_test.go b/backend/resource_test.go new file mode 100644 index 000000000..53f0663b3 --- /dev/null +++ b/backend/resource_test.go @@ -0,0 +1,81 @@ +package backend + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestCallResourceRequest(t *testing.T) { + req := &CallResourceRequest{} + const customHeaderName = "X-Custom" + + t.Run("Legacy headers", func(t *testing.T) { + req.Headers = map[string][]string{ + "Authorization": {"a"}, + "X-ID-Token": {"b"}, + "Cookies": {"c"}, + customHeaderName: {"d"}, + } + + t.Run("GetHTTPHeaders canonical form", func(t *testing.T) { + headers := req.GetHTTPHeaders() + require.Equal(t, "a", headers.Get(OAuthIdentityTokenHeaderName)) + require.Equal(t, "b", headers.Get(OAuthIdentityIDTokenHeaderName)) + require.Equal(t, "c", headers.Get(CookiesHeaderName)) + require.Equal(t, "d", headers.Get(customHeaderName)) + }) + + t.Run("GetHTTPHeader canonical form", func(t *testing.T) { + require.Equal(t, "a", req.GetHTTPHeader(OAuthIdentityTokenHeaderName)) + require.Equal(t, "b", req.GetHTTPHeader(OAuthIdentityIDTokenHeaderName)) + require.Equal(t, "c", req.GetHTTPHeader(CookiesHeaderName)) + require.Equal(t, "d", req.GetHTTPHeader(customHeaderName)) + }) + }) + + t.Run("SetHTTPHeader canonical form", func(t *testing.T) { + req.SetHTTPHeader(OAuthIdentityTokenHeaderName, "a") + req.SetHTTPHeader(OAuthIdentityIDTokenHeaderName, "b") + req.SetHTTPHeader(CookiesHeaderName, "c") + req.SetHTTPHeader(customHeaderName, "d") + + t.Run("GetHTTPHeaders canonical form", func(t *testing.T) { + headers := req.GetHTTPHeaders() + require.Equal(t, "a", headers.Get(OAuthIdentityTokenHeaderName)) + require.Equal(t, "b", headers.Get(OAuthIdentityIDTokenHeaderName)) + require.Equal(t, "c", headers.Get(CookiesHeaderName)) + require.Equal(t, "d", headers.Get(customHeaderName)) + }) + + t.Run("GetHTTPHeader canonical form", func(t *testing.T) { + require.Equal(t, "a", req.GetHTTPHeader(OAuthIdentityTokenHeaderName)) + require.Equal(t, "b", req.GetHTTPHeader(OAuthIdentityIDTokenHeaderName)) + require.Equal(t, "c", req.GetHTTPHeader(CookiesHeaderName)) + require.Equal(t, "d", req.GetHTTPHeader(customHeaderName)) + }) + }) + + t.Run("SetHTTPHeader non-canonical form", func(t *testing.T) { + req.SetHTTPHeader(strings.ToLower(OAuthIdentityTokenHeaderName), "a") + req.SetHTTPHeader(strings.ToLower(OAuthIdentityIDTokenHeaderName), "b") + req.SetHTTPHeader(strings.ToLower(CookiesHeaderName), "c") + req.SetHTTPHeader(strings.ToLower(customHeaderName), "d") + + t.Run("GetHTTPHeaders non-canonical form", func(t *testing.T) { + headers := req.GetHTTPHeaders() + require.Equal(t, "a", headers.Get(strings.ToLower(OAuthIdentityTokenHeaderName))) + require.Equal(t, "b", headers.Get(strings.ToLower(OAuthIdentityIDTokenHeaderName))) + require.Equal(t, "c", headers.Get(strings.ToLower(CookiesHeaderName))) + require.Equal(t, "d", headers.Get(strings.ToLower(customHeaderName))) + }) + + t.Run("GetHTTPHeader non-canonical form", func(t *testing.T) { + require.Equal(t, "a", req.GetHTTPHeader(strings.ToLower(OAuthIdentityTokenHeaderName))) + require.Equal(t, "b", req.GetHTTPHeader(strings.ToLower(OAuthIdentityIDTokenHeaderName))) + require.Equal(t, "c", req.GetHTTPHeader(strings.ToLower(CookiesHeaderName))) + require.Equal(t, "d", req.GetHTTPHeader(strings.ToLower(customHeaderName))) + }) + }) +} From b672e9417e830ef8a2e04425c3469860b699b896 Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Tue, 6 Dec 2022 16:34:56 +0100 Subject: [PATCH 3/6] change to pointer receivers --- backend/data.go | 4 ++-- backend/diagnostics.go | 4 ++-- backend/resource.go | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/backend/data.go b/backend/data.go index b9c67df58..8fa54d639 100644 --- a/backend/data.go +++ b/backend/data.go @@ -69,12 +69,12 @@ func (req *QueryDataRequest) SetHTTPHeader(key, value string) { // It is case insensitive; textproto.CanonicalMIMEHeaderKey is // used to canonicalize the provided key. Get assumes that all // keys are stored in canonical form. -func (req QueryDataRequest) GetHTTPHeader(key string) string { +func (req *QueryDataRequest) GetHTTPHeader(key string) string { return req.GetHTTPHeaders().Get(key) } // GetHTTPHeaders returns HTTP headers. -func (req QueryDataRequest) GetHTTPHeaders() http.Header { +func (req *QueryDataRequest) GetHTTPHeaders() http.Header { return getHTTPHeadersFromStringMap(req.Headers) } diff --git a/backend/diagnostics.go b/backend/diagnostics.go index ea7ebd004..d3d3de799 100644 --- a/backend/diagnostics.go +++ b/backend/diagnostics.go @@ -82,12 +82,12 @@ func (req *CheckHealthRequest) SetHTTPHeader(key, value string) { // It is case insensitive; textproto.CanonicalMIMEHeaderKey is // used to canonicalize the provided key. Get assumes that all // keys are stored in canonical form. -func (req CheckHealthRequest) GetHTTPHeader(key string) string { +func (req *CheckHealthRequest) GetHTTPHeader(key string) string { return req.GetHTTPHeaders().Get(key) } // GetHTTPHeaders returns HTTP headers. -func (req CheckHealthRequest) GetHTTPHeaders() http.Header { +func (req *CheckHealthRequest) GetHTTPHeaders() http.Header { return getHTTPHeadersFromStringMap(req.Headers) } diff --git a/backend/resource.go b/backend/resource.go index e12ac7226..97d1e5c14 100644 --- a/backend/resource.go +++ b/backend/resource.go @@ -47,12 +47,12 @@ func (req *CallResourceRequest) SetHTTPHeader(key, value string) { // It is case insensitive; textproto.CanonicalMIMEHeaderKey is // used to canonicalize the provided key. Get assumes that all // keys are stored in canonical form. -func (req CallResourceRequest) GetHTTPHeader(key string) string { +func (req *CallResourceRequest) GetHTTPHeader(key string) string { return req.GetHTTPHeaders().Get(key) } // GetHTTPHeaders returns HTTP headers. -func (req CallResourceRequest) GetHTTPHeaders() http.Header { +func (req *CallResourceRequest) GetHTTPHeaders() http.Header { httpHeaders := http.Header{} for k, v := range req.Headers { From bfbb1b7dc216606484d982ca604144e5339ec172 Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Tue, 6 Dec 2022 17:05:31 +0100 Subject: [PATCH 4/6] fix cookie header name --- backend/data_test.go | 2 +- backend/diagnostics_test.go | 2 +- backend/http_headers.go | 2 +- backend/http_headers_test.go | 12 ++++++------ backend/resource_test.go | 2 +- 5 files changed, 10 insertions(+), 10 deletions(-) diff --git a/backend/data_test.go b/backend/data_test.go index f24329eca..5002a6b3f 100644 --- a/backend/data_test.go +++ b/backend/data_test.go @@ -15,7 +15,7 @@ func TestQueryDataRequest(t *testing.T) { req.Headers = map[string]string{ "Authorization": "a", "X-ID-Token": "b", - "Cookies": "c", + "Cookie": "c", customHeaderName: "d", } diff --git a/backend/diagnostics_test.go b/backend/diagnostics_test.go index 00a802888..4b8e115e8 100644 --- a/backend/diagnostics_test.go +++ b/backend/diagnostics_test.go @@ -15,7 +15,7 @@ func TestCheckHealthRequest(t *testing.T) { req.Headers = map[string]string{ "Authorization": "a", "X-ID-Token": "b", - "Cookies": "c", + "Cookie": "c", customHeaderName: "d", } diff --git a/backend/http_headers.go b/backend/http_headers.go index 3340e734b..89d728c1f 100644 --- a/backend/http_headers.go +++ b/backend/http_headers.go @@ -17,7 +17,7 @@ const ( // CookiesHeaderName the header name used for forwarding // cookies. - CookiesHeaderName = "Cookies" + CookiesHeaderName = "Cookie" httpHeaderPrefix = "http_" ) diff --git a/backend/http_headers_test.go b/backend/http_headers_test.go index b2c2afdab..2a65cc1ae 100644 --- a/backend/http_headers_test.go +++ b/backend/http_headers_test.go @@ -15,7 +15,7 @@ func TestGetHTTPHeadersFromStringMap(t *testing.T) { input: map[string]string{ "authorization": "a", "x-id-token": "b", - "cookies": "c", + "cookie": "c", }, expected: map[string]string{ "": "", @@ -24,15 +24,15 @@ func TestGetHTTPHeadersFromStringMap(t *testing.T) { "Authorization": "a", "x-id-token": "b", "X-Id-Token": "b", - "cookies": "c", - "Cookies": "c", + "cookie": "c", + "Cookie": "c", }, }, { input: map[string]string{ "Authorization": "a", "X-ID-Token": "b", - "Cookies": "c", + "Cookie": "c", }, expected: map[string]string{ "": "", @@ -41,8 +41,8 @@ func TestGetHTTPHeadersFromStringMap(t *testing.T) { "Authorization": "a", "x-id-token": "b", "X-Id-Token": "b", - "cookies": "c", - "Cookies": "c", + "cookie": "c", + "Cookie": "c", }, }, } diff --git a/backend/resource_test.go b/backend/resource_test.go index 53f0663b3..03ad437c2 100644 --- a/backend/resource_test.go +++ b/backend/resource_test.go @@ -15,7 +15,7 @@ func TestCallResourceRequest(t *testing.T) { req.Headers = map[string][]string{ "Authorization": {"a"}, "X-ID-Token": {"b"}, - "Cookies": {"c"}, + "Cookie": {"c"}, customHeaderName: {"d"}, } From 29a26824c4df5df36c9d37c8a7625a9b42d610de Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Thu, 8 Dec 2022 13:02:16 +0100 Subject: [PATCH 5/6] add DeleteHTTPHeader method --- backend/data.go | 23 +++++++++++++++++++++++ backend/data_test.go | 24 ++++++++++++++++++++++++ backend/diagnostics.go | 23 +++++++++++++++++++++++ backend/diagnostics_test.go | 24 ++++++++++++++++++++++++ backend/http_headers.go | 5 +++++ backend/resource.go | 22 ++++++++++++++++++++++ backend/resource_test.go | 24 ++++++++++++++++++++++++ 7 files changed, 145 insertions(+) diff --git a/backend/data.go b/backend/data.go index 8fa54d639..acec88bb8 100644 --- a/backend/data.go +++ b/backend/data.go @@ -6,6 +6,7 @@ import ( "errors" "fmt" "net/http" + "net/textproto" "time" "github.com/grafana/grafana-plugin-sdk-go/data" @@ -64,6 +65,28 @@ func (req *QueryDataRequest) SetHTTPHeader(key, value string) { req.Headers[fmt.Sprintf("%s%s", httpHeaderPrefix, key)] = value } +// DeleteHTTPHeader deletes the values associated with key. +// The key is case insensitive; it is canonicalized by +// CanonicalHeaderKey. +func (req *QueryDataRequest) DeleteHTTPHeader(key string) { + if req.Headers == nil { + return + } + + var deleteKey string + for k := range req.Headers { + if textproto.CanonicalMIMEHeaderKey(k) == textproto.CanonicalMIMEHeaderKey(key) || + textproto.CanonicalMIMEHeaderKey(k) == textproto.CanonicalMIMEHeaderKey(fmt.Sprintf("%s%s", httpHeaderPrefix, key)) { + deleteKey = k + break + } + } + + if deleteKey != "" { + delete(req.Headers, deleteKey) + } +} + // GetHTTPHeader gets the first value associated with the given key. If // there are no values associated with the key, Get returns "". // It is case insensitive; textproto.CanonicalMIMEHeaderKey is diff --git a/backend/data_test.go b/backend/data_test.go index 5002a6b3f..140b26c19 100644 --- a/backend/data_test.go +++ b/backend/data_test.go @@ -33,6 +33,14 @@ func TestQueryDataRequest(t *testing.T) { require.Equal(t, "c", req.GetHTTPHeader(CookiesHeaderName)) require.Empty(t, req.GetHTTPHeader(customHeaderName)) }) + + t.Run("DeleteHTTPHeader canonical form", func(t *testing.T) { + req.DeleteHTTPHeader(OAuthIdentityTokenHeaderName) + req.DeleteHTTPHeader(OAuthIdentityIDTokenHeaderName) + req.DeleteHTTPHeader(CookiesHeaderName) + req.DeleteHTTPHeader(customHeaderName) + require.Empty(t, req.Headers) + }) }) t.Run("SetHTTPHeader canonical form", func(t *testing.T) { @@ -55,6 +63,14 @@ func TestQueryDataRequest(t *testing.T) { require.Equal(t, "c", req.GetHTTPHeader(CookiesHeaderName)) require.Equal(t, "d", req.GetHTTPHeader(customHeaderName)) }) + + t.Run("DeleteHTTPHeader canonical form", func(t *testing.T) { + req.DeleteHTTPHeader(OAuthIdentityTokenHeaderName) + req.DeleteHTTPHeader(OAuthIdentityIDTokenHeaderName) + req.DeleteHTTPHeader(CookiesHeaderName) + req.DeleteHTTPHeader(customHeaderName) + require.Empty(t, req.Headers) + }) }) t.Run("SetHTTPHeader non-canonical form", func(t *testing.T) { @@ -77,5 +93,13 @@ func TestQueryDataRequest(t *testing.T) { require.Equal(t, "c", req.GetHTTPHeader(strings.ToLower(CookiesHeaderName))) require.Equal(t, "d", req.GetHTTPHeader(strings.ToLower(customHeaderName))) }) + + t.Run("DeleteHTTPHeader non-canonical form", func(t *testing.T) { + req.DeleteHTTPHeader(strings.ToLower(OAuthIdentityTokenHeaderName)) + req.DeleteHTTPHeader(strings.ToLower(OAuthIdentityIDTokenHeaderName)) + req.DeleteHTTPHeader(strings.ToLower(CookiesHeaderName)) + req.DeleteHTTPHeader(strings.ToLower(customHeaderName)) + require.Empty(t, req.Headers) + }) }) } diff --git a/backend/diagnostics.go b/backend/diagnostics.go index d3d3de799..02388daab 100644 --- a/backend/diagnostics.go +++ b/backend/diagnostics.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "net/http" + "net/textproto" "strconv" ) @@ -77,6 +78,28 @@ func (req *CheckHealthRequest) SetHTTPHeader(key, value string) { req.Headers[fmt.Sprintf("%s%s", httpHeaderPrefix, key)] = value } +// DeleteHTTPHeader deletes the values associated with key. +// The key is case insensitive; it is canonicalized by +// CanonicalHeaderKey. +func (req *CheckHealthRequest) DeleteHTTPHeader(key string) { + if req.Headers == nil { + return + } + + var deleteKey string + for k := range req.Headers { + if textproto.CanonicalMIMEHeaderKey(k) == textproto.CanonicalMIMEHeaderKey(key) || + textproto.CanonicalMIMEHeaderKey(k) == textproto.CanonicalMIMEHeaderKey(fmt.Sprintf("%s%s", httpHeaderPrefix, key)) { + deleteKey = k + break + } + } + + if deleteKey != "" { + delete(req.Headers, deleteKey) + } +} + // GetHTTPHeader gets the first value associated with the given key. If // there are no values associated with the key, Get returns "". // It is case insensitive; textproto.CanonicalMIMEHeaderKey is diff --git a/backend/diagnostics_test.go b/backend/diagnostics_test.go index 4b8e115e8..56b0057e3 100644 --- a/backend/diagnostics_test.go +++ b/backend/diagnostics_test.go @@ -33,6 +33,14 @@ func TestCheckHealthRequest(t *testing.T) { require.Equal(t, "c", req.GetHTTPHeader(CookiesHeaderName)) require.Empty(t, req.GetHTTPHeader(customHeaderName)) }) + + t.Run("DeleteHTTPHeader canonical form", func(t *testing.T) { + req.DeleteHTTPHeader(OAuthIdentityTokenHeaderName) + req.DeleteHTTPHeader(OAuthIdentityIDTokenHeaderName) + req.DeleteHTTPHeader(CookiesHeaderName) + req.DeleteHTTPHeader(customHeaderName) + require.Empty(t, req.Headers) + }) }) t.Run("SetHTTPHeader canonical form", func(t *testing.T) { @@ -55,6 +63,14 @@ func TestCheckHealthRequest(t *testing.T) { require.Equal(t, "c", req.GetHTTPHeader(CookiesHeaderName)) require.Equal(t, "d", req.GetHTTPHeader(customHeaderName)) }) + + t.Run("DeleteHTTPHeader canonical form", func(t *testing.T) { + req.DeleteHTTPHeader(OAuthIdentityTokenHeaderName) + req.DeleteHTTPHeader(OAuthIdentityIDTokenHeaderName) + req.DeleteHTTPHeader(CookiesHeaderName) + req.DeleteHTTPHeader(customHeaderName) + require.Empty(t, req.Headers) + }) }) t.Run("SetHTTPHeader non-canonical form", func(t *testing.T) { @@ -77,5 +93,13 @@ func TestCheckHealthRequest(t *testing.T) { require.Equal(t, "c", req.GetHTTPHeader(strings.ToLower(CookiesHeaderName))) require.Equal(t, "d", req.GetHTTPHeader(strings.ToLower(customHeaderName))) }) + + t.Run("DeleteHTTPHeader non-canonical form", func(t *testing.T) { + req.DeleteHTTPHeader(strings.ToLower(OAuthIdentityTokenHeaderName)) + req.DeleteHTTPHeader(strings.ToLower(OAuthIdentityIDTokenHeaderName)) + req.DeleteHTTPHeader(strings.ToLower(CookiesHeaderName)) + req.DeleteHTTPHeader(strings.ToLower(customHeaderName)) + require.Empty(t, req.Headers) + }) }) } diff --git a/backend/http_headers.go b/backend/http_headers.go index 89d728c1f..1201b10e8 100644 --- a/backend/http_headers.go +++ b/backend/http_headers.go @@ -30,6 +30,11 @@ type ForwardHTTPHeaders interface { // canonicalized by textproto.CanonicalMIMEHeaderKey. SetHTTPHeader(key, value string) + // DeleteHTTPHeader deletes the values associated with key. + // The key is case insensitive; it is canonicalized by + // CanonicalHeaderKey. + DeleteHTTPHeader(key string) + // GetHTTPHeader gets the first value associated with the given key. If // there are no values associated with the key, Get returns "". // It is case insensitive; textproto.CanonicalMIMEHeaderKey is diff --git a/backend/resource.go b/backend/resource.go index 97d1e5c14..c99e7117c 100644 --- a/backend/resource.go +++ b/backend/resource.go @@ -3,6 +3,7 @@ package backend import ( "context" "net/http" + "net/textproto" ) // CallResourceRequest represents a request for a resource call. @@ -42,6 +43,27 @@ func (req *CallResourceRequest) SetHTTPHeader(key, value string) { req.Headers[key] = []string{value} } +// DeleteHTTPHeader deletes the values associated with key. +// The key is case insensitive; it is canonicalized by +// CanonicalHeaderKey. +func (req *CallResourceRequest) DeleteHTTPHeader(key string) { + if req.Headers == nil { + return + } + + var deleteKey string + for k := range req.Headers { + if textproto.CanonicalMIMEHeaderKey(k) == textproto.CanonicalMIMEHeaderKey(key) { + deleteKey = k + break + } + } + + if deleteKey != "" { + delete(req.Headers, deleteKey) + } +} + // GetHTTPHeader gets the first value associated with the given key. If // there are no values associated with the key, Get returns "". // It is case insensitive; textproto.CanonicalMIMEHeaderKey is diff --git a/backend/resource_test.go b/backend/resource_test.go index 03ad437c2..5dfe2420e 100644 --- a/backend/resource_test.go +++ b/backend/resource_test.go @@ -33,6 +33,14 @@ func TestCallResourceRequest(t *testing.T) { require.Equal(t, "c", req.GetHTTPHeader(CookiesHeaderName)) require.Equal(t, "d", req.GetHTTPHeader(customHeaderName)) }) + + t.Run("DeleteHTTPHeader canonical form", func(t *testing.T) { + req.DeleteHTTPHeader(OAuthIdentityTokenHeaderName) + req.DeleteHTTPHeader(OAuthIdentityIDTokenHeaderName) + req.DeleteHTTPHeader(CookiesHeaderName) + req.DeleteHTTPHeader(customHeaderName) + require.Empty(t, req.Headers) + }) }) t.Run("SetHTTPHeader canonical form", func(t *testing.T) { @@ -55,6 +63,14 @@ func TestCallResourceRequest(t *testing.T) { require.Equal(t, "c", req.GetHTTPHeader(CookiesHeaderName)) require.Equal(t, "d", req.GetHTTPHeader(customHeaderName)) }) + + t.Run("DeleteHTTPHeader canonical form", func(t *testing.T) { + req.DeleteHTTPHeader(OAuthIdentityTokenHeaderName) + req.DeleteHTTPHeader(OAuthIdentityIDTokenHeaderName) + req.DeleteHTTPHeader(CookiesHeaderName) + req.DeleteHTTPHeader(customHeaderName) + require.Empty(t, req.Headers) + }) }) t.Run("SetHTTPHeader non-canonical form", func(t *testing.T) { @@ -77,5 +93,13 @@ func TestCallResourceRequest(t *testing.T) { require.Equal(t, "c", req.GetHTTPHeader(strings.ToLower(CookiesHeaderName))) require.Equal(t, "d", req.GetHTTPHeader(strings.ToLower(customHeaderName))) }) + + t.Run("DeleteHTTPHeader non-canonical form", func(t *testing.T) { + req.DeleteHTTPHeader(strings.ToLower(OAuthIdentityTokenHeaderName)) + req.DeleteHTTPHeader(strings.ToLower(OAuthIdentityIDTokenHeaderName)) + req.DeleteHTTPHeader(strings.ToLower(CookiesHeaderName)) + req.DeleteHTTPHeader(strings.ToLower(customHeaderName)) + require.Empty(t, req.Headers) + }) }) } From 635b577aa7a5f5f9cbf91fee2166a3d1ae462a29 Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Mon, 12 Dec 2022 10:17:10 +0100 Subject: [PATCH 6/6] refactor/cleanup --- backend/data.go | 21 +---- backend/diagnostics.go | 21 +---- backend/http_headers.go | 19 +++++ backend/http_headers_test.go | 155 ++++++++++++++++++++++++++++++++++- backend/resource.go | 7 +- 5 files changed, 178 insertions(+), 45 deletions(-) diff --git a/backend/data.go b/backend/data.go index acec88bb8..2421e6628 100644 --- a/backend/data.go +++ b/backend/data.go @@ -4,9 +4,7 @@ import ( "context" "encoding/json" "errors" - "fmt" "net/http" - "net/textproto" "time" "github.com/grafana/grafana-plugin-sdk-go/data" @@ -62,29 +60,14 @@ func (req *QueryDataRequest) SetHTTPHeader(key, value string) { req.Headers = map[string]string{} } - req.Headers[fmt.Sprintf("%s%s", httpHeaderPrefix, key)] = value + setHTTPHeaderInStringMap(req.Headers, key, value) } // DeleteHTTPHeader deletes the values associated with key. // The key is case insensitive; it is canonicalized by // CanonicalHeaderKey. func (req *QueryDataRequest) DeleteHTTPHeader(key string) { - if req.Headers == nil { - return - } - - var deleteKey string - for k := range req.Headers { - if textproto.CanonicalMIMEHeaderKey(k) == textproto.CanonicalMIMEHeaderKey(key) || - textproto.CanonicalMIMEHeaderKey(k) == textproto.CanonicalMIMEHeaderKey(fmt.Sprintf("%s%s", httpHeaderPrefix, key)) { - deleteKey = k - break - } - } - - if deleteKey != "" { - delete(req.Headers, deleteKey) - } + deleteHTTPHeaderInStringMap(req.Headers, key) } // GetHTTPHeader gets the first value associated with the given key. If diff --git a/backend/diagnostics.go b/backend/diagnostics.go index 02388daab..49d48fe20 100644 --- a/backend/diagnostics.go +++ b/backend/diagnostics.go @@ -2,9 +2,7 @@ package backend import ( "context" - "fmt" "net/http" - "net/textproto" "strconv" ) @@ -75,29 +73,14 @@ func (req *CheckHealthRequest) SetHTTPHeader(key, value string) { req.Headers = map[string]string{} } - req.Headers[fmt.Sprintf("%s%s", httpHeaderPrefix, key)] = value + setHTTPHeaderInStringMap(req.Headers, key, value) } // DeleteHTTPHeader deletes the values associated with key. // The key is case insensitive; it is canonicalized by // CanonicalHeaderKey. func (req *CheckHealthRequest) DeleteHTTPHeader(key string) { - if req.Headers == nil { - return - } - - var deleteKey string - for k := range req.Headers { - if textproto.CanonicalMIMEHeaderKey(k) == textproto.CanonicalMIMEHeaderKey(key) || - textproto.CanonicalMIMEHeaderKey(k) == textproto.CanonicalMIMEHeaderKey(fmt.Sprintf("%s%s", httpHeaderPrefix, key)) { - deleteKey = k - break - } - } - - if deleteKey != "" { - delete(req.Headers, deleteKey) - } + deleteHTTPHeaderInStringMap(req.Headers, key) } // GetHTTPHeader gets the first value associated with the given key. If diff --git a/backend/http_headers.go b/backend/http_headers.go index 1201b10e8..689d8ae59 100644 --- a/backend/http_headers.go +++ b/backend/http_headers.go @@ -1,6 +1,7 @@ package backend import ( + "fmt" "net/http" "net/textproto" "strings" @@ -46,6 +47,14 @@ type ForwardHTTPHeaders interface { GetHTTPHeaders() http.Header } +func setHTTPHeaderInStringMap(headers map[string]string, key string, value string) { + if headers == nil { + headers = map[string]string{} + } + + headers[fmt.Sprintf("%s%s", httpHeaderPrefix, key)] = value +} + func getHTTPHeadersFromStringMap(headers map[string]string) http.Header { httpHeaders := http.Header{} @@ -70,3 +79,13 @@ func getHTTPHeadersFromStringMap(headers map[string]string) http.Header { return httpHeaders } + +func deleteHTTPHeaderInStringMap(headers map[string]string, key string) { + for k := range headers { + if textproto.CanonicalMIMEHeaderKey(k) == textproto.CanonicalMIMEHeaderKey(key) || + textproto.CanonicalMIMEHeaderKey(k) == textproto.CanonicalMIMEHeaderKey(fmt.Sprintf("%s%s", httpHeaderPrefix, key)) { + delete(headers, k) + break + } + } +} diff --git a/backend/http_headers_test.go b/backend/http_headers_test.go index 2a65cc1ae..7b4070a6c 100644 --- a/backend/http_headers_test.go +++ b/backend/http_headers_test.go @@ -3,19 +3,27 @@ package backend import ( "testing" + "github.com/davecgh/go-spew/spew" "github.com/stretchr/testify/require" ) -func TestGetHTTPHeadersFromStringMap(t *testing.T) { +func TestSetHTTPHeaderInStringMap(t *testing.T) { tcs := []struct { input map[string]string expected map[string]string }{ + { + expected: map[string]string{ + "": "", + "a": "", + }, + }, { input: map[string]string{ "authorization": "a", "x-id-token": "b", "cookie": "c", + "x-custom": "d", }, expected: map[string]string{ "": "", @@ -26,6 +34,8 @@ func TestGetHTTPHeadersFromStringMap(t *testing.T) { "X-Id-Token": "b", "cookie": "c", "Cookie": "c", + "x-custom": "d", + "X-Custom": "d", }, }, { @@ -33,6 +43,74 @@ func TestGetHTTPHeadersFromStringMap(t *testing.T) { "Authorization": "a", "X-ID-Token": "b", "Cookie": "c", + "X-Custom": "d", + }, + expected: map[string]string{ + "": "", + "a": "", + "authorization": "a", + "Authorization": "a", + "x-id-token": "b", + "X-Id-Token": "b", + "cookie": "c", + "Cookie": "c", + "x-custom": "d", + "X-Custom": "d", + }, + }, + } + + for _, tc := range tcs { + headerMap := map[string]string{} + for k, v := range tc.input { + setHTTPHeaderInStringMap(headerMap, k, v) + } + headers := getHTTPHeadersFromStringMap(headerMap) + spew.Dump(headers) + + for k, v := range tc.expected { + require.Equal(t, v, headers.Get(k)) + } + } +} + +func TestGetHTTPHeadersFromStringMap(t *testing.T) { + tcs := []struct { + input map[string]string + expected map[string]string + }{ + { + expected: map[string]string{ + "": "", + "a": "", + }, + }, + { + input: map[string]string{ + "authorization": "a", + "x-id-token": "b", + "cookie": "c", + httpHeaderPrefix + "x-custom": "d", + }, + expected: map[string]string{ + "": "", + "a": "", + "authorization": "a", + "Authorization": "a", + "x-id-token": "b", + "X-Id-Token": "b", + "cookie": "c", + "Cookie": "c", + "x-custom": "d", + "X-Custom": "d", + }, + }, + { + input: map[string]string{ + "Authorization": "a", + "X-ID-Token": "b", + "Cookie": "c", + httpHeaderPrefix + "X-Custom": "d", }, expected: map[string]string{ "": "", @@ -43,6 +121,8 @@ func TestGetHTTPHeadersFromStringMap(t *testing.T) { "X-Id-Token": "b", "cookie": "c", "Cookie": "c", + "x-custom": "d", + "X-Custom": "d", }, }, } @@ -55,3 +135,76 @@ func TestGetHTTPHeadersFromStringMap(t *testing.T) { } } } + +func TestDeleteHTTPHeaderInStringMap(t *testing.T) { + tcs := []struct { + input map[string]string + deleteKeys []string + expected map[string]string + }{ + { + expected: map[string]string{ + "": "", + "a": "", + }, + }, + { + input: map[string]string{ + "authorization": "a", + "x-id-token": "b", + "cookie": "c", + httpHeaderPrefix + "x-custom": "d", + }, + deleteKeys: []string{"authorization", "x-id-token", "cookie", "x-custom"}, + expected: map[string]string{ + "": "", + "a": "", + "authorization": "", + "Authorization": "", + "x-id-token": "", + "X-Id-Token": "", + "cookie": "", + "Cookie": "", + "x-custom": "", + "X-Custom": "", + }, + }, + { + input: map[string]string{ + "Authorization": "a", + "X-ID-Token": "b", + "Cookie": "c", + httpHeaderPrefix + "X-Custom": "d", + }, + deleteKeys: []string{"Authorization", "X-Id-Token", "Cookie", "X-Custom"}, + expected: map[string]string{ + "": "", + "a": "", + "authorization": "", + "Authorization": "", + "x-id-token": "", + "X-Id-Token": "", + "cookie": "", + "Cookie": "", + "x-custom": "", + "X-Custom": "", + }, + }, + } + + for _, tc := range tcs { + headerMap := make(map[string]string, len(tc.input)) + for k, v := range tc.input { + headerMap[k] = v + } + + for _, key := range tc.deleteKeys { + deleteHTTPHeaderInStringMap(headerMap, key) + } + headers := getHTTPHeadersFromStringMap(headerMap) + + for k, v := range tc.expected { + require.Equal(t, v, headers.Get(k)) + } + } +} diff --git a/backend/resource.go b/backend/resource.go index c99e7117c..83198dcd5 100644 --- a/backend/resource.go +++ b/backend/resource.go @@ -51,17 +51,12 @@ func (req *CallResourceRequest) DeleteHTTPHeader(key string) { return } - var deleteKey string for k := range req.Headers { if textproto.CanonicalMIMEHeaderKey(k) == textproto.CanonicalMIMEHeaderKey(key) { - deleteKey = k + delete(req.Headers, k) break } } - - if deleteKey != "" { - delete(req.Headers, deleteKey) - } } // GetHTTPHeader gets the first value associated with the given key. If