fix(auth): force invalidate invalid session cookies in middleware (#1358)

* fix(auth): invalidate session cookies in middleware

* fix(auth): set path for invalid cookie

internal/http/auth.go

@@ -66,36 +66,37 @@ func (h authHandler) login(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	h.cookieStore.Options.HttpOnly = true
-	h.cookieStore.Options.SameSite = http.SameSiteLaxMode
-	h.cookieStore.Options.Path = h.config.BaseURL
+	if _, err := h.service.Login(r.Context(), data.Username, data.Password); err != nil {
+		h.log.Error().Err(err).Msgf("Auth: Failed login attempt username: [%s] ip: %s", data.Username, r.RemoteAddr)
+		h.encoder.StatusError(w, http.StatusForbidden, errors.New("could not login: bad credentials"))
+		return
+	}
+
+	// create new session
+	session, err := h.cookieStore.Get(r, "user_session")
+	if err != nil {
+		h.log.Error().Err(err).Msgf("Auth: Failed to create cookies with attempt username: [%s] ip: %s", data.Username, r.RemoteAddr)
+		h.encoder.StatusError(w, http.StatusInternalServerError, errors.New("could not create cookies"))
+		return
+	}
+
+	// Set user as authenticated
+	session.Values["authenticated"] = true
+
+	// Set cookie options
+	session.Options.HttpOnly = true
+	session.Options.SameSite = http.SameSiteLaxMode
+	session.Options.Path = h.config.BaseURL
 
 	// autobrr does not support serving on TLS / https, so this is only available behind reverse proxy
 	// if forwarded protocol is https then set cookie secure
 	// SameSite Strict can only be set with a secure cookie. So we overwrite it here if possible.
 	// https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie/SameSite
 	if r.Header.Get("X-Forwarded-Proto") == "https" {
-		h.cookieStore.Options.Secure = true
-		h.cookieStore.Options.SameSite = http.SameSiteStrictMode
+		session.Options.Secure = true
+		session.Options.SameSite = http.SameSiteStrictMode
 	}
 
-	if _, err := h.service.Login(r.Context(), data.Username, data.Password); err != nil {
-		h.log.Error().Err(err).Msgf("Auth: Failed login attempt username: [%s] ip: %s", data.Username, r.RemoteAddr)
-		h.encoder.StatusError(w, http.StatusUnauthorized, errors.New("could not login: bad credentials"))
-		return
-	}
-
-	// create new session
-	session, err := h.cookieStore.New(r, "user_session")
-	if err != nil {
-		h.log.Error().Err(err).Msgf("Auth: Failed to parse cookies with attempt username: [%s] ip: %s", data.Username, r.RemoteAddr)
-		h.encoder.StatusError(w, http.StatusUnauthorized, errors.New("could not parse cookies"))
-		return
-	}
-
-	// Set user as authenticated
-	session.Values["authenticated"] = true
-
 	if err := session.Save(r, w); err != nil {
 		h.encoder.StatusError(w, http.StatusInternalServerError, errors.Wrap(err, "could not save session"))
 		return
@@ -118,6 +119,8 @@ func (h authHandler) logout(w http.ResponseWriter, r *http.Request) {
 		// MaxAge<0 means delete cookie immediately
 		session.Options.MaxAge = -1
 
+		session.Options.Path = h.config.BaseURL
+
 		if err := session.Save(r, w); err != nil {
 			h.log.Error().Err(err).Msgf("could not store session: %s", r.RemoteAddr)
 			h.encoder.StatusError(w, http.StatusInternalServerError, err)
@@ -168,7 +171,7 @@ func (h authHandler) onboardEligible(ctx context.Context) (int, error) {
 	}
 
 	if userCount > 0 {
-		return http.StatusForbidden, errors.New("onboarding unavailable")
+		return http.StatusServiceUnavailable, errors.New("onboarding unavailable")
 	}
 
 	return http.StatusOK, nil

internal/http/auth_test.go

@@ -272,8 +272,8 @@ func TestAuthHandlerValidateBad(t *testing.T) {
 
 	defer resp.Body.Close()
 
-	if status := resp.StatusCode; status != http.StatusUnauthorized {
-		t.Errorf("validate: handler returned wrong status code: got %v want %v", status, http.StatusUnauthorized)
+	if status := resp.StatusCode; status != http.StatusNoContent {
+		t.Errorf("validate: handler returned wrong status code: got %v want %v", status, http.StatusNoContent)
 	}
 }
 
@@ -320,9 +320,9 @@ func TestAuthHandlerLoginBad(t *testing.T) {
 
 	defer resp.Body.Close()
 
-	// check for response, here we'll just check for 204 NoContent
-	if status := resp.StatusCode; status != http.StatusUnauthorized {
-		t.Errorf("handler returned wrong status code: got %v want %v", status, http.StatusUnauthorized)
+	// check for response, here we'll just check for 403 Forbidden
+	if status := resp.StatusCode; status != http.StatusForbidden {
+		t.Errorf("handler returned wrong status code: got %v want %v", status, http.StatusForbidden)
 	}
 }
 

internal/http/middleware.go

@@ -24,7 +24,7 @@ func (s Server) IsAuthenticated(next http.Handler) http.Handler {
 			}
 
 		} else if key := r.URL.Query().Get("apikey"); key != "" {
-			// check query param lke ?apikey=TOKEN
+			// check query param like ?apikey=TOKEN
 			if !s.apiService.ValidateAPIKey(r.Context(), key) {
 				http.Error(w, http.StatusText(http.StatusUnauthorized), http.StatusUnauthorized)
 				return
@@ -33,18 +33,31 @@ func (s Server) IsAuthenticated(next http.Handler) http.Handler {
 			// check session
 			session, err := s.cookieStore.Get(r, "user_session")
 			if err != nil {
-				http.Error(w, http.StatusText(http.StatusUnauthorized), http.StatusUnauthorized)
+				s.log.Error().Err(err).Msgf("could not get session from cookieStore")
+				session.Values["authenticated"] = false
+
+				// MaxAge<0 means delete cookie immediately
+				session.Options.MaxAge = -1
+
+				session.Options.Path = s.config.Config.BaseURL
+
+				if err := session.Save(r, w); err != nil {
+					s.log.Error().Err(err).Msgf("could not store session: %s", r.RemoteAddr)
+					http.Error(w, err.Error(), http.StatusInternalServerError)
+					return
+				}
+				http.Error(w, err.Error(), http.StatusForbidden)
 				return
 			}
 
 			if session.IsNew {
-				http.Error(w, http.StatusText(http.StatusUnauthorized), http.StatusUnauthorized)
+				http.Error(w, http.StatusText(http.StatusNoContent), http.StatusNoContent)
 				return
 			}
 
 			// Check if user is authenticated
 			if auth, ok := session.Values["authenticated"].(bool); !ok || !auth {
-				http.Error(w, http.StatusText(http.StatusUnauthorized), http.StatusUnauthorized)
+				http.Error(w, http.StatusText(http.StatusForbidden), http.StatusForbidden)
 				return
 			}
 

web/src/api/APIClient.ts

@@ -89,10 +89,18 @@ export async function HttpClient<T = unknown>(
   case 401: {
     // Remove auth info from localStorage
     AuthContext.reset();
-  }
 
     // Show an error toast to notify the user what occurred
-    return Promise.reject(new Error(`[401] Unauthorized: "${endpoint}"`));
+    // return Promise.reject(new Error(`[401] Unauthorized: "${endpoint}"`));
+    return Promise.reject(response);
+  }
+  case 403: {
+    // Remove auth info from localStorage
+    AuthContext.reset();
+
+    // Show an error toast to notify the user what occurred
+    return Promise.reject(response);
+  }
   case 404: {
     return Promise.reject(new Error(`[404] Not found: "${endpoint}"`));
   }
@@ -105,6 +113,10 @@ export async function HttpClient<T = unknown>(
     }
     break;
   }
+  case 503: {
+    // Show an error toast to notify the user what occurred
+    return Promise.reject(new Error(`[503] Service unavailable: "${endpoint}"`));
+  }
   default:
     break;
   }