mirror of
https://github.com/netbirdio/netbird.git
synced 2026-10-06 05:29:07 +02:00
[client] Skip late session warnings on desktop and schedule them in the app on Android (#7548)
* Skip session warnings that fire after their window The warning timers run on the monotonic clock, which does not advance while an Android device is suspended. A timer armed for T-10 or T-2 can therefore fire long after the window it was armed for, delivering a "session expires soon" notification once that window is already gone. Gate both callbacks on the wall clock at fire time: the T-10 warning is skipped once the final-warning window has been reached, and the final warning is skipped once the deadline itself has passed. Both set their edge guard before returning so a skipped warning cannot fire again for the same deadline. * Harden the late-warning guards Clamp a non-positive final lead to zero in the T-10 guard so a disabled final warning cannot move the cutoff past the deadline, matching how armTimerLocked already treats it. Strip the monotonic reading from both sides of the comparison so the guard measures wall-clock time regardless of how the caller built the deadline. The production deadline comes from a protobuf timestamp and has no monotonic reading; this keeps the guard correct for callers that derive one from time.Now. * Log the deadline and lateness on skipped warnings Include the deadline and how far past the cutoff the timer fired, so a debug bundle shows how long the device was suspended. * Inject the clock into the late-warning guard and cover it with tests The guard read time.Now internally, so the skip paths were reachable only through a deadline already in the past and the boundary depended on real time. Extract the comparison into isLate and read the time through a nowFn field, so tests can place a resume anywhere around the deadline without sleeping. * Send the final warning when the T-10 timer fires inside its window A suspend between roughly eight and ten minutes long made the T-10 timer fire inside the final-warning window and the final timer fire after the deadline, so both were skipped and a user who resumed with time left got no warning at all. When the T-10 timer fires late but before the deadline, send the final warning in its place and mark it fired so the delayed final timer does not repeat it. * Respect dismissal when promoting a late warning to the final one fireFinal skips the final warning once the user dismissed the deadline, but the promoted path did not, so a dismissed deadline could still get a final warning. Check the dismissal first, and give each skip reason its own log line so an already-fired final warning no longer logs a negative lateness. * Add a deadline-only mode to the session watcher Android will schedule its own expiry warnings from the deadline, so the engine must not arm the T-10 and T-2 timers there. NewDeadlineOnly keeps the deadline validation, the recorder propagation and the logging, and skips only the timers, so the status snapshot the app reads stays correct and an out-of-range deadline is still rejected. * Use the deadline-only watcher on Android and drop the warning callbacks The warning timers run on the monotonic clock, which does not advance while the device sleeps, so a warning armed for T-10 could fire long after its window. The app now schedules the warnings itself with WorkManager, anchored to the wall clock, from the deadline it reads through SessionExpiresAtUnix on every OnStateChanged. Wire the deadline-only watcher into the android build and remove the event-driven path from the gomobile surface: OnSessionExpiring, the event subscription behind it and DismissSessionWarning, which the app never called. * Describe the late-warning guard without naming Android The guard stays for the desktop builds, where a timer can also stall across a sleep. Android no longer arms the timers at all.
This commit is contained in:
Zoltan Papp
@@ -90,8 +90,9 @@ type StatusRecorder interface {
|
||||
// fallback T-FinalWarningLead dialog (suppressed when the user dismissed
|
||||
// the first one for the same deadline). Safe for concurrent use.
|
||||
type Watcher struct {
|
||||
lead time.Duration
|
||||
finalLead time.Duration
|
||||
lead time.Duration
|
||||
finalLead time.Duration
|
||||
deadlineOnly bool
|
||||
|
||||
mu sync.Mutex
|
||||
current time.Time
|
||||
@@ -102,6 +103,7 @@ type Watcher struct {
|
||||
dismissedAt time.Time // deadline value the user dismissed via Dismiss(); gates fireFinal
|
||||
closed bool
|
||||
recorder StatusRecorder
|
||||
nowFn func() time.Time
|
||||
}
|
||||
|
||||
// New returns a watcher with the package defaults WarningLead and
|
||||
@@ -122,9 +124,17 @@ func NewWithLeads(lead, final time.Duration, recorder StatusRecorder) *Watcher {
|
||||
lead: lead,
|
||||
finalLead: final,
|
||||
recorder: recorder,
|
||||
nowFn: time.Now,
|
||||
}
|
||||
}
|
||||
|
||||
// NewDeadlineOnly returns a watcher that validates and records deadlines but arms no warning timers.
|
||||
func NewDeadlineOnly(recorder StatusRecorder) *Watcher {
|
||||
w := New(recorder)
|
||||
w.deadlineOnly = true
|
||||
return w
|
||||
}
|
||||
|
||||
// Update sets the latest deadline. Pass the zero time to clear (e.g. when
|
||||
// a Sync push from the server omits the field because login expiration
|
||||
// was disabled).
|
||||
@@ -181,7 +191,7 @@ func (w *Watcher) Update(deadline time.Time) error {
|
||||
w.finalFiredAt = time.Time{}
|
||||
w.dismissedAt = time.Time{}
|
||||
|
||||
if deadline.After(now) {
|
||||
if deadline.After(now) && !w.deadlineOnly {
|
||||
w.armTimerLocked(deadline)
|
||||
}
|
||||
recorder := w.recorder
|
||||
@@ -303,6 +313,11 @@ func (w *Watcher) fire(armedFor time.Time) {
|
||||
w.mu.Unlock()
|
||||
return
|
||||
}
|
||||
now := w.nowFn()
|
||||
if isLate(now, armedFor, max(w.finalLead, 0)) {
|
||||
w.fireLateLocked(armedFor, now)
|
||||
return
|
||||
}
|
||||
w.firedAt = armedFor
|
||||
recorder := w.recorder
|
||||
w.mu.Unlock()
|
||||
@@ -331,6 +346,14 @@ func (w *Watcher) fireFinal(armedFor time.Time) {
|
||||
log.Infof("auth session final-warning skipped (dismissed by user)")
|
||||
return
|
||||
}
|
||||
now := w.nowFn()
|
||||
if isLate(now, armedFor, 0) {
|
||||
w.finalFiredAt = armedFor
|
||||
w.mu.Unlock()
|
||||
log.Infof("auth session final-warning skipped for deadline %s (passed %s ago)",
|
||||
armedFor.Format(time.RFC3339), now.Round(0).Sub(armedFor).Round(time.Second))
|
||||
return
|
||||
}
|
||||
w.finalFiredAt = armedFor
|
||||
recorder := w.recorder
|
||||
w.mu.Unlock()
|
||||
@@ -341,6 +364,39 @@ func (w *Watcher) fireFinal(armedFor time.Time) {
|
||||
publishWarning(recorder, armedFor, true)
|
||||
}
|
||||
|
||||
// fireLateLocked handles a T-WarningLead callback that fired inside the
|
||||
// final-warning window: it sends the final warning in its place while the
|
||||
// deadline has not passed and the user has not dismissed it, so a resume
|
||||
// with time left still warns. The caller must hold w.mu; this helper
|
||||
// releases it.
|
||||
func (w *Watcher) fireLateLocked(armedFor, now time.Time) {
|
||||
w.firedAt = armedFor
|
||||
switch {
|
||||
case w.dismissedAt.Equal(armedFor):
|
||||
w.mu.Unlock()
|
||||
log.Infof("auth session expiry soon warning skipped (dismissed by user)")
|
||||
return
|
||||
case w.finalFiredAt.Equal(armedFor):
|
||||
w.mu.Unlock()
|
||||
log.Infof("auth session expiry soon warning skipped (final warning already fired)")
|
||||
return
|
||||
case isLate(now, armedFor, 0):
|
||||
w.mu.Unlock()
|
||||
log.Infof("auth session expiry soon warning skipped for deadline %s (passed %s ago)",
|
||||
armedFor.Format(time.RFC3339), now.Round(0).Sub(armedFor).Round(time.Second))
|
||||
return
|
||||
}
|
||||
w.finalFiredAt = armedFor
|
||||
recorder := w.recorder
|
||||
w.mu.Unlock()
|
||||
if recorder == nil {
|
||||
return
|
||||
}
|
||||
log.Infof("auth session expiry soon warning fired inside the final-warning window, sending final warning for deadline %s",
|
||||
armedFor.Format(time.RFC3339))
|
||||
publishWarning(recorder, armedFor, true)
|
||||
}
|
||||
|
||||
// armOneShotLocked schedules cb at fireAt. When fireAt is already in the
|
||||
// past it dispatches on the next scheduler tick so a state-change recorder
|
||||
// notification (invoked after w.mu is released) lands first. Caller must
|
||||
@@ -380,3 +436,11 @@ func publishWarning(recorder StatusRecorder, deadline time.Time, final bool) {
|
||||
meta,
|
||||
)
|
||||
}
|
||||
|
||||
// isLate reports whether the wall clock now has already reached armedFor
|
||||
// minus cutoffLead. The timers run on the monotonic clock, which can stall
|
||||
// while the host sleeps, so a timer can fire long after the window it was
|
||||
// armed for.
|
||||
func isLate(now, armedFor time.Time, cutoffLead time.Duration) bool {
|
||||
return !now.Round(0).Before(armedFor.Add(-cutoffLead).Round(0))
|
||||
}
|
||||
|
||||
@@ -527,3 +527,201 @@ func TestDismissBeforeUpdateIsNoop(t *testing.T) {
|
||||
}
|
||||
t.Fatalf("final-warning did not publish after no-op pre-Update Dismiss, events=%+v", r.snapshot())
|
||||
}
|
||||
|
||||
func TestIsLate(t *testing.T) {
|
||||
armedFor := time.Date(2026, 10, 1, 12, 0, 0, 0, time.UTC)
|
||||
lead := 2 * time.Minute
|
||||
tests := []struct {
|
||||
name string
|
||||
now time.Time
|
||||
cutoffLead time.Duration
|
||||
want bool
|
||||
}{
|
||||
{"before cutoff", armedFor.Add(-3 * time.Minute), lead, false},
|
||||
{"at cutoff", armedFor.Add(-lead), lead, true},
|
||||
{"after cutoff", armedFor.Add(-time.Minute), lead, true},
|
||||
{"zero lead before deadline", armedFor.Add(-time.Second), 0, false},
|
||||
{"zero lead at deadline", armedFor, 0, true},
|
||||
{"zero lead after deadline", armedFor.Add(time.Second), 0, true},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
if got := isLate(tt.now, armedFor, tt.cutoffLead); got != tt.want {
|
||||
t.Fatalf("isLate(%s, %s, %s) = %v, want %v", tt.now, armedFor, tt.cutoffLead, got, tt.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestIsLateIgnoresMonotonicReading(t *testing.T) {
|
||||
now := time.Now()
|
||||
wallOnly := now.Round(0)
|
||||
if isLate(now, wallOnly.Add(time.Second), 0) {
|
||||
t.Fatalf("now with monotonic reading must compare as wall clock before a later wall-only deadline")
|
||||
}
|
||||
if !isLate(now, wallOnly, 0) {
|
||||
t.Fatalf("now with monotonic reading must compare as wall clock at an equal wall-only deadline")
|
||||
}
|
||||
}
|
||||
|
||||
func TestLateTimerFiring(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
final bool
|
||||
beforeDl time.Duration
|
||||
wantWarns int
|
||||
wantFinals int
|
||||
}{
|
||||
{"warning on resume inside window", false, 3 * time.Minute, 1, 0},
|
||||
{"warning promoted to final inside final window", false, time.Minute, 0, 1},
|
||||
{"warning skipped past deadline", false, -time.Minute, 0, 0},
|
||||
{"final on resume before deadline", true, time.Minute, 0, 1},
|
||||
{"final skipped past deadline", true, -time.Minute, 0, 0},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
r := &fakeRecorder{}
|
||||
w := New(r)
|
||||
defer w.Close()
|
||||
|
||||
// The deadline is an hour out so the real timers never fire
|
||||
// during the test; the late callback is invoked directly with an
|
||||
// injected clock that simulates a resume near the deadline.
|
||||
d := time.Now().Add(time.Hour).Round(0)
|
||||
w.nowFn = func() time.Time { return d.Add(-tt.beforeDl) }
|
||||
if err := w.Update(d); err != nil {
|
||||
t.Fatalf("Update: %v", err)
|
||||
}
|
||||
|
||||
if tt.final {
|
||||
w.fireFinal(d)
|
||||
} else {
|
||||
w.fire(d)
|
||||
}
|
||||
|
||||
events := r.snapshot()
|
||||
if got := countWhere(events, event.isWarning); got != tt.wantWarns {
|
||||
t.Fatalf("expected %d warning publishes, got %d: %+v", tt.wantWarns, got, events)
|
||||
}
|
||||
if got := countWhere(events, event.isFinalWarning); got != tt.wantFinals {
|
||||
t.Fatalf("expected %d final-warning publishes, got %d: %+v", tt.wantFinals, got, events)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestPromotedFinalWarningIsNotRepeated(t *testing.T) {
|
||||
r := &fakeRecorder{}
|
||||
w := New(r)
|
||||
defer w.Close()
|
||||
|
||||
d := time.Now().Add(time.Hour).Round(0)
|
||||
now := d.Add(-time.Minute)
|
||||
w.nowFn = func() time.Time { return now }
|
||||
if err := w.Update(d); err != nil {
|
||||
t.Fatalf("Update: %v", err)
|
||||
}
|
||||
|
||||
w.fire(d)
|
||||
// The final timer was suspended too, so it fires even later than the
|
||||
// warning timer, here still just before the deadline.
|
||||
now = d.Add(-30 * time.Second)
|
||||
w.fireFinal(d)
|
||||
|
||||
events := r.snapshot()
|
||||
if got := countWhere(events, event.isFinalWarning); got != 1 {
|
||||
t.Fatalf("expected exactly 1 final-warning publish, got %d: %+v", got, events)
|
||||
}
|
||||
if got := countWhere(events, event.isWarning); got != 0 {
|
||||
t.Fatalf("expected no regular warning publish, got %d: %+v", got, events)
|
||||
}
|
||||
}
|
||||
|
||||
func TestPromotionRespectsDismiss(t *testing.T) {
|
||||
r := &fakeRecorder{}
|
||||
w := New(r)
|
||||
defer w.Close()
|
||||
|
||||
d := time.Now().Add(time.Hour).Round(0)
|
||||
w.nowFn = func() time.Time { return d.Add(-time.Minute) }
|
||||
if err := w.Update(d); err != nil {
|
||||
t.Fatalf("Update: %v", err)
|
||||
}
|
||||
|
||||
w.Dismiss()
|
||||
w.fire(d)
|
||||
|
||||
events := r.snapshot()
|
||||
if got := countWhere(events, func(e event) bool { return e.kind == publish }); got != 0 {
|
||||
t.Fatalf("expected no publish after dismiss, got %d: %+v", got, events)
|
||||
}
|
||||
}
|
||||
|
||||
func TestPromotionSkippedWhenFinalAlreadyFired(t *testing.T) {
|
||||
r := &fakeRecorder{}
|
||||
w := New(r)
|
||||
defer w.Close()
|
||||
|
||||
// Both timers fall in the past after a long suspend and are dispatched
|
||||
// with a zero delay, so the final callback can run before the warning one.
|
||||
d := time.Now().Add(time.Hour).Round(0)
|
||||
w.nowFn = func() time.Time { return d.Add(-time.Minute) }
|
||||
if err := w.Update(d); err != nil {
|
||||
t.Fatalf("Update: %v", err)
|
||||
}
|
||||
|
||||
w.fireFinal(d)
|
||||
w.fire(d)
|
||||
|
||||
events := r.snapshot()
|
||||
if got := countWhere(events, event.isFinalWarning); got != 1 {
|
||||
t.Fatalf("expected exactly 1 final-warning publish, got %d: %+v", got, events)
|
||||
}
|
||||
if got := countWhere(events, event.isWarning); got != 0 {
|
||||
t.Fatalf("expected no regular warning publish, got %d: %+v", got, events)
|
||||
}
|
||||
}
|
||||
|
||||
func TestDeadlineOnlyRecordsDeadlineWithoutWarnings(t *testing.T) {
|
||||
r := &fakeRecorder{}
|
||||
w := NewDeadlineOnly(r)
|
||||
defer w.Close()
|
||||
|
||||
// With the default leads this deadline would otherwise fire both
|
||||
// timers on the next tick.
|
||||
d := time.Now().Add(50 * time.Millisecond).Round(0)
|
||||
if err := w.Update(d); err != nil {
|
||||
t.Fatalf("Update: %v", err)
|
||||
}
|
||||
if got := r.deadline(); !got.Equal(d) {
|
||||
t.Fatalf("expected recorder deadline %v, got %v", d, got)
|
||||
}
|
||||
|
||||
time.Sleep(100 * time.Millisecond)
|
||||
|
||||
events := r.snapshot()
|
||||
if got := countWhere(events, func(e event) bool { return e.kind == publish }); got != 0 {
|
||||
t.Fatalf("expected no publish in deadline-only mode, got %d: %+v", got, events)
|
||||
}
|
||||
if w.timer != nil || w.finalTimer != nil {
|
||||
t.Fatal("expected no timers armed in deadline-only mode")
|
||||
}
|
||||
}
|
||||
|
||||
func TestDeadlineOnlyStillRejectsOutOfRangeDeadlines(t *testing.T) {
|
||||
r := &fakeRecorder{}
|
||||
w := NewDeadlineOnly(r)
|
||||
defer w.Close()
|
||||
|
||||
if err := w.Update(time.Now().Add(time.Hour)); err != nil {
|
||||
t.Fatalf("Update: %v", err)
|
||||
}
|
||||
|
||||
err := w.Update(time.Now().Add(-maxPastHorizon - time.Hour))
|
||||
if !errors.Is(err, ErrDeadlineInPast) {
|
||||
t.Fatalf("expected ErrDeadlineInPast, got %v", err)
|
||||
}
|
||||
if got := r.deadline(); !got.IsZero() {
|
||||
t.Fatalf("expected recorder cleared after rejection, got %v", got)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user