mirror of
https://github.com/netbirdio/netbird.git
synced 2026-10-04 12:39:06 +02:00
[client] Clear MDM upload URL when the policy empties; cut complexity
Two review follow-ups on the MDM debug-bundle upload override: - applyMDMPolicy returned early on an empty policy before reaching the clear-on-absent path, so a policy that dropped every key left a stale upload target directing bundles on a reused Config. Resolve the override unconditionally, up front, so an absent, empty, or invalid value fails closed to "" (falling back to the management-supplied or default target). - Extract the resolution into mdmDebugBundleUploadURL, dropping the outer function's cognitive complexity from 26 to 20 (SonarCloud gate is 25). Tests cover the empty-policy and invalid-URL clearing paths.
This commit is contained in:
@@ -691,6 +691,14 @@ func (config *Config) apply(input ConfigInput) (updated bool, err error) {
|
|||||||
// for the key, so per-field rejection of user writes still applies).
|
// for the key, so per-field rejection of user writes still applies).
|
||||||
func (config *Config) applyMDMPolicy(policy *mdm.Policy) {
|
func (config *Config) applyMDMPolicy(policy *mdm.Policy) {
|
||||||
config.policy = policy
|
config.policy = policy
|
||||||
|
|
||||||
|
// DebugBundleUploadURL is a runtime-only override re-derived from MDM on
|
||||||
|
// every apply. Resolve it unconditionally (before the IsEmpty early return)
|
||||||
|
// so a policy that drops the key, becomes empty, or carries an invalid
|
||||||
|
// value can never leave a previously-enforced upload target active on a
|
||||||
|
// reused Config instance.
|
||||||
|
config.DebugBundleUploadURL = mdmDebugBundleUploadURL(policy)
|
||||||
|
|
||||||
if policy.IsEmpty() {
|
if policy.IsEmpty() {
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
@@ -767,22 +775,28 @@ func (config *Config) applyMDMPolicy(policy *mdm.Policy) {
|
|||||||
logApplied(mdm.KeyLazyConnection, state)
|
logApplied(mdm.KeyLazyConnection, state)
|
||||||
}
|
}
|
||||||
|
|
||||||
if v, ok := policy.GetString(mdm.KeyBundleUploadURL); ok {
|
}
|
||||||
// Must be a well-formed https URL with a host, matching the client's
|
|
||||||
// remote-job upload-URL validation. Invalid values are skipped so a
|
// mdmDebugBundleUploadURL resolves the MDM-enforced debug-bundle upload URL
|
||||||
// bad policy cannot break bundle uploads. The URL is not logged: it
|
// override from the policy, returning the empty string when the policy does
|
||||||
// can embed credentials or signed query tokens.
|
// not carry a valid KeyBundleUploadURL. An absent or invalid value fails
|
||||||
if u, err := url.Parse(v); err != nil || u.Scheme != "https" || u.Host == "" {
|
// closed to "" so it falls back to the management-supplied or default upload
|
||||||
log.Warnf("MDM debug bundle upload URL is invalid (must be an https URL with a host); keeping previous value")
|
// target rather than a previously-enforced one. The URL is never logged: it
|
||||||
} else {
|
// can embed credentials or signed query tokens (KeyBundleUploadURL is in
|
||||||
config.DebugBundleUploadURL = v
|
// mdm.SecretKeys).
|
||||||
logApplied(mdm.KeyBundleUploadURL, "")
|
func mdmDebugBundleUploadURL(policy *mdm.Policy) string {
|
||||||
}
|
v, ok := policy.GetString(mdm.KeyBundleUploadURL)
|
||||||
} else {
|
if !ok {
|
||||||
// The key was dropped from the policy: clear any stale override so it
|
return ""
|
||||||
// can never keep directing uploads to a previously-enforced host.
|
|
||||||
config.DebugBundleUploadURL = ""
|
|
||||||
}
|
}
|
||||||
|
// Must be a well-formed https URL with a host, matching the client's
|
||||||
|
// remote-job upload-URL validation.
|
||||||
|
if u, err := url.Parse(v); err != nil || u.Scheme != "https" || u.Host == "" {
|
||||||
|
log.Warnf("MDM debug bundle upload URL is invalid (must be an https URL with a host); ignoring the override")
|
||||||
|
return ""
|
||||||
|
}
|
||||||
|
log.Infof("MDM override %s = ********** (secret)", mdm.KeyBundleUploadURL)
|
||||||
|
return v
|
||||||
}
|
}
|
||||||
|
|
||||||
// parseURL parses and validates the URL for the named service. The URL
|
// parseURL parses and validates the URL for the named service. The URL
|
||||||
|
|||||||
@@ -332,6 +332,21 @@ func TestApplyMDMPolicyRemoteJobs(t *testing.T) {
|
|||||||
cfg.applyMDMPolicy(mdm.NewPolicy(map[string]any{mdm.KeyRemoteJobsAllowed: true}))
|
cfg.applyMDMPolicy(mdm.NewPolicy(map[string]any{mdm.KeyRemoteJobsAllowed: true}))
|
||||||
assert.Empty(t, cfg.DebugBundleUploadURL, "the stale upload URL override must be cleared")
|
assert.Empty(t, cfg.DebugBundleUploadURL, "the stale upload URL override must be cleared")
|
||||||
})
|
})
|
||||||
|
|
||||||
|
t.Run("an empty replacement policy clears a previously-applied override", func(t *testing.T) {
|
||||||
|
cfg := &Config{DebugBundleUploadURL: "https://old.example.com"}
|
||||||
|
// A policy that becomes empty entirely hits the IsEmpty early return;
|
||||||
|
// the override must still be cleared rather than surviving on the
|
||||||
|
// reused Config instance.
|
||||||
|
cfg.applyMDMPolicy(mdm.NewPolicy(map[string]any{}))
|
||||||
|
assert.Empty(t, cfg.DebugBundleUploadURL, "the stale upload URL override must be cleared when the policy empties")
|
||||||
|
})
|
||||||
|
|
||||||
|
t.Run("an invalid upload URL clears a previously-applied override (fail closed)", func(t *testing.T) {
|
||||||
|
cfg := &Config{DebugBundleUploadURL: "https://old.example.com"}
|
||||||
|
cfg.applyMDMPolicy(mdm.NewPolicy(map[string]any{mdm.KeyBundleUploadURL: "not-a-url"}))
|
||||||
|
assert.Empty(t, cfg.DebugBundleUploadURL, "an invalid override must fail closed, not keep the stale target")
|
||||||
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
func TestUpdateOldManagementURL(t *testing.T) {
|
func TestUpdateOldManagementURL(t *testing.T) {
|
||||||
|
|||||||
Reference in New Issue
Block a user