From d94e38076b9d332e7c05488fa5c6d70a4e993b15 Mon Sep 17 00:00:00 2001 From: Rajendra kadam Date: Mon, 25 May 2026 21:27:24 +0530 Subject: [PATCH] Extract Apple APNs/SCEP pair validation onto MDMConfig (#46166) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extracts the Apple APNs/SCEP both-or-neither check out of `runServeCmd` and puts it on `MDMConfig` as `ValidateAppleAPNSAndSCEPPair(initFatal)`. Same pattern as `ConditionalAccessConfig.Validate`, `AndroidAgentConfig.Validate`, and the validators added in #45583. The call site (inside the existing `if len(toInsert) > 0` gate) goes from six lines of inline conditional `initFatal` calls to one method call. Behavior, error messages, and gating are unchanged. Tests live in `server/config/config_test.go`: one smoke case plus two error branches (APNs-only and SCEP-only). Skipped the "neither set" case on purpose — the outer `if config.MDM.IsAppleAPNsSet() || config.MDM.IsAppleSCEPSet()` gate in `runServeCmd` guarantees at least one is set before the validator is ever reached. This is the last pure config validation left in `runServeCmd` per the broader-plan note on #45583. Remaining `initFatal` sites are runtime failure paths (datastore init, Redis init, MDM init wiring) which need the injection from #45343 — those would be the next slice. **Related issue:** Refs #33370 # Checklist for submitter - [x] Added/updated automated tests - [x] Input validation (validator method plus tests; no SQL/JS/shell paths involved) - Changes file: not applicable, internal refactor with no user-visible behavior change ## Summary by CodeRabbit * **Bug Fixes** * Improved Apple MDM configuration validation to ensure APNs and SCEP certificates are properly paired during setup. [![Review Change Stack](https://storage.googleapis.com/coderabbit_public_assets/review-stack-in-coderabbit-ui.svg)](https://app.coderabbit.ai/change-stack/fleetdm/fleet/pull/46166?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) --- cmd/fleet/serve.go | 8 +------- server/config/config.go | 14 ++++++++++++++ server/config/config_test.go | 25 +++++++++++++++++++++++++ 3 files changed, 40 insertions(+), 7 deletions(-) diff --git a/cmd/fleet/serve.go b/cmd/fleet/serve.go index 78279167ea..ad85bf7ad3 100644 --- a/cmd/fleet/serve.go +++ b/cmd/fleet/serve.go @@ -711,13 +711,7 @@ func runServeCmd(cmd *cobra.Command, configManager configpkg.Manager, debug, dev } if len(toInsert) > 0 { - if !config.MDM.IsAppleAPNsSet() { - initFatal(errors.New("Apple APNs MDM configuration must be provided when Apple SCEP is provided"), - "validate Apple MDM") - } else if !config.MDM.IsAppleSCEPSet() { - initFatal(errors.New("Apple SCEP MDM configuration must be provided when Apple APNs is provided"), - "validate Apple MDM") - } + config.MDM.ValidateAppleAPNSAndSCEPPair(initFatal) // parse the APNs and SCEP assets from the config _, apnsCertPEM, apnsKeyPEM, err := config.MDM.AppleAPNs() diff --git a/server/config/config.go b/server/config/config.go index 2ece59edf7..b5ece1c320 100644 --- a/server/config/config.go +++ b/server/config/config.go @@ -1045,6 +1045,20 @@ func (m *MDMConfig) IsAppleBMSet() bool { return pair.IsSet() || m.AppleBMServerToken != "" || m.AppleBMServerTokenBytes != "" } +// ValidateAppleAPNSAndSCEPPair enforces that Apple APNs and SCEP are +// configured together — neither half of the pair is usable on its own. +// Callers should gate this on a precondition that at least one side is set +// (the outer Apple-MDM init flow handles that today). +func (m *MDMConfig) ValidateAppleAPNSAndSCEPPair(initFatal func(err error, msg string)) { + if !m.IsAppleAPNsSet() { + initFatal(errors.New("Apple APNs MDM configuration must be provided when Apple SCEP is provided"), + "validate Apple MDM") + } else if !m.IsAppleSCEPSet() { + initFatal(errors.New("Apple SCEP MDM configuration must be provided when Apple APNs is provided"), + "validate Apple MDM") + } +} + // AppleAPNs returns the parsed TLS certificate for Apple APNs. func (m *MDMConfig) AppleAPNs() (cert *tls.Certificate, pemCert, pemKey []byte, err error) { if m.appleAPNs == nil { diff --git a/server/config/config_test.go b/server/config/config_test.go index d8036824a8..15115e29d7 100644 --- a/server/config/config_test.go +++ b/server/config/config_test.go @@ -1041,3 +1041,28 @@ func TestServerConfigURLPrefix(t *testing.T) { require.True(t, called) }) } + +func TestMDMConfigValidateAppleAPNSAndSCEPPair(t *testing.T) { + t.Parallel() + + t.Run("both APNs and SCEP set is valid", func(t *testing.T) { + cfg := MDMConfig{AppleAPNsCert: "apns.cert", AppleSCEPCert: "scep.cert"} + cfg.ValidateAppleAPNSAndSCEPPair(func(err error, msg string) { + t.Fatalf("unexpected error: %v", err) + }) + }) + + t.Run("SCEP set without APNs is rejected", func(t *testing.T) { + cfg := MDMConfig{AppleSCEPCert: "scep.cert"} + called := false + cfg.ValidateAppleAPNSAndSCEPPair(func(err error, msg string) { called = true }) + require.True(t, called) + }) + + t.Run("APNs set without SCEP is rejected", func(t *testing.T) { + cfg := MDMConfig{AppleAPNsCert: "apns.cert"} + called := false + cfg.ValidateAppleAPNSAndSCEPPair(func(err error, msg string) { called = true }) + require.True(t, called) + }) +}