From d0ba91bdb22e2cfe8b0f35a37cc0b540842f0011 Mon Sep 17 00:00:00 2001
From: Nick Khyl <nickk@tailscale.com>
Date: Sat, 11 Jan 2025 17:58:27 -0600
Subject: [PATCH] ipn/ipnserver: use ipnauth.Actor instead of *ipnserver.actor
whenever possible
In preparation for adding test coverage for ipn/ipnserver.Server, we update it
to use ipnauth.Actor instead of its concrete implementation where possible.
Updates tailscale/corp#25804
Signed-off-by: Nick Khyl <nickk@tailscale.com>
---
ipn/ipnserver/actor.go | 8 ++++----
ipn/ipnserver/server.go | 24 ++++++++++++++++--------
2 files changed, 20 insertions(+), 12 deletions(-)
diff --git a/ipn/ipnserver/actor.go b/ipn/ipnserver/actor.go
index 0e716009c..2df8986c3 100644
--- a/ipn/ipnserver/actor.go
+++ b/ipn/ipnserver/actor.go
@@ -112,11 +112,11 @@ func (a *actor) Username() (string, error) {
}
type actorOrError struct {
- actor *actor
+ actor ipnauth.Actor
err error
}
-func (a actorOrError) unwrap() (*actor, error) {
+func (a actorOrError) unwrap() (ipnauth.Actor, error) {
return a.actor, a.err
}
@@ -131,9 +131,9 @@ func contextWithActor(ctx context.Context, logf logger.Logf, c net.Conn) context
return actorKey.WithValue(ctx, actorOrError{actor: actor, err: err})
}
-// actorFromContext returns an [actor] associated with ctx,
+// actorFromContext returns an [ipnauth.Actor] associated with ctx,
// or an error if the context does not carry an actor's identity.
-func actorFromContext(ctx context.Context) (*actor, error) {
+func actorFromContext(ctx context.Context) (ipnauth.Actor, error) {
return actorKey.Value(ctx).unwrap()
}
diff --git a/ipn/ipnserver/server.go b/ipn/ipnserver/server.go
index 73b5e82ab..574d1a55c 100644
--- a/ipn/ipnserver/server.go
+++ b/ipn/ipnserver/server.go
@@ -22,6 +22,7 @@ import (
"tailscale.com/envknob"
"tailscale.com/ipn"
+ "tailscale.com/ipn/ipnauth"
"tailscale.com/ipn/ipnlocal"
"tailscale.com/ipn/localapi"
"tailscale.com/net/netmon"
@@ -30,6 +31,7 @@ import (
"tailscale.com/util/mak"
"tailscale.com/util/set"
"tailscale.com/util/systemd"
+ "tailscale.com/util/testenv"
)
// Server is an IPN backend and its set of 0 or more active localhost
@@ -50,7 +52,7 @@ type Server struct {
// lock order: mu, then LocalBackend.mu
mu sync.Mutex
lastUserID ipn.WindowsUserID // tracks last userid; on change, Reset state for paranoia
- activeReqs map[*http.Request]*actor
+ activeReqs map[*http.Request]ipnauth.Actor
backendWaiter waiterSet // of LocalBackend waiters
zeroReqWaiter waiterSet // of blockUntilZeroConnections waiters
}
@@ -195,8 +197,12 @@ func (s *Server) serveHTTP(w http.ResponseWriter, r *http.Request) {
if strings.HasPrefix(r.URL.Path, "/localapi/") {
lah := localapi.NewHandler(lb, s.logf, s.backendLogID)
- lah.PermitRead, lah.PermitWrite = ci.Permissions(lb.OperatorUserID())
- lah.PermitCert = ci.CanFetchCerts()
+ if actor, ok := ci.(*actor); ok {
+ lah.PermitRead, lah.PermitWrite = actor.Permissions(lb.OperatorUserID())
+ lah.PermitCert = actor.CanFetchCerts()
+ } else if testenv.InTest() {
+ lah.PermitRead, lah.PermitWrite = true, true
+ }
lah.Actor = ci
lah.ServeHTTP(w, r)
return
@@ -230,11 +236,11 @@ func (e inUseOtherUserError) Unwrap() error { return e.error }
// The returned error, when non-nil, will be of type inUseOtherUserError.
//
// s.mu must be held.
-func (s *Server) checkConnIdentityLocked(ci *actor) error {
+func (s *Server) checkConnIdentityLocked(ci ipnauth.Actor) error {
// If clients are already connected, verify they're the same user.
// This mostly matters on Windows at the moment.
if len(s.activeReqs) > 0 {
- var active *actor
+ var active ipnauth.Actor
for _, active = range s.activeReqs {
break
}
@@ -251,7 +257,9 @@ func (s *Server) checkConnIdentityLocked(ci *actor) error {
if username, err := active.Username(); err == nil {
fmt.Fprintf(&b, " by %s", username)
}
- fmt.Fprintf(&b, ", pid %d", active.pid())
+ if active, ok := active.(*actor); ok {
+ fmt.Fprintf(&b, ", pid %d", active.pid())
+ }
return inUseOtherUserError{errors.New(b.String())}
}
}
@@ -267,7 +275,7 @@ func (s *Server) checkConnIdentityLocked(ci *actor) error {
//
// This is primarily used for the Windows GUI, to block until one user's done
// controlling the tailscaled process.
-func (s *Server) blockWhileIdentityInUse(ctx context.Context, actor *actor) error {
+func (s *Server) blockWhileIdentityInUse(ctx context.Context, actor ipnauth.Actor) error {
inUse := func() bool {
s.mu.Lock()
defer s.mu.Unlock()
@@ -361,7 +369,7 @@ func (a *actor) CanFetchCerts() bool {
// The returned error may be of type [inUseOtherUserError].
//
// onDone must be called when the HTTP request is done.
-func (s *Server) addActiveHTTPRequest(req *http.Request, actor *actor) (onDone func(), err error) {
+func (s *Server) addActiveHTTPRequest(req *http.Request, actor ipnauth.Actor) (onDone func(), err error) {
if actor == nil {
return nil, errors.New("internal error: nil actor")
}