fix webauth + autoapprove routes (#2528)

* types/node: add helper funcs for node tags Signed-off-by: Kristoffer Dalby <kristoffer@tailscale.com> * types/node: add DebugString method for node Signed-off-by: Kristoffer Dalby <kristoffer@tailscale.com> * policy/v2: add String func to AutoApprover interface Signed-off-by: Kristoffer Dalby <kristoffer@tailscale.com> * policy/v2: simplify, use slices.Contains Signed-off-by: Kristoffer Dalby <kristoffer@tailscale.com> * policy/v2: debug, use nodes.DebugString Signed-off-by: Kristoffer Dalby <kristoffer@tailscale.com> * policy/v1: fix potential nil pointer in NodeCanApproveRoute Signed-off-by: Kristoffer Dalby <kristoffer@tailscale.com> * policy/v1: slices.Contains Signed-off-by: Kristoffer Dalby <kristoffer@tailscale.com> * integration/tsic: fix diff in login commands Signed-off-by: Kristoffer Dalby <kristoffer@tailscale.com> * integration: fix webauth running with wrong scenario Signed-off-by: Kristoffer Dalby <kristoffer@tailscale.com> * integration: move common oidc opts to func Signed-off-by: Kristoffer Dalby <kristoffer@tailscale.com> * integration: require node count, more verbose Signed-off-by: Kristoffer Dalby <kristoffer@tailscale.com> * auth: remove uneffective route approve Signed-off-by: Kristoffer Dalby <kristoffer@tailscale.com> * .github/workflows: fmt Signed-off-by: Kristoffer Dalby <kristoffer@tailscale.com> * integration/tsic: add id func Signed-off-by: Kristoffer Dalby <kristoffer@tailscale.com> * integration: remove call that might be nil Signed-off-by: Kristoffer Dalby <kristoffer@tailscale.com> * integration: test autoapprovers against web/authkey x group/tag/user Signed-off-by: Kristoffer Dalby <kristoffer@tailscale.com> * integration: unique network id per scenario Signed-off-by: Kristoffer Dalby <kristoffer@tailscale.com> * Revert "integration: move common oidc opts to func" This reverts commit 7e9d165d4a900c304f1083b665f1a24a26e06e55. * remove cmd Signed-off-by: Kristoffer Dalby <kristoffer@tailscale.com> * integration: clean docker images between runs in ci Signed-off-by: Kristoffer Dalby <kristoffer@tailscale.com> * integration: run autoapprove test against differnt policy modes Signed-off-by: Kristoffer Dalby <kristoffer@tailscale.com> * integration/tsic: append, not overrwrite extra login args Signed-off-by: Kristoffer Dalby <kristoffer@tailscale.com> * .github/workflows: remove polv2 Signed-off-by: Kristoffer Dalby <kristoffer@tailscale.com> --------- Signed-off-by: Kristoffer Dalby <kristoffer@tailscale.com>
2025-04-30 08:54:04 +03:00 · 2025-04-30 08:54:04 +03:00 · f1206328dc
commit f1206328dc
parent 57861507ab
17 changed files with 732 additions and 401 deletions
--- a/hscontrol/policy/v1/policy.go
+++ b/hscontrol/policy/v1/policy.go
@ -7,6 +7,8 @@ import (
 	"os"
 	"sync"

+	"slices"
+
 	"github.com/juanfont/headscale/hscontrol/types"
 	"github.com/rs/zerolog/log"
 	"tailscale.com/tailcfg"
@ -145,13 +147,7 @@ func (pm *PolicyManager) NodeCanHaveTag(node *types.Node, tag string) bool {
 	tags, invalid := pm.pol.TagsOfNode(pm.users, node)
 	log.Debug().Strs("authorised_tags", tags).Strs("unauthorised_tags", invalid).Uint64("node.id", node.ID.Uint64()).Msg("tags provided by policy")

-	for _, t := range tags {
-		if t == tag {
-			return true
-		}
-	}
-
-	return false
+	return slices.Contains(tags, tag)
 }

 func (pm *PolicyManager) NodeCanApproveRoute(node *types.Node, route netip.Prefix) bool {
@ -174,7 +170,7 @@ func (pm *PolicyManager) NodeCanApproveRoute(node *types.Node, route netip.Prefi
 			}

 			// approvedIPs should contain all of node's IPs if it matches the rule, so check for first
-			if ips.Contains(*node.IPv4) {
+			if ips != nil && ips.Contains(*node.IPv4) {
 				return true
 			}
 		}
--- a/hscontrol/policy/v2/policy.go
+++ b/hscontrol/policy/v2/policy.go
@ -7,6 +7,8 @@ import (
 	"strings"
 	"sync"

+	"slices"
+
 	"github.com/juanfont/headscale/hscontrol/types"
 	"go4.org/netipx"
 	"tailscale.com/net/tsaddr"
@ -174,10 +176,8 @@ func (pm *PolicyManager) NodeCanHaveTag(node *types.Node, tag string) bool {
 	defer pm.mu.Unlock()

 	if ips, ok := pm.tagOwnerMap[Tag(tag)]; ok {
-		for _, nodeAddr := range node.IPs() {
-			if ips.Contains(nodeAddr) {
-				return true
-			}
+		if slices.ContainsFunc(node.IPs(), ips.Contains) {
+			return true
 		}
 	}

@ -196,10 +196,8 @@ func (pm *PolicyManager) NodeCanApproveRoute(node *types.Node, route netip.Prefi
 	// where there is an exact entry, e.g. 10.0.0.0/8, then
 	// check and return quickly
 	if _, ok := pm.autoApproveMap[route]; ok {
-		for _, nodeAddr := range node.IPs() {
-			if pm.autoApproveMap[route].Contains(nodeAddr) {
-				return true
-			}
+		if slices.ContainsFunc(node.IPs(), pm.autoApproveMap[route].Contains) {
+			return true
 		}
 	}

@ -220,10 +218,8 @@ func (pm *PolicyManager) NodeCanApproveRoute(node *types.Node, route netip.Prefi
 		// Check if prefix is larger (so containing) and then overlaps
 		// the route to see if the node can approve a subset of an autoapprover
 		if prefix.Bits() <= route.Bits() && prefix.Overlaps(route) {
-			for _, nodeAddr := range node.IPs() {
-				if approveAddrs.Contains(nodeAddr) {
-					return true
-				}
+			if slices.ContainsFunc(node.IPs(), approveAddrs.Contains) {
+				return true
 			}
 		}
 	}
@ -279,5 +275,8 @@ func (pm *PolicyManager) DebugString() string {
 		}
 	}

+	sb.WriteString("\n\n")
+	sb.WriteString(pm.nodes.DebugString())
+
 	return sb.String()
 }
--- a/hscontrol/policy/v2/types.go
+++ b/hscontrol/policy/v2/types.go
@ -162,6 +162,10 @@ func (g Group) CanBeAutoApprover() bool {
 	return true
 }

+func (g Group) String() string {
+	return string(g)
+}
+
 func (g Group) Resolve(p *Policy, users types.Users, nodes types.Nodes) (*netipx.IPSet, error) {
 	var ips netipx.IPSetBuilder
 	var errs []error
@ -235,6 +239,10 @@ func (t Tag) CanBeAutoApprover() bool {
 	return true
 }

+func (t Tag) String() string {
+	return string(t)
+}
+
 // Host is a string that represents a hostname.
 type Host string

@ -590,6 +598,7 @@ func unmarshalPointer[T any](
 type AutoApprover interface {
 	CanBeAutoApprover() bool
 	UnmarshalJSON([]byte) error
+	String() string
 }

 type AutoApprovers []AutoApprover
--- a/hscontrol/types/node.go
+++ b/hscontrol/types/node.go
@ -5,6 +5,7 @@ import (
 	"fmt"
 	"net/netip"
 	"slices"
+	"sort"
 	"strconv"
 	"strings"
 	"time"
@ -194,19 +195,26 @@ func (node *Node) IsTagged() bool {
 // Currently, this function only handles tags set
 // via CLI ("forced tags" and preauthkeys)
 func (node *Node) HasTag(tag string) bool {
-	if slices.Contains(node.ForcedTags, tag) {
-		return true
-	}
+	return slices.Contains(node.Tags(), tag)
+}

-	if node.AuthKey != nil && slices.Contains(node.AuthKey.Tags, tag) {
-		return true
+func (node *Node) Tags() []string {
+	var tags []string
+
+	if node.AuthKey != nil {
+		tags = append(tags, node.AuthKey.Tags...)
 	}

 	// TODO(kradalby): Figure out how tagging should work
 	// and hostinfo.requestedtags.
 	// Do this in other work.
+	// #2417

-	return false
+	tags = append(tags, node.ForcedTags...)
+	sort.Strings(tags)
+	tags = slices.Compact(tags)
+
+	return tags
 }

 func (node *Node) RequestTags() []string {
@ -549,3 +557,25 @@ func (nodes Nodes) IDMap() map[NodeID]*Node {

 	return ret
 }
+
+func (nodes Nodes) DebugString() string {
+	var sb strings.Builder
+	sb.WriteString("Nodes:\n")
+	for _, node := range nodes {
+		sb.WriteString(node.DebugString())
+		sb.WriteString("\n")
+	}
+	return sb.String()
+}
+
+func (node Node) DebugString() string {
+	var sb strings.Builder
+	fmt.Fprintf(&sb, "%s(%s):\n", node.Hostname, node.ID)
+	fmt.Fprintf(&sb, "\tUser: %s (%d, %q)\n", node.User.Display(), node.User.ID, node.User.Username())
+	fmt.Fprintf(&sb, "\tTags: %v\n", node.Tags())
+	fmt.Fprintf(&sb, "\tIPs: %v\n", node.IPs())
+	fmt.Fprintf(&sb, "\tApprovedRoutes: %v\n", node.ApprovedRoutes)
+	fmt.Fprintf(&sb, "\tSubnetRoutes: %v\n", node.SubnetRoutes())
+	sb.WriteString("\n")
+	return sb.String()
+}
--- a/hscontrol/util/util.go
+++ b/hscontrol/util/util.go
@ -5,6 +5,7 @@ import (
 	"fmt"
 	"net/netip"
 	"net/url"
+	"os"
 	"regexp"
 	"strconv"
 	"strings"
@ -173,3 +174,15 @@ func ParseTraceroute(output string) (Traceroute, error) {

 	return result, nil
 }
+
+func IsCI() bool {
+	if _, ok := os.LookupEnv("CI"); ok {
+		return true
+	}
+
+	if _, ok := os.LookupEnv("GITHUB_RUN_ID"); ok {
+		return true
+	}
+
+	return false
+}