diff --git a/management/server/account.go b/management/server/account.go index 68150a947..7cab037a2 100644 --- a/management/server/account.go +++ b/management/server/account.go @@ -806,6 +806,21 @@ func (am *DefaultAccountManager) schedulePeerLoginExpiration(ctx context.Context } } +// schedulePeerInactivityExpiration arms the account's inactivity expiration job for a +// connecting peer unless one is already scheduled: a connected peer cannot expire, so it +// never moves the scheduled run earlier. Disconnects must not skip a running job, which may +// be finishing its last run without seeing the newly disconnected peer. +func (am *DefaultAccountManager) schedulePeerInactivityExpiration(ctx context.Context, accountID string) { + if am.peerInactivityExpiry.IsSchedulerRunning(accountID) { + log.WithContext(ctx).Tracef("peer inactivity expiration job for account %s is already scheduled", accountID) + return + } + jobCtx := context.WithoutCancel(ctx) + if nextRun, ok := am.getNextInactivePeerExpiration(ctx, accountID); ok { + go am.peerInactivityExpiry.Schedule(jobCtx, nextRun, accountID, am.peerInactivityExpirationJob(jobCtx, accountID)) + } +} + // peerInactivityExpirationJob marks login expired for all inactive peers and returns the minimum duration in which the next peer of the account will expire by inactivity if found func (am *DefaultAccountManager) peerInactivityExpirationJob(ctx context.Context, accountID string) func() (time.Duration, bool) { return func() (time.Duration, bool) { diff --git a/management/server/account_test.go b/management/server/account_test.go index 983ee8680..7acc72d1f 100644 --- a/management/server/account_test.go +++ b/management/server/account_test.go @@ -13,6 +13,7 @@ import ( "strconv" "strings" "sync" + "sync/atomic" "testing" "testing/synctest" "time" @@ -2277,6 +2278,57 @@ func TestDefaultAccountManager_MarkPeerDisconnected_SchedulesInactivityExpiratio } } +func TestDefaultAccountManager_PeerConnect_KeepsScheduledInactivityExpiration(t *testing.T) { + manager, _, err := createManager(t) + require.NoError(t, err, "unable to create account manager") + + accountID, err := manager.GetAccountIDByUserID(context.Background(), auth.UserAuth{UserId: userID}) + require.NoError(t, err, "unable to create an account") + + key, err := wgtypes.GenerateKey() + require.NoError(t, err, "unable to generate WireGuard key") + peerPubKey := key.PublicKey().String() + + _, _, _, _, err = manager.AddPeer(context.Background(), "", "", userID, &nbpeer.Peer{ + Key: peerPubKey, + Meta: nbpeer.PeerSystemMeta{Hostname: "test-peer"}, + InactivityExpirationEnabled: true, + }, false) + require.NoError(t, err, "unable to add peer") + + _, err = manager.UpdateAccountSettings(context.Background(), accountID, userID, &types.Settings{ + PeerLoginExpiration: time.Hour, + PeerLoginExpirationEnabled: true, + PeerInactivityExpiration: time.Hour, + PeerInactivityExpirationEnabled: true, + Extra: &types.ExtraSettings{}, + }) + require.NoError(t, err, "expecting to update account settings successfully but got error") + + // The account already has an inactivity job scheduled. A connecting peer + // cannot expire, so connect must leave that job alone instead of cancelling + // it and re-scanning every peer of the account. Cancel runs synchronously, + // so counting it observes the decision deterministically. + var cancels atomic.Int32 + manager.peerInactivityExpiry = &MockScheduler{ + IsSchedulerRunningFunc: func(ID string) bool { return ID == accountID }, + CancelFunc: func(ctx context.Context, IDs []string) { cancels.Add(1) }, + ScheduleFunc: func(ctx context.Context, in time.Duration, ID string, job func() (nextRunIn time.Duration, reschedule bool)) { + }, + } + + streamStartTime := time.Now().UTC() + err = manager.MarkPeerConnected(context.Background(), peerPubKey, accountID, streamStartTime.UnixNano(), nil) + require.NoError(t, err, "unable to mark peer connected") + assert.Zero(t, cancels.Load(), "connect must not cancel a scheduled inactivity job") + + // A disconnect still cancels and re-arms: the running job may be finishing + // its last run without having seen this peer as disconnected. + err = manager.MarkPeerDisconnected(context.Background(), peerPubKey, accountID, streamStartTime.UnixNano()) + require.NoError(t, err, "unable to mark peer disconnected") + assert.Equal(t, int32(1), cancels.Load(), "disconnect must cancel and re-arm the inactivity job") +} + func TestDefaultAccountManager_MarkPeerDisconnected_SkipsInactivityExpirationWhenDisabled(t *testing.T) { manager, _, err := createManager(t) require.NoError(t, err, "unable to create account manager") diff --git a/management/server/peer.go b/management/server/peer.go index aa3abcb86..5b33ca9f8 100644 --- a/management/server/peer.go +++ b/management/server/peer.go @@ -140,7 +140,7 @@ func (am *DefaultAccountManager) MarkPeerConnected(ctx context.Context, peerPubK return nil } -// schedulePeerExpirations reschedules the account's login/inactivity expiration +// schedulePeerExpirations arms the account's login/inactivity expiration // timers for an SSO peer that just connected. func (am *DefaultAccountManager) schedulePeerExpirations(ctx context.Context, accountID string, peer *nbpeer.Peer) error { if !peer.AddedWithSSOLogin() { @@ -154,7 +154,7 @@ func (am *DefaultAccountManager) schedulePeerExpirations(ctx context.Context, ac am.schedulePeerLoginExpiration(ctx, accountID) } if peer.InactivityExpirationEnabled && settings.PeerInactivityExpirationEnabled { - am.checkAndSchedulePeerInactivityExpiration(ctx, accountID) + am.schedulePeerInactivityExpiration(ctx, accountID) } return nil }