diff --git a/client/web/qnap.go b/client/web/qnap.go
index 5ccd76798..07145a77c 100644
--- a/client/web/qnap.go
+++ b/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.
-// authorizeQNAP manages writing out any relevant authorization errors to the
-// ResponseWriter itself.
-func authorizeQNAP(w http.ResponseWriter, r *http.Request) (ok bool) {
+// If the user is not authorized to use the client, an error is returned.
+func authorizeQNAP(r *http.Request) (ar authResponse, err error) {
_, resp, err := qnapAuthn(r)
if err != nil {
- http.Error(w, err.Error(), http.StatusUnauthorized)
- return false
+ return ar, err
}
if resp.IsAdmin == 0 {
- http.Error(w, "user is not an admin", http.StatusForbidden)
- return false
+ return ar, errors.New("user is not an admin")
}
- return true
+ return authResponse{OK: true}, nil
}
type qnapAuthResponse struct {
diff --git a/client/web/src/components/app.tsx b/client/web/src/components/app.tsx
index 8b10ae475..edbd885aa 100644
--- a/client/web/src/components/app.tsx
+++ b/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 (
- {!data || loadingAuth ? (
+ {loadingAuth ? (
+
Loading...
// TODO(sonia): add a loading view
+ ) : (
+
+ )}
+
+ )
+}
+
+function WebClient({
+ auth,
+ waitOnAuth,
+}: {
+ auth?: AuthResponse
+ waitOnAuth: () => Promise
+}) {
+ const { data, refreshData, updateNode } = useNodeData()
+
+ return (
+ <>
+ {!data ? (
Loading...
// 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 && }
-
+ {data && }
+ >
)
}
diff --git a/client/web/src/hooks/auth.ts b/client/web/src/hooks/auth.ts
index 238049288..ca1a3add1 100644
--- a/client/web/src/hooks/auth.ts
+++ b/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()
- const [loading, setLoading] = useState(false)
+ const [loading, setLoading] = useState(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)
diff --git a/client/web/synology.go b/client/web/synology.go
index 5f36cc93e..126e8ed84 100644
--- a/client/web/synology.go
+++ b/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.
-// authorizeSynology manages writing out any relevant authorization errors to the
-// ResponseWriter itself.
-func authorizeSynology(w http.ResponseWriter, r *http.Request) (ok bool) {
- if synoTokenRedirect(w, r) {
- return false
+// The returned authResponse indicates if the user is authorized,
+// and if additional steps are needed to authenticate the user.
+// If the user is authenticated, but not authorized to use the client, an error is returned.
+func authorizeSynology(r *http.Request) (resp authResponse, err error) {
+ if !hasSynoToken(r) {
+ 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 false
+ return resp, fmt.Errorf("auth: %v: %s", err, out)
}
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 false
+ return resp, err
}
if !isAdmin {
- http.Error(w, "not a member of administrators group", http.StatusForbidden)
- return false
+ return resp, errors.New("not a member of administrators group")
}
- 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.
- // So we tell the client to get one.
- _, _ = fmt.Fprint(w, synoTokenRedirectHTML)
- return true
+ return false
}
-
-const synoTokenRedirectHTML = `
-Redirecting with session token...
-
-`
diff --git a/client/web/web.go b/client/web/web.go
index 8f3a20ae0..2560bfbb1 100644
--- a/client/web/web.go
+++ b/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 {
- case strings.HasPrefix(r.URL.Path, "/assets/") && r.Method == httpm.GET:
- // Don't require authorization for static assets.
- return true
- case d == distro.Synology:
- return authorizeSynology(w, r)
- case d == distro.QNAP:
- return authorizeQNAP(w, r)
+ switch distro.Get() {
+ case distro.Synology:
+ resp, _ := authorizeSynology(r)
+ return resp.OK
+ case distro.QNAP:
+ resp, _ := authorizeQNAP(r)
+ return resp.OK
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:
diff --git a/client/web/web_test.go b/client/web/web_test.go
index a3a94d599..d1efb1abd 100644
--- a/client/web/web_test.go
+++ b/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()