From d79e0fde9cf468f9b22995573cfb0d7fc948c675 Mon Sep 17 00:00:00 2001
From: Sonia Appasamy <sonia@tailscale.com>
Date: Tue, 24 Oct 2023 15:15:10 -0400
Subject: [PATCH] client/web: split errTaggedSelf resp from
 getTailscaleBrowserSession

Previously returned errTaggedSource in the case that of any tagged
source. Now distinguishing whether the source was local or remote.
We'll be presenting the two cases with varying copy on the frontend.

Updates tailscale/corp#14335

Signed-off-by: Sonia Appasamy <sonia@tailscale.com>
---
 client/web/web.go      | 43 +++++++++++++++++++++++-------------------
 client/web/web_test.go | 25 +++++++++++++++++++-----
 2 files changed, 44 insertions(+), 24 deletions(-)

diff --git a/client/web/web.go b/client/web/web.go
index 3b76f6568..2b010ecc6 100644
--- a/client/web/web.go
+++ b/client/web/web.go
@@ -289,10 +289,11 @@ func (s *Server) serveLoginAPI(w http.ResponseWriter, r *http.Request) {
 }
 
 var (
-	errNoSession         = errors.New("no-browser-session")
-	errNotUsingTailscale = errors.New("not-using-tailscale")
-	errTaggedSource      = errors.New("tagged-source")
-	errNotOwner          = errors.New("not-owner")
+	errNoSession          = errors.New("no-browser-session")
+	errNotUsingTailscale  = errors.New("not-using-tailscale")
+	errTaggedRemoteSource = errors.New("tagged-remote-source")
+	errTaggedLocalSource  = errors.New("tagged-local-source")
+	errNotOwner           = errors.New("not-owner")
 )
 
 // getTailscaleBrowserSession retrieves the browser session associated with
@@ -304,8 +305,13 @@ var (
 //
 //   - (errNoSession) The request does not have a session.
 //
-//   - (errTaggedSource) The source is a tagged node. Users must use their
-//     own user-owned devices to manage other nodes' web clients.
+//   - (errTaggedRemoteSource) The source is remote (another node) and tagged.
+//     Users must use their own user-owned devices to manage other nodes'
+//     web clients.
+//
+//   - (errTaggedLocalSource) The source is local (the same node) and tagged.
+//     Tagged nodes can only be remotely managed, allowing ACLs to dictate
+//     access to web clients.
 //
 //   - (errNotOwner) The source is not the owner of this client (if the
 //     client is user-owned). Only the owner is allowed to manage the
@@ -318,26 +324,25 @@ var (
 // The WhoIsResponse is always populated, with a non-nil Node and UserProfile,
 // unless getTailscaleBrowserSession reports errNotUsingTailscale.
 func (s *Server) getTailscaleBrowserSession(r *http.Request) (*browserSession, *apitype.WhoIsResponse, error) {
-	whoIs, err := s.lc.WhoIs(r.Context(), r.RemoteAddr)
+	whoIs, whoIsErr := s.lc.WhoIs(r.Context(), r.RemoteAddr)
+	status, statusErr := s.lc.StatusWithoutPeers(r.Context())
 	switch {
-	case err != nil:
+	case whoIsErr != nil:
 		return nil, nil, errNotUsingTailscale
+	case statusErr != nil:
+		return nil, whoIs, statusErr
+	case status.Self == nil:
+		return nil, whoIs, errors.New("missing self node in tailscale status")
+	case whoIs.Node.IsTagged() && whoIs.Node.StableID == status.Self.ID:
+		return nil, whoIs, errTaggedLocalSource
 	case whoIs.Node.IsTagged():
-		return nil, whoIs, errTaggedSource
+		return nil, whoIs, errTaggedRemoteSource
+	case !status.Self.IsTagged() && status.Self.UserID != whoIs.UserProfile.ID:
+		return nil, whoIs, errNotOwner
 	}
 	srcNode := whoIs.Node.ID
 	srcUser := whoIs.UserProfile.ID
 
-	status, err := s.lc.StatusWithoutPeers(r.Context())
-	switch {
-	case err != nil:
-		return nil, whoIs, err
-	case status.Self == nil:
-		return nil, whoIs, errors.New("missing self node in tailscale status")
-	case !status.Self.IsTagged() && status.Self.UserID != srcUser:
-		return nil, whoIs, errNotOwner
-	}
-
 	cookie, err := r.Cookie(sessionCookieName)
 	if errors.Is(err, http.ErrNoCookie) {
 		return nil, whoIs, errNoSession
diff --git a/client/web/web_test.go b/client/web/web_test.go
index 9967f2114..372ebcb92 100644
--- a/client/web/web_test.go
+++ b/client/web/web_test.go
@@ -151,15 +151,15 @@ func TestGetTailscaleBrowserSession(t *testing.T) {
 	tags := views.SliceOf([]string{"tag:server"})
 	tailnetNodes := map[string]*apitype.WhoIsResponse{
 		userANodeIP: {
-			Node:        &tailcfg.Node{ID: 1},
+			Node:        &tailcfg.Node{ID: 1, StableID: "1"},
 			UserProfile: userA,
 		},
 		userBNodeIP: {
-			Node:        &tailcfg.Node{ID: 2},
+			Node:        &tailcfg.Node{ID: 2, StableID: "2"},
 			UserProfile: userB,
 		},
 		taggedNodeIP: {
-			Node: &tailcfg.Node{ID: 3, Tags: tags.AsSlice()},
+			Node: &tailcfg.Node{ID: 3, StableID: "3", Tags: tags.AsSlice()},
 		},
 	}
 
@@ -240,11 +240,26 @@ func TestGetTailscaleBrowserSession(t *testing.T) {
 			wantError:   errNotOwner,
 		},
 		{
-			name:        "tagged-source",
+			name:        "tagged-remote-source",
 			selfNode:    &ipnstate.PeerStatus{ID: "self", UserID: userA.ID},
 			remoteAddr:  taggedNodeIP,
 			wantSession: nil,
-			wantError:   errTaggedSource,
+			wantError:   errTaggedRemoteSource,
+		},
+		{
+			name:        "tagged-local-source",
+			selfNode:    &ipnstate.PeerStatus{ID: "3"},
+			remoteAddr:  taggedNodeIP, // same node as selfNode
+			wantSession: nil,
+			wantError:   errTaggedLocalSource,
+		},
+		{
+			name:        "not-tagged-local-source",
+			selfNode:    &ipnstate.PeerStatus{ID: "1", UserID: userA.ID},
+			remoteAddr:  userANodeIP, // same node as selfNode
+			cookie:      userASession.ID,
+			wantSession: userASession,
+			wantError:   nil, // should not error
 		},
 		{
 			name:        "has-session",