Compare commits

...

13 Commits

Author SHA1 Message Date
mlsmaycon
b506c52023 [management] Record activity for a peer that was never seen
peer_status_last_seen is nullable — Status is an embedded pointer, so a peer
stored without one leaves the column NULL — and NULL loses the cutoff
comparison, so such a peer was silently skipped forever instead of recording
its first activity.
2026-08-09 11:06:48 +00:00
mlsmaycon
796b48e49c [management] Enforce the peer activity throttle inside the update
The manager checked LastSeen on the peer it already held and then issued an
unconditional UPDATE, so concurrent requests for one peer could each pass the
check off the same stale read and write. The cutoff now travels to the store
and lands in the statement's WHERE, matching how MarkPeerConnectedIfNewerSession
fences its own write, and the local check stays as the query-free fast path.
2026-08-09 10:39:13 +00:00
mlsmaycon
25b1081933 [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.
2026-08-09 08:20:40 +00:00
mlsmaycon
d2f93fcd90 [management] Confine the activity writes to the reverse proxy
The user half reused nothing: SaveUserLastLogin already exists and is the
same call the dashboard and device login paths make, so the parallel
RefreshUserLastLogin is gone and the proxy uses the established one.

Reaching it no longer widens shared interfaces. The proxy service already
receives the store, narrowed to ProxyTokenChecker; that interface now
carries the two writes the proxy makes, so users.Manager, peers.Manager and
Peer are untouched and the exclusion predicate moved into the proxy package
next to its only caller.

RefreshPeerLastSeen stays on the store because nothing there fits:
SavePeerStatus rewrites the connected flag and session token from a caller
snapshot, which would race the sync stream that owns them.
2026-08-09 08:11:30 +00:00
mlsmaycon
48d9161056 [management] Stamp proxy peer activity with the database clock
The activity write took a Go-side timestamp, which is exactly what
MarkPeerConnectedIfNewerSession documents as the cause of previous ordering
bugs: a value read before the write can land after a connect that stamped
CURRENT_TIMESTAMP, dragging LastSeen backwards.

The write now uses the database clock like the other status writers, so the
column only ever moves forward. The throttle is unaffected; it reads the
peer already in hand and never needed the write's timestamp.
2026-08-09 07:38:35 +00:00
mlsmaycon
356f6bdda0 [management] Record proxy logins and mesh activity for active-user accounting
Activity accounting counts a user as active from their last login or from a
peer of theirs being seen. Neither timestamp moved when someone reached a
service through the reverse proxy, so a person who only ever uses
proxy-protected services and never opens the dashboard has no login on
record at all and is skipped outright.

The two proxy entry points mean different things, so they write different
things. GenerateSessionToken is only reached after an ID token was verified,
so a completed SSO sign-in records a login on the user. ValidateTunnelPeer
authorises by tunnel IP with no IdP involved, so it records that the peer
was seen instead; the owner counts through that. Both write on the granted
path only, in UTC, and log and drop failures — no authorisation decision
reads them back.

The peer write is throttled to once an hour against the peer already in
hand, so a busy peer does not rewrite its row behind every request. Both
store methods update one column and leave the session-ownership fields to
the sync stream that owns them.

Peers that accounting excludes, embedded proxy peers and browser clients,
are skipped rather than written for nothing.
2026-08-09 07:31:18 +00:00
Maycon Santos
f65f7b347e [management] Deny reverse proxy access to pending and blocked users (#7105)
A user in the Pending Approval state could complete SSO and reach any
SSO-protected reverse proxy service distributed to a group they belong
to, including the All Users group. The reverse proxy authorization path
checked the session token signature, that the user exists, that the
user's account matches the service's account, and group membership —
never the user's account status. The REST API (`permissions/manager.go`)
and peer registration both gate on that state, but the proxy gRPC
service does not go through the permissions manager, so neither gate
applied. A pending user is persisted as blocked and pending approval, so
blocked users reached those services the same way.

`ValidateSession` now denies on account status, reporting
`pending_approval` or `user_blocked` so the proxy access log and the
denied page carry the cause rather than a generic refusal.
`GenerateSessionToken` refuses to mint a token for such a user at all,
so the browser never receives a session cookie and the OIDC callback can
tell the user why instead of showing "Service configuration error".
`ValidateUserGroupAccess` and `ValidateTunnelPeer` close the same gap;
for the tunnel path this covers a user blocked after their peer was
registered, since peer group membership alone kept mesh-origin access
open.

A single helper produces both the denied reason for the RPC responses
and the sentinel error for the error-returning callers, so the four
entry points cannot drift apart. A user the store cannot resolve is
denied rather than passed through.

One thing deliberately left out: session cookies are validated locally
by the proxy against the service public key with no management
round-trip, so a cookie issued before a user is blocked stays valid
until it expires (24h by default). That is a revocation-propagation
problem rather than this authorization gap, and every option for it
(per-request validation with a cache, short-lived tokens with refresh,
push-based revocation) changes the proxy hot path or the
proxy/management protocol. Worth its own ticket.
2026-08-08 20:48:34 +09:00
Maycon Santos
179e8f6e13 [infrastructure] add grafana dashboard for licensed management (#7095) 2026-08-08 15:04:06 +09:00
Pascal Fischer
2ee21d2b5c [management] Affected peers for user updates (#7099) 2026-08-07 18:07:53 +02:00
Riccardo Manfrin
eb619fc7e3 [client] disambiguate the connection_type metric tag (#7043)
## Describe your changes

`recordConnectionMetrics` mapped only `conntype.Relay` to `relay` and
let a `default` branch
record everything else as `ice`. That silently included `ICETurn` — an
ICE connection through
a TURN server, which
[`conn.isRelayed`](https://github.com/netbirdio/netbird/blob/main/client/internal/peer/conn.go#L788-L795)
itself counts as relayed — and `None`, the transient state set when the
relay drops

([conn.go:632](https://github.com/netbirdio/netbird/blob/main/client/internal/peer/conn.go#L632))
or the peer state is reset
([conn.go:757](https://github.com/netbirdio/netbird/blob/main/client/internal/peer/conn.go#L757)).
Both were reported as direct peer-to-peer, so the `ice` share overstated
direct connections on
every platform.

The mapping now lists every priority explicitly and emits `ice_p2p`,
`ice_turn`, `relay` or
`unknown`. The new values deliberately do not reuse `ice` to avoid
ambuguity.


## Issue ticket number and link

No public issue. Found while reviewing the first production sample of
client metrics: 38% of iOS
connection events were tagged `ice` on a platform that forces relay by
default, which traced back
to the `default` branch at
[client/internal/peer/conn.go#L963-L968](https://github.com/netbirdio/netbird/blob/main/client/internal/peer/conn.go#L963-L968).

## Stack

<!-- branch-stack -->

### Checklist
- [x] Is it a bug fix
- [ ] Is a typo/documentation fix
- [ ] Is a feature enhancement
- [ ] It is a refactor
- [x] Created tests that fail without the change (if possible)

> By submitting this pull request, you confirm that you have read and
agree to the terms of the [Contributor License
Agreement](https://github.com/netbirdio/netbird/blob/main/CONTRIBUTOR_LICENSE_AGREEMENT.md).

## Documentation
Select exactly one:

- [] I added/updated documentation for this change
- [x] Documentation is **not needed** for this change (explain why)

Internal metrics documentation only, in
`client/internal/metrics/infra/README.md`: the four
`connection_type` values with their derivation, and a note that pre-fix
`ice` samples are not
comparable with `ice_p2p`. No public API, CLI or configuration change,
so no netbirdio/docs PR.

### Docs PR URL (required if "docs added" is checked)
Paste the PR link from https://github.com/netbirdio/docs here:

N/A

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

## Summary by CodeRabbit

* **New Features**
* Connection metrics now distinguish direct peer-to-peer, TURN-assisted,
relay, and unknown connection types.
  * Metrics include clearer connection and peer identification details.

* **Documentation**
* Updated connection timing metric values, traffic semantics, priority
behavior, and historical data guidance.

* **Bug Fixes**
* Unset or unrecognized connection priorities are no longer incorrectly
classified as peer-to-peer.
* Unknown-transport metrics are skipped to prevent misleading connection
data.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
2026-08-07 17:11:05 +02:00
Nicolas Frati
8632a0d215 [infrastructure] Detect community GHCR images during enterprise migration (#7101)
The migration wizard detected the community deployment only by the
Docker Hub image prefix (netbirdio/netbird-server), so deployments
installed from the ghcr.io mirror failed with "Could not find a service
running netbirdio/netbird-server*".

This broadens the server and dashboard detection to also accept
ghcr.io/netbirdio/... images. The regexes are anchored at the tag/digest
separator so Enterprise images (netbird-server-cloud, dashboard-cloud)
are still rejected, an already-migrated deployment must not be detected
as a community one. Error messages updated to mention both forms.
2026-08-07 17:04:58 +02:00
Riccardo Manfrin
f63fd21e0c [client] peer: re-arm the WireGuard watcher after a lazy wake (#7091)
## Describe your changes

The Conn struct is reused across lazy-connection deactivate/activate.
Close
cancels the WireGuard watcher (via wgWatcherCancel, and ctxCancel also
tears
down its context) but left conn.wgWatcher pointing at the stopped
instance.
enableWgWatcherIfNeeded skips while conn.wgWatcher is non-nil, so the
next Open
never started a fresh watcher: once a lazy connection had idled and
woken, the
peer ran with no watcher at all — no WireGuard handshake-timeout
detection and
none of the escalation that depends on it.

Clear conn.wgWatcher and conn.wgWatcherCancel in Close so the next Open
re-arms
a fresh watcher.

## Issue ticket number and link

<!--
Required for anything that changes behavior. Link the issue (or the
validated
discussion it came from) that the NetBird team already agreed on. See

https://github.com/netbirdio/netbird/blob/main/CONTRIBUTING.md#ticket-first-pr-second
-->

## Stack

<!-- branch-stack -->

### Checklist
- [X] Is it a bug fix
- [ ] Is a typo/documentation fix
- [ ] Is a feature enhancement
- [ ] It is a refactor
- [ ] Created tests that fail without the change (if possible)
- [ ] I ran and tested this change locally — I did not rely on CI to
find out whether it works
- [ ] This PR has a single purpose (not a fix + refactor + feature in
one)
- [ ] This change is a trivial fix, **OR** it links an issue the NetBird
team agreed on beforehand. Changes to the public API, gRPC protocols,
functionality behavior, CLI / service flags, or new features always need
that agreement first. See
[CONTRIBUTING.md](https://github.com/netbirdio/netbird/blob/main/CONTRIBUTING.md#ticket-first-pr-second).

> By submitting this pull request, you confirm that you have read and
agree to the terms of the [Contributor License
Agreement](https://github.com/netbirdio/netbird/blob/main/CONTRIBUTOR_LICENSE_AGREEMENT.md).

## Documentation
Select exactly one:

- [ ] I added/updated documentation for this change
- [X] Documentation is **not needed** for this change (explain why)

### Docs PR URL (required if "docs added" is checked)
Paste the PR link from https://github.com/netbirdio/docs here:

https://github.com/netbirdio/docs/pull/__


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Bug Fixes**
* Improved connection cleanup by fully releasing WireGuard watcher
resources when a connection closes.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
2026-08-07 17:01:08 +02:00
Pascal Fischer
524b8b9718 [management] prewarm a posture check cache on network map generation (#7093) 2026-08-07 15:03:40 +02:00
32 changed files with 11047 additions and 179 deletions

View File

@@ -4,11 +4,17 @@ package metrics
type ConnectionType string
const (
// ConnectionTypeICE represents a direct peer-to-peer connection using ICE
ConnectionTypeICE ConnectionType = "ice"
// ConnectionTypeICEP2P represents a direct peer-to-peer connection using ICE
ConnectionTypeICEP2P ConnectionType = "ice_p2p"
// ConnectionTypeICETurn represents an ICE connection through a TURN server
ConnectionTypeICETurn ConnectionType = "ice_turn"
// ConnectionTypeRelay represents a relayed connection
ConnectionTypeRelay ConnectionType = "relay"
// ConnectionTypeUnknown represents a connection with no active transport. It is not pushed.
ConnectionTypeUnknown ConnectionType = "unknown"
)
// String returns the string representation of the connection type

View File

@@ -28,7 +28,7 @@ func TestInfluxDBMetrics_RecordAndExport(t *testing.T) {
WgHandshakeSuccess: time.Now().Add(-1 * time.Second),
}
m.RecordConnectionStages(context.Background(), agentInfo, "pair123", ConnectionTypeICE, false, ts)
m.RecordConnectionStages(context.Background(), agentInfo, "pair123", ConnectionTypeICEP2P, false, ts)
var buf bytes.Buffer
err := m.Export(&buf)
@@ -60,7 +60,7 @@ func TestInfluxDBMetrics_ExportDeterministicFieldOrder(t *testing.T) {
// Record multiple times and verify consistent field order
for i := 0; i < 10; i++ {
m.RecordConnectionStages(context.Background(), agentInfo, "pair123", ConnectionTypeICE, false, ts)
m.RecordConnectionStages(context.Background(), agentInfo, "pair123", ConnectionTypeICEP2P, false, ts)
}
var buf bytes.Buffer

View File

@@ -56,14 +56,33 @@ Measurement: `netbird_peer_connection`
Tags:
- `deployment_type`: "cloud" | "selfhosted" | "unknown"
- `connection_type`: "ice" | "relay"
- `connection_type`: "ice_p2p" | "ice_turn" | "relay" (see below)
- `attempt_type`: "initial" | "reconnection"
- `version`: NetBird version string
- `os`: Operating system (linux, darwin, windows, android, ios, etc.)
- `arch`: CPU architecture (amd64, arm64, etc.)
- `peer_id`: anonymised peer identifier (truncated SHA-256 of the WireGuard public key)
- `connection_pair_id`: deterministic identifier for the peer pair, identical on both sides
**Note:** `SignalingReceived` is set when the first offer or answer arrives from the remote peer (in both initial and reconnection paths). It excludes the potentially unbounded wait for the remote peer to come online.
#### `connection_type` values
Derived from the connection priority (`conntype.ConnPriority`) by `metricsConnType` in `client/internal/peer/conn.go`:
| Value | Priority | Traffic is |
|-------|----------|------------|
| `ice_p2p` | `ICEP2P` | direct peer-to-peer |
| `ice_turn` | `ICETurn` | relayed, through a TURN server |
| `relay` | `Relay` | relayed, through a NetBird relay |
| `unknown` | `None` or unrecognised | no active transport — **the sample is not pushed** |
**Direct traffic is `ice_p2p` only.** `ice_turn` is relayed despite being negotiated by ICE, matching `Conn.isRelayed`.
`None` means no transport is active: not established yet, or reset after a relay drop or a peer-state reset. Such a sample cannot be attributed to a transport, so `recordConnectionMetrics` drops it instead of pushing it — `unknown` therefore never appears in the bucket. Connection counts are counts of connections whose transport was known at sampling time.
**Samples recorded before 0.77 used a single `ice` value** which covered `ICEP2P`, `ICETurn` *and* `None`, so historical `ice` samples overstate direct connections by an unknown amount and must not be compared with `ice_p2p`.
### Sync Duration
Measurement: `netbird_sync`

View File

@@ -307,6 +307,8 @@ func (conn *Conn) Close(signalToRemote bool) {
if conn.wgWatcherCancel != nil {
conn.wgWatcherCancel()
conn.wgWatcher = nil
conn.wgWatcherCancel = nil
}
conn.workerRelay.CloseConn()
if conn.workerICE != nil {
@@ -959,12 +961,9 @@ func (conn *Conn) recordConnectionMetrics() {
priority := conn.currentConnPriority
conn.mu.Unlock()
var connType metrics.ConnectionType
switch priority {
case conntype.Relay:
connType = metrics.ConnectionTypeRelay
default:
connType = metrics.ConnectionTypeICE
connType := metricsConnType(priority)
if connType == metrics.ConnectionTypeUnknown {
return
}
// Record metrics with timestamps - duration calculation happens in metrics package
@@ -1065,3 +1064,16 @@ func boolToConnStatus(connected bool) guard.ConnStatus {
}
return guard.ConnStatusDisconnected
}
func metricsConnType(priority conntype.ConnPriority) metrics.ConnectionType {
switch priority {
case conntype.Relay:
return metrics.ConnectionTypeRelay
case conntype.ICETurn:
return metrics.ConnectionTypeICETurn
case conntype.ICEP2P:
return metrics.ConnectionTypeICEP2P
default:
return metrics.ConnectionTypeUnknown
}
}

View File

@@ -11,6 +11,8 @@ import (
"github.com/stretchr/testify/assert"
"github.com/netbirdio/netbird/client/iface"
"github.com/netbirdio/netbird/client/internal/metrics"
"github.com/netbirdio/netbird/client/internal/peer/conntype"
"github.com/netbirdio/netbird/client/internal/peer/dispatcher"
"github.com/netbirdio/netbird/client/internal/peer/guard"
"github.com/netbirdio/netbird/client/internal/peer/ice"
@@ -386,3 +388,33 @@ func TestConn_onWGDisconnected_NoEscalationWithoutRosenpass(t *testing.T) {
}
assert.Empty(t, disconnected, "escalation must be limited to rosenpass connections")
}
func TestMetricsConnType(t *testing.T) {
tests := []struct {
name string
priority conntype.ConnPriority
expected metrics.ConnectionType
}{
{"relay", conntype.Relay, metrics.ConnectionTypeRelay},
{"ice over turn is relayed, not p2p", conntype.ICETurn, metrics.ConnectionTypeICETurn},
{"direct p2p", conntype.ICEP2P, metrics.ConnectionTypeICEP2P},
{"unset priority is unknown, not p2p", conntype.None, metrics.ConnectionTypeUnknown},
{"unrecognised priority is unknown", conntype.ConnPriority(99), metrics.ConnectionTypeUnknown},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
assert.Equal(t, tc.expected, metricsConnType(tc.priority))
})
}
}
func TestMetricsConnType_RelayedMatchesIsRelayed(t *testing.T) {
for _, priority := range []conntype.ConnPriority{conntype.None, conntype.Relay, conntype.ICETurn, conntype.ICEP2P} {
conn := &Conn{currentConnPriority: priority}
tag := metricsConnType(priority)
relayedTag := tag == metrics.ConnectionTypeRelay || tag == metrics.ConnectionTypeICETurn
assert.Equal(t, conn.isRelayed(), relayedTag,
"priority %s: isRelayed and the %q metric tag must agree", priority, tag)
}
}

View File

@@ -173,11 +173,11 @@ EOF
# ---------------------------------------------------------------------------
detect_combined_service() {
yq eval '.services | to_entries | map(select(.value.image | test("^netbirdio/netbird-server"))) | .[0].key // ""' "$COMPOSE_FILE"
yq eval '.services | to_entries | map(select(.value.image | test("^(ghcr\\.io/)?netbirdio/netbird-server([:@]|$)"))) | .[0].key // ""' "$COMPOSE_FILE"
}
detect_dashboard_service() {
yq eval '.services | to_entries | map(select(.value.image | test("^netbirdio/dashboard"))) | .[0].key // ""' "$COMPOSE_FILE"
yq eval '.services | to_entries | map(select(.value.image | test("^(ghcr\\.io/)?netbirdio/dashboard([:@]|$)"))) | .[0].key // ""' "$COMPOSE_FILE"
}
detect_config_yaml_host_path() {
@@ -661,12 +661,12 @@ init_migration() {
COMPOSE_NETWORK=$(detect_compose_network)
if [[ -z "$COMBINED_SERVICE" ]]; then
echo "Could not find a service running netbirdio/netbird-server* in $COMPOSE_FILE." > /dev/stderr
echo "Could not find a service running netbirdio/netbird-server or ghcr.io/netbirdio/netbird-server in $COMPOSE_FILE." > /dev/stderr
echo "This script targets the community combined-server deployment." > /dev/stderr
exit 1
fi
if [[ -z "$DASHBOARD_SERVICE" ]]; then
echo "Could not find a service running netbirdio/dashboard* in $COMPOSE_FILE." > /dev/stderr
echo "Could not find a service running netbirdio/dashboard or ghcr.io/netbirdio/dashboard in $COMPOSE_FILE." > /dev/stderr
exit 1
fi
if [[ -z "$CONFIG_YAML_HOST" ]]; then

File diff suppressed because it is too large Load Diff

View File

@@ -176,6 +176,7 @@ func (c *Controller) sendUpdateAccountPeers(ctx context.Context, accountID strin
semaphore := make(chan struct{}, 10)
c.injectAllProxyPolicies(ctx, account)
account.PrecomputePostureValidation(ctx)
dnsCache := &cache.DNSConfigCache{}
dnsDomain := c.GetDNSDomain(account.Settings)
peersCustomZone := account.GetPeersCustomZone(ctx, dnsDomain)
@@ -357,6 +358,7 @@ func (c *Controller) sendUpdateForAffectedPeers(ctx context.Context, accountID s
// network map that omitted the synth DNS zone, and the agent kept
// resolving against the stale or absent record.
c.injectAllProxyPolicies(ctx, account)
account.PrecomputePostureValidation(ctx)
dnsCache := &cache.DNSConfigCache{}
dnsDomain := c.GetDNSDomain(account.Settings)
peersCustomZone := account.GetPeersCustomZone(ctx, dnsDomain)

View File

@@ -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
}

View File

@@ -0,0 +1,66 @@
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 peer the caller already holds answers the throttle without a
// query, so a peer seen inside the interval costs nothing to skip; the same
// cutoff goes to the store, which enforces it inside the UPDATE so concurrent
// requests for one peer cannot each write off their own stale read.
func (m *managerImpl) RecordPeerSeen(ctx context.Context, accountID string, peer *peer.Peer) error {
if peer == nil || !countsTowardActivity(peer) {
return nil
}
staleBefore := time.Now().UTC().Add(-peerSeenInterval)
if peer.Status != nil && peer.Status.LastSeen.After(staleBefore) {
return nil
}
_, err := m.store.RefreshPeerLastSeen(ctx, accountID, peer.ID, staleBefore)
return err
}
// 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"
}

View File

@@ -0,0 +1,149 @@
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
staleBefore time.Time
}
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, staleBefore time.Time) (bool, error) {
s.seen = append(s.seen, seenWrite{accountID: accountID, peerID: peerID, staleBefore: staleBefore})
return true, 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")
assert.Equal(t, time.UTC, st.seen[0].staleBefore.Location(), "cutoffs are passed in UTC")
assert.WithinDuration(t, time.Now().UTC().Add(-peerSeenInterval), st.seen[0].staleBefore, time.Minute,
"the store must enforce the same interval the local check applies")
})
}
}

View File

@@ -27,6 +27,8 @@ import (
"github.com/netbirdio/netbird/management/internals/modules/agentnetwork"
"github.com/netbirdio/netbird/management/internals/modules/reverseproxy/accesslogs"
accesslogsmanager "github.com/netbirdio/netbird/management/internals/modules/reverseproxy/accesslogs/manager"
proxyactivity "github.com/netbirdio/netbird/management/internals/modules/reverseproxy/activity"
proxyactivitymanager "github.com/netbirdio/netbird/management/internals/modules/reverseproxy/activity/manager"
rpservice "github.com/netbirdio/netbird/management/internals/modules/reverseproxy/service"
nbgrpc "github.com/netbirdio/netbird/management/internals/shared/grpc"
"github.com/netbirdio/netbird/management/server/activity"
@@ -231,6 +233,7 @@ func (s *BaseServer) ReverseProxyGRPCServer() *nbgrpc.ProxyServiceServer {
proxyService := nbgrpc.NewProxyServiceServer(s.AccessLogsManager(), s.ProxyTokenStore(), s.PKCEVerifierStore(), s.proxyOIDCConfig(), s.PeersManager(), s.UsersManager(), s.IdpManager(), s.ProxyManager(), s.Store())
s.AfterInit(func(s *BaseServer) {
proxyService.SetServiceManager(s.ServiceManager())
proxyService.SetActivityManager(s.ProxyActivityManager())
proxyService.SetProxyController(s.ServiceProxyController())
proxyService.SetAgentNetworkSynthesizer(newAgentNetworkSynthesizer(s.Store()))
proxyService.SetAgentNetworkLimitsService(s.AgentNetworkManager())
@@ -290,6 +293,13 @@ func (s *BaseServer) PKCEVerifierStore() *nbgrpc.PKCEVerifierStore {
})
}
// ProxyActivityManager records reverse proxy usage for activity accounting.
func (s *BaseServer) ProxyActivityManager() proxyactivity.Manager {
return Create(s, func() proxyactivity.Manager {
return proxyactivitymanager.NewManager(s.Store())
})
}
func (s *BaseServer) AccessLogsManager() accesslogs.Manager {
return Create(s, func() accesslogs.Manager {
accessLogManager := accesslogsmanager.NewManager(s.Store(), s.PermissionsManager(), s.GeoLocationManager())

View File

@@ -29,14 +29,15 @@ import (
"github.com/netbirdio/netbird/shared/management/domain"
"github.com/netbirdio/netbird/management/internals/modules/agentnetwork"
"github.com/netbirdio/netbird/management/internals/modules/peers"
"github.com/netbirdio/netbird/management/internals/modules/reverseproxy/accesslogs"
"github.com/netbirdio/netbird/management/internals/modules/reverseproxy/activity"
"github.com/netbirdio/netbird/management/internals/modules/reverseproxy/proxy"
rpservice "github.com/netbirdio/netbird/management/internals/modules/reverseproxy/service"
"github.com/netbirdio/netbird/management/internals/modules/reverseproxy/sessionkey"
"github.com/netbirdio/netbird/management/server/idp"
"github.com/netbirdio/netbird/management/server/peer"
"github.com/netbirdio/netbird/management/internals/modules/agentnetwork"
"github.com/netbirdio/netbird/management/server/types"
"github.com/netbirdio/netbird/management/server/users"
proxyauth "github.com/netbirdio/netbird/proxy/auth"
@@ -114,6 +115,9 @@ type ProxyServiceServer struct {
// Manager for IdP-enriched user data (may be nil when no IdP is configured)
idpManager idp.Manager
// Manager that records reverse proxy usage for activity accounting
activityManager activity.Manager
// Store for one-time authentication tokens
tokenStore *OneTimeTokenStore
@@ -236,6 +240,13 @@ func (s *ProxyServiceServer) SetServiceManager(manager rpservice.Manager) {
s.serviceManager = manager
}
// SetActivityManager wires the manager that records reverse proxy usage.
func (s *ProxyServiceServer) SetActivityManager(manager activity.Manager) {
s.mu.Lock()
defer s.mu.Unlock()
s.activityManager = manager
}
// SetAgentNetworkSynthesizer wires the agent-network service synthesiser.
// Optional — when nil the snapshot path skips agent-network synthesis. The
// modules layer injects this after both the proxy server and the agent-network
@@ -1579,9 +1590,62 @@ func (s *ProxyServiceServer) ValidateState(state string) (verifier, redirectURL
return verifier, redirectURL, nil
}
// Denied reasons reported to the proxy when access is refused because of the
// account status of the user behind the request.
const (
deniedReasonPendingApproval = "pending_approval"
deniedReasonUserBlocked = "user_blocked"
deniedReasonUserNotFound = "user_not_found"
)
var (
// ErrUserPendingApproval reports a user whose account still awaits approval
// by an administrator and may therefore not hold a proxy session.
ErrUserPendingApproval = errors.New("user pending approval")
// ErrUserBlocked reports a blocked user, who may not hold a proxy session.
ErrUserBlocked = errors.New("user blocked")
errUserUnresolved = errors.New("user could not be resolved")
)
// checkUserStatus reports whether the user's account status permits reverse
// proxy access, returning the denied reason for the proxy access log together
// with the sentinel error callers match on. A user awaiting approval is stored
// as both pending and blocked, so the pending state is reported first: it is
// the one an administrator can act on.
func checkUserStatus(user *types.User) (string, error) {
switch {
case user == nil:
return deniedReasonUserNotFound, errUserUnresolved
case user.PendingApproval:
return deniedReasonPendingApproval, ErrUserPendingApproval
case user.IsBlocked():
return deniedReasonUserBlocked, ErrUserBlocked
default:
return "", nil
}
}
// userStatusDeniedReason returns the denied reason for callers that report a
// decision rather than an error, and an empty string when the user may proceed.
func userStatusDeniedReason(user *types.User) string {
reason, _ := checkUserStatus(user)
return reason
}
// sameAccount reports whether a user belongs to a service's account. An empty
// identifier on either side never matches: two unset accounts must not compare
// equal into a grant.
func sameAccount(userAccountID, serviceAccountID string) bool {
return userAccountID != "" && serviceAccountID != "" && userAccountID == serviceAccountID
}
// GenerateSessionToken creates a signed session JWT for the given domain and
// user. The user's group memberships are embedded in the token so policy-aware
// middlewares on the proxy can authorise without an extra management round-trip.
// A user the store cannot resolve, or whose account is pending approval or
// blocked, gets no token at all, so the browser never receives a session cookie.
func (s *ProxyServiceServer) GenerateSessionToken(ctx context.Context, domain, userID string, method proxyauth.Method) (string, error) {
service, err := s.getServiceByDomain(ctx, domain)
if err != nil {
@@ -1592,31 +1656,62 @@ func (s *ProxyServiceServer) GenerateSessionToken(ctx context.Context, domain, u
return "", fmt.Errorf("no session key configured for domain: %s", domain)
}
var (
email string
groupIDs []string
groupNames []string
)
if s.usersManager != nil {
user, userGroups, uerr := s.usersManager.GetUserWithGroups(ctx, userID)
if uerr != nil {
log.WithContext(ctx).Debugf("session token mint: lookup user %s: %v", userID, uerr)
} else if user != nil {
email = user.Email
groupIDs, groupNames = pairGroupIDsAndNames(userGroups)
}
if s.usersManager == nil {
return "", errors.New("users manager not configured")
}
return sessionkey.SignToken(
user, userGroups, err := s.usersManager.GetUserWithGroups(ctx, userID)
if err != nil {
return "", fmt.Errorf("get user %s: %w", userID, err)
}
if user == nil {
return "", fmt.Errorf("get user %s: %w", userID, errUserUnresolved)
}
// Bind the OIDC identity to the service's account before signing anything
// with that service's session key. The proxy validates an installed cookie
// locally against the service public key, so a token minted for a user of
// another account would be honoured without a management round-trip.
if !sameAccount(user.AccountID, service.AccountID) {
return "", fmt.Errorf("user %s does not belong to the service account", userID)
}
if _, err := checkUserStatus(user); err != nil {
return "", fmt.Errorf("session token for user %s: %w", userID, err)
}
groupIDs, groupNames := pairGroupIDsAndNames(userGroups)
token, err := sessionkey.SignToken(
service.SessionPrivateKey,
userID,
email,
user.Email,
domain,
method,
groupIDs,
groupNames,
proxyauth.DefaultSessionExpiry,
)
if err != nil {
return "", err
}
s.recordUserLogin(ctx, service.AccountID, user)
return token, nil
}
// recordUserLogin hands the sign-in to the activity manager. The RPC must not
// fail on it, so the error is logged and dropped here rather than returned.
func (s *ProxyServiceServer) recordUserLogin(ctx context.Context, accountID string, user *types.User) {
if s.activityManager == nil {
return
}
if err := s.activityManager.RecordUserLogin(ctx, accountID, user); 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.
@@ -1628,6 +1723,10 @@ func (s *ProxyServiceServer) ValidateUserGroupAccess(ctx context.Context, domain
return fmt.Errorf("user not found: %s", userID)
}
if _, err := checkUserStatus(user); err != nil {
return fmt.Errorf("user %s denied access to domain %s: %w", userID, domain, err)
}
service, err := s.getAccountServiceByDomain(ctx, user.AccountID, domain)
if err != nil {
return err
@@ -1682,10 +1781,7 @@ func (s *ProxyServiceServer) ValidateSession(ctx context.Context, req *proto.Val
sessionToken := req.GetSessionToken()
if domain == "" || sessionToken == "" {
return &proto.ValidateSessionResponse{
Valid: false,
DeniedReason: "missing domain or session_token",
}, nil
return deniedSessionResponse("missing domain or session_token"), nil
}
service, err := s.getServiceByDomain(ctx, domain)
@@ -1695,83 +1791,49 @@ func (s *ProxyServiceServer) ValidateSession(ctx context.Context, req *proto.Val
"error": err.Error(),
}).Debug("ValidateSession: service not found")
//nolint:nilerr
return &proto.ValidateSessionResponse{
Valid: false,
DeniedReason: "service_not_found",
}, nil
return deniedSessionResponse("service_not_found"), nil
}
if err := enforceAccountScope(ctx, service.AccountID); err != nil {
return nil, err
}
pubKeyBytes, err := base64.StdEncoding.DecodeString(service.SessionPublicKey)
if err != nil {
log.WithFields(log.Fields{
"domain": domain,
"error": err.Error(),
}).Error("ValidateSession: decode public key")
//nolint:nilerr
return &proto.ValidateSessionResponse{
Valid: false,
DeniedReason: "invalid_service_config",
}, nil
}
userID, _, _, _, _, err := proxyauth.ValidateSessionJWT(sessionToken, domain, pubKeyBytes)
if err != nil {
log.WithFields(log.Fields{
"domain": domain,
"error": err.Error(),
}).Debug("ValidateSession: invalid session token")
//nolint:nilerr
return &proto.ValidateSessionResponse{
Valid: false,
DeniedReason: "invalid_token",
}, nil
userID, reason := sessionTokenSubject(domain, service, sessionToken)
if reason != "" {
return deniedSessionResponse(reason), nil
}
user, userGroups, err := s.usersManager.GetUserWithGroups(ctx, userID)
if err != nil {
if err != nil || user == nil {
log.WithFields(log.Fields{
"domain": domain,
"user_id": userID,
"error": err.Error(),
"error": err,
}).Debug("ValidateSession: user not found")
//nolint:nilerr
return &proto.ValidateSessionResponse{
Valid: false,
DeniedReason: "user_not_found",
}, nil
return deniedSessionResponse(deniedReasonUserNotFound), nil
}
if user.AccountID != service.AccountID {
// A user from another account gets a bare response: none of their identity
// belongs in an answer to a proxy serving a different account.
if !sameAccount(user.AccountID, service.AccountID) {
log.WithFields(log.Fields{
"domain": domain,
"user_id": userID,
"user_account": user.AccountID,
"service_account": service.AccountID,
}).Debug("ValidateSession: user account mismatch")
//nolint:nilerr
return &proto.ValidateSessionResponse{
Valid: false,
DeniedReason: "account_mismatch",
}, nil
return deniedSessionResponse("account_mismatch"), nil
}
if err := s.checkGroupAccess(service, user); err != nil {
log.WithFields(log.Fields{
"domain": domain,
"user_id": userID,
"error": err.Error(),
}).Debug("ValidateSession: access denied")
groupIDs, groupNames := pairGroupIDsAndNames(userGroups)
//nolint:nilerr
groupIDs, groupNames := pairGroupIDsAndNames(userGroups)
if reason := s.accountUserDeniedReason(domain, service, user); reason != "" {
return &proto.ValidateSessionResponse{
Valid: false,
UserId: user.Id,
UserEmail: user.Email,
DeniedReason: "not_in_group",
DeniedReason: reason,
PeerGroupIds: groupIDs,
PeerGroupNames: groupNames,
}, nil
@@ -1783,7 +1845,6 @@ func (s *ProxyServiceServer) ValidateSession(ctx context.Context, req *proto.Val
"email": user.Email,
}).Debug("ValidateSession: access granted")
groupIDs, groupNames := pairGroupIDsAndNames(userGroups)
return &proto.ValidateSessionResponse{
Valid: true,
UserId: user.Id,
@@ -1793,6 +1854,66 @@ func (s *ProxyServiceServer) ValidateSession(ctx context.Context, req *proto.Val
}, nil
}
// deniedSessionResponse builds a denial that carries no identity, for the
// checks that run before a user of this service's account is resolved.
func deniedSessionResponse(reason string) *proto.ValidateSessionResponse {
return &proto.ValidateSessionResponse{
Valid: false,
DeniedReason: reason,
}
}
// sessionTokenSubject verifies the session token against the service's session
// key and returns the user it was minted for, or the reason it cannot be
// trusted.
func sessionTokenSubject(domain string, service *rpservice.Service, sessionToken string) (userID, deniedReason string) {
pubKeyBytes, err := base64.StdEncoding.DecodeString(service.SessionPublicKey)
if err != nil {
log.WithFields(log.Fields{
"domain": domain,
"error": err.Error(),
}).Error("ValidateSession: decode public key")
return "", "invalid_service_config"
}
userID, _, _, _, _, err = proxyauth.ValidateSessionJWT(sessionToken, domain, pubKeyBytes)
if err != nil {
log.WithFields(log.Fields{
"domain": domain,
"error": err.Error(),
}).Debug("ValidateSession: invalid session token")
return "", "invalid_token"
}
return userID, ""
}
// accountUserDeniedReason gates a user of the service's own account, returning
// an empty string when access is granted. Account status comes before group
// membership: a user awaiting approval or blocked has no access regardless of
// the groups they were auto-assigned.
func (s *ProxyServiceServer) accountUserDeniedReason(domain string, service *rpservice.Service, user *types.User) string {
if reason := userStatusDeniedReason(user); reason != "" {
log.WithFields(log.Fields{
"domain": domain,
"user_id": user.Id,
"reason": reason,
}).Debug("ValidateSession: user status denies access")
return reason
}
if err := s.checkGroupAccess(service, user); err != nil {
log.WithFields(log.Fields{
"domain": domain,
"user_id": user.Id,
"error": err.Error(),
}).Debug("ValidateSession: access denied")
return "not_in_group"
}
return ""
}
func (s *ProxyServiceServer) getServiceByDomain(ctx context.Context, domain string) (*rpservice.Service, error) {
service, err := s.serviceManager.GetServiceByDomain(ctx, domain)
if err == nil {
@@ -1907,7 +2028,20 @@ func (s *ProxyServiceServer) ValidateTunnelPeer(ctx context.Context, req *proto.
}
groupIDs, groupNames := pairGroupIDsAndNames(peerGroups)
principalID, displayIdentity := s.getTunnelPeerInfo(ctx, domain, service, peer)
owner := s.resolvePeerOwner(ctx, peer, service.AccountID)
principalID, displayIdentity := s.getTunnelPeerInfo(ctx, domain, service, peer, owner)
if reason := peerOwnerDeniedReason(peer, owner); reason != "" {
log.WithFields(log.Fields{"domain": domain, "peer_id": peer.ID, "user_id": peer.UserID, "reason": reason}).Debug("ValidateTunnelPeer: owner status denies access")
return &proto.ValidateTunnelPeerResponse{
Valid: false,
UserId: principalID,
UserEmail: displayIdentity,
DeniedReason: reason,
PeerGroupIds: groupIDs,
PeerGroupNames: groupNames,
}, nil
}
if err := checkPeerGroupAccess(service, groupIDs); err != nil {
log.WithFields(log.Fields{"domain": domain, "peer_id": peer.ID, "error": err.Error()}).Debug("ValidateTunnelPeer: access denied")
@@ -1927,6 +2061,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,
@@ -1944,9 +2080,67 @@ func (s *ProxyServiceServer) ValidateTunnelPeer(ctx context.Context, req *proto.
}, nil
}
// recordPeerSeen hands the mesh request to the activity manager. The RPC must
// not fail on it, so the error is logged and dropped here rather than returned.
func (s *ProxyServiceServer) recordPeerSeen(ctx context.Context, accountID string, peer *peer.Peer) {
if s.activityManager == nil {
return
}
if err := s.activityManager.RecordPeerSeen(ctx, accountID, peer); 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
// nil rather than an error: both callers treat an unresolved owner the same
// way, and neither may trust one it could not read.
func (s *ProxyServiceServer) resolvePeerOwner(ctx context.Context, peer *peer.Peer, accountID string) *types.User {
if peer.UserID == "" {
return nil
}
user, err := s.usersManager.GetUser(ctx, peer.UserID)
if err != nil {
log.WithContext(ctx).Debugf("ValidateTunnelPeer: look up owner %s of peer %s: %v", peer.UserID, peer.ID, err)
return nil
}
// The lookup is by user ID alone, so a peer row pointing outside the
// service's account would otherwise resolve a foreign user. Leave the owner
// unresolved instead: the gate denies it, and neither the response nor the
// minted token carries an identity from another account.
if !sameAccount(user.AccountID, accountID) {
log.WithContext(ctx).Debugf("ValidateTunnelPeer: owner %s of peer %s belongs to another account", peer.UserID, peer.ID)
return nil
}
return user
}
// peerOwnerDeniedReason gates the mesh fast-path on the account status of the
// peer's owning user, so a user blocked after registering a peer loses
// mesh-origin access too. Unlinked peers (machine agents) have no owner to gate
// on and stay first-class callers. An owner the store cannot resolve denies:
// an unavailable lookup must not grant access.
func peerOwnerDeniedReason(peer *peer.Peer, owner *types.User) string {
if peer.UserID == "" {
return ""
}
if owner == nil {
return deniedReasonUserNotFound
}
return userStatusDeniedReason(owner)
}
// getTunnelPeerInfo returns the principal ID and display name for a peer, e.g. a
// user or peer ID, and peer name or user email.
func (s *ProxyServiceServer) getTunnelPeerInfo(ctx context.Context, domain string, service *rpservice.Service, peer *peer.Peer) (string, string) {
// user or peer ID, and peer name or user email. owner is the already-resolved
// user the peer is linked to, or nil.
func (s *ProxyServiceServer) getTunnelPeerInfo(ctx context.Context, domain string, service *rpservice.Service, peer *peer.Peer, owner *types.User) (string, string) {
// Resolve the principal: when the peer is linked to a user, the human is the
// principal so multiple peers owned by the same user share a single
// identity. Unlinked peers (machine agents) are their own principal keyed on
@@ -1963,10 +2157,10 @@ func (s *ProxyServiceServer) getTunnelPeerInfo(ctx context.Context, domain strin
principalID := peer.UserID
displayIdentity := peer.Name
// Stored column first (cheap, but often empty for OIDC-provisioned users).
if user, uerr := s.usersManager.GetUser(ctx, peer.UserID); uerr == nil && user != nil {
principalID = user.Id
if user.Email != "" {
displayIdentity = user.Email
if owner != nil {
principalID = owner.Id
if owner.Email != "" {
displayIdentity = owner.Email
}
}
// IdP enrichment wins when available — the stored email column is a

View File

@@ -5,6 +5,7 @@ import (
"errors"
"net"
"testing"
"time"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
@@ -119,11 +120,13 @@ func (m *mockReverseProxyManager) GetClusters(_ context.Context, _, _ string) ([
}
type mockUsersManager struct {
users map[string]*types.User
err error
users map[string]*types.User
err error
getUserCalls int
}
func (m *mockUsersManager) GetUser(ctx context.Context, userID string) (*types.User, error) {
m.getUserCalls++
if m.err != nil {
return nil, m.err
}
@@ -153,6 +156,27 @@ type mockTunnelPeersManager struct {
groupsErr error
}
// mockActivityManager records what the RPC handed to the activity manager. The
// policy (throttling, exclusions) is the manager's and is tested there; these
// tests only pin which requests reach it.
type mockActivityManager struct {
seenMarks []seenMark
}
type seenMark struct {
accountID string
peerID string
}
func (m *mockActivityManager) RecordUserLogin(_ context.Context, _ string, _ *types.User) error {
return nil
}
func (m *mockActivityManager) RecordPeerSeen(_ context.Context, accountID string, peer *peer.Peer) error {
m.seenMarks = append(m.seenMarks, seenMark{accountID: accountID, peerID: peer.ID})
return nil
}
func (m *mockTunnelPeersManager) GetPeerByTunnelIP(_ context.Context, _ string, _ net.IP) (*peer.Peer, error) {
return m.peer, m.peerErr
}
@@ -350,6 +374,64 @@ func TestValidateUserGroupAccess(t *testing.T) {
},
expectErr: false,
},
{
name: "user pending approval denied despite group membership",
domain: "app.example.com",
userID: "user1",
proxiesByAccount: map[string][]*service.Service{
"account1": {{
Domain: "app.example.com",
AccountID: "account1",
Auth: service.AuthConfig{
BearerAuth: &service.BearerAuthConfig{
Enabled: true,
DistributionGroups: []string{"group1"},
},
},
}},
},
users: map[string]*types.User{
// The approval flow stores a pending user as blocked as well.
"user1": {Id: "user1", AccountID: "account1", AutoGroups: []string{"group1"}, Blocked: true, PendingApproval: true},
},
expectErr: true,
expectErrMsg: "user pending approval",
},
{
name: "blocked user denied despite group membership",
domain: "app.example.com",
userID: "user1",
proxiesByAccount: map[string][]*service.Service{
"account1": {{
Domain: "app.example.com",
AccountID: "account1",
Auth: service.AuthConfig{
BearerAuth: &service.BearerAuthConfig{
Enabled: true,
DistributionGroups: []string{"group1"},
},
},
}},
},
users: map[string]*types.User{
"user1": {Id: "user1", AccountID: "account1", AutoGroups: []string{"group1"}, Blocked: true},
},
expectErr: true,
expectErrMsg: "user blocked",
},
{
name: "blocked user denied on a service with no auth configured",
domain: "app.example.com",
userID: "user1",
proxiesByAccount: map[string][]*service.Service{
"account1": {{Domain: "app.example.com", AccountID: "account1", Auth: service.AuthConfig{}}},
},
users: map[string]*types.User{
"user1": {Id: "user1", AccountID: "account1", Blocked: true},
},
expectErr: true,
expectErrMsg: "user blocked",
},
{
name: "proxy manager error",
domain: "app.example.com",
@@ -421,17 +503,18 @@ func TestValidateTunnelPeerUserEmailEnrichment(t *testing.T) {
storedUserNoEmail := map[string]*types.User{userID: {Id: userID, AccountID: accountID, Email: ""}}
tests := []struct {
name string
peerUserID string
storedUsers map[string]*types.User
storedErr error
noIdP bool
idpEmail string
idpHasData bool
idpErr error
expectEmail string
expectUserID string
expectIdPHit bool
name string
peerUserID string
storedUsers map[string]*types.User
storedErr error
noIdP bool
idpEmail string
idpHasData bool
idpErr error
expectEmail string
expectUserID string
expectIdPHit bool
expectDeniedReason string
}{
{
name: "idp email wins over stored email",
@@ -490,14 +573,17 @@ func TestValidateTunnelPeerUserEmailEnrichment(t *testing.T) {
expectIdPHit: true,
},
{
name: "idp email when stored user missing keeps peer.UserID as principal",
peerUserID: userID,
storedUsers: map[string]*types.User{},
idpEmail: "idp@example.com",
idpHasData: true,
expectEmail: "idp@example.com",
expectUserID: userID,
expectIdPHit: true,
// The identity still resolves from the IdP, but an owner the store
// cannot resolve denies the fast-path rather than granting it.
name: "idp email when stored user missing keeps peer.UserID as principal",
peerUserID: userID,
storedUsers: map[string]*types.User{},
idpEmail: "idp@example.com",
idpHasData: true,
expectEmail: "idp@example.com",
expectUserID: userID,
expectIdPHit: true,
expectDeniedReason: deniedReasonUserNotFound,
},
{
name: "unlinked peer uses peer name and never consults idp",
@@ -545,9 +631,13 @@ func TestValidateTunnelPeerUserEmailEnrichment(t *testing.T) {
require.NoError(t, err)
require.NotNil(t, resp)
assert.True(t, resp.GetValid(), "expected access granted")
assert.Equal(t, tt.expectDeniedReason == "", resp.GetValid(), "unexpected access decision")
assert.Equal(t, tt.expectDeniedReason, resp.GetDeniedReason(), "unexpected denied reason")
assert.Equal(t, tt.expectEmail, resp.GetUserEmail())
assert.Equal(t, tt.expectUserID, resp.GetUserId())
if tt.expectDeniedReason != "" {
assert.Empty(t, resp.GetSessionToken(), "a denied peer must not receive a session token")
}
if idpMock != nil {
if tt.expectIdPHit {
@@ -562,6 +652,193 @@ func TestValidateTunnelPeerUserEmailEnrichment(t *testing.T) {
}
}
// TestDeniedReasonValues pins the wire values of the account status denied
// reasons. The proxy logs them and operators filter access logs on them, so a
// rename is a breaking change rather than an internal detail.
// TestSameAccount pins the fail-closed behaviour of the account binding: an
// unset account on either side must never compare equal into a grant.
func TestSameAccount(t *testing.T) {
assert.True(t, sameAccount("account1", "account1"), "matching accounts should bind")
assert.False(t, sameAccount("account1", "account2"), "different accounts must not bind")
assert.False(t, sameAccount("", ""), "two unset accounts must not bind")
assert.False(t, sameAccount("account1", ""), "an unset service account must not bind")
assert.False(t, sameAccount("", "account1"), "an unset user account must not bind")
}
func TestDeniedReasonValues(t *testing.T) {
assert.Equal(t, "pending_approval", deniedReasonPendingApproval, "pending approval denied reason wire value")
assert.Equal(t, "user_blocked", deniedReasonUserBlocked, "blocked user denied reason wire value")
assert.Equal(t, "user_not_found", deniedReasonUserNotFound, "unresolved user denied reason wire value")
}
// TestValidateTunnelPeerOwnerStatus verifies that the mesh fast-path gates on
// the account status of the peer's owning user. A peer whose owner was blocked
// after the peer registered must lose access, while an unlinked machine peer
// keeps it.
func TestValidateTunnelPeerOwnerStatus(t *testing.T) {
const (
domain = "app.example.com"
accountID = "account1"
peerID = "peer1"
peerName = "peer-display-name"
userID = "user1"
)
tests := []struct {
name string
peerUserID string
owner *types.User
expectDeniedReason string
expectEmail string
}{
{
name: "active owner allowed",
peerUserID: userID,
owner: &types.User{Id: userID, AccountID: accountID, Email: "user@example.com"},
},
{
name: "owner pending approval denied",
peerUserID: userID,
owner: &types.User{Id: userID, AccountID: accountID, Email: "user@example.com", Blocked: true, PendingApproval: true},
expectDeniedReason: deniedReasonPendingApproval,
},
{
name: "owner blocked after registering the peer denied",
peerUserID: userID,
owner: &types.User{Id: userID, AccountID: accountID, Email: "user@example.com", Blocked: true},
expectDeniedReason: deniedReasonUserBlocked,
},
{
name: "unlinked machine peer stays allowed",
peerUserID: "",
owner: &types.User{Id: userID, AccountID: accountID, Blocked: true},
},
{
// The user lookup is not account-scoped, so a peer row pointing at
// another account's user must not resolve into an owner: the peer is
// denied and the foreign email never reaches the response.
name: "owner in another account denied and not disclosed",
peerUserID: userID,
owner: &types.User{Id: userID, AccountID: "otherAccount", Email: "foreign@example.com"},
expectDeniedReason: deniedReasonUserNotFound,
expectEmail: peerName,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
svc := &service.Service{Domain: domain, AccountID: accountID}
usersManager := &mockUsersManager{users: map[string]*types.User{userID: tt.owner}}
server := &ProxyServiceServer{
serviceManager: &mockReverseProxyManager{
proxiesByAccount: map[string][]*service.Service{accountID: {svc}},
},
peersManager: &mockTunnelPeersManager{
peer: &peer.Peer{ID: peerID, Name: peerName, UserID: tt.peerUserID},
},
usersManager: usersManager,
}
resp, err := server.ValidateTunnelPeer(context.Background(), &proto.ValidateTunnelPeerRequest{
Domain: domain,
TunnelIp: "100.64.0.1",
})
require.NoError(t, err)
require.NotNil(t, resp)
assert.Equal(t, tt.expectDeniedReason, resp.GetDeniedReason(), "unexpected denied reason")
assert.Equal(t, tt.expectDeniedReason == "", resp.GetValid(), "unexpected access decision")
if tt.expectDeniedReason != "" {
assert.Empty(t, resp.GetSessionToken(), "a denied peer must not receive a session token")
}
if tt.expectEmail != "" {
assert.Equal(t, tt.expectEmail, resp.GetUserEmail(), "unexpected identity on the response")
}
// The status gate and the identity resolution share one lookup;
// an unlinked peer has no owner to look up at all.
wantLookups := 1
if tt.peerUserID == "" {
wantLookups = 0
}
assert.Equal(t, wantLookups, usersManager.getUserCalls, "owner must be resolved exactly once per request")
})
}
}
// TestValidateTunnelPeerRecordsActivity pins that a granted mesh request is
// handed to the activity manager. Which of those the manager then writes is its
// own decision, covered by its tests.
func TestValidateTunnelPeerRecordsActivity(t *testing.T) {
const (
domain = "app.example.com"
accountID = "account1"
peerID = "peer1"
)
activityManager := &mockActivityManager{}
server := &ProxyServiceServer{
activityManager: activityManager,
serviceManager: &mockReverseProxyManager{
proxiesByAccount: map[string][]*service.Service{
accountID: {{Domain: domain, AccountID: accountID}},
},
},
peersManager: &mockTunnelPeersManager{
peer: &peer.Peer{ID: peerID, Name: "agent", Status: &peer.PeerStatus{LastSeen: time.Now().Add(-3 * time.Hour)}},
},
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")
require.Len(t, activityManager.seenMarks, 1, "a granted peer should reach the activity manager once")
assert.Equal(t, accountID, activityManager.seenMarks[0].accountID, "activity must be attributed to the service account")
assert.Equal(t, peerID, activityManager.seenMarks[0].peerID, "activity must be attributed to the calling peer")
}
// 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"
)
activityManager := &mockActivityManager{}
server := &ProxyServiceServer{
activityManager: activityManager,
serviceManager: &mockReverseProxyManager{
proxiesByAccount: map[string][]*service.Service{
accountID: {{Domain: domain, AccountID: accountID}},
},
},
peersManager: &mockTunnelPeersManager{
peer: &peer.Peer{ID: "peer1", Name: "agent", UserID: "user1", Status: &peer.PeerStatus{LastSeen: time.Now().Add(-3 * time.Hour)}},
},
// 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, activityManager.seenMarks, "a denied peer must not be marked seen")
}
func TestGetAccountProxyByDomain(t *testing.T) {
tests := []struct {
name string

View File

@@ -46,6 +46,7 @@ func setupValidateSessionTest(t *testing.T) *validateSessionTestSetup {
proxyService.SetServiceManager(serviceManager)
createTestProxies(t, ctx, testStore)
createStatusTestUsers(t, ctx, testStore)
return &validateSessionTestSetup{
proxyService: proxyService,
@@ -91,6 +92,82 @@ func createTestProxies(t *testing.T, ctx context.Context, testStore store.Store)
},
}
require.NoError(t, testStore.CreateService(ctx, restrictedProxy))
// Distributed to the account's "All" group, the configuration that hands a
// service to every user in the account.
allUsersProxy := &service.Service{
ID: "allUsersProxyId",
AccountID: "testAccountId",
Name: "All Users Proxy",
Domain: "all-users-proxy.example.com",
Enabled: true,
SessionPrivateKey: privKey,
SessionPublicKey: pubKey,
Auth: service.AuthConfig{
BearerAuth: &service.BearerAuthConfig{
Enabled: true,
DistributionGroups: []string{allUsersGroupID},
},
},
}
require.NoError(t, testStore.CreateService(ctx, allUsersProxy))
}
const (
allUsersGroupID = "allUsersGroupId"
pendingUserID = "pendingUserId"
blockedUserID = "blockedUserId"
pendingAllUsersID = "pendingAllUsersUserId"
)
// createStatusTestUsers adds the users whose account status must keep them out
// of a proxy session. A user awaiting approval is persisted as both blocked and
// pending approval, the way the approval flow stores one.
func createStatusTestUsers(t *testing.T, ctx context.Context, testStore store.Store) {
t.Helper()
require.NoError(t, testStore.CreateGroup(ctx, &types.Group{
ID: allUsersGroupID,
AccountID: "testAccountId",
Name: "All",
Issued: types.GroupIssuedAPI,
}))
users := []*types.User{
{
Id: pendingUserID,
AccountID: "testAccountId",
Role: types.UserRoleUser,
AutoGroups: []string{"allowedGroupId"},
Blocked: true,
PendingApproval: true,
Issued: "api",
CreatedAt: time.Now(),
},
{
Id: pendingAllUsersID,
AccountID: "testAccountId",
Role: types.UserRoleUser,
AutoGroups: []string{allUsersGroupID},
Blocked: true,
PendingApproval: true,
Issued: "api",
CreatedAt: time.Now(),
},
{
Id: blockedUserID,
AccountID: "testAccountId",
Role: types.UserRoleUser,
AutoGroups: []string{"allowedGroupId"},
Blocked: true,
PendingApproval: false,
Issued: "api",
CreatedAt: time.Now(),
},
}
for _, user := range users {
require.NoError(t, testStore.SaveUser(ctx, user))
}
}
func generateSessionKeyPair(t *testing.T) (string, string) {
@@ -149,6 +226,114 @@ func TestValidateSession_UserNotInAllowedGroup(t *testing.T) {
assert.Empty(t, resp.GetPeerGroupIds(), "PeerGroupIds must mirror the resolved user's actual (empty) memberships on denial")
}
// TestValidateSession_PendingApprovalUserDenied covers a user who is a member of
// the service's distribution group but is still waiting for an administrator to
// approve the account. Group membership alone must not open the service.
func TestValidateSession_PendingApprovalUserDenied(t *testing.T) {
setup := setupValidateSessionTest(t)
defer setup.cleanup()
proxy, err := setup.store.GetServiceByID(context.Background(), store.LockingStrengthNone, "testAccountId", "restrictedProxyId")
require.NoError(t, err)
token := createSessionToken(t, proxy.SessionPrivateKey, pendingUserID, "restricted-proxy.example.com")
resp, err := setup.proxyService.ValidateSession(context.Background(), &proto.ValidateSessionRequest{
Domain: "restricted-proxy.example.com",
SessionToken: token,
})
require.NoError(t, err)
assert.False(t, resp.Valid, "User pending approval should be denied")
assert.Equal(t, deniedReasonPendingApproval, resp.DeniedReason, "Denied reason should name the pending approval state")
assert.Equal(t, pendingUserID, resp.UserId, "Denial should identify the user it applies to")
assert.Equal(t, []string{"allowedGroupId"}, resp.GetPeerGroupIds(), "PeerGroupIds must mirror the resolved user's group memberships on denial")
assert.Equal(t, []string{"Allowed Group"}, resp.GetPeerGroupNames(), "PeerGroupNames must pair with PeerGroupIds on denial")
}
// TestValidateSession_PendingApprovalUserInAllUsersGroupDenied covers the same
// user against a service distributed to the account's "All" group, where every
// user of the account is a member by default.
func TestValidateSession_PendingApprovalUserInAllUsersGroupDenied(t *testing.T) {
setup := setupValidateSessionTest(t)
defer setup.cleanup()
proxy, err := setup.store.GetServiceByID(context.Background(), store.LockingStrengthNone, "testAccountId", "allUsersProxyId")
require.NoError(t, err)
token := createSessionToken(t, proxy.SessionPrivateKey, pendingAllUsersID, "all-users-proxy.example.com")
resp, err := setup.proxyService.ValidateSession(context.Background(), &proto.ValidateSessionRequest{
Domain: "all-users-proxy.example.com",
SessionToken: token,
})
require.NoError(t, err)
assert.False(t, resp.Valid, "User pending approval should be denied even in the All Users group")
assert.Equal(t, deniedReasonPendingApproval, resp.DeniedReason, "Denied reason should name the pending approval state")
assert.Equal(t, pendingAllUsersID, resp.UserId, "Denial should identify the user it applies to")
assert.Equal(t, []string{allUsersGroupID}, resp.GetPeerGroupIds(), "PeerGroupIds must mirror the resolved user's group memberships on denial")
}
// TestValidateSession_BlockedUserDenied covers a user blocked after having been
// approved, so PendingApproval is false and only the blocked flag is set.
func TestValidateSession_BlockedUserDenied(t *testing.T) {
setup := setupValidateSessionTest(t)
defer setup.cleanup()
proxy, err := setup.store.GetServiceByID(context.Background(), store.LockingStrengthNone, "testAccountId", "restrictedProxyId")
require.NoError(t, err)
token := createSessionToken(t, proxy.SessionPrivateKey, blockedUserID, "restricted-proxy.example.com")
resp, err := setup.proxyService.ValidateSession(context.Background(), &proto.ValidateSessionRequest{
Domain: "restricted-proxy.example.com",
SessionToken: token,
})
require.NoError(t, err)
assert.False(t, resp.Valid, "Blocked user should be denied")
assert.Equal(t, deniedReasonUserBlocked, resp.DeniedReason, "Denied reason should name the blocked state")
assert.Equal(t, blockedUserID, resp.UserId, "Denial should identify the user it applies to")
}
// TestValidateSession_UserAllowedAfterApproval walks the same session token
// through the approval transition: denied while pending, allowed once an
// administrator clears both flags.
func TestValidateSession_UserAllowedAfterApproval(t *testing.T) {
setup := setupValidateSessionTest(t)
defer setup.cleanup()
ctx := context.Background()
proxy, err := setup.store.GetServiceByID(ctx, store.LockingStrengthNone, "testAccountId", "restrictedProxyId")
require.NoError(t, err)
token := createSessionToken(t, proxy.SessionPrivateKey, pendingUserID, "restricted-proxy.example.com")
req := &proto.ValidateSessionRequest{
Domain: "restricted-proxy.example.com",
SessionToken: token,
}
resp, err := setup.proxyService.ValidateSession(ctx, req)
require.NoError(t, err)
require.False(t, resp.Valid, "User pending approval should be denied before approval")
assert.Equal(t, deniedReasonPendingApproval, resp.DeniedReason, "Denied reason should name the pending approval state")
user, err := setup.store.GetUserByUserID(ctx, store.LockingStrengthNone, pendingUserID)
require.NoError(t, err)
user.PendingApproval = false
user.Blocked = false
require.NoError(t, setup.store.SaveUser(ctx, user))
resp, err = setup.proxyService.ValidateSession(ctx, req)
require.NoError(t, err)
assert.True(t, resp.Valid, "Approved user should be allowed access")
assert.Empty(t, resp.DeniedReason)
assert.Equal(t, pendingUserID, resp.UserId, "Approved user should be identified in the response")
assert.Equal(t, []string{"allowedGroupId"}, resp.GetPeerGroupIds(), "PeerGroupIds must mirror the approved user's group memberships")
}
func TestValidateSession_UserInDifferentAccount(t *testing.T) {
setup := setupValidateSessionTest(t)
defer setup.cleanup()

View File

@@ -33,6 +33,7 @@ import (
nbconfig "github.com/netbirdio/netbird/management/internals/server/config"
"github.com/netbirdio/netbird/management/server/account"
"github.com/netbirdio/netbird/management/server/activity"
"github.com/netbirdio/netbird/management/server/affectedpeers"
nbcache "github.com/netbirdio/netbird/management/server/cache"
nbcontext "github.com/netbirdio/netbird/management/server/context"
"github.com/netbirdio/netbird/management/server/geolocation"
@@ -1626,6 +1627,8 @@ func (am *DefaultAccountManager) SyncUserJWTGroups(ctx context.Context, userAuth
var removeOldGroups []string
var hasChanges bool
var user *types.User
var change affectedpeers.Change
var snap *affectedpeers.Snapshot
err = am.Store.ExecuteInTransaction(ctx, func(transaction store.Store) error {
user, err = transaction.GetUserByUserID(ctx, store.LockingStrengthNone, userAuth.UserId)
if err != nil {
@@ -1664,14 +1667,25 @@ func (am *DefaultAccountManager) SyncUserJWTGroups(ctx context.Context, userAuth
return fmt.Errorf("error saving user: %w", err)
}
allGroupChanges := slices.Concat(addNewGroups, removeOldGroups)
// The user's auto-groups changed, so the SSH rules authorizing them ship a new
// group -> user mapping even when no peer moves between groups.
change.UserGroupIDs = allGroupChanges
// The user's peers are the changed entity in every scenario the sync can
// produce — group membership, IPv6 assignment, SSH mappings — so they refresh
// together with every peer they can connect to, like on a regular peer update.
userPeers, err := transaction.GetUserPeers(ctx, store.LockingStrengthNone, userAuth.AccountId, userAuth.UserId)
if err != nil {
return fmt.Errorf("error getting user peers: %w", err)
}
for _, peer := range userPeers {
change.ChangedPeerIDs = append(change.ChangedPeerIDs, peer.ID)
}
// Propagate changes to peers if group propagation is enabled
if settings.GroupsPropagationEnabled {
peers, err := transaction.GetUserPeers(ctx, store.LockingStrengthNone, userAuth.AccountId, userAuth.UserId)
if err != nil {
return fmt.Errorf("error getting user peers: %w", err)
}
for _, peer := range peers {
for _, peer := range userPeers {
for _, g := range addNewGroups {
if err := transaction.AddPeerToGroup(ctx, userAuth.AccountId, peer.ID, g); err != nil {
return fmt.Errorf("error adding peer %s to group %s: %w", peer.ID, g, err)
@@ -1684,7 +1698,8 @@ func (am *DefaultAccountManager) SyncUserJWTGroups(ctx context.Context, userAuth
}
}
allGroupChanges := slices.Concat(addNewGroups, removeOldGroups)
change.LinkGroups = allGroupChanges
if err = am.reconcileIPv6ForGroupChanges(ctx, transaction, userAuth.AccountId, allGroupChanges); err != nil {
return fmt.Errorf("reconcile IPv6 for group changes: %w", err)
}
@@ -1694,6 +1709,10 @@ func (am *DefaultAccountManager) SyncUserJWTGroups(ctx context.Context, userAuth
}
}
if snap, err = affectedpeers.Load(ctx, transaction, userAuth.AccountId, change); err != nil {
return err
}
return nil
})
if err != nil {
@@ -1730,20 +1749,17 @@ func (am *DefaultAccountManager) SyncUserJWTGroups(ctx context.Context, userAuth
}
}
removedGroupAffectsPeers, err := areGroupChangesAffectPeers(ctx, am.Store, userAuth.AccountId, removeOldGroups)
if err != nil {
return err
}
newGroupsAffectsPeers, err := areGroupChangesAffectPeers(ctx, am.Store, userAuth.AccountId, addNewGroups)
if err != nil {
return err
}
if removedGroupAffectsPeers || newGroupsAffectsPeers {
log.WithContext(ctx).Tracef("user %s: JWT group membership changed, updating account peers", userAuth.UserId)
am.BufferUpdateAccountPeers(ctx, userAuth.AccountId, types.UpdateReason{Resource: types.UpdateResourceUser, Operation: types.UpdateOperationUpdate})
}
log.WithContext(ctx).Tracef("user %s: JWT group membership changed, updating affected peers", userAuth.UserId)
bgCtx := context.WithoutCancel(ctx)
go func() {
affectedPeerIDs := snap.Expand(bgCtx, userAuth.AccountId, change)
if len(affectedPeerIDs) == 0 {
return
}
if err := am.networkMapController.BufferUpdateAffectedPeers(bgCtx, userAuth.AccountId, affectedPeerIDs, types.UpdateReason{Resource: types.UpdateResourceUser, Operation: types.UpdateOperationUpdate}); err != nil {
log.WithContext(bgCtx).Errorf("failed to update affected peers after JWT group sync for account %s: %v", userAuth.AccountId, err)
}
}()
return nil
}
@@ -2426,30 +2442,24 @@ func (am *DefaultAccountManager) reconcileIPv6ForGroupChanges(ctx context.Contex
return fmt.Errorf("get account settings: %w", err)
}
if len(settings.IPv6EnabledGroups) == 0 {
return nil
}
enabledSet := make(map[string]struct{}, len(settings.IPv6EnabledGroups))
for _, gid := range settings.IPv6EnabledGroups {
enabledSet[gid] = struct{}{}
}
affected := false
for _, gid := range groupIDs {
if _, ok := enabledSet[gid]; ok {
affected = true
break
}
}
if !affected {
if !ipv6ReconcileNeeded(settings, groupIDs) {
return nil
}
return am.updatePeerIPv6Addresses(ctx, transaction, accountID, settings)
}
// ipv6ReconcileNeeded reports whether changes to the given groups trigger an IPv6
// reconciliation.
func ipv6ReconcileNeeded(settings *types.Settings, groupIDs []string) bool {
for _, groupID := range groupIDs {
if slices.Contains(settings.IPv6EnabledGroups, groupID) {
return true
}
}
return false
}
func (am *DefaultAccountManager) ensureIPv6Subnet(ctx context.Context, transaction store.Store, accountID string, settings *types.Settings, network *types.Network) error {
if settings.NetworkRangeV6.IsValid() {
network.NetV6 = net.IPNet{

View File

@@ -1757,6 +1757,7 @@ func TestAccount_Copy(t *testing.T) {
AccountID: "account1",
},
},
PostureValidation: map[string]map[string]bool{"1": {"1": true}},
}
err := hasNilField(account)
if err != nil {

View File

@@ -0,0 +1,179 @@
package server
import (
"context"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"golang.zx2c4.com/wireguard/wgctrl/wgtypes"
"github.com/netbirdio/netbird/management/server/affectedpeers"
nbpeer "github.com/netbirdio/netbird/management/server/peer"
"github.com/netbirdio/netbird/management/server/store"
"github.com/netbirdio/netbird/management/server/types"
"github.com/netbirdio/netbird/shared/auth"
)
// A user's auto-group change refreshes the destinations of the SSH rules authorizing
// that group — they carry the group -> user mapping — even though no peer moved
// between groups.
func TestAffectedPeers_UserGroupChange_RefreshesSSHAuthorizedDestinations(t *testing.T) {
manager, s, accountID, peerIDs, groupIDs := setupAffectedPeersTest(t)
ctx := context.Background()
_, err := manager.SavePolicy(ctx, accountID, userID, &types.Policy{
Enabled: true,
Rules: []*types.PolicyRule{
{
Enabled: true,
Sources: []string{groupIDs[0]},
Destinations: []string{groupIDs[1]},
Protocol: types.PolicyRuleProtocolNetbirdSSH,
Action: types.PolicyTrafficActionAccept,
AuthorizedGroups: map[string][]string{groupIDs[3]: {"root"}},
},
},
}, true)
require.NoError(t, err)
result := resolveAffected(t, s, accountID, affectedpeers.Change{UserGroupIDs: []string{groupIDs[3]}})
assert.ElementsMatch(t, []string{peerIDs[1]}, result,
"only the SSH rule's destination peers carry the changed group -> user mapping")
result = resolveAffected(t, s, accountID, affectedpeers.Change{UserGroupIDs: []string{groupIDs[4]}})
assert.Empty(t, result, "a group no SSH rule authorizes affects nobody")
}
// Creating, blocking or unblocking a user changes the account's allowed-user set, which
// reaches only the destinations of the SSH rules that ship it.
func TestAffectedPeers_AllowedUsersChange_RefreshesSSHDestinations(t *testing.T) {
manager, s, accountID, peerIDs, groupIDs := setupAffectedPeersTest(t)
ctx := context.Background()
// Ships the allowed-user set: an SSH rule naming no groups and no user.
_, err := manager.SavePolicy(ctx, accountID, userID, &types.Policy{
Enabled: true,
Rules: []*types.PolicyRule{{
Enabled: true,
Sources: []string{groupIDs[0]},
Destinations: []string{groupIDs[1]},
Protocol: types.PolicyRuleProtocolNetbirdSSH,
Action: types.PolicyTrafficActionAccept,
}},
}, true)
require.NoError(t, err)
// Does not ship it: an SSH rule that authorizes a specific group.
_, err = manager.SavePolicy(ctx, accountID, userID, &types.Policy{
Enabled: true,
Rules: []*types.PolicyRule{{
Enabled: true,
Sources: []string{groupIDs[2]},
Destinations: []string{groupIDs[3]},
Protocol: types.PolicyRuleProtocolNetbirdSSH,
Action: types.PolicyTrafficActionAccept,
AuthorizedGroups: map[string][]string{groupIDs[0]: {"root"}},
}},
}, true)
require.NoError(t, err)
result := resolveAffected(t, s, accountID, affectedpeers.Change{AllowedUsersChanged: true})
assert.ElementsMatch(t, []string{peerIDs[1]}, result,
"only the destinations of the rule shipping the allowed-user set refresh")
}
// TestAffectedPeers_SyncUserJWTGroups_OnlyAffectedPeersUpdated verifies that a JWT
// auto-group change updates only the user's peers and the peers linked to the changed
// group through policies, instead of fanning out to the whole account.
func TestAffectedPeers_SyncUserJWTGroups_OnlyAffectedPeersUpdated(t *testing.T) {
manager, updateManager, account, _, peer2, peer3 := setupNetworkMapTest(t)
ctx := context.Background()
accountID := account.Id
key, err := wgtypes.GeneratePrivateKey()
require.NoError(t, err)
userPeer, _, _, _, err := manager.AddPeer(ctx, accountID, "", userID, &nbpeer.Peer{
Key: key.PublicKey().String(),
Meta: nbpeer.PeerSystemMeta{Hostname: "user-peer"},
}, false)
require.NoError(t, err)
policies, err := manager.Store.GetAccountPolicies(ctx, store.LockingStrengthNone, accountID)
require.NoError(t, err)
for _, p := range policies {
require.NoError(t, manager.Store.DeletePolicy(ctx, accountID, p.ID))
}
account, err = manager.Store.GetAccount(ctx, accountID)
require.NoError(t, err)
account.Settings.JWTGroupsEnabled = true
account.Settings.JWTGroupsClaimName = "groups"
account.Settings.GroupsPropagationEnabled = true
require.NoError(t, manager.Store.SaveAccount(ctx, account))
require.NoError(t, manager.CreateGroup(ctx, accountID, userID, &types.Group{ID: "jwt-grp", Name: "jwt-linked", Issued: types.GroupIssuedJWT, Peers: []string{}}))
require.NoError(t, manager.CreateGroup(ctx, accountID, userID, &types.Group{ID: "jwt-dest", Name: "jwt-dest", Peers: []string{peer2.ID}}))
_, err = manager.SavePolicy(ctx, accountID, userID, &types.Policy{
Enabled: true,
Rules: []*types.PolicyRule{
{
Enabled: true,
Sources: []string{"jwt-grp"},
Destinations: []string{"jwt-dest"},
Bidirectional: true,
Action: types.PolicyTrafficActionAccept,
},
},
}, true)
require.NoError(t, err)
updUser := updateManager.CreateChannel(ctx, userPeer.ID)
upd2 := updateManager.CreateChannel(ctx, peer2.ID)
upd3 := updateManager.CreateChannel(ctx, peer3.ID)
t.Cleanup(func() {
updateManager.CloseChannel(ctx, userPeer.ID)
updateManager.CloseChannel(ctx, peer2.ID)
updateManager.CloseChannel(ctx, peer3.ID)
})
userAuth := auth.UserAuth{
AccountId: accountID,
UserId: userID,
Groups: []string{"jwt-linked"},
}
t.Run("adding JWT group updates only linked peers", func(t *testing.T) {
drainPeerUpdates(updUser)
drainPeerUpdates(upd2)
drainPeerUpdates(upd3)
require.NoError(t, manager.SyncUserJWTGroups(ctx, userAuth))
peerShouldReceiveUpdate(t, updUser)
peerShouldReceiveUpdate(t, upd2)
peerShouldNotReceiveUpdate(t, upd3)
user, err := manager.Store.GetUserByUserID(ctx, store.LockingStrengthNone, userID)
require.NoError(t, err)
assert.Contains(t, user.AutoGroups, "jwt-grp")
})
t.Run("removing JWT group updates only linked peers", func(t *testing.T) {
drainPeerUpdates(updUser)
drainPeerUpdates(upd2)
drainPeerUpdates(upd3)
userAuth.Groups = nil
require.NoError(t, manager.SyncUserJWTGroups(ctx, userAuth))
peerShouldReceiveUpdate(t, updUser)
peerShouldReceiveUpdate(t, upd2)
peerShouldNotReceiveUpdate(t, upd3)
user, err := manager.Store.GetUserByUserID(ctx, store.LockingStrengthNone, userID)
require.NoError(t, err)
assert.NotContains(t, user.AutoGroups, "jwt-grp")
})
}

View File

@@ -0,0 +1,170 @@
package server
import (
"context"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"golang.zx2c4.com/wireguard/wgctrl/wgtypes"
"github.com/netbirdio/netbird/management/server/activity"
nbpeer "github.com/netbirdio/netbird/management/server/peer"
"github.com/netbirdio/netbird/management/server/store"
"github.com/netbirdio/netbird/management/server/types"
)
// A user update refreshes only the peers its auto-group change reaches, and a user
// update that changes no group membership refreshes nobody.
func TestAffectedPeers_SaveUser_OnlyAffectedPeersUpdated(t *testing.T) {
manager, updateManager, account, _, peer2, peer3 := setupNetworkMapTest(t)
ctx := context.Background()
accountID := account.Id
const targetUserID = "target-user"
require.NoError(t, manager.Store.SaveUser(ctx, &types.User{
Id: targetUserID, AccountID: accountID, Role: types.UserRoleUser,
}))
key, err := wgtypes.GeneratePrivateKey()
require.NoError(t, err)
targetPeer, _, _, _, err := manager.AddPeer(ctx, accountID, "", targetUserID, &nbpeer.Peer{
Key: key.PublicKey().String(),
Meta: nbpeer.PeerSystemMeta{Hostname: "target-peer"},
}, false)
require.NoError(t, err)
policies, err := manager.Store.GetAccountPolicies(ctx, store.LockingStrengthNone, accountID)
require.NoError(t, err)
for _, p := range policies {
require.NoError(t, manager.Store.DeletePolicy(ctx, accountID, p.ID))
}
account, err = manager.Store.GetAccount(ctx, accountID)
require.NoError(t, err)
account.Settings.GroupsPropagationEnabled = true
require.NoError(t, manager.Store.SaveAccount(ctx, account))
require.NoError(t, manager.CreateGroup(ctx, accountID, userID, &types.Group{ID: "ug-linked", Name: "ug-linked"}))
require.NoError(t, manager.CreateGroup(ctx, accountID, userID, &types.Group{ID: "ug-dest", Name: "ug-dest", Peers: []string{peer2.ID}}))
_, err = manager.SavePolicy(ctx, accountID, userID, &types.Policy{
Enabled: true,
Rules: []*types.PolicyRule{
{
Enabled: true,
Sources: []string{"ug-linked"},
Destinations: []string{"ug-dest"},
Bidirectional: true,
Action: types.PolicyTrafficActionAccept,
},
},
}, true)
require.NoError(t, err)
updTarget := updateManager.CreateChannel(ctx, targetPeer.ID)
upd2 := updateManager.CreateChannel(ctx, peer2.ID)
upd3 := updateManager.CreateChannel(ctx, peer3.ID)
t.Cleanup(func() {
updateManager.CloseChannel(ctx, targetPeer.ID)
updateManager.CloseChannel(ctx, peer2.ID)
updateManager.CloseChannel(ctx, peer3.ID)
})
t.Run("auto group change updates only linked peers", func(t *testing.T) {
drainPeerUpdates(updTarget)
drainPeerUpdates(upd2)
drainPeerUpdates(upd3)
_, err := manager.SaveUser(ctx, accountID, activity.SystemInitiator, &types.User{
Id: targetUserID, AccountID: accountID, Role: types.UserRoleUser,
AutoGroups: []string{"ug-linked"},
})
require.NoError(t, err)
peerShouldReceiveUpdate(t, updTarget)
peerShouldReceiveUpdate(t, upd2)
peerShouldNotReceiveUpdate(t, upd3)
})
t.Run("update without group changes refreshes nobody", func(t *testing.T) {
drainPeerUpdates(updTarget)
drainPeerUpdates(upd2)
drainPeerUpdates(upd3)
_, err := manager.SaveUser(ctx, accountID, activity.SystemInitiator, &types.User{
Id: targetUserID, AccountID: accountID, Role: types.UserRoleUser,
AutoGroups: []string{"ug-linked"}, Name: "renamed",
})
require.NoError(t, err)
peerShouldNotReceiveUpdate(t, updTarget)
peerShouldNotReceiveUpdate(t, upd2)
peerShouldNotReceiveUpdate(t, upd3)
user, err := manager.Store.GetUserByUserID(ctx, store.LockingStrengthNone, targetUserID)
require.NoError(t, err)
assert.Equal(t, "renamed", user.Name)
})
t.Run("auto group change reassigning IPv6 refreshes the changed peers and their observers", func(t *testing.T) {
account, err := manager.Store.GetAccount(ctx, accountID)
require.NoError(t, err)
account.Settings.IPv6EnabledGroups = []string{"ug-v6"}
require.NoError(t, manager.Store.SaveAccount(ctx, account))
require.NoError(t, manager.CreateGroup(ctx, accountID, userID, &types.Group{ID: "ug-v6", Name: "ug-v6"}))
drainPeerUpdates(updTarget)
drainPeerUpdates(upd2)
drainPeerUpdates(upd3)
_, err = manager.SaveUser(ctx, accountID, activity.SystemInitiator, &types.User{
Id: targetUserID, AccountID: accountID, Role: types.UserRoleUser,
AutoGroups: []string{"ug-linked", "ug-v6"}, Name: "renamed",
})
require.NoError(t, err)
// The reassigned peer refreshes with everyone it can reach: peer2 via the
// policy, but not peer3, which shares no group or policy with it.
peerShouldReceiveUpdate(t, updTarget)
peerShouldReceiveUpdate(t, upd2)
peerShouldNotReceiveUpdate(t, upd3)
})
t.Run("unblocking a user refreshes only the SSH rule destinations", func(t *testing.T) {
// An SSH rule that authorizes no group of its own ships the account's
// allowed-user set to its destinations, so those are the peers an unblock
// reaches — not the whole account.
_, err := manager.SavePolicy(ctx, accountID, userID, &types.Policy{
Enabled: true,
Rules: []*types.PolicyRule{{
Enabled: true,
Sources: []string{"ug-linked"},
Destinations: []string{"ug-dest"},
Protocol: types.PolicyRuleProtocolNetbirdSSH,
Action: types.PolicyTrafficActionAccept,
}},
}, true)
require.NoError(t, err)
blocked, err := manager.Store.GetUserByUserID(ctx, store.LockingStrengthNone, targetUserID)
require.NoError(t, err)
blocked.Blocked = true
require.NoError(t, manager.Store.SaveUser(ctx, blocked))
drainPeerUpdates(updTarget)
drainPeerUpdates(upd2)
drainPeerUpdates(upd3)
// Same auto-groups as the previous subtest left them, so no group change and
// no IPv6 reconciliation interferes: the unblock alone drives the refresh.
_, err = manager.SaveUser(ctx, accountID, activity.SystemInitiator, &types.User{
Id: targetUserID, AccountID: accountID, Role: types.UserRoleUser,
AutoGroups: []string{"ug-linked", "ug-v6"}, Name: "renamed",
})
require.NoError(t, err)
peerShouldReceiveUpdate(t, upd2)
peerShouldNotReceiveUpdate(t, upd3)
})
}

View File

@@ -18,6 +18,7 @@ import (
"context"
log "github.com/sirupsen/logrus"
"golang.org/x/exp/maps"
nbdns "github.com/netbirdio/netbird/dns"
rpservice "github.com/netbirdio/netbird/management/internals/modules/reverseproxy/service"
@@ -83,7 +84,7 @@ func (snap *Snapshot) loadCollections(ctx context.Context, s store.Store, accoun
hasGroupOrPeerChange := len(c.ChangedGroupIDs) > 0 || len(c.ChangedPeerIDs) > 0 || len(c.LinkGroups) > 0 || len(c.Resources) > 0
hasNetworkObject := len(c.Routers) > 0 || len(c.Resources) > 0 || len(c.Networks) > 0
// the resource<->router bridge can fire for any of these
needsRoutersResources := hasGroupOrPeerChange || len(c.PostureCheckIDs) > 0 || len(c.Policies) > 0 || hasNetworkObject
needsRoutersResources := hasGroupOrPeerChange || len(c.PostureCheckIDs) > 0 || len(c.Policies) > 0 || hasNetworkObject || len(c.UserGroupIDs) > 0 || c.AllowedUsersChanged
if needsRoutersResources {
if err := snap.loadPolicyRoutersResources(ctx, s, accountID); err != nil {
@@ -219,6 +220,18 @@ type Change struct {
// (correct when the peer's own attributes changed, e.g. IP/status).
OutputPeerIDs []string
// UserGroupIDs are groups whose USER membership changed (a user's auto-groups),
// as opposed to their peer membership. Peers ship the group -> user mapping only
// for the groups an SSH rule authorizes, so these refresh the destinations of the
// SSH rules authorizing them — independently of any peer moving between groups.
UserGroupIDs []string
// AllowedUsersChanged marks a change to the set of users allowed to open SSH
// sessions — a user was created, blocked or unblocked. That set is account-wide,
// and peers receive it through the SSH rules that name no group or user of their
// own, so those rules' destinations refresh.
AllowedUsersChanged bool
// LinkGroups are groups used ONLY to match policies/routes/routers and walk to the
// OPPOSITE side — they are never expanded to their own members. Use this when a
// peer's group membership changed: pass the peer in ChangedPeerIDs and its
@@ -240,6 +253,8 @@ func (c Change) isEmpty() bool {
len(c.Resources) == 0 &&
len(c.Networks) == 0 &&
len(c.PostureCheckIDs) == 0 &&
len(c.UserGroupIDs) == 0 &&
!c.AllowedUsersChanged &&
len(c.DistributionGroupIDs) == 0 &&
len(c.RemovedPeersByGroup) == 0 &&
len(c.LinkGroups) == 0 &&
@@ -359,6 +374,9 @@ func (r *resolver) walk() {
r.collectFromProxyServices()
}
r.collectFromSSHAuthorizedGroups()
r.collectFromAllowedUsers()
r.collectFromChangedRoutes(r.change.Routes)
r.collectFromChangedRouters(r.change.Routers)
r.collectFromChangedResources(r.change.Resources)
@@ -811,6 +829,59 @@ func (r *resolver) collectFromNameServers() {
}
}
// collectFromSSHAuthorizedGroups folds the destinations of the enabled SSH rules that
// authorize a group whose user membership changed. Those destination peers carry the
// group -> user mapping for the groups they authorize, so they refresh even when no
// peer moved between groups.
func (r *resolver) collectFromSSHAuthorizedGroups() {
if len(r.change.UserGroupIDs) == 0 {
return
}
changed := toSet(r.change.UserGroupIDs)
for _, policy := range r.policies() {
for _, rule := range policy.Rules {
if !rule.Enabled || rule.Protocol != types.PolicyRuleProtocolNetbirdSSH {
continue
}
if !anyInSet(maps.Keys(rule.AuthorizedGroups), changed) {
continue
}
log.WithContext(r.ctx).Tracef("collectFromSSHAuthorizedGroups: rule %s authorizes a changed user group -> folding its destinations", rule.ID)
r.foldPolicySideForRule(policy, rule, sideDestination)
}
}
}
// collectFromAllowedUsers folds the destinations of the rules that make a peer carry
// the account's allowed-user set, for a change to who is in that set.
func (r *resolver) collectFromAllowedUsers() {
if !r.change.AllowedUsersChanged {
return
}
for _, policy := range r.policies() {
for _, rule := range policy.Rules {
if !rule.Enabled || !ruleShipsAllowedUsers(rule) {
continue
}
log.WithContext(r.ctx).Tracef("collectFromAllowedUsers: rule %s ships the allowed-user set -> folding its destinations", rule.ID)
r.foldPolicySideForRule(policy, rule, sideDestination)
}
}
}
// ruleShipsAllowedUsers reports whether a rule makes its destination peers carry the
// account's allowed-user set. It mirrors the network map's SSH requirements except for
// the destination peer's own SSH flag, which the snapshot does not hold — so it folds a
// superset and never misses a peer.
func ruleShipsAllowedUsers(rule *types.PolicyRule) bool {
if rule.Protocol == types.PolicyRuleProtocolNetbirdSSH {
return len(rule.AuthorizedGroups) == 0 && rule.AuthorizedUser == ""
}
return types.PolicyRuleImpliesLegacySSH(rule)
}
func (r *resolver) collectFromDNSSettings() {
if len(r.linkGroups) == 0 || r.snap.dnsSettings == nil {
return

View File

@@ -85,6 +85,8 @@ func TestChangeIsEmpty(t *testing.T) {
assert.False(t, Change{Resources: []*resourceTypes.NetworkResource{{ID: "r"}}}.isEmpty())
assert.False(t, Change{Networks: []*networkTypes.Network{{ID: "n"}}}.isEmpty())
assert.False(t, Change{PostureCheckIDs: []string{"pc"}}.isEmpty())
assert.False(t, Change{UserGroupIDs: []string{"g"}}.isEmpty())
assert.False(t, Change{AllowedUsersChanged: true}.isEmpty())
}
func TestPolicyReferencesPostureChecks(t *testing.T) {

View File

@@ -2,6 +2,7 @@ package proxy
import (
"context"
"errors"
"net"
"net/http"
"net/netip"
@@ -108,7 +109,7 @@ func (h *AuthCallbackHandler) handleCallback(w http.ResponseWriter, r *http.Requ
redirectURL.Scheme = "https"
query := redirectURL.Query()
query.Set("error", "access_denied")
query.Set("error_description", "Service configuration error")
query.Set("error_description", sessionTokenErrorDescription(err))
redirectURL.RawQuery = query.Encode()
http.Redirect(w, r, redirectURL.String(), http.StatusFound)
return
@@ -124,6 +125,20 @@ func (h *AuthCallbackHandler) handleCallback(w http.ResponseWriter, r *http.Requ
http.Redirect(w, r, redirectURL.String(), http.StatusFound)
}
// sessionTokenErrorDescription maps a session token failure to the text the
// proxy renders on its access denied page. Account status denials get a message
// the user can act on, while everything else stays generic so a lookup or
// signing failure does not describe management internals to the browser.
func sessionTokenErrorDescription(err error) string {
if errors.Is(err, nbgrpc.ErrUserPendingApproval) {
return "Your account is pending approval by an administrator"
}
if errors.Is(err, nbgrpc.ErrUserBlocked) {
return "Your account is blocked"
}
return "Service configuration error"
}
func extractUserIDFromToken(ctx context.Context, provider *oidc.Provider, config nbgrpc.ProxyOIDCConfig, token *oauth2.Token) string {
rawIDToken, ok := token.Extra("id_token").(string)
if !ok {

View File

@@ -19,6 +19,7 @@ import (
"github.com/stretchr/testify/require"
"github.com/netbirdio/netbird/management/internals/modules/reverseproxy/accesslogs"
activitymanager "github.com/netbirdio/netbird/management/internals/modules/reverseproxy/activity/manager"
nbproxy "github.com/netbirdio/netbird/management/internals/modules/reverseproxy/proxy"
"github.com/netbirdio/netbird/management/internals/modules/reverseproxy/service"
nbgrpc "github.com/netbirdio/netbird/management/internals/shared/grpc"
@@ -221,6 +222,7 @@ func setupAuthCallbackTest(t *testing.T) *testSetup {
)
proxyService.SetServiceManager(&testServiceManager{store: testStore})
proxyService.SetActivityManager(activitymanager.NewManager(testStore))
handler := NewAuthCallbackHandler(proxyService, nil)
@@ -360,6 +362,51 @@ func createTestAccountsAndUsers(t *testing.T, ctx context.Context, testStore sto
Issued: "api",
}
require.NoError(t, testStore.SaveUser(ctx, allowedUser))
// A second tenant, whose users must never be issued a token signed with
// the first tenant's service session key.
otherAccount := &types.Account{
Id: "otherAccountId",
Domain: "other.com",
DomainCategory: "private",
IsDomainPrimaryAccount: true,
CreatedAt: time.Now(),
}
require.NoError(t, testStore.SaveAccount(ctx, otherAccount))
otherAccountUser := &types.User{
Id: "otherAccountUserId",
AccountID: "otherAccountId",
Role: types.UserRoleUser,
CreatedAt: time.Now(),
Issued: "api",
}
require.NoError(t, testStore.SaveUser(ctx, otherAccountUser))
// A user awaiting approval is stored as blocked and pending approval, and
// carries the same group membership as the approved one.
pendingUser := &types.User{
Id: "pendingUserId",
AccountID: "testAccountId",
Role: types.UserRoleUser,
AutoGroups: []string{"allowedGroupId"},
Blocked: true,
PendingApproval: true,
CreatedAt: time.Now(),
Issued: "api",
}
require.NoError(t, testStore.SaveUser(ctx, pendingUser))
blockedUser := &types.User{
Id: "blockedUserId",
AccountID: "testAccountId",
Role: types.UserRoleUser,
AutoGroups: []string{"allowedGroupId"},
Blocked: true,
CreatedAt: time.Now(),
Issued: "api",
}
require.NoError(t, testStore.SaveUser(ctx, blockedUser))
}
// testServiceManager is a minimal implementation for testing.
@@ -490,6 +537,113 @@ func TestAuthCallback_UserAllowedToLogin(t *testing.T) {
require.Empty(t, parsedLocation.Query().Get("error"), "Should not have error parameter")
}
// 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
subject string
expectErrorDesc string
}{
{
name: "pending approval",
subject: "pendingUserId",
expectErrorDesc: "Your account is pending approval by an administrator",
},
{
name: "blocked",
subject: "blockedUserId",
expectErrorDesc: "Your account is blocked",
},
{
name: "unknown to management",
subject: "userMissingFromStoreId",
expectErrorDesc: "Service configuration error",
},
{
// The account topology stays out of the browser-visible message.
name: "belongs to another account",
subject: "otherAccountUserId",
expectErrorDesc: "Service configuration error",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
setup := setupAuthCallbackTest(t)
defer setup.cleanup()
setup.oidcServer.tokenSubject = tt.subject
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)
parsedLocation, err := url.Parse(rec.Header().Get("Location"))
require.NoError(t, err)
require.Empty(t, parsedLocation.Query().Get("session_token"), "Denied user must not receive a session token")
require.Equal(t, "access_denied", parsedLocation.Query().Get("error"))
require.Equal(t, tt.expectErrorDesc, parsedLocation.Query().Get("error_description"))
})
}
}
func TestAuthCallback_ProxyNotFound(t *testing.T) {
setup := setupAuthCallbackTest(t)
defer setup.cleanup()

View File

@@ -599,6 +599,34 @@ 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.
//
// LastSeen comes from the database clock for the same reason it does there: a
// Go-side timestamp is taken before the write and can land after a connect that
// used CURRENT_TIMESTAMP, dragging the column backwards.
//
// staleBefore carries the caller's throttle into the same statement, so
// concurrent requests for one peer collapse into a single write instead of
// each racing on its own stale read. The column is nullable — Status is an
// embedded pointer, so a peer stored without one leaves it NULL — and NULL
// loses every comparison, hence the explicit branch for a peer never seen.
func (s *SqlStore) RefreshPeerLastSeen(ctx context.Context, accountID, peerID string, staleBefore time.Time) (bool, error) {
result := s.db.WithContext(ctx).
Model(&nbpeer.Peer{}).
Where(accountAndIDQueryCondition, accountID, peerID).
Where("(peer_status_last_seen IS NULL OR peer_status_last_seen < ?)", staleBefore).
Update("peer_status_last_seen", gorm.Expr("CURRENT_TIMESTAMP"))
if result.Error != nil {
return false, status.Errorf(status.Internal, "refresh peer last seen: %v", result.Error)
}
return result.RowsAffected > 0, 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 {

View File

@@ -0,0 +1,122 @@
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 TestRefreshPeerLastSeen(t *testing.T) {
ctx := context.Background()
store := newActivityTestStore(t)
stored := time.Now().UTC().Add(-3 * time.Hour)
require.NoError(t, store.AddPeerToAccount(ctx, activityPeer(stored)))
refreshed, err := store.RefreshPeerLastSeen(ctx, activityAccountID, "activityPeer", time.Now().UTC().Add(-time.Hour))
require.NoError(t, err)
assert.True(t, refreshed, "a peer seen three hours ago is stale enough to refresh")
peer, err := store.GetPeerByID(ctx, LockingStrengthNone, activityAccountID, "activityPeer")
require.NoError(t, err)
assert.WithinDuration(t, time.Now().UTC(), peer.Status.LastSeen.UTC(), time.Minute, "last seen should be stamped at write time")
assert.True(t, peer.Status.LastSeen.After(stored), "last seen must move forward")
}
// TestRefreshPeerLastSeenHonoursCutoff covers the throttle the caller relies on:
// two concurrent requests both read the same stale peer, but only the statement
// that still finds LastSeen behind the cutoff writes.
func TestRefreshPeerLastSeenHonoursCutoff(t *testing.T) {
ctx := context.Background()
store := newActivityTestStore(t)
stored := time.Now().UTC().Add(-10 * time.Minute)
require.NoError(t, store.AddPeerToAccount(ctx, activityPeer(stored)))
refreshed, err := store.RefreshPeerLastSeen(ctx, activityAccountID, "activityPeer", time.Now().UTC().Add(-time.Hour))
require.NoError(t, err)
assert.False(t, refreshed, "a peer seen inside the interval must not be written")
peer, err := store.GetPeerByID(ctx, LockingStrengthNone, activityAccountID, "activityPeer")
require.NoError(t, err)
assert.WithinDuration(t, stored, peer.Status.LastSeen.UTC(), time.Second, "last seen must be left where it was")
}
// TestRefreshPeerLastSeenRecordsNeverSeenPeer covers the nullable column. Status
// is an embedded pointer, so a peer stored without one leaves last seen NULL,
// and NULL loses the cutoff comparison — such a peer would never record its
// first activity.
func TestRefreshPeerLastSeenRecordsNeverSeenPeer(t *testing.T) {
ctx := context.Background()
store := newActivityTestStore(t)
stored := activityPeer(time.Time{})
stored.Status = nil
require.NoError(t, store.AddPeerToAccount(ctx, stored))
refreshed, err := store.RefreshPeerLastSeen(ctx, activityAccountID, "activityPeer", time.Now().UTC().Add(-time.Hour))
require.NoError(t, err)
assert.True(t, refreshed, "a peer that was never seen must record its first activity")
peer, err := store.GetPeerByID(ctx, LockingStrengthNone, activityAccountID, "activityPeer")
require.NoError(t, err)
assert.WithinDuration(t, time.Now().UTC(), peer.Status.LastSeen.UTC(), time.Minute, "last seen should be stamped at write time")
}
// 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. This is why
// SavePeerStatus is not reused for an activity bump.
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))
refreshed, err := store.RefreshPeerLastSeen(ctx, activityAccountID, "activityPeer", time.Now().UTC().Add(-time.Hour))
require.NoError(t, err)
require.True(t, refreshed, "the peer is stale enough to refresh")
peer, err := store.GetPeerByID(ctx, LockingStrengthNone, activityAccountID, "activityPeer")
require.NoError(t, err)
assert.WithinDuration(t, time.Now().UTC(), peer.Status.LastSeen.UTC(), time.Minute, "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},
}
}

View File

@@ -180,6 +180,14 @@ 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 just seen, stamping the
// database clock like the other status writers. Connected and
// SessionStartedAt are left alone, so this never interferes with the
// session-ownership protocol MarkPeerConnectedIfNewerSession implements.
// The write only lands when the stored LastSeen is older than
// staleBefore, which keeps a caller's throttle atomic under concurrent
// requests for the same peer. Returns true when the update happened.
RefreshPeerLastSeen(ctx context.Context, accountID, peerID string, staleBefore time.Time) (bool, error)
// MarkPeerDisconnectedIfSameSession sets the peer to disconnected and
// resets SessionStartedAt to zero, but only when the stored
// SessionStartedAt equals the given sessionStartedAt. LastSeen is

View File

@@ -3203,6 +3203,21 @@ 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, staleBefore time.Time) (bool, error) {
m.ctrl.T.Helper()
ret := m.ctrl.Call(m, "RefreshPeerLastSeen", ctx, accountID, peerID, staleBefore)
ret0, _ := ret[0].(bool)
ret1, _ := ret[1].(error)
return ret0, ret1
}
// RefreshPeerLastSeen indicates an expected call of RefreshPeerLastSeen.
func (mr *MockStoreMockRecorder) RefreshPeerLastSeen(ctx, accountID, peerID, staleBefore interface{}) *gomock.Call {
mr.mock.ctrl.T.Helper()
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RefreshPeerLastSeen", reflect.TypeOf((*MockStore)(nil).RefreshPeerLastSeen), ctx, accountID, peerID, staleBefore)
}
// RemovePeerFromAllGroups mocks base method.
func (m *MockStore) RemovePeerFromAllGroups(ctx context.Context, peerID string) error {
m.ctrl.T.Helper()

View File

@@ -91,6 +91,8 @@ type Account struct {
Onboarding AccountOnboarding `gorm:"foreignKey:AccountID;references:id;constraint:OnDelete:CASCADE"`
ReverseProxyFreeDomainNonce string
PostureValidation map[string]map[string]bool `gorm:"-"`
}
// this class is used by gorm only
@@ -874,6 +876,7 @@ func (a *Account) Copy() *Account {
Services: services,
Onboarding: a.Onboarding,
Domains: domains,
PostureValidation: a.PostureValidation,
}
}

View File

@@ -10,6 +10,8 @@ import (
nbdns "github.com/netbirdio/netbird/dns"
"github.com/netbirdio/netbird/management/internals/modules/zones"
routerTypes "github.com/netbirdio/netbird/management/server/networks/routers/types"
nbpeer "github.com/netbirdio/netbird/management/server/peer"
"github.com/netbirdio/netbird/management/server/posture"
"github.com/netbirdio/netbird/management/server/telemetry"
"github.com/netbirdio/netbird/route"
)
@@ -506,8 +508,8 @@ func (a *Account) getPeersGroupsPoliciesRoutes(
func (a *Account) getPeersFromGroups(ctx context.Context, groups []string, peerID string, sourcePostureChecksIDs []string,
validatedPeersMap map[string]struct{}, postureFailedPeers *map[string]map[string]struct{}) ([]string, bool) {
peerInGroups := false
filteredPeerIDs := make([]string, 0, len(groups))
seenPeerIds := make(map[string]struct{}, len(groups))
var filteredPeerIDs []string
var seenPeerIds map[string]struct{}
for _, gid := range groups {
group := a.GetGroup(gid)
@@ -547,6 +549,17 @@ func (a *Account) getPeersFromGroups(ctx context.Context, groups []string, peerI
return filteredPeerIDs, peerInGroups
}
if seenPeerIds == nil {
totalGroupPeers := 0
for _, g := range groups {
if grp := a.GetGroup(g); grp != nil {
totalGroupPeers += len(grp.Peers)
}
}
filteredPeerIDs = make([]string, 0, totalGroupPeers)
seenPeerIds = make(map[string]struct{}, totalGroupPeers)
}
for _, pid := range group.Peers {
if _, seen := seenPeerIds[pid]; seen {
continue
@@ -589,21 +602,109 @@ func (a *Account) validatePostureChecksOnPeerGetFailed(ctx context.Context, sour
}
for _, postureChecksID := range sourcePostureChecksID {
if valid, cached := a.cachedPostureCheckResult(postureChecksID, peerID); cached {
if !valid {
return false, postureChecksID
}
continue
}
postureChecks := a.GetPostureChecks(postureChecksID)
if postureChecks == nil {
continue
}
for _, check := range postureChecks.GetChecks() {
isValid, _ := check.Check(ctx, *peer)
if !isValid {
return false, postureChecksID
}
if !peerPassesPostureChecks(ctx, postureChecks.GetChecks(), peer) {
return false, postureChecksID
}
}
return true, ""
}
// PrecomputePostureValidation evaluates every posture check referenced by an enabled
// policy once against the peers of that policy's source groups and stores the results,
// so the per-peer network map calculations that follow look them up instead of
// re-evaluating checks for every peer pair. It must be called before the account is
// shared across goroutines; lookups not covered by the precomputed results fall back
// to direct evaluation.
func (a *Account) PrecomputePostureValidation(ctx context.Context) {
if len(a.PostureChecks) == 0 {
a.PostureValidation = nil
return
}
checkPeerIDs := make(map[string]map[string]struct{})
for _, policy := range a.Policies {
if !policy.Enabled || len(policy.SourcePostureChecks) == 0 {
continue
}
peerIDs := a.getUniquePeerIDsFromGroupsIDs(ctx, policy.SourceGroups())
for _, rule := range policy.Rules {
if rule.SourceResource.Type == ResourceTypePeer && rule.SourceResource.ID != "" {
peerIDs = append(peerIDs, rule.SourceResource.ID)
}
}
for _, postureChecksID := range policy.SourcePostureChecks {
set := checkPeerIDs[postureChecksID]
if set == nil {
set = make(map[string]struct{}, len(peerIDs))
checkPeerIDs[postureChecksID] = set
}
for _, pid := range peerIDs {
set[pid] = struct{}{}
}
}
}
results := make(map[string]map[string]bool, len(checkPeerIDs))
for postureChecksID, peerIDs := range checkPeerIDs {
results[postureChecksID] = a.evaluatePostureChecksForPeers(ctx, postureChecksID, peerIDs)
}
a.PostureValidation = results
}
func (a *Account) evaluatePostureChecksForPeers(ctx context.Context, postureChecksID string, peerIDs map[string]struct{}) map[string]bool {
postureChecks := a.GetPostureChecks(postureChecksID)
if postureChecks == nil {
return nil
}
checks := postureChecks.GetChecks()
results := make(map[string]bool, len(peerIDs))
for peerID := range peerIDs {
peer, ok := a.Peers[peerID]
if !ok || peer == nil {
continue
}
results[peerID] = peerPassesPostureChecks(ctx, checks, peer)
}
return results
}
func (a *Account) cachedPostureCheckResult(postureChecksID, peerID string) (bool, bool) {
results, ok := a.PostureValidation[postureChecksID]
if !ok {
return false, false
}
if results == nil {
return true, true
}
valid, found := results[peerID]
return valid, found
}
func peerPassesPostureChecks(ctx context.Context, checks []posture.Check, peer *nbpeer.Peer) bool {
for _, check := range checks {
isValid, _ := check.Check(ctx, *peer)
if !isValid {
return false
}
}
return true
}
func (a *Account) getPostureValidPeersSaveFailed(inputPeers []string, postureChecksIDs []string, validatedPeersMap map[string]struct{}, postureFailedPeers *map[string]map[string]struct{}) []string {
var dest []string
for _, peerID := range inputPeers {

View File

@@ -0,0 +1,72 @@
package types_test
import (
"context"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
nbdns "github.com/netbirdio/netbird/dns"
"github.com/netbirdio/netbird/management/server/posture"
)
func TestPrecomputePostureValidation_MatchesDirectEvaluation(t *testing.T) {
account, validatedPeers := scalableTestAccount(60, 5)
account.PostureChecks = append(account.PostureChecks, &posture.Checks{
ID: "posture-check-strict", Name: "Strict version",
Checks: posture.ChecksDefinition{
NBVersionCheck: &posture.NBVersionCheck{MinVersion: "0.50.0"},
},
})
account.Policies[0].SourcePostureChecks = []string{"posture-check-ver", "posture-check-unknown"}
account.Policies[1].SourcePostureChecks = []string{"posture-check-strict"}
account.Policies[2].SourcePostureChecks = []string{"posture-check-ver"}
account.Policies[2].Enabled = false
ctx := context.Background()
resourcePolicies := account.GetResourcePoliciesMap()
routers := account.GetResourceRoutersMap()
type result struct {
peers map[string]struct{}
postureFailedPeers map[string]map[string]struct{}
}
snapshot := func() map[string]result {
results := make(map[string]result, len(account.Peers))
for peerID := range account.Peers {
components := account.GetPeerNetworkMapComponents(ctx, peerID, nbdns.CustomZone{}, nil, validatedPeers, resourcePolicies, routers, nil)
require.NotNil(t, components)
peerSet := make(map[string]struct{}, len(components.Peers))
for id := range components.Peers {
peerSet[id] = struct{}{}
}
results[peerID] = result{peers: peerSet, postureFailedPeers: components.PostureFailedPeers}
}
return results
}
direct := snapshot()
account.PrecomputePostureValidation(ctx)
memoized := snapshot()
require.Equal(t, len(direct), len(memoized))
for peerID, want := range direct {
got := memoized[peerID]
assert.Equal(t, want.peers, got.peers, "visible peers changed for %s", peerID)
assert.Equal(t, want.postureFailedPeers, got.postureFailedPeers, "posture failed peers changed for %s", peerID)
}
}
func TestPrecomputePostureValidation_NoPostureChecks(t *testing.T) {
account, validatedPeers := scalableTestAccount(10, 2)
account.PostureChecks = nil
ctx := context.Background()
account.PrecomputePostureValidation(ctx)
components := account.GetPeerNetworkMapComponents(ctx, "peer-0", nbdns.CustomZone{}, nil, validatedPeers, account.GetResourcePoliciesMap(), account.GetResourceRoutersMap(), nil)
require.NotNil(t, components)
assert.NotEmpty(t, components.Peers)
}

View File

@@ -86,6 +86,43 @@ func BenchmarkNetworkMapGeneration_AllPeers(b *testing.B) {
b.ReportAllocs()
b.ResetTimer()
for range b.N {
account.PrecomputePostureValidation(ctx)
for _, peerID := range peerIDs {
_ = account.GetPeerNetworkMapFromComponents(ctx, peerID, nbdns.CustomZone{}, nil, validatedPeers, resourcePolicies, routers, nil, groupIDToUserIDs)
}
}
})
}
}
// BenchmarkNetworkMapGeneration_AllPeersPostureChecks benchmarks the UpdateAccountPeers
// hot path with a posture check attached to the account-wide policy, so posture
// validation runs for every source peer of every target peer's map.
func BenchmarkNetworkMapGeneration_AllPeersPostureChecks(b *testing.B) {
skipCIBenchmark(b)
scales := []benchmarkScale{
{"500peers_20groups", 500, 20},
{"1000peers_50groups", 1000, 50},
}
for _, scale := range scales {
account, validatedPeers := scalableTestAccount(scale.peers, scale.groups)
account.Policies[0].SourcePostureChecks = []string{"posture-check-ver"}
ctx := context.Background()
peerIDs := make([]string, 0, len(account.Peers))
for peerID := range account.Peers {
peerIDs = append(peerIDs, peerID)
}
b.Run("components/"+scale.name, func(b *testing.B) {
resourcePolicies := account.GetResourcePoliciesMap()
routers := account.GetResourceRoutersMap()
groupIDToUserIDs := account.GetActiveGroupUsers()
b.ReportAllocs()
b.ResetTimer()
for range b.N {
account.PrecomputePostureValidation(ctx)
for _, peerID := range peerIDs {
_ = account.GetPeerNetworkMapFromComponents(ctx, peerID, nbdns.CustomZone{}, nil, validatedPeers, resourcePolicies, routers, nil, groupIDToUserIDs)
}

View File

@@ -593,7 +593,8 @@ func (am *DefaultAccountManager) SaveOrAddUsers(ctx context.Context, accountID,
return nil, err
}
var updateAccountPeers bool
var snaps []*affectedpeers.Snapshot
var changes []affectedpeers.Change
var peersToExpire []*nbpeer.Peer
var addUserEvents []func()
var usersToSave = make([]*types.User, 0, len(updates))
@@ -629,20 +630,25 @@ func (am *DefaultAccountManager) SaveOrAddUsers(ctx context.Context, accountID,
}
err = am.Store.ExecuteInTransaction(ctx, func(transaction store.Store) error {
_, updatedUser, userPeersToExpire, userEvents, err := am.processUserUpdate(
change, updatedUser, userPeersToExpire, userEvents, err := am.processUserUpdate(
ctx, transaction, groupsMap, accountID, initiatorUserID, initiatorUser, update, addIfNotExists, settings,
)
if err != nil {
return fmt.Errorf("failed to process update for user %s: %w", update.Id, err)
}
updateAccountPeers = true
err = transaction.SaveUser(ctx, updatedUser)
if err != nil {
return fmt.Errorf("failed to save updated user %s: %w", update.Id, err)
}
snap, err := affectedpeers.Load(ctx, transaction, accountID, change)
if err != nil {
return err
}
snaps = append(snaps, snap)
changes = append(changes, change)
usersToSave = append(usersToSave, updatedUser)
addUserEvents = append(addUserEvents, userEvents...)
peersToExpire = append(peersToExpire, userPeersToExpire...)
@@ -683,11 +689,11 @@ func (am *DefaultAccountManager) SaveOrAddUsers(ctx context.Context, accountID,
log.WithContext(ctx).Errorf("failed update expired peers: %s", err)
return nil, err
}
} else if updateAccountPeers {
} else if len(usersToSave) > 0 {
if err = am.Store.IncrementNetworkSerial(ctx, accountID); err != nil {
return nil, fmt.Errorf("failed to increment network serial: %w", err)
}
am.UpdateAccountPeers(ctx, accountID, types.UpdateReason{Resource: types.UpdateResourceUser, Operation: types.UpdateOperationUpdate})
go am.dispatchAffected(ctx, accountID, snaps, changes)
}
return updatedUsersInfo, globalErr
@@ -759,19 +765,21 @@ func (am *DefaultAccountManager) prepareUserUpdateEvents(ctx context.Context, ac
}
func (am *DefaultAccountManager) processUserUpdate(ctx context.Context, transaction store.Store, groupsMap map[string]*types.Group,
accountID, initiatorUserId string, initiatorUser, update *types.User, addIfNotExists bool, settings *types.Settings) (bool, *types.User, []*nbpeer.Peer, []func(), error) {
accountID, initiatorUserId string, initiatorUser, update *types.User, addIfNotExists bool, settings *types.Settings) (affectedpeers.Change, *types.User, []*nbpeer.Peer, []func(), error) {
var change affectedpeers.Change
if update == nil {
return false, nil, nil, nil, status.Errorf(status.InvalidArgument, "provided user update is nil")
return change, nil, nil, nil, status.Errorf(status.InvalidArgument, "provided user update is nil")
}
oldUser, isNewUser, err := getUserOrCreateIfNotExists(ctx, transaction, accountID, update, addIfNotExists)
if err != nil {
return false, nil, nil, nil, err
return change, nil, nil, nil, err
}
if err := validateUserUpdate(groupsMap, initiatorUser, oldUser, update); err != nil {
return false, nil, nil, nil, err
return change, nil, nil, nil, err
}
// only auto groups, revoked status, and integration reference can be updated for now
@@ -792,13 +800,13 @@ func (am *DefaultAccountManager) processUserUpdate(ctx context.Context, transact
var transferredOwnerRole bool
result, err := handleOwnerRoleTransfer(ctx, transaction, initiatorUser, update)
if err != nil {
return false, nil, nil, nil, err
return change, nil, nil, nil, err
}
transferredOwnerRole = result
userPeers, err := transaction.GetUserPeers(ctx, store.LockingStrengthNone, updatedUser.AccountID, update.Id)
if err != nil {
return false, nil, nil, nil, err
return change, nil, nil, nil, err
}
var peersToExpire []*nbpeer.Peer
@@ -807,6 +815,32 @@ func (am *DefaultAccountManager) processUserUpdate(ctx context.Context, transact
peersToExpire = userPeers
}
// A user reaches a peer's network map only through the SSH rules: as part of a
// group -> user mapping, and as part of the account's allowed-user set. Creating,
// blocking or unblocking a user adds it to or removes it from both, so every group
// it maps into changes — including the All group that holds every active user.
// Otherwise only the auto-groups it joined or left do.
if isNewUser || oldUser.IsBlocked() != updatedUser.IsBlocked() {
change.AllowedUsersChanged = true
change.UserGroupIDs = slices.Concat(oldUser.AutoGroups, updatedUser.AutoGroups, allGroupIDs(groupsMap))
} else {
change.UserGroupIDs = slices.Concat(
util.Difference(oldUser.AutoGroups, updatedUser.AutoGroups),
util.Difference(updatedUser.AutoGroups, oldUser.AutoGroups),
)
}
// The user's peers are the changed entity in every scenario the update can
// produce — group membership, IPv6 assignment, SSH mappings — so they refresh
// together with every peer they can connect to, like on a regular peer update.
// An update that changes neither the auto-groups nor the active-user set has no
// peer-visible effect and refreshes nobody.
if len(change.UserGroupIDs) > 0 || change.AllowedUsersChanged {
for _, peer := range userPeers {
change.ChangedPeerIDs = append(change.ChangedPeerIDs, peer.ID)
}
}
var removedGroups, addedGroups []string
if update.AutoGroups != nil && settings.GroupsPropagationEnabled {
removedGroups = util.Difference(oldUser.AutoGroups, update.AutoGroups)
@@ -814,26 +848,38 @@ func (am *DefaultAccountManager) processUserUpdate(ctx context.Context, transact
for _, peer := range userPeers {
for _, groupID := range removedGroups {
if err := transaction.RemovePeerFromGroup(ctx, peer.ID, groupID); err != nil {
return false, nil, nil, nil, fmt.Errorf("failed to remove peer %s from group %s: %w", peer.ID, groupID, err)
return change, nil, nil, nil, fmt.Errorf("failed to remove peer %s from group %s: %w", peer.ID, groupID, err)
}
}
for _, groupID := range addedGroups {
if err := transaction.AddPeerToGroup(ctx, accountID, peer.ID, groupID); err != nil {
return false, nil, nil, nil, fmt.Errorf("failed to add peer %s to group %s: %w", peer.ID, groupID, err)
return change, nil, nil, nil, fmt.Errorf("failed to add peer %s to group %s: %w", peer.ID, groupID, err)
}
}
}
allGroupChanges := slices.Concat(removedGroups, addedGroups)
change.LinkGroups = allGroupChanges
if err := am.reconcileIPv6ForGroupChanges(ctx, transaction, accountID, allGroupChanges); err != nil {
return false, nil, nil, nil, fmt.Errorf("reconcile IPv6 for group changes: %w", err)
return change, nil, nil, nil, fmt.Errorf("reconcile IPv6 for group changes: %w", err)
}
}
updateAccountPeers := len(userPeers) > 0
userEventsToAdd := am.prepareUserUpdateEvents(ctx, updatedUser.AccountID, initiatorUserId, oldUser, updatedUser, transferredOwnerRole, isNewUser, removedGroups, addedGroups, transaction)
return updateAccountPeers, updatedUser, peersToExpire, userEventsToAdd, nil
return change, updatedUser, peersToExpire, userEventsToAdd, nil
}
// allGroupIDs returns the ID of the account's All group, which every active user maps
// into, as a slice so callers can concatenate it.
func allGroupIDs(groupsMap map[string]*types.Group) []string {
for _, group := range groupsMap {
if group.IsGroupAll() {
return []string{group.ID}
}
}
return nil
}
// getUserOrCreateIfNotExists retrieves the existing user or creates a new one if it doesn't exist.