<!-- Add the related story/sub-task/bug number, like Resolves #123, or remove if NA --> **Related issue:** Fixes #46643 BaseClient.URL() built request paths as `URLPrefix + path`, overwriting any path already present in BaseURL. Orbit and Fleet Desktop parse the full fleet URL (subpath included) into BaseURL and pass an empty URLPrefix, so the subpath was discarded and every API call 404'd when Fleet was deployed at https://host/subpath. Preserve BaseURL.Path as a prefix on each request. fleetctl is unaffected since it carries the subpath in URLPrefix with an empty BaseURL.Path, and non-subpath deployments are unchanged. # Checklist for submitter If some of the following don't apply, delete the relevant line. - [X] Changes file added for user-visible changes in `changes/`, `orbit/changes/` or `ee/fleetd-chrome/changes`. See [Changes files](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/guides/committing-changes.md#changes-files) for more information. ## Testing - [X] Added/updated automated tests - [x] QA'd all new/changed functionality manually ## fleetd/orbit/Fleet Desktop - [x] Verified compatibility with the latest released version of Fleet (see [Must rule](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/workflows/fleetd-development-and-release-strategy.md)) - [x] Verified that fleetd runs on macOS, Linux and Windows <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Fixed API request URL construction for deployments hosted under a subpath, preventing broken requests and 404 errors. * Preserved query parameters when generating request URLs. * Improved resolution of request paths with and without a leading slash when combined with a base URL subpath. * **Documentation** * Added clearer guidance on how base URL subpaths and additional path prefixes are combined. * **Tests** * Expanded URL-generation coverage to verify correct behavior across subpath and prefix combinations. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
348 lines
12 KiB
Go
348 lines
12 KiB
Go
package client
|
|
|
|
import (
|
|
"bytes"
|
|
"crypto/tls"
|
|
"io"
|
|
"net/http"
|
|
"net/http/httptest"
|
|
"path/filepath"
|
|
"strings"
|
|
"testing"
|
|
|
|
"github.com/fleetdm/fleet/v4/pkg/certificate"
|
|
"github.com/fleetdm/fleet/v4/server/fleet"
|
|
"github.com/stretchr/testify/require"
|
|
)
|
|
|
|
func TestUrlGeneration(t *testing.T) {
|
|
t.Run("without prefix", func(t *testing.T) {
|
|
bc, err := NewBaseClient("https://test.com", true, "", "", nil, fleet.CapabilityMap{}, nil)
|
|
require.NoError(t, err)
|
|
require.Equal(t, "https://test.com/test/path", bc.URL("test/path", "").String())
|
|
require.Equal(t, "https://test.com/test/path?raw=query", bc.URL("test/path", "raw=query").String())
|
|
})
|
|
|
|
t.Run("with prefix", func(t *testing.T) {
|
|
bc, err := NewBaseClient("https://test.com", true, "", "prefix/", nil, fleet.CapabilityMap{}, nil)
|
|
require.NoError(t, err)
|
|
require.Equal(t, "https://test.com/prefix/test/path", bc.URL("test/path", "").String())
|
|
require.Equal(t, "https://test.com/prefix/test/path?raw=query", bc.URL("test/path", "raw=query").String())
|
|
})
|
|
|
|
t.Run("with subpath in base URL", func(t *testing.T) {
|
|
bc, err := NewBaseClient("https://test.com/subpath", true, "", "", nil, fleet.CapabilityMap{}, nil)
|
|
require.NoError(t, err)
|
|
require.Equal(t, "https://test.com/subpath/api/fleet/orbit/enroll", bc.URL("/api/fleet/orbit/enroll", "").String())
|
|
require.Equal(t, "https://test.com/subpath/api/fleet/orbit/enroll?raw=query", bc.URL("/api/fleet/orbit/enroll", "raw=query").String())
|
|
})
|
|
|
|
t.Run("with subpath and trailing slash in base URL", func(t *testing.T) {
|
|
bc, err := NewBaseClient("https://test.com/subpath/", true, "", "", nil, fleet.CapabilityMap{}, nil)
|
|
require.NoError(t, err)
|
|
require.Equal(t, "https://test.com/subpath/api/fleet/orbit/enroll", bc.URL("/api/fleet/orbit/enroll", "").String())
|
|
})
|
|
|
|
t.Run("with subpath and path without leading slash", func(t *testing.T) {
|
|
bc, err := NewBaseClient("https://test.com/subpath", true, "", "", nil, fleet.CapabilityMap{}, nil)
|
|
require.NoError(t, err)
|
|
require.Equal(t, "https://test.com/subpath/test/path", bc.URL("test/path", "").String())
|
|
})
|
|
|
|
t.Run("with subpath in base URL and a prefix", func(t *testing.T) {
|
|
bc, err := NewBaseClient("https://test.com/subpath", true, "", "prefix/", nil, fleet.CapabilityMap{}, nil)
|
|
require.NoError(t, err)
|
|
require.Equal(t, "https://test.com/subpath/prefix/test/path", bc.URL("test/path", "").String())
|
|
require.Equal(t, "https://test.com/subpath/prefix/test/path?raw=query", bc.URL("test/path", "raw=query").String())
|
|
})
|
|
}
|
|
|
|
func TestParseResponseKnownErrors(t *testing.T) {
|
|
cases := []struct {
|
|
message string
|
|
code int
|
|
out error
|
|
}{
|
|
{"not found errors", http.StatusNotFound, &NotFoundErr{}},
|
|
{"unauthenticated errors", http.StatusUnauthorized, ErrUnauthenticated},
|
|
{"license errors", http.StatusPaymentRequired, ErrMissingLicense},
|
|
}
|
|
|
|
for _, c := range cases {
|
|
t.Run(c.message, func(t *testing.T) {
|
|
bc, err := NewBaseClient("https://test.com", true, "", "", nil, fleet.CapabilityMap{}, nil)
|
|
require.NoError(t, err)
|
|
response := &http.Response{
|
|
StatusCode: c.code,
|
|
Body: io.NopCloser(bytes.NewBufferString(`{"test": "ok"}`)),
|
|
}
|
|
err = bc.ParseResponse("GET", "", response, &struct{}{})
|
|
require.ErrorIs(t, err, c.out)
|
|
})
|
|
}
|
|
}
|
|
|
|
func TestParseResponseOK(t *testing.T) {
|
|
bc, err := NewBaseClient("https://test.com", true, "", "", nil, fleet.CapabilityMap{}, nil)
|
|
require.NoError(t, err)
|
|
response := &http.Response{
|
|
StatusCode: http.StatusOK,
|
|
Body: io.NopCloser(bytes.NewBufferString(`{"test": "ok"}`)),
|
|
}
|
|
|
|
var resDest struct{ Test string }
|
|
err = bc.ParseResponse("", "", response, &resDest)
|
|
require.NoError(t, err)
|
|
require.Equal(t, "ok", resDest.Test)
|
|
}
|
|
|
|
func TestParseResponseOKNoContent(t *testing.T) {
|
|
bc, err := NewBaseClient("https://test.com", true, "", "", nil, fleet.CapabilityMap{}, nil)
|
|
require.NoError(t, err)
|
|
response := &http.Response{
|
|
StatusCode: http.StatusNoContent,
|
|
Body: io.NopCloser(bytes.NewBufferString("")),
|
|
}
|
|
|
|
var resDest struct{ Err error }
|
|
err = bc.ParseResponse("", "", response, &resDest)
|
|
require.NoError(t, err)
|
|
require.Nil(t, resDest.Err)
|
|
}
|
|
|
|
func TestParseResponseGeneralErrors(t *testing.T) {
|
|
t.Run("general HTTP errors", func(t *testing.T) {
|
|
bc, err := NewBaseClient("https://test.com", true, "", "", nil, fleet.CapabilityMap{}, nil)
|
|
require.NoError(t, err)
|
|
response := &http.Response{
|
|
StatusCode: http.StatusBadRequest,
|
|
Body: io.NopCloser(bytes.NewBufferString(`{"test": "ok"}`)),
|
|
}
|
|
err = bc.ParseResponse("GET", "", response, &struct{}{})
|
|
require.Error(t, err)
|
|
})
|
|
|
|
t.Run("parse errors", func(t *testing.T) {
|
|
bc, err := NewBaseClient("https://test.com", true, "", "", nil, fleet.CapabilityMap{}, nil)
|
|
require.NoError(t, err)
|
|
response := &http.Response{
|
|
StatusCode: http.StatusBadRequest,
|
|
Body: io.NopCloser(bytes.NewBufferString(`invalid json`)),
|
|
}
|
|
err = bc.ParseResponse("GET", "", response, &struct{}{})
|
|
require.Error(t, err)
|
|
})
|
|
}
|
|
|
|
func TestNewBaseClient(t *testing.T) {
|
|
t.Run("invalid addresses are an error", func(t *testing.T) {
|
|
_, err := NewBaseClient("http://foo\x7f.com/", true, "", "", nil, fleet.CapabilityMap{}, nil)
|
|
require.Error(t, err)
|
|
})
|
|
|
|
t.Run("http is only valid in development", func(t *testing.T) {
|
|
cases := []struct {
|
|
name string
|
|
address string
|
|
insecureSkipVerify bool
|
|
expectedErr error
|
|
}{
|
|
{"http non-local URL without insecureSkipVerify", "http://test.com", false, ErrInvalidScheme},
|
|
{"http non-local URL with insecureSkipVerify", "http://test.com", true, nil},
|
|
{"https", "https://test.com", false, nil},
|
|
{"http localhost with insecureSkipVerify", "http://localhost:8080", true, nil},
|
|
{"http localhost without insecureSkipVerify", "http://localhost:8080", false, nil},
|
|
{"http local ip with insecureSkipVerify", "http://127.0.0.1:8080", true, nil},
|
|
{"http local ip without insecureSkipVerify", "http://127.0.0.1:8080", false, nil},
|
|
}
|
|
|
|
for _, c := range cases {
|
|
_, err := NewBaseClient(c.address, c.insecureSkipVerify, "", "", nil, fleet.CapabilityMap{}, nil)
|
|
require.Equal(t, c.expectedErr, err, c.name)
|
|
}
|
|
})
|
|
}
|
|
|
|
func TestClientCapabilities(t *testing.T) {
|
|
cases := []struct {
|
|
name string
|
|
capabilities fleet.CapabilityMap
|
|
expected string
|
|
}{
|
|
{"no capabilities", fleet.CapabilityMap{}, ""},
|
|
{"one capability", fleet.CapabilityMap{fleet.Capability("test_capability"): {}}, "test_capability"},
|
|
{
|
|
"multiple capabilities",
|
|
fleet.CapabilityMap{
|
|
fleet.Capability("test_capability"): {},
|
|
fleet.Capability("test_capability_2"): {},
|
|
},
|
|
"test_capability,test_capability_2",
|
|
},
|
|
}
|
|
|
|
for _, c := range cases {
|
|
t.Run(c.name, func(t *testing.T) {
|
|
bc, err := NewBaseClient("https://test.com", true, "", "", nil, c.capabilities, nil)
|
|
require.NoError(t, err)
|
|
|
|
var req http.Request
|
|
bc.SetClientCapabilitiesHeader(&req)
|
|
require.ElementsMatch(t, strings.Split(c.expected, ","), strings.Split(req.Header.Get(fleet.CapabilitiesHeader), ","))
|
|
})
|
|
}
|
|
}
|
|
|
|
func TestServerCapabilities(t *testing.T) {
|
|
// initial response has a single capability
|
|
response := &http.Response{
|
|
StatusCode: http.StatusOK,
|
|
Body: io.NopCloser(bytes.NewBufferString(`{}`)),
|
|
Header: http.Header{fleet.CapabilitiesHeader: []string{"test_capability"}},
|
|
}
|
|
bc, err := NewBaseClient("https://test.com", true, "", "", nil, fleet.CapabilityMap{}, nil)
|
|
require.NoError(t, err)
|
|
testCapability := fleet.Capability("test_capability")
|
|
|
|
err = bc.ParseResponse("", "", response, &struct{}{})
|
|
require.NoError(t, err)
|
|
require.True(t, bc.GetServerCapabilities().Has(testCapability))
|
|
|
|
// later on, the server is downgraded and no longer has the capability
|
|
response = &http.Response{
|
|
StatusCode: http.StatusOK,
|
|
Body: io.NopCloser(bytes.NewBufferString(`{}`)),
|
|
Header: http.Header{},
|
|
}
|
|
err = bc.ParseResponse("", "", response, &struct{}{})
|
|
require.NoError(t, err)
|
|
require.Equal(t, fleet.CapabilityMap{}, bc.ServerCapabilities)
|
|
require.False(t, bc.GetServerCapabilities().Has(testCapability))
|
|
|
|
// after an upgrade, the server has many capabilities
|
|
response = &http.Response{
|
|
StatusCode: http.StatusOK,
|
|
Body: io.NopCloser(bytes.NewBufferString(`{}`)),
|
|
Header: http.Header{fleet.CapabilitiesHeader: []string{"test_capability,test_capability_2"}},
|
|
}
|
|
err = bc.ParseResponse("", "", response, &struct{}{})
|
|
require.NoError(t, err)
|
|
require.Equal(t, fleet.CapabilityMap{
|
|
testCapability: {},
|
|
fleet.Capability("test_capability_2"): {},
|
|
}, bc.ServerCapabilities)
|
|
require.True(t, bc.GetServerCapabilities().Has(testCapability))
|
|
require.True(t, bc.GetServerCapabilities().Has(fleet.Capability("test_capability")))
|
|
}
|
|
|
|
func TestFileResponseHandlePathTraversal(t *testing.T) {
|
|
t.Run("unix path traversal is stripped to base filename", func(t *testing.T) {
|
|
destDir := t.TempDir()
|
|
fr := &FileResponse{DestPath: destDir}
|
|
resp := &http.Response{
|
|
StatusCode: http.StatusOK,
|
|
Body: io.NopCloser(strings.NewReader("content")),
|
|
Header: http.Header{
|
|
"Content-Disposition": []string{`attachment;filename="../../../etc/cron.d/backdoor"`},
|
|
},
|
|
}
|
|
|
|
err := fr.Handle(resp)
|
|
require.NoError(t, err)
|
|
require.Equal(t, "backdoor", filepath.Base(fr.DestFilePath))
|
|
require.True(t, strings.HasPrefix(fr.DestFilePath, destDir+string(filepath.Separator)))
|
|
})
|
|
|
|
t.Run("normal filename is unchanged", func(t *testing.T) {
|
|
destDir := t.TempDir()
|
|
fr := &FileResponse{DestPath: destDir}
|
|
resp := &http.Response{
|
|
StatusCode: http.StatusOK,
|
|
Body: io.NopCloser(strings.NewReader("content")),
|
|
Header: http.Header{
|
|
"Content-Disposition": []string{`attachment;filename="installer.pkg"`},
|
|
},
|
|
}
|
|
|
|
err := fr.Handle(resp)
|
|
require.NoError(t, err)
|
|
require.Equal(t, "installer.pkg", filepath.Base(fr.DestFilePath))
|
|
require.True(t, strings.HasPrefix(fr.DestFilePath, destDir+string(filepath.Separator)))
|
|
})
|
|
|
|
t.Run("dot filename falls back to DestFile", func(t *testing.T) {
|
|
destDir := t.TempDir()
|
|
fr := &FileResponse{DestPath: destDir, DestFile: "fallback.txt"}
|
|
resp := &http.Response{
|
|
StatusCode: http.StatusOK,
|
|
Body: io.NopCloser(strings.NewReader("content")),
|
|
Header: http.Header{
|
|
"Content-Disposition": []string{`attachment;filename="."`},
|
|
},
|
|
}
|
|
|
|
err := fr.Handle(resp)
|
|
require.NoError(t, err)
|
|
require.Equal(t, "fallback.txt", filepath.Base(fr.DestFilePath))
|
|
require.True(t, strings.HasPrefix(fr.DestFilePath, destDir+string(filepath.Separator)))
|
|
})
|
|
|
|
t.Run("dotdot filename falls back to UUID", func(t *testing.T) {
|
|
destDir := t.TempDir()
|
|
fr := &FileResponse{DestPath: destDir}
|
|
resp := &http.Response{
|
|
StatusCode: http.StatusOK,
|
|
Body: io.NopCloser(strings.NewReader("content")),
|
|
Header: http.Header{
|
|
"Content-Disposition": []string{`attachment;filename=".."`},
|
|
},
|
|
}
|
|
|
|
err := fr.Handle(resp)
|
|
require.NoError(t, err)
|
|
require.True(t, strings.HasPrefix(fr.DestFilePath, destDir+string(filepath.Separator)))
|
|
})
|
|
}
|
|
|
|
func TestClientCertificateAuth(t *testing.T) {
|
|
httpRequestReceived := false
|
|
|
|
clientCAs, err := certificate.LoadPEM(filepath.Join("testdata", "client-ca.crt"))
|
|
require.NoError(t, err)
|
|
|
|
ts := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
|
httpRequestReceived = true
|
|
}))
|
|
ts.TLS = &tls.Config{
|
|
MinVersion: tls.VersionTLS12,
|
|
ClientAuth: tls.RequireAndVerifyClientCert,
|
|
ClientCAs: clientCAs,
|
|
}
|
|
|
|
ts.StartTLS()
|
|
t.Cleanup(func() {
|
|
ts.Close()
|
|
})
|
|
|
|
// Try connecting without setting TLS client certificates.
|
|
bc, err := NewBaseClient(ts.URL, true, "", "", nil, fleet.CapabilityMap{}, nil)
|
|
require.NoError(t, err)
|
|
request, err := http.NewRequest("GET", ts.URL, nil)
|
|
require.NoError(t, err)
|
|
_, err = bc.HTTP.Do(request)
|
|
require.Error(t, err)
|
|
require.False(t, httpRequestReceived)
|
|
|
|
// Now try connecting by setting the correct TLS client certificates.
|
|
clientCrt, err := certificate.LoadClientCertificateFromFiles(filepath.Join("testdata", "client.crt"), filepath.Join("testdata", "client.key"))
|
|
require.NoError(t, err)
|
|
require.NotNil(t, clientCrt)
|
|
bc, err = NewBaseClient(ts.URL, true, "", "", &clientCrt.Crt, fleet.CapabilityMap{}, nil)
|
|
require.NoError(t, err)
|
|
request, err = http.NewRequest("GET", ts.URL, nil)
|
|
require.NoError(t, err)
|
|
_, err = bc.HTTP.Do(request)
|
|
require.NoError(t, err)
|
|
require.True(t, httpRequestReceived)
|
|
}
|