ipn/*: make new WindowsUserID type to consolidate docs

The "userID is empty everywhere but Windows" docs on lots of places
but not everywhere while using just a string type was getting
confusing. This makes a new type to wrap up those rules, however
weird/historical they might be.

Change-Id: I142e85a8e38760988d6c0c91d0efecedade81b9b
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>

ipn/ipnauth/ipnauth.go

@@ -17,6 +17,8 @@ import (
 	"syscall"
 
 	"inet.af/peercred"
+	"tailscale.com/envknob"
+	"tailscale.com/ipn"
 	"tailscale.com/net/netstat"
 	"tailscale.com/safesocket"
 	"tailscale.com/types/logger"
@@ -40,19 +42,31 @@ type ConnIdentity struct {
 	// TODO(bradfitz): merge these into the peercreds package and
 	// use that for all.
 	pid    int
-	userID string
+	userID ipn.WindowsUserID
 	user   *user.User
 }
 
-// UserID returns the local machine's userid of the connection.
+// WindowsUserID returns the local machine's userid of the connection
+// if it's on Windows. Otherwise it returns the empty string.
 //
 // It's suitable for passing to LookupUserFromID (os/user.LookupId) on any
 // operating system.
-//
+func (ci *ConnIdentity) WindowsUserID() ipn.WindowsUserID {
-// TODO(bradfitz): it currently returns an empty string on everything
+	if envknob.GOOS() != "windows" {
-// but Windows. We should make it return the actual uid also on all supported
+		return ""
-// peercred platforms from the creds if non-nil.
+	}
-func (ci *ConnIdentity) UserID() string { return ci.userID }
+	if ci.userID != "" {
+		return ci.userID
+	}
+	// For Linux tests running as Windows:
+	const isBroken = true // TODO(bradfitz,maisem): fix tests; this doesn't work yet
+	if ci.creds != nil && !isBroken {
+		if uid, ok := ci.creds.UserID(); ok {
+			return ipn.WindowsUserID(uid)
+		}
+	}
+	return ""
+}
 
 func (ci *ConnIdentity) User() *user.User       { return ci.user }
 func (ci *ConnIdentity) Pid() int               { return ci.pid }
@@ -99,7 +113,7 @@ func GetConnIdentity(logf logger.Logf, c net.Conn) (ci *ConnIdentity, err error)
 		}
 		return ci, fmt.Errorf("failed to map connection's pid to a user%s: %w", hint, err)
 	}
-	ci.userID = uid
+	ci.userID = ipn.WindowsUserID(uid)
 	u, err := LookupUserFromID(logf, uid)
 	if err != nil {
 		return ci, fmt.Errorf("failed to look up user from userid: %w", err)

ipn/ipnlocal/local.go

@@ -2076,12 +2076,12 @@ func (b *LocalBackend) CheckIPNConnectionAllowed(ci *ipnauth.ConnIdentity) error
 	if !b.pm.CurrentPrefs().ForceDaemon() {
 		return nil
 	}
-	uid := ci.UserID()
+	uid := ci.WindowsUserID()
 	if uid == "" {
 		return errors.New("empty user uid in connection identity")
 	}
 	if uid != serverModeUid {
-		return fmt.Errorf("Tailscale running in server mode (%q); connection from %q not allowed", b.tryLookupUserName(serverModeUid), b.tryLookupUserName(uid))
+		return fmt.Errorf("Tailscale running in server mode (%q); connection from %q not allowed", b.tryLookupUserName(string(serverModeUid)), b.tryLookupUserName(string(uid)))
 	}
 	return nil
 }
@@ -2257,7 +2257,7 @@ func (b *LocalBackend) shouldUploadServices() bool {
 // changed.
 //
 // On non-multi-user systems, the uid should be set to empty string.
-func (b *LocalBackend) SetCurrentUserID(uid string) {
+func (b *LocalBackend) SetCurrentUserID(uid ipn.WindowsUserID) {
 	b.mu.Lock()
 	if b.pm.CurrentUserID() == uid {
 		b.mu.Unlock()

ipn/ipnlocal/profiles.go

@@ -28,7 +28,7 @@ type profileManager struct {
 	store ipn.StateStore
 	logf  logger.Logf
 
-	currentUserID  string // only used on Windows
+	currentUserID  ipn.WindowsUserID
 	knownProfiles  map[ipn.ProfileID]*ipn.LoginProfile
 	currentProfile *ipn.LoginProfile // always non-nil
 	prefs          ipn.PrefsView     // always Valid.
@@ -42,13 +42,13 @@ type profileManager struct {
 
 // CurrentUserID returns the current user ID. It is only non-empty on
 // Windows where we have a multi-user system.
-func (pm *profileManager) CurrentUserID() string {
+func (pm *profileManager) CurrentUserID() ipn.WindowsUserID {
 	return pm.currentUserID
 }
 
 // SetCurrentUserID sets the current user ID. The uid is only non-empty
 // on Windows where we have a multi-user system.
-func (pm *profileManager) SetCurrentUserID(uid string) error {
+func (pm *profileManager) SetCurrentUserID(uid ipn.WindowsUserID) error {
 	if pm.currentUserID == uid {
 		return nil
 	}
@@ -63,7 +63,7 @@ func (pm *profileManager) SetCurrentUserID(uid string) error {
 
 	// Read the CurrentProfileKey from the store which stores
 	// the selected profile for the current user.
-	b, err := pm.store.ReadState(ipn.CurrentProfileKey(uid))
+	b, err := pm.store.ReadState(ipn.CurrentProfileKey(string(uid)))
 	if err == ipn.ErrStateNotExist || len(b) == 0 {
 		pm.NewProfile()
 		return nil
@@ -310,7 +310,7 @@ func (pm *profileManager) SwitchProfile(id ipn.ProfileID) error {
 }
 
 func (pm *profileManager) setAsUserSelectedProfileLocked() error {
-	k := ipn.CurrentProfileKey(pm.currentUserID)
+	k := ipn.CurrentProfileKey(string(pm.currentUserID))
 	return pm.store.WriteState(k, []byte(pm.currentProfile.Key))
 }
 
@@ -487,7 +487,7 @@ func newProfileManagerWithGOOS(store ipn.StateStore, logf logger.Logf, stateKey
 		}
 		if pm.currentProfile == nil {
 			if suf, ok := strs.CutPrefix(string(stateKey), "user-"); ok {
-				pm.currentUserID = suf
+				pm.currentUserID = ipn.WindowsUserID(suf)
 			}
 			pm.NewProfile()
 		} else {

ipn/ipnserver/server.go

@@ -14,7 +14,6 @@ import (
 	"os"
 	"os/user"
 	"path/filepath"
-	"runtime"
 	"strings"
 	"sync"
 	"time"
@@ -87,7 +86,7 @@ type Server struct {
 	// mu guards the fields that follow.
 	// lock order: mu, then LocalBackend.mu
 	mu         sync.Mutex
-	lastUserID string // tracks last userid; on change, Reset state for paranoia
+	lastUserID ipn.WindowsUserID // tracks last userid; on change, Reset state for paranoia
 	activeReqs map[*http.Request]*ipnauth.ConnIdentity
 }
 
@@ -168,7 +167,7 @@ func (s *Server) checkConnIdentityLocked(ci *ipnauth.ConnIdentity) error {
 		for _, active = range s.activeReqs {
 			break
 		}
-		if active != nil && ci.UserID() != active.UserID() {
+		if active != nil && ci.WindowsUserID() != active.WindowsUserID() {
 			return inUseOtherUserError{fmt.Errorf("Tailscale already in use by %s, pid %d", active.User().Username, active.Pid())}
 		}
 	}
@@ -183,7 +182,7 @@ func (s *Server) checkConnIdentityLocked(ci *ipnauth.ConnIdentity) error {
 //
 // s.mu must not be held.
 func (s *Server) localAPIPermissions(ci *ipnauth.ConnIdentity) (read, write bool) {
-	switch runtime.GOOS {
+	switch envknob.GOOS() {
 	case "windows":
 		s.mu.Lock()
 		defer s.mu.Unlock()
@@ -272,8 +271,7 @@ func (s *Server) addActiveHTTPRequest(req *http.Request, ci *ipnauth.ConnIdentit
 
 	mak.Set(&s.activeReqs, req, ci)
 
-	if envknob.GOOS() == "windows" && len(s.activeReqs) == 1 {
+	if uid := ci.WindowsUserID(); uid != "" && len(s.activeReqs) == 1 {
-		uid := ci.UserID()
 		// Tell the LocalBackend about the identity we're now running as.
 		s.b.SetCurrentUserID(uid)
 		if s.lastUserID != uid {

ipn/prefs.go

@@ -702,6 +702,12 @@ func SavePrefs(filename string, p *Prefs) {
 // profile. It is a 4 character hex string like "1ab3".
 type ProfileID string
 
+// WindowsUserID is a userid (suitable for passing to ipnauth.LookupUserFromID
+// or os/user.LookupId) but only set on Windows. It's empty on all other
+// platforms, unless envknob.GOOS is in used, making Linux act like Windows for
+// tests.
+type WindowsUserID string
+
 // LoginProfile represents a single login profile as managed
 // by the ProfileManager.
 type LoginProfile struct {
@@ -734,5 +740,5 @@ type LoginProfile struct {
 	// LocalUserID is the user ID of the user who created this profile.
 	// It is only relevant on Windows where we have a multi-user system.
 	// It is assigned once at profile creation time and never changes.
-	LocalUserID string
+	LocalUserID WindowsUserID
 }