diff --git a/management/internals/modules/peers/manager.go b/management/internals/modules/peers/manager.go index 6f292f6ed..fd234103e 100644 --- a/management/internals/modules/peers/manager.go +++ b/management/internals/modules/peers/manager.go @@ -44,6 +44,8 @@ type Manager interface { // to. Used by the proxy's auth path to authorise a request by the calling // peer's group memberships. GetPeerWithGroups(ctx context.Context, accountID, peerID string) (*peer.Peer, []*types.Group, error) + // RefreshLastSeen records that a peer was seen at seenAt. + RefreshLastSeen(ctx context.Context, accountID, peerID string, seenAt time.Time) error } type managerImpl struct { @@ -128,6 +130,10 @@ func (m *managerImpl) GetPeerWithGroups(ctx context.Context, accountID, peerID s return p, groups, nil } +func (m *managerImpl) RefreshLastSeen(ctx context.Context, accountID, peerID string, seenAt time.Time) error { + return m.store.RefreshPeerLastSeen(ctx, accountID, peerID, seenAt) +} + func (m *managerImpl) DeletePeers(ctx context.Context, accountID string, peerIDs []string, userID string, checkConnected bool) error { settings, err := m.store.GetAccountSettings(ctx, store.LockingStrengthNone, accountID) if err != nil { diff --git a/management/internals/modules/peers/manager_mock.go b/management/internals/modules/peers/manager_mock.go index 3836ac909..21f91c06a 100644 --- a/management/internals/modules/peers/manager_mock.go +++ b/management/internals/modules/peers/manager_mock.go @@ -8,6 +8,7 @@ import ( context "context" net "net" reflect "reflect" + time "time" gomock "github.com/golang/mock/gomock" network_map "github.com/netbirdio/netbird/management/internals/controllers/network_map" @@ -174,6 +175,20 @@ func (mr *MockManagerMockRecorder) GetPeersByGroupIDs(ctx, accountID, groupsIDs return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetPeersByGroupIDs", reflect.TypeOf((*MockManager)(nil).GetPeersByGroupIDs), ctx, accountID, groupsIDs) } +// RefreshLastSeen mocks base method. +func (m *MockManager) RefreshLastSeen(ctx context.Context, accountID, peerID string, seenAt time.Time) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "RefreshLastSeen", ctx, accountID, peerID, seenAt) + ret0, _ := ret[0].(error) + return ret0 +} + +// RefreshLastSeen indicates an expected call of RefreshLastSeen. +func (mr *MockManagerMockRecorder) RefreshLastSeen(ctx, accountID, peerID, seenAt interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RefreshLastSeen", reflect.TypeOf((*MockManager)(nil).RefreshLastSeen), ctx, accountID, peerID, seenAt) +} + // SetAccountManager mocks base method. func (m *MockManager) SetAccountManager(accountManager account.Manager) { m.ctrl.T.Helper() diff --git a/management/internals/shared/grpc/proxy.go b/management/internals/shared/grpc/proxy.go index 4cae872e2..28c326677 100644 --- a/management/internals/shared/grpc/proxy.go +++ b/management/internals/shared/grpc/proxy.go @@ -1672,7 +1672,7 @@ func (s *ProxyServiceServer) GenerateSessionToken(ctx context.Context, domain, u groupIDs, groupNames := pairGroupIDsAndNames(userGroups) - return sessionkey.SignToken( + token, err := sessionkey.SignToken( service.SessionPrivateKey, userID, user.Email, @@ -1682,6 +1682,28 @@ func (s *ProxyServiceServer) GenerateSessionToken(ctx context.Context, domain, u groupNames, proxyauth.DefaultSessionExpiry, ) + if err != nil { + return "", err + } + + s.recordUserLogin(ctx, service.AccountID, user) + + return token, nil +} + +// recordUserLogin marks a completed reverse proxy SSO login on the user. The +// timestamp is what activity accounting reads to count someone who only ever +// reaches proxy-protected services and never opens the dashboard. Service users +// have no interactive login to record. A failure is logged and dropped: the +// next login marks it again and no authorization decision reads it. +func (s *ProxyServiceServer) recordUserLogin(ctx context.Context, accountID string, user *types.User) { + if user.IsServiceUser { + return + } + + if err := s.usersManager.RefreshLastLogin(ctx, accountID, user.Id, time.Now().UTC()); err != nil { + log.WithContext(ctx).Debugf("record proxy login for user %s: %v", user.Id, err) + } } // ValidateUserGroupAccess checks if a user has access to a service. @@ -2031,6 +2053,8 @@ func (s *ProxyServiceServer) ValidateTunnelPeer(ctx context.Context, req *proto. return nil, err } + s.recordPeerSeen(ctx, service.AccountID, peer) + log.WithFields(log.Fields{ "domain": domain, "tunnel_ip": tunnelIPStr, @@ -2048,6 +2072,33 @@ func (s *ProxyServiceServer) ValidateTunnelPeer(ctx context.Context, req *proto. }, nil } +// proxyPeerSeenInterval is how stale a peer's LastSeen must be before reaching +// a private service refreshes it. Positive tunnel validations are cached on the +// proxy for five minutes, so without a floor a busy peer would rewrite its row +// all day; an hour still sits well inside the window activity accounting asks +// about. +const proxyPeerSeenInterval = time.Hour + +// recordPeerSeen marks a peer as seen when it reaches a private service over +// the mesh, which is what lets its owner count as active. Peers that activity +// accounting excludes are skipped rather than written for nothing, and so is a +// peer already seen inside the interval — the row is in hand, so the throttle +// costs nothing. A failure is logged and dropped: the next request marks it +// again. +func (s *ProxyServiceServer) recordPeerSeen(ctx context.Context, accountID string, peer *peer.Peer) { + if !peer.CountsTowardActivity() { + return + } + + if peer.Status != nil && time.Since(peer.Status.LastSeen) < proxyPeerSeenInterval { + return + } + + if err := s.peersManager.RefreshLastSeen(ctx, accountID, peer.ID, time.Now().UTC()); err != nil { + log.WithContext(ctx).Debugf("record proxy activity for peer %s: %v", peer.ID, err) + } +} + // resolvePeerOwner returns the user a peer is linked to, once per request so // the status gate and the identity resolution below share a single lookup. // Unlinked peers (machine agents) have no owner. A lookup that fails returns diff --git a/management/internals/shared/grpc/proxy_group_access_test.go b/management/internals/shared/grpc/proxy_group_access_test.go index cb30a07b9..fc0fbc6ac 100644 --- a/management/internals/shared/grpc/proxy_group_access_test.go +++ b/management/internals/shared/grpc/proxy_group_access_test.go @@ -5,6 +5,7 @@ import ( "errors" "net" "testing" + "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -122,6 +123,20 @@ type mockUsersManager struct { users map[string]*types.User err error getUserCalls int + loginMarks []loginMark +} + +// loginMark records a RefreshLastLogin call so tests can assert what the proxy +// wrote, rather than that it merely called something. +type loginMark struct { + accountID string + userID string + at time.Time +} + +func (m *mockUsersManager) RefreshLastLogin(_ context.Context, accountID, userID string, loginAt time.Time) error { + m.loginMarks = append(m.loginMarks, loginMark{accountID: accountID, userID: userID, at: loginAt}) + return nil } func (m *mockUsersManager) GetUser(ctx context.Context, userID string) (*types.User, error) { @@ -153,6 +168,19 @@ type mockTunnelPeersManager struct { peerErr error groups []*types.Group groupsErr error + seenMarks []seenMark +} + +// seenMark records a RefreshLastSeen call. +type seenMark struct { + accountID string + peerID string + at time.Time +} + +func (m *mockTunnelPeersManager) RefreshLastSeen(_ context.Context, accountID, peerID string, seenAt time.Time) error { + m.seenMarks = append(m.seenMarks, seenMark{accountID: accountID, peerID: peerID, at: seenAt}) + return nil } func (m *mockTunnelPeersManager) GetPeerByTunnelIP(_ context.Context, _ string, _ net.IP) (*peer.Peer, error) { @@ -745,6 +773,121 @@ func TestValidateTunnelPeerOwnerStatus(t *testing.T) { } } +// TestValidateTunnelPeerRecordsActivity covers the activity write on the mesh +// fast-path: a peer reaching a private service is what lets its owner count as +// active, but only peers that activity accounting actually counts are written, +// and only once per interval. +func TestValidateTunnelPeerRecordsActivity(t *testing.T) { + const ( + domain = "app.example.com" + accountID = "account1" + peerID = "peer1" + ) + + tests := []struct { + name string + peer *peer.Peer + expectMark bool + }{ + { + name: "peer seen long ago is marked", + peer: &peer.Peer{ID: peerID, Name: "agent", Status: &peer.PeerStatus{LastSeen: time.Now().Add(-3 * time.Hour)}}, + expectMark: true, + }, + { + name: "peer never seen is marked", + peer: &peer.Peer{ID: peerID, Name: "agent", Status: &peer.PeerStatus{}}, + expectMark: true, + }, + { + // The throttle. The peer row is already in hand, so a recently seen + // peer costs nothing to skip. + name: "peer seen inside the interval is skipped", + peer: &peer.Peer{ID: peerID, Name: "agent", Status: &peer.PeerStatus{LastSeen: time.Now().Add(-10 * time.Minute)}}, + expectMark: false, + }, + { + name: "embedded proxy peer is skipped", + peer: &peer.Peer{ID: peerID, Name: "embedded", ProxyMeta: peer.ProxyMeta{Embedded: true}, Status: &peer.PeerStatus{LastSeen: time.Now().Add(-3 * time.Hour)}}, + expectMark: false, + }, + { + name: "browser client is skipped", + peer: &peer.Peer{ID: peerID, Name: "browser", Meta: peer.PeerSystemMeta{KernelVersion: "wasm"}, Status: &peer.PeerStatus{LastSeen: time.Now().Add(-3 * time.Hour)}}, + expectMark: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + peersManager := &mockTunnelPeersManager{peer: tt.peer} + server := &ProxyServiceServer{ + serviceManager: &mockReverseProxyManager{ + proxiesByAccount: map[string][]*service.Service{ + accountID: {{Domain: domain, AccountID: accountID}}, + }, + }, + peersManager: peersManager, + usersManager: &mockUsersManager{users: map[string]*types.User{}}, + } + + resp, err := server.ValidateTunnelPeer(context.Background(), &proto.ValidateTunnelPeerRequest{ + Domain: domain, + TunnelIp: "100.64.0.1", + }) + + require.NoError(t, err) + require.True(t, resp.GetValid(), "peer should be granted access") + + if !tt.expectMark { + assert.Empty(t, peersManager.seenMarks, "peer should not have been marked seen") + return + } + + require.Len(t, peersManager.seenMarks, 1, "peer should have been marked seen exactly once") + mark := peersManager.seenMarks[0] + assert.Equal(t, accountID, mark.accountID, "activity must be recorded against the service account") + assert.Equal(t, peerID, mark.peerID, "activity must be recorded against the calling peer") + assert.Equal(t, time.UTC, mark.at.Location(), "timestamps are written in UTC") + assert.WithinDuration(t, time.Now().UTC(), mark.at, time.Minute, "seen timestamp should be now") + }) + } +} + +// TestValidateTunnelPeerDeniedRecordsNoActivity keeps the write on the granted +// path only: a refused peer is not evidence its owner was active. +func TestValidateTunnelPeerDeniedRecordsNoActivity(t *testing.T) { + const ( + domain = "app.example.com" + accountID = "account1" + ) + + peersManager := &mockTunnelPeersManager{ + peer: &peer.Peer{ID: "peer1", Name: "agent", UserID: "user1", Status: &peer.PeerStatus{LastSeen: time.Now().Add(-3 * time.Hour)}}, + } + server := &ProxyServiceServer{ + serviceManager: &mockReverseProxyManager{ + proxiesByAccount: map[string][]*service.Service{ + accountID: {{Domain: domain, AccountID: accountID}}, + }, + }, + peersManager: peersManager, + // The owner is blocked, so the tunnel gate denies before the mint. + usersManager: &mockUsersManager{users: map[string]*types.User{ + "user1": {Id: "user1", AccountID: accountID, Blocked: true}, + }}, + } + + resp, err := server.ValidateTunnelPeer(context.Background(), &proto.ValidateTunnelPeerRequest{ + Domain: domain, + TunnelIp: "100.64.0.1", + }) + + require.NoError(t, err) + require.False(t, resp.GetValid(), "blocked owner should be denied") + assert.Empty(t, peersManager.seenMarks, "a denied peer must not be marked seen") +} + func TestGetAccountProxyByDomain(t *testing.T) { tests := []struct { name string diff --git a/management/internals/shared/grpc/validate_session_test.go b/management/internals/shared/grpc/validate_session_test.go index 03f200414..9a99c42f4 100644 --- a/management/internals/shared/grpc/validate_session_test.go +++ b/management/internals/shared/grpc/validate_session_test.go @@ -587,6 +587,10 @@ type testValidateSessionUsersManager struct { store store.Store } +func (m *testValidateSessionUsersManager) RefreshLastLogin(ctx context.Context, accountID, userID string, loginAt time.Time) error { + return m.store.RefreshUserLastLogin(ctx, accountID, userID, loginAt) +} + func (m *testValidateSessionUsersManager) GetUser(ctx context.Context, userID string) (*types.User, error) { return m.store.GetUserByUserID(ctx, store.LockingStrengthNone, userID) } diff --git a/management/server/http/handlers/proxy/auth_callback_integration_test.go b/management/server/http/handlers/proxy/auth_callback_integration_test.go index 51e54ebce..c7a05c1c8 100644 --- a/management/server/http/handlers/proxy/auth_callback_integration_test.go +++ b/management/server/http/handlers/proxy/auth_callback_integration_test.go @@ -538,6 +538,55 @@ func TestAuthCallback_UserAllowedToLogin(t *testing.T) { // TestAuthCallback_UserDeniedByAccountStatus asserts that a user whose account // is pending approval or blocked never receives a session token from the OIDC // callback, and that the redirect carries a description the proxy can render. +// TestAuthCallback_RecordsUserLogin drives the real OIDC callback and asserts +// the login lands on the user row. That timestamp is what activity accounting +// reads, and it is the only signal that can ever count someone who reaches +// proxy-protected services from a browser and never opens the dashboard. +func TestAuthCallback_RecordsUserLogin(t *testing.T) { + setup := setupAuthCallbackTest(t) + defer setup.cleanup() + + ctx := context.Background() + + before, err := setup.store.GetUserByUserID(ctx, store.LockingStrengthNone, "allowedUserId") + require.NoError(t, err) + require.Nil(t, before.LastLogin, "fixture user starts with no login on record") + + setup.oidcServer.tokenSubject = "allowedUserId" + state := createTestState(t, setup.proxyService, "https://test-proxy.example.com/dashboard") + + req := httptest.NewRequest(http.MethodGet, "/reverse-proxy/callback?code=test-auth-code&state="+url.QueryEscape(state), nil) + rec := httptest.NewRecorder() + setup.router.ServeHTTP(rec, req) + require.Equal(t, http.StatusFound, rec.Code) + + after, err := setup.store.GetUserByUserID(ctx, store.LockingStrengthNone, "allowedUserId") + require.NoError(t, err) + require.NotNil(t, after.LastLogin, "a completed proxy SSO login must be recorded on the user") + require.WithinDuration(t, time.Now().UTC(), after.LastLogin.UTC(), time.Minute, "login should be stamped at sign-in time") +} + +// TestAuthCallback_DeniedUserLoginNotRecorded keeps the write on the granted +// path: a refused sign-in is not a login. +func TestAuthCallback_DeniedUserLoginNotRecorded(t *testing.T) { + setup := setupAuthCallbackTest(t) + defer setup.cleanup() + + ctx := context.Background() + + setup.oidcServer.tokenSubject = "blockedUserId" + state := createTestState(t, setup.proxyService, "https://test-proxy.example.com/dashboard") + + req := httptest.NewRequest(http.MethodGet, "/reverse-proxy/callback?code=test-auth-code&state="+url.QueryEscape(state), nil) + rec := httptest.NewRecorder() + setup.router.ServeHTTP(rec, req) + require.Equal(t, http.StatusFound, rec.Code) + + after, err := setup.store.GetUserByUserID(ctx, store.LockingStrengthNone, "blockedUserId") + require.NoError(t, err) + require.Nil(t, after.LastLogin, "a denied user must not be recorded as having logged in") +} + func TestAuthCallback_UserDeniedByAccountStatus(t *testing.T) { tests := []struct { name string diff --git a/management/server/peer/peer.go b/management/server/peer/peer.go index 7c4971285..a8567a1fd 100644 --- a/management/server/peer/peer.go +++ b/management/server/peer/peer.go @@ -477,6 +477,13 @@ func sameMultiset[T comparable](a, b []T) bool { return len(counts) == 0 } +// CountsTowardActivity reports whether the peer represents a device a person +// actually runs. Embedded proxy peers are infrastructure and browser (WASM) +// clients are ephemeral sessions, so activity accounting ignores both. +func (p *Peer) CountsTowardActivity() bool { + return !p.ProxyMeta.Embedded && p.Meta.KernelVersion != "wasm" +} + // GetLastLogin returns the last login time of the peer. func (p *Peer) GetLastLogin() time.Time { if p.LastLogin != nil { diff --git a/management/server/store/sql_store.go b/management/server/store/sql_store.go index 670d9f781..3578a3ff2 100644 --- a/management/server/store/sql_store.go +++ b/management/server/store/sql_store.go @@ -599,6 +599,27 @@ func (s *SqlStore) ApproveAccountPeers(ctx context.Context, accountID string) (i return int(result.RowsAffected), nil } +// RefreshPeerLastSeen updates only peer_status_last_seen. Every other status +// column is left untouched: peer_status_connected and +// peer_status_session_started_at belong to the sync stream that owns the +// session, and a blind write here would corrupt the fencing +// MarkPeerConnectedIfNewerSession relies on. +func (s *SqlStore) RefreshPeerLastSeen(ctx context.Context, accountID, peerID string, seenAt time.Time) error { + if seenAt.IsZero() { + return nil + } + + result := s.db.WithContext(ctx). + Model(&nbpeer.Peer{}). + Where(accountAndIDQueryCondition, accountID, peerID). + Update("peer_status_last_seen", seenAt) + if result.Error != nil { + return status.Errorf(status.Internal, "refresh peer last seen: %v", result.Error) + } + + return nil +} + // SaveUsers saves the given list of users to the database. func (s *SqlStore) SaveUsers(ctx context.Context, users []*types.User) error { if len(users) == 0 { @@ -3001,6 +3022,26 @@ func (s *SqlStore) SaveUserLastLogin(ctx context.Context, accountID, userID stri return nil } +// RefreshUserLastLogin updates only the last_login column, and only when it +// moves the timestamp forward. A user who has never logged in has a NULL that +// must also be written, so it counts as older than anything. +func (s *SqlStore) RefreshUserLastLogin(ctx context.Context, accountID, userID string, loginAt time.Time) error { + if loginAt.IsZero() { + return nil + } + + result := s.db.WithContext(ctx). + Model(&types.User{}). + Where(accountAndIDQueryCondition, accountID, userID). + Where("last_login IS NULL OR last_login < ?", loginAt). + Update("last_login", loginAt) + if result.Error != nil { + return status.Errorf(status.Internal, "refresh user last login: %v", result.Error) + } + + return nil +} + func (s *SqlStore) GetPostureCheckByChecksDefinition(accountID string, checks *posture.ChecksDefinition) (*posture.Checks, error) { definitionJSON, err := json.Marshal(checks) if err != nil { diff --git a/management/server/store/sql_store_activity_test.go b/management/server/store/sql_store_activity_test.go new file mode 100644 index 000000000..596139394 --- /dev/null +++ b/management/server/store/sql_store_activity_test.go @@ -0,0 +1,166 @@ +package store + +import ( + "context" + "net/netip" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + nbpeer "github.com/netbirdio/netbird/management/server/peer" + "github.com/netbirdio/netbird/management/server/types" +) + +const activityAccountID = "activityAccountId" + +func newActivityTestStore(t *testing.T) Store { + t.Helper() + + store, cleanUp, err := NewTestStoreFromSQL(context.Background(), "", t.TempDir()) + require.NoError(t, err) + t.Cleanup(cleanUp) + + require.NoError(t, store.SaveAccount(context.Background(), &types.Account{ + Id: activityAccountID, + Domain: "activity.example.com", + CreatedAt: time.Now().UTC(), + })) + + return store +} + +func TestRefreshUserLastLogin(t *testing.T) { + ctx := context.Background() + base := time.Date(2026, 3, 1, 12, 0, 0, 0, time.UTC) + + tests := []struct { + name string + stored *time.Time + loginAt time.Time + expect *time.Time + }{ + { + // A user who has only ever reached proxy services has no login on + // record at all, and activity accounting skips those users. + name: "never logged in gets the timestamp", + stored: nil, + loginAt: base, + expect: &base, + }, + { + name: "older timestamp moves forward", + stored: ptrTime(base.Add(-2 * time.Hour)), + loginAt: base, + expect: &base, + }, + { + name: "newer timestamp is left alone", + stored: ptrTime(base.Add(time.Hour)), + loginAt: base, + expect: ptrTime(base.Add(time.Hour)), + }, + { + name: "zero login is ignored", + stored: ptrTime(base), + loginAt: time.Time{}, + expect: ptrTime(base), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + store := newActivityTestStore(t) + + require.NoError(t, store.SaveUser(ctx, &types.User{ + Id: "activityUser", + AccountID: activityAccountID, + Role: types.UserRoleUser, + Issued: "api", + LastLogin: tt.stored, + CreatedAt: base.Add(-24 * time.Hour), + })) + + require.NoError(t, store.RefreshUserLastLogin(ctx, activityAccountID, "activityUser", tt.loginAt)) + + user, err := store.GetUserByUserID(ctx, LockingStrengthNone, "activityUser") + require.NoError(t, err) + require.NotNil(t, user.LastLogin, "user should have a login timestamp") + assert.WithinDuration(t, *tt.expect, user.LastLogin.UTC(), time.Second, "unexpected stored last login") + }) + } +} + +func TestRefreshUserLastLoginUnknownUserIsNotAnError(t *testing.T) { + ctx := context.Background() + store := newActivityTestStore(t) + + // The write is best-effort telemetry on an auth path; a row that no longer + // exists must not surface as a failure to the caller. + assert.NoError(t, store.RefreshUserLastLogin(ctx, activityAccountID, "goneUser", time.Now().UTC())) +} + +func TestRefreshPeerLastSeen(t *testing.T) { + ctx := context.Background() + store := newActivityTestStore(t) + require.NoError(t, store.AddPeerToAccount(ctx, activityPeer(time.Date(2026, 3, 1, 9, 0, 0, 0, time.UTC)))) + + seenAt := time.Date(2026, 3, 1, 12, 0, 0, 0, time.UTC) + require.NoError(t, store.RefreshPeerLastSeen(ctx, activityAccountID, "activityPeer", seenAt)) + + peer, err := store.GetPeerByID(ctx, LockingStrengthNone, activityAccountID, "activityPeer") + require.NoError(t, err) + assert.WithinDuration(t, seenAt, peer.Status.LastSeen.UTC(), time.Second, "unexpected stored last seen") +} + +func TestRefreshPeerLastSeenZeroIsIgnored(t *testing.T) { + ctx := context.Background() + store := newActivityTestStore(t) + stored := time.Date(2026, 3, 1, 9, 0, 0, 0, time.UTC) + require.NoError(t, store.AddPeerToAccount(ctx, activityPeer(stored))) + + require.NoError(t, store.RefreshPeerLastSeen(ctx, activityAccountID, "activityPeer", time.Time{})) + + peer, err := store.GetPeerByID(ctx, LockingStrengthNone, activityAccountID, "activityPeer") + require.NoError(t, err) + assert.WithinDuration(t, stored, peer.Status.LastSeen.UTC(), time.Second, "a zero timestamp must not clear last seen") +} + +// TestRefreshPeerLastSeenLeavesSessionStateAlone pins the column boundary: the +// connected flag and the session token belong to the sync stream that owns the +// peer's session, and a blind write here would corrupt its fencing. +func TestRefreshPeerLastSeenLeavesSessionStateAlone(t *testing.T) { + ctx := context.Background() + store := newActivityTestStore(t) + + stored := activityPeer(time.Date(2026, 3, 1, 9, 0, 0, 0, time.UTC)) + stored.Status.Connected = true + stored.Status.SessionStartedAt = 1234567890 + require.NoError(t, store.AddPeerToAccount(ctx, stored)) + + seenAt := time.Date(2026, 3, 1, 12, 0, 0, 0, time.UTC) + require.NoError(t, store.RefreshPeerLastSeen(ctx, activityAccountID, "activityPeer", seenAt)) + + peer, err := store.GetPeerByID(ctx, LockingStrengthNone, activityAccountID, "activityPeer") + require.NoError(t, err) + assert.WithinDuration(t, seenAt, peer.Status.LastSeen.UTC(), time.Second, "last seen should move forward") + assert.True(t, peer.Status.Connected, "connected flag must survive an activity write") + assert.Equal(t, int64(1234567890), peer.Status.SessionStartedAt, "session token must survive an activity write") +} + +func activityPeer(lastSeen time.Time) *nbpeer.Peer { + return &nbpeer.Peer{ + ID: "activityPeer", + AccountID: activityAccountID, + Key: "activityPeerKey", + IP: netip.MustParseAddr("100.64.0.9"), + Name: "activity-peer", + DNSLabel: "activity-peer", + Status: &nbpeer.PeerStatus{LastSeen: lastSeen}, + } +} + +func ptrTime(t time.Time) *time.Time { + return &t +} diff --git a/management/server/store/store.go b/management/server/store/store.go index 1beea72fd..ebd6bcc3c 100644 --- a/management/server/store/store.go +++ b/management/server/store/store.go @@ -94,6 +94,11 @@ type Store interface { SaveUsers(ctx context.Context, users []*types.User) error SaveUser(ctx context.Context, user *types.User) error SaveUserLastLogin(ctx context.Context, accountID, userID string, lastLogin time.Time) error + // RefreshUserLastLogin moves a user's last login forward to loginAt, + // touching no other column and leaving a newer stored value alone. Used by + // login paths that only need the timestamp, so they neither read the row + // first nor rewrite fields they did not change. + RefreshUserLastLogin(ctx context.Context, accountID, userID string, loginAt time.Time) error DeleteUser(ctx context.Context, accountID, userID string) error GetTokenIDByHashedToken(ctx context.Context, secret string) (string, error) DeleteHashedPAT2TokenIDIndex(hashedToken string) error @@ -180,6 +185,11 @@ type Store interface { // Returns true when the update happened, false when this stream lost // the race against a newer session. MarkPeerConnectedIfNewerSession(ctx context.Context, accountID, peerID string, newSessionStartedAt int64) (bool, error) + // RefreshPeerLastSeen records that a peer was seen at seenAt. Connected and + // SessionStartedAt are left alone, so this never interferes with the + // session-ownership protocol MarkPeerConnectedIfNewerSession implements. + // Callers decide how often to call it; the store does not throttle. + RefreshPeerLastSeen(ctx context.Context, accountID, peerID string, seenAt time.Time) error // MarkPeerDisconnectedIfSameSession sets the peer to disconnected and // resets SessionStartedAt to zero, but only when the stored // SessionStartedAt equals the given sessionStartedAt. LastSeen is diff --git a/management/server/store/store_mock.go b/management/server/store/store_mock.go index 428632a86..2fe261547 100644 --- a/management/server/store/store_mock.go +++ b/management/server/store/store_mock.go @@ -3203,6 +3203,34 @@ func (mr *MockStoreMockRecorder) MarkProxyAccessTokenUsed(ctx, tokenID interface return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "MarkProxyAccessTokenUsed", reflect.TypeOf((*MockStore)(nil).MarkProxyAccessTokenUsed), ctx, tokenID) } +// RefreshPeerLastSeen mocks base method. +func (m *MockStore) RefreshPeerLastSeen(ctx context.Context, accountID, peerID string, seenAt time.Time) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "RefreshPeerLastSeen", ctx, accountID, peerID, seenAt) + ret0, _ := ret[0].(error) + return ret0 +} + +// RefreshPeerLastSeen indicates an expected call of RefreshPeerLastSeen. +func (mr *MockStoreMockRecorder) RefreshPeerLastSeen(ctx, accountID, peerID, seenAt interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RefreshPeerLastSeen", reflect.TypeOf((*MockStore)(nil).RefreshPeerLastSeen), ctx, accountID, peerID, seenAt) +} + +// RefreshUserLastLogin mocks base method. +func (m *MockStore) RefreshUserLastLogin(ctx context.Context, accountID, userID string, loginAt time.Time) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "RefreshUserLastLogin", ctx, accountID, userID, loginAt) + ret0, _ := ret[0].(error) + return ret0 +} + +// RefreshUserLastLogin indicates an expected call of RefreshUserLastLogin. +func (mr *MockStoreMockRecorder) RefreshUserLastLogin(ctx, accountID, userID, loginAt interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RefreshUserLastLogin", reflect.TypeOf((*MockStore)(nil).RefreshUserLastLogin), ctx, accountID, userID, loginAt) +} + // RemovePeerFromAllGroups mocks base method. func (m *MockStore) RemovePeerFromAllGroups(ctx context.Context, peerID string) error { m.ctrl.T.Helper() diff --git a/management/server/users/manager.go b/management/server/users/manager.go index 1a05b1a7c..9f6a74df6 100644 --- a/management/server/users/manager.go +++ b/management/server/users/manager.go @@ -3,6 +3,7 @@ package users import ( "context" "errors" + "time" "github.com/netbirdio/netbird/management/server/store" "github.com/netbirdio/netbird/management/server/types" @@ -11,6 +12,9 @@ import ( type Manager interface { GetUser(ctx context.Context, userID string) (*types.User, error) GetUserWithGroups(ctx context.Context, userID string) (*types.User, []*types.Group, error) + // RefreshLastLogin records an interactive login on the user without + // rewriting the rest of the row, keeping a newer stored timestamp. + RefreshLastLogin(ctx context.Context, accountID, userID string, loginAt time.Time) error } type managerImpl struct { @@ -26,6 +30,10 @@ func NewManager(store store.Store) Manager { } } +func (m *managerImpl) RefreshLastLogin(ctx context.Context, accountID, userID string, loginAt time.Time) error { + return m.store.RefreshUserLastLogin(ctx, accountID, userID, loginAt) +} + func (m *managerImpl) GetUser(ctx context.Context, userID string) (*types.User, error) { return m.store.GetUserByUserID(ctx, store.LockingStrengthNone, userID) } @@ -74,6 +82,10 @@ func (m *managerMock) GetUser(ctx context.Context, userID string) (*types.User, } } +func (m *managerMock) RefreshLastLogin(_ context.Context, _, _ string, _ time.Time) error { + return nil +} + func (m *managerMock) GetUserWithGroups(ctx context.Context, userID string) (*types.User, []*types.Group, error) { user, err := m.GetUser(ctx, userID) if err != nil {