From cbf92413a557aa160cc16f2acfea2b381b0651a6 Mon Sep 17 00:00:00 2001 From: Gesa Stupperich Date: Mon, 20 Oct 2025 09:50:32 +0100 Subject: [PATCH] client/web: move auth wait to backend Signed-off-by: Gesa Stupperich --- client/web/auth.go | 35 +++++++------------------ client/web/src/hooks/auth.ts | 35 ------------------------- client/web/web.go | 14 +++++++++- client/web/web_test.go | 51 +++++++++++++++++++++++------------- 4 files changed, 55 insertions(+), 80 deletions(-) diff --git a/client/web/auth.go b/client/web/auth.go index f62fdada1..577c5ba2e 100644 --- a/client/web/auth.go +++ b/client/web/auth.go @@ -37,6 +37,7 @@ type browserSession struct { AuthURL string // from tailcfg.WebClientAuthResponse Created time.Time Authenticated bool + PendingAuth bool } // isAuthorized reports true if the given session is authorized @@ -127,23 +128,12 @@ func (s *Server) getSession(r *http.Request) (*browserSession, *apitype.WhoIsRes cookie, err := r.Cookie(sessionCookieName) if errors.Is(err, http.ErrNoCookie) { - s.logf("auth: no session cookie found") return nil, whoIs, status, errNoSession } else if err != nil { return nil, whoIs, status, err } v, ok := s.browserSessions.Load(cookie.Value) if !ok { - s.logf("auth: session for %v not found in session store", cookie.Value) - count := 0 - s.browserSessions.Range(func(k, v interface{}) bool { - count++ - bs := v.(*browserSession) - s.logf("auth: session for %v: ID=%v, AuthID=%v, SrcNode=%v, SrcUser=%v, Created=%v, Authenticated=%v", - k, bs.ID, bs.AuthID, bs.SrcNode, bs.SrcUser, bs.Created, bs.Authenticated) - return true - }) - s.logf("auth: total sessions in store: %d", count) return nil, whoIs, status, errNoSession } session := v.(*browserSession) @@ -175,7 +165,7 @@ func (s *Server) newSession(ctx context.Context, src *apitype.WhoIsResponse) (*b Created: s.timeNow(), } - if true { + if s.controlSupportsCheckMode(ctx) { // control supports check mode, so get a new auth URL and return. a, err := s.newAuthURL(ctx, src.Node.ID) if err != nil { @@ -183,22 +173,14 @@ func (s *Server) newSession(ctx context.Context, src *apitype.WhoIsResponse) (*b } session.AuthID = a.ID session.AuthURL = a.URL + session.PendingAuth = true } else { // control does not support check mode, so there is no additional auth we can do. session.Authenticated = true } - s.logf("auth: session created for %v, %v", session.AuthID, session.ID) - // Make a deep copy of the session to ensure it's properly stored - sessionCopy := *session - s.browserSessions.Store(sid, &sessionCopy) - value, ok := s.browserSessions.Load(sid) - if !ok { - s.logf("auth: session not stored for %v", session.AuthID) - } else { - session = value.(*browserSession) - s.logf("auth: session stored for %v, %v: %v", session.AuthID, session.ID, value) - } + s.browserSessions.Store(sid, session) + return session, nil } @@ -227,23 +209,24 @@ func (s *Server) awaitUserAuth(ctx context.Context, session *browserSession) err if session.isAuthorized(s.timeNow()) { return nil // already authorized } + a, err := s.waitAuthURL(ctx, session.AuthID, session.SrcNode) if err != nil { // Don't delete the session on context cancellation, as this is expected - // when users navigate away or refresh the page + // when users navigate away or refresh the page. if errors.Is(err, context.Canceled) { - s.logf("awaitUserAuth: context canceled for session %v - not deleting session", session.ID) return err } - s.logf("awaitUserAuth: ERROR from waitAuthURL: %v - DELETING SESSION %v", err, session.ID) // Clean up the session for non-cancellation errors from control server // to avoid the user getting stuck with a bad session cookie. s.browserSessions.Delete(session.ID) return err } + if a.Complete { session.Authenticated = a.Complete + session.PendingAuth = false s.browserSessions.Store(session.ID, session) } return nil diff --git a/client/web/src/hooks/auth.ts b/client/web/src/hooks/auth.ts index e79080b1f..51eb0c400 100644 --- a/client/web/src/hooks/auth.ts +++ b/client/web/src/hooks/auth.ts @@ -81,25 +81,7 @@ export default function useAuth() { return apiFetch<{ authUrl?: string }>("/auth/session/new", "GET") .then((d) => { if (d.authUrl) { - // Store a flag to signal that auth is in progress. - // We try to open the auth URL in a new tab, but it might open - // in the same tab depending on browser settings - // (https://github.com/tailscale/tailscale/issues/11905). - // The flag will be used when returning to the app (in any tab). - localStorage.setItem("ts-web-needs-auth-wait", "true") - - // Open the auth URL - this might open in a new tab or - // replace the current tab depending on browser settings. window.open(d.authUrl, "_blank") - - // If execution continues here, we're still in the original tab - // (the URL opened in a new tab or the popup was blocked). - // If it opened in the same tab, we'll handle this when the user - // returns via the effect that checks for the pending auth flag. - // If it opened in a new tab, we can wait for auth to complete now. - // (If the popup was blocked, we'll also wait here.) - - // Wait for the auth session to complete. return apiFetch("/auth/session/wait", "GET") } }) @@ -133,23 +115,6 @@ export default function useAuth() { // eslint-disable-next-line react-hooks/exhaustive-deps }, [ranSynoAuth]) - // Handle returning to the UI after having completed the auth flow in any tab. - useEffect(() => { - // Check if we have a pending auth session when the component mounts. - const pendingAuth = localStorage.getItem("ts-web-needs-auth-wait") - if (pendingAuth === "true") { - // Wait for the auth session to complete. - apiFetch("/auth/session/wait", "GET") - .then(() => { - loadAuth() - localStorage.removeItem("ts-web-needs-auth-wait") - }) - .catch((error) => { - console.error(error) - }) - } - }, [loadAuth]) - return { data, loading, diff --git a/client/web/web.go b/client/web/web.go index cd5a80a31..37b0c7918 100644 --- a/client/web/web.go +++ b/client/web/web.go @@ -370,7 +370,6 @@ func (s *Server) serve(w http.ResponseWriter, r *http.Request) { s.serveAPIAuthSessionNew(w, r) // create new session return case r.URL.Path == "/api/auth/session/wait" && r.Method == httpm.GET: - s.logf("web: wait for auth session") s.serveAPIAuthSessionWait(w, r) // wait for session to be authorized return } @@ -772,6 +771,19 @@ func (s *Server) serveAPIAuth(w http.ResponseWriter, r *http.Request) { } } + // We might have a session for which we haven't awaited the result yet. + // This can happen when the AuthURL opens in the same browser tab instead + // of a new one due to browser settings. + // (See https://github.com/tailscale/tailscale/issues/11905) + // We therefore set a PendingAuth flag when creating a new session, check + // it here and call awaitUserAuth if we find it to be true. Once the auth + // wait completes, awaitUserAuth will set PendingAuth to false. + if sErr == nil && session.PendingAuth == true { + if err := s.awaitUserAuth(r.Context(), session); err != nil { + sErr = err + } + } + switch { case sErr != nil && errors.Is(sErr, errNotUsingTailscale): s.lc.IncrementCounter(r.Context(), "web_client_viewing_local", 1) diff --git a/client/web/web_test.go b/client/web/web_test.go index 9ba16bccf..8adc137a3 100644 --- a/client/web/web_test.go +++ b/client/web/web_test.go @@ -582,12 +582,23 @@ func TestServeAuth(t *testing.T) { successCookie := "ts-cookie-success" s.browserSessions.Store(successCookie, &browserSession{ - ID: successCookie, - SrcNode: remoteNode.Node.ID, - SrcUser: user.ID, - Created: oneHourAgo, - AuthID: testAuthPathSuccess, - AuthURL: *testControlURL + testAuthPathSuccess, + ID: successCookie, + SrcNode: remoteNode.Node.ID, + SrcUser: user.ID, + Created: oneHourAgo, + AuthID: testAuthPathSuccess, + AuthURL: *testControlURL + testAuthPathSuccess, + PendingAuth: true, + }) + successCookie2 := "ts-cookie-success-2" + s.browserSessions.Store(successCookie2, &browserSession{ + ID: successCookie2, + SrcNode: remoteNode.Node.ID, + SrcUser: user.ID, + Created: oneHourAgo, + AuthID: testAuthPathSuccess, + AuthURL: *testControlURL + testAuthPathSuccess, + PendingAuth: true, }) failureCookie := "ts-cookie-failure" s.browserSessions.Store(failureCookie, &browserSession{ @@ -642,14 +653,15 @@ func TestServeAuth(t *testing.T) { AuthID: testAuthPath, AuthURL: *testControlURL + testAuthPath, Authenticated: false, + PendingAuth: true, }, }, { - name: "query-existing-incomplete-session", - path: "/api/auth", + name: "existing-session-used", + path: "/api/auth/session/new", // should not create new session cookie: successCookie, wantStatus: http.StatusOK, - wantResp: &authResponse{ViewerIdentity: vi, ServerMode: ManageServerMode}, + wantResp: &newSessionAuthResponse{AuthURL: *testControlURL + testAuthPathSuccess}, wantSession: &browserSession{ ID: successCookie, SrcNode: remoteNode.Node.ID, @@ -658,14 +670,15 @@ func TestServeAuth(t *testing.T) { AuthID: testAuthPathSuccess, AuthURL: *testControlURL + testAuthPathSuccess, Authenticated: false, + PendingAuth: true, }, }, { - name: "existing-session-used", - path: "/api/auth/session/new", // should not create new session + name: "transition-to-successful-session-via-api-auth-session-wait", + path: "/api/auth/session/wait", cookie: successCookie, wantStatus: http.StatusOK, - wantResp: &newSessionAuthResponse{AuthURL: *testControlURL + testAuthPathSuccess}, + wantResp: nil, wantSession: &browserSession{ ID: successCookie, SrcNode: remoteNode.Node.ID, @@ -673,17 +686,17 @@ func TestServeAuth(t *testing.T) { Created: oneHourAgo, AuthID: testAuthPathSuccess, AuthURL: *testControlURL + testAuthPathSuccess, - Authenticated: false, + Authenticated: true, }, }, { - name: "transition-to-successful-session", - path: "/api/auth/session/wait", - cookie: successCookie, + name: "transition-to-successful-session-via-api-auth", + path: "/api/auth", + cookie: successCookie2, wantStatus: http.StatusOK, - wantResp: nil, + wantResp: &authResponse{Authorized: true, ViewerIdentity: vi, ServerMode: ManageServerMode}, wantSession: &browserSession{ - ID: successCookie, + ID: successCookie2, SrcNode: remoteNode.Node.ID, SrcUser: user.ID, Created: oneHourAgo, @@ -731,6 +744,7 @@ func TestServeAuth(t *testing.T) { AuthID: testAuthPath, AuthURL: *testControlURL + testAuthPath, Authenticated: false, + PendingAuth: true, }, }, { @@ -748,6 +762,7 @@ func TestServeAuth(t *testing.T) { AuthID: testAuthPath, AuthURL: *testControlURL + testAuthPath, Authenticated: false, + PendingAuth: true, }, }, {