ipn/localapi,client/web: clean up auth error handling

This commit makes two changes to the web client auth flow error
handling:

1. Properly passes back the error code from the noise request from
   the localapi. Previously we were using io.Copy, which was always
   setting a 200 response status code.
2. Clean up web client browser sessions on any /wait endpoint error.
   This avoids the user getting in a stuck state if something goes
   wrong with their auth path.

Updates tailscale/corp#14335

Signed-off-by: Sonia Appasamy <sonia@tailscale.com>
This commit is contained in:
Sonia Appasamy 2023-10-20 13:24:29 -04:00 committed by Sonia Appasamy
parent 70f9c8a6ed
commit 68da15516f
2 changed files with 12 additions and 16 deletions

View File

@ -292,7 +292,6 @@ func (s *Server) serveLoginAPI(w http.ResponseWriter, r *http.Request) {
errNotUsingTailscale = errors.New("not-using-tailscale") errNotUsingTailscale = errors.New("not-using-tailscale")
errTaggedSource = errors.New("tagged-source") errTaggedSource = errors.New("tagged-source")
errNotOwner = errors.New("not-owner") errNotOwner = errors.New("not-owner")
errFailedAuth = errors.New("failed-auth")
) )
// getTailscaleBrowserSession retrieves the browser session associated with // getTailscaleBrowserSession retrieves the browser session associated with
@ -413,12 +412,12 @@ func (s *Server) serveTailscaleAuth(w http.ResponseWriter, r *http.Request) {
if r.URL.Query().Get("wait") == "true" { if r.URL.Query().Get("wait") == "true" {
// Client requested we block until user completes auth. // Client requested we block until user completes auth.
d, err := s.getOrAwaitAuth(r.Context(), session.AuthID, whois.Node.ID) d, err := s.getOrAwaitAuth(r.Context(), session.AuthID, whois.Node.ID)
if errors.Is(err, errFailedAuth) { if err != nil {
http.Error(w, "user is unauthorized", http.StatusUnauthorized) http.Error(w, err.Error(), http.StatusUnauthorized)
s.browserSessions.Delete(session.ID) // clean up the failed session // Clean up the session. Doing this on any error from control
return // server to avoid the user getting stuck with a bad session
} else if err != nil { // cookie.
http.Error(w, err.Error(), http.StatusInternalServerError) s.browserSessions.Delete(session.ID)
return return
} }
if d.Complete { if d.Complete {
@ -485,11 +484,7 @@ type data struct {
} }
body, _ := io.ReadAll(resp.Body) body, _ := io.ReadAll(resp.Body)
resp.Body.Close() resp.Body.Close()
if resp.StatusCode == http.StatusUnauthorized { if resp.StatusCode != http.StatusOK {
// User completed auth, but control server reported
// them unauthorized to manage this node.
return nil, errFailedAuth
} else if resp.StatusCode != http.StatusOK {
return nil, fmt.Errorf("failed request: %s", body) return nil, fmt.Errorf("failed request: %s", body)
} }
var authResp *tailcfg.WebClientAuthResponse var authResp *tailcfg.WebClientAuthResponse

View File

@ -2172,12 +2172,13 @@ type reqData struct {
http.Error(w, err.Error(), http.StatusInternalServerError) http.Error(w, err.Error(), http.StatusInternalServerError)
return return
} }
defer resp.Body.Close() body, _ := io.ReadAll(resp.Body)
resp.Body.Close()
if _, err := io.Copy(w, resp.Body); err != nil { if resp.StatusCode != http.StatusOK {
http.Error(w, err.Error(), http.StatusInternalServerError) http.Error(w, string(body), resp.StatusCode)
return return
} }
w.Write(body)
w.Header().Set("Content-Type", "application/json") w.Header().Set("Content-Type", "application/json")
} }