mirror of
https://github.com/netbirdio/netbird.git
synced 2026-10-01 19:19:07 +02:00
[management] Move the activity policy out of the gRPC service
Recording proxy usage is business logic, and it had ended up in the RPC handler: the throttle interval, the service-user skip, the exclusion rule for embedded and browser peers, and a store handle to write through. It moves to a reverseproxy module manager, matching how accesslogs, domain, service and proxy are already structured, and the RPC keeps only what is its own: calling the manager and deciding the request must not fail when the write does. The proxy service goes back to holding ProxyTokenChecker rather than a widened store interface. The policy tests move with the policy. The handler tests now assert only that a granted request reaches the manager, which is all the transport decides.
This commit is contained in:
mlsmaycon
@@ -0,0 +1,25 @@
|
||||
// Package activity records that a principal used a reverse proxy service, so
|
||||
// that activity accounting counts people and devices which reach services
|
||||
// through the proxy but never touch the dashboard or the management API.
|
||||
package activity
|
||||
|
||||
import (
|
||||
"context"
|
||||
|
||||
"github.com/netbirdio/netbird/management/server/peer"
|
||||
"github.com/netbirdio/netbird/management/server/types"
|
||||
)
|
||||
|
||||
// Manager records reverse proxy usage against the timestamps activity
|
||||
// accounting reads. Both methods are best effort from the caller's point of
|
||||
// view: a lost record is corrected by the next request, and no authorization
|
||||
// decision reads them back.
|
||||
type Manager interface {
|
||||
// RecordUserLogin records a completed SSO sign-in to a proxied service.
|
||||
// Service users have no interactive login and are ignored.
|
||||
RecordUserLogin(ctx context.Context, accountID string, user *types.User) error
|
||||
// RecordPeerSeen records that a peer reached a private service over the
|
||||
// mesh, which is what lets its owner count as active. Peers activity
|
||||
// accounting excludes, and peers already seen recently, are ignored.
|
||||
RecordPeerSeen(ctx context.Context, accountID string, peer *peer.Peer) error
|
||||
}
|
||||
@@ -0,0 +1,61 @@
|
||||
package manager
|
||||
|
||||
import (
|
||||
"context"
|
||||
"time"
|
||||
|
||||
"github.com/netbirdio/netbird/management/internals/modules/reverseproxy/activity"
|
||||
"github.com/netbirdio/netbird/management/server/peer"
|
||||
"github.com/netbirdio/netbird/management/server/store"
|
||||
"github.com/netbirdio/netbird/management/server/types"
|
||||
)
|
||||
|
||||
// peerSeenInterval 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
|
||||
// behind every request; an hour still sits well inside the window activity
|
||||
// accounting asks about.
|
||||
const peerSeenInterval = time.Hour
|
||||
|
||||
type managerImpl struct {
|
||||
store store.Store
|
||||
}
|
||||
|
||||
// NewManager returns the activity manager backed by the management store.
|
||||
func NewManager(store store.Store) activity.Manager {
|
||||
return &managerImpl{store: store}
|
||||
}
|
||||
|
||||
// RecordUserLogin stamps the login the same way the dashboard and device login
|
||||
// paths do, so a person who only ever reaches proxied services still has a
|
||||
// login on record.
|
||||
func (m *managerImpl) RecordUserLogin(ctx context.Context, accountID string, user *types.User) error {
|
||||
if user == nil || user.IsServiceUser {
|
||||
return nil
|
||||
}
|
||||
|
||||
return m.store.SaveUserLastLogin(ctx, accountID, user.Id, time.Now().UTC())
|
||||
}
|
||||
|
||||
// RecordPeerSeen stamps LastSeen, the column a peer activates its owner
|
||||
// through. The throttle reads the peer the caller already holds, so a peer seen
|
||||
// inside the interval costs nothing to skip.
|
||||
func (m *managerImpl) RecordPeerSeen(ctx context.Context, accountID string, peer *peer.Peer) error {
|
||||
if peer == nil || !countsTowardActivity(peer) {
|
||||
return nil
|
||||
}
|
||||
|
||||
if peer.Status != nil && time.Since(peer.Status.LastSeen) < peerSeenInterval {
|
||||
return nil
|
||||
}
|
||||
|
||||
return m.store.RefreshPeerLastSeen(ctx, accountID, peer.ID)
|
||||
}
|
||||
|
||||
// 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 and a
|
||||
// write for them could never count.
|
||||
func countsTowardActivity(peer *peer.Peer) bool {
|
||||
return !peer.ProxyMeta.Embedded && peer.Meta.KernelVersion != "wasm"
|
||||
}
|
||||
@@ -0,0 +1,145 @@
|
||||
package manager
|
||||
|
||||
import (
|
||||
"context"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
|
||||
"github.com/netbirdio/netbird/management/server/peer"
|
||||
"github.com/netbirdio/netbird/management/server/store"
|
||||
"github.com/netbirdio/netbird/management/server/types"
|
||||
)
|
||||
|
||||
// recordingStore captures the two writes the activity manager makes. The
|
||||
// embedded interface satisfies the rest and panics if anything else is called,
|
||||
// which keeps the manager honest about its surface.
|
||||
type recordingStore struct {
|
||||
store.Store
|
||||
logins []loginWrite
|
||||
seen []seenWrite
|
||||
}
|
||||
|
||||
type loginWrite struct {
|
||||
accountID string
|
||||
userID string
|
||||
at time.Time
|
||||
}
|
||||
|
||||
type seenWrite struct {
|
||||
accountID string
|
||||
peerID string
|
||||
}
|
||||
|
||||
func (s *recordingStore) SaveUserLastLogin(_ context.Context, accountID, userID string, lastLogin time.Time) error {
|
||||
s.logins = append(s.logins, loginWrite{accountID: accountID, userID: userID, at: lastLogin})
|
||||
return nil
|
||||
}
|
||||
|
||||
func (s *recordingStore) RefreshPeerLastSeen(_ context.Context, accountID, peerID string) error {
|
||||
s.seen = append(s.seen, seenWrite{accountID: accountID, peerID: peerID})
|
||||
return nil
|
||||
}
|
||||
|
||||
func TestRecordUserLogin(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
user *types.User
|
||||
expectWrite bool
|
||||
}{
|
||||
{
|
||||
name: "regular user is recorded",
|
||||
user: &types.User{Id: "user1", AccountID: "account1"},
|
||||
expectWrite: true,
|
||||
},
|
||||
{
|
||||
// Activity accounting never counts service users, so a row for one
|
||||
// would be noise.
|
||||
name: "service user is ignored",
|
||||
user: &types.User{Id: "svc1", AccountID: "account1", IsServiceUser: true},
|
||||
expectWrite: false,
|
||||
},
|
||||
{
|
||||
name: "missing user is ignored",
|
||||
user: nil,
|
||||
expectWrite: false,
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
st := &recordingStore{}
|
||||
require.NoError(t, NewManager(st).RecordUserLogin(context.Background(), "account1", tt.user))
|
||||
|
||||
if !tt.expectWrite {
|
||||
assert.Empty(t, st.logins, "no login should have been recorded")
|
||||
return
|
||||
}
|
||||
|
||||
require.Len(t, st.logins, 1, "exactly one login should have been recorded")
|
||||
assert.Equal(t, "account1", st.logins[0].accountID, "login must be recorded against the service account")
|
||||
assert.Equal(t, tt.user.Id, st.logins[0].userID, "login must be recorded against the signing-in user")
|
||||
assert.Equal(t, time.UTC, st.logins[0].at.Location(), "timestamps are written in UTC")
|
||||
assert.WithinDuration(t, time.Now().UTC(), st.logins[0].at, time.Minute, "login should be stamped now")
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestRecordPeerSeen(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
peer *peer.Peer
|
||||
expectWrite bool
|
||||
}{
|
||||
{
|
||||
name: "peer seen long ago is recorded",
|
||||
peer: &peer.Peer{ID: "peer1", Status: &peer.PeerStatus{LastSeen: time.Now().Add(-3 * time.Hour)}},
|
||||
expectWrite: true,
|
||||
},
|
||||
{
|
||||
name: "peer never seen is recorded",
|
||||
peer: &peer.Peer{ID: "peer1", Status: &peer.PeerStatus{}},
|
||||
expectWrite: true,
|
||||
},
|
||||
{
|
||||
// The throttle. The caller already holds the peer, so skipping a
|
||||
// recently seen one costs nothing.
|
||||
name: "peer seen inside the interval is skipped",
|
||||
peer: &peer.Peer{ID: "peer1", Status: &peer.PeerStatus{LastSeen: time.Now().Add(-10 * time.Minute)}},
|
||||
expectWrite: false,
|
||||
},
|
||||
{
|
||||
name: "embedded proxy peer is skipped",
|
||||
peer: &peer.Peer{ID: "peer1", ProxyMeta: peer.ProxyMeta{Embedded: true}, Status: &peer.PeerStatus{LastSeen: time.Now().Add(-3 * time.Hour)}},
|
||||
expectWrite: false,
|
||||
},
|
||||
{
|
||||
name: "browser client is skipped",
|
||||
peer: &peer.Peer{ID: "peer1", Meta: peer.PeerSystemMeta{KernelVersion: "wasm"}, Status: &peer.PeerStatus{LastSeen: time.Now().Add(-3 * time.Hour)}},
|
||||
expectWrite: false,
|
||||
},
|
||||
{
|
||||
name: "missing peer is ignored",
|
||||
peer: nil,
|
||||
expectWrite: false,
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
st := &recordingStore{}
|
||||
require.NoError(t, NewManager(st).RecordPeerSeen(context.Background(), "account1", tt.peer))
|
||||
|
||||
if !tt.expectWrite {
|
||||
assert.Empty(t, st.seen, "no activity should have been recorded")
|
||||
return
|
||||
}
|
||||
|
||||
require.Len(t, st.seen, 1, "exactly one activity write should have been recorded")
|
||||
assert.Equal(t, "account1", st.seen[0].accountID, "activity must be recorded against the service account")
|
||||
assert.Equal(t, tt.peer.ID, st.seen[0].peerID, "activity must be recorded against the calling peer")
|
||||
})
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user