From 4e012794fcae25db3eae1681718b4ed40704ec34 Mon Sep 17 00:00:00 2001
From: Mario Minardi <mario@tailscale.com>
Date: Mon, 11 Dec 2023 13:50:15 -0700
Subject: [PATCH] client/web: add metric logging when viewing local / remote
 node (#10555)

Add metric logging for the case where a user is viewing a local or remote
node.

Updates https://github.com/tailscale/tailscale/issues/10261

Signed-off-by: Mario Minardi <mario@tailscale.com>
---
 client/web/web.go      |  7 +++++
 client/web/web_test.go | 61 ++++++++++++++++++++++++++++++++++--------
 2 files changed, 57 insertions(+), 11 deletions(-)

diff --git a/client/web/web.go b/client/web/web.go
index 956d3defa..8dcb2525b 100644
--- a/client/web/web.go
+++ b/client/web/web.go
@@ -448,6 +448,7 @@ func (s *Server) serveAPIAuth(w http.ResponseWriter, r *http.Request) {
 	switch {
 	case sErr != nil && errors.Is(sErr, errNotUsingTailscale):
 		// Restricted to the readonly view, no auth action to take.
+		s.lc.IncrementCounter(r.Context(), "web_client_viewing_local", 1)
 		resp.AuthNeeded = ""
 	case sErr != nil && errors.Is(sErr, errNotOwner):
 		// Restricted to the readonly view, no auth action to take.
@@ -474,6 +475,12 @@ func (s *Server) serveAPIAuth(w http.ResponseWriter, r *http.Request) {
 		resp.CanManageNode = true
 		resp.AuthNeeded = ""
 	default:
+		// whois being nil implies local as the request did not come over Tailscale
+		if whois == nil || (whois.Node.StableID == status.Self.ID) {
+			s.lc.IncrementCounter(r.Context(), "web_client_viewing_local", 1)
+		} else {
+			s.lc.IncrementCounter(r.Context(), "web_client_viewing_remote", 1)
+		}
 		resp.AuthNeeded = tailscaleAuth
 	}
 
diff --git a/client/web/web_test.go b/client/web/web_test.go
index dc5670fbb..a55dfdfd1 100644
--- a/client/web/web_test.go
+++ b/client/web/web_test.go
@@ -790,6 +790,7 @@ func TestServeAPIAuthMetricLogging(t *testing.T) {
 		},
 		UserProfile: otherUser,
 	}
+	nonTailscaleIP := "10.100.2.3"
 
 	testControlURL := &defaultControlURL
 	var loggedMetrics []string
@@ -820,9 +821,9 @@ func TestServeAPIAuthMetricLogging(t *testing.T) {
 		waitAuthURL: mockWaitAuthURL,
 	}
 
-	remoteNodeCookie := "ts-cookie-remote-node"
-	s.browserSessions.Store(remoteNodeCookie, &browserSession{
-		ID:            remoteNodeCookie,
+	authenticatedRemoteNodeCookie := "ts-cookie-remote-node-authenticated"
+	s.browserSessions.Store(authenticatedRemoteNodeCookie, &browserSession{
+		ID:            authenticatedRemoteNodeCookie,
 		SrcNode:       remoteNode.Node.ID,
 		SrcUser:       user.ID,
 		Created:       oneHourAgo,
@@ -830,9 +831,9 @@ func TestServeAPIAuthMetricLogging(t *testing.T) {
 		AuthURL:       *testControlURL + testAuthPathSuccess,
 		Authenticated: true,
 	})
-	localNodeCookie := "ts-cookie-local-node"
-	s.browserSessions.Store(localNodeCookie, &browserSession{
-		ID:            localNodeCookie,
+	authenticatedLocalNodeCookie := "ts-cookie-local-node-authenticated"
+	s.browserSessions.Store(authenticatedLocalNodeCookie, &browserSession{
+		ID:            authenticatedLocalNodeCookie,
 		SrcNode:       localNode.Node.ID,
 		SrcUser:       user.ID,
 		Created:       oneHourAgo,
@@ -840,6 +841,26 @@ func TestServeAPIAuthMetricLogging(t *testing.T) {
 		AuthURL:       *testControlURL + testAuthPathSuccess,
 		Authenticated: true,
 	})
+	unauthenticatedRemoteNodeCookie := "ts-cookie-remote-node-unauthenticated"
+	s.browserSessions.Store(unauthenticatedRemoteNodeCookie, &browserSession{
+		ID:            unauthenticatedRemoteNodeCookie,
+		SrcNode:       remoteNode.Node.ID,
+		SrcUser:       user.ID,
+		Created:       oneHourAgo,
+		AuthID:        testAuthPathSuccess,
+		AuthURL:       *testControlURL + testAuthPathSuccess,
+		Authenticated: false,
+	})
+	unauthenticatedLocalNodeCookie := "ts-cookie-local-node-unauthenticated"
+	s.browserSessions.Store(unauthenticatedLocalNodeCookie, &browserSession{
+		ID:            unauthenticatedLocalNodeCookie,
+		SrcNode:       localNode.Node.ID,
+		SrcUser:       user.ID,
+		Created:       oneHourAgo,
+		AuthID:        testAuthPathSuccess,
+		AuthURL:       *testControlURL + testAuthPathSuccess,
+		Authenticated: false,
+	})
 
 	tests := []struct {
 		name       string
@@ -850,34 +871,52 @@ func TestServeAPIAuthMetricLogging(t *testing.T) {
 	}{
 		{
 			name:             "managing-remote",
-			cookie:           remoteNodeCookie,
+			cookie:           authenticatedRemoteNodeCookie,
 			remoteAddr:       remoteIP,
 			wantLoggedMetric: "web_client_managing_remote",
 		},
 		{
 			name:             "managing-local",
-			cookie:           localNodeCookie,
+			cookie:           authenticatedLocalNodeCookie,
 			remoteAddr:       localIP,
 			wantLoggedMetric: "web_client_managing_local",
 		},
 		{
 			name:             "viewing-not-owner",
-			cookie:           remoteNodeCookie,
+			cookie:           authenticatedRemoteNodeCookie,
 			remoteAddr:       otherIP,
 			wantLoggedMetric: "web_client_viewing_not_owner",
 		},
 		{
 			name:             "viewing-local-tagged",
-			cookie:           localNodeCookie,
+			cookie:           authenticatedLocalNodeCookie,
 			remoteAddr:       localTaggedIP,
 			wantLoggedMetric: "web_client_viewing_local_tag",
 		},
 		{
 			name:             "viewing-remote-tagged",
-			cookie:           remoteNodeCookie,
+			cookie:           authenticatedRemoteNodeCookie,
 			remoteAddr:       remoteTaggedIP,
 			wantLoggedMetric: "web_client_viewing_remote_tag",
 		},
+		{
+			name:             "viewing-local-non-tailscale",
+			cookie:           authenticatedLocalNodeCookie,
+			remoteAddr:       nonTailscaleIP,
+			wantLoggedMetric: "web_client_viewing_local",
+		},
+		{
+			name:             "viewing-local-unauthenticated",
+			cookie:           unauthenticatedLocalNodeCookie,
+			remoteAddr:       localIP,
+			wantLoggedMetric: "web_client_viewing_local",
+		},
+		{
+			name:             "viewing-remote-unauthenticated",
+			cookie:           unauthenticatedRemoteNodeCookie,
+			remoteAddr:       remoteIP,
+			wantLoggedMetric: "web_client_viewing_remote",
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {