client/web: limit authorization checks to API calls

This completes the migration to setting up authentication state in the
client first before fetching any node data or rendering the client view.

Notable changes:
 - `authorizeRequest` is now only enforced on `/api/*` calls (with the
   exception of /api/auth, which is handled early because it's needed to
   initially setup auth, particularly for synology)
 - re-separate the App and WebClient components to ensure that auth is
   completed before moving on
 - refactor platform auth (synology and QNAP) to fit into this new
   structure. Synology no longer returns redirect for auth, but returns
   authResponse instructing the client to fetch a SynoToken

Updates tailscale/corp#14335

Signed-off-by: Will Norris <will@tailscale.com>

client/web/qnap.go

@@ -9,6 +9,7 @@ package web
 import (
 	"crypto/tls"
 	"encoding/xml"
+	"errors"
 	"fmt"
 	"io"
 	"log"
@@ -18,21 +19,17 @@ import (
 
 // authorizeQNAP authenticates the logged-in QNAP user and verifies that they
 // are authorized to use the web client.
-// It reports true if the request is authorized to continue, and false otherwise.
+// If the user is not authorized to use the client, an error is returned.
-// authorizeQNAP manages writing out any relevant authorization errors to the
+func authorizeQNAP(r *http.Request) (ar authResponse, err error) {
-// ResponseWriter itself.
-func authorizeQNAP(w http.ResponseWriter, r *http.Request) (ok bool) {
 	_, resp, err := qnapAuthn(r)
 	if err != nil {
-		http.Error(w, err.Error(), http.StatusUnauthorized)
+		return ar, err
-		return false
 	}
 	if resp.IsAdmin == 0 {
-		http.Error(w, "user is not an admin", http.StatusForbidden)
+		return ar, errors.New("user is not an admin")
-		return false
 	}
 
-	return true
+	return authResponse{OK: true}, nil
 }
 
 type qnapAuthResponse struct {

client/web/src/components/app.tsx

@@ -3,17 +3,36 @@ import React from "react"
 import LegacyClientView from "src/components/views/legacy-client-view"
 import LoginClientView from "src/components/views/login-client-view"
 import ReadonlyClientView from "src/components/views/readonly-client-view"
-import useAuth from "src/hooks/auth"
+import useAuth, { AuthResponse } from "src/hooks/auth"
 import useNodeData from "src/hooks/node-data"
 import ManagementClientView from "./views/management-client-view"
 
 export default function App() {
-  const { data, refreshData, updateNode } = useNodeData()
   const { data: auth, loading: loadingAuth, waitOnAuth } = useAuth()
 
   return (
     <div className="flex flex-col items-center min-w-sm max-w-lg mx-auto py-14">
-      {!data || loadingAuth ? (
+      {loadingAuth ? (
+        <div className="text-center py-14">Loading...</div> // TODO(sonia): add a loading view
+      ) : (
+        <WebClient auth={auth} waitOnAuth={waitOnAuth} />
+      )}
+    </div>
+  )
+}
+
+function WebClient({
+  auth,
+  waitOnAuth,
+}: {
+  auth?: AuthResponse
+  waitOnAuth: () => Promise<void>
+}) {
+  const { data, refreshData, updateNode } = useNodeData()
+
+  return (
+    <>
+      {!data ? (
         <div className="text-center py-14">Loading...</div> // TODO(sonia): add a loading view
       ) : data?.Status === "NeedsLogin" || data?.Status === "NoState" ? (
         // Client not on a tailnet, render login.
@@ -35,8 +54,8 @@ export default function App() {
           updateNode={updateNode}
         />
       )}
-      {data && !loadingAuth && <Footer licensesURL={data.LicensesURL} />}
+      {data && <Footer licensesURL={data.LicensesURL} />}
-    </div>
+    </>
   )
 }
 

client/web/src/hooks/auth.ts

@@ -16,7 +16,7 @@ export type AuthResponse = {
 // for the web client.
 export default function useAuth() {
   const [data, setData] = useState<AuthResponse>()
-  const [loading, setLoading] = useState<boolean>(false)
+  const [loading, setLoading] = useState<boolean>(true)
 
   const loadAuth = useCallback((wait?: boolean) => {
     const url = wait ? "/auth?wait=true" : "/auth"
@@ -24,16 +24,19 @@ export default function useAuth() {
     return apiFetch(url, "GET")
       .then((r) => r.json())
       .then((d) => {
-        if ((d as AuthResponse).authNeeded == AuthType.synology) {
-          fetch("/webman/login.cgi")
-            .then((r) => r.json())
-            .then((data) => {
-              setSynoToken(data.SynoToken)
-            })
-        }
-
-        setLoading(false)
         setData(d)
+        switch ((d as AuthResponse).authNeeded) {
+          case AuthType.synology:
+            fetch("/webman/login.cgi")
+              .then((r) => r.json())
+              .then((a) => {
+                setSynoToken(a.SynoToken)
+                setLoading(false)
+              })
+            break
+          default:
+            setLoading(false)
+        }
       })
       .catch((error) => {
         setLoading(false)

client/web/synology.go

@@ -7,6 +7,7 @@
 package web
 
 import (
+	"errors"
 	"fmt"
 	"net/http"
 	"os/exec"
@@ -17,62 +18,44 @@ import (
 
 // authorizeSynology authenticates the logged-in Synology user and verifies
 // that they are authorized to use the web client.
-// It reports true if the request is authorized to continue, and false otherwise.
+// The returned authResponse indicates if the user is authorized,
-// authorizeSynology manages writing out any relevant authorization errors to the
+// and if additional steps are needed to authenticate the user.
-// ResponseWriter itself.
+// If the user is authenticated, but not authorized to use the client, an error is returned.
-func authorizeSynology(w http.ResponseWriter, r *http.Request) (ok bool) {
+func authorizeSynology(r *http.Request) (resp authResponse, err error) {
-	if synoTokenRedirect(w, r) {
+	if !hasSynoToken(r) {
-		return false
+		return authResponse{OK: false, AuthNeeded: synoAuth}, nil
 	}
 
 	// authenticate the Synology user
 	cmd := exec.Command("/usr/syno/synoman/webman/modules/authenticate.cgi")
 	out, err := cmd.CombinedOutput()
 	if err != nil {
-		http.Error(w, fmt.Sprintf("auth: %v: %s", err, out), http.StatusUnauthorized)
+		return resp, fmt.Errorf("auth: %v: %s", err, out)
-		return false
 	}
 	user := strings.TrimSpace(string(out))
 
 	// check if the user is in the administrators group
 	isAdmin, err := groupmember.IsMemberOfGroup("administrators", user)
 	if err != nil {
-		http.Error(w, err.Error(), http.StatusForbidden)
+		return resp, err
-		return false
 	}
 	if !isAdmin {
-		http.Error(w, "not a member of administrators group", http.StatusForbidden)
+		return resp, errors.New("not a member of administrators group")
-		return false
 	}
 
-	return true
+	return authResponse{OK: true}, nil
 }
 
-func synoTokenRedirect(w http.ResponseWriter, r *http.Request) bool {
+// hasSynoToken returns true if the request include a SynoToken used for synology auth.
+func hasSynoToken(r *http.Request) bool {
 	if r.Header.Get("X-Syno-Token") != "" {
-		return false
+		return true
 	}
 	if r.URL.Query().Get("SynoToken") != "" {
-		return false
+		return true
 	}
 	if r.Method == "POST" && r.FormValue("SynoToken") != "" {
-		return false
+		return true
 	}
-	// We need a SynoToken for authenticate.cgi.
+	return false
-	// So we tell the client to get one.
-	_, _ = fmt.Fprint(w, synoTokenRedirectHTML)
-	return true
 }
-
-const synoTokenRedirectHTML = `<html>
-Redirecting with session token...
-<script>
-  fetch("/webman/login.cgi")
-    .then(r => r.json())
-    .then(data => {
-	u = new URL(window.location)
-	u.searchParams.set("SynoToken", data.SynoToken)
-	document.location = u
-    })
-</script>
-`

client/web/web.go

@@ -178,10 +178,15 @@ func (s *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 }
 
 func (s *Server) serve(w http.ResponseWriter, r *http.Request) {
-	if ok := s.authorizeRequest(w, r); !ok {
-		return
-	}
 	if strings.HasPrefix(r.URL.Path, "/api/") {
+		if r.Method == httpm.GET && r.URL.Path == "/api/auth" {
+			s.serveAPIAuth(w, r)
+			return
+		}
+		if ok := s.authorizeRequest(w, r); !ok {
+			http.Error(w, "not authorized", http.StatusUnauthorized)
+			return
+		}
 		// Pass API requests through to the API handler.
 		s.apiHandler.ServeHTTP(w, r)
 		return
@@ -208,9 +213,6 @@ func (s *Server) authorizeRequest(w http.ResponseWriter, r *http.Request) (ok bo
 		case r.URL.Path == "/api/data" && r.Method == httpm.GET:
 			// Readonly endpoint allowed without browser session.
 			return true
-		case r.URL.Path == "/api/auth":
-			// Endpoint for browser to request auth allowed without browser session.
-			return true
 		case strings.HasPrefix(r.URL.Path, "/api/"):
 			// All other /api/ endpoints require a valid browser session.
 			//
@@ -229,15 +231,13 @@ func (s *Server) authorizeRequest(w http.ResponseWriter, r *http.Request) (ok bo
 		}
 	}
 	// Client using system-specific auth.
-	d := distro.Get()
+	switch distro.Get() {
-	switch {
+	case distro.Synology:
-	case strings.HasPrefix(r.URL.Path, "/assets/") && r.Method == httpm.GET:
+		resp, _ := authorizeSynology(r)
-		// Don't require authorization for static assets.
+		return resp.OK
-		return true
+	case distro.QNAP:
-	case d == distro.Synology:
+		resp, _ := authorizeQNAP(r)
-		return authorizeSynology(w, r)
+		return resp.OK
-	case d == distro.QNAP:
-		return authorizeQNAP(w, r)
 	default:
 		return true // no additional auth for this distro
 	}
@@ -252,11 +252,6 @@ func (s *Server) serveLoginAPI(w http.ResponseWriter, r *http.Request) {
 		http.Error(w, "invalid endpoint", http.StatusNotFound)
 		return
 	}
-	if r.URL.Path != "/api/auth" {
-		// empty JSON response until we serve auth for the login client
-		fmt.Fprintf(w, "{}")
-		return
-	}
 	switch r.Method {
 	case httpm.GET:
 		// TODO(soniaappasamy): we may want a minimal node data response here
@@ -282,7 +277,9 @@ type authResponse struct {
 	AuthNeeded authType `json:"authNeeded,omitempty"` // filled when user needs to complete a specific type of auth
 }
 
-func (s *Server) serveTailscaleAuth(w http.ResponseWriter, r *http.Request) {
+// serverAPIAuth handles requests to the /api/auth endpoint
+// and returns an authResponse indicating the current auth state and any steps the user needs to take.
+func (s *Server) serveAPIAuth(w http.ResponseWriter, r *http.Request) {
 	if r.Method != httpm.GET {
 		http.Error(w, "method not allowed", http.StatusMethodNotAllowed)
 		return
@@ -291,6 +288,24 @@ func (s *Server) serveTailscaleAuth(w http.ResponseWriter, r *http.Request) {
 
 	session, whois, err := s.getSession(r)
 	switch {
+	case err != nil && errors.Is(err, errNotUsingTailscale):
+		// not using tailscale, so perform platform auth
+		switch distro.Get() {
+		case distro.Synology:
+			resp, err = authorizeSynology(r)
+			if err != nil {
+				http.Error(w, err.Error(), http.StatusUnauthorized)
+				return
+			}
+		case distro.QNAP:
+			resp, err = authorizeQNAP(r)
+			if err != nil {
+				http.Error(w, err.Error(), http.StatusUnauthorized)
+				return
+			}
+		default:
+			resp.OK = true // no additional auth for this distro
+		}
 	case err != nil && !errors.Is(err, errNoSession):
 		http.Error(w, err.Error(), http.StatusUnauthorized)
 		return
@@ -341,14 +356,6 @@ func (s *Server) serveAPI(w http.ResponseWriter, r *http.Request) {
 	w.Header().Set("X-CSRF-Token", csrf.Token(r))
 	path := strings.TrimPrefix(r.URL.Path, "/api")
 	switch {
-	case path == "/auth":
-		if s.tsDebugMode == "full" { // behind debug flag
-			s.serveTailscaleAuth(w, r)
-		} else {
-			// empty JSON response until we serve auth for other modes
-			fmt.Fprintf(w, "{}")
-		}
-		return
 	case path == "/data":
 		switch r.Method {
 		case httpm.GET:

client/web/web_test.go

@@ -369,12 +369,6 @@ func TestAuthorizeRequest(t *testing.T) {
 		wantOkNotOverTailscale: false,
 		wantOkWithoutSession:   false,
 		wantOkWithSession:      true,
-	}, {
-		reqPath:                "/api/auth",
-		reqMethod:              httpm.GET,
-		wantOkNotOverTailscale: false,
-		wantOkWithoutSession:   true,
-		wantOkWithSession:      true,
 	}, {
 		reqPath:                "/api/somethingelse",
 		reqMethod:              httpm.GET,
@@ -587,7 +581,7 @@ func TestServeTailscaleAuth(t *testing.T) {
 			r.RemoteAddr = remoteIP
 			r.AddCookie(&http.Cookie{Name: sessionCookieName, Value: tt.cookie})
 			w := httptest.NewRecorder()
-			s.serveTailscaleAuth(w, r)
+			s.serveAPIAuth(w, r)
 			res := w.Result()
 			defer res.Body.Close()