Catalog Client overriding http.DefaultClient.Transport

[andy-mcgrath]

When calling catalog.NewClient(WithToken("AUTH TOKEN")) the functions updates the http.DefaultClient.Transport with a new http.Roundtrip function.

This means any other call using http.DefaultClient, including http.Get() and http.Post(), as they are wrappers of http.DefaultClient, also call the http.Roundtrip function so overriding any Authorization Header set.

I think catalog.Client needs to either make a clean copy of http.DefaultClient or use &http.Client{} for it's httpClient.

[andy-mcgrath]

Pulled together a quick test to catalog/client_test.go to test for authorization header leak:

package catalog

import (
	"context"
	"net/http"
	"net/http/httptest"
	"testing"

	"gotest.tools/v3/assert"
)

const (
	catalogTestToken = "HELLO_WORLD"
	httpTestToken    = "DLROW_OLLEH"
)

func TestNewClient(t *testing.T) {
	ctx := context.Background()

	t.Run("authorization", func(t *testing.T) {
		var authorization string
		client := newTestClients(t, func(w http.ResponseWriter, r *http.Request) {
			authorization = r.Header.Get("authorization")
			_, err := w.Write([]byte("{}"))
			assert.NilError(t, err)
		})
		_, err := client.GetEntityByUID(ctx, &GetEntityByUIDRequest{
			UID: "test",
		})
		assert.NilError(t, err)
		assert.Equal(t, "Bearer "+catalogTestToken, authorization)
	})

	t.Run("no_authorization_leak", func(t *testing.T) {
		var authorization string
		server := newHTTPTestServer(t, func(w http.ResponseWriter, r *http.Request) {
			authorization = r.Header.Get("authorization")
			_, err := w.Write([]byte("{}"))
			assert.NilError(t, err)
		})

		NewClient(
			WithBaseURL(server.URL),
			WithToken(catalogTestToken),
		)
		req, err := http.NewRequest(http.MethodGet, server.URL, nil)
		assert.NilError(t, err)
		req.Header.Add("Authorization", "Bearer "+httpTestToken)

		_, err = http.DefaultClient.Do(req)
		assert.NilError(t, err)
		assert.Equal(t, "Bearer "+httpTestToken, authorization)
	})
}

func newTestClients(t *testing.T, handler func(http.ResponseWriter, *http.Request)) *Client {
	server := httptest.NewServer(http.HandlerFunc(handler))
	t.Cleanup(server.Close)
	return NewClient(
		WithBaseURL(server.URL),
		WithToken(catalogTestToken),
	)
}

func newHTTPTestServer(t *testing.T, handler func(http.ResponseWriter, *http.Request)) *httptest.Server {
	server := httptest.NewServer(http.HandlerFunc(handler))
	t.Cleanup(server.Close)

	return server
}

[github-actions]

This issue has been open for 365 days with no activity. Marking as stale.