From f2b2e23b0ad54032d5580b59a4eefba49d79f91c Mon Sep 17 00:00:00 2001 From: Nico <32375741+nulmete@users.noreply.github.com> Date: Tue, 5 May 2026 18:18:08 +0200 Subject: [PATCH] GitOps changes for custom org's logo uploads (#44550) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit **Related issue:** Resolves #44333 # Checklist for submitter - [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. Also added some integration tests as a follow-up of the first PR (https://github.com/fleetdm/fleet/pull/44390). - [x] QA'd all new/changed functionality manually #### generate-gitops - Branched off to main, no URLs set, then ran generate-gitops on this branch. Deprecated keys gone, new keys present. nourls_new - Branched off to main, set external URLs for both light and dark modes, then ran generate-gitops on this branch. Deprecated keys gone, new keys set with the external URLs. externalurl_main externalurl_new - Within this branch, after uploading a custom logo for light mode, ran generate-gitops. The logo was saved in lib/org_logo/light.webp Screenshot 2026-05-04 at 4 06 59 PM Screenshot 2026-05-04 at 4 07 30 PM #### gitops - Applied gitops with two external URLs. Verified in the UI that those are still present Screenshot 2026-05-04 at 7 54 53 AM Screenshot 2026-05-04 at 8 01 04 AM - Applied gitops with "" as the URLs to clear them. Verified the default fleet logo is shown. Screenshot 2026-05-04 at 8 15 11 AM Screenshot 2026-05-04 at 8 15 50 AM - Applied gitops with a custom logo for light theme, using **org_logo_path_light_mode**: Screenshot 2026-05-04 at 4 10 05 PM Screenshot 2026-05-04 at 4 10 35 PM ## New Fleet configuration settings - [x] Verified that the setting is exported via `fleetctl generate-gitops` - [x] Verified the setting is documented in a separate PR to [the GitOps documentation](https://github.com/fleetdm/fleet/blob/main/docs/Configuration/yaml-files.md#L485) - See https://github.com/fleetdm/fleet/pull/43808. - [x] Verified that the setting is cleared on the server if it is not supplied in a YAML file (or that it is documented as being optional) - [x] Verified that any relevant UI is disabled when GitOps mode is enabled ## Summary by CodeRabbit * **New Features** * GitOps support for uploading custom org logos (dark/light) via local files. * `fleetctl generate-gitops` exports Fleet-hosted logos as local files and inserts path references. * New API endpoints to upload, delete, and fetch org logos. * **Deprecated** * Legacy logo keys consolidated into mode-specific URL keys (`org_logo_url_dark_mode`, `org_logo_url_light_mode`). * **Bug Fixes / Validation** * Validation/error when both a path and URL are provided for the same mode; file size and image-format checks enforced. --- changes/44333-gitops-custom-org-logo-upload | 1 + cmd/fleetctl/fleetctl/generate_gitops.go | 88 +++++- cmd/fleetctl/fleetctl/generate_gitops_test.go | 120 ++++++++ .../expectedOrgSettings-insecure.yaml | 2 - .../generateGitops/expectedOrgSettings.yaml | 2 - .../generateGitops/test_dir_free/default.yml | 2 - .../test_dir_premium/default.yml | 2 - pkg/spec/gitops.go | 36 +++ pkg/spec/gitops_deprecations.go | 4 + pkg/spec/gitops_test.go | 92 ++++++ pkg/spec/gitops_validate.go | 1 + server/fleet/app.go | 16 + server/fleet/org_logo.go | 58 ++++ server/service/client.go | 21 ++ server/service/client_appconfig.go | 235 +++++++++++++++ server/service/client_appconfig_test.go | 273 ++++++++++++++++++ server/service/integration_core_test.go | 85 ++++++ server/service/integration_enterprise_test.go | 47 +++ server/service/org_logo.go | 78 +---- server/service/org_logo_test.go | 5 +- server/service/testing_utils.go | 5 +- 21 files changed, 1088 insertions(+), 85 deletions(-) create mode 100644 changes/44333-gitops-custom-org-logo-upload create mode 100644 server/service/client_appconfig_test.go diff --git a/changes/44333-gitops-custom-org-logo-upload b/changes/44333-gitops-custom-org-logo-upload new file mode 100644 index 0000000000..ead8559efa --- /dev/null +++ b/changes/44333-gitops-custom-org-logo-upload @@ -0,0 +1 @@ +* Added gitops support for uploading custom org logos: `fleetctl gitops` accepts `org_logo_path_dark_mode`/`org_logo_path_light_mode` keys to upload local files, and `fleetctl generate-gitops` exports Fleet-hosted logos as local files alongside path keys while keeping external URLs as `org_logo_url_*_mode` keys. diff --git a/cmd/fleetctl/fleetctl/generate_gitops.go b/cmd/fleetctl/fleetctl/generate_gitops.go index 7c1533f5a9..d578467394 100644 --- a/cmd/fleetctl/fleetctl/generate_gitops.go +++ b/cmd/fleetctl/fleetctl/generate_gitops.go @@ -7,6 +7,7 @@ import ( "errors" "fmt" "math" + "mime" "os" pathUtils "path" "path/filepath" @@ -75,6 +76,7 @@ type generateGitopsClient interface { GetProfileContents(profileID string) ([]byte, error) GetEULAMetadata() (*fleet.MDMEULA, error) GetEULAContent(token string) ([]byte, error) + GetOrgLogoContent(mode fleet.OrgLogoMode) (body []byte, contentType string, err error) GetTeam(teamID uint) (*fleet.Team, error) ListSoftwareTitles(query string) ([]fleet.SoftwareTitleListResult, error) GetSoftwareTitleByID(ID uint, teamID *uint) (*fleet.SoftwareTitle, error) @@ -792,12 +794,17 @@ func (cmd *GenerateGitopsCommand) generateOrgSettings() (orgSettings map[string] return nil, err } + orgInfo, err := cmd.generateOrgInfo() + if err != nil { + return nil, err + } + orgSettings = map[string]interface{}{ jsonFieldName(t, "ActivityExpirySettings"): cmd.AppConfig.ActivityExpirySettings, jsonFieldName(t, "Features"): cmd.AppConfig.Features, jsonFieldName(t, "FleetDesktop"): cmd.AppConfig.FleetDesktop, jsonFieldName(t, "HostExpirySettings"): cmd.AppConfig.HostExpirySettings, - jsonFieldName(t, "OrgInfo"): cmd.AppConfig.OrgInfo, + jsonFieldName(t, "OrgInfo"): orgInfo, jsonFieldName(t, "ServerSettings"): cmd.AppConfig.ServerSettings, jsonFieldName(t, "WebhookSettings"): webhookSettings, } @@ -1057,6 +1064,85 @@ func (cmd *GenerateGitopsCommand) generateCertificateAuthorities(filePath string return result, nil } +// generateOrgInfo returns OrgInfo as a map so we can swap a Fleet-hosted +// logo URL for a path key plus an exported file. External URLs flow through +// unchanged; the deprecated logo URL keys are renamed to the mode-aware +// variants by yamlMarshalRenamed at write time. +func (cmd *GenerateGitopsCommand) generateOrgInfo() (map[string]any, error) { + raw, err := json.Marshal(cmd.AppConfig.OrgInfo) + if err != nil { + return nil, fmt.Errorf("marshalling org_info: %w", err) + } + orgInfo := map[string]any{} + if err := json.Unmarshal(raw, &orgInfo); err != nil { + return nil, fmt.Errorf("unmarshalling org_info: %w", err) + } + + if err := cmd.exportFleetHostedLogo(orgInfo, fleet.OrgLogoModeLight, + cmd.AppConfig.OrgInfo.OrgLogoURLLightMode, + "org_logo_path_light_mode", "org_logo_url_light_mode", "org_logo_url_light_background"); err != nil { + return nil, err + } + if err := cmd.exportFleetHostedLogo(orgInfo, fleet.OrgLogoModeDark, + cmd.AppConfig.OrgInfo.OrgLogoURLDarkMode, + "org_logo_path_dark_mode", "org_logo_url_dark_mode", "org_logo_url"); err != nil { + return nil, err + } + return orgInfo, nil +} + +// exportFleetHostedLogo, when the given URL points at the Fleet logo serving +// endpoint, downloads the logo bytes, writes them to lib/org_logo/., +// and rewrites the orgInfo map to reference the file via pathKey instead of +// the URL keys. External URLs are left untouched. +func (cmd *GenerateGitopsCommand) exportFleetHostedLogo( + orgInfo map[string]any, mode fleet.OrgLogoMode, urlValue string, + pathKey, newURLKey, deprecatedURLKey string, +) error { + if !fleet.IsFleetHostedLogoURL(urlValue) { + return nil + } + body, contentType, err := cmd.Client.GetOrgLogoContent(mode) + if err != nil { + // Server reports a Fleet-hosted URL but no blob is stored. Leave + // the URL keys in place so the user can investigate; the rest of + // the export should still succeed. + if service.IsNotFoundErr(err) { + fmt.Fprintf(cmd.CLI.App.ErrWriter, + "warning: org logo for %s mode references Fleet but no logo content was found; leaving URL as-is\n", mode) + return nil + } + return fmt.Errorf("fetching org logo (%s): %w", mode, err) + } + ext, err := orgLogoExtFromContentType(contentType) + if err != nil { + return fmt.Errorf("org logo (%s): %w", mode, err) + } + fileName := fmt.Sprintf("lib/org_logo/%s%s", mode, ext) + cmd.FilesToWrite[fileName] = string(body) + + orgInfo[pathKey] = fmt.Sprintf("./%s", fileName) + delete(orgInfo, newURLKey) + delete(orgInfo, deprecatedURLKey) + return nil +} + +func orgLogoExtFromContentType(contentType string) (string, error) { + mediaType, _, err := mime.ParseMediaType(contentType) + if err != nil { + return "", fmt.Errorf("parsing logo Content-Type %q: %w", contentType, err) + } + switch mediaType { + case "image/png": + return ".png", nil + case "image/jpeg": + return ".jpg", nil + case "image/webp": + return ".webp", nil + } + return "", fmt.Errorf("unsupported logo Content-Type %q (expected image/png, image/jpeg, or image/webp)", mediaType) +} + func (cmd *GenerateGitopsCommand) generateEULA() (string, error) { // Download the eula metadata for the token. eulaMetadata, err := cmd.Client.GetEULAMetadata() diff --git a/cmd/fleetctl/fleetctl/generate_gitops_test.go b/cmd/fleetctl/fleetctl/generate_gitops_test.go index 5f91ded3d8..a58b98676e 100644 --- a/cmd/fleetctl/fleetctl/generate_gitops_test.go +++ b/cmd/fleetctl/fleetctl/generate_gitops_test.go @@ -14,10 +14,12 @@ import ( "strings" "testing" + "github.com/fleetdm/fleet/v4/client" "github.com/fleetdm/fleet/v4/pkg/optjson" "github.com/fleetdm/fleet/v4/server/dev_mode" "github.com/fleetdm/fleet/v4/server/fleet" "github.com/fleetdm/fleet/v4/server/ptr" + "github.com/fleetdm/fleet/v4/server/service" "github.com/ghodss/yaml" "github.com/google/uuid" "github.com/stretchr/testify/assert" @@ -686,6 +688,11 @@ func (MockClient) GetEULAContent(token string) ([]byte, error) { return []byte("This is the EULA content."), nil } +func (MockClient) GetOrgLogoContent(mode fleet.OrgLogoMode) ([]byte, string, error) { + // PNG magic bytes + filler so validators that sniff content type see it as a real PNG. + return []byte{0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A, 0x00, 0x01, 0x02, 0x03}, "image/png", nil +} + func (MockClient) GetSetupExperienceSoftware(platform string, teamID uint) ([]fleet.SoftwareTitleListResult, error) { if teamID == 1 { return []fleet.SoftwareTitleListResult{ @@ -2402,3 +2409,116 @@ func TestReplaceAliasKeys(t *testing.T) { replaceAliasKeys(m, rules, true) }) } + +// orgLogoStub embeds *MockClient so the full generateGitopsClient interface is +// implemented; only GetOrgLogoContent is overridden so each test can drive the +// fetch outcome (200, 404, or generic error). +type orgLogoStub struct { + *MockClient + body []byte + contentType string + err error +} + +func (s *orgLogoStub) GetOrgLogoContent(_ fleet.OrgLogoMode) ([]byte, string, error) { + return s.body, s.contentType, s.err +} + +// newOrgLogoCommand builds a minimal GenerateGitopsCommand for exercising +// generateOrgInfo / exportFleetHostedLogo without going through the full +// generate-gitops action. +func newOrgLogoCommand(t *testing.T, client generateGitopsClient, orgInfo fleet.OrgInfo) (*GenerateGitopsCommand, *bytes.Buffer) { + t.Helper() + errBuf := new(bytes.Buffer) + return &GenerateGitopsCommand{ + Client: client, + CLI: cli.NewContext(&cli.App{ + Writer: new(bytes.Buffer), + ErrWriter: errBuf, + }, nil, nil), + AppConfig: &fleet.EnrichedAppConfig{ + AppConfig: fleet.AppConfig{OrgInfo: orgInfo}, + }, + FilesToWrite: map[string]any{}, + }, errBuf +} + +func TestGenerateGitopsExportOrgLogos(t *testing.T) { + t.Parallel() + + pngBody := []byte{0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A, 0x00, 0x01, 0x02} + + t.Run("Fleet-hosted logo is downloaded and emitted as a path key", func(t *testing.T) { + cmd, _ := newOrgLogoCommand(t, + &orgLogoStub{MockClient: &MockClient{}, body: pngBody, contentType: "image/png"}, + fleet.OrgInfo{ + OrgName: "ACME", + OrgLogoURLDarkMode: "https://fleet.example.com/api/latest/fleet/logo?mode=dark", + OrgLogoURLLightMode: "", // no light logo + }, + ) + + orgInfo, err := cmd.generateOrgInfo() + require.NoError(t, err) + + // Path key replaces the URL key for dark; light is left empty. + assert.Equal(t, "./lib/org_logo/dark.png", orgInfo["org_logo_path_dark_mode"]) + _, hasDarkURL := orgInfo["org_logo_url_dark_mode"] + assert.False(t, hasDarkURL, "Fleet-hosted dark URL should be removed in favor of the path key") + // The deprecated alias is also cleared so aliasRules can't resurrect the URL. + _, hasOldDark := orgInfo["org_logo_url"] + assert.False(t, hasOldDark) + + // Bytes were queued for the on-disk export. + assert.Equal(t, string(pngBody), cmd.FilesToWrite["lib/org_logo/dark.png"]) + }) + + t.Run("external URLs are exported unchanged (existing customer configs)", func(t *testing.T) { + stub := &orgLogoStub{ + MockClient: &MockClient{}, + err: errors.New("GetOrgLogoContent should not be called for external URLs"), + } + cmd, _ := newOrgLogoCommand(t, stub, fleet.OrgInfo{ + OrgName: "ACME", + OrgLogoURLDarkMode: "https://customer.example.com/dark.png", + OrgLogoURLLightMode: "https://customer.example.com/light.png", + }) + + orgInfo, err := cmd.generateOrgInfo() + require.NoError(t, err) + + assert.Equal(t, "https://customer.example.com/dark.png", orgInfo["org_logo_url_dark_mode"]) + assert.Equal(t, "https://customer.example.com/light.png", orgInfo["org_logo_url_light_mode"]) + _, hasDarkPath := orgInfo["org_logo_path_dark_mode"] + _, hasLightPath := orgInfo["org_logo_path_light_mode"] + assert.False(t, hasDarkPath) + assert.False(t, hasLightPath) + assert.Empty(t, cmd.FilesToWrite, "no logo files should be written for external URLs") + }) + + t.Run("404 from server prints a warning and keeps the URL", func(t *testing.T) { + stub := &orgLogoStub{ + MockClient: &MockClient{}, + err: &client.NotFoundErr{Msg: "no logo stored"}, + } + // Sanity: the stub error is recognized as a not-found. + require.True(t, service.IsNotFoundErr(stub.err)) + + cmd, errBuf := newOrgLogoCommand(t, stub, fleet.OrgInfo{ + OrgName: "ACME", + OrgLogoURLDarkMode: "https://fleet.example.com/api/latest/fleet/logo?mode=dark", + }) + + orgInfo, err := cmd.generateOrgInfo() + require.NoError(t, err, "404 must not abort the export") + + // URL is kept (the inconsistency is upstream — operator needs to investigate), + // no path key is added, no bytes are queued for export. + assert.Equal(t, "https://fleet.example.com/api/latest/fleet/logo?mode=dark", orgInfo["org_logo_url_dark_mode"]) + _, hasDarkPath := orgInfo["org_logo_path_dark_mode"] + assert.False(t, hasDarkPath) + assert.Empty(t, cmd.FilesToWrite) + assert.Contains(t, errBuf.String(), "warning") + assert.Contains(t, errBuf.String(), "dark") + }) +} diff --git a/cmd/fleetctl/fleetctl/testdata/generateGitops/expectedOrgSettings-insecure.yaml b/cmd/fleetctl/fleetctl/testdata/generateGitops/expectedOrgSettings-insecure.yaml index 1e79d5b82a..e8f94636ff 100644 --- a/cmd/fleetctl/fleetctl/testdata/generateGitops/expectedOrgSettings-insecure.yaml +++ b/cmd/fleetctl/fleetctl/testdata/generateGitops/expectedOrgSettings-insecure.yaml @@ -99,9 +99,7 @@ mdm: - "\U0001F4F1\U0001F510 Personal mobile devices" org_info: contact_url: https://fleetdm.com/company/contact - org_logo_url: http://some-org-logo-url.com org_logo_url_dark_mode: http://some-org-logo-url.com - org_logo_url_light_background: http://some-org-logo-url-light-background.com org_logo_url_light_mode: http://some-org-logo-url-light-background.com org_name: Fleet secrets: diff --git a/cmd/fleetctl/fleetctl/testdata/generateGitops/expectedOrgSettings.yaml b/cmd/fleetctl/fleetctl/testdata/generateGitops/expectedOrgSettings.yaml index 4cd4cc5e3f..23d10d299c 100644 --- a/cmd/fleetctl/fleetctl/testdata/generateGitops/expectedOrgSettings.yaml +++ b/cmd/fleetctl/fleetctl/testdata/generateGitops/expectedOrgSettings.yaml @@ -98,9 +98,7 @@ mdm: - "\U0001F4F1\U0001F510 Personal mobile devices" org_info: contact_url: https://fleetdm.com/company/contact - org_logo_url: http://some-org-logo-url.com org_logo_url_dark_mode: http://some-org-logo-url.com - org_logo_url_light_background: http://some-org-logo-url-light-background.com org_logo_url_light_mode: http://some-org-logo-url-light-background.com org_name: Fleet secrets: diff --git a/cmd/fleetctl/fleetctl/testdata/generateGitops/test_dir_free/default.yml b/cmd/fleetctl/fleetctl/testdata/generateGitops/test_dir_free/default.yml index 391662bae3..a7ba3859d8 100644 --- a/cmd/fleetctl/fleetctl/testdata/generateGitops/test_dir_free/default.yml +++ b/cmd/fleetctl/fleetctl/testdata/generateGitops/test_dir_free/default.yml @@ -137,9 +137,7 @@ org_settings: metadata_url: # TODO: Add your MDM end user auth metadata URL here org_info: contact_url: https://fleetdm.com/company/contact - org_logo_url: http://some-org-logo-url.com org_logo_url_dark_mode: http://some-org-logo-url.com - org_logo_url_light_background: http://some-org-logo-url-light-background.com org_logo_url_light_mode: http://some-org-logo-url-light-background.com org_name: Fleet secrets: diff --git a/cmd/fleetctl/fleetctl/testdata/generateGitops/test_dir_premium/default.yml b/cmd/fleetctl/fleetctl/testdata/generateGitops/test_dir_premium/default.yml index 97b4de68aa..27d9e55811 100644 --- a/cmd/fleetctl/fleetctl/testdata/generateGitops/test_dir_premium/default.yml +++ b/cmd/fleetctl/fleetctl/testdata/generateGitops/test_dir_premium/default.yml @@ -132,9 +132,7 @@ org_settings: location: Fleet Device Management Inc. org_info: contact_url: https://fleetdm.com/company/contact - org_logo_url: http://some-org-logo-url.com org_logo_url_dark_mode: http://some-org-logo-url.com - org_logo_url_light_background: http://some-org-logo-url-light-background.com org_logo_url_light_mode: http://some-org-logo-url-light-background.com org_name: Fleet secrets: diff --git a/pkg/spec/gitops.go b/pkg/spec/gitops.go index 58c6ea1e7c..e926b12b0f 100644 --- a/pkg/spec/gitops.go +++ b/pkg/spec/gitops.go @@ -325,6 +325,18 @@ type GitOpsOrgSettings struct { CertificateAuthorities any `json:"certificate_authorities"` } +// GitOpsOrgInfo extends fleet.OrgInfo with gitops-only path keys for uploading +// a custom org logo from a local file. The path keys are extracted from the +// OrgInfo before it's sent to the AppConfig PATCH endpoint, and the actual +// PUT /api/v1/fleet/logo upload runs after the PATCH succeeds (see +// Client.DoGitOps in server/service/client.go) so a PATCH failure leaves +// logo storage untouched. +type GitOpsOrgInfo struct { + fleet.OrgInfo + OrgLogoPathDarkMode string `json:"org_logo_path_dark_mode,omitempty"` + OrgLogoPathLightMode string `json:"org_logo_path_light_mode,omitempty"` +} + // GitOpsFleetSettings defines the valid keys for the top-level `settings:` section (fleet-level). // It embeds fleet.TeamConfig for all standard settings and adds gitops-only keys // that are extracted before the config is sent to the server API. @@ -624,6 +636,7 @@ func parseOrgSettings(raw json.RawMessage, result *GitOps, baseDir string, fileP multiError = multierror.Append(multiError, MaybeParseTypeError(filePath, []string{"org_settings"}, err)) } else { multiError = parseSecrets(result, multiError) + multiError = validateOrgInfoLogo(result.OrgSettings, multiError) } // Validate unknown keys in org_settings section. multiError = multierror.Append(multiError, validateYAMLKeys(raw, reflect.TypeFor[GitOpsOrgSettings](), settingsFilePath, []string{"org_settings"})...) @@ -632,6 +645,29 @@ func parseOrgSettings(raw json.RawMessage, result *GitOps, baseDir string, fileP return multiError } +// validateOrgInfoLogo rejects org_info configurations that specify both a path +// and a URL for the same mode. Deprecated URL keys are already migrated to the +// new mode-aware names by ApplyDeprecatedKeyMappings before this runs. +func validateOrgInfoLogo(orgSettings map[string]any, multiError *multierror.Error) *multierror.Error { + orgInfo, _ := orgSettings["org_info"].(map[string]any) + if orgInfo == nil { + return multiError + } + check := func(mode, pathKey, urlKey string) { + path, _ := orgInfo[pathKey].(string) + urlVal, _ := orgInfo[urlKey].(string) + if path != "" && urlVal != "" { + multiError = multierror.Append(multiError, fmt.Errorf( + "org_settings.org_info: cannot specify both '%s' and '%s' for %s mode; choose one", + pathKey, urlKey, mode, + )) + } + } + check("dark", "org_logo_path_dark_mode", "org_logo_url_dark_mode") + check("light", "org_logo_path_light_mode", "org_logo_url_light_mode") + return multiError +} + func parseTeamSettings(raw json.RawMessage, result *GitOps, baseDir string, filePath string, multiError *multierror.Error) *multierror.Error { var teamSettingsTop fleet.BaseItem if err := json.Unmarshal(raw, &teamSettingsTop); err != nil { diff --git a/pkg/spec/gitops_deprecations.go b/pkg/spec/gitops_deprecations.go index ea3261e5b0..07003c7f97 100644 --- a/pkg/spec/gitops_deprecations.go +++ b/pkg/spec/gitops_deprecations.go @@ -54,6 +54,10 @@ var DeprecatedGitOpsKeyMappings = []DeprecatedKeyMapping{ {"org_settings.server_settings.query_reports_disabled", "org_settings.server_settings.discard_reports_data"}, {"org_settings.server_settings.query_report_cap", "org_settings.server_settings.report_cap"}, + // Org settings: org_info logo URL fields renamed to mode-aware variants. + {"org_settings.org_info.org_logo_url", "org_settings.org_info.org_logo_url_dark_mode"}, + {"org_settings.org_info.org_logo_url_light_background", "org_settings.org_info.org_logo_url_light_mode"}, + // Nested keys in org_settings.mdm.apple_business_manager[] {"org_settings.mdm.apple_business_manager[].macos_team", "org_settings.mdm.apple_business_manager[].macos_fleet"}, {"org_settings.mdm.apple_business_manager[].ios_team", "org_settings.mdm.apple_business_manager[].ios_fleet"}, diff --git a/pkg/spec/gitops_test.go b/pkg/spec/gitops_test.go index 6dea822184..df5c9589ef 100644 --- a/pkg/spec/gitops_test.go +++ b/pkg/spec/gitops_test.go @@ -1030,6 +1030,98 @@ func TestGitOpsNullArrays(t *testing.T) { assert.Nil(t, gitops.Policies) } +func TestGitOpsOrgLogo(t *testing.T) { + t.Parallel() + + // New mode-aware path keys are accepted under org_info. + t.Run("path keys accepted", func(t *testing.T) { + config := getGlobalConfig([]string{"org_settings"}) + config += ` +org_settings: + server_settings: + server_url: https://fleet.example.com + org_info: + contact_url: https://example.com/contact + org_name: Test Org + org_logo_path_dark_mode: ./dark.png + org_logo_path_light_mode: ./light.png + secrets: +` + gitops, err := gitOpsFromString(t, config) + require.NoError(t, err) + orgInfo := gitops.OrgSettings["org_info"].(map[string]any) + assert.Equal(t, "./dark.png", orgInfo["org_logo_path_dark_mode"]) + assert.Equal(t, "./light.png", orgInfo["org_logo_path_light_mode"]) + }) + + // Setting both a path and a URL for the same mode is rejected at parse time. + t.Run("path and url mutually exclusive", func(t *testing.T) { + for _, mode := range []string{"dark", "light"} { + config := getGlobalConfig([]string{"org_settings"}) + config += fmt.Sprintf(` +org_settings: + server_settings: + server_url: https://fleet.example.com + org_info: + contact_url: https://example.com/contact + org_name: Test Org + org_logo_path_%[1]s_mode: ./logo.png + org_logo_url_%[1]s_mode: https://example.com/logo.png + secrets: +`, mode) + _, err := gitOpsFromString(t, config) + require.Error(t, err) + require.ErrorContains(t, err, "cannot specify both") + require.ErrorContains(t, err, mode) + } + }) + + // Deprecated org_logo_url and org_logo_url_light_background keys are migrated to the new mode-aware names. + t.Run("deprecated URL keys are renamed", func(t *testing.T) { + config := getGlobalConfig([]string{"org_settings"}) + config += ` +org_settings: + server_settings: + server_url: https://fleet.example.com + org_info: + contact_url: https://example.com/contact + org_name: Test Org + org_logo_url: https://example.com/dark-logo.png + org_logo_url_light_background: https://example.com/light-logo.png + secrets: +` + gitops, err := gitOpsFromString(t, config) + require.NoError(t, err) + orgInfo := gitops.OrgSettings["org_info"].(map[string]any) + assert.Equal(t, "https://example.com/dark-logo.png", orgInfo["org_logo_url_dark_mode"]) + assert.Equal(t, "https://example.com/light-logo.png", orgInfo["org_logo_url_light_mode"]) + _, hasOldDark := orgInfo["org_logo_url"] + _, hasOldLight := orgInfo["org_logo_url_light_background"] + assert.False(t, hasOldDark, "deprecated org_logo_url should be removed after migration") + assert.False(t, hasOldLight, "deprecated org_logo_url_light_background should be removed after migration") + }) + + // Setting both an old and new URL key for the same mode errors out + // (handled by the generic deprecated-key migrator). + t.Run("old + new URL keys conflict", func(t *testing.T) { + config := getGlobalConfig([]string{"org_settings"}) + config += ` +org_settings: + server_settings: + server_url: https://fleet.example.com + org_info: + contact_url: https://example.com/contact + org_name: Test Org + org_logo_url: https://example.com/old.png + org_logo_url_dark_mode: https://example.com/new.png + secrets: +` + _, err := gitOpsFromString(t, config) + require.Error(t, err) + assert.ErrorContains(t, err, "org_logo_url") + }) +} + func TestGitOpsPaths(t *testing.T) { t.Parallel() tests := map[string]struct { diff --git a/pkg/spec/gitops_validate.go b/pkg/spec/gitops_validate.go index 73c71d36d8..7f57d3b7e9 100644 --- a/pkg/spec/gitops_validate.go +++ b/pkg/spec/gitops_validate.go @@ -139,6 +139,7 @@ var anyFieldTypes = map[reflect.Type]map[string]reflect.Type{ reflect.TypeFor[GitOpsOrgSettings](): { "certificate_authorities": reflect.TypeFor[fleet.GroupedCertificateAuthorities](), "mdm": reflect.TypeFor[GitOpsMDM](), + "org_info": reflect.TypeFor[GitOpsOrgInfo](), }, } diff --git a/server/fleet/app.go b/server/fleet/app.go index 23363dbcc4..2d1fb84eb6 100644 --- a/server/fleet/app.go +++ b/server/fleet/app.go @@ -1287,6 +1287,22 @@ func (o *OrgInfo) AbsolutizeLogoURLs(serverURL string) { o.OrgLogoURLLightMode = AbsolutizeLogoURL(o.OrgLogoURLLightMode, serverURL) } +// IsFleetHostedLogoURL reports whether the given URL points at the Fleet logo +// serving endpoint. Handles both the persisted relative form +// ("/api/latest/fleet/logo?mode=...") and the absolutized form returned by +// AbsolutizeLogoURLs. Match must be on the parsed Path so a sibling endpoint +// like "/api/latest/fleet/logo-proxy" doesn't get falsely identified. +func IsFleetHostedLogoURL(rawURL string) bool { + if rawURL == "" { + return false + } + u, err := url.Parse(rawURL) + if err != nil { + return false + } + return u.Path == orgLogoServingPathPrefix +} + const DefaultOrgInfoContactURL = "https://fleetdm.com/company/contact" // ServerSettings contains general settings about the Fleet application. diff --git a/server/fleet/org_logo.go b/server/fleet/org_logo.go index 4cd6196d9b..ed6a223f7c 100644 --- a/server/fleet/org_logo.go +++ b/server/fleet/org_logo.go @@ -1,8 +1,14 @@ package fleet import ( + "bytes" "context" + "image" + _ "image/jpeg" + _ "image/png" "io" + + _ "golang.org/x/image/webp" ) const OrgLogoMaxFileSize = 100 * 1024 @@ -39,3 +45,55 @@ type OrgLogoStore interface { Delete(ctx context.Context, mode OrgLogoMode) error Exists(ctx context.Context, mode OrgLogoMode) (bool, error) } + +// Magic-byte signatures used to identify accepted image formats. We compare +// against raw upload bytes rather than trusting the multipart Content-Type +// header. +var ( + orgLogoPNGMagic = []byte{0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A} + orgLogoJPEGMagic = []byte{0xFF, 0xD8, 0xFF} +) + +// hasWebPMagic reports whether b begins with a WebP RIFF container header +// ("RIFF" at bytes 0-3, "WEBP" at bytes 8-11). WebP isn't a simple prefix +// check because the 4 bytes between the two markers carry the file size. +func hasWebPMagic(b []byte) bool { + return len(b) >= 12 && bytes.Equal(b[0:4], []byte("RIFF")) && bytes.Equal(b[8:12], []byte("WEBP")) +} + +// ContentTypeForOrgLogo returns the HTTP Content-Type for the accepted org +// logo formats (PNG, JPEG, WebP) based on the leading bytes, or "" for +// anything else. +func ContentTypeForOrgLogo(b []byte) string { + switch { + case bytes.HasPrefix(b, orgLogoPNGMagic): + return "image/png" + case bytes.HasPrefix(b, orgLogoJPEGMagic): + return "image/jpeg" + case hasWebPMagic(b): + return "image/webp" + } + return "" +} + +// ValidateOrgLogoBytes is the canonical org-logo validator. The HTTP upload +// handler uses it to gate uploads, and gitops apply uses it as a pre-flight +// before sending bytes to the server, so a YAML referencing an invalid +// image fails fast at apply time rather than mid-PATCH. +func ValidateOrgLogoBytes(b []byte) error { + if int64(len(b)) > OrgLogoMaxFileSize { + return &BadRequestError{Message: "logo must be 100KB or less"} + } + _, format, err := image.DecodeConfig(bytes.NewReader(b)) + if err != nil { + return &BadRequestError{ + Message: "logo must be a valid PNG, JPEG, or WebP image", + InternalErr: err, + } + } + switch format { + case "png", "jpeg", "webp": + return nil + } + return &BadRequestError{Message: "logo must be a PNG, JPEG, or WebP file"} +} diff --git a/server/service/client.go b/server/service/client.go index 9c4eec66e8..fde1f2fa6e 100644 --- a/server/service/client.go +++ b/server/service/client.go @@ -1931,6 +1931,7 @@ func (c *Client) DoGitOps( } } + var orgLogoActions []orgLogoAction if incoming.TeamName == nil { // OrgSettings is the basis of the group AppConfig, but we will be adding and removing some // items because the GitOps structure is not the same as the AppConfig structure. @@ -1959,6 +1960,19 @@ func (c *Client) DoGitOps( group.CertificateAuthorities = groupedCAs delete(incoming.OrgSettings, "certificate_authorities") + // Plan org logo upload/delete actions and strip the gitops-only path + // keys before the AppConfig PATCH. Execution runs after ApplyGroup so + // a PATCH failure leaves the logo store untouched. Skipped entirely + // when appConfig wasn't fetched (e.g. validation-only call paths in + // tests) — the planner needs the current OrgInfo to decide whether + // stale Fleet-hosted blobs should be deleted. + if appConfig != nil { + orgLogoActions, err = c.planAndStripOrgLogos(incoming.OrgSettings, &appConfig.OrgInfo, baseDir, dryRun, logFn) + if err != nil { + return nil, err + } + } + // Update labels if there were any changes. if incoming.LabelChangesSummary.HasChanges() { err := c.doGitOpsLabels(incoming, logFn, dryRun) @@ -2424,6 +2438,13 @@ func (c *Client) DoGitOps( return nil, err } + // Apply org logo uploads/deletes after the AppConfig PATCH succeeded. + if incoming.TeamName == nil { + if err := c.doGitOpsOrgLogos(orgLogoActions, dryRun, logFn); err != nil { + return nil, err + } + } + var teamSoftwareInstallers []fleet.SoftwarePackageResponse var teamVPPApps []fleet.VPPAppResponse var teamScripts []fleet.ScriptResponse diff --git a/server/service/client_appconfig.go b/server/service/client_appconfig.go index ab5627619b..941b1cf163 100644 --- a/server/service/client_appconfig.go +++ b/server/service/client_appconfig.go @@ -1,7 +1,15 @@ package service import ( + "bytes" + "context" "encoding/json" + "fmt" + "io" + "mime/multipart" + "net/http" + "os" + "path/filepath" "github.com/fleetdm/fleet/v4/server/fleet" "github.com/fleetdm/fleet/v4/server/platform/endpointer" @@ -67,3 +75,230 @@ func (c *Client) Version() (*version.Info, error) { err := c.authenticatedRequest(nil, verb, path, &responseBody) return responseBody.Info, err } + +type orgLogoAction struct { + mode fleet.OrgLogoMode + uploadPath string +} + +// planAndStripOrgLogos plans the PUT/DELETE /logo calls that run after the +// AppConfig PATCH, and removes any org_info keys that shouldn't ride along +// on that PATCH: +// +// - path keys are always removed: they're gitops-only, with no matching +// field on fleet.OrgInfo. +// - the URL key for a given mode is also removed when the YAML supplied +// a path key for that same mode. Example: if the YAML sets +// org_logo_path_dark_mode, we strip org_logo_url_dark_mode too so the +// follow-up PUT is the sole writer of OrgLogoURLDarkMode — otherwise +// PATCH would blank that field briefly before PUT corrects it. +// +// Modes the YAML doesn't mention are left alone (current server state preserved). +func (c *Client) planAndStripOrgLogos( + orgSettings map[string]any, + currentOrgInfo *fleet.OrgInfo, + baseDir string, + dryRun bool, + logFn func(format string, args ...any), +) ([]orgLogoAction, error) { + orgInfo, _ := orgSettings["org_info"].(map[string]any) + if orgInfo == nil { + return nil, nil + } + + type modeSpec struct { + mode fleet.OrgLogoMode + pathKey string + urlKey string + deprecatedURLKey string + currentURL string + } + specs := []modeSpec{ + {fleet.OrgLogoModeLight, "org_logo_path_light_mode", "org_logo_url_light_mode", "org_logo_url_light_background", currentOrgInfo.OrgLogoURLLightMode}, + {fleet.OrgLogoModeDark, "org_logo_path_dark_mode", "org_logo_url_dark_mode", "org_logo_url", currentOrgInfo.OrgLogoURLDarkMode}, + } + + var actions []orgLogoAction + for _, s := range specs { + _, pathPresent := orgInfo[s.pathKey] + _, urlPresent := orgInfo[s.urlKey] + if !pathPresent && !urlPresent { + continue + } + + yamlPath, _ := orgInfo[s.pathKey].(string) + yamlURL, _ := orgInfo[s.urlKey].(string) + if yamlPath != "" && yamlURL != "" { + return nil, fmt.Errorf( + "org_settings.org_info: cannot specify both '%s' and '%s' for %s mode", + s.pathKey, s.urlKey, s.mode, + ) + } + + switch { + case yamlPath != "": + absPath := resolveApplyRelativePath(baseDir, yamlPath) + if err := validateOrgLogoFile(absPath); err != nil { + return nil, fmt.Errorf("org logo (%s): %w", s.mode, err) + } + actions = append(actions, orgLogoAction{mode: s.mode, uploadPath: absPath}) + // Strip every URL key for this mode: PUT will set the served URL + // (and its deprecated alias) after the PATCH, so we must keep + // PATCH from writing anything to either URL field. + delete(orgInfo, s.pathKey) + delete(orgInfo, s.urlKey) + delete(orgInfo, s.deprecatedURLKey) + if dryRun { + logFn("[+] would upload org logo (%s) from %s\n", s.mode, yamlPath) + } + case yamlURL != "": + if fleet.IsFleetHostedLogoURL(s.currentURL) { + actions = append(actions, orgLogoAction{mode: s.mode}) + } + delete(orgInfo, s.pathKey) + // Mirror the new key into the deprecated alias so PATCH carries + // both with the same value. Without this, a PATCH that only sets + // the new key leaves the deprecated field unchanged on the + // server, and the post-merge NormalizeLogoFields copies the old + // value back into the new field — silently undoing a clear or + // rewrite. See server/service/appconfig.go ModifyAppConfig. + orgInfo[s.deprecatedURLKey] = yamlURL + default: + if fleet.IsFleetHostedLogoURL(s.currentURL) { + actions = append(actions, orgLogoAction{mode: s.mode}) + } + delete(orgInfo, s.pathKey) + // Same reason as above: send both keys as "" so the server + // can't restore the previous value via NormalizeLogoFields. + orgInfo[s.deprecatedURLKey] = "" + } + } + return actions, nil +} + +// doGitOpsOrgLogos executes the actions planned by planAndStripOrgLogos. Runs +// after the AppConfig PATCH so a PATCH failure leaves storage untouched. +func (c *Client) doGitOpsOrgLogos( + actions []orgLogoAction, dryRun bool, logFn func(format string, args ...any), +) error { + for _, a := range actions { + if a.uploadPath != "" { + if dryRun { + continue // already logged at planning time + } + if err := c.UploadOrgLogo(a.mode, a.uploadPath); err != nil { + return fmt.Errorf("uploading org logo (%s): %w", a.mode, err) + } + logFn("[+] applied org logo (%s) from %s\n", a.mode, a.uploadPath) + continue + } + if dryRun { + logFn("[+] would delete org logo (%s)\n", a.mode) + continue + } + if err := c.DeleteOrgLogo(a.mode); err != nil { + return fmt.Errorf("deleting org logo (%s): %w", a.mode, err) + } + logFn("[+] deleted org logo (%s)\n", a.mode) + } + return nil +} + +// validateOrgLogoFile reads the file at path and runs the canonical +// fleet.ValidateOrgLogoBytes check on its contents, so a YAML referencing +// an invalid image fails fast at gitops apply time rather than mid-PATCH. +// The LimitReader caps the read at the file-size cap so a mis-pointed +// huge file doesn't get slurped into memory before being rejected. +func validateOrgLogoFile(path string) error { + f, err := os.Open(path) + if err != nil { + return fmt.Errorf("opening logo file %q: %w", path, err) + } + defer f.Close() + body, err := io.ReadAll(io.LimitReader(f, fleet.OrgLogoMaxFileSize+1)) + if err != nil { + return fmt.Errorf("reading logo file %q: %w", path, err) + } + if err := fleet.ValidateOrgLogoBytes(body); err != nil { + return fmt.Errorf("logo file at %q: %w", path, err) + } + return nil +} + +// UploadOrgLogo uploads the file at logoPath as the org logo for the given +// mode (light or dark) via PUT /api/latest/fleet/logo. The endpoint is +// multipart/form-data with a single "logo" field. Server-side validation +// rejects files larger than 100KB or that aren't PNG/JPEG/WebP. +func (c *Client) UploadOrgLogo(mode fleet.OrgLogoMode, logoPath string) error { + verb, path := "PUT", "/api/latest/fleet/logo" + + var b bytes.Buffer + w := multipart.NewWriter(&b) + fw, err := w.CreateFormFile("logo", filepath.Base(logoPath)) + if err != nil { + return err + } + file, err := os.Open(logoPath) + if err != nil { + return err + } + defer file.Close() + if _, err := io.Copy(fw, file); err != nil { + return err + } + if err := w.Close(); err != nil { + return fmt.Errorf("closing writer: %w", err) + } + + resp, err := c.doContextWithBodyAndHeaders(context.Background(), verb, path, + fmt.Sprintf("mode=%s", mode), + b.Bytes(), + map[string]string{ + "Content-Type": w.FormDataContentType(), + "Accept": "application/json", + "Authorization": fmt.Sprintf("Bearer %s", c.token), + }) + if err != nil { + return fmt.Errorf("do multipart request: %w", err) + } + defer resp.Body.Close() + if resp.StatusCode >= 400 { + body, _ := io.ReadAll(resp.Body) + return fmt.Errorf("uploading org logo (%s): %s: %s", mode, resp.Status, string(body)) + } + return nil +} + +// DeleteOrgLogo clears the stored org logo for the given mode (light, dark, or +// all) via DELETE /api/latest/fleet/logo. The endpoint is idempotent — deleting +// an absent logo is a no-op server-side. +func (c *Client) DeleteOrgLogo(mode fleet.OrgLogoMode) error { + verb, path := "DELETE", "/api/latest/fleet/logo" + var responseBody deleteOrgLogoResponse + return c.authenticatedRequestWithQuery(nil, verb, path, &responseBody, fmt.Sprintf("mode=%s", mode)) +} + +// GetOrgLogoContent fetches the stored org logo bytes for the given mode +// (light or dark) via GET /api/latest/fleet/logo. Returns the bytes and the +// detected Content-Type. The endpoint returns 404 when no logo is stored for +// the requested mode; callers should treat that as "no logo present". +func (c *Client) GetOrgLogoContent(mode fleet.OrgLogoMode) (body []byte, contentType string, err error) { + verb, path := "GET", "/api/latest/fleet/logo" + resp, err := c.AuthenticatedDo(verb, path, fmt.Sprintf("mode=%s", mode), nil) + if err != nil { + return nil, "", fmt.Errorf("fetching org logo (%s): %w", mode, err) + } + defer resp.Body.Close() + if resp.StatusCode == http.StatusNotFound { + return nil, "", ¬FoundErr{Msg: fmt.Sprintf("no org logo stored for %s mode", mode)} + } + if resp.StatusCode >= 400 { + errBody, _ := io.ReadAll(resp.Body) + return nil, "", fmt.Errorf("fetching org logo (%s): %s: %s", mode, resp.Status, string(errBody)) + } + body, err = io.ReadAll(resp.Body) + if err != nil { + return nil, "", fmt.Errorf("reading org logo (%s) body: %w", mode, err) + } + return body, resp.Header.Get("Content-Type"), nil +} diff --git a/server/service/client_appconfig_test.go b/server/service/client_appconfig_test.go new file mode 100644 index 0000000000..d04c2614db --- /dev/null +++ b/server/service/client_appconfig_test.go @@ -0,0 +1,273 @@ +package service + +import ( + "bytes" + "fmt" + "image" + "image/color" + "image/jpeg" + "image/png" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/fleetdm/fleet/v4/server/fleet" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func makePNG(t *testing.T) []byte { + t.Helper() + img := image.NewRGBA(image.Rect(0, 0, 1, 1)) + img.Set(0, 0, color.RGBA{R: 0, G: 128, B: 0, A: 255}) + var buf bytes.Buffer + require.NoError(t, png.Encode(&buf, img)) + return buf.Bytes() +} + +func makeJPEG(t *testing.T) []byte { + t.Helper() + img := image.NewRGBA(image.Rect(0, 0, 1, 1)) + img.Set(0, 0, color.RGBA{R: 0, G: 128, B: 0, A: 255}) + var buf bytes.Buffer + require.NoError(t, jpeg.Encode(&buf, img, nil)) + return buf.Bytes() +} + +func writeTempFile(t *testing.T, name string, body []byte) string { + t.Helper() + path := filepath.Join(t.TempDir(), name) + require.NoError(t, os.WriteFile(path, body, 0o600)) + return path +} + +func TestValidateOrgLogoFile(t *testing.T) { + t.Parallel() + + t.Run("accepts png", func(t *testing.T) { + assert.NoError(t, validateOrgLogoFile(writeTempFile(t, "logo.png", makePNG(t)))) + }) + t.Run("accepts jpeg", func(t *testing.T) { + assert.NoError(t, validateOrgLogoFile(writeTempFile(t, "logo.jpg", makeJPEG(t)))) + }) + t.Run("rejects unknown format", func(t *testing.T) { + err := validateOrgLogoFile(writeTempFile(t, "logo.txt", []byte("not an image"))) + require.Error(t, err) + assert.ErrorContains(t, err, "PNG, JPEG, or WebP") + }) + t.Run("rejects oversized file", func(t *testing.T) { + // fleet.ValidateOrgLogoBytes fires its size check before + // image.DecodeConfig, so the body content doesn't need to + // decode as a real image. + body := make([]byte, orgLogoMaxFileSize+1) + err := validateOrgLogoFile(writeTempFile(t, "big.png", body)) + require.Error(t, err) + assert.ErrorContains(t, err, "100KB or less") + }) + t.Run("missing file", func(t *testing.T) { + err := validateOrgLogoFile(filepath.Join(t.TempDir(), "absent.png")) + require.Error(t, err) + }) +} + +func TestPlanAndStripOrgLogos(t *testing.T) { + t.Parallel() + + c := &Client{} + logFn := func(string, ...any) {} + dir := t.TempDir() + pngPath := filepath.Join(dir, "logo.png") + require.NoError(t, os.WriteFile(pngPath, makePNG(t), 0o600)) + + orgSettings := func(orgInfo map[string]any) map[string]any { + return map[string]any{"org_info": orgInfo} + } + + t.Run("path key plans upload and strips every URL key for the mode", func(t *testing.T) { + os := orgSettings(map[string]any{ + "org_logo_path_dark_mode": "logo.png", + "org_logo_url_dark_mode": "", + }) + actions, err := c.planAndStripOrgLogos(os, &fleet.OrgInfo{}, dir, false, logFn) + require.NoError(t, err) + require.Len(t, actions, 1) + assert.Equal(t, fleet.OrgLogoModeDark, actions[0].mode) + assert.NotEmpty(t, actions[0].uploadPath) + + orgInfo := os["org_info"].(map[string]any) + for _, k := range []string{"org_logo_path_dark_mode", "org_logo_url_dark_mode", "org_logo_url"} { + _, present := orgInfo[k] + assert.False(t, present, "%s should be stripped (PUT controls the stored URLs)", k) + } + }) + + t.Run("external URL with current Fleet-hosted blob plans delete and mirrors deprecated alias", func(t *testing.T) { + os := orgSettings(map[string]any{ + "org_logo_url_dark_mode": "https://example.com/logo.png", + }) + actions, err := c.planAndStripOrgLogos(os, &fleet.OrgInfo{ + OrgLogoURLDarkMode: "https://fleet.example.com/api/latest/fleet/logo?mode=dark", + }, dir, false, logFn) + require.NoError(t, err) + require.Len(t, actions, 1) + assert.Equal(t, fleet.OrgLogoModeDark, actions[0].mode) + assert.Empty(t, actions[0].uploadPath, "empty uploadPath signals delete") + + // URL key kept so PATCH writes the external URL, and the deprecated + // alias is mirrored so server-side NormalizeLogoFields can't undo it. + orgInfo := os["org_info"].(map[string]any) + assert.Equal(t, "https://example.com/logo.png", orgInfo["org_logo_url_dark_mode"]) + assert.Equal(t, "https://example.com/logo.png", orgInfo["org_logo_url"]) + }) + + t.Run("explicit empty URL with Fleet-hosted blob plans delete and mirrors deprecated alias as empty", func(t *testing.T) { + os := orgSettings(map[string]any{ + "org_logo_url_light_mode": "", + }) + actions, err := c.planAndStripOrgLogos(os, &fleet.OrgInfo{ + OrgLogoURLLightMode: "/api/latest/fleet/logo?mode=light", + }, dir, false, logFn) + require.NoError(t, err) + require.Len(t, actions, 1) + assert.Equal(t, fleet.OrgLogoModeLight, actions[0].mode) + assert.Empty(t, actions[0].uploadPath) + + // Both new and deprecated keys must be sent as "" — otherwise the + // server preserves the deprecated field on merge and copies it back + // into the new one in NormalizeLogoFields. + orgInfo := os["org_info"].(map[string]any) + assert.Empty(t, orgInfo["org_logo_url_light_mode"]) + assert.Empty(t, orgInfo["org_logo_url_light_background"]) + }) + + t.Run("clearing new URL keeps the deprecated alias in sync", func(t *testing.T) { + os := orgSettings(map[string]any{ + "org_logo_url_dark_mode": "", + "org_logo_url_light_mode": "", + }) + actions, err := c.planAndStripOrgLogos(os, &fleet.OrgInfo{ + OrgLogoURLDarkMode: "https://customer.example.com/dark.png", + OrgLogoURL: "https://customer.example.com/dark.png", + OrgLogoURLLightMode: "https://customer.example.com/light.png", + OrgLogoURLLightBackground: "https://customer.example.com/light.png", + }, dir, false, logFn) + require.NoError(t, err) + // Current URLs aren't Fleet-hosted, so no DELETE actions queued. + assert.Empty(t, actions) + + orgInfo := os["org_info"].(map[string]any) + assert.Empty(t, orgInfo["org_logo_url_dark_mode"]) + assert.Empty(t, orgInfo["org_logo_url"], "deprecated dark alias must be sent as \"\"") + assert.Empty(t, orgInfo["org_logo_url_light_mode"]) + assert.Empty(t, orgInfo["org_logo_url_light_background"], "deprecated light alias must be sent as \"\"") + }) + + t.Run("missing keys preserve current state", func(t *testing.T) { + os := orgSettings(map[string]any{"org_name": "ACME"}) + actions, err := c.planAndStripOrgLogos(os, &fleet.OrgInfo{ + OrgLogoURLDarkMode: "/api/latest/fleet/logo?mode=dark", + }, dir, false, logFn) + require.NoError(t, err) + assert.Empty(t, actions, "absent keys must not trigger any action") + }) + + t.Run("both path and url for same mode rejected", func(t *testing.T) { + os := orgSettings(map[string]any{ + "org_logo_path_dark_mode": "logo.png", + "org_logo_url_dark_mode": "https://example.com/logo.png", + }) + _, err := c.planAndStripOrgLogos(os, &fleet.OrgInfo{}, dir, false, logFn) + require.Error(t, err) + assert.ErrorContains(t, err, "cannot specify both") + }) + + t.Run("missing org_info is no-op", func(t *testing.T) { + actions, err := c.planAndStripOrgLogos(map[string]any{}, &fleet.OrgInfo{}, dir, false, logFn) + require.NoError(t, err) + assert.Empty(t, actions) + }) + + t.Run("both modes set are processed independently", func(t *testing.T) { + os := orgSettings(map[string]any{ + "org_logo_path_dark_mode": "logo.png", + "org_logo_url_light_mode": "https://example.com/light.png", + }) + actions, err := c.planAndStripOrgLogos(os, &fleet.OrgInfo{ + OrgLogoURLLightMode: "/api/latest/fleet/logo?mode=light", // current is Fleet-hosted + }, dir, false, logFn) + require.NoError(t, err) + require.Len(t, actions, 2) + + byMode := map[fleet.OrgLogoMode]orgLogoAction{} + for _, a := range actions { + byMode[a.mode] = a + } + // Dark: path → upload action. + darkAct, ok := byMode[fleet.OrgLogoModeDark] + require.True(t, ok) + assert.NotEmpty(t, darkAct.uploadPath, "dark mode should plan an upload") + // Light: external URL replacing a Fleet-hosted blob → delete action. + lightAct, ok := byMode[fleet.OrgLogoModeLight] + require.True(t, ok) + assert.Empty(t, lightAct.uploadPath, "light mode should plan a delete") + + orgInfo := os["org_info"].(map[string]any) + // Dark: every URL key for the mode is stripped (PUT will set them). + for _, k := range []string{"org_logo_path_dark_mode", "org_logo_url_dark_mode", "org_logo_url"} { + _, present := orgInfo[k] + assert.False(t, present, "%s should be stripped", k) + } + // Light: URL key kept so PATCH writes the external URL, and the + // deprecated alias is mirrored to keep the server's + // NormalizeLogoFields a no-op. + assert.Equal(t, "https://example.com/light.png", orgInfo["org_logo_url_light_mode"]) + assert.Equal(t, "https://example.com/light.png", orgInfo["org_logo_url_light_background"]) + }) + + t.Run("missing path file surfaces a validation error", func(t *testing.T) { + os := orgSettings(map[string]any{ + "org_logo_path_dark_mode": "does-not-exist.png", + }) + _, err := c.planAndStripOrgLogos(os, &fleet.OrgInfo{}, dir, false, logFn) + require.Error(t, err) + require.ErrorContains(t, err, "dark") + require.ErrorContains(t, err, "does-not-exist.png") + }) + + t.Run("invalid file format surfaces a validation error", func(t *testing.T) { + badPath := filepath.Join(dir, "bad.png") + require.NoError(t, os.WriteFile(badPath, []byte("not an image"), 0o600)) + settings := orgSettings(map[string]any{ + "org_logo_path_dark_mode": "bad.png", + }) + _, err := c.planAndStripOrgLogos(settings, &fleet.OrgInfo{}, dir, false, logFn) + require.Error(t, err) + assert.ErrorContains(t, err, "PNG, JPEG, or WebP") + }) + + t.Run("dry run still validates and logs would-upload", func(t *testing.T) { + var logs []string + captureLog := func(format string, args ...any) { + logs = append(logs, fmt.Sprintf(format, args...)) + } + + // Bad file should error in dry-run. + osBad := orgSettings(map[string]any{ + "org_logo_path_dark_mode": "does-not-exist.png", + }) + _, err := c.planAndStripOrgLogos(osBad, &fleet.OrgInfo{}, dir, true, captureLog) + require.Error(t, err) + + // Valid file should plan an upload and log the would-upload line. + osGood := orgSettings(map[string]any{ + "org_logo_path_dark_mode": "logo.png", + }) + actions, err := c.planAndStripOrgLogos(osGood, &fleet.OrgInfo{}, dir, true, captureLog) + require.NoError(t, err) + require.Len(t, actions, 1) + require.NotEmpty(t, logs) + joined := strings.Join(logs, "\n") + assert.Contains(t, joined, "would upload org logo (dark)") + }) +} diff --git a/server/service/integration_core_test.go b/server/service/integration_core_test.go index c682e9eda2..bbeedd70be 100644 --- a/server/service/integration_core_test.go +++ b/server/service/integration_core_test.go @@ -10,8 +10,12 @@ import ( "encoding/json" "errors" "fmt" + "image" + "image/color" + "image/png" "io" "log/slog" + "mime/multipart" "net/http" "net/http/httptest" "net/url" @@ -17172,3 +17176,84 @@ func (s *integrationTestSuite) TestLabelScopePremiumGate() { s.DoJSON("POST", "/api/latest/fleet/queries", payload, http.StatusPaymentRequired, &fleet.CreateQueryResponse{}) } } + +func (s *integrationTestSuite) TestOrgLogoUpload() { + t := s.T() + + pngImg := image.NewRGBA(image.Rect(0, 0, 1, 1)) + pngImg.Set(0, 0, color.RGBA{R: 0, G: 128, B: 0, A: 255}) + var pngBuf bytes.Buffer + require.NoError(t, png.Encode(&pngBuf, pngImg)) + pngBytes := pngBuf.Bytes() + + buildLogoBody := func(filename string, content []byte) ([]byte, map[string]string) { + var body bytes.Buffer + w := multipart.NewWriter(&body) + fw, err := w.CreateFormFile("logo", filename) + require.NoError(t, err) + _, err = io.Copy(fw, bytes.NewReader(content)) + require.NoError(t, err) + require.NoError(t, w.Close()) + return body.Bytes(), map[string]string{ + "Content-Type": w.FormDataContentType(), + "Accept": "application/json", + "Authorization": "Bearer " + s.token, + } + } + + // 1. Upload as admin: 200, AppConfig URL set to the Fleet-hosted serving + // path, GET returns the bytes back with the right content type. + body, headers := buildLogoBody("logo.png", pngBytes) + s.DoRawWithHeaders("PUT", "/api/v1/fleet/logo?mode=light", body, http.StatusOK, headers) + + var acResp appConfigResponse + s.DoJSON("GET", "/api/v1/fleet/config", nil, http.StatusOK, &acResp) + require.Contains(t, acResp.OrgInfo.OrgLogoURLLightMode, "/api/latest/fleet/logo") + require.Contains(t, acResp.OrgInfo.OrgLogoURLLightMode, "mode=light") + // Deprecated key is in sync. + require.Equal(t, acResp.OrgInfo.OrgLogoURLLightMode, acResp.OrgInfo.OrgLogoURLLightBackground) + + res := s.DoRawNoAuth("GET", "/api/latest/fleet/logo?mode=light", nil, http.StatusOK) + gotBody, err := io.ReadAll(res.Body) + require.NoError(t, res.Body.Close()) + require.NoError(t, err) + require.Equal(t, pngBytes, gotBody) + require.Equal(t, "image/png", res.Header.Get("Content-Type")) + + // 2. Upload a second mode (dark) as admin so the delete-lifecycle assertions + // at the bottom can confirm modes are independent. + body, headers = buildLogoBody("dark.png", pngBytes) + s.DoRawWithHeaders("PUT", "/api/v1/fleet/logo?mode=dark", body, http.StatusOK, headers) + + // 3. Auth: a maintainer is rejected. + maintainerEmail := "maintainer-logo@example.com" + maintainerUser := &fleet.User{ + Name: "Maintainer Logo", + Email: maintainerEmail, + GlobalRole: ptr.String(fleet.RoleMaintainer), + } + require.NoError(t, maintainerUser.SetPassword(test.GoodPassword, 10, 10)) + _, err = s.ds.NewUser(t.Context(), maintainerUser) + require.NoError(t, err) + + s.token = s.getCachedUserToken(maintainerEmail, test.GoodPassword) + body, headers = buildLogoBody("nope.png", pngBytes) + s.DoRawWithHeaders("PUT", "/api/v1/fleet/logo?mode=light", body, http.StatusForbidden, headers) + s.token = s.getTestAdminToken() + + // 4. A non-image payload is rejected at upload time. + body, headers = buildLogoBody("not-an-image.png", []byte("plain text, definitely not a PNG")) + s.DoRawWithHeaders("PUT", "/api/v1/fleet/logo?mode=light", body, http.StatusBadRequest, headers) + + // 5. DELETE clears the URL field and the GET endpoint returns 404 for + // the affected mode while the other mode is unaffected. + s.Do("DELETE", "/api/v1/fleet/logo", nil, http.StatusOK, "mode", "light") + + s.DoJSON("GET", "/api/v1/fleet/config", nil, http.StatusOK, &acResp) + require.Empty(t, acResp.OrgInfo.OrgLogoURLLightMode) + require.Empty(t, acResp.OrgInfo.OrgLogoURLLightBackground) + require.Contains(t, acResp.OrgInfo.OrgLogoURLDarkMode, "/api/latest/fleet/logo") + + res = s.DoRawNoAuth("GET", "/api/latest/fleet/logo?mode=light", nil, http.StatusNotFound) + require.NoError(t, res.Body.Close()) +} diff --git a/server/service/integration_enterprise_test.go b/server/service/integration_enterprise_test.go index 14c331d8b4..357def65b1 100644 --- a/server/service/integration_enterprise_test.go +++ b/server/service/integration_enterprise_test.go @@ -15,6 +15,9 @@ import ( "encoding/pem" "errors" "fmt" + "image" + "image/color" + "image/png" "io" "log/slog" "math/big" @@ -29701,3 +29704,47 @@ func (s *integrationEnterpriseTestSuite) TestQueryLabelsIncludeAll() { require.False(t, hasQueryFor(hostA.ID), "host with one of two required labels should not match include_all query") require.True(t, hasQueryFor(hostBoth.ID), "host with both required labels should match include_all query") } + +func (s *integrationEnterpriseTestSuite) TestOrgLogoUploadGitOpsAuth() { + t := s.T() + + pngImg := image.NewRGBA(image.Rect(0, 0, 1, 1)) + pngImg.Set(0, 0, color.RGBA{R: 0, G: 128, B: 0, A: 255}) + var pngBuf bytes.Buffer + require.NoError(t, png.Encode(&pngBuf, pngImg)) + pngBytes := pngBuf.Bytes() + + gitopsEmail := "gitops-logo-enterprise@example.com" + gitopsUser := &fleet.User{ + Name: "GitOps Logo", + Email: gitopsEmail, + GlobalRole: ptr.String(fleet.RoleGitOps), + } + require.NoError(t, gitopsUser.SetPassword(test.GoodPassword, 10, 10)) + _, err := s.ds.NewUser(t.Context(), gitopsUser) + require.NoError(t, err) + + s.token = s.getCachedUserToken(gitopsEmail, test.GoodPassword) + defer func() { s.token = s.getTestAdminToken() }() + + var body bytes.Buffer + w := multipart.NewWriter(&body) + fw, err := w.CreateFormFile("logo", "dark.png") + require.NoError(t, err) + _, err = io.Copy(fw, bytes.NewReader(pngBytes)) + require.NoError(t, err) + require.NoError(t, w.Close()) + + s.DoRawWithHeaders("PUT", "/api/v1/fleet/logo?mode=dark", body.Bytes(), http.StatusOK, map[string]string{ + "Content-Type": w.FormDataContentType(), + "Accept": "application/json", + "Authorization": "Bearer " + s.token, + }) + + var acResp appConfigResponse + s.DoJSON("GET", "/api/v1/fleet/config", nil, http.StatusOK, &acResp) + require.Contains(t, acResp.OrgInfo.OrgLogoURLDarkMode, "/api/latest/fleet/logo") + + s.token = s.getTestAdminToken() + s.Do("DELETE", "/api/v1/fleet/logo", nil, http.StatusOK, "mode", "dark") +} diff --git a/server/service/org_logo.go b/server/service/org_logo.go index cd1e8e50cc..40164bcec4 100644 --- a/server/service/org_logo.go +++ b/server/service/org_logo.go @@ -5,55 +5,20 @@ import ( "context" "errors" "fmt" - "image" - _ "image/jpeg" - _ "image/png" "io" "net/http" "time" - "github.com/fleetdm/fleet/v4/server/authz" "github.com/fleetdm/fleet/v4/server/contexts/ctxdb" "github.com/fleetdm/fleet/v4/server/contexts/ctxerr" "github.com/fleetdm/fleet/v4/server/contexts/viewer" "github.com/fleetdm/fleet/v4/server/fleet" platform_http "github.com/fleetdm/fleet/v4/server/platform/http" "github.com/gorilla/mux" - _ "golang.org/x/image/webp" ) const orgLogoMaxFileSize = fleet.OrgLogoMaxFileSize -// Magic-byte signatures used to identify accepted image formats. We compare -// against raw upload bytes rather than trusting the multipart Content-Type -// header. -var ( - pngMagic = []byte{0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A} - jpegMagic = []byte{0xFF, 0xD8, 0xFF} -) - -// hasWebPMagic reports whether b begins with a WebP RIFF container header -// ("RIFF" at bytes 0-3, "WEBP" at bytes 8-11). WebP isn't a simple prefix -// check because the 4 bytes between the two markers carry the file size. -func hasWebPMagic(b []byte) bool { - return len(b) >= 12 && bytes.Equal(b[0:4], []byte("RIFF")) && bytes.Equal(b[8:12], []byte("WEBP")) -} - -// contentTypeForBytes returns the HTTP Content-Type for the accepted -// formats (PNG, JPEG, WebP) and "" for anything else. Used by the GET -// HijackRender to set the correct response header. -func contentTypeForBytes(b []byte) string { - switch { - case bytes.HasPrefix(b, pngMagic): - return "image/png" - case bytes.HasPrefix(b, jpegMagic): - return "image/jpeg" - case hasWebPMagic(b): - return "image/webp" - } - return "" -} - // PUT /api/v1/fleet/logo type putOrgLogoRequest struct { @@ -91,9 +56,6 @@ func (putOrgLogoRequest) DecodeRequest(_ context.Context, r *http.Request) (any, if err != nil { return nil, &fleet.BadRequestError{Message: "failed to read uploaded logo", InternalErr: err} } - if err := validateOrgLogoBytes(body); err != nil { - return nil, err - } return putOrgLogoRequest{Mode: mode, Body: body}, nil } @@ -150,7 +112,7 @@ func (r getOrgLogoResponse) HijackRender(_ context.Context, w http.ResponseWrite if r.Err != nil { return } - contentType := contentTypeForBytes(r.Body) + contentType := fleet.ContentTypeForOrgLogo(r.Body) if contentType == "" { contentType = "application/octet-stream" } @@ -205,33 +167,12 @@ func parseLogoModeQuery(raw string) (fleet.OrgLogoMode, error) { return m, nil } -func validateOrgLogoBytes(b []byte) error { - if int64(len(b)) > orgLogoMaxFileSize { - return &fleet.BadRequestError{Message: "logo must be 100KB or less"} - } - _, format, err := image.DecodeConfig(bytes.NewReader(b)) - if err != nil { - return &fleet.BadRequestError{ - Message: "logo must be a valid PNG, JPEG, or WebP image", - InternalErr: err, - } - } - switch format { - case "png", "jpeg", "webp": - return nil - } - return &fleet.BadRequestError{Message: "logo must be a PNG, JPEG, or WebP file"} -} - // Service implementation func (svc *Service) UploadOrgLogo(ctx context.Context, mode fleet.OrgLogoMode, content io.ReadSeeker) error { if err := svc.authz.Authorize(ctx, &fleet.AppConfig{}, fleet.ActionWrite); err != nil { return err } - if err := requireGlobalAdmin(ctx); err != nil { - return err - } if !mode.IsValid() { return &fleet.BadRequestError{Message: fmt.Sprintf("invalid mode %q", mode)} } @@ -245,8 +186,8 @@ func (svc *Service) UploadOrgLogo(ctx context.Context, mode fleet.OrgLogoMode, c if err != nil { return ctxerr.Wrap(ctx, err, "buffering logo content") } - if int64(len(body)) > orgLogoMaxFileSize { - return &fleet.BadRequestError{Message: "logo must be 100KB or less"} + if err := fleet.ValidateOrgLogoBytes(body); err != nil { + return err } modes := mode.Modes() @@ -282,9 +223,6 @@ func (svc *Service) DeleteOrgLogo(ctx context.Context, mode fleet.OrgLogoMode) e if err := svc.authz.Authorize(ctx, &fleet.AppConfig{}, fleet.ActionWrite); err != nil { return err } - if err := requireGlobalAdmin(ctx); err != nil { - return err - } if !mode.IsValid() { return &fleet.BadRequestError{Message: fmt.Sprintf("invalid mode %q", mode)} } @@ -363,20 +301,12 @@ func (svc *Service) GetOrgLogo(ctx context.Context, mode fleet.OrgLogoMode) ([]b if int64(len(body)) > orgLogoMaxFileSize { return nil, 0, ctxerr.New(ctx, "stored org logo exceeds max size") } - if contentTypeForBytes(body) == "" { + if fleet.ContentTypeForOrgLogo(body) == "" { return nil, 0, ctxerr.New(ctx, "stored org logo is not a recognized image format") } return body, int64(len(body)), nil } -func requireGlobalAdmin(ctx context.Context) error { - vc, ok := viewer.FromContext(ctx) - if !ok || vc.User == nil || vc.User.GlobalRole == nil || *vc.User.GlobalRole != fleet.RoleAdmin { - return authz.ForbiddenWithInternal("org logo write requires global admin", nil, nil, nil) - } - return nil -} - // orgLogoServingURL builds the URL persisted in AppConfig after an upload. The `v` param is a cache-buster (ignored server-side; only `mode` is read). func orgLogoServingURL(mode fleet.OrgLogoMode) string { return fmt.Sprintf("/api/latest/fleet/logo?mode=%s&v=%d", mode, time.Now().UnixNano()) diff --git a/server/service/org_logo_test.go b/server/service/org_logo_test.go index a5f2d1458a..b1e1ce12f7 100644 --- a/server/service/org_logo_test.go +++ b/server/service/org_logo_test.go @@ -50,9 +50,12 @@ func TestOrgLogoAuth(t *testing.T) { true, }, { + // Global gitops can write app_config (per the rego policy), + // which is what fleetctl gitops uses to upload custom org + // logos via the new org_logo_path_*_mode keys. "global gitops", &fleet.User{GlobalRole: ptr.String(fleet.RoleGitOps)}, - true, + false, }, { "team admin", diff --git a/server/service/testing_utils.go b/server/service/testing_utils.go index 342b33b326..d522e6a829 100644 --- a/server/service/testing_utils.go +++ b/server/service/testing_utils.go @@ -228,6 +228,9 @@ func newTestServiceWithConfig(t *testing.T, ds fleet.Datastore, fleetConfig conf require.NoError(t, err) } + orgLogoStore, err := filesystem.NewOrgLogoStore(t.TempDir()) + require.NoError(t, err) + svc, err := NewService( ctx, ds, @@ -254,7 +257,7 @@ func newTestServiceWithConfig(t *testing.T, ds fleet.Datastore, fleetConfig conf conditionalAccessMicrosoftProxy, keyValueStore, androidService, - nil, // orgLogoStore + orgLogoStore, ) if err != nil { panic(err)