policy/v2: refresh clients when a user change drops the last via grant

SetUsers read needsPerNodeFilter after the recompile cleared it, so
the change was reported as empty and clients kept the via rules.
This commit is contained in:
Kristoffer Dalby committed 2026-10-28 19:06:04 +00:00
1 parent 1b724a5bb9
commit f6ec374c23
3 files changed
+45 -10

No files matched your search

+6 -1
View File
@@ -981,6 +981,10 @@ func (pm *PolicyManager) SetUsers(users []types.User) (bool, bool, error) {
prev := pm.users
pm.users = users
// A via grant left without sources is not compiled, so the recompile
// can clear the flag while clients still hold its rules.
hadPerNodeFilter := pm.needsPerNodeFilter
// SSH policies and autogroup:self sources resolve users by name, and
// the self sources are outside the filter hash, so updateLocked can
// report no change while per-node results moved.
@@ -1000,7 +1004,8 @@ func (pm *PolicyManager) SetUsers(users []types.User) (bool, bool, error) {
// SSH rules and per-node filters embed user identity outside the filter
// hash, so a user change needs a client refresh even when it did not move.
if pm.needsPerNodeFilter || (pm.pol != nil && len(pm.pol.SSHs) > 0) {
if hadPerNodeFilter || pm.needsPerNodeFilter ||
(pm.pol != nil && len(pm.pol.SSHs) > 0) {
policyChanged = true
}
+39
View File
@@ -556,6 +556,45 @@ func TestSetUsersDropsStaleSelfFilters(t *testing.T) {
}
}
// TestSetUsersReportsLostViaSources: a user change that leaves the only via
// grant without sources must be reported, or clients keep its rules.
func TestSetUsersReportsLostViaSources(t *testing.T) {
pol := `{
"tagOwners": {"tag:router": ["u1@"]},
"grants": [{"src": ["u3@"], "dst": ["10.0.0.0/16"], "via": ["tag:router"], "ip": ["*"]}]}`
u1, u3, x3 := types.User{Name: "u1"}, types.User{Name: "u3"}, types.User{Name: "x3"}
u1.ID, u3.ID, x3.ID = 1, 3, 3
router := node("router", "100.64.0.1", "fd7a:115c:a1e0::1", types.User{})
router.User, router.UserID = nil, nil
router.Tags = []string{"tag:router"}
router.Hostinfo = &tailcfg.Hostinfo{
RoutableIPs: []netip.Prefix{netip.MustParsePrefix("10.0.0.0/16")},
}
router.ApprovedRoutes = router.Hostinfo.RoutableIPs
nodes := types.Nodes{router, node("u3-a", "100.64.0.3", "fd7a:115c:a1e0::3", u3)}
for i, n := range nodes {
n.ID = types.NodeID(i + 1) //nolint:gosec // safe conversion in test
}
pm, err := NewPolicyManager([]byte(pol), types.Users{u1, u3}, nodes.ViewSlice())
require.NoError(t, err)
rules, err := pm.FilterForNode(router.View())
require.NoError(t, err)
require.NotEmpty(t, rules, "the router admits u3 before the rename")
policyChanged, _, err := pm.SetUsers(types.Users{u1, x3})
require.NoError(t, err)
rules, err = pm.FilterForNode(router.View())
require.NoError(t, err)
require.Empty(t, rules, "nobody is named u3 any more")
require.True(t, policyChanged, "the withdrawn rule must reach clients")
}
// TestInvalidateGlobalPolicyCache tests the cache invalidation logic for global policies.
func TestInvalidateGlobalPolicyCache(t *testing.T) {
mustIPPtr := func(s string) *netip.Addr {
-9
View File
@@ -107,13 +107,6 @@ var routesMapCompatTests = []struct {
var viaCompatSkipReasons = map[string]string{
"tagged-self": "TAGGED_SELF_PROFILE: SaaS gives a tagged node a profile of its own, headscale the shared tagged-devices one",
// TestViaGrantSourceLossCompat, keyed <test_id>/rename/<node>/<check>.
"via-grant-v67/rename/squirtle/peers": viaSourceLossNotSent,
"via-grant-v67/rename/squirtle/packet-filter": viaSourceLossNotSent,
"via-grant-v67/rename/blastoise/peers": viaSourceLossNotSent,
"via-grant-v67/rename/blastoise/packet-filter": viaSourceLossNotSent,
"via-grant-v67/rename/ivysaur/peers": viaSourceLossNotSent,
"routes-f2-subnet-as-acl-source/ivysaur/packet-filter": filterDstMerged,
"routes-f2-subnet-as-acl-source/venusaur/packet-filter": filterDstMerged,
@@ -263,8 +256,6 @@ var viaCompatSkipReasons = map[string]string{
"routes-m6-subnet-to-subnet-multi-route/m-odin2/peers": sameUserPeers,
}
const viaSourceLossNotSent = "VIA_SOURCE_LOSS_NOT_SENT: a user change that leaves a via grant without sources does not reach open streams"
const exitRoutesOnOverlap = "EXIT_ROUTES_ON_OVERLAP: headscale offers a peer's exit routes to a viewer whose rules reach only nodes or subnets; SaaS leaves them out"
// Adjacent member addresses merge into one prefix, and the reduced filter