From 7ec0dc3834f377618d7030cfb8e425a3902f776b Mon Sep 17 00:00:00 2001
From: Brad Fitzpatrick <bradfitz@tailscale.com>
Date: Mon, 15 Apr 2024 15:05:50 -0700
Subject: [PATCH] ipn/ipnlocal: make StartLoginInteractive take (yet unused)
 context

In prep for future fix to undermentioned issue.

Updates tailscale/tailscale#7036

Change-Id: Ide114db917dcba43719482ffded6a9a54630d99e
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
---
 cmd/tsconnect/wasm/wasm_js.go |  2 +-
 ipn/ipnlocal/local.go         | 11 ++++++-----
 ipn/ipnlocal/local_test.go    |  4 ----
 ipn/ipnlocal/state_test.go    |  8 ++++----
 ipn/localapi/localapi.go      |  2 +-
 tsnet/tsnet.go                |  4 +++-
 6 files changed, 15 insertions(+), 16 deletions(-)

diff --git a/cmd/tsconnect/wasm/wasm_js.go b/cmd/tsconnect/wasm/wasm_js.go
index 055cb6f39..cb1f4fff1 100644
--- a/cmd/tsconnect/wasm/wasm_js.go
+++ b/cmd/tsconnect/wasm/wasm_js.go
@@ -322,7 +322,7 @@ func (i *jsIPN) run(jsCallbacks js.Value) {
 }
 
 func (i *jsIPN) login() {
-	go i.lb.StartLoginInteractive()
+	go i.lb.StartLoginInteractive(context.Background())
 }
 
 func (i *jsIPN) logout() {
diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go
index 6848ed10d..6c575d2d4 100644
--- a/ipn/ipnlocal/local.go
+++ b/ipn/ipnlocal/local.go
@@ -2834,11 +2834,11 @@ func (b *LocalBackend) tryLookupUserName(uid string) string {
 	return u.Username
 }
 
-// StartLoginInteractive implements Backend. It requests a new
-// interactive login from controlclient, unless such a flow is already
-// in progress, in which case StartLoginInteractive attempts to pick
-// up the in-progress flow where it left off.
-func (b *LocalBackend) StartLoginInteractive() {
+// StartLoginInteractive requests a new interactive login from controlclient,
+// unless such a flow is already in progress, in which case
+// StartLoginInteractive attempts to pick up the in-progress flow where it left
+// off.
+func (b *LocalBackend) StartLoginInteractive(ctx context.Context) error {
 	b.mu.Lock()
 	if b.cc == nil {
 		panic("LocalBackend.assertClient: b.cc == nil")
@@ -2858,6 +2858,7 @@ func (b *LocalBackend) StartLoginInteractive() {
 	} else {
 		cc.Login(nil, b.loginFlags|controlclient.LoginInteractive)
 	}
+	return nil
 }
 
 func (b *LocalBackend) Ping(ctx context.Context, ip netip.Addr, pingType tailcfg.PingType, size int) (*ipnstate.PingResult, error) {
diff --git a/ipn/ipnlocal/local_test.go b/ipn/ipnlocal/local_test.go
index c5c0c35a9..f3c8fbcff 100644
--- a/ipn/ipnlocal/local_test.go
+++ b/ipn/ipnlocal/local_test.go
@@ -862,10 +862,6 @@ type legacyBackend interface {
 	// Start starts or restarts the backend, typically when a
 	// frontend client connects.
 	Start(ipn.Options) error
-	// StartLoginInteractive requests to start a new interactive login
-	// flow. This should trigger a new BrowseToURL notification
-	// eventually.
-	StartLoginInteractive()
 }
 
 // Verify that LocalBackend still implements the legacyBackend interface
diff --git a/ipn/ipnlocal/state_test.go b/ipn/ipnlocal/state_test.go
index 9ed8a2f98..0e1769f1a 100644
--- a/ipn/ipnlocal/state_test.go
+++ b/ipn/ipnlocal/state_test.go
@@ -441,7 +441,7 @@ func TestStateMachine(t *testing.T) {
 	// indicating that the UI should browse to the given URL.
 	t.Logf("\n\nLogin (interactive)")
 	notifies.expect(1)
-	b.StartLoginInteractive()
+	b.StartLoginInteractive(context.Background())
 	{
 		nn := notifies.drain(1)
 		cc.assertCalls()
@@ -457,7 +457,7 @@ func TestStateMachine(t *testing.T) {
 	// we must always get a *new* login URL first.
 	t.Logf("\n\nLogin2 (interactive)")
 	notifies.expect(0)
-	b.StartLoginInteractive()
+	b.StartLoginInteractive(context.Background())
 	{
 		notifies.drain(0)
 		// backend asks control for another login sequence
@@ -677,7 +677,7 @@ func TestStateMachine(t *testing.T) {
 		c.Assert(ipn.NeedsLogin, qt.Equals, b.State())
 	}
 
-	b.StartLoginInteractive()
+	b.StartLoginInteractive(context.Background())
 	t.Logf("\n\nLoginFinished3")
 	notifies.expect(3)
 	cc.persist.UserProfile.LoginName = "user2"
@@ -800,7 +800,7 @@ func TestStateMachine(t *testing.T) {
 			ControlURL: "https://localhost:1/",
 		},
 	})
-	b.StartLoginInteractive()
+	b.StartLoginInteractive(context.Background())
 	url3 := "https://localhost:1/3"
 	cc.send(nil, url3, false, nil)
 	{
diff --git a/ipn/localapi/localapi.go b/ipn/localapi/localapi.go
index c7bac90e0..cef21ddd5 100644
--- a/ipn/localapi/localapi.go
+++ b/ipn/localapi/localapi.go
@@ -1306,7 +1306,7 @@ func (h *Handler) serveLoginInteractive(w http.ResponseWriter, r *http.Request)
 		http.Error(w, "want POST", http.StatusBadRequest)
 		return
 	}
-	h.b.StartLoginInteractive()
+	h.b.StartLoginInteractive(r.Context())
 	w.WriteHeader(http.StatusNoContent)
 	return
 }
diff --git a/tsnet/tsnet.go b/tsnet/tsnet.go
index 57aa7791c..d782834c2 100644
--- a/tsnet/tsnet.go
+++ b/tsnet/tsnet.go
@@ -597,7 +597,9 @@ func (s *Server) start() (reterr error) {
 	st := lb.State()
 	if st == ipn.NeedsLogin || envknob.Bool("TSNET_FORCE_LOGIN") {
 		logf("LocalBackend state is %v; running StartLoginInteractive...", st)
-		s.lb.StartLoginInteractive()
+		if err := s.lb.StartLoginInteractive(s.shutdownCtx); err != nil {
+			return fmt.Errorf("StartLoginInteractive: %w", err)
+		}
 	} else if authKey != "" {
 		logf("Authkey is set; but state is %v. Ignoring authkey. Re-run with TSNET_FORCE_LOGIN=1 to force use of authkey.", st)
 	}