diff --git a/ipn/backend.go b/ipn/backend.go index 3e956f473..ab01d2fde 100644 --- a/ipn/backend.go +++ b/ipn/backend.go @@ -81,6 +81,8 @@ const ( NotifyInitialHealthState NotifyWatchOpt = 1 << 7 // if set, the first Notify message (sent immediately) will contain the current health.State of the client NotifyRateLimit NotifyWatchOpt = 1 << 8 // if set, rate limit spammy netmap updates to every few seconds + + NotifyHealthActions NotifyWatchOpt = 1 << 9 // if set, include PrimaryActions in health.State. Otherwise append the action URL to the text ) // Notify is a communication from a backend (e.g. tailscaled) to a frontend diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index e494920b1..0efec6b9f 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -2948,28 +2948,19 @@ func (b *LocalBackend) WatchNotifications(ctx context.Context, mask ipn.NotifyWa b.WatchNotificationsAs(ctx, nil, mask, onWatchAdded, fn) } -// WatchNotificationsAs is like WatchNotifications but takes an [ipnauth.Actor] +// WatchNotificationsAs is like [LocalBackend.WatchNotifications] but takes an [ipnauth.Actor] // as an additional parameter. If non-nil, the specified callback is invoked // only for notifications relevant to this actor. func (b *LocalBackend) WatchNotificationsAs(ctx context.Context, actor ipnauth.Actor, mask ipn.NotifyWatchOpt, onWatchAdded func(), fn func(roNotify *ipn.Notify) (keepGoing bool)) { ch := make(chan *ipn.Notify, 128) sessionID := rands.HexString(16) - origFn := fn if mask&ipn.NotifyNoPrivateKeys != 0 { - fn = func(n *ipn.Notify) bool { - if n.NetMap == nil || n.NetMap.PrivateKey.IsZero() { - return origFn(n) - } - - // The netmap in n is shared across all watchers, so to mutate it for a - // single watcher we have to clone the notify and the netmap. We can - // make shallow clones, at least. - nm2 := *n.NetMap - n2 := *n - n2.NetMap = &nm2 - n2.NetMap.PrivateKey = key.NodePrivate{} - return origFn(&n2) - } + fn = filterPrivateKeys(fn) + } + if mask&ipn.NotifyHealthActions == 0 { + // if UI does not support PrimaryAction in health warnings, append + // action URLs to the warning text instead. + fn = appendHealthActions(fn) } var ini *ipn.Notify @@ -3060,6 +3051,53 @@ func (b *LocalBackend) WatchNotificationsAs(ctx context.Context, actor ipnauth.A sender.Run(ctx, ch) } +// filterPrivateKeys returns an IPN listener func that wraps the supplied IPN +// listener and zeroes out the PrivateKey in the NetMap passed to the wrapped +// listener. +func filterPrivateKeys(fn func(roNotify *ipn.Notify) (keepGoing bool)) func(*ipn.Notify) bool { + return func(n *ipn.Notify) bool { + if n.NetMap == nil || n.NetMap.PrivateKey.IsZero() { + return fn(n) + } + + // The netmap in n is shared across all watchers, so to mutate it for a + // single watcher we have to clone the notify and the netmap. We can + // make shallow clones, at least. + nm2 := *n.NetMap + n2 := *n + n2.NetMap = &nm2 + n2.NetMap.PrivateKey = key.NodePrivate{} + return fn(&n2) + } +} + +// appendHealthActions returns an IPN listener func that wraps the supplied IPN +// listener func and transforms health messages passed to the wrapped listener. +// If health messages with PrimaryActions are present, it appends the label & +// url in the PrimaryAction to the text of the message. For use for clients that +// do not process the PrimaryAction. +func appendHealthActions(fn func(roNotify *ipn.Notify) (keepGoing bool)) func(*ipn.Notify) bool { + return func(n *ipn.Notify) bool { + if n.Health == nil || len(n.Health.Warnings) == 0 { + return fn(n) + } + + // Shallow clone the notify and health so we can mutate them + h2 := *n.Health + n2 := *n + n2.Health = &h2 + n2.Health.Warnings = make(map[health.WarnableCode]health.UnhealthyState, len(n.Health.Warnings)) + for k, v := range n.Health.Warnings { + if v.PrimaryAction != nil { + v.Text = fmt.Sprintf("%s %s: %s", v.Text, v.PrimaryAction.Label, v.PrimaryAction.URL) + v.PrimaryAction = nil + } + n2.Health.Warnings[k] = v + } + return fn(&n2) + } +} + // pollRequestEngineStatus calls b.e.RequestStatus every 2 seconds until ctx // is done. func (b *LocalBackend) pollRequestEngineStatus(ctx context.Context) { diff --git a/ipn/ipnlocal/local_test.go b/ipn/ipnlocal/local_test.go index d23bd1e26..8f9b6ee68 100644 --- a/ipn/ipnlocal/local_test.go +++ b/ipn/ipnlocal/local_test.go @@ -5348,6 +5348,8 @@ func TestDisplayMessages(t *testing.T) { ht.SetIPNState("NeedsLogin", true) ht.GotStreamedMapResponse() + b.mu.Lock() + defer b.mu.Unlock() b.setNetMapLocked(&netmap.NetworkMap{ DisplayMessages: map[tailcfg.DisplayMessageID]tailcfg.DisplayMessage{ "test-message": { @@ -5374,7 +5376,8 @@ func TestDisplayMessagesURLFilter(t *testing.T) { ht.SetIPNState("NeedsLogin", true) ht.GotStreamedMapResponse() - defer b.lockAndGetUnlock()() + b.mu.Lock() + defer b.mu.Unlock() b.setNetMapLocked(&netmap.NetworkMap{ DisplayMessages: map[tailcfg.DisplayMessageID]tailcfg.DisplayMessage{ "test-message": { @@ -5405,3 +5408,104 @@ func TestDisplayMessagesURLFilter(t *testing.T) { t.Errorf("Unexpected message content (-want/+got):\n%s", diff) } } + +// TestDisplayMessageIPNBus checks that we send health messages appropriately +// based on whether the watcher has sent the [ipn.NotifyHealthActions] watch +// option or not. +func TestDisplayMessageIPNBus(t *testing.T) { + type test struct { + name string + mask ipn.NotifyWatchOpt + wantWarning health.UnhealthyState + } + + msgs := map[tailcfg.DisplayMessageID]tailcfg.DisplayMessage{ + "test-message": { + Title: "Message title", + Text: "Message text.", + Severity: tailcfg.SeverityMedium, + PrimaryAction: &tailcfg.DisplayMessageAction{ + URL: "https://example.com", + Label: "Learn more", + }, + }, + } + + for _, tt := range []test{ + { + name: "older-client-no-actions", + mask: 0, + wantWarning: health.UnhealthyState{ + WarnableCode: "test-message", + Severity: health.SeverityMedium, + Title: "Message title", + Text: "Message text. Learn more: https://example.com", // PrimaryAction appended to text + PrimaryAction: nil, // PrimaryAction not included + }, + }, + { + name: "new-client-with-actions", + mask: ipn.NotifyHealthActions, + wantWarning: health.UnhealthyState{ + WarnableCode: "test-message", + Severity: health.SeverityMedium, + Title: "Message title", + Text: "Message text.", + PrimaryAction: &health.UnhealthyStateAction{ + URL: "https://example.com", + Label: "Learn more", + }, + }, + }, + } { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + lb := newLocalBackendWithTestControl(t, false, func(tb testing.TB, opts controlclient.Options) controlclient.Client { + return newClient(tb, opts) + }) + + ipnWatcher := newNotificationWatcher(t, lb, nil) + ipnWatcher.watch(tt.mask, []wantedNotification{{ + name: "test", + cond: func(_ testing.TB, _ ipnauth.Actor, n *ipn.Notify) bool { + if n.Health == nil { + return false + } + got, ok := n.Health.Warnings["test-message"] + if ok { + if diff := cmp.Diff(tt.wantWarning, got); diff != "" { + t.Errorf("unexpected warning details (-want/+got):\n%s", diff) + return true // we failed the test so tell the watcher we've seen what we need to to stop it waiting + } + } + return ok + }, + }}) + + lb.SetPrefsForTest(&ipn.Prefs{ + ControlURL: "https://localhost:1/", + WantRunning: true, + LoggedOut: false, + }) + if err := lb.Start(ipn.Options{}); err != nil { + t.Fatalf("(*LocalBackend).Start(): %v", err) + } + + cc := lb.cc.(*mockControl) + + // Assert that we are logged in and authorized, and also send our DisplayMessages + cc.send(nil, "", true, &netmap.NetworkMap{ + SelfNode: (&tailcfg.Node{MachineAuthorized: true}).View(), + DisplayMessages: msgs, + }) + + // Tell the health tracker that we are in a map poll because + // mockControl doesn't tell it + lb.HealthTracker().GotStreamedMapResponse() + + // Assert that we got the expected notification + ipnWatcher.check() + }) + } +}